From 76f8a37853f44da7186e07970913d4863af1613d Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 26 Aug 2026 23:07:27 -0700 Subject: [PATCH 1/3] feat(cli): walk the ledger from 0.0.0 when no marker is recorded An absent `rules.reconciledTo` meant "nothing to walk", on the reasoning that a project created at the installed version has no history. That was right about new projects and wrong about every existing one. A project that predates the ledger has had NONE of its entries applied, so reading absence as up to date silently excused exactly the population the entries were written for: the 0.11.x entry would have reached nobody, because nothing has ever recorded a marker. Absent now means `0.0.0` and every section applies. New projects stay correct because `init` stamps the marker at creation, which is what makes the two distinguishable by construction rather than by guessing: present means accounted for, absent means predates the ledger. The stamp never overwrites, so re-running setup cannot reset a marker a real walk earned, and it swallows its own failures because the cost of missing it is one extra walk, which is the safe direction to be wrong in. `--reconciledTo=` becomes `--rules`. The value was never load-bearing: the CLI knows its own version, the only sensible endpoint of a walk is the installed one, and accepting a value only made it possible to claim a walk that did not finish. Removing it removes the two guards that existed to police the value and every way of supplying it wrongly, including the garbage-input hole found in review. The backwards guard stays, because an older CLI on the same project would otherwise rewind the marker. Free to change: `--reconciledTo` reached `main` but stable is still 0.10.2, so no consumer has it. Four tests policed a supplied value and are gone with it. The version-ordering cases they covered moved to a unit test of `reconciliationStart`, where they can actually be exercised: an older CLI and a nightly against its release both depend on which build is running, which a spawned process cannot simulate. That includes the nightly-equals-release case, which exists to stop someone "correcting" the comparison into semver. --- .changeset/ledger-baseline.md | 13 ++ packages/cli/src/agent/update.txt | 44 +++--- packages/cli/src/commands/init.ts | 30 ++-- packages/cli/src/rules/reconcile-marker.ts | 88 ++++++++--- packages/cli/test/reconcile-marker.test.ts | 142 ++++-------------- .../cli/test/reconciliation-start.test.ts | 57 +++++++ 6 files changed, 208 insertions(+), 166 deletions(-) create mode 100644 .changeset/ledger-baseline.md create mode 100644 packages/cli/test/reconciliation-start.test.ts diff --git a/.changeset/ledger-baseline.md b/.changeset/ledger-baseline.md new file mode 100644 index 00000000..3625e874 --- /dev/null +++ b/.changeset/ledger-baseline.md @@ -0,0 +1,13 @@ +--- +"@taskless/cli": patch +--- + +A project with no recorded rules marker now walks the ledger from the beginning, and `taskless update --rules` replaces `--reconciledTo=`. + +Previously an absent `rules.reconciledTo` meant "nothing to walk", on the reasoning that a project created at the installed version has no history. That was right about new projects and wrong about every existing one: a project that predates the ledger has had none of its entries applied, so reading absence as up to date silently excused exactly the population the entries were written for. The 0.11.x entry would have reached nobody. + +Absent now means `0.0.0`, so every section applies. New projects stay correct because `init` stamps the marker at creation, which is what makes the two distinguishable: present means accounted for, absent means predates the ledger. The stamp never overwrites an existing marker, so re-running setup cannot reset one a real walk earned. + +`--reconciledTo=` is replaced by the flag `--rules`, which stamps the running CLI's version. The value was never load-bearing: the CLI knows its own version, the only sensible endpoint of a walk is the installed one, and accepting a value only made it possible to claim a walk that did not finish. Removing it removes the two guards that existed to police it and every way of supplying it wrongly. The backwards guard remains, because an older CLI running on the same project would otherwise rewind the marker. + +The ledger heading is now `Migrating to 0.11.x`, since the entry describes the release series rather than one patch. diff --git a/packages/cli/src/agent/update.txt b/packages/cli/src/agent/update.txt index e88c5978..e2a4068c 100644 --- a/packages/cli/src/agent/update.txt +++ b/packages/cli/src/agent/update.txt @@ -1,4 +1,4 @@ -# Topic: update (CLI v%(CLI_VERSION)s / topic v2) +# Topic: update (CLI v%(CLI_VERSION)s / topic v3) ## You are here This is `update`. It tells you what an upgrade changed for the rules @@ -34,11 +34,16 @@ that you finished. `rules.engines.vale` for the engine versions the rules were built against. - **No `rules.reconciledTo` at all** means this project has never - recorded a reconciliation. That is NOT the same as being behind. A - project created at the installed version has no history to walk, so - do nothing except step 4. Only walk sections if you can see an - earlier version to walk FROM. + **No `rules.reconciledTo` at all** means this project PREDATES the + ledger, so treat it as `0.0.0` and walk every section. It does not + mean the project is new: a project this CLI set up has the field + stamped at creation, so an absent marker is the one case where none + of the entries below have ever been applied. + + `info --json` also reports `walk`, which is this decision already + made: `{ "from": ..., "to": ... }` when there is something to walk, + and `null` when there is not. Prefer it to re-deriving the boundary + yourself. 2. **Walk forward, in order.** Start at the section after `reconciledTo` and continue to the installed version. Sections are @@ -51,21 +56,24 @@ that you finished. 4. **Record that you finished.** Run: ``` - %(TASKLESS_CLI)s update --reconciledTo= + %(TASKLESS_CLI)s update --rules ``` - Only after the walk is complete. A partial or abandoned walk must not - be recorded: the next agent will start from what you wrote and skip - everything you did not do, and it will look like there was nothing to - do. The CLI rejects a version it has no entries for, and refuses to - move the marker backwards, but it cannot tell whether you actually - read the sections. + Only after the walk is complete. There is no version to pass: the CLI + stamps its own, because the only sensible endpoint of a walk is the + version you are running. + + A partial or abandoned walk must not be recorded. The next agent + starts from what you wrote and skips everything you did not do, and + it will look like there was nothing to do. Nothing can check this for + you: the CLI refuses to move the marker backwards, but it cannot tell + whether you read the sections. This also records the engine versions the rules are now valid against, which is what lets a later upgrade say what moved. ## The ledger -### Migrating to 0.11.0 +### Migrating to 0.11.x The vendored ast-grep moves from 0.41.0 to 0.45.2, and Vale is at 3.18.0. Four things follow for existing rules. @@ -117,11 +125,11 @@ tests while reporting differently on real code. ## Errors -With `--json`, `--reconciledTo` failures emit `{ ok: false, code, message }`: +With `--json`, `--rules` failures emit `{ ok: false, code, message }`: -| code | meaning | fix | -|-----------------|--------------------------------------------------|------------------------------------------| -| `INVALID_INPUT` | version is ahead of this CLI, or moves backwards | reconcile to the installed version | +| code | meaning | fix | +|-----------------|------------------------------------------------|--------------------------------------------| +| `INVALID_INPUT` | no `.taskless/`, or an older CLI would rewind the marker | set the project up, or upgrade first | ## See Also diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 27f80902..59729f88 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -25,6 +25,7 @@ import { import { recordReconciliation, reconciliationStart, + stampNewProjectRules, } from "../rules/reconcile-marker"; import { readManifest } from "../filesystem/migrate"; import { TASKLESS_DIRECTORY } from "../rules/vale/formats"; @@ -103,8 +104,8 @@ export const initCommand = defineCommand({ * rewriting after an engine or CLI upgrade. * * With no flags it serves the ledger recipe, so `taskless update` and - * `taskless agent update` are the same thing. With `--reconciledTo` it records - * that a walk completed. + * `taskless agent update` are the same thing. With `--rules` it stamps the + * walk as complete. */ export const updateCommand = defineCommand({ meta: { @@ -118,15 +119,16 @@ export const updateCommand = defineCommand({ alias: "d", description: "Working directory", }, - reconciledTo: { - type: "string", + rules: { + type: "boolean", description: - "Record that the ledger walk completed up to this CLI version", + "Stamp the rules as reconciled to this CLI, after completing the ledger walk", + default: false, }, json: { type: "boolean", description: - "Output as JSON: the recipe plus where the walk starts, or the recorded result with --reconciledTo", + "Output as JSON: the recipe plus where the walk starts, or the stamped result with --rules", default: false, }, anonymous: { @@ -138,10 +140,10 @@ export const updateCommand = defineCommand({ async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); - // No `--reconciledTo`: this is the teaching path. Serve the SAME recipe + // No `--rules`: this is the teaching path. Serve the SAME recipe // `agent update` serves, from the same renderer, so the two spellings // cannot drift into two different sets of instructions. - if (args.reconciledTo === undefined) { + if (!args.rules) { const telemetry = await getTelemetry(cwd); const recipe = getRecipe("update", { anonymous: args.anonymous, @@ -190,14 +192,14 @@ export const updateCommand = defineCommand({ const telemetry = await getTelemetry(cwd); try { - const result = await recordReconciliation(cwd, args.reconciledTo); + const result = await recordReconciliation(cwd); if (args.json) { console.log(JSON.stringify({ ok: true, ...result })); } else { console.log( result.previous === undefined - ? `Recorded: rules reconciled to ${result.reconciledTo} (ast-grep ${result.engines.sg}, Vale ${result.engines.vale}).` - : `Recorded: rules reconciled to ${result.reconciledTo}, was ${result.previous} (ast-grep ${result.engines.sg}, Vale ${result.engines.vale}).` + ? `Rules reconciled to ${result.reconciledTo} (ast-grep ${result.engines.sg}, Vale ${result.engines.vale}).` + : `Rules reconciled to ${result.reconciledTo}, was ${result.previous} (ast-grep ${result.engines.sg}, Vale ${result.engines.vale}).` ); } telemetry.capture("cli_rules_reconciled"); @@ -219,6 +221,12 @@ async function runNonInteractive( cwd: string ): Promise<{ commandsInstalled: boolean }> { await ensureTasklessDirectory(cwd); + // Stamp the rules marker on a project this CLI is setting up. That is what + // lets an ABSENT marker mean "predates the ledger" rather than "new": a + // project created here has no entries to walk, and one that never recorded + // anything has all of them. Without this the two are indistinguishable and + // the walk has to guess. + await stampNewProjectRules(cwd); const allSkills = getEmbeddedSkills(); const mandatoryNames = new Set(getMandatorySkillNames()); diff --git a/packages/cli/src/rules/reconcile-marker.ts b/packages/cli/src/rules/reconcile-marker.ts index f54a641a..db7e38fd 100644 --- a/packages/cli/src/rules/reconcile-marker.ts +++ b/packages/cli/src/rules/reconcile-marker.ts @@ -71,27 +71,26 @@ function compareVersions(a: string, b: string): number { * the marker with nothing reported, which is the opposite of the validation * this function exists to do. */ -const VERSION_SHAPE = /^\d+(?:\.\d+)*(?:-[\w.]+)?$/; +/** + * What a project with no recorded marker counts as. + * + * Not "current": a project that predates the ledger has had none of its + * entries applied, and reading absence as up-to-date would silently excuse it + * from all of them. + */ +export const BASELINE_VERSION = "0.0.0"; export async function recordReconciliation( - cwd: string, - reconciledTo: string + cwd: string ): Promise { - if (!VERSION_SHAPE.test(reconciledTo)) { - throw new CLIError( - `"${reconciledTo}" is not a version. Pass the CLI version the ledger walk completed up to, such as the \`version\` reported by \`info --json\`.`, - "INVALID_INPUT" - ); - } - - const installed = getCliVersion(); - - if (compareVersions(reconciledTo, installed) > 0) { - throw new CLIError( - `Cannot record reconciliation to ${reconciledTo}: this CLI is ${installed}, so it carries no ledger entries for that version. Upgrade first, then reconcile.`, - "INVALID_INPUT" - ); - } + // Stamped from the running CLI, never supplied by the caller. + // + // An agent has nothing to contribute here: the only sensible endpoint of a + // walk is the installed version, and letting a value in only made it + // possible to claim a walk that did not finish, which this design forbids + // anyway. Removing the parameter removes the two guards that existed to + // police it, along with every way of getting it wrong. + const reconciledTo = getCliVersion(); const tasklessDirectory = join(cwd, TASKLESS_DIRECTORY); @@ -113,9 +112,12 @@ export async function recordReconciliation( const { manifest, raw } = await readManifest(tasklessDirectory); const previous = manifest.rules?.reconciledTo; + // Still guarded, because the stamp comes from whichever CLI is running: an + // older one on the same project would otherwise rewind the marker and send + // the next walk back through entries already applied. if (previous !== undefined && compareVersions(reconciledTo, previous) < 0) { throw new CLIError( - `Cannot record reconciliation to ${reconciledTo}: the project is already reconciled to ${previous}. Ledger entries are cumulative and a walk only moves forward; recording an earlier version would claim work was undone.`, + `This CLI is ${reconciledTo} and the project is already reconciled to ${previous}. Ledger entries are cumulative and a walk only moves forward, so recording an older CLI would claim work was undone. Upgrade before reconciling.`, "INVALID_INPUT" ); } @@ -147,9 +149,15 @@ export function reconciliationStart( recorded: string | undefined ): { from: string; to: string } | undefined { const installed = getCliVersion(); - if (recorded === undefined) return undefined; - if (compareVersions(recorded, installed) >= 0) return undefined; - return { from: recorded, to: installed }; + // A missing marker means the project PREDATES the marker, so every entry + // still applies and the walk starts from the beginning. `init` stamps the + // field on a genuinely new project, which is what makes the two + // distinguishable: absent is old, present is accounted for. Treating absent + // as "nothing to do" would have quietly excused every project that existed + // before this feature from the entries written for it. + const from = recorded ?? BASELINE_VERSION; + if (compareVersions(from, installed) >= 0) return undefined; + return { from, to: installed }; } /** Whether a path exists, without distinguishing why it does not. */ @@ -161,3 +169,39 @@ async function pathExists(path: string): Promise { return false; } } + +/** + * Stamp the rules marker on a project being set up by this CLI. + * + * A new project has no ledger entries to walk: everything the entries describe + * is already true of a scaffold this CLI just wrote. Recording that up front + * is what gives an ABSENT marker its meaning, which is "this project predates + * the ledger and has had none of it applied". + * + * Never overwrites. Re-running setup on an existing project must not reset a + * marker that a real walk earned, which would send the next walk back through + * entries already applied. + */ +export async function stampNewProjectRules(cwd: string): Promise { + const tasklessDirectory = join(cwd, TASKLESS_DIRECTORY); + try { + const { manifest, raw } = await readManifest(tasklessDirectory); + if (manifest.rules?.reconciledTo !== undefined) return; + await writeManifest( + tasklessDirectory, + { + ...manifest, + rules: { + ...manifest.rules, + reconciledTo: getCliVersion(), + engines: { sg: AST_GREP_VERSION, vale: VALE_VERSION }, + }, + }, + raw + ); + } catch { + // Setting up a project must not fail because a marker could not be + // written. The cost of missing it is one extra ledger walk, which is the + // safe direction to be wrong in. + } +} diff --git a/packages/cli/test/reconcile-marker.test.ts b/packages/cli/test/reconcile-marker.test.ts index c8acdc81..90f800ae 100644 --- a/packages/cli/test/reconcile-marker.test.ts +++ b/packages/cli/test/reconcile-marker.test.ts @@ -62,12 +62,7 @@ describe("recording a rules reconciliation", () => { it("records the version and the engines the rules are valid against", async () => { const version = await installedVersion(); - const result = await runCli([ - "update", - `--reconciledTo=${version}`, - "-d", - cwd, - ]); + const result = await runCli(["update", "--rules", "-d", cwd]); expect(result.exitCode).toBe(0); const rules = await readRules(); @@ -79,7 +74,7 @@ describe("recording a rules reconciliation", () => { it("reports the marker through info", async () => { const version = await installedVersion(); - await runCli(["update", `--reconciledTo=${version}`, "-d", cwd]); + await runCli(["update", "--rules", "-d", cwd]); const info = await runCli(["info", "--json", "-d", cwd]); const parsed = JSON.parse(info.stdout) as { @@ -88,97 +83,6 @@ describe("recording a rules reconciliation", () => { expect(parsed.rules.reconciledTo).toBe(version); }); - it("refuses a version this CLI has no entries for", async () => { - // The quiet failure this prevents: recording a version whose ledger - // sections do not exist here, so a later walk starts past work nobody did. - const result = await runCli([ - "update", - "--reconciledTo=99.0.0", - "--json", - "-d", - cwd, - ]); - expect(result.exitCode).not.toBe(0); - const envelope = JSON.parse(result.stdout) as { - ok: boolean; - code: string; - }; - expect(envelope.ok).toBe(false); - expect(envelope.code).toBe("INVALID_INPUT"); - expect(await readRules()).toBeUndefined(); - }); - - it("treats a nightly and its release as the same version", async () => { - // Deliberately NOT semver ordering, and this test exists to stop someone - // "correcting" it into semver. Our nightlies are valid semver - // (`0.11.0-20260827050231x45e9997`), and the spec ranks a prerelease BELOW - // its release, so semver would say a project reconciled on the nightly is - // behind the identical release and send it to re-walk a ledger entry it - // already walked. They are built from the same commit and carry the same - // entries, so only the numeric core is compared. - const version = await installedVersion(); - const nightly = `${version}-20260827050231x45e9997`; - - const first = await runCli([ - "update", - `--reconciledTo=${nightly}`, - "-d", - cwd, - ]); - expect(first.exitCode).toBe(0); - - // Equal, not backwards: recording the plain release after the nightly is - // allowed rather than rejected as moving the marker back. - const second = await runCli([ - "update", - `--reconciledTo=${version}`, - "-d", - cwd, - ]); - expect(second.exitCode).toBe(0); - const rules = await readRules(); - expect(rules?.reconciledTo).toBe(version); - }); - - it("refuses to move the marker backwards", async () => { - const version = await installedVersion(); - await runCli(["update", `--reconciledTo=${version}`, "-d", cwd]); - - const result = await runCli([ - "update", - "--reconciledTo=0.0.1", - "--json", - "-d", - cwd, - ]); - expect(result.exitCode).not.toBe(0); - expect((JSON.parse(result.stdout) as { code: string }).code).toBe( - "INVALID_INPUT" - ); - // Unchanged, not clobbered with the rejected value. - const rules = await readRules(); - expect(rules?.reconciledTo).toBe(version); - }); - - it("refuses a value that is not a version", async () => { - // `versionCore` coerces an unparseable segment to 0, so garbage compares - // LOWER than any real version and would sail past both guards to be - // written verbatim. A pasted SHA or a truncated interpolation would - // corrupt the marker with nothing reported, which is the opposite of the - // validation this command claims to do. - for (const bad of ["abc", "", "0.11.0; rm -rf /", "deadbeef"]) { - const result = await runCli([ - "update", - `--reconciledTo=${bad}`, - "--json", - "-d", - cwd, - ]); - expect(result.exitCode, `accepted ${JSON.stringify(bad)}`).not.toBe(0); - } - expect(await readRules()).toBeUndefined(); - }); - it("refuses when there is no .taskless/ to reconcile", async () => { // `readManifest` tolerates a missing file but `writeManifest` does not // create parent directories, so this used to die on a raw ENOENT reported @@ -187,13 +91,7 @@ describe("recording a rules reconciliation", () => { // claiming work that could not have happened. const empty = await mkdtemp(join(tmpdir(), "taskless-noscaffold-")); try { - const result = await runCli([ - "update", - `--reconciledTo=${await installedVersion()}`, - "--json", - "-d", - empty, - ]); + const result = await runCli(["update", "--rules", "--json", "-d", empty]); expect(result.exitCode).not.toBe(0); const envelope = JSON.parse(result.stdout) as { code: string; @@ -207,16 +105,30 @@ describe("recording a rules reconciliation", () => { } }); + it("reports a walk from the baseline when no marker is recorded", async () => { + // The behaviour that reaches existing projects: absent means "predates + // the ledger", so every entry still applies. + const result = await runCli(["update", "--json", "-d", cwd]); + const payload = JSON.parse(result.stdout) as { + reconciledTo: string | null; + walk: { from: string; to: string } | null; + }; + expect(payload.reconciledTo).toBeNull(); + expect(payload.walk?.from).toBe("0.0.0"); + }); + + it("has nothing to walk once the rules are stamped", async () => { + await runCli(["update", "--rules", "-d", cwd]); + const result = await runCli(["update", "--json", "-d", cwd]); + const payload = JSON.parse(result.stdout) as { walk: unknown }; + expect(payload.walk).toBeNull(); + }); + it("leaves install untouched, since the two namespaces drift apart", async () => { const before = JSON.parse( await readFile(join(cwd, ".taskless", "taskless.json"), "utf8") ) as { install?: unknown }; - await runCli([ - "update", - `--reconciledTo=${await installedVersion()}`, - "-d", - cwd, - ]); + await runCli(["update", "--rules", "-d", cwd]); const after = JSON.parse( await readFile(join(cwd, ".taskless", "taskless.json"), "utf8") ) as { install?: unknown }; @@ -243,9 +155,9 @@ describe("taskless update with no flags", () => { expect(payload.topic).toBe("update"); expect(payload.installed).toBeTruthy(); expect(payload.recipe).toContain("# Topic: update"); - // No marker recorded here, so there is nothing to walk. That is not the - // same as being behind, and it must not read as a version to walk from. - expect(payload.walk).toBeNull(); + // No marker recorded, so the walk starts at the baseline: absent means + // the project predates the ledger and every entry still applies. + expect(payload.walk).toEqual({ from: "0.0.0", to: payload.installed }); }); it("serves the same ledger recipe as `agent update`", async () => { @@ -266,7 +178,7 @@ describe("taskless update with no flags", () => { it("carries the 0.11.0 ledger entry", async () => { const result = await runCli(["update"]); - expect(result.stdout).toContain("Migrating to 0.11.0"); + expect(result.stdout).toContain("Migrating to 0.11.x"); // The four things an author cannot discover from the diff. expect(result.stdout).toContain("rewriter now requires `fix`"); expect(result.stdout).toContain("Markdown is now a language"); diff --git a/packages/cli/test/reconciliation-start.test.ts b/packages/cli/test/reconciliation-start.test.ts new file mode 100644 index 00000000..f2b35d37 --- /dev/null +++ b/packages/cli/test/reconciliation-start.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "vitest"; + +import { + BASELINE_VERSION, + reconciliationStart, +} from "../src/rules/reconcile-marker"; +import { getCliVersion } from "../src/wizard/intro"; + +/** + * The walk boundary, unit-tested rather than driven through the CLI. + * + * Two of these cases depend on which CLI is running (an older build, a + * nightly against its release), which a spawned process cannot be made to + * simulate. They are the cases most likely to be "corrected" later, so they + * are pinned where they can actually be exercised. + */ +describe("reconciliationStart", () => { + const installed = getCliVersion(); + + it("treats a missing marker as the baseline, not as up to date", () => { + // The whole point of the 0.0.0 baseline: a project that predates the + // ledger has had NONE of its entries applied. Reading absence as + // "nothing to do" would silently excuse every existing project from the + // entries written for it, which is the failure this replaced. + const noMarker = undefined as string | undefined; + const walk = reconciliationStart(noMarker); + expect(walk).toEqual({ from: BASELINE_VERSION, to: installed }); + }); + + it("has nothing to walk when the marker is the installed version", () => { + expect(reconciliationStart(installed)).toBeUndefined(); + }); + + it("has nothing to walk when the marker is ahead, which is a downgrade", () => { + expect(reconciliationStart("999.0.0")).toBeUndefined(); + }); + + it("walks forward from an older marker", () => { + expect(reconciliationStart("0.0.1")).toEqual({ + from: "0.0.1", + to: installed, + }); + }); + + it("treats a nightly and its release as the same version", () => { + // Deliberately NOT semver ordering, and this test exists to stop someone + // "correcting" it into semver. Our nightlies are valid semver + // (`0.11.0-20260827050231x45e9997`) and the spec ranks a prerelease BELOW + // its release, so semver would call a project reconciled on the nightly + // behind the identical release and send it back through an entry it + // already walked. Same commit, same entries, so only the numeric core is + // compared. + expect( + reconciliationStart(`${installed}-20260827050231x45e9997`) + ).toBeUndefined(); + }); +}); From 723877be815756376a2da216c64446c59607bb27 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 26 Aug 2026 23:32:37 -0700 Subject: [PATCH 2/3] docs(agent): say what to DO about each semantics change, not what to watch for The 0.11.x ledger's hardest section said "re-run its fixtures and read the findings rather than the exit code", which tells an author to look without saying what for. Measuring both shapes against the two binaries (#199) turned each into an instruction. A rule using `nthChild` with a binding `ofRule` was INERT at 0.41.0 and now reports. The rule is not broken, it never ran, so its findings on this version have never been seen and may be numerous on a codebase that was never actually checked against it. Fixtures cannot surface this: a rule that matched nothing passed the `pass/` side of its own tests. A non-binding `ofRule`, `kind:` or `regex:`, is unaffected, which is the check that tells an author whether their rule is in scope. A metavariable bound inside a negated `not` and referenced in `message`, `fix` or a constraint now renders empty, because a `not` contributes no bindings and the old value was leaked from the candidate the negation REJECTED. This one changes no counts at all: same file, same range, same rule, different rendered output. So the instruction is to grep for a metavariable appearing both inside a `not` and in a `message` or `fix`, and the consequence is worth stating plainly, since a `fix` in that shape has been writing leaked text into files. Root metavariables and comments is named as unreproduced rather than dressed up as guidance, because no shape we tried showed a difference. Refs #180 Refs #184 --- packages/cli/src/agent/update.txt | 65 ++++++++++++++++++---- packages/cli/test/reconcile-marker.test.ts | 25 +++++++++ 2 files changed, 80 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/agent/update.txt b/packages/cli/src/agent/update.txt index e2a4068c..53f50416 100644 --- a/packages/cli/src/agent/update.txt +++ b/packages/cli/src/agent/update.txt @@ -112,16 +112,61 @@ This affects the flag only, not a rule's `language:` field, so no rule file needs changing. It is recorded because the divergence used to be documented as a thing to work around. -**Matching semantics moved, and this is the part you cannot check -locally.** 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 and no warning. - -There is no differential tooling yet. If a rule uses `nthChild`, a -negated `not`, or a bare root metavariable, re-run its fixtures and read -the findings rather than the exit code: the rule can still pass its -tests while reporting differently on real code. +**Matching semantics moved, and a valid, unchanged rule can now match a +DIFFERENT SET OF NODES with no error and no warning.** Two shapes are +affected. Both were measured against the two binaries, so this section +says what to do rather than what to watch for. + +**A rule was silently dead and now fires.** If a rule uses `nthChild` +with an `ofRule` whose body binds a metavariable: + +``` +nthChild: + position: 2 + ofRule: { pattern: $S } +``` + +then at 0.41.0 it matched **nothing**, because `ofRule` reused one +environment across siblings: the first match committed `$S` and every +later sibling failed the consistency check and went uncounted. It now +counts correctly and reports. + +So the rule is not broken, it was inert, and the findings it produces on +this version have never been seen. Run `%(TASKLESS_CLI)s check` and read +them as new: they are real matches the rule was always meant to make, +and they may be numerous on a codebase that has never been checked +against it. Fixtures are no help here, since a rule that matched nothing +passed the `pass/` side of its own tests. + +A rule whose `ofRule` used a non-binding matcher, `kind:` or `regex:` +with no metavariable, is unaffected: only binding was broken. + +**A metavariable is now empty where it used to carry a value.** If a +rule binds a metavariable inside a negated `not` and then references it +in `message`, `fix`, or a constraint: + +``` +follows: + not: + pattern: return $A + stopBy: end +message: "found after $A" +``` + +then at 0.41.0 `$A` rendered with a value leaked from the candidate the +negation REJECTED. A `not` contributes no bindings by definition, since +a successful negation means the inner rule did not match, so that value +was never meaningful. It is now unbound and renders empty. + +This one will not change your finding counts. The match is identical: +same file, same range, same rule. Only the rendered output differs, so +grep your rules for a metavariable that appears both inside a `not` and +in a `message` or `fix`. A `fix` in that shape has been writing the +leaked text into people's files. + +**Root metavariables and comments** also changed upstream +(ast-grep/ast-grep#2868), but no shape we tried reproduced a difference, +including the TSX case that PR names. Nothing to do unless you see one. ## Errors diff --git a/packages/cli/test/reconcile-marker.test.ts b/packages/cli/test/reconcile-marker.test.ts index 90f800ae..c695335a 100644 --- a/packages/cli/test/reconcile-marker.test.ts +++ b/packages/cli/test/reconcile-marker.test.ts @@ -176,6 +176,31 @@ describe("taskless update with no flags", () => { expect(result.stdout).toContain("The directory is"); }); + it("tells an author what to DO about each semantics change", async () => { + // The section used to say "re-run your fixtures and read the findings", + // which tells someone to look without saying what for. Both shapes were + // measured across the two binaries, so it can name the action instead. + const result = await runCli(["update"]); + + // Shape 1: the rule was inert and now reports. Fixtures cannot catch it, + // because a rule matching nothing passes its own `pass/` side. + expect(result.stdout).toContain("silently dead and now fires"); + expect(result.stdout).toContain("read\nthem as new"); + + // Shape 2: identical findings, different rendered output. Counting will + // not surface it, so the instruction is to grep the rules. + expect(result.stdout).toContain("will not change your finding counts"); + expect(result.stdout).toContain( + "writing the\nleaked text into people's files" + ); + + // And the one we could not reproduce is named as such rather than + // dressed up as guidance. + expect(result.stdout).toContain( + "no shape we tried reproduced a difference" + ); + }); + it("carries the 0.11.0 ledger entry", async () => { const result = await runCli(["update"]); expect(result.stdout).toContain("Migrating to 0.11.x"); From 84142aab311515fd969cbaa1b4b79d1e2fcb0e3c Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 26 Aug 2026 23:45:06 -0700 Subject: [PATCH 3/3] fix(cli): only stamp the marker on a genuinely new project From review of #198. Two findings, both real, and the first reintroduced the exact bug this PR exists to fix. `stampNewProjectRules` ran unconditionally after `ensureTasklessDirectory`, which `mkdir -p`s. By the time it executed, a pre-existing project and a fresh one were indistinguishable, so it fell back to "no marker means new". Its caller is `runNonInteractive`, which is also `init --no-interactive`, whose documented job is refreshing an EXISTING project. So an ordinary older project that had never walked the ledger, refreshed once after a CLI upgrade, got stamped as fully reconciled and skipped every entry. That is the silent skip this change was written to prevent, reached through setup instead of through the walk. Newness is now sampled BEFORE the directory is created, and that ordering is the fix. The `reconciledTo !== undefined` check inside stays as a second belt against resetting a marker a real walk earned. Covered by a regression test verified to bite: it fails with the guard removed and passes with it restored. A test that passes either way would have been worse than none here, since the failure is silent by construction. The recipe also told an agent to read `walk` from `info --json`, which did not report it: it was computed only for `update --json`. Rather than weaken the guidance, `info` now reports `rules.walk`, since `info` is already the command consulted for capability state and this keeps one place to look. An agent following the recipe literally would otherwise have found no field and fallen back to deriving the boundary by hand, which is what that paragraph exists to prevent. Refs #180 --- packages/cli/src/agent/update.txt | 8 ++--- packages/cli/src/commands/info.ts | 5 +++ packages/cli/src/commands/init.ts | 21 ++++++++---- packages/cli/src/rules/reconcile-marker.ts | 15 ++++++--- packages/cli/src/schemas/info.ts | 6 ++++ packages/cli/test/reconcile-marker.test.ts | 39 ++++++++++++++++++++++ 6 files changed, 80 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/agent/update.txt b/packages/cli/src/agent/update.txt index 53f50416..da725b80 100644 --- a/packages/cli/src/agent/update.txt +++ b/packages/cli/src/agent/update.txt @@ -40,10 +40,10 @@ that you finished. stamped at creation, so an absent marker is the one case where none of the entries below have ever been applied. - `info --json` also reports `walk`, which is this decision already - made: `{ "from": ..., "to": ... }` when there is something to walk, - and `null` when there is not. Prefer it to re-deriving the boundary - yourself. + `rules.walk` on that same payload is this decision already made: + `{ "from": ..., "to": ... }` when there is something to walk, and + `null` when there is not. Prefer it to re-deriving the boundary + yourself, so the offer and the enforcement cannot disagree. 2. **Walk forward, in order.** Start at the section after `reconciledTo` and continue to the installed version. Sections are diff --git a/packages/cli/src/commands/info.ts b/packages/cli/src/commands/info.ts index 48266638..5dacbf24 100644 --- a/packages/cli/src/commands/info.ts +++ b/packages/cli/src/commands/info.ts @@ -8,6 +8,7 @@ import { outputSchema as infoOutputSchema } from "../schemas/info"; import { makeErrorEnvelope } from "../types/errors"; import { resolveRepositoryContext } from "../util/git-remote"; import { readManifest } from "../filesystem/migrate"; +import { reconciliationStart } from "../rules/reconcile-marker"; import { TASKLESS_DIRECTORY } from "../rules/vale/formats"; export const infoCommand = defineCommand({ @@ -86,6 +87,10 @@ export const infoCommand = defineCommand({ sg: manifest?.rules?.engines?.sg ?? null, vale: manifest?.rules?.engines?.vale ?? null, }, + // The walk boundary, decided once here rather than by each caller. + // A missing marker resolves to the baseline, so a project predating + // the ledger reports a walk rather than "nothing to do". + walk: reconciliationStart(manifest?.rules?.reconciledTo) ?? null, }, }; diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 59729f88..48cca8bc 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -23,6 +23,7 @@ import { processLauncherContext, } from "../util/package-manager"; import { + pathExists, recordReconciliation, reconciliationStart, stampNewProjectRules, @@ -220,13 +221,21 @@ export const updateCommand = defineCommand({ async function runNonInteractive( cwd: string ): Promise<{ commandsInstalled: boolean }> { + // Sampled BEFORE the directory is created, and that order is the whole + // point. `ensureTasklessDirectory` mkdir -p's, so afterwards a pre-existing + // project is indistinguishable from a fresh one. + // + // This path is also `init --no-interactive`, whose documented job is + // refreshing an EXISTING project. Stamping there would mark a project that + // never walked the ledger as fully reconciled and skip every entry, which is + // the silent skip this feature exists to prevent. + const wasNewProject = !(await pathExists(join(cwd, TASKLESS_DIRECTORY))); await ensureTasklessDirectory(cwd); - // Stamp the rules marker on a project this CLI is setting up. That is what - // lets an ABSENT marker mean "predates the ledger" rather than "new": a - // project created here has no entries to walk, and one that never recorded - // anything has all of them. Without this the two are indistinguishable and - // the walk has to guess. - await stampNewProjectRules(cwd); + if (wasNewProject) { + // A project this CLI just created has no entries to walk: everything the + // ledger describes is already true of the scaffold it wrote. + await stampNewProjectRules(cwd); + } const allSkills = getEmbeddedSkills(); const mandatoryNames = new Set(getMandatorySkillNames()); diff --git a/packages/cli/src/rules/reconcile-marker.ts b/packages/cli/src/rules/reconcile-marker.ts index db7e38fd..bc36d638 100644 --- a/packages/cli/src/rules/reconcile-marker.ts +++ b/packages/cli/src/rules/reconcile-marker.ts @@ -161,7 +161,7 @@ export function reconciliationStart( } /** Whether a path exists, without distinguishing why it does not. */ -async function pathExists(path: string): Promise { +export async function pathExists(path: string): Promise { try { await access(path); return true; @@ -178,9 +178,16 @@ async function pathExists(path: string): Promise { * is what gives an ABSENT marker its meaning, which is "this project predates * the ledger and has had none of it applied". * - * Never overwrites. Re-running setup on an existing project must not reset a - * marker that a real walk earned, which would send the next walk back through - * entries already applied. + * ONLY call this when the project was genuinely new, which the caller has to + * establish BEFORE creating the directory. This function cannot tell: by the + * time it runs, `ensureTasklessDirectory` has already `mkdir -p`'d, so a + * pre-existing project and a fresh one look identical. Stamping one that + * merely never recorded a marker would mark it fully reconciled and skip every + * entry, which is the exact silent-skip this whole feature exists to prevent, + * reachable by re-running setup on an ordinary older project. + * + * Never overwrites either. Re-running setup on a project that HAS a marker + * must not reset one a real walk earned. */ export async function stampNewProjectRules(cwd: string): Promise { const tasklessDirectory = join(cwd, TASKLESS_DIRECTORY); diff --git a/packages/cli/src/schemas/info.ts b/packages/cli/src/schemas/info.ts index b15832ec..bfdae1b3 100644 --- a/packages/cli/src/schemas/info.ts +++ b/packages/cli/src/schemas/info.ts @@ -47,6 +47,12 @@ export const outputSchema = z.object({ sg: z.string().nullable(), vale: z.string().nullable(), }), + walk: z + .object({ from: z.string(), to: z.string() }) + .nullable() + .describe( + "Where a ledger walk should start and end, or null when there is nothing to walk. Computed here so a caller does not re-derive the boundary" + ), }) .describe( "What the rules are valid against. Distinct from `install`: these advance only on a completed reconciliation, never on an upgrade" diff --git a/packages/cli/test/reconcile-marker.test.ts b/packages/cli/test/reconcile-marker.test.ts index c695335a..02023279 100644 --- a/packages/cli/test/reconcile-marker.test.ts +++ b/packages/cli/test/reconcile-marker.test.ts @@ -105,6 +105,45 @@ describe("recording a rules reconciliation", () => { } }); + it("does not stamp a pre-existing project that never reconciled", async () => { + // The silent skip this whole feature exists to prevent, reachable through + // setup rather than through the walk. `init --no-interactive` is the + // documented refresh path for an EXISTING project, and it runs after + // `ensureTasklessDirectory` has already created the directory, so + // "was this new" has to be sampled before that or it always reads new. + // + // Stamping here would mark a project that has walked nothing as fully + // reconciled and skip every ledger entry. + const before = await runCli(["update", "--json", "-d", cwd]); + expect( + (JSON.parse(before.stdout) as { walk: unknown }).walk + ).not.toBeNull(); + + await runCli(["init", "--no-interactive", "-d", cwd]); + + const after = await runCli(["update", "--json", "-d", cwd]); + const walk = ( + JSON.parse(after.stdout) as { + walk: { from: string } | null; + } + ).walk; + expect(walk).not.toBeNull(); + expect(walk?.from).toBe("0.0.0"); + }); + + it("reports the walk boundary on info as well, so callers agree", async () => { + // The recipe tells an agent to read `rules.walk` from `info --json`. It + // has to actually be there, or the guidance sends it looking for a field + // that does not exist and back to deriving the boundary by hand. + const result = await runCli(["info", "--json", "-d", cwd]); + const rules = ( + JSON.parse(result.stdout) as { + rules: { walk: { from: string } | null }; + } + ).rules; + expect(rules.walk?.from).toBe("0.0.0"); + }); + it("reports a walk from the baseline when no marker is recorded", async () => { // The behaviour that reaches existing projects: absent means "predates // the ledger", so every entry still applies.