chore(codex): bootstrap tests and docs defaults#10
Conversation
- Add dev:lean workflow with temporary Cargo and Vite cache locations - Add clean:heavy and clean:all scripts for targeted and full cleanup - Document normal vs lean dev tradeoffs and cleanup usage in README - Update npm scripts to call local tool binaries reliably in this environment Tests: ./scripts/verify.sh (fails in tauri smoke due ':' in workspace path)
Summary of ChangesHello @saagar210, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request bootstraps new repositories with essential quality and development practices. It introduces global enforcement for testing and documentation, ensuring that all new projects adhere to a high standard from inception. Additionally, it provides utilities for efficient local development by managing build artifacts and offering flexible cleanup options. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - uses: actions/setup-node@v5 | ||
| with: | ||
| node-version: 22 | ||
|
|
||
| - uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 9 | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Policy checks | ||
| run: node scripts/ci/require-tests-and-docs.mjs | ||
|
|
||
| - name: Verify commands | ||
| run: bash .codex/scripts/run_verify_commands.sh | ||
|
|
||
| - name: Diff coverage | ||
| run: | | ||
| python -m pip install --upgrade pip diff-cover | ||
| diff-cover coverage/lcov.info --compare-branch=origin/main --fail-under=90 | ||
|
|
||
| - name: Upload test artifacts on failure | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: test-artifacts | ||
| path: | | ||
| playwright-report/ | ||
| test-results/ | ||
| coverage/ |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
In general, to fix this class of problem you explicitly define a permissions block at the workflow root or for the specific job, granting only the minimal scopes required (often contents: read for typical CI tasks). This prevents unintended broad access from inherited defaults and makes the workflow’s needs clear.
For this workflow, the simplest and best fix without changing existing functionality is to add a job-level permissions block under jobs.quality, setting contents: read. None of the steps require writing to the repository or modifying issues/PRs via API; they only need to read the code and upload artifacts. GitHub-hosted runners always have network/FS access needed for dependency installation and diff analysis; these do not require extra GITHUB_TOKEN scopes. So we will edit .github/workflows/quality-gates.yml, and directly under quality: (before runs-on:) add:
permissions:
contents: readNo additional imports, methods, or definitions are needed since this is a YAML workflow configuration change only.
| @@ -7,6 +7,8 @@ | ||
|
|
||
| jobs: | ||
| quality: | ||
| permissions: | ||
| contents: read | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 |
Codacy's Analysis Summary39 new issues (≤ 0 issue) Review Pull Request in Codacy →
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a74d788805
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| version: 9 | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile |
There was a problem hiding this comment.
Use a lockfile-compatible install command in quality gate
This job installs dependencies with pnpm install --frozen-lockfile, but the repository only contains package-lock.json (no pnpm-lock.yaml), so pnpm exits with ERR_PNPM_NO_LOCKFILE before any checks run. In practice this makes the new quality-gates workflow fail on every run unless a pnpm lockfile is added or the install command is switched to an npm-compatible path.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,7 @@ | |||
| pnpm lint | |||
There was a problem hiding this comment.
Replace verify command list with existing repo scripts
The verification runner executes .codex/verify.commands verbatim, and the first entry is pnpm lint even though package.json does not define a lint script (same issue for typecheck, test:integration, test:e2e:smoke, and docs:*). pnpm returns Command "lint" not found, so the gate stops immediately and cannot validate the repository as intended.
Useful? React with 👍 / 👎.
|
| .map((line) => line.trim()) | ||
| .filter(Boolean); | ||
|
|
||
| const isProdCode = (file) => /^(src|app|server|api)\//.test(file) && !/\.(test|spec)\.[cm]?[jt]sx?$/.test(file); |
There was a problem hiding this comment.
Codacy has a fix for the issue: ES2015 arrow function expressions are forbidden.
| const isProdCode = (file) => /^(src|app|server|api)\//.test(file) && !/\.(test|spec)\.[cm]?[jt]sx?$/.test(file); | |
| const isProdCode = function(file) { return /^(src|app|server|api)\//.test(file) && !/\.(test|spec)\.[cm]?[jt]sx?$/.test(file) }; |
|
|
||
| const isProdCode = (file) => /^(src|app|server|api)\//.test(file) && !/\.(test|spec)\.[cm]?[jt]sx?$/.test(file); | ||
| const isTest = (file) => /^tests\//.test(file) || /\.(test|spec)\.[cm]?[jt]sx?$/.test(file); | ||
| const isDoc = (file) => /^docs\//.test(file) || /^openapi\//.test(file) || file === 'README.md'; |
There was a problem hiding this comment.
Codacy has a fix for the issue: ES2015 arrow function expressions are forbidden.
| const isDoc = (file) => /^docs\//.test(file) || /^openapi\//.test(file) || file === 'README.md'; | |
| const isDoc = function(file) { return /^docs\//.test(file) || /^openapi\//.test(file) || file === 'README.md' }; |
| } | ||
| })(); | ||
|
|
||
| const baseRef = process.env.GITHUB_BASE_REF ? `origin/${process.env.GITHUB_BASE_REF}` : defaultBaseRef; |
There was a problem hiding this comment.
Codacy has a fix for the issue: ES2015 template literals are forbidden.
| const baseRef = process.env.GITHUB_BASE_REF ? `origin/${process.env.GITHUB_BASE_REF}` : defaultBaseRef; | |
| const baseRef = process.env.GITHUB_BASE_REF ? "origin/"+process.env.GITHUB_BASE_REF : defaultBaseRef; |
| .filter(Boolean); | ||
|
|
||
| const isProdCode = (file) => /^(src|app|server|api)\//.test(file) && !/\.(test|spec)\.[cm]?[jt]sx?$/.test(file); | ||
| const isTest = (file) => /^tests\//.test(file) || /\.(test|spec)\.[cm]?[jt]sx?$/.test(file); |
There was a problem hiding this comment.
Codacy has a fix for the issue: ES2015 arrow function expressions are forbidden.
| const isTest = (file) => /^tests\//.test(file) || /\.(test|spec)\.[cm]?[jt]sx?$/.test(file); | |
| const isTest = function(file) { return /^tests\//.test(file) || /\.(test|spec)\.[cm]?[jt]sx?$/.test(file) }; |
| const isTest = (file) => /^tests\//.test(file) || /\.(test|spec)\.[cm]?[jt]sx?$/.test(file); | ||
| const isDoc = (file) => /^docs\//.test(file) || /^openapi\//.test(file) || file === 'README.md'; | ||
| const isApiSurface = (file) => /^(src|app|server|api)\/.*(route|controller|handler|webhook|api|command)/.test(file); | ||
| const isArchChange = (file) => /^src\/(auth|db|infra|queue|events|architecture)\//.test(file) || /^infra\//.test(file); |
There was a problem hiding this comment.
Codacy has a fix for the issue: ES2015 arrow function expressions are forbidden.
| const isArchChange = (file) => /^src\/(auth|db|infra|queue|events|architecture)\//.test(file) || /^infra\//.test(file); | |
| const isArchChange = function(file) { return /^src\/(auth|db|infra|queue|events|architecture)\//.test(file) || /^infra\//.test(file) }; |
| const isProdCode = (file) => /^(src|app|server|api)\//.test(file) && !/\.(test|spec)\.[cm]?[jt]sx?$/.test(file); | ||
| const isTest = (file) => /^tests\//.test(file) || /\.(test|spec)\.[cm]?[jt]sx?$/.test(file); | ||
| const isDoc = (file) => /^docs\//.test(file) || /^openapi\//.test(file) || file === 'README.md'; | ||
| const isApiSurface = (file) => /^(src|app|server|api)\/.*(route|controller|handler|webhook|api|command)/.test(file); |
There was a problem hiding this comment.
Codacy has a fix for the issue: ES2015 arrow function expressions are forbidden.
| const isApiSurface = (file) => /^(src|app|server|api)\/.*(route|controller|handler|webhook|api|command)/.test(file); | |
| const isApiSurface = function(file) { return /^(src|app|server|api)\/.*(route|controller|handler|webhook|api|command)/.test(file) }; |
| const isDoc = (file) => /^docs\//.test(file) || /^openapi\//.test(file) || file === 'README.md'; | ||
| const isApiSurface = (file) => /^(src|app|server|api)\/.*(route|controller|handler|webhook|api|command)/.test(file); | ||
| const isArchChange = (file) => /^src\/(auth|db|infra|queue|events|architecture)\//.test(file) || /^infra\//.test(file); | ||
| const isAdr = (file) => /^docs\/adr\/\d{4}-.*\.md$/.test(file); |
There was a problem hiding this comment.
Codacy has a fix for the issue: ES2015 arrow function expressions are forbidden.
| const isAdr = (file) => /^docs\/adr\/\d{4}-.*\.md$/.test(file); | |
| const isAdr = function(file) { return /^docs\/adr\/\d{4}-.*\.md$/.test(file) }; |
There was a problem hiding this comment.
Code Review
This pull request aims to enforce testing and documentation policies by introducing new scripts for 'lean development', cleanup, and quality gate verification, along with policy documents. However, it introduces two critical command injection vulnerabilities: one in .codex/scripts/run_verify_commands.sh due to unsanitized cmd execution, and another in scripts/ci/require-tests-and-docs.mjs where baseRef is used directly in execSync. Other identified issues include a hardcoded user path, a potentially brittle regex, and the use of a login shell in the verification script which could affect reproducibility.
| [[ -z "$cmd" ]] && continue | ||
| [[ "$cmd" =~ ^# ]] && continue | ||
| echo ">>> $cmd" | ||
| if ! bash -lc "$cmd"; then |
There was a problem hiding this comment.
This line introduces a critical command injection vulnerability. The cmd variable, read directly from the VERIFY_FILE, is executed without sanitization, allowing arbitrary command injection and potential remote code execution. Additionally, using bash -l makes the script's behavior dependent on the user's login shell configuration, which can lead to non-reproducible behavior. The suggested change addresses both the command injection by properly quoting the command and improves reproducibility.
| if ! bash -lc "$cmd"; then | |
| if ! bash -lc "$(printf %q "$cmd")"; then |
| })(); | ||
|
|
||
| const baseRef = process.env.GITHUB_BASE_REF ? `origin/${process.env.GITHUB_BASE_REF}` : defaultBaseRef; | ||
| const diff = execSync(`git diff --name-only ${baseRef}...HEAD`, { encoding: 'utf8' }) |
There was a problem hiding this comment.
The baseRef variable, which can be controlled by the GITHUB_BASE_REF environment variable, is directly used in an execSync call without proper sanitization. This allows for command injection if an attacker can control the GITHUB_BASE_REF environment variable.
To remediate this, ensure that baseRef is properly sanitized before being used in the execSync command. Consider using a library that safely escapes shell arguments or strictly validating the input.
| const diff = execSync(`git diff --name-only ${baseRef}...HEAD`, { encoding: 'utf8' }) | |
| const diff = execSync(`git diff --name-only ${JSON.stringify(baseRef)}...HEAD`, { encoding: 'utf8' }) |
| "adapter": "node-ts", | ||
| "branch": "codex/bootstrap-tests-docs-v1", | ||
| "generated_at": "2026-02-17T05:41:48.224Z", | ||
| "generated_by": "/Users/d/.codex/scripts/bootstrap/global_tests_docs_bootstrap.mjs", |
There was a problem hiding this comment.
The generated_by field contains a hardcoded absolute path to a user's local directory. This is not portable and exposes details about the local machine setup. It's better to use just the script name to improve portability and avoid leaking local machine details.
| "generated_by": "/Users/d/.codex/scripts/bootstrap/global_tests_docs_bootstrap.mjs", | |
| "generated_by": "global_tests_docs_bootstrap.mjs", |
| .map((line) => line.trim()) | ||
| .filter(Boolean); | ||
|
|
||
| const isProdCode = (file) => /^(src|app|server|api)\//.test(file) && !/\.(test|spec)\.[cm]?[jt]sx?$/.test(file); |
There was a problem hiding this comment.
The isProdCode check relies on a whitelist of directories (src, app, server, api). This means if production code is added to a new directory (e.g., lib/, shared/), it will not be covered by this policy check, potentially allowing production code changes without corresponding tests. Consider making this more robust, for example by defining production code as any code that is not part of other explicit categories like tests, docs, or config.


What
Why
Testing
Risk / Notes