diff --git a/.changeset/rules-reconciliation-ledger.md b/.changeset/rules-reconciliation-ledger.md new file mode 100644 index 00000000..9a6c3943 --- /dev/null +++ b/.changeset/rules-reconciliation-ledger.md @@ -0,0 +1,19 @@ +--- +"@taskless/cli": minor +--- + +`taskless update` now tells an agent what an upgrade changed for the rules already in a project, and records when that work is done. + +**BREAKING for anyone scripting `taskless update`.** It used to mean "reinstall the skills non-interactively", which is what `taskless init --no-interactive` already does through the same code path, and what running `taskless` does on its own. Scripts relying on the old behavior should call `taskless init --no-interactive`. + +The word is reclaimed for the job an agent actually needs. Running the CLI migrates the `.taskless/` layout and refreshes skills: that is the directory, and it is automatic. No migration can rewrite the rules themselves, and a rewriter that now requires a `fix`, or a rule whose matching semantics shifted under a new engine, is a question about content. An agent that has run a migration and watched it succeed will otherwise reasonably conclude the upgrade is finished. + +`taskless update` with no flags serves a ledger: one section per release, in order, saying what that version means for existing rules. Sections are cumulative, and a version with nothing to do says so explicitly, because an agent cannot tell "nothing here" from "nobody wrote this". The first entry covers 0.11.0: the newly required `fix` on rewriters, Markdown's block-only grammar and its two opposite failure shapes, `sg run --lang` accepting alias spellings, and the matching-semantics changes that alter what a valid rule matches with no error at all. + +`taskless update --reconciledTo=` records that the walk finished, in a new `rules` section of `.taskless/taskless.json` alongside the existing `install`. It stores the CLI version and the ast-grep and Vale versions the rules are now valid against. + +The two namespaces are separate because they 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 on a completed reconciliation. Keying rule work off `install.cliVersion` would let an agent skip entries it never performed, and it would fail quietly: the walk would report nothing to do while the rules stayed wrong. + +The version is validated rather than trusted. A value ahead of the installed CLI is rejected, since this build carries no entries for it, and the marker is never moved backwards. + +`taskless info --json` reports both namespaces, so an agent reads where to start from the same payload it already fetches. diff --git a/packages/cli/src/agent/update.txt b/packages/cli/src/agent/update.txt index c0b3b3a0..e88c5978 100644 --- a/packages/cli/src/agent/update.txt +++ b/packages/cli/src/agent/update.txt @@ -1,48 +1,130 @@ -# Topic: update (CLI v%(CLI_VERSION)s / topic v1) +# Topic: update (CLI v%(CLI_VERSION)s / topic v2) -## Goal -Update Taskless skills in the user's coding-agent tools to the -latest bundled version. Non-interactive, no wizard, no prompts. -Installs to all detected tool locations using the same logic as -`%(TASKLESS_CLI)s init --no-interactive`, but exposed as its own subcommand -so the agent can run it directly without explaining flags. +## You are here +This is `update`. It tells you what an upgrade changed for the rules +ALREADY IN THIS PROJECT, and what to do about them. + +It is not about installing. Running `%(TASKLESS_CLI)s` migrates the +`.taskless/` layout and refreshes the installed skills on its own. That +handles the DIRECTORY. This recipe handles the RULES, which no migration +can rewrite for you: a rewriter that now needs a `fix`, a rule whose +matching semantics shifted under a new engine, a rule that could now be +expressed better with a newly supported language. + +An agent that has run a migration and watched it succeed will otherwise +reasonably conclude the upgrade is done. It is not. The directory is +migrated; the rules may still need work. -This is the right command when the user has Taskless already -installed and just wants to refresh to a new version (e.g. after -running `%(TASKLESS_CLI)s`). +## Goal +Walk the ledger below from the version this project was last reconciled +to, up to the installed CLI, doing what each section says. Then record +that you finished. ## Preconditions -- None at the user level. Works in any directory. -- For tool detection: at least one of `.claude/`, `.opencode/`, - `.cursor/`, or a related marker file exists. Otherwise installs - to `.agents/skills/` as a fallback. +- `.taskless/` exists. If it does not, there are no rules to reconcile. +- You can read `.taskless/taskless.json`. ## Steps -``` -%(TASKLESS_CLI)s update -``` - -The CLI: -1. Detects installed tools (Claude Code, OpenCode, Cursor, etc.) -2. Reads the previous install state from `.taskless/taskless.json` -3. Computes the diff (skills/commands to add, remove) -4. Rewrites the canonical `taskless` skill and `tskl` command in - `.taskless/`, and refreshes the reference stub in each detected - tool directory -5. Converts any stale full copies or symlinks left by older installs - into stubs, and removes obsolete files from prior versions -6. Updates the install manifest -7. Prints a summary including what was written and removed +1. **Find where to start.** Run: + ``` + %(TASKLESS_CLI)s info --json + ``` + and read `rules.reconciledTo`, plus `rules.engines.sg` and + `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. + +2. **Walk forward, in order.** Start at the section after + `reconciledTo` and continue to the installed version. Sections are + cumulative, never replaceable: a later one can depend on an earlier + one having run, so do not skip ahead to the newest. + +3. **Do the work each section names.** Each says what changed and what + it means for existing rules. A section that says there is nothing to + do means exactly that; it is a claim, not an oversight. + +4. **Record that you finished.** Run: + ``` + %(TASKLESS_CLI)s update --reconciledTo= + ``` + 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. + + 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 + +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. + +**A rewriter now requires `fix`.** `SerializableRewriter.required` goes +from `["rule","id"]` to `["id","fix","rule"]`, so a `rewriters:` entry +without a `fix:` is now rejected where it was accepted before. + +Run `%(TASKLESS_CLI)s verify` and it names the offending rewriter +directly. It cannot be auto-fixed: `fix` is replacement text, which is +authorial intent. A rewriter with no `fix` could never have done +anything, so this surfaces a rule that was already dead rather than +breaking one that worked. + +**Markdown is now a language, with a narrow shape.** `language: +Markdown` parses. What it sees is the BLOCK tree only: `document`, +`section`, `atx_heading`, `setext_heading`, `fenced_code_block`, +`list_item`, `paragraph`. Everything inside a line collapses into one +opaque `inline` node, so there is no `link`, no `emphasis`, no +`strong_emphasis`. + +Two failure shapes, and they are opposites. `kind: link` is a HARD +CONFIG ERROR: exit 8, `Kind \`link\` is invalid`, which aborts config +parsing and takes every other rule's report down with it. A pattern like +`[$T]($U)` is the quiet one: it parses, runs, exits 0, and matches +nothing forever. + +So "no bare URLs" and "link text must not say click here" are not +ast-grep rules even now. Use Vale, which reads prose, and note that Vale +DOES see YAML frontmatter: with `scope: raw` it sees the keys too, and +`extends: occurrence` with `min: 1` can require a field to be present. + +**`sg run --lang` accepts the alias spellings.** At 0.41.0 `--lang C++` +was rejected while `language: C++` parsed. At 0.45.2 both are accepted. +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. ## Errors -`update` is non-interactive. On success it exits 0 and prints a -human-readable summary of skills/commands added, removed, or kept in -sync (same surface as `init --no-interactive`). On failure it exits -non-zero with the error message on stderr. +With `--json`, `--reconciledTo` 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 | ## See Also -- `%(TASKLESS_CLI)s agent init`: interactive variant (wizard with prompts) -- `%(TASKLESS_CLI)s agent info`: verify what's installed and check staleness +- `%(TASKLESS_CLI)s agent check`: run every engine over the repo +- `%(TASKLESS_CLI)s agent improve-rule`: rewrite a rule the walk flagged +- `%(TASKLESS_CLI)s agent info`: what is installed, and staleness diff --git a/packages/cli/src/commands/info.ts b/packages/cli/src/commands/info.ts index f5d73326..48266638 100644 --- a/packages/cli/src/commands/info.ts +++ b/packages/cli/src/commands/info.ts @@ -1,4 +1,4 @@ -import { resolve } from "node:path"; +import { join, resolve } from "node:path"; import { defineCommand } from "citty"; import { checkStaleness } from "../install/install"; @@ -7,6 +7,8 @@ import { fetchWhoami } from "../auth/whoami"; import { outputSchema as infoOutputSchema } from "../schemas/info"; import { makeErrorEnvelope } from "../types/errors"; import { resolveRepositoryContext } from "../util/git-remote"; +import { readManifest } from "../filesystem/migrate"; +import { TASKLESS_DIRECTORY } from "../rules/vale/formats"; export const infoCommand = defineCommand({ meta: { @@ -36,10 +38,17 @@ export const infoCommand = defineCommand({ // The repository context resolves regardless of `--anonymous`: it comes // from the local git remote, not from the API, so suppressing it would // hide capability state that has nothing to do with the auth probe. - const [tools, token, repository] = await Promise.all([ + const [tools, token, repository, manifest] = await Promise.all([ checkStaleness(cwd), args.anonymous ? Promise.resolve() : getToken(cwd), resolveRepositoryContext(cwd), + // Never fails: an absent or unreadable manifest is an ordinary state for + // a project that has not been initialised, and `info` still has plenty + // to report about one. + readManifest(join(cwd, TASKLESS_DIRECTORY)).then( + (read) => read.manifest, + () => null + ), ]); let auth: { user: string; email?: string; orgs: string[] } | undefined; @@ -66,6 +75,18 @@ export const infoCommand = defineCommand({ // payload already; these fields ride along on a call it makes anyway. repositoryUrl: repository.repositoryUrl, ghOwner: repository.ghOwner, + // Two namespaces, reported separately because they answer different + // questions and drift apart. `install` is how the scaffold got here. + // `rules` is what the rules are valid against, and it moves only when a + // reconciliation is recorded. + install: { cliVersion: manifest?.install?.cliVersion ?? null }, + rules: { + reconciledTo: manifest?.rules?.reconciledTo ?? null, + engines: { + sg: manifest?.rules?.engines?.sg ?? null, + vale: manifest?.rules?.engines?.vale ?? null, + }, + }, }; if (args.json) { diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index b3f0b7dc..27f80902 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -1,4 +1,4 @@ -import { resolve } from "node:path"; +import { join, resolve } from "node:path"; import { defineCommand } from "citty"; import { ensureTasklessDirectory } from "../filesystem/directory"; @@ -17,6 +17,19 @@ import { runWizard } from "../wizard"; import { getCliVersion } from "../wizard/intro"; import { getOnboardTrailer } from "./onboard"; +import { getRecipe } from "../prompts/recipes"; +import { + detectCliInvocation, + processLauncherContext, +} from "../util/package-manager"; +import { + recordReconciliation, + reconciliationStart, +} from "../rules/reconcile-marker"; +import { readManifest } from "../filesystem/migrate"; +import { TASKLESS_DIRECTORY } from "../rules/vale/formats"; +import { CLIError } from "../util/cli-error"; +import { makeErrorEnvelope } from "../types/errors"; function shouldRunInteractively(noInteractiveFlag: boolean): boolean { if (noInteractiveFlag) return false; @@ -79,11 +92,25 @@ export const initCommand = defineCommand({ }, }); +/** + * `update` is about the RULES, not about the installation. + * + * It used to mean "reinstall the skills non-interactively", which is what + * `init --no-interactive` already does through the very same + * `runNonInteractive`, and what the wizard does on any ordinary run. A second + * name for that bought nothing, and it held a word that describes the job an + * agent actually needs: deciding whether the rules in front of it need + * 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. + */ export const updateCommand = defineCommand({ meta: { name: "update", description: - "Update Taskless skills in detected tools (non-interactive install)", + "Learn what an upgrade changed for existing rules, or record a completed reconciliation", }, args: { dir: { @@ -91,6 +118,17 @@ export const updateCommand = defineCommand({ alias: "d", description: "Working directory", }, + reconciledTo: { + type: "string", + description: + "Record that the ledger walk completed up to this CLI version", + }, + json: { + type: "boolean", + description: + "Output as JSON: the recipe plus where the walk starts, or the recorded result with --reconciledTo", + default: false, + }, anonymous: { type: "boolean", description: "Accepted for compatibility; update has no auth dependency", @@ -99,17 +137,80 @@ export const updateCommand = defineCommand({ }, async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); - const telemetry = await getTelemetry(cwd); - let success = false; + // No `--reconciledTo`: 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) { + const telemetry = await getTelemetry(cwd); + const recipe = getRecipe("update", { + anonymous: args.anonymous, + invocation: detectCliInvocation(processLauncherContext()), + }); + if (recipe === undefined) { + console.error("No `update` recipe is bundled with this CLI."); + process.exitCode = 1; + return; + } + telemetry.capture("cli_agent", { topic: "update" }); + + // `--json` is honoured here too. It used to be read only on the + // recording path, so `taskless update --json` printed plain prose and + // gave no sign the flag had done nothing. + // + // The payload also carries where the walk should START, computed by the + // CLI rather than reasoned out of the recipe's prose. Same argument as + // `route` reading `ghOwner` from `info` instead of shelling out to git: + // two places deriving one answer can disagree, and the one that acts on + // it should not be the one guessing. + if (args.json) { + const { manifest } = await readManifest( + join(cwd, TASKLESS_DIRECTORY) + ).catch(() => ({ manifest: undefined })); + const walk = reconciliationStart(manifest?.rules?.reconciledTo); + console.log( + JSON.stringify({ + ok: true, + topic: "update", + reconciledTo: manifest?.rules?.reconciledTo ?? null, + installed: getCliVersion(), + // `null` when there is nothing to walk: either the project has + // never recorded a reconciliation, which is not the same as being + // behind, or it is already current. + walk: walk ?? null, + recipe, + }) + ); + return; + } + + console.log(recipe.trimEnd()); + return; + } + + const telemetry = await getTelemetry(cwd); try { - await runNonInteractive(cwd); - success = true; - } finally { - // Concrete state event: skills/commands were installed/updated. - if (success) { - telemetry.capture("cli_installed"); + const result = await recordReconciliation(cwd, args.reconciledTo); + 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}).` + ); + } + telemetry.capture("cli_rules_reconciled"); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + const code = + error instanceof CLIError && error.code ? error.code : "INTERNAL_ERROR"; + if (args.json) { + console.log(JSON.stringify(makeErrorEnvelope(code, message))); + } else { + console.error(message); } + process.exitCode = 1; } }, }); diff --git a/packages/cli/src/filesystem/migrate.ts b/packages/cli/src/filesystem/migrate.ts index 61a7667d..c51f97f7 100644 --- a/packages/cli/src/filesystem/migrate.ts +++ b/packages/cli/src/filesystem/migrate.ts @@ -27,9 +27,40 @@ export interface TasklessInstallManifest { onboarded?: boolean; } +/** + * What the project's rules were last reconciled against. + * + * Separate from `install` on purpose, because the two answer different + * questions and drift apart. `install` records how the scaffold got here; + * `rules` records what the rules are valid against. Conflating them is what + * made `install.cliVersion` a bad candidate for this: a skills refresh moves + * it without anyone having read a rule. + * + * Every field here advances ONLY on a completed reconciliation, never on an + * upgrade. If a CLI bump silently rewrote `engines.sg` to the newly vendored + * version, the field would always report "current" and the divergence it + * exists to expose would be invisible. + */ +export interface TasklessRulesManifest { + /** CLI version whose ledger entries have all been walked and acted on. */ + reconciledTo?: string; + /** + * Engine versions the rules were authored and last reconciled against. + * + * Engine version is what determines whether matching semantics moved under + * a rule, so recording it is what lets a later differential ask a concrete + * question instead of reconstructing one. + */ + engines?: { + sg?: string; + vale?: string; + }; +} + export interface TasklessManifest { version: number; install?: TasklessInstallManifest; + rules?: TasklessRulesManifest; } const MANIFEST_FILE = "taskless.json"; @@ -127,10 +158,12 @@ export async function readManifest( ): Promise<{ manifest: TasklessManifest; raw: Record }> { const { version, raw } = await readRawManifest(directory); const install = raw.install as TasklessInstallManifest | undefined; + const rules = raw.rules as TasklessRulesManifest | undefined; return { manifest: { version, install: isPlainObject(install) ? install : undefined, + rules: isPlainObject(rules) ? rules : undefined, }, raw, }; @@ -152,6 +185,11 @@ export async function writeManifest( } else { merged.install = manifest.install; } + if (manifest.rules === undefined) { + delete merged.rules; + } else { + merged.rules = manifest.rules; + } await writeRawManifest(directory, merged); } diff --git a/packages/cli/src/rules/reconcile-marker.ts b/packages/cli/src/rules/reconcile-marker.ts new file mode 100644 index 00000000..f54a641a --- /dev/null +++ b/packages/cli/src/rules/reconcile-marker.ts @@ -0,0 +1,163 @@ +import { access } from "node:fs/promises"; +import { join } from "node:path"; + +import { AST_GREP_VERSION, VALE_VERSION } from "./capabilities"; +import { readManifest, writeManifest } from "../filesystem/migrate"; +import { TASKLESS_DIRECTORY } from "./vale/formats"; +import { CLIError } from "../util/cli-error"; +import { getCliVersion } from "../wizard/intro"; + +/** + * Recording that a project's RULES have been reconciled to a CLI version. + * + * Distinct from every other version in the manifest, and deliberately so. + * `install.cliVersion` records which CLI last wrote the scaffold, and the + * scaffold `version` records the layout schema. Both advance without anyone + * reading a rule: a skills refresh moves the first, a layout migration moves + * the second. Keying rule work off either would let an agent skip ledger + * entries it never performed, and it would fail in the quiet direction, since + * the walk would report nothing to do while the rules stayed wrong. + */ + +/** What a completed reconciliation records. */ +export interface ReconcileResult { + reconciledTo: string; + engines: { sg: string; vale: string }; + /** The value replaced, or `undefined` on a project that had never recorded one. */ + previous: string | undefined; +} + +/** + * Compare two dotted version strings numerically, ignoring any prerelease + * suffix. + * + * A nightly is `0.11.0-20260826062304x3c78ffe`, so a plain string comparison + * would sort it after `0.11.0` and let a nightly-built project refuse a + * release-built one. Only the numeric core is compared, which makes a nightly + * and its release equal for this purpose. That is the right answer: they carry + * the same ledger entries. + */ +function versionCore(version: string): number[] { + return (version.split("-")[0] ?? "") + .split(".") + .map((part) => Number.parseInt(part, 10) || 0); +} + +function compareVersions(a: string, b: string): number { + const left = versionCore(a); + const right = versionCore(b); + for (let index = 0; index < Math.max(left.length, right.length); index++) { + const delta = (left[index] ?? 0) - (right[index] ?? 0); + if (delta !== 0) return delta; + } + return 0; +} + +/** + * Record that the ledger walk completed up to `reconciledTo`. + * + * The version is validated rather than trusted. An agent supplies it, and an + * agent can be wrong: the two ways it can be wrong are claiming a version this + * CLI does not have entries for, and moving the marker backwards. Both are + * rejected here rather than written and puzzled over later. + */ +/** + * A dotted numeric version, with an optional prerelease suffix. + * + * Checked BEFORE either comparison, because `versionCore` coerces an + * unparseable segment to `0`: without this, `abc` parses as `[0]`, compares + * lower than any real version, sails past both guards, and is written to the + * manifest verbatim. A pasted SHA or a truncated interpolation would corrupt + * the marker with nothing reported, which is the opposite of the validation + * this function exists to do. + */ +const VERSION_SHAPE = /^\d+(?:\.\d+)*(?:-[\w.]+)?$/; + +export async function recordReconciliation( + cwd: string, + reconciledTo: 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" + ); + } + + const tasklessDirectory = join(cwd, TASKLESS_DIRECTORY); + + // Checked, not created. `readManifest` tolerates a missing file, but + // `writeManifest` writes directly and does not make parent directories, so + // without this the command dies on a raw ENOENT reported as INTERNAL_ERROR. + // + // Creating the directory would be worse than failing: 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 + // states this precondition; this is the code holding to it. + if (!(await pathExists(tasklessDirectory))) { + throw new CLIError( + `No \`${TASKLESS_DIRECTORY}/\` in this project, so there are no rules to reconcile. Run the CLI once to set it up before recording a reconciliation.`, + "INVALID_INPUT" + ); + } + + const { manifest, raw } = await readManifest(tasklessDirectory); + const previous = manifest.rules?.reconciledTo; + + 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.`, + "INVALID_INPUT" + ); + } + + const rules = { + ...manifest.rules, + reconciledTo, + // The engines the rules are now valid against. Recorded here and nowhere + // else, so an upgrade cannot silently refresh them. + engines: { sg: AST_GREP_VERSION, vale: VALE_VERSION }, + }; + + await writeManifest(tasklessDirectory, { ...manifest, rules }, raw); + + return { reconciledTo, engines: rules.engines, previous }; +} + +/** + * Where a ledger walk should start, or `undefined` when there is nothing to + * walk. + * + * A project with no marker has never recorded a reconciliation. That is NOT + * the same as being behind: a project created at the installed version has no + * history, and telling its author to walk every entry ever written would be + * noise. Callers record the current version and do nothing, rather than + * backfilling a claim nobody earned. + */ +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 }; +} + +/** Whether a path exists, without distinguishing why it does not. */ +async function pathExists(path: string): Promise { + try { + await access(path); + return true; + } catch { + return false; + } +} diff --git a/packages/cli/src/schemas/info.ts b/packages/cli/src/schemas/info.ts index 66a58564..b15832ec 100644 --- a/packages/cli/src/schemas/info.ts +++ b/packages/cli/src/schemas/info.ts @@ -35,6 +35,22 @@ export const outputSchema = z.object({ .describe( "GitHub owner segment, or the literal `[unknown]` when none is resolvable" ), + install: z + .object({ + cliVersion: z.string().nullable(), + }) + .describe("How the scaffold got here: the CLI that last wrote it"), + rules: z + .object({ + reconciledTo: z.string().nullable(), + engines: z.object({ + sg: z.string().nullable(), + vale: z.string().nullable(), + }), + }) + .describe( + "What the rules are valid against. Distinct from `install`: these advance only on a completed reconciliation, never on an upgrade" + ), }); export const errorSchema = z.object({ diff --git a/packages/cli/test/init-no-interactive.test.ts b/packages/cli/test/init-no-interactive.test.ts index bfcf51e6..f390e03b 100644 --- a/packages/cli/test/init-no-interactive.test.ts +++ b/packages/cli/test/init-no-interactive.test.ts @@ -94,34 +94,34 @@ describe("taskless init --no-interactive", () => { expect(stdout).toContain("Claude Code (.claude/)"); }); - it("`taskless update` runs the same non-interactive install path", async () => { - await mkdir(join(cwd, ".claude"), { recursive: true }); - + it("falls back to .agents/ when no tools are detected", async () => { const { stdout } = await execFileAsync("node", [ binPath, - "update", + "init", + "--no-interactive", "-d", cwd, ]); - expect(stdout).toContain("Claude Code (.claude/)"); + expect(stdout).toContain("No tools detected. Using fallback: .agents/"); expect( - await exists(join(cwd, ".claude", "skills", "taskless", "SKILL.md")) + await exists(join(cwd, ".agents", "skills", "taskless", "SKILL.md")) ).toBe(true); }); - it("`taskless update` falls back to .agents/ when no tools are detected", async () => { - const { stdout } = await execFileAsync("node", [ - binPath, - "update", - "-d", - cwd, - ]); + it("`taskless update` no longer installs anything", async () => { + // `update` used to be a second name for this install path. It now means + // the rules ledger, and installing is what running the CLI does on its + // own. Pinned because the two words are close enough that a future change + // could quietly wire installing back into it, and nothing else would + // notice: the install would simply start happening again. + await mkdir(join(cwd, ".claude"), { recursive: true }); + + await execFileAsync("node", [binPath, "update", "-d", cwd]); - expect(stdout).toContain("No tools detected. Using fallback: .agents/"); expect( - await exists(join(cwd, ".agents", "skills", "taskless", "SKILL.md")) - ).toBe(true); + await exists(join(cwd, ".claude", "skills", "taskless", "SKILL.md")) + ).toBe(false); }); it("writes taskless.json with install state recorded", async () => { diff --git a/packages/cli/test/reconcile-marker.test.ts b/packages/cli/test/reconcile-marker.test.ts new file mode 100644 index 00000000..c8acdc81 --- /dev/null +++ b/packages/cli/test/reconcile-marker.test.ts @@ -0,0 +1,283 @@ +import { execFile } from "node:child_process"; +import { cp, mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { promisify } from "node:util"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +const execFileAsync = promisify(execFile); +const binPath = resolve(import.meta.dirname, "../dist/index.js"); + +async function runCli( + args: string[] +): Promise<{ stdout: string; stderr: string; exitCode: number }> { + try { + const { stdout, stderr } = await execFileAsync("node", [binPath, ...args]); + return { stdout, stderr, exitCode: 0 }; + } catch (error) { + const failure = error as { + stdout: string; + stderr: string; + code?: number; + }; + return { + stdout: failure.stdout, + stderr: failure.stderr, + exitCode: failure.code ?? 1, + }; + } +} + +/** + * The marker records that RULE CONTENT was reconciled, which nothing else in + * the manifest does. `install.cliVersion` moves on a skills refresh and the + * scaffold `version` moves on a layout migration, both without anyone reading + * a rule, so neither can stand in for this. + */ +describe("recording a rules reconciliation", () => { + let cwd: string; + + beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "taskless-ledger-")); + await cp( + resolve(import.meta.dirname, "../../../.taskless"), + join(cwd, ".taskless"), + { recursive: true } + ); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); + }); + + async function readRules(): Promise | undefined> { + const raw = await readFile(join(cwd, ".taskless", "taskless.json"), "utf8"); + return (JSON.parse(raw) as { rules?: Record }).rules; + } + + async function installedVersion(): Promise { + const result = await runCli(["info", "--json", "-d", cwd]); + return (JSON.parse(result.stdout) as { version: string }).version; + } + + 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, + ]); + expect(result.exitCode).toBe(0); + + const rules = await readRules(); + expect(rules?.reconciledTo).toBe(version); + // Engine versions are the input a later differential needs. Recorded here + // and nowhere else, so an upgrade cannot silently refresh them. + expect(rules?.engines).toEqual({ sg: "0.45.2", vale: "3.18.0" }); + }); + + it("reports the marker through info", async () => { + const version = await installedVersion(); + await runCli(["update", `--reconciledTo=${version}`, "-d", cwd]); + + const info = await runCli(["info", "--json", "-d", cwd]); + const parsed = JSON.parse(info.stdout) as { + rules: { reconciledTo: string | null }; + }; + 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 + // as INTERNAL_ERROR. Checked rather than created: creating it would record + // a reconciliation for a project that has no rules, which is the marker + // 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, + ]); + expect(result.exitCode).not.toBe(0); + const envelope = JSON.parse(result.stdout) as { + code: string; + message: string; + }; + expect(envelope.code).toBe("INVALID_INPUT"); + expect(envelope.code).not.toBe("INTERNAL_ERROR"); + expect(envelope.message).toContain("no rules to reconcile"); + } finally { + await rm(empty, { recursive: true, force: true }); + } + }); + + 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, + ]); + const after = JSON.parse( + await readFile(join(cwd, ".taskless", "taskless.json"), "utf8") + ) as { install?: unknown }; + expect(after.install).toEqual(before.install); + }); +}); + +describe("taskless update with no flags", () => { + it("honours --json, reporting the recipe and where the walk starts", async () => { + // The flag used to be read only on the recording path, so this printed + // plain prose and gave no sign it had done nothing. The walk start is + // computed by the CLI rather than reasoned out of the recipe's prose: + // two places deriving one answer can disagree. + const result = await runCli(["update", "--json"]); + expect(result.exitCode).toBe(0); + const payload = JSON.parse(result.stdout) as { + ok: boolean; + topic: string; + installed: string; + walk: unknown; + recipe: string; + }; + expect(payload.ok).toBe(true); + 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(); + }); + + it("serves the same ledger recipe as `agent update`", async () => { + const direct = await runCli(["update"]); + const viaAgent = await runCli(["agent", "update"]); + expect(direct.exitCode).toBe(0); + expect(direct.stdout).toContain("# Topic: update"); + // One renderer, so the two spellings cannot drift into two different sets + // of instructions. + expect(direct.stdout).toBe(viaAgent.stdout); + }); + + it("tells the reader the layout and the rules are different jobs", async () => { + const result = await runCli(["update"]); + expect(result.stdout).toContain("It is not about installing"); + expect(result.stdout).toContain("The directory is"); + }); + + it("carries the 0.11.0 ledger entry", async () => { + const result = await runCli(["update"]); + expect(result.stdout).toContain("Migrating to 0.11.0"); + // 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"); + expect(result.stdout).toContain("Matching semantics moved"); + }); + + it("warns that kind: link is loud, not a silent zero-match", async () => { + // The correction that took a verified 0.45.2 binary to establish: an + // invalid kind aborts config parsing and takes every other rule down. + const result = await runCli(["update"]); + expect(result.stdout).toContain("HARD\nCONFIG ERROR"); + expect(result.stdout).toContain("exit 8"); + }); +}); diff --git a/skills/taskless/SKILL.md b/skills/taskless/SKILL.md index ab04b829..c48ce5ca 100644 --- a/skills/taskless/SKILL.md +++ b/skills/taskless/SKILL.md @@ -59,7 +59,8 @@ linter, the `create-legacy-rule` path needs nothing installed. | -------------------------- | -------------------------------------- | | Author/create a rule | `npx @taskless/cli agent route` | | First-time install | tell user to run `npx @taskless/cli` | -| Update existing install | `npx @taskless/cli update` | +| Update an existing install | tell user to run `npx @taskless/cli` | +| Fix rules after an upgrade | `npx @taskless/cli update` | | Discover candidate rules | `npx @taskless/cli agent onboard` | | Improve an existing rule | `npx @taskless/cli agent improve-rule` | | Delete a rule | `npx @taskless/cli agent delete-rule` | @@ -67,6 +68,17 @@ linter, the `create-legacy-rule` path needs nothing installed. | Log in, log out, or status | `npx @taskless/cli agent auth` | | Wire into CI | `npx @taskless/cli agent ci` | +Two of those rows look alike and are not. Running `npx @taskless/cli` +migrates the `.taskless/` layout and refreshes the installed skills: that is +the DIRECTORY, and it is automatic. `npx @taskless/cli update` is about the +RULES already in the project, which no migration can rewrite for you, since a +rewriter that now needs a `fix` or a rule whose matching semantics shifted +under a new engine is a question about content rather than layout. + +An agent that has run a migration and watched it succeed will otherwise +reasonably conclude the upgrade is finished. The directory is migrated; the +rules may still need work, and `update` is how to find out. + If the user's intent is ambiguous between two topics, run `npx @taskless/cli agent` (no args) to see the disambiguation table, or ask the user.