diff --git a/SETUP.md b/SETUP.md index b38d53bc..ff211298 100644 --- a/SETUP.md +++ b/SETUP.md @@ -52,7 +52,8 @@ concurrent edits to quest link lists. It adds hook entries running `quest worktree hook` to `.claude/settings.json`, keeping the file's other settings, so Quest creates and reclaims Claude Code's worktrees; Claude Code then needs `quest` on its `PATH` to create any worktree in the repository. It -prints each path it changed and is safe to rerun. +prints each path it changed and is safe to rerun. Rerun after an upgrade, it +also removes the unmodified stubs of skills the new version no longer ships. If init refuses, it names the conflicting path. Report it and ask the user how to proceed; never delete or rename their files to get past it. @@ -88,9 +89,9 @@ work or `/quest-import` to bring in open GitHub issues. 1. Run `quest skill export` and follow it, so active quests become GitHub issues. Skip this only if the user says so. -2. Run `quest uninstall`. It removes only unmodified stubs, the `Quests: ` - line, Quest's hook entries, and the merge driver, and never deletes the - quest tree. +2. Run `quest uninstall`. It removes only unmodified stubs of any version, the + `Quests: ` line, Quest's hook entries, and the merge driver, and never + deletes the quest tree. 3. Remove the pin added above: the mise entry, or the flake input and dev shell package. For an unpinned install, delete the `quest` binary the installer placed (`~/.local/bin/quest` by default, or `quest.exe` in the diff --git a/docs/getting-started.md b/docs/getting-started.md index 3c3adb3d..2826d2d7 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -66,7 +66,8 @@ can become ready. ## Update or remove it Upgrade by changing the pin, then rerun `quest init` to add stubs for new -skills; existing stubs stay as they are. To remove Quest, ask +skills and remove the unmodified stubs of retired ones; existing stubs stay as +they are. To remove Quest, ask your agent to follow SETUP.md to remove Quest. It exports active quests to GitHub issues, runs `quest uninstall`, and drops the pin. Uninstall never deletes your quest tree, and completed plans remain in Git history. diff --git a/quest/m0/README.md b/quest/m0/README.md index 94cb18d3..b5c482c8 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -90,7 +90,6 @@ ownership-manifest design: - [Kix decision UI](/quest/m0/kix/README.md) - planning and implementation through prompts and summaries, run on the maintainer's own runners - [Codex cloud off](/quest/m0/codex-cloud-off.md) - automatic Codex cloud reviews stop once no open PR relies on them - [Review handles expired and environment logins](/quest/m0/review-login.md) - `quest review` says to log in again instead of failing with a 401, and reviews with credentials from the environment -- [Stale skill stubs](/quest/m0/init-prune.md) - `quest init` and `quest uninstall` remove stubs of retired skills such as `quest-ready` - [Bun EFAULT report](/quest/m0/bun-efault.md) - condition: Bun fixes concurrent `rmSync` on Windows; report it, then drop the workaround - [Codex project-config skip](/quest/m0/codex-project-config.md) - condition: Codex keeps a supported way to skip a project's `.codex/` layer; then reviews keep the user's config - [Gate bench](/quest/m0/gate-bench.md) - measure headroom admission on an idle host diff --git a/quest/m0/helpers/headless-agents.md b/quest/m0/helpers/headless-agents.md index 6ae6926b..1735a30e 100644 --- a/quest/m0/helpers/headless-agents.md +++ b/quest/m0/helpers/headless-agents.md @@ -32,8 +32,8 @@ every user would have to add it. invoking it authorizes (starting, resuming, and stopping background agents in isolated worktrees, and with `api`, billing them to the user's keys), since stub text is part of the user turn the classifier reads. That changes the - stub contract ("the stub never changes between versions") once: `init` and - `uninstall` accept only a stub that exactly matches the current text, so they - must also recognize the known older stub text. + stub body once: `init` and `uninstall` recognize a stub of any version by + its body (`STUB` in `src/cli/setup.ts`), so it must accept both the old and + the new body. - The command does no load or CPU admission, and the skill no longer caps agents: the guide routes their expensive commands through `quest gate`. diff --git a/quest/m0/init-prune.md b/quest/m0/init-prune.md deleted file mode 100644 index 1a330066..00000000 --- a/quest/m0/init-prune.md +++ /dev/null @@ -1,22 +0,0 @@ -# [S] quest init removes stale skill stubs - -## Goal - -`quest init` and `quest uninstall` remove Quest skill stubs whose skill -no longer exists, such as `quest-ready`, from `.claude/skills` and -`.agents/skills`. They recognize a stub by its Quest-written content, so a -user's own skill with a `quest-` name is never touched. - -## Plan - -Decided 2026-10-28 (user, /quest-plan of the follow-ups from [moq-dev/quest#271](https://github.com/moq-dev/quest/pull/271)). - -- #271 removed `/quest-ready`, but repositories that ran `quest init` - earlier still list it, and agents invoke it. -- `src/cli/setup.ts` recognizes a stub only by exact match with a current - skill's stub. Recognize a retired one by the stub's shape (frontmatter, then - the ``Run `quest skill ` and follow its output.`` body; the - description varies) for a name `quest skill` no longer has, rather than a - list of retired names. -- Re-running `quest init` after upgrading the pin is how a repository picks - this up; no new check command. diff --git a/quest/m0/skill-stubs.md b/quest/m0/skill-stubs.md index 652cc233..b656439f 100644 --- a/quest/m0/skill-stubs.md +++ b/quest/m0/skill-stubs.md @@ -28,7 +28,3 @@ Decided 2026-11-01 (user, /quest-plan of the follow-ups from the - The refresh amends m0's "Upgrading never rewrites repository files"; the [milestone's Plan](/quest/m0/README.md) records it. `docs/getting-started.md` says "existing stubs stay as they are"; update it. - -## Related - -- [Stale skill stubs](/quest/m0/init-prune.md) - removes stubs of retired skills; this quest refreshes those of current ones diff --git a/src/cli/setup.ts b/src/cli/setup.ts index c99fcc7f..c528fa61 100644 --- a/src/cli/setup.ts +++ b/src/cli/setup.ts @@ -1,7 +1,8 @@ // `quest init` and `quest uninstall`: install skill stubs and reverse it. // -// Init touches only stubs, one reference line, one `.gitattributes` line, an -// empty `quest/README.md` when missing, Quest's hook entries in +// Init touches only stubs (writing missing ones and removing those of retired +// skills), one reference line, one `.gitattributes` line, an empty +// `quest/README.md` when missing, Quest's hook entries in // `.claude/settings.json`, and the clone's merge driver config. Uninstall never // deletes the quest tree. @@ -18,11 +19,11 @@ import { statSync, writeFileSync, } from "node:fs"; -import { dirname, join, resolve } from "node:path"; +import { basename, dirname, join, resolve } from "node:path"; import { gitOutput } from "./claims"; import { DRIVER } from "./resolve"; -import { SKILLS } from "./skills"; +import { PREFIX, SKILLS } from "./skills"; import { HOOK_EVENTS } from "./worktree-hook"; /** @@ -63,7 +64,8 @@ export function init(root: string): string[] { const path = `${dir}/SKILL.md`; checkPath(root, path, "file"); if (isFile(join(root, path))) { - if (isStub(root, path, skill.stub)) return []; + // An older version's stub stays as it is; only a missing one is written. + if (isStub(root, path, skill.installed)) return []; throw new Error(`${path} is not a Quest stub; remove or rename it before running \`quest init\``); } else if (existsSync(join(root, dir))) { throw new Error(`${dir} exists without SKILL.md; remove or rename it before running \`quest init\``); @@ -71,6 +73,8 @@ export function init(root: string): string[] { return [{ path, stub: skill.stub }]; }), ); + const current = new Set(SKILLS.map((skill) => skill.installed)); + const retired = dirs.flatMap((skills) => stubs(root, skills).filter((dir) => !current.has(basename(dir)))); const readme = "quest/README.md"; checkPath(root, readme, "file"); const instructions = ["AGENTS.md", "CLAUDE.md"].find((name) => isFile(join(root, name))) ?? "AGENTS.md"; @@ -85,6 +89,7 @@ export function init(root: string): string[] { writeFileSync(join(root, path), stub); changes.push(path); } + for (const dir of retired) changes.push(removeStub(root, dir)); if (!existsSync(join(root, readme))) { mkdirSync(join(root, "quest"), { recursive: true }); @@ -165,14 +170,7 @@ export function uninstall(root: string): string[] { if (uninstallHooks(root)) changes.push(CLAUDE_SETTINGS); for (const skills of SKILL_DIRS) { if (!isRealDir(join(root, skills))) continue; - for (const skill of SKILLS) { - const dir = `${skills}/${skill.installed}`; - const path = `${dir}/SKILL.md`; - if (!isFile(join(root, path)) || !isStub(root, path, skill.stub)) continue; - rmSync(join(root, path)); - changes.push(path); - removeIfEmpty(join(root, dir)); - } + for (const dir of stubs(root, skills)) changes.push(removeStub(root, dir)); removeIfEmpty(join(root, skills)); } // Remove only the conventional link from an older Quest install whose target went away. @@ -213,10 +211,11 @@ function skillDirs(root: string): string[] { } /** What sits at `path` without following a symlink, or `null` for nothing. */ -function kind(path: string): "dir" | "symlink" | "other" | null { +function kind(path: string): "dir" | "file" | "symlink" | "other" | null { try { const stat = lstatSync(path); - return stat.isDirectory() ? "dir" : stat.isSymbolicLink() ? "symlink" : "other"; + if (stat.isDirectory()) return "dir"; + return stat.isFile() ? "file" : stat.isSymbolicLink() ? "symlink" : "other"; } catch { return null; } @@ -247,9 +246,49 @@ function read(root: string, path: string): string { return readFileSync(join(root, path), "utf8"); } -// Git may check out unchanged stubs with CRLF in an adopting repository. -function isStub(root: string, path: string, stub: string): boolean { - return read(root, path).replaceAll("\r\n", "\n") === stub; +/** + * Any version's stub: frontmatter naming the skill, then the pointer to + * `quest skill`. Descriptions and the setup link change between versions, so + * only this shape identifies a file Quest wrote. Skill frontmatter has only + * ever held `name` and `description`, so any other field marks an edited stub. + */ +const STUB = + /^---\nname: (\S+)\n(?:(?:name|description): .*\n)*---\n\nRun `quest skill (\S+)` and follow its output\.\nIf `quest` is not installed, follow \S+ first\.\n$/; + +/** + * Whether `path` holds a Quest stub of any version for the skill installed as + * `installed`. Git may check out unchanged stubs with CRLF in an adopting repository. + */ +function isStub(root: string, path: string, installed: string): boolean { + const match = STUB.exec(read(root, path).replaceAll("\r\n", "\n")); + return match?.[1] === installed && `${PREFIX}${match[2]}` === installed; +} + +/** + * The skill directories under `skills`, relative to `root`, that hold a Quest + * stub of any version, including skills this version no longer ships. Quest + * writes neither links nor other files there, so it skips symlinks. + */ +function stubs(root: string, skills: string): string[] { + if (kind(join(root, skills)) === null) return []; + return readdirSync(join(root, skills)) + .filter((name) => name.startsWith(PREFIX)) + .map((name) => `${skills}/${name}`) + .filter( + (dir) => + isRealDir(join(root, dir)) && + kind(join(root, dir, "SKILL.md")) === "file" && + isStub(root, `${dir}/SKILL.md`, basename(dir)), + ) + .sort(); +} + +/** Delete the stub in skill directory `dir`, and the directory once empty. Returns the stub's path. */ +function removeStub(root: string, dir: string): string { + const path = `${dir}/SKILL.md`; + rmSync(join(root, path)); + removeIfEmpty(join(root, dir)); + return path; } function newline(content: string): string { diff --git a/src/cli/skills.ts b/src/cli/skills.ts index f94c7606..2a9d5b98 100644 --- a/src/cli/skills.ts +++ b/src/cli/skills.ts @@ -1,9 +1,9 @@ // The agent skills and the quest guide, bundled into the CLI. // // A repository installs only a stub per skill, which asks the agent to run -// `quest skill `. The stub never changes between versions, so the -// version a repository pins decides which instructions its agents follow and -// upgrading never rewrites repository files. +// `quest skill `. The stub only points at the binary, so the version a +// repository pins decides which instructions its agents follow, and upgrading +// never rewrites repository files. import guide from "../../assets/AGENTS.md"; import afk from "../../assets/skills/afk.md"; @@ -28,7 +28,7 @@ export const GUIDE: string = guide; const SETUP = "https://github.com/moq-dev/quest/blob/main/SETUP.md"; /** Installed skills carry this prefix so they don't collide with a repository's own. */ -const PREFIX = "quest-"; +export const PREFIX = "quest-"; /** One skill: YAML frontmatter, then the instructions. */ export class Skill { diff --git a/test/init.test.ts b/test/init.test.ts index 7f8af4e5..8048e687 100644 --- a/test/init.test.ts +++ b/test/init.test.ts @@ -244,6 +244,102 @@ test("init keeps a reworded reference line", () => { expect(read(dir, "AGENTS.md")).toBe("# Repo\n"); }); +/** The stub an earlier release installed for `/quest-ready`, before `/quest-review` replaced it. */ +const READY_STUB = + "---\nname: quest-ready\ndescription: Mark a draft PR ready for review, wait for an automated review, then decide whether to merge.\n---\n\nRun `quest skill ready` and follow its output.\nIf `quest` is not installed, follow https://github.com/kixelated/quest/blob/main/SETUP.md first.\n"; + +/** A current skill's stub as an earlier release wrote it, with another description and setup link. */ +const OLD_AUDIT_STUB = + "---\nname: quest-audit\ndescription: Audit outstanding quests for disagreements, then resolve them with /quest-plan.\n---\n\nRun `quest skill audit` and follow its output.\nIf `quest` is not installed, follow https://github.com/kixelated/quest/blob/main/SETUP.md first.\n"; + +function write(dir: string, path: string, content: string) { + mkdirSync(dirname(join(dir, path)), { recursive: true }); + writeFileSync(join(dir, path), content); +} + +test("init removes stubs of retired skills and keeps older stubs of current ones", () => { + const dir = tempDir(); + init(dir); + for (const skills of [".claude/skills", ".agents/skills"]) { + write( + dir, + `${skills}/quest-ready/SKILL.md`, + READY_STUB.replaceAll("\n", skills === ".agents/skills" ? "\r\n" : "\n"), + ); + write(dir, `${skills}/quest-audit/SKILL.md`, OLD_AUDIT_STUB); + } + expect(init(dir)).toEqual([".claude/skills/quest-ready/SKILL.md", ".agents/skills/quest-ready/SKILL.md"]); + for (const skills of [".claude/skills", ".agents/skills"]) { + expect(existsSync(join(dir, skills, "quest-ready"))).toBe(false); + expect(read(dir, `${skills}/quest-audit/SKILL.md`)).toBe(OLD_AUDIT_STUB); + } + expect(init(dir)).toEqual([]); +}); + +test("init keeps a retired stub's directory when it holds other files", () => { + const dir = tempDir(); + write(dir, ".claude/skills/quest-ready/SKILL.md", READY_STUB); + write(dir, ".claude/skills/quest-ready/notes.md", "Mine.\n"); + expect(init(dir)).toContain(".claude/skills/quest-ready/SKILL.md"); + expect(readdirSync(join(dir, ".claude/skills/quest-ready"))).toEqual(["notes.md"]); +}); + +for (const [name, content] of [ + [ + "a user's own quest- skill", + "---\nname: quest-ready\ndescription: My release checklist.\n---\n\nCheck the changelog.\n", + ], + ["a stub under another directory name", READY_STUB.replace("quest-ready", "quest-mine")], + ["a stub naming another skill", READY_STUB.replace("`quest skill ready`", "`quest skill mine`")], + ["a stub with added instructions", `${READY_STUB}Then tell the team.\n`], + ["a stub with an added frontmatter field", READY_STUB.replace("\n---\n\n", "\nallowed-tools: Bash\n---\n\n")], +] as const) + test(`init and uninstall keep ${name}`, () => { + const dir = tempDir(); + write(dir, ".claude/skills/quest-ready/SKILL.md", content); + init(dir); + uninstall(dir); + expect(read(dir, ".claude/skills/quest-ready/SKILL.md")).toBe(content); + }); + +test("init leaves a user's skill without the quest- prefix", () => { + const dir = tempDir(); + const ready = READY_STUB.replace("name: quest-ready", "name: ready"); + write(dir, ".claude/skills/ready/SKILL.md", ready); + init(dir); + expect(read(dir, ".claude/skills/ready/SKILL.md")).toBe(ready); +}); + +test.skipIf(process.platform === "win32")("init and uninstall leave linked retired stubs", () => { + const dir = tempDir(); + write(dir, "elsewhere/quest-ready/SKILL.md", READY_STUB); + write(dir, "stub.md", READY_STUB); + mkdirSync(join(dir, ".claude/skills/quest-other"), { recursive: true }); + symlinkSync("../../elsewhere/quest-ready", join(dir, ".claude/skills/quest-ready")); + symlinkSync("../../../stub.md", join(dir, ".claude/skills/quest-other/SKILL.md")); + init(dir); + uninstall(dir); + expect(read(dir, "elsewhere/quest-ready/SKILL.md")).toBe(READY_STUB); + expect(isSymlink(dir, ".claude/skills/quest-ready")).toBe(true); + expect(isSymlink(dir, ".claude/skills/quest-other/SKILL.md")).toBe(true); +}); + +test("uninstall removes stubs that older versions installed", () => { + const dir = tempDir(); + for (const skills of [".claude/skills", ".agents/skills"]) { + write(dir, `${skills}/quest-ready/SKILL.md`, READY_STUB); + write(dir, `${skills}/quest-audit/SKILL.md`, OLD_AUDIT_STUB); + } + expect(uninstall(dir)).toEqual([ + ".claude/skills/quest-audit/SKILL.md", + ".claude/skills/quest-ready/SKILL.md", + ".agents/skills/quest-audit/SKILL.md", + ".agents/skills/quest-ready/SKILL.md", + ]); + expect(existsSync(join(dir, ".claude"))).toBe(false); + expect(existsSync(join(dir, ".agents"))).toBe(false); +}); + const QUEST_HOOK = { type: "command", command: HOOK_COMMAND }; const SESSION_HOOK = { hooks: [{ type: "command", command: "bash .claude/hooks/direnv.sh" }] };