feat(cli): reclaim `update` for the rules ledger, and record reconciliation by thecodedrift · Pull Request #196 · taskless/cli

thecodedrift · 2026-08-27T00:57:35Z

Reclaims taskless update for the job an agent actually needs after an upgrade, and gives it a place to record that the job is done.

The gap

Running the CLI migrates the .taskless/ layout and refreshes installed skills. That handles the directory, automatically.

Nothing handled the rules. A rewriter that now requires a fix, a rule whose matching semantics shifted under a new engine, a rule that could now be expressed with a newly supported language: none of that is a layout change, none is mechanical, and no migration will ever do it. So an agent that ran a migration and watched it succeed reasonably concluded the upgrade was finished. It was not.

`update` is the right word, and it was spent

taskless update meant "reinstall the skills non-interactively", reaching the same runNonInteractive as taskless init --no-interactive. A second name for one behavior, holding the word that describes the rules question.

With no flags it now serves the ledger, from the same renderer agent update uses, so taskless update and taskless agent update cannot drift into two different sets of instructions.

The ledger

One section per release, walked in order from where the project was last reconciled to the installed version. Cumulative rather than replaceable, since a later migration can depend on an earlier one having run.

A version with nothing to do says so. An agent cannot tell "nothing here" from "nobody wrote this section": an explicit no-op is a claim, a missing section is an unknown.

The first entry covers 0.11.0 and carries what a diff cannot reconstruct later:

fix is now required on a rewriter. verify names the offender. It cannot be auto-fixed, because fix is replacement text and therefore authorial intent, and a rewriter without one could never have run, so this surfaces a dead rule rather than breaking a working one.
Markdown, with its real shape. Block tree only; everything inside a line collapses to one opaque inline node. Two opposite failure modes: kind: link is a hard config error, exit 8, which aborts config parsing and takes every other rule's report down with it, while a pattern like [$T]($U) parses, runs, exits 0 and matches nothing forever. The first was originally recorded as a silent zero-match; it took a verified 0.45.2 binary to establish otherwise.
sg run --lang accepts alias spellings, where 0.41.0 rejected them. Flag only, no rule file changes, recorded because the old divergence was documented as a thing to work around.
Matching semantics moved. Metavariable bindings no longer leak across nthChild siblings nor out of a negated not, and root metavariables now match comments. A valid, unchanged rule can match a different set of nodes with no error. With ast-grep matching-semantics differential: verify rules match the same nodes across an upgrade #184 deferred there is no differential tooling, so the ledger is the only thing that will tell an author to look.

`rules.` beside `install.`, not inside it

"install": { "cliVersion": "0.11.0", "targets": { … } },
"rules":   { "reconciledTo": "0.11.0",
             "engines": { "sg": "0.45.2", "vale": "3.18.0" } }

Two namespaces because they answer different questions and drift apart. install records how the scaffold got here and moves on a skills refresh. rules records what the rules are valid against and moves only when a walk completes.

Keying rule work off install.cliVersion would let an agent skip entries it never performed, and it would fail in the quiet direction: the walk reports nothing to do while the rules stay wrong.

rules.engines is recorded for the same reason it matters elsewhere. Engine version is what determines whether matching semantics moved under a rule, so storing it lets a later differential ask a concrete question instead of reconstructing one.

Engine versions advance only here. If an upgrade refreshed them, the field would always read "current" and the divergence it exists to expose would be invisible, which is precisely what disqualified install.cliVersion.

Validated, not trusted

--reconciledTo is checked twice: a version ahead of this build is rejected, since it carries no entries for it, and the marker never moves backwards. Neither guards against an agent that skipped sections, and the recipe says so outright rather than implying the CLI can tell.

Fresh projects and downgrades fall out of this: no marker means no history to walk, and a marker at or past the installed version means nothing to do.

`info --json` reports both

So the walk reads its start from a call it already makes. install.cliVersion was previously written and never read by anything; surfacing it turns it into provenance rather than dead weight.

Breaking

For anyone scripting taskless update, which no longer installs. taskless init --no-interactive is the replacement and is the same code path the old update called. Changeset is minor and says so.

Two tests asserted the old behavior. One moved to init --no-interactive, where it belongs. The other now pins that update installs nothing, because the two words are close enough that a future change could wire installing back in and nothing else would notice.

Verification

pnpm typecheck, pnpm lint, pnpm test (57 files, 938 tests), pnpm cli check (exit 0).

