Skip to content

fix(cli): derive --version from package.json to end version-stamp drift - #141

Merged
yingyangxu2026 merged 1 commit into
mainfrom
fix/134-cli-version-drift
Jul 23, 2026
Merged

fix(cli): derive --version from package.json to end version-stamp drift#141
yingyangxu2026 merged 1 commit into
mainfrom
fix/134-cli-version-drift

Conversation

@sophiej-story

Copy link
Copy Markdown
Collaborator

Summary

Closes #134. The published cdr-cli printed a stale version for --version (the 0.2.2 binary reported 0.1.2), because apps/cli/src/index.ts hardcoded .version("0.1.2") into commander. The release workflow bumps only the four package.json manifests, so the npm-level version was correct but the binary never knew its own version.

The version is now derived from package.json at runtime, so the stamp cannot drift from the published version again. This is issue #134's Option 1, kept compatible with dual (ESM + CJS) publishing via a tshy dialect polyfill rather than dropping the CJS build or generating a file at build time.

Approach

  • src/version.ts (ESM): createRequire(import.meta.url)("@piplabs/cdr-cli/package.json").version. Self-referencing through the package's own exports map resolves correctly from both src/ (tsx dev) and dist/esm/ without a depth-fragile relative path.
  • src/version-cjs.cts: the CommonJS counterpart using native require. tshy's -cjs.cts dialect-polyfill convention swaps this in for the CJS build only, where import.meta won't compile. A //@ts-ignore on the ESM file's import.meta line is required because tshy still type-checks it during the CJS pass — this is the documented convention.
  • src/index.ts: .version("0.1.2").version(version).

Why this survives the next release

The release workflow's bump step runs npm pkg set version on apps/cli (it's in the PACKAGES list) and nothing else, then rebuilds before publish via prepublishOnly. Both build dialects read the version at runtime from that freshly-bumped manifest, so a bump-only change flows to the binary automatically — verified by simulation below.

Test plan

  • pnpm lint (tsc --noEmit) and pnpm build (tshy) clean
  • --version reports 0.2.2 from all run modes: built ESM (dist/esm/index.js, the bin), built CJS (dist/commonjs/index.js), and dev (tsx src/index.ts)
  • Future-bump simulation: npm pkg set version=9.9.9 on package.json only (no source edits), rebuild → ESM/CJS/dev all report 9.9.9; restored to 0.2.2 after
  • Packaged-artifact check: pnpm pack → installed the tarball into a clean project → ./node_modules/.bin/cdr-cli --version reports the correct version (reproduces the environment where 0.2.2 failed)
  • New integration case in apps/cli/__integration__/cli.test.ts asserts the binary's --version equals package.json (network-independent; guards against reintroducing a hardcoded literal)

Notes

  • The --version regression is caught by the integration suite (source tree) and by post-release verification (published tarball, which is what caught it last time). Neither gates the publish itself — this PR makes the drift structurally impossible rather than adding a pre-publish gate. A release-time assertion can be added separately if we want.
  • The new integration assertion exercises the ESM build, which is the only dialect bin.cdr-cli points at — i.e. the only version surface a CLI user can reach. The CJS --version path is verified manually but is not consumer-reachable given the current package shape (no require-able version API; bin is a literal ESM path, not an exports-conditional entry).

@jinn-agent

jinn-agent Bot commented Jul 21, 2026

Copy link
Copy Markdown

The fix is structurally sound and the approach is well-reasoned. Two issues worth verifying before merge:

  1. exports map dependency (both version.ts and version-cjs.cts): require("@piplabs/cdr-cli/package.json") is a self-referencing sub-path import. In Node.js 12+ this throws ERR_PACKAGE_PATH_NOT_EXPORTED unless "./package.json": "./package.json" appears in the package's exports map. The diff doesn't show apps/cli/package.json, so this constraint can't be confirmed from the review. The pack+install test in the test plan would catch a missing entry, but CI should guard this too.

  2. Integration test placement / misleading "no network, no wallet" comment: The new describe("binary metadata") block sits in the same file as the chain-mutation suite and shares its module-level beforeAll, which deploys a smart contract and requires CDR_TEST_PRIVATE_KEY, CDR_API_URL, and CDR_RPC_URL. The test will not run in a network-less environment despite the comment claiming otherwise, and the 30 s timeout reflects the blockchain wait rather than the actual work of the test.

No other high-confidence issues found.


Review iteration 1 · Commit fd7ddfd · 2026-07-21T07:48:29Z

@yingyangxu2026
yingyangxu2026 merged commit d861218 into main Jul 23, 2026
15 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cdr-cli --version reports stale hardcoded version (0.2.2 binary prints 0.1.2)

2 participants