fix(cli): support --version from the installed package manifest - #304
Conversation
|
Verdict: APPROVE, with one Windows item to confirm on exact-head CI before merge. What the PR does. Root command gets Verified independently
Findings
Not verified. This worktree has no dependencies installed and I was constrained to read-only actions, so I did not run build, typecheck, the full suite, the fail-first run, or the mutant runs. Those remain builder claims. I did not execute anything on Windows. Repro for the Commander probe: Review notes are also saved at |
Independent Codex review — APPROVEReviewed exact head Inspected complete diff, CLAUDE.md, PLAN package/CLI sections, startup callers, module-relative manifest resolution, bundle layout, action dispatch, Windows junction/path handling, and Windows include-list coverage. Coverage limitation: action spies, network denial and filesystem checks do not directly detect invisible provider construction/config reads; static inspection found no startup introduced. This is not a requested change. Execution limitation: independently ran fixture preflight, blocked by |
|
APPROVE for exact head c465ba6, with exact-head Windows CI still the merge gate. Scope of the delta. The diff from 3574b5e touches only two files: the relocated-package test in The alternate-version assertion is preserved. The test still copies dist into a relocated directory, writes a manifest with version 9.8.7-version-test.1 beside it, writes a conflicting 1.2.3 manifest in the working directory, and asserts stdout is exactly the relocated version with empty stderr and exit 0. The stripped subprocess environment is byte-identical to the approved head. Only the dependency-provisioning lines changed. The fixture repair is sound. I verified the assumptions it rests on against the installed tree in the main checkout:
Earlier environment concern needs no change. The first-head Windows run passed the normal offline version, -V, and help subprocess tests under the identical stripped environment, and the relocated failure was a module-resolution error rather than anything environment-shaped. That is direct evidence the missing SystemRoot and TEMP variables do not affect these code paths. Leaving the env alone keeps the credential-free guarantee intact. Minor, non-blocking. The fix hardcodes the one current external startup dependency. If a future bundle change adds another external on the startup path, this test will fail on every platform with a clear ERR_MODULE_NOT_FOUND, which is the desired signal rather than a silent gap. The STATUS.md wording accurately describes the failure and repair. Execution limitations. I cannot run Windows here. This worktree has no node_modules or dist, so I did not re-execute the suite. I relied on the builder's reported green run and on reading the installed yoga-layout package and built bundle in the main checkout. The only thing that can confirm the Windows repair is the exact-head Windows job, which remains the gate. |
Independent Codex final-delta review — APPROVEExact head
Static review only: isolated read-only checkout lacks dependencies and built artifacts. Builder green-trio and earlier Windows failure are supplied evidence, not independently reproduced. Exact-head Windows CI remains the merge gate. |
Final-head CI diagnosis; one bounded diagnostic rerunAt Rerun evidence: base main |
Landing complete — post-merge CI verifiedSquash merge: Both main workflows completed successfully on that exact merge SHA, first attempt:
No linked issues awaited closure. The only other open PR is #305; it overlaps |
Summary
Continue the interrupted
fix/cli-versiontask, preserving its useful tests. Commander--version/-Vreads the validated installed CLI package manifest relative to its module, not cwd. It exits before action dispatch or provider/session startup. No version literal is copied into source or baked into the bundle.Coverage includes action non-dispatch, credential-free offline execution with no session files, help compatibility, and a relocated package with an alternate prerelease version and conflicting cwd manifest. The root-option allowlist explicitly admits --version and the Windows lane includes the new tests. Yoga embedded data: fetches are allowed by the offline guard; network remains denied.
Verification
pnpm build,pnpm typecheck,TMPDIR=/home/amit/agentrig-test-tmp.eEvVwx pnpm test: each exit 0; 247 files, 3,791 passing tests, four existing skips.Independent reviews and resolutions
Both tools reviewed in separate detached worktrees. One full pair reviewed 3574b5e; one bounded delta pair reviewed final head c465ba6. No endless broad rounds.
Exact-head CI
At c465ba6:
Authorization and landing
User direction for this exact task: “Open one PR, get independent Claude Code and Codex reviews, address material findings without endless broad review rounds, and merge after exact-head CI passes. Verify post-merge CI. This authorizes the merge.”
This is the single user-directed fix, not a roadmap band. Squash merge after final head recheck; post-merge receipts will be posted here.