fix(version): derive every CLI version surface from package.json + gate it in CI (VER-001) - #46
fix(version): derive every CLI version surface from package.json + gate it in CI (VER-001)#46yakimoto wants to merge 1 commit into
Conversation
…-001) Published @wave-av/cli@1.0.8 printed `1.0.0` from `wave --version`. The `--version` half was fixed in 1.0.9, but two hardcoded literals survived on the wire, so even a correct 1.0.9 release would have told the gateway it was 1.0.0: src/lib/api-client.ts:49 "X-Wave-CLI-Version": "1.0.0" src/commands/api/index.ts:34 "User-Agent": "wave-cli/1.0.0" The literal is the defect class, not the value. Bumping it to 1.0.9 would reproduce the bug at the next release, so this removes it instead. - NEW src/lib/version.ts: the single source of truth. CLI_VERSION and cliUserAgent() derive from package.json. Resolution walks UP to the nearest package.json rather than reading a fixed `../package.json`, because the dev layout (src/lib/version.ts) and the shipped bundle (dist/index.js) sit at different depths below the package root — a fixed relative path is correct in exactly one of them and silently wrong in the other. - src/cli.ts, api-client.ts, commands/api/index.ts now consume it. Verified in the built bundle: zero `wave-cli/1.0.0` and zero hardcoded X-Wave-CLI-Version remain. - src/cli.test.ts: three gates keyed on package.json — --version agreement, banner agreement, and a scan of src/ that fails on any new version literal outside a documented allowlist (config-file schema version, and the 0.0.0-unknown sentinel). - smoke-install.yml: adds a `unit` job so `npm test` runs on PRs at all (it previously ran ONLY in release.yml on a `v*` tag, so the version tests never gated a PR), and hardens the existing smoke step, which ran `wave --version` and discarded the output, to compare it and the banner against package.json. release.yml is deliberately untouched — PRs #17 and #45 own that file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
ⓘ Qodo reviews are paused because your workspace is out of credits. Ask your workspace admin to add credits to resume reviews. Manage billing |
There was a problem hiding this comment.
Sorry @yakimoto, this account has used its review budget of 2,500,000 diff characters for the last 7 days.
You can request another review in 18 hours and 57 minutes by commenting @sourcery-ai review.
|
Warning Review limit reachedNext included review available in 42 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 91 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (6)
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_696bbdca-e30e-4d83-922c-02387a00e309) |
Reviewer's GuideThe PR establishes package.json as the single source of truth for every CLI version surface, removes stale wire literals, adds source and packed-tarball regression checks, and makes those checks enforceable on pull requests through CI. Sequence diagram for runtime version resolution and outbound headerssequenceDiagram
participant CLI as CLI process
participant Version as version.ts
participant Package as nearest package.json
participant Gateway as Gateway
CLI->>Version: readOwnVersion()
Version->>Package: walk upward and read version
Package-->>Version: version
Version-->>CLI: CLI_VERSION
CLI->>Gateway: request with X-Wave-CLI-Version: CLI_VERSION
CLI->>Gateway: request with User-Agent: cliUserAgent()
Flow diagram for the VER-001 pull request gateflowchart TD
A[Pull request] --> B[Build and install dependencies]
B --> C[npm test\nsource version gate]
C --> D[Pack and install tarball]
D --> E[Compare wave --version to package.json]
E --> F[Clear banner-suppressing environment variables]
F --> G[Compare help banner to v plus package.json version]
G --> H{All surfaces agree?}
H -- Yes --> I[Pass]
H -- No --> J[Fail CI with VER-001 error]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a focused version-consistency fix that centralizes package.json version usage across CLI output and outbound metadata, with tests covering source and packed-install behavior. The remaining changes only strengthen CI enforcement, and all changed files are within the supplied author ownership scope. Not approved because:
Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
| try { | ||
| let dir = dirname(fileURLToPath(import.meta.url)); | ||
| const { root } = parse(dir); | ||
|
|
||
| for (;;) { | ||
| const candidate = join(dir, "package.json"); | ||
| if (existsSync(candidate)) { | ||
| const pkg = JSON.parse(readFileSync(candidate, "utf-8")) as { version?: unknown }; | ||
| if (typeof pkg.version === "string" && pkg.version.length > 0) { | ||
| return pkg.version; | ||
| } | ||
| } | ||
|
|
||
| if (dir === root) break; | ||
| const parent = dirname(dir); |
There was a problem hiding this comment.
💡 Edge Case: A malformed ancestor package.json aborts the walk-up entirely
The try/catch wraps the whole walk-up loop, so if any ancestor directory (not the CLI's own) contains a package.json with invalid JSON — plausible when installed nested inside another tool's node_modules or a monorepo workspace — JSON.parse throws and the function immediately returns UNKNOWN_VERSION instead of continuing to walk further up to the real package.json. Move the JSON.parse/read into a per-iteration try/catch (continue the loop on parse failure) so only a fatal error before the loop starts (e.g. fileURLToPath failing) falls through to the outer catch.
Catch parse errors per-directory so one bad ancestor package.json doesn't abort the whole resolution.:
for (;;) {
const candidate = join(dir, "package.json");
if (existsSync(candidate)) {
try {
const pkg = JSON.parse(readFileSync(candidate, "utf-8")) as { version?: unknown };
if (typeof pkg.version === "string" && pkg.version.length > 0) {
return pkg.version;
}
} catch {
// malformed package.json at this level; keep walking up
}
}
if (dir === root) break;
const parent = dirname(dir);
if (parent === dir) break;
dir = parent;
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| Authorization: `Bearer ${apiKey}`, | ||
| "Content-Type": "application/json", | ||
| "User-Agent": "wave-cli/1.0.0", | ||
| "User-Agent": cliUserAgent(), |
There was a problem hiding this comment.
💡 Quality: No direct test asserts the outbound header values at the call sites
cliUserAgent() and CLI_VERSION are unit-tested in isolation, but no test asserts that the actual User-Agent / X-Wave-CLI-Version headers built in commands/api/index.ts and lib/api-client.ts use them (e.g. via a mocked client/fetch). This is low-risk given the trivial substitution, but a quick assertion on the constructed headers object in each call site would close the gap the incident was specifically about (headers silently drifting from the printed version).
Was this helpful? React with 👍 / 👎
|
Note Automatic reviews are paused because your team has used its included automatic processing for this billing period (headroom scales with your seat count). You can still comment "Gitar review" to run one anytime, and automatic reviews resume on their own by October 1. Add seats for more headroom. Code Review 👍 Approved with suggestions 0 resolved / 2 findingsCentralizes CLI versioning in new 💡 Edge Case: A malformed ancestor package.json aborts the walk-up entirelyThe try/catch wraps the whole walk-up loop, so if any ancestor directory (not the CLI's own) contains a package.json with invalid JSON — plausible when installed nested inside another tool's node_modules or a monorepo workspace — Catch parse errors per-directory so one bad ancestor package.json doesn't abort the whole resolution.💡 Quality: No direct test asserts the outbound header values at the call sites📄 src/commands/api/index.ts:35 📄 src/lib/api-client.ts:50
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
VER-001 / ART-001 — CLI version truth
Criterion: package.json version ==
wave --version== help banner == outbound version headers, enforced by a gate that fails on disagreement.The defect
Published
@wave-av/cli@1.0.8prints1.0.0. Reproduced in a clean room against the real registry:The
--versionhalf was fixed onmain(package.json is now 1.0.9), but two hardcoded literals survived on the wire, so even a correct 1.0.9 release would still have told the gateway it was 1.0.0:src/lib/api-client.ts:49"X-Wave-CLI-Version": "1.0.0"src/commands/api/index.ts:34"User-Agent": "wave-cli/1.0.0"Both were confirmed present in the built bundle of both the published 1.0.8 tarball and a fresh 1.0.9 pack.
Separately, the version tests never ran on a PR.
git grep -n 'npm test' -- .githubonmainreturns exactly one hit —release.yml:67, which ison: push: tags: v*.smoke-install.ymldoes run on PRs, but its version step rannpx wave --versionand discarded the output, so a build printing any version at all passed it. That is precisely how 1.0.8 shipped.The fix
A hardcoded version literal is the defect class, not the value — updating it to
1.0.9reproduces the bug at the next release. So the literals are removed, not corrected.src/lib/version.ts— the single source of truth.CLI_VERSIONandcliUserAgent()derive from package.json. Resolution walks up to the nearest package.json rather than reading a fixed../package.json: in development this module issrc/lib/version.ts(two levels below the package root) while the shipped bundle isdist/index.js(one level below it), so a fixed relative path is correct in exactly one layout and silently wrong in the other. This is verified against a real installed tarball below, not assumed.src/cli.ts,src/lib/api-client.ts,src/commands/api/index.tsall consume it. Post-build bundle check:grep -c 'wave-cli/1\.0\.0' dist/index.js->0, andX-Wave-CLI-Versionnow readsCLI_VERSION.src/lib/config/schema.ts:11(that"1.0.0"is the on-disk config file schema version and must not track releases) andtemplates/*/package.json(scaffold output).The proving gate
Three arms, each demonstrated failing before passing.
1. Source gate —
src/cli.test.ts. 11 tests -> 15. Adds--versionagreement, banner agreement (via the only reachable path — clear the env varsdetectEnvironment()keys on, thenprogram.helpInformation()), acliUserAgent()assertion, and a scan ofsrc/**/*.tsthat fails on any x.y.z literal outside a documented, reviewable allowlist.Injecting the original defect back makes it fail with exactly the two real offenders:
Drifting only the banner fails it independently:
AssertionError: expected '1.0.0' to be '1.0.9'.Restored:
Test Files 4 passed (4) / Tests 15 passed (15).2. End-to-end — real packed tarball, clean-room install. This is what proves the depth-independent package.json walk actually works from
node_modules/@wave-av/cli/dist/index.js:3. CI gate —
.github/workflows/smoke-install.yml. Adds aunitjob sonpm testruns on PRs at all, and rewrites the version step to compare against package.json. I executed the step's actual shell (extracted from the YAML) against three builds:VER-001: package.json == --version == banner == 1.0.9, exit 0::error::VER-001: installed CLI reports '1.0.0' but package.json says '1.0.9', exit 1::error::VER-001: help banner reported 'v1.0.0', expected 'v1.0.9', exit 1One subtlety worth review: the CLI suppresses its banner under CI/agent env vars, so the banner check clears all eleven that
detectEnvironment()reads. Without that it would assert against a banner that was never printed and pass for the wrong reason. It compares the extracted version for equality rather than grepping for a substring, so a missing banner fails rather than quietly passing.Scope / collisions
.github/workflows/release.ymlis deliberately untouched — open PRs #17 and #45 both own it. Two release-side items therefore belong to those PRs, not this one:release.ymlpublishes to npm but has nogh release createstep, which is why GitHub Releases stop at v1.0.0 while eight npm versions exist (git ls-remote --tags originreturns exactly one tag). It should addgh release create "$GITHUB_REF_NAME" --generate-notes --verify-tagwithcontents: writescoped to that job.release.yml:82-88already compares--versionto package.json; the banner half added here is worth mirroring there.The CHANGELOG's own compare links currently 404 as a result (
compare/v1.0.8...v1.0.9-> 404).Verified / not verified
origin/main(70e0ad8). Type-check error count is 148 on this branch and 148 atorigin/main— unchanged, none in any file this PR touches, andtscis not wired into any workflow.npm run lintfails identically atorigin/main(noeslint.config.*in the repo) — pre-existing, untouched here.33821620422) — the newunitjob and the hardened smoke step both passed, on Node 20 and Node 22:The banner half passing on GitHub Actions is the notable one: it confirms the env-var clearing works on a real runner, where the banner is otherwise suppressed.
release.yml, which this PR does not touch.Publishing (operator, named-floor — not done here)
This PR does not publish and does not tag. npm
latestremains 1.0.8, so the fix is not live for users until 1.0.9 is published. Blocker from recon, re-confirmed:gh secret list --repo wave-av/clishows onlyOPENAI_KEY— noNPM_TOKEN— sorelease.yml's token fallback is inert and it depends entirely on npm OIDC trusted publishing, which I could not verify is registered.Note for the operator: the Doppler
NPM_TOKENcould not publish@wave-avpackages. A registry 404 on PUT means the token lacks org publish rights, not a missing package — do not read that 404 as "the package does not exist."After merge, one of:
Rollback
git revert 91093d5. The change is source-only plus one workflow file; it adds no dependency, alters no runtime behaviour beyond the reported version string, and touches no published artifact. Reverting restores the two literals and the previous CI step exactly.🤖 Generated with Claude Code