fix: report the nightly's own version, not the release it anticipates - #149
Conversation
…pates Installing a nightly wrote `install.cliVersion: "0.10.2"` into `.taskless/taskless.json` while the skills written beside it — same command, same run — pinned every invocation to `@taskless/cli-nightly@0.11.0-…`. The manifest attributed the install to a version that never performed it. The cause is an ordering the nightly design chose on purpose: the version is stamped when the publishable artifact is produced, and the committed manifest is deliberately left alone. But `__VERSION__` was baked from that committed manifest, so `package.json` was asked a question it cannot answer and answered confidently with the previous release. `resolveCliVersion` now returns the stamped version for a nightly and the committed one for every other target, and delegates to `resolveNightlyVersion` so the no-stamp refusal is shared rather than restated — a nightly that cannot name itself fails the build instead of falling back, since the fallback is the wrong answer that produced this. Reproduced first: one local nightly build reported v0.10.2 while pinning the nightly invocation, which is both halves of the report from a single artifact. Also confirmed the write path was not at fault — the wizard persists `getCliVersion()` and reads the previous value only to render the diff — so nothing was preserving a stale version. Corrects `--version`, the `%(CLI_VERSION)s` recipe header, the telemetry property, and `install.cliVersion` together; all four read this define. Manifests already written by a nightly are not corrected retroactively; the next install fixes them. Fixes #148 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms
…ts version Post-mortem guard for the bug in the parent commit. #148 shipped not because the two version-bearing defines were derived from different sources by design, but because nothing ever compared them: the invocation read TASKLESS_NIGHTLY_VERSION while the version read package.json, so one artifact announced v0.10.2 and sent every agent to v0.11.0-… . Nothing failed. The disagreement was visible only by comparing two outputs of the same build against each other, which is exactly what nobody does. The fix couples them by construction, so the check is tautological today. That is the point — it exists for the refactor that decouples them again: a second env var, a cached value, a default reintroduced "for local builds". Deriving from one source is not the same as being checked against it, which is precisely what this bug demonstrated. Asserts on the resolved defines rather than scanning the emitted bundle. The build already holds the structured values, and re-deriving them from generated text would be the weaker tool — the styleguide's "verify build output in the build" rule. Verified by simulating the decoupling: reverting the version to `pkg.version` reproduces #148 exactly and the build now refuses it, naming the issue and the env var both values derive from. Had this guard existed, the bug could not have been emitted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms
3324ec4 to
d983e4b
Compare
|
@claude /review |
|
Claude finished @thecodedrift's task in 3m 6s —— View job Review completeReview mode: incremental — read 0 prior review thread(s) before reviewing. The prior-review data contained no inline threads and no substantive prior review (Copilot's review failed with a quota-limit error, and the only comment was the trigger itself), so this is effectively the first full review of this PR.
Assessment: solid, well-tested fix. One inline finding (spec consistency, not code correctness). The core logic in
[New] One finding, posted inline on
Neither file appears in the proposal's "Impact" (unchanged) list, so this looks like a genuine gap rather than an intentional scope decision, and No other correctness, security, performance, or test-adequacy issues found. |
…from package.json Review caught a real gap: making a nightly report its stamped version falsified two requirements this change never touched. - `cli`: "CLI version is injected at build time" said `__VERSION__` is the version string read from `packages/cli/package.json`, full stop. Now scoped — the committed version for every target but nightly, which uses its stamp — with a scenario for the nightly case beside the existing one. - `analytics`: "All capture calls include standard properties" described `cliVersion` as the package version read from `package.json`, and its scenario said the same. That one is pointed: this change's own claim to fix the `cliVersion` telemetry property is exactly what the scenario described, so the spec asserted the behavior being removed. Both are recorded as MODIFIED deltas on the archived change, so the record matches what shipped rather than only the permanent specs agreeing. `openspec validate --strict` cannot catch a contradiction between two specs — it validates each in isolation — so nothing would have failed here. Swept the rest of `openspec/specs/` for the same claim; the remaining hits are skill-version sync and the ast-grep schema pin, which correctly mean the committed manifest and are untouched by this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms
The spec-consistency finding was correct and is fixed in b061eb6; thread replied to and resolved. It was worse than reported in one respect: Both corrections are also recorded as MODIFIED deltas on the archived change, so the change record matches what shipped rather than only the permanent specs agreeing. I swept the rest of Your note that — AI Coding Agent |
What
A nightly build now reports the version it is published as, rather than the released version its committed manifest declares — and the build refuses to emit a nightly whose two version-bearing values disagree.
The bug
Installing
@taskless/cli-nightly@0.11.0-20260823220841xd9dee3dwrote:…while the skills written beside it — same command, same run — pinned every invocation to
@taskless/cli-nightly@0.11.0-20260823220841xd9dee3d. The manifest attributed the install to a version that never performed it.That matters because
install.cliVersionis what answers "what installed this?" — and that question gets asked precisely when someone is running a nightly to reproduce unreleased behavior.Root cause
An ordering the nightly design chose on purpose. Per
cli-nightly-builds, the version is stamped when the publishable artifact is produced and the committed manifest is deliberately left unchanged. Butvite.config.tsbaked__VERSION__from that committedpackage.json. So the build asked a file a question it cannot answer, and got a confident wrong answer: the previous release.Reproduced from a single local artifact before changing anything —
I also confirmed the write path was not at fault:
wizard/index.tspersistsgetCliVersion()and readspreviousState.cliVersiononly to render the diff, so nothing was preserving a stale value.The fix
resolveCliVersion(environment, packageVersion)returns the stamped version for thenightlytarget and the committed version for every other. It delegates to the existingresolveNightlyVersion, so the no-stamp refusal is shared rather than restated — a nightly that cannot name itself fails the build instead of falling back, since that fallback is the wrong answer that produced this bug.The stamp was already computed once and already passed to the build as
TASKLESS_NIGHTLY_VERSION. This consumes a value that was there, rather than plumbing a new one.The post-mortem guard (second commit)
The bug was not that the two values had different sources by design — it is that nothing ever compared them. One artifact announced one version and sent every agent to another, and nothing failed. The disagreement was visible only by comparing two outputs of the same build against each other, which is exactly what nobody does.
assertVersionConsistencynow fails the build when a nightly's reported version and its embedded invocation name different versions.It is tautological today — the fix above couples both to one stamp — and that is deliberate. It exists for the refactor that decouples them again: a second env var, a cached value, a default reintroduced "for local builds." Deriving from one source is not the same as being checked against it, which is exactly what this bug demonstrated.
It asserts on the resolved defines, not the emitted bundle: the build already holds the structured values, and re-deriving them from generated text would be the weaker tool (the styleguide's "verify build output in the build").
Verified by reproducing the bug against it — reverting the version to
pkg.versionand rebuilding:Had the guard existed, #148 could not have been emitted.
Blast radius
Four things read this define, and all four were wrong on a nightly:
taskless --version, the%(CLI_VERSION)srecipe header, thecliVersiontelemetry property, andinstall.cliVersion. All four are corrected together.Unchanged:
packages/cli/package.json;nightly-pack.cjs; and theassert-skill-versionsbuild check, which compares skill frontmatter against the committedpackage.json— that is a claim about source files in the repository, not about what a build reports, and a nightly must not rewrite it.Not corrected retroactively: manifests already written by a nightly still record the anticipated release. The next install fixes them.
Verification
pnpm --filter @taskless/cli test— 699 passed, 52 filespnpm typecheck,pnpm lint— cleanpnpm openspec validate --all --strict— 24 passedTwelve new unit tests over an injected environment, matching how
build-target.tsis already tested: the nightly reports its stamp, a missing stamp throws naming the env var, prod/dev/self plus an empty environment keep the committed version, the guard rejects a mismatch and names the issue, and the guard does not constrain targets where a version pin is meaningless.Delivery shape
Single PR, two commits — the fix, then the guard that would have prevented it. The OpenSpec change is archived here, since a single PR lands spec, implementation, and archive together.
Fixes #148
🤖 Generated with Claude Code
https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms