diff --git a/packages/cli/test/migrate-install.test.ts b/packages/cli/test/migrate-install.test.ts index 11f424fd..45bf72de 100644 --- a/packages/cli/test/migrate-install.test.ts +++ b/packages/cli/test/migrate-install.test.ts @@ -207,6 +207,20 @@ describe("migration version matrix", () => { // forgets to handle an older starting point. // Every prior version, derived. The list used to be written out, so each new // migration silently stopped testing the version it had just made "prior". + // + // NOTE on migration 0006 specifically (0006-refresh-readme.ts): every start + // version here is seeded with a bare `taskless.json` and no pre-existing + // `README.md`, so by the time 0006 runs there is no stale README for its + // rewrite to act on, and this test only reads the final `manifest.version` + // counter afterward. A regression that made 0006 a no-op (e.g. it stopped + // writing the file, or wrote the wrong template) would NOT be caught here -- + // this loop would still pass, because a missing README and a freshly + // written one look identical to a test that never reads README.md. Do not + // mistake this matrix for coverage of 0006's actual rewrite; that coverage + // lives in `installed-documentation.test.ts` ("migration 0006, on a project + // that is already current" / "rewrites a stale README that no other + // migration would touch"), which seeds a real stale README and asserts on + // its rewritten content. const priorVersions = Array.from( { length: LATEST_SCHEMA_VERSION }, (_, index) => index diff --git a/packages/cli/test/reference.test.ts b/packages/cli/test/reference.test.ts index 6ccb30c5..e175524f 100644 --- a/packages/cli/test/reference.test.ts +++ b/packages/cli/test/reference.test.ts @@ -41,6 +41,63 @@ function ruleFor(reference: Reference, engine: string) { return rule; } +/** + * Record every (path, kind) pair reachable in `value` into `into`. + * + * `path` uses `[]` for "inside some array", never an index, and an object's + * keys are visited in sorted order rather than insertion order. Both choices + * exist so that reordering an array, or reordering an object literal's keys, + * changes nothing this records — those are the cosmetic edits the fingerprint + * below is required NOT to fail on. What it does record is which keys exist at + * each position and what kind of value sits there (object / array / string / + * number / boolean / null) -- never the value itself, so renaming a rule's + * `id` from "no-eval-call" to something else does not move the fingerprint, + * but adding, removing, or retyping a field does. + */ +function shapeOf(value: unknown, path: string, into: Set): void { + if (Array.isArray(value)) { + into.add(`${path}:array`); + for (const item of value) shapeOf(item, `${path}[]`, into); + return; + } + if (value !== null && typeof value === "object") { + into.add(`${path}:object`); + for (const key of Object.keys( + value as Record + ).toSorted()) { + shapeOf((value as Record)[key], `${path}.${key}`, into); + } + return; + } + into.add(`${path}:${value === null ? "null" : typeof value}`); +} + +/** + * Every (path, kind) pair the reference payload reaches, sorted. + * + * Derived from the live object rather than hand-copied, so it moves when + * `Reference` / `ReferenceRule` / `ReferenceTests` / `ReferenceLayout` change + * shape and cannot silently rot the way a hand-maintained field list would. + * + * This is the check the `v1 -> v2` episode (see `REFERENCE_VERSION`'s own + * comment, and CLAUDE.md's account of it) needed and did not have: `tests` + * went from an array to `{ grouping, files, cases? }` and nothing forced + * `REFERENCE_VERSION` to move with it. A change of that kind now fails here, + * and the fix is to bump `REFERENCE_VERSION`, update the list below, and say + * why in the commit that touches both. + * + * ASSERTED AS A LIST, NOT A HASH, ON PURPOSE. A digest comparison reports + * "expected 0add… to be ab12…", which says the shape moved but not what moved, + * leaving whoever hit it to rebuild the shape set by hand to find the one added + * field. A guard that fires this rarely has to be legible the one time it does, + * and 56 lines is a small price for a diff that names the field. + */ +function shapePaths(value: unknown): string[] { + const paths = new Set(); + shapeOf(value, "$", paths); + return [...paths].toSorted(); +} + describe("the demo reference payload", () => { it("is current — regenerate with `pnpm --filter @taskless/cli reference`", async () => { expect(await readReference()).toEqual(buildReference(DEMO_RULES)); @@ -248,6 +305,83 @@ describe("the demo reference payload", () => { expect(reference.version).toBe(2); }); + it("carries a structural fingerprint that moves with the corpus shape", async () => { + const reference = await readReference(); + + // Nothing else in this file ties a shape change to `REFERENCE_VERSION`. + // Every other test here asserts the shape it expects (a `layout` block, a + // `tests.grouping`, a `signature` on `runtime` only, ...), so any one of + // them would fail if a field vanished -- but NONE of them fail if a field + // is ADDED, and #263 was exactly an addition of shape (`tests` gained a + // structure `layout` had no way to describe) that changed what a + // consumer had to parse without changing what any existing assertion + // here checked. + // + // If this assertion is the one that broke: you changed what `Reference` + // (or `ReferenceRule` / `ReferenceTests` / `ReferenceLayout`) publishes. + // Bump `REFERENCE_VERSION`, update its doc comment to say what changed and + // why, regenerate `assets/reference.json` (`pnpm --filter @taskless/cli + // reference`), and update the hash below to match. + expect(shapePaths(reference)).toEqual([ + "$.constraints:array", + "$.constraints[].enforcedBy:string", + "$.constraints[].engine:string", + "$.constraints[].id:string", + "$.constraints[].rationale:string", + "$.constraints[].summary:string", + "$.constraints[]:object", + "$.layout.engines.runtime.capturesDirectory:string", + "$.layout.engines.runtime.fixtureLayout:string", + "$.layout.engines.runtime.ruleConfigFile:null", + "$.layout.engines.runtime.ruleFile:string", + "$.layout.engines.runtime:object", + "$.layout.engines.sg.capturesDirectory:null", + "$.layout.engines.sg.fixtureLayout:string", + "$.layout.engines.sg.ruleConfigFile:null", + "$.layout.engines.sg.ruleFile:string", + "$.layout.engines.sg:object", + "$.layout.engines.vale.capturesDirectory:null", + "$.layout.engines.vale.fixtureLayout:string", + "$.layout.engines.vale.ruleConfigFile:string", + "$.layout.engines.vale.ruleFile:string", + "$.layout.engines.vale:object", + "$.layout.engines:object", + "$.layout.ruleDirectory:string", + "$.layout.rulesRoot:string", + "$.layout.testsDirectory:string", + "$.layout:object", + "$.protocol:array", + "$.protocol[]:string", + "$.rules:array", + "$.rules[].directory:string", + "$.rules[].engine:string", + "$.rules[].id:string", + "$.rules[].prompt:string", + "$.rules[].rule:array", + "$.rules[].ruleFile:string", + "$.rules[].rule[].content:string", + "$.rules[].rule[].path:string", + "$.rules[].rule[]:object", + "$.rules[].signature:string", + "$.rules[].tests.cases:array", + "$.rules[].tests.cases[].bucket:string", + "$.rules[].tests.cases[].files:array", + "$.rules[].tests.cases[].files[]:string", + "$.rules[].tests.cases[].name:string", + "$.rules[].tests.cases[].path:string", + "$.rules[].tests.cases[]:object", + "$.rules[].tests.files:array", + "$.rules[].tests.files[].content:string", + "$.rules[].tests.files[].path:string", + "$.rules[].tests.files[]:object", + "$.rules[].tests.grouping:string", + "$.rules[].tests:object", + "$.rules[]:object", + "$.version:number", + "$:object", + ]); + }); + it("carries the prompt each rule answers", async () => { const reference = await readReference(); for (const rule of reference.rules) { diff --git a/packages/cli/test/wizard-integration.test.ts b/packages/cli/test/wizard-integration.test.ts index 2e787e9c..8de38ef4 100644 --- a/packages/cli/test/wizard-integration.test.ts +++ b/packages/cli/test/wizard-integration.test.ts @@ -271,3 +271,88 @@ describe("runWizard with an unreadable manifest", () => { expect(captureSpy).not.toHaveBeenCalledWith("cli_installed"); }); }); + +/** + * `init-no-interactive.test.ts` covers this same wiring for `init`, and its + * own comment names the risk directly: a dropped `console.log` or a swapped + * field leaves every unit test green while the user is told nothing and their + * agent keeps serving the previous skills. `runWizard` (src/wizard/index.ts) + * grew the identical `getReloadNotice` call, but only the `init` path got a + * test for it -- this mirrors that test's shape for the wizard. + */ +describe("the wizard's restart-your-agents banner", () => { + it("prints on a second run whose recorded version moved", async () => { + clackResponses.locations = [".claude"]; + clackResponses.summary = true; + + const { runWizard } = await import("../src/wizard"); + await runWizard({ cwd }); + + // Plant an older recorded version, the same way + // `init-no-interactive.test.ts` stages an upgrade: the build under test + // cannot report two versions in one process, so the move has to be staged + // in the manifest between two runs. + const manifestPath = join(cwd, ".taskless", "taskless.json"); + const manifest = JSON.parse(await readFile(manifestPath, "utf8")) as { + install?: { cliVersion?: string }; + }; + if (manifest.install) manifest.install.cliVersion = "0.0.1-planted"; + await writeFile(manifestPath, JSON.stringify(manifest, null, 2)); + + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + try { + const result = await runWizard({ cwd }); + expect(result.status).toBe("completed"); + + const printed = logSpy.mock.calls.map((call) => String(call[0])); + expect(printed.some((line) => line.includes("RESTART YOUR AGENTS"))).toBe( + true + ); + // The version it moved FROM, which is the half a reader needs to tell + // an upgrade from a downgrade. + expect(printed.some((line) => line.includes("0.0.1-planted"))).toBe(true); + } finally { + logSpy.mockRestore(); + } + }); + + it("stays quiet on a first run", async () => { + clackResponses.locations = [".claude"]; + clackResponses.summary = true; + + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + try { + const { runWizard } = await import("../src/wizard"); + const result = await runWizard({ cwd }); + expect(result.status).toBe("completed"); + + const printed = logSpy.mock.calls.map((call) => String(call[0])); + expect(printed.some((line) => line.includes("RESTART YOUR AGENTS"))).toBe( + false + ); + } finally { + logSpy.mockRestore(); + } + }); + + it("stays quiet when the recorded version did not move", async () => { + clackResponses.locations = [".claude"]; + clackResponses.summary = true; + + const { runWizard } = await import("../src/wizard"); + await runWizard({ cwd }); + + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + try { + const result = await runWizard({ cwd }); + expect(result.status).toBe("completed"); + + const printed = logSpy.mock.calls.map((call) => String(call[0])); + expect(printed.some((line) => line.includes("RESTART YOUR AGENTS"))).toBe( + false + ); + } finally { + logSpy.mockRestore(); + } + }); +});