9 new tests, plus hand-verified end to end: recording writes both the version and the engines, info reflects it, 9.9.9 is rejected as having no entries, 0.1.0 is refused as backwards without clobbering the stored value, and install is left untouched by a reconciliation.

Worth a reviewer's eye

readManifest takes the .taskless directory, not the project root. Both of my call sites originally passed cwd, silently read a nonexistent file, and fell back to empty, so install.cliVersion reported null while the manifest plainly held a value. Typecheck was green throughout. Caught by checking output against the file rather than trusting the build, which is the same class of quiet failure this feature exists to prevent.

Fixes #180
Refs #178
Refs #184
Refs #162

The guard added in #185 caught the zero-turn run on #196, which is what it was for. It then reported `permission_denials=0` while the streamed job log carried `permission_denials_count: 6`, so it skipped naming the denied tools, and that zero was read as evidence that permissions were not the problem. Reporting the wrong number was worse than reporting nothing. MEASURED on run 33039882331: the saved execution file's result record omits `permission_denials_count`, while the streamed stdout carries it. Every other field matched exactly, including `total_cost_usd` to sixteen digits, so the guard found the right record and the file simply lacks that one field. Denials are now the maximum of the count field and the number of denial records found in the file, and the naming loop reads those records rather than re-deriving them behind a gate that could never open. The old code already had the scan; it sat inside `if denials:`, so a missing count made it unreachable. Also added: a structural census of the execution file, message types and counts only. Two zero-turn runs have now billed real inference, #182 at $1.08 and #196 at $2.20, and neither left anything to diagnose from afterwards. No message content, no tool inputs, nothing the model produced while reading an untrusted diff; those are the reason `show_full_output` stays off on a public repo, and none of it is needed to say what shape a run had. Verified against three fixtures: the #196 shape with the count omitted and denial records present now reports 2 denials and names them, where the old guard reported 0 and named nothing; the #182 shape with the count present is unchanged; a healthy run still exits 0.

…iation Running the CLI migrates the `.taskless/` layout and refreshes skills. That handles the DIRECTORY, automatically. Nothing handled the RULES: a rewriter that now requires a `fix`, a rule whose matching semantics shifted under a new engine, a rule that could now be expressed with a newly supported language. None of that is a layout change and no migration will ever do it, so an agent that ran a migration and watched it succeed reasonably concluded the upgrade was finished. It was not. `taskless update` used to be a second name for `init --no-interactive`, reaching the same `runNonInteractive`. That bought nothing and held the word describing the job an agent actually needs. With no flags it now serves the ledger, from the same renderer `agent update` uses so the two spellings cannot drift into two sets of instructions. The ledger is sequential, one section per release, cumulative rather than replaceable, since a later migration can depend on an earlier one. A version with nothing to do says so: an agent cannot tell "nothing here" from "nobody wrote this", and an explicit no-op is a claim while a missing section is an unknown. The 0.11.0 entry carries what a diff cannot reconstruct later, including that `kind: link` is exit 8 aborting config parsing rather than the silent zero-match first recorded, and that matching semantics moved in ways no local check can detect while #184 stays deferred. The marker lives in a new `rules` section beside `install`, not inside it. The two answer different questions and drift: `install` records how the scaffold got here and moves on a skills refresh, `rules` records what the rules are valid against and moves only when a walk completes. Keying off `install.cliVersion` would let an agent skip entries it never performed, and would fail in the quiet direction, reporting nothing to do while the rules stayed wrong. `rules.engines` records the ast-grep and Vale versions for the same reason: engine version is what determines whether semantics moved, so a later differential can ask a concrete question instead of reconstructing one. Engine versions advance ONLY here. If an upgrade refreshed them, the field would always read "current" and the divergence it exists to expose would be invisible, which is the exact failure that disqualified `install.cliVersion`. `--reconciledTo` is validated, not trusted: a version ahead of this build is rejected because it carries no entries for it, and the marker never moves backwards. Neither guards against an agent that skipped sections, which the recipe says outright rather than implying the CLI can tell. `info --json` reports both namespaces so the walk reads its start from a call it already makes. `install.cliVersion` was previously written and never read; surfacing it makes it provenance rather than dead weight. Two tests asserted `update` installs. One is rewritten onto `init --no-interactive`, which is where that behaviour lives; the other now pins that `update` installs NOTHING, because the two words are close enough that a future change could wire it back in and nothing else would notice. Fixes #180 Refs #178 Refs #184

