chore(dev): dev environment doctor + standardized dev commands + CI uv migration - #72
chore(dev): dev environment doctor + standardized dev commands + CI uv migration#721012839419a-alt wants to merge 2 commits into
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds a profile-based environment validator, root npm commands for development and Docker workflows, updated Node.js and development instructions, and uv-based backend and migration CI execution. ChangesDevelopment tooling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant npm_run_doctor
participant dev_environment_mjs
participant ProfileRules
participant process
Developer->>npm_run_doctor: run environment diagnostics
npm_run_doctor->>dev_environment_mjs: pass CLI arguments and environment
dev_environment_mjs->>ProfileRules: validate selected profiles
ProfileRules-->>dev_environment_mjs: return notes and errors
dev_environment_mjs->>process: exit 0 when ready or 1 when errors exist
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
150-151: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake CI fail on lockfile drift.
If
pyproject.tomlanduv.lockdiverge,uv sync --extra devcan update the lock in the runner instead of failing. CI can then test a dependency resolution that was not reviewed. Useuv sync --locked --extra devin both jobs.uvdocuments--lockedas the fail-on-update mode. (docs.astral.sh)Suggested change
- run: uv sync --extra dev + run: uv sync --locked --extra devRun
uv lock --checkbefore merge.Also applies to: 244-245
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 150 - 151, Update both “Install dependencies” steps in the CI workflow to run uv sync with the --locked flag alongside --extra dev, ensuring lockfile drift fails CI instead of updating uv.lock. Add a uv lock --check step before dependency installation or merge validation as requested.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 147-148: Update both `astral-sh/setup-uv` references in the CI
workflow to immutable full-length commit SHAs, retaining comments that identify
the corresponding v7 release versions.
In `@scripts/dev-environment.mjs`:
- Around line 136-138: Remove the unconditional Docker Compose validation from
the default doctor checks in the relevant command flow, and update the shared
check path used by npm run check so it does not require Docker by default. Move
the existing validation behind a Docker-specific command or profile, or
explicitly add Docker Compose to the documented base prerequisites if it remains
required.
In `@scripts/dev-environment.test.mjs`:
- Around line 20-23: Update the child-process environment in
scripts/dev-environment.test.mjs at lines 20-23, 28-30, and 38-40 to use a
controlled environment containing only required runtime lookup variables such as
PATH. Ensure inherited TASK_EXECUTOR, CHROME_SUFFIX, and AGENT_ADVERTISE_URL
cannot affect the fixture tests, so the core fixture supplies the complete
configuration and the empty or missing fixture values are consistently rejected.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 150-151: Update both “Install dependencies” steps in the CI
workflow to run uv sync with the --locked flag alongside --extra dev, ensuring
lockfile drift fails CI instead of updating uv.lock. Add a uv lock --check step
before dependency installation or merge validation as requested.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bd2358fe-54d1-4e15-813d-8436d002da48
📒 Files selected for processing (7)
.github/workflows/ci.yml.nvmrcREADME.mddocs/ptt-acceptance.mdpackage.jsonscripts/dev-environment.mjsscripts/dev-environment.test.mjs
| - name: Setup uv | ||
| uses: astral-sh/setup-uv@v7 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
GitHub Actions security hardened runners use full-length commit SHA for third-party actions
💡 Result:
Pinning GitHub Actions to a full-length 40-character commit SHA is a security best practice for mitigating supply chain risks, particularly for third-party actions [1][2][3]. Because tags (e.g., @v1) and branches (e.g., @main) are mutable references, they can be re-pointed by a compromised account to execute malicious code [2][3][4]. Pinning to a specific, full-length SHA ensures the code remains immutable, protecting workflows from silent, unauthorized updates [1][2][4]. Key aspects of this practice include: 1. Enforcement and Governance: Organizations can now mandate SHA pinning via GitHub's allowed actions policy [5][6]. When enabled, any workflow attempting to use an action not pinned to a full commit SHA will fail [5][6]. This policy evaluates the entire dependency tree, meaning nested sub-actions must also be pinned [6]. 2. Maintaining Updates: To ensure security patches are still received, it is standard practice to append the human-readable version as a trailing comment (e.g., uses: actions/checkout@<40-char-sha> # v4.2.0) [2][3][4]. Tools like Dependabot and Renovate are designed to parse these comments, allowing them to continue opening pull requests to update the SHA while maintaining the pinning requirement [2][3][4]. 3. Implementation: - Always use the full 40-character hexadecimal commit SHA, not a shortened version [3][4][7]. - For Docker-based actions, use the immutable @sha256 digest [4][7]. - You can automate the enforcement of these requirements by using third-party workflow checks, such as step-security/github-actions-ensure-sha-pinned-actions, which will fail the build if unpinned actions are detected [8][4]. - While official GitHub-owned actions (actions/*) are considered lower risk, pinning them as well is recommended for consistency and a more rigorous security posture [3][4].
Citations:
- 1: https://github.com/github/docs/blob/962a1c8dccb8c0f66548b324e5b921b5e4fbc3d6/content/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions.md
- 2: https://starsling.dev/best-practices/github-actions/pin-action-shas
- 3: https://tomodahinata.com/en/blog/dependabot-github-actions-sha-pinning-supply-chain-security-guide
- 4: https://safeguard.sh/resources/blog/how-to-pin-github-actions-to-shas-correctly
- 5: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 6: https://www.romainlespinasse.dev/posts/github-actions-commit-sha-pinning/
- 7: https://docs.ozarksecuritylabs.com/supply-chain/tier-1-baseline/sha-pin-actions/
- 8: https://github.com/step-security/github-actions-ensure-sha-pinned-actions
🌐 Web query:
astral-sh setup-uv releases v7 Git tag
💡 Result:
The repository astral-sh/setup-uv does not have a single v7 Git tag; rather, it uses a series of versioned tags starting with the prefix "v7" (e.g., v7.1.1, v7.6.0) [1][2]. The "v7" range in the repository history represents a collection of commits and releases preceding later major versions, such as v8 and v9 [1][3][4][5]. As of August 8, 2026, the current major release series for setup-uv is v9 [1][5]. The project has moved away from publishing major and minor tags (such as @v7, @v8, or @v9) to increase supply chain security, instead recommending the use of specific immutable tags (e.g., v9.0.0) or full Git hashes for pinning in GitHub Actions workflows [5][6]. For installing uv in GitHub Actions, the official recommendation is to use the astral-sh/setup-uv action [7]. A standard implementation example is [7]: - uses: astral-sh/setup-uv@c771a70 # v9.0.0 with: version: "0.12.2" The uv tool itself uses a different versioning scheme (e.g., v0.12.2) [8][9] and is distinct from the setup-uv action versioning [1].
Citations:
- 1: https://github.com/astral-sh/setup-uv/releases
- 2: https://github.com/astral-sh/setup-uv/releases/tag/v7.1.1
- 3: astral-sh/setup-uv@v5...v7
- 4: astral-sh/setup-uv@v2...v7
- 5: https://github.com/astral-sh/setup-uv/releases/tag/v9.0.0
- 6: https://github.com/astral-sh/setup-uv/releases/tag/v8.0.0
- 7: https://docs.astral.sh/uv/guides/integration/github/
- 8: https://github.com/astral-sh/uv/releases
- 9: https://github.com/astral-sh/uv/releases/tag/0.12.2
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Workflow setup-uv references:"
rg -n "astral-sh/setup-uv@|name: Setup uv" .github/workflows/ci.yml || true
echo
echo "Nearby references:"
if [ -f .github/workflows/ci.yml ]; then
sed -n '135,155p;234,246p' .github/workflows/ci.yml
fiRepository: 2233admin/opencli-Razormind
Length of output: 1118
Security Misconfiguration (CWE-494): Download of Code Without Integrity Check
Reachability: External
Pin both setup-uv references to full commit SHAs.
astral-sh/setup-uv@v7 is a mutable reference, so workflow runs can execute unreviewed action code if it moves. Use a specific release tag or full-length commit SHA instead, for example astral-sh/setup-uv@<full-commit-sha> # v7.x.y.
Also applies to: 241-242
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 147 - 148, Update both
`astral-sh/setup-uv` references in the CI workflow to immutable full-length
commit SHAs, retaining comments that identify the corresponding v7 release
versions.
| checkCommand('uv', ['--version'], 'uv') | ||
| checkCommand('uv', ['lock', '--check'], 'uv lock') | ||
| checkCommand('docker', ['compose', '--env-file', envFile, '-f', 'docker-compose.yml', '-f', 'docker-compose.build.yml', 'config', '--quiet'], 'Docker Compose configuration') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not require Docker Compose in the default source-development check.
Line 138 runs Docker Compose validation for every npm run doctor invocation. The documented source-development prerequisites do not include Docker, but the documented setup runs npm run doctor. A developer without Docker cannot complete the documented setup. npm run check has the same dependency.
Run Compose validation only in a Docker-specific command or profile. Otherwise, list Docker Compose as a base prerequisite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/dev-environment.mjs` around lines 136 - 138, Remove the unconditional
Docker Compose validation from the default doctor checks in the relevant command
flow, and update the shared check path used by npm run check so it does not
require Docker by default. Move the existing validation behind a Docker-specific
command or profile, or explicitly add Docker Compose to the documented base
prerequisites if it remains required.
| const output = execFileSync(process.execPath, [script, `--env-file=${envFile(validCore)}`], { | ||
| cwd: root, | ||
| encoding: 'utf8', | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a controlled child environment for environment-fixture tests.
The validator merges process.env after the selected environment file. Each test inherits the parent environment. A developer setting TASK_EXECUTOR=celery, CHROME_SUFFIX=-chrome, or AGENT_ADVERTISE_URL can change the expected result.
scripts/dev-environment.test.mjs#L20-L23: pass a controlled environment so the core fixture is the complete configuration source.scripts/dev-environment.test.mjs#L28-L30: omit inheritedCHROME_SUFFIXso the test always rejects the empty fixture value.scripts/dev-environment.test.mjs#L38-L40: omit inheritedAGENT_ADVERTISE_URLso the test always rejects the missing fixture value.
Preserve only runtime lookup variables that the child process needs, such as PATH.
📍 Affects 1 file
scripts/dev-environment.test.mjs#L20-L23(this comment)scripts/dev-environment.test.mjs#L28-L30scripts/dev-environment.test.mjs#L38-L40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/dev-environment.test.mjs` around lines 20 - 23, Update the
child-process environment in scripts/dev-environment.test.mjs at lines 20-23,
28-30, and 38-40 to use a controlled environment containing only required
runtime lookup variables such as PATH. Ensure inherited TASK_EXECUTOR,
CHROME_SUFFIX, and AGENT_ADVERTISE_URL cannot affect the fixture tests, so the
core fixture supplies the complete configuration and the empty or missing
fixture values are consistently rejected.
1 participant
Summary
dev:backend,sync:backend,test:backend,lint:backend:all,format:backend,check:backend,check,docker:up/down/logsnpm scripts (package.json)scripts/dev-environment.mjsCLI that validates the local dev environment (Chrome image suffix, agent registration URL, core profile), with unit tests (dev-environment.test.mjs)astral-sh/setup-uv@v7+uv sync --extra dev+uv runinstead of raw pip (matches the project's existinguv-based dev flow)Origin
This is T3 split out of draft PR #61 (7 commits → 4 independent themes). T3 =
8517817(standardize local dev commands) +e68cbbb(development environment doctor), cherry-picked onto current main (7838811, #60) with zero conflicts. T1 (→ #68) and T2 (→ #70) are already split; T4 (fixed API image,bc1891f) is intentionally NOT split because0.4.1-fixedis a locally-built image not present on the upstream registry — it stays as a local ops workaround.Verification
node --test scripts/dev-environment.test.mjs→ 3/3 passed (core profile, embedded Chrome rejection, HTTP agent registration rejection)Notes
setup-uv@v7+uv sync --extra devrequires the repo'suv.lock/.python-versionto be in sync; the job self-skips coverage gates as before.