fix(install): reclaim a stub frozen with another build's recovery command - #237
Conversation
|
@claude /review |
|
Claude finished @theCodeDrift's task in 4m 9s —— View job Review completeReview mode: incremental — read 0 prior review thread(s) before reviewing.
Files reviewed in full: Correctness. I traced
No correctness, security, or test-adequacy bugs found. The new tests ( One minor finding posted inline (not a bug): Branch: |
No correctness findings to act on, and the one minor finding (the duplicated I did re-verify the load-bearing property empirically rather than by reading the code, using real
Byte-stability also holds: a prod build of this branch and a prod build of Local verification on the branch is clean: — AI Coding Agent |
…mand Every reference stub outside `.taskless` ends with the line that makes a missing canonical file recoverable, and the invocation inside it was frozen at whichever build wrote the stub. `stubPredatesRecovery` matches only the build-independent tail, deliberately, so prod and a nightly do not treat each other's stubs as stale — but nothing else looked at the command, so no later install could correct it. Reproduced: nightly `init`, then a released `init` in the same directory. The canonical files and `install.cliVersion` revert to the release; the stub keeps `npx @taskless/cli-nightly@<pinned> init`, a version that may no longer be published, in the one line a reader reaches for when the canonical file is already gone. `stubRecoveryInvocationStale` reads the command back out of the stub body and compares it to this build's, asymmetrically: the released, version-free form is accepted by every build, anything else is reclaimed. That makes the released form a fixed point every build converges on and none moves away from, so a prod install reclaims a nightly-written stub exactly once and a nightly install afterwards leaves the result alone. The comparison reads the body rather than a new frontmatter field, so a released build's stub bytes are unchanged and correct installs need no migration rewrite. The whole nightly stub path had no coverage: `__TASKLESS_CLI__` is a compile-time define, and canonical-store.test.ts asserts `isProductionInvocation()`, so a nightly's invocation could not appear in any test. vitest now runs two projects, and `test/nightly/` builds stubs under a real nightly define. Fixes #227
da48fb4 to
84d2918
Compare
The defect
Every reference stub outside
.taskless(.claude/skills/taskless/SKILL.mdand friends) ends with the line that turns a missing canonical file from a dead end into a recoverable state:The invocation inside that sentence comes from
applyCliInvocation, so it is whatever the build that wrote the stub was. But nothing ever looked at it again.referenceNeedsRewriterewrites a stub when it is missing, a symlink, not a shim, missing the recovery tail, or has driftedname/description; the body probe,stubPredatesRecovery, matches only the build-independent fragment"to restore it, then read it.".That fragment is deliberate — matching the whole sentence would make a released build and a nightly treat each other's stubs as stale and rewrite on every install. The unconsidered cost is that the invocation is frozen at whichever build wrote the stub, and no later install can correct it.
The result is a recovery instruction naming a possibly-unpublished nightly, reachable only when the canonical file is already missing, which is exactly when it has to work.
It also violated
openspec/specs/cli-init/spec.md: two installs of different CLI versions SHALL write identical stubs.Reproduction
Before this change, the second install reported the reload notice and rewrote the canonical store —
install.cliVersionwent from0.11.0-nightly.20260901back to0.11.0— while the stub kept:permanently, however many released installs followed.
Why no test caught it
__TASKLESS_CLI__is a compile-time define, so whatevervite.config.tsresolves is what every test in the run sees.test/canonical-store.test.tsassertsisProductionInvocation()istrueoutright, so the entire suite ran under a released build where the recovery invocation carries no version and this divergence cannot be expressed, let alone asserted on. The nightly and self stub paths had no coverage at all.What changed
stubRecoveryInvocationStalereads the command back out of an existing stub's recovery sentence and compares it to this build's, asymmetrically:npx @taskless/cli initselfpath)That makes the released form a fixed point every build converges on and none moves away from. A released install reclaims a nightly-written stub exactly once; a nightly install afterwards leaves the result alone. Two different nightlies do rewrite each other, and should — the alternative is leaving a pin for a build that is not present, which is the defect itself.
Design notes:
.tasklessis byte-stable across releases" rule intact.RECOVERY_RUN/RECOVERY_AFTER_COMMAND/RECOVERY_TAILconstants, so the builder and the probe cannot drift apart.Verification that the ping-pong did not return
End-to-end, against real
build:nightlyandbuildartifacts in one directory, hashing the stub at each step:npx @taskless/cli-nightly@0.11.0-nightly.20260901 initnpx @taskless/cli init(reclaimed)86a883d…86a883d…86a883d…86a883d…86a883d…Step 4 is the one that matters: the nightly install reported zero stub writes over a released-build stub. After one reclaiming rewrite the file is stable under either build, in either order, indefinitely.
The unit and integration tests were also confirmed to fail with the
referenceNeedsRewritehook disabled and pass with it, so they bite.Test coverage
vite.config.tsnow declares two vitest projects, sovitest runexercises both defines in one command:cli— the existing suite, released define, unchanged behaviour.nightly—test/nightly/, built fromresolveCliInvocation({ TASKLESS_BUILD_TARGET: "nightly", … })so the define is whatbuild:nightlywould really emit rather than a lookalike string.New tests:
test/nightly/stub-recovery-invocation.test.ts(nightly writes its own pin; leaves a released stub alone through a realapplyInstallPlanround trip; reclaims another nightly's pin and aselfpath; is idempotent), plus released-side unit tests intest/canonical-store.test.tsand an end-to-end reclaim test intest/apply-install-plan.test.ts.Checks
pnpm build,pnpm typecheck,pnpm lint,pnpm --filter @taskless/cli exec vitest run(71 files, 1156 tests), andpnpm cli checkall clean —checkreports only the 4 pre-existingno-hedgingwarnings inroute.txt/onboard.txt.openspec/specs/cli-init/spec.mdgains the requirement text and two scenarios describing reclamation and the released form's immunity, since the existing requirement said detection must not depend on the writing build.Fixes #227