thecodedrift · 2026-08-27T05:01:49Z

@claude /review

claude · 2026-08-27T05:02:05Z

Claude finished @thecodedrift's task in 7m 8s —— View job

Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no review threads, review summaries, or prior top-level comments (only the @claude /review trigger comment itself), so this is the first review of this PR — everything below is new.

Read prior review data
Understand the request (/review)
Gather full diff and context (git diff origin/main...HEAD, 10 files, +613/-63)
Run finder angles (correctness, removed-behavior, cross-file, reuse/simplification, conventions)
Verify candidates against the code
Post inline comments + summary

Findings (5 inline comments posted)

packages/cli/src/rules/reconcile-marker.ts:39-53 — versionCore's Number.parseInt(part, 10) || 0 silently coerces any non-numeric segment to 0. A malformed --reconciledTo value (typo, truncated interpolation, pasted SHA, empty string) compares as [0], sails past the "ahead of this build" guard, and gets written verbatim into rules.reconciledTo — no error surfaced. This undercuts the PR's own "Validated, not trusted" design goal, since neither of the two checks actually validates the value is a version at all.
packages/cli/src/rules/reconcile-marker.ts:76-95 — recordReconciliation never calls ensureTasklessDirectory (every other manifest writer does). writeManifest → writeFile doesn't create missing parent directories, so update --reconciledTo=X against a project with no .taskless/ yet throws a raw ENOENT surfaced as a generic INTERNAL_ERROR, even though update.txt's "Preconditions" section promises this case is handled — that promise is prose only, not enforced in code.
packages/cli/src/rules/reconcile-marker.ts:100-117 — reconciliationStart is exported but never called anywhere; dead code introduced by this diff.
packages/cli/src/rules/reconcile-marker.ts:1-2 — Import order violates .conventions/STYLEGUIDE-CODE.md's "Group and Order Imports" rule: ./capabilities (internal) precedes node:path (built-in), with no blank-line group separation from the internal imports on lines 4-7.
packages/cli/src/commands/init.ts:121-152 — --json is silently ignored on the no---reconciledTo (recipe-print) path; only the --reconciledTo branch consults args.json. Minor UX inconsistency, not documented in the flag description.

Everything else checked out: readManifest/writeManifest call sites (install/state.ts, commands/onboard.ts) correctly preserve the new rules field via round-tripping the read manifest; taskless update and taskless agent update genuinely share one renderer with identical options, matching the PR's stated goal; the backward/ahead version-ordering logic is otherwise sound for well-formed inputs; the readManifest-takes-directory-not-root pitfall flagged in the PR description is correctly avoided at both new call sites.

…recipe Five findings from review of #196, all verified before acting. A value that is not a version was written verbatim. `versionCore` coerces an unparseable segment to `0`, so `abc` parses as `[0]`, compares lower than any real version, and sails past BOTH guards into the manifest. A pasted SHA or a truncated interpolation would corrupt the marker with nothing reported, which is the opposite of the validation this command claims. The shape is now checked before either comparison runs. Bucketed low by the reviewer and treated as higher: it is a silent-corruption hole in the one property the change advertises, and the two guards it defeats are the reason the flag takes a value at all. A missing `.taskless/` died on a raw ENOENT reported as `INTERNAL_ERROR`. `readManifest` tolerates a missing file but `writeManifest` does not create parent directories. Checked rather than created, deliberately: creating it would record a reconciliation for a project that has no rules to reconcile, which is the marker claiming work that could not have happened. The recipe already stated this precondition and only the prose held it. `--json` was read only on the recording path, so `taskless update --json` printed plain prose with no sign the flag had done nothing. It now returns the recipe together with the installed version, the recorded marker, and where the walk should start. That start comes from `reconciliationStart`, which was exported and called by nothing. Wiring it beats deleting it for the same reason `route` reads `ghOwner` from `info` rather than shelling out to git: the walk boundary is computed once by the CLI instead of reasoned out of recipe prose by whoever is acting on it. `null` when there is nothing to walk, since never having recorded a reconciliation is not the same as being behind. Import order in `reconcile-marker.ts` now follows the styleguide: node built-ins, blank line, internal modules.

thecodedrift · 2026-08-27T05:23:31Z

Re: @claude[bot] — "Review complete — Review mode: incremental…"
#196 (comment)

All five verified, all five real, all five fixed in 5a77fcd. Threads replied to and resolved individually.

One was mis-bucketed, and I treated it as higher than low. --reconciledTo accepting non-version input is a silent-corruption hole in the one property this PR advertises. versionCore coerces an unparseable segment to 0, so abc parses as [0], compares lower than any real version, and defeats both guards on its way into the manifest verbatim. Those two guards are the reason the flag takes a value at all, so a value that walks past them is not a nit. Shape is now validated before either comparison runs, covered by a test over four such values asserting the marker stays unwritten.

The missing-.taskless/ case is fixed by checking, not by creating. ensureTasklessDirectory would have silenced the crash and been wrong: it would record a reconciliation for a project with no rules to reconcile, which is the marker claiming work that could not have happened. The recipe already stated that precondition and only the prose was holding it.

The dead export got wired rather than deleted. reconciliationStart now computes the walk boundary for the --json payload, which also fixes the ignored flag. Same argument as route reading ghOwner from info instead of shelling out to git: the answer is derived once by the CLI rather than reasoned out of prose by whoever acts on it. null when there is nothing to walk, since never having recorded a reconciliation is not the same as being behind.

Verified end to end: garbage rejected with the marker untouched, a scaffold-less project reporting INVALID_INPUT rather than INTERNAL_ERROR, and update --json returning {ok, topic, reconciledTo, installed, walk, recipe}.

pnpm typecheck, pnpm lint, 960 tests, pnpm cli check exit 0.

Worth noting this is the first review that reached this PR: two earlier attempts died at zero turns without posting. #197 fixes the guard that misreported why.

— AI Coding Agent

The comparison ignores the prerelease suffix and nothing said so in a way that would fail if someone changed it. That is the property most likely to be "corrected" by reaching for semver, which would be more standard and less right here. Our nightlies are valid semver (`0.11.0-20260827050231x45e9997`), and the spec ranks a prerelease BELOW its release. Under semver a project reconciled on the nightly is behind the identical release, so the walk would send it back through a ledger entry it already walked. The two are built from the same commit and carry the same entries, so only the numeric core is compared. Adopting semver would also mean a new runtime dependency on a published package, for one comparison, and then special-casing around its answer for the case above. Carrying both the dependency and the custom logic is worse than carrying the custom logic.

…recipe Five findings from review of #196, all verified before acting. A value that is not a version was written verbatim. `versionCore` coerces an unparseable segment to `0`, so `abc` parses as `[0]`, compares lower than any real version, and sails past BOTH guards into the manifest. A pasted SHA or a truncated interpolation would corrupt the marker with nothing reported, which is the opposite of the validation this command claims. The shape is now checked before either comparison runs. Bucketed low by the reviewer and treated as higher: it is a silent-corruption hole in the one property the change advertises, and the two guards it defeats are the reason the flag takes a value at all. A missing `.taskless/` died on a raw ENOENT reported as `INTERNAL_ERROR`. `readManifest` tolerates a missing file but `writeManifest` does not create parent directories. Checked rather than created, deliberately: creating it would record a reconciliation for a project that has no rules to reconcile, which is the marker claiming work that could not have happened. The recipe already stated this precondition and only the prose held it. `--json` was read only on the recording path, so `taskless update --json` printed plain prose with no sign the flag had done nothing. It now returns the recipe together with the installed version, the recorded marker, and where the walk should start. That start comes from `reconciliationStart`, which was exported and called by nothing. Wiring it beats deleting it for the same reason `route` reads `ghOwner` from `info` rather than shelling out to git: the walk boundary is computed once by the CLI instead of reasoned out of recipe prose by whoever is acting on it. `null` when there is nothing to walk, since never having recorded a reconciliation is not the same as being behind. Import order in `reconcile-marker.ts` now follows the styleguide: node built-ins, blank line, internal modules.

thecodedrift force-pushed the feat/180-agent-update-ledger branch from 615a611 to 6c95477 Compare August 27, 2026 01:14

thecodedrift mentioned this pull request Aug 27, 2026

chore(ci): count review denials from every source, and census the run #197

Merged

taskless deleted a comment from claude Bot Aug 27, 2026

thecodedrift force-pushed the feat/180-agent-update-ledger branch from 6c95477 to 66ead69 Compare August 27, 2026 05:01