From 2e0e8ab53bf600e730b41036a78ea7fa9026289e Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Fri, 28 Aug 2026 22:01:09 -0700 Subject: [PATCH 1/7] perf: bundle mutually-exclusive reviewer loops instead of inlining them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent Skills environments (Grok Build, Codex, Antigravity) have no runtime `!cat`, so the transformer inlined every referenced lib into SKILL.md. That put all four reviewer BACKEND loops — copilot, github, local-agent, ollama — into each of the seven commands that run a review, ~130KB (~33K tokens) of which a run uses at most one. `/do:better-swift` reached 414KB, and the whole skill set 2.8MB. Those four are now written into the skill's own `lib/` directory and cited by a relative path with an imperative read directive; the dispatcher (`multi-reviewer-loop.md`) stays inline, since it is always on the taken path and is what names the backend to load. Skill set drops to 1.82MB (-35%); per-command 32-52% on everything that reviews. Supporting changes: - A bundled child is transformed against the parent's present-set, so it cites the dispatcher it was split from rather than re-appending it. Without this each backend file came out larger than its source and the split saved nothing at read time. - The backticked see-also form (`lib/x.md`) now resolves too, since that token denotes a real bundled file once bundling exists. It renames only — routing it through the appendix queue pulled whole docs into skills that merely name-drop them, inflating /do:config from 19KB to 91KB. - Uninstall removes the bundle directory rather than stranding it beside a deleted SKILL.md. Claude Code and OpenCode keep runtime `!cat` and never reach this path; their output is byte-identical, verified against all 21 installed command files. Content reachability verified across 8657 substantive lines: nothing dropped. --- PRD.md | 2 +- README.md | 2 +- src/environments.js | 12 ++++ src/installer.js | 71 ++++++++++++++++++- src/transformer.js | 119 +++++++++++++++++++++++++++++-- test/installer.test.js | 99 ++++++++++++++++++++++++++ test/transformer.test.js | 147 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 442 insertions(+), 10 deletions(-) diff --git a/PRD.md b/PRD.md index 8258c84d..5005273a 100644 --- a/PRD.md +++ b/PRD.md @@ -50,7 +50,7 @@ Aligned with [GOALS.md](./GOALS.md)'s Core Goals: | FR-2 | The system MUST support installing into an explicit subset of environments via `--env ` (comma-separated, case-insensitive, trimmed), including the aliases `gemini`/`agy` → `antigravity`. | Must | `--env CLAUDE,agy` installs into the claude and antigravity targets only. | | FR-3 | The system MUST support installing/uninstalling a filtered subset of commands by name, accepting both bare (`push`) and `do:`-prefixed (`do:push`) forms. | Must | `npx slash-do@latest push pr` installs only the push and pr commands. | | FR-4 | The system MUST transform each source command file into the native format of its target environment — subdirectory layout for Claude Code, flat `do-.md` for OpenCode, directory-per-skill (`SKILL.md`) for Antigravity/Codex/Grok — without per-environment hand-authoring. | Must | A single source file in `commands/do/` produces a correctly formatted, working command/skill file in every installed environment. | -| FR-5 | For environments without `!cat` file-inclusion support, the system MUST inline referenced `lib/*.md` content directly into the transformed command file (recursively, with cycle termination) rather than leaving a broken path reference. | Must | No dangling `~/.claude/lib/.md` references appear in Agent Skills output, even when the referenced file is missing. | +| FR-5 | For environments without `!cat` file-inclusion support, the system MUST make every referenced `lib/*.md` reachable — inlining content into the transformed command file (recursively, with cycle termination), or, for a mutually-exclusive lib the run selects at most one of, writing it into the skill's own `lib/` directory and citing it by a relative path the agent reads on demand. Neither may leave a broken path reference. | Must | No dangling `~/.claude/lib/.md` references appear in Agent Skills output, even when the referenced file is missing; every `lib/.md` cited in a SKILL.md resolves to a file installed beside it. | | FR-6 | Re-running install MUST be idempotent — unchanged files report up to date, changed files are updated in place, and hooks/config are not duplicated. | Must | Two consecutive installs with no source changes report 0 updates on the second run. | | FR-7 | `--dry-run` MUST preview changes without writing, creating, or deleting any file, directory, hook registration, or config entry. | Must | A dry-run install on a clean host leaves the filesystem byte-for-byte unchanged. | | FR-8 | `--list` MUST show all commands and their install status per environment without making changes. | Must | Output lists every command with an installed/not-installed/outdated status per detected environment. | diff --git a/README.md b/README.md index 6315a2d9..825635d4 100644 --- a/README.md +++ b/README.md @@ -454,7 +454,7 @@ npx slash-do@latest push pr release # install specific commands only +------------------+ | Transformer | Converts format per environment: | | - YAML frontmatter (Claude, OpenCode) - +------------------+ - Agent Skills / SKILL.md with inlined libs (Antigravity, Codex, Grok Build) + +------------------+ - Agent Skills / SKILL.md + bundled lib/ (Antigravity, Codex, Grok Build) | v +------------------+ diff --git a/src/environments.js b/src/environments.js index c3987e06..2e3f91d3 100644 --- a/src/environments.js +++ b/src/environments.js @@ -78,6 +78,10 @@ const ENVIRONMENTS = { ext: null, namespacing: 'directory', libPathPrefix: null, + // Agent Skills environment: no runtime `!cat`, but the installer writes lib docs + // into each skill directory, so mutually-exclusive libs can be cited by + // path and read on demand instead of inlined into every SKILL.md. + bundlesLibs: true, supportsHooks: false, supportsCatInclusion: false, supportsTeams: false, @@ -94,6 +98,10 @@ const ENVIRONMENTS = { ext: null, namespacing: 'directory', libPathPrefix: null, + // Agent Skills environment: no runtime `!cat`, but the installer writes lib docs + // into each skill directory, so mutually-exclusive libs can be cited by + // path and read on demand instead of inlined into every SKILL.md. + bundlesLibs: true, supportsHooks: false, supportsCatInclusion: false, supportsTeams: false, @@ -114,6 +122,10 @@ const ENVIRONMENTS = { ext: null, namespacing: 'directory', libPathPrefix: null, + // Agent Skills environment: no runtime `!cat`, but the installer writes lib docs + // into each skill directory, so mutually-exclusive libs can be cited by + // path and read on demand instead of inlined into every SKILL.md. + bundlesLibs: true, supportsHooks: false, supportsCatInclusion: false, supportsTeams: false, diff --git a/src/installer.js b/src/installer.js index 75ac1256..39d0f122 100644 --- a/src/installer.js +++ b/src/installer.js @@ -2,7 +2,7 @@ const fs = require('fs'); const path = require('path'); -const { getTargetFilename, transformCommand, transformLib } = require('./transformer'); +const { getTargetFilename, transformCommand, transformLib, BUNDLED_LIB_DIR } = require('./transformer'); const { readConfig, writeConfig } = require('./config'); const { registerHooksInSettings, @@ -223,6 +223,37 @@ function finalizeInstall(env, hookFiles, packageDir, dryRun, filterNames, autoUp writeVersionAndRefreshCache(env, packageDir, dryRun); } +// Writes the lib docs a command defers into `/lib/`, so the read +// directive in SKILL.md points at a file that exists. Transitive: a bundled lib +// may itself defer a sibling backend (local-agent cites ollama), so drain the set +// as it grows rather than iterating a snapshot. +function syncBundledLibs(commands, bundledByCommand, libDir, env, dryRun, results) { + for (const cmd of commands) { + const entry = bundledByCommand.get(cmd.relPath); + if (!entry || entry.bundled.size === 0) continue; + const { bundled: pending, present } = entry; + + const skillDir = path.dirname(path.join(env.commandsDir, getTargetFilename(cmd.relPath, env))); + const written = new Set(); + // `pending` grows while draining when a bundled lib defers another. + while (written.size < pending.size) { + for (const filename of Array.from(pending)) { + if (written.has(filename)) continue; + written.add(filename); + const absPath = path.join(libDir, filename); + if (!fs.existsSync(absPath)) continue; + syncFile({ + label: `/do:${cmd.name} ${BUNDLED_LIB_DIR}/${filename}`, + content: transformLib(fs.readFileSync(absPath, 'utf8'), env, libDir, { bundled: pending, present }), + targetPath: path.join(skillDir, BUNDLED_LIB_DIR, filename), + dryRun, + results, + }); + } + } + } +} + function install({ env, packageDir, filterNames, dryRun, uninstall, autoUpdate }) { const commandsDir = path.join(packageDir, 'commands'); const libDir = path.join(packageDir, 'lib'); @@ -241,14 +272,30 @@ function install({ env, packageDir, filterNames, dryRun, uninstall, autoUpdate } return doUninstall(filtered, libFiles, hookFiles, env, results, dryRun, filterNames); } + // Per-command set of lib docs to write beside its SKILL.md. Filled while each + // command is transformed (getContent runs for every item, up-to-date ones + // included), then drained below. + const bundledByCommand = new Map(); + syncFileSet(filtered, { - getContent: cmd => transformCommand(fs.readFileSync(cmd.absPath, 'utf8'), env, libDir, cmd.relPath), + getContent: (cmd) => { + const bundled = new Set(); + const present = new Set(); + const content = transformCommand( + fs.readFileSync(cmd.absPath, 'utf8'), env, libDir, cmd.relPath, { bundled, present }); + if (env.bundlesLibs) bundledByCommand.set(cmd.relPath, { bundled, present }); + return content; + }, getTargetPath: cmd => path.join(env.commandsDir, getTargetFilename(cmd.relPath, env)), getLabel: cmd => `/do:${cmd.name}`, dryRun, results, }); + if (env.bundlesLibs) { + syncBundledLibs(filtered, bundledByCommand, libDir, env, dryRun, results); + } + if (env.libDir) { syncFileSet(libFiles, { getContent: lib => transformLib(fs.readFileSync(lib.absPath, 'utf8'), env), @@ -282,6 +329,26 @@ function doUninstall(commands, libFiles, hookFiles, env, results, dryRun, filter results, }); + // Bundled lib docs live INSIDE the skill directory, so removing SKILL.md alone + // would strand them (and leave the directory behind). Remove every file the + // bundle dir holds, then the now-empty dir. + if (env.bundlesLibs) { + for (const cmd of commands) { + const skillDir = path.dirname(path.join(env.commandsDir, getTargetFilename(cmd.relPath, env))); + const bundleDir = path.join(skillDir, BUNDLED_LIB_DIR); + if (!fs.existsSync(bundleDir)) continue; + for (const name of fs.readdirSync(bundleDir)) { + removeFile({ + label: `/do:${cmd.name} ${BUNDLED_LIB_DIR}/${name}`, + targetPath: path.join(bundleDir, name), + dryRun, + results, + }); + } + if (!dryRun && fs.readdirSync(bundleDir).length === 0) fs.rmdirSync(bundleDir); + } + } + if (env.libDir) { removeFileSet(libFiles, { getTargetPath: lib => path.join(env.libDir, lib.relPath), diff --git a/src/transformer.js b/src/transformer.js index ddb53c4c..44867fac 100644 --- a/src/transformer.js +++ b/src/transformer.js @@ -108,6 +108,48 @@ const LIB_MD_LINK_RE = /\[[^\]]*\]\((?:\.\.\/)+lib\/([A-Za-z0-9._-]+\.md)\)/g; // rather than silently mis-rewriting. Teach this to resolve source-relative paths if // that invariant ever needs to be relaxed. const LIB_SIBLING_LINK_RE = /\[[^\]]*\]\(\.\/([A-Za-z0-9._-]+\.md)\)/g; +// Matches a BACKTICKED bare citation, e.g. `` `lib/multi-reviewer-loop.md` `` — the +// "full mechanics in X" pointers the command specs use. Harmless prose while every +// lib was inlined, but once `lib/.md` denotes a real bundled file beside +// SKILL.md the same token reads as a path, and a non-bundled one is then a dangling +// reference. Resolved like the other prose forms: to a bundled path when the lib is +// deferred (the file exists), to a bare doc name when it is inlined nearby. Guarded +// on the name existing under lib/, so an unrelated `lib/...md` in a shell snippet is +// left alone. +const LIB_BACKTICK_RE = /`lib\/([A-Za-z0-9._-]+\.md)`/g; + +// Reviewer BACKEND loops are mutually exclusive: one `--review-with` entry +// dispatches to exactly ONE of these per reviewer, yet inlining all four costs +// ~130KB (~33K tokens) in every command that runs a review — `/do:better`, +// `/do:better-swift`, `/do:review`, `/do:pr`, `/do:release`, `/do:depfree`, +// `/do:rpr`. The dispatcher (`multi-reviewer-loop.md`) stays inline because it is +// always on the taken path and is what names the backend to load; the backends +// themselves are written as sibling docs beside SKILL.md and cited by path, for +// the agent to read on demand. Environments with runtime `!cat` (Claude/OpenCode) +// never reach this path and are unaffected. +const DEFERRED_LIBS = new Set([ + 'copilot-review-loop.md', + 'github-reviewer-loop.md', + 'local-agent-review-loop.md', + 'ollama-review-loop.md', +]); + +// Subdirectory, relative to a skill's own directory, that bundled lib docs are +// written into by the installer. Cited from SKILL.md as `lib/.md`. +const BUNDLED_LIB_DIR = 'lib'; + +// The read directive that replaces a deferred lib's inline content. Written as an +// imperative instruction rather than a passive link: the agent must treat it as a +// required read, not an optional reference, or the loop it names is lost. +function deferredLibDirective(ref, name) { + return [ + `> **Read \`${ref}\` now — required.** The full ${name} procedure lives in that`, + '> file, bundled alongside this skill. Read it in full before running this loop and', + '> follow it exactly. Do NOT improvise the loop from the summary above: the file', + '> carries the load-bearing detail (exit codes, verdict parsing, iteration caps,', + '> convergence and push rules) that the summary deliberately omits.', + ].join('\n'); +} // For Agent Skills environments (Codex/Antigravity/Grok — `libDir: null`, no // runtime `!cat`, and no `~/.claude/lib/` on disk for a host-only user), make @@ -124,10 +166,24 @@ const LIB_SIBLING_LINK_RE = /\[[^\]]*\]\(\.\/([A-Za-z0-9._-]+\.md)\)/g; // and cycle-safe — so the load-bearing detail is available host-side. // Claude/OpenCode never reach this path (they keep runtime `~/.claude/lib/` via // cat inclusion), so their output is unchanged. -function inlineLibReferences(body, libDir) { +function inlineLibReferences(body, libDir, opts = {}) { const inlined = new Set(); // libs whose full content is present in the document const queued = new Set(); // libs already appended or scheduled for the appendix const appendQueue = []; // ordered absent-but-cited libs to inline as appendix + // Deferral is opt-in per environment (`bundlesLibs`). When off, every code path + // below behaves exactly as before, so Claude/OpenCode output is byte-identical. + const deferred = opts.bundlesLibs ? DEFERRED_LIBS : new Set(); + // Every lib the installer must write beside SKILL.md, filled as they are cited. + const bundled = opts.bundled instanceof Set ? opts.bundled : new Set(); + const fromLibDir = opts.fromLibDir === true; + // Libs the READER already has in front of them (the parent SKILL.md's inlined + // content and appendix). A bundled child cites those by name instead of + // re-inlining them — without this, every backend file re-appends the whole + // dispatcher it was split away from, and the split saves nothing on read. + const present = opts.present instanceof Set ? opts.present : new Set(); + // Reports back everything this document makes available, so a parent transform + // can hand its own set to the children it bundles. + const presentOut = opts.presentOut instanceof Set ? opts.presentOut : null; const readLib = (filename) => { const libFile = path.join(libDir, filename); @@ -138,20 +194,48 @@ function inlineLibReferences(body, libDir) { const inlineCatIncludes = (text) => text.replace(LIB_CAT_RE, (match, filename) => { const content = readLib(filename); if (content === null) return match; + // A deferred backend is bundled as its own file and cited, never inlined. + if (deferred.has(filename)) { + bundled.add(filename); + return deferredLibDirective(bundledRef(filename), bareName(filename)); + } inlined.add(filename); return content; }); + // Where a bundled lib lives from the citing document: `lib/.md` from a + // SKILL.md, `./.md` from a doc already inside that bundle directory. + const bundledRef = (filename) => + (fromLibDir ? `./${filename}` : `${BUNDLED_LIB_DIR}/${filename}`); + const bareName = (filename) => filename.replace(/\.md$/, ''); + // Rewrite a cited lib to its bare doc name, queueing any cited-but-absent lib // (one never `!cat`-inlined) for the appendix so its content is available. const queueAndName = (filename) => { - if (!inlined.has(filename) && !queued.has(filename) && readLib(filename) !== null) { + // A deferred backend must never reach the appendix — that would re-inline the + // very content the deferral exists to keep out. Cite the bundled file instead, + // which is a path the agent can actually open. + if (deferred.has(filename) && readLib(filename) !== null) { + bundled.add(filename); + return bundledRef(filename); + } + if (!present.has(filename) && !inlined.has(filename) && !queued.has(filename) + && readLib(filename) !== null) { queued.add(filename); appendQueue.push(filename); } - return filename.replace(/\.md$/, ''); + return bareName(filename); }; + // The backticked "full mechanics in `lib/x.md`" form is a SEE-ALSO pointer, not a + // demand for the content. Routing it through queueAndName would drag the whole doc + // (and its transitive citations) into the appendix of every skill that merely + // name-drops it — which inflated /do:config from 19KB to 91KB. So: cite the real + // bundled path when this skill actually bundles the file, otherwise strip it to a + // bare doc name and pull in nothing. + const nameOnly = (filename) => + (bundled.has(filename) ? bundledRef(filename) : bareName(filename)); + // Resolve all three citation forms — relative Markdown links, intra-lib sibling // links, and `~/.claude/lib/` prose refs — to bare doc names. Links are handled // first so their `lib/.md` link text isn't matched by the prose regex @@ -163,6 +247,8 @@ function inlineLibReferences(body, libDir) { .replace(LIB_MD_LINK_RE, (match, filename) => queueAndName(filename)) .replace(LIB_SIBLING_LINK_RE, (match, filename) => readLib(filename) === null ? match : queueAndName(filename)) + .replace(LIB_BACKTICK_RE, (match, filename) => + readLib(filename) === null ? match : `\`${nameOnly(filename)}\``) .replace(LIB_PROSE_RE, (match, filename) => queueAndName(filename)); // Main body: inline includes first, then resolve the prose refs left behind @@ -182,6 +268,11 @@ function inlineLibReferences(body, libDir) { sections.push(`### ${filename.replace(/\.md$/, '')}\n\n${content}`); } + if (presentOut) { + for (const f of inlined) presentOut.add(f); + for (const f of queued) presentOut.add(f); + } + if (sections.length === 0) return out; const appendix = @@ -246,7 +337,7 @@ function getTargetFilename(relPath, env) { } } -function transformCommand(content, env, sourceLibDir, relPath) { +function transformCommand(content, env, sourceLibDir, relPath, opts = {}) { const { frontmatter, body } = parseFrontmatter(content); let transformedBody = body; @@ -260,7 +351,11 @@ function transformCommand(content, env, sourceLibDir, relPath) { if (env.supportsCatInclusion && env.libPathPrefix) { transformedBody = rewriteLibPaths(transformedBody, env.libPathPrefix); } else if (!env.supportsCatInclusion && sourceLibDir) { - transformedBody = inlineLibReferences(transformedBody, sourceLibDir); + transformedBody = inlineLibReferences(transformedBody, sourceLibDir, { + bundlesLibs: env.bundlesLibs === true, + bundled: opts.bundled, + presentOut: opts.present, + }); } // Run on the full body (after inlining) so config-path tokens that arrived via @@ -289,16 +384,28 @@ function transformCommand(content, env, sourceLibDir, relPath) { return header + '\n' + transformedBody; } -function transformLib(content, env) { +function transformLib(content, env, sourceLibDir, opts = {}) { let transformed = rewriteClaudeRootPaths(content, env); if (env.supportsCatInclusion && env.libPathPrefix) { transformed = rewriteLibPaths(transformed, env.libPathPrefix); + } else if (!env.supportsCatInclusion && sourceLibDir) { + // A bundled lib is read standalone, so its own citations must resolve the same + // way SKILL.md's do. `fromLibDir` makes sibling references relative to the lib + // directory the file itself lives in (`./x.md`, not `lib/x.md`). + transformed = inlineLibReferences(transformed, sourceLibDir, { + bundlesLibs: env.bundlesLibs === true, + bundled: opts.bundled, + present: opts.present, + fromLibDir: true, + }); } transformed = rewriteConfigPath(transformed, env); return applyConditionalBlocks(transformed, env); } module.exports = { + DEFERRED_LIBS, + BUNDLED_LIB_DIR, parseFrontmatter, rewriteLibPaths, rewriteConfigPath, diff --git a/test/installer.test.js b/test/installer.test.js index a0d3a468..5fb372a3 100644 --- a/test/installer.test.js +++ b/test/installer.test.js @@ -773,3 +773,102 @@ describe('renamed command cleanup', () => { cleanup(tmpDir); }); }); + +// ── bundled lib docs (Agent Skills envs) ──────────────────────────── + +describe('bundled lib docs', () => { + const { DEFERRED_LIBS, BUNDLED_LIB_DIR } = require('../src/transformer'); + + function makeSkillEnv() { + const { tmpDir, env } = makeTmpEnv({ + namespacing: 'directory', + ext: null, + libDir: null, + libPathPrefix: null, + hooksDir: null, + supportsHooks: false, + supportsCatInclusion: false, + }); + env.bundlesLibs = true; + return { tmpDir, env }; + } + + it('writes each deferred lib beside the SKILL.md that cites it', () => { + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + const bundleDir = path.join(env.commandsDir, 'do-pr', BUNDLED_LIB_DIR); + assert.ok(fs.existsSync(bundleDir), 'do-pr must get a bundle dir'); + const written = fs.readdirSync(bundleDir); + for (const name of DEFERRED_LIBS) { + assert.ok(written.includes(name), `${name} must be bundled with /do:pr`); + } + } finally { cleanup(tmpDir); } + }); + + it('every cited bundle path resolves to a file on disk', () => { + // The read directive is only as good as the path it names — a dangling one + // silently drops the whole reviewer loop. + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + for (const skill of fs.readdirSync(env.commandsDir)) { + const skillFile = path.join(env.commandsDir, skill, 'SKILL.md'); + if (!fs.existsSync(skillFile)) continue; + const body = fs.readFileSync(skillFile, 'utf8'); + const cited = [...body.matchAll(/`(lib\/[A-Za-z0-9._-]+\.md)`/g)].map(m => m[1]); + for (const ref of new Set(cited)) { + assert.ok(fs.existsSync(path.join(env.commandsDir, skill, ref)), + `${skill}/SKILL.md cites ${ref}, which was not written`); + } + } + } finally { cleanup(tmpDir); } + }); + + it('keeps deferred bodies out of SKILL.md but reachable in the bundle', () => { + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + const skillDir = path.join(env.commandsDir, 'do-pr'); + const body = fs.readFileSync(path.join(skillDir, 'SKILL.md'), 'utf8'); + const source = fs.readFileSync( + path.join(PACKAGE_DIR, 'lib', 'ollama-review-loop.md'), 'utf8'); + // A long, distinctive line from the backend: present in the bundle, absent + // from the skill body it was split out of. + const marker = source.split('\n').filter(l => l.trim().length > 100)[0].trim(); + assert.ok(!body.includes(marker), 'deferred body must not remain in SKILL.md'); + const bundled = fs.readFileSync( + path.join(skillDir, BUNDLED_LIB_DIR, 'ollama-review-loop.md'), 'utf8'); + assert.ok(bundled.includes(marker), 'deferred body must survive in the bundle'); + } finally { cleanup(tmpDir); } + }); + + it('does not bundle anything for a cat-inclusion environment', () => { + const { tmpDir, env } = makeTmpEnv({ namespacing: 'directory', ext: null }); + try { + install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + assert.ok(!fs.existsSync(path.join(env.commandsDir, 'do-pr', BUNDLED_LIB_DIR))); + } finally { cleanup(tmpDir); } + }); + + it('removes bundled libs on uninstall', () => { + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + const bundleDir = path.join(env.commandsDir, 'do-pr', BUNDLED_LIB_DIR); + assert.ok(fs.existsSync(bundleDir)); + install({ env, packageDir: PACKAGE_DIR, dryRun: false, uninstall: true }); + assert.ok(!fs.existsSync(bundleDir), 'bundle dir must not be stranded'); + } finally { cleanup(tmpDir); } + }); + + it('reports bundled libs as up to date on a second install', () => { + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + const second = install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + assert.equal(second.installed, 0); + assert.equal(second.updated, 0); + } finally { cleanup(tmpDir); } + }); +}); diff --git a/test/transformer.test.js b/test/transformer.test.js index 7f8ac157..d1ee28d6 100644 --- a/test/transformer.test.js +++ b/test/transformer.test.js @@ -686,3 +686,150 @@ describe('transformLib', () => { ); }); }); + +// ── deferred (bundled) reviewer-backend libs ──────────────────────── + +describe('deferred lib bundling (bundlesLibs)', () => { + const { DEFERRED_LIBS, BUNDLED_LIB_DIR } = require('../src/transformer'); + + // Use a real deferred name so the production DEFERRED_LIBS set is exercised + // rather than a fixture-only substitute. + const BACKEND = 'ollama-review-loop.md'; + const SENTINEL = 'OLLAMA-BACKEND-BODY-SENTINEL'; + + function withLibs(libs, fn) { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'slashdo-defer-')); + for (const [name, content] of Object.entries(libs)) { + fs.writeFileSync(path.join(tmpDir, name), content, 'utf8'); + } + try { + return fn(tmpDir); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + } + + it('names every deferred lib as a file that exists in the repo', () => { + for (const name of DEFERRED_LIBS) { + assert.ok( + fs.existsSync(path.join(__dirname, '..', 'lib', name)), + `DEFERRED_LIBS names ${name}, which is not a file under lib/`); + } + }); + + it('leaves the dispatcher out of the deferred set', () => { + // multi-reviewer-loop is always on the taken path and is what tells the agent + // which backend to load — deferring it would defer the instruction to defer. + assert.ok(!DEFERRED_LIBS.has('multi-reviewer-loop.md')); + }); + + it('replaces a deferred `!cat` with a read directive instead of the body', () => { + withLibs({ [BACKEND]: SENTINEL }, (dir) => { + const body = `Intro.\n!\`cat ~/.claude/lib/${BACKEND}\`\nOutro.`; + const result = inlineLibReferences(body, dir, { bundlesLibs: true }); + assert.ok(!result.includes(SENTINEL), 'deferred body must not be inlined'); + assert.ok(result.includes(`${BUNDLED_LIB_DIR}/${BACKEND}`), 'must cite the bundled path'); + assert.match(result, /Read .*now — required/); + }); + }); + + it('reports the deferred lib so the installer knows to write it', () => { + withLibs({ [BACKEND]: SENTINEL }, (dir) => { + const bundled = new Set(); + inlineLibReferences(`!\`cat ~/.claude/lib/${BACKEND}\``, dir, { bundlesLibs: true, bundled }); + assert.deepEqual([...bundled], [BACKEND]); + }); + }); + + it('keeps a deferred lib out of the Referenced-libraries appendix', () => { + // A prose citation must not smuggle the body back in via the appendix — that + // path is what the deferral exists to close. + withLibs({ [BACKEND]: SENTINEL }, (dir) => { + const body = `See [lib/${BACKEND}](../../lib/${BACKEND}) for the loop.`; + const result = inlineLibReferences(body, dir, { bundlesLibs: true }); + assert.ok(!result.includes(SENTINEL), 'appendix must not re-inline a deferred lib'); + assert.ok(result.includes(`${BUNDLED_LIB_DIR}/${BACKEND}`)); + }); + }); + + it('does not defer when the environment lacks bundlesLibs', () => { + withLibs({ [BACKEND]: SENTINEL }, (dir) => { + const body = `!\`cat ~/.claude/lib/${BACKEND}\``; + assert.ok(inlineLibReferences(body, dir).includes(SENTINEL)); + }); + }); + + it('cites a sibling backend relatively from inside the bundle dir', () => { + withLibs({ [BACKEND]: SENTINEL }, (dir) => { + const body = `Falls back to [lib/${BACKEND}](../../lib/${BACKEND}).`; + const result = inlineLibReferences(body, dir, { bundlesLibs: true, fromLibDir: true }); + assert.ok(result.includes(`./${BACKEND}`), 'sibling ref must be ./-relative'); + assert.ok(!result.includes(`${BUNDLED_LIB_DIR}/${BACKEND}`), 'must not nest lib/lib/'); + }); + }); + + it('does not re-inline a lib the parent skill already carries', () => { + // Without the present-set, every bundled backend re-appends the dispatcher it + // was split away from and the split saves nothing at read time. + withLibs({ [BACKEND]: SENTINEL, 'shared.md': 'SHARED-BODY' }, (dir) => { + const body = 'Consult [lib/shared.md](../../lib/shared.md).'; + const present = new Set(['shared.md']); + const result = inlineLibReferences(body, dir, { bundlesLibs: true, present }); + assert.ok(!result.includes('SHARED-BODY'), 'already-present lib must not be re-inlined'); + }); + }); + + it('reports what it made present so children can skip those', () => { + withLibs({ 'shared.md': 'SHARED-BODY' }, (dir) => { + const presentOut = new Set(); + inlineLibReferences('!`cat ~/.claude/lib/shared.md`', dir, { presentOut }); + assert.ok(presentOut.has('shared.md')); + }); + }); + + it('emits no dangling ~/.claude/lib ref when deferring', () => { + withLibs({ [BACKEND]: SENTINEL }, (dir) => { + const body = `!\`cat ~/.claude/lib/${BACKEND}\`\nAlso \`~/.claude/lib/${BACKEND}\`.`; + const result = inlineLibReferences(body, dir, { bundlesLibs: true }); + assert.doesNotMatch(result, /~\/\.claude\/lib\/[A-Za-z0-9._-]+\.md/); + }); + }); +}); + +describe('backticked lib citations', () => { + function withLibs(libs, fn) { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'slashdo-tick-')); + for (const [name, content] of Object.entries(libs)) { + fs.writeFileSync(path.join(tmpDir, name), content, 'utf8'); + } + try { return fn(tmpDir); } finally { fs.rmSync(tmpDir, { recursive: true }); } + } + + it('does not drag a name-dropped lib into the appendix', () => { + // Regression: resolving the see-also form through the appendix queue inflated + // /do:config from 19KB to 91KB by pulling in a doc it only mentions. + withLibs({ 'shared.md': 'SHARED-BODY-SENTINEL' }, (dir) => { + const result = inlineLibReferences('Full mechanics in `lib/shared.md`.', dir, + { bundlesLibs: true }); + assert.ok(!result.includes('SHARED-BODY-SENTINEL'), 'see-also must pull in nothing'); + assert.ok(!result.includes('Referenced libraries'), 'no appendix for a mere mention'); + assert.ok(result.includes('`shared`'), 'reads as a doc name, not a path'); + }); + }); + + it('cites the bundled path when the skill actually bundles that lib', () => { + const backend = 'ollama-review-loop.md'; + withLibs({ [backend]: 'BODY' }, (dir) => { + const body = `!\`cat ~/.claude/lib/${backend}\`\nFull mechanics in \`lib/${backend}\`.`; + const result = inlineLibReferences(body, dir, { bundlesLibs: true }); + assert.ok(result.includes(`\`lib/${backend}\``), 'bundled lib keeps a real path'); + }); + }); + + it('leaves an unrelated lib/-looking path alone', () => { + withLibs({}, (dir) => { + const body = 'Run `lib/not-a-slashdo-doc.md` from the project.'; + assert.equal(inlineLibReferences(body, dir, { bundlesLibs: true }), body); + }); + }); +}); From b1e169320e86fe241ea9df130988d11dd8f7757c Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Fri, 28 Aug 2026 22:07:38 -0700 Subject: [PATCH 2/7] docs: deduplicate the two restated instructions in /do:next MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two places in next.md state the same thing twice, which is drift risk more than it is bulk: - The Phase 1 pre-flight bash comment restated the whole GH_HOST derivation ("seed with $ORIGIN_HOST, then apply the snippet's fallbacks and per-host auth precheck") that the prose immediately above the `!cat` states again. The comment now keeps only what is unique to it — why `gh api` needs an explicit --hostname — and defers the rest to the one authoritative spot. - The deletions-win conflict rule appeared in Phase 5 and again in Phase 6, and the two had already drifted: only Phase 6 carried the load-bearing ban on `git add -A` while paths are unmerged. Phase 5 is now the complete statement, including that ban, and Phase 6 refers to it. Deliberate repetition elsewhere in this file is left alone. The four DEFAULT_BRANCH derivations and the two jq probes are separate shell invocations that cannot share variables — the file says so at each site — and the Parse Arguments flag reference legitimately restates what the execution steps do. --- commands/do/next.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/commands/do/next.md b/commands/do/next.md index b5d8521d..d3b4e6a1 100644 --- a/commands/do/next.md +++ b/commands/do/next.md @@ -211,9 +211,8 @@ Runs **once per invocation**, after the last wave, over every result the batch p > echo "/do:next detected a GitHub repo ($ORIGIN_HOST) but gh is not authenticated to it. Run 'gh auth login'."; exit 1; } > # Seed the API host for the `gh api` calls below. `gh api` ignores the repo remote > # and defaults to github.com, so on a GHES repo it must be passed --hostname "$GH_HOST". -> # $ORIGIN_HOST is the first step of the shared derivation included at the end of this -> # section — seed GH_HOST with it, then apply that snippet's remaining fallbacks and its -> # per-host auth precheck. `gh issue`/`gh pr` calls resolve the host on their own. +> # `gh issue`/`gh pr` calls resolve the host on their own. This is only the seed — the +> # shared snippet at the end of this section finishes the derivation. > GH_HOST="$ORIGIN_HOST" > else > glab auth status >/dev/null 2>&1 || { @@ -577,7 +576,7 @@ Write the code, tests, and docs the item requires, following the **target repo's > [ -n "$DEFAULT_BRANCH" ] || DEFAULT_BRANCH="$(git -C "${WORKTREE}" remote show origin | sed -n 's/.*HEAD branch: //p')" > cd "${WORKTREE}" && git fetch origin "${DEFAULT_BRANCH}" && git merge --no-edit "origin/${DEFAULT_BRANCH}" > ``` -> **Conflict rule — deletions win.** Resolve any PLAN.md / changelog conflict so a line removed on *either* side stays removed; keep additions from both. Then `git add` and `git commit --no-edit`. +> **Conflict rule — deletions win.** Resolve any PLAN.md / changelog conflict so a line removed on *either* side stays removed; keep additions from both. Then `git add` the **specific resolved files** and `git commit --no-edit`. **Do NOT `git add -A`/`git add .` while paths are still unmerged** — that would stage raw conflict markers. A clean merge (or "Already up to date") needs no commit at all. Phase 6 re-syncs under this same rule. **Mark the work item done:** - **PLAN.md mode** — **remove the picked `- [ ]` line outright** (the changelog and git history are the audit trail; don't leave a checked `- [x]` behind unless the repo keeps items as a design log). If removing it empties a heading, leave the heading — section curation is `/do:replan`'s job. @@ -645,7 +644,7 @@ DEFAULT_BRANCH="$(git -C "${WORKTREE}" symbolic-ref --quiet --short refs/remotes cd "${WORKTREE}" && git fetch origin "${DEFAULT_BRANCH}" && git merge --no-edit "origin/${DEFAULT_BRANCH}" ``` -**If that merge reports a conflict** (unmerged PLAN.md / changelog paths — `git merge` exits non-zero and leaves `<<<<<<<` markers), **STOP and resolve it by hand** before going further: apply the deletions-win rule (a line removed on *either* side stays removed; keep additions from both), then `git add` the **specific resolved files** and `git commit --no-edit`. **Do NOT `git add -A`/`git add .` while paths are still unmerged** — that would stage raw conflict markers and push a broken tree. Only once `git status` shows no unmerged paths (a clean merge or "Already up to date" needs no commit at all) is it safe to push and merge: +**If that merge reports a conflict** (unmerged PLAN.md / changelog paths — `git merge` exits non-zero and leaves `<<<<<<<` markers), **STOP and resolve it by hand** before going further, under Phase 5's **deletions win** conflict rule — including its ban on `git add -A` while paths are unmerged. Only once `git status` shows no unmerged paths is it safe to push and merge: ```bash git push From a800882693dfbbeae91535340576b14475d9c432 Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Fri, 28 Aug 2026 22:21:03 -0700 Subject: [PATCH 3/7] perf: defer every conditional-path lib, and split /do:next's swarm flow out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the bundling mechanism from the four reviewer backends to every lib that sits on a branch a run may not take, and splits the one large conditional section that lived inline rather than in lib/. Newly deferred, each verified conditional at its include site: - plan-issue-mode (18.9K x6) and epic-children (8.9K x2) — issues mode only; PLAN.md mode, the default, never opens the tracker. - The six review lenses (46K) — review-agent-selection dispatches only the lenses a diff signals, often one or two, sometimes none. - enhance-loop (23.7K) — its heading already reads "only when --enhance-with". - ci-flake-handling (3.4K x2) — reached only when a check fails like a flake. - next-swarm (32.8K, new) — /do:next's --swarm flow, extracted from next.md. A single-issue run, the default, no longer carries the parallel-batch flow. Deliberately NOT deferred: code-review-checklist sits under a REQUIRED GATE, and Phase 1 of /do:better-swift says to load swift-gotchas "into your context" before launching agents. Both are always on the taken path, where deferring buys an extra read and risks the agent skipping content it always needed. That rule is now written down beside the list. DEFERRED_LIBS becomes ON_DEMAND_LIBS, a map carrying `when` (the branch that makes the read required) and `what` (the content) per entry. Both render into the directive, so the agent is told when it must read a file instead of being left to infer it. Skill set 2.80MB -> 1.55MB (-45%, ~314K tokens); /do:review -56%, /do:plan-task -84%, /do:rpr -60%. No skill grew. Two contract suites read commands/do/next.md directly and would have lost sight of the swarm rules once they moved. Their readers now resolve `!cat` includes, so they scan the composed document the agent actually sees. Claude/OpenCode keep runtime `!cat`; next-swarm.md installs into ~/.claude/lib like any other lib and composes back to the same content. --- commands/do/next.md | 137 +------------------------- install.sh | 2 +- lib/next-swarm.md | 142 +++++++++++++++++++++++++++ src/transformer.js | 95 +++++++++++++----- test/glab-jq-contract.test.js | 14 ++- test/installer.test.js | 25 ++++- test/worktree-merge-contract.test.js | 14 ++- uninstall.sh | 2 +- 8 files changed, 268 insertions(+), 163 deletions(-) create mode 100644 lib/next-swarm.md diff --git a/commands/do/next.md b/commands/do/next.md index d3b4e6a1..ed2f55c9 100644 --- a/commands/do/next.md +++ b/commands/do/next.md @@ -45,142 +45,13 @@ Collect targets into an ordered list `TARGETS` in three steps, **in this order** - **`--plan`** — before writing code, enter an **interactive plan-mode session** (Phase 3.5): present a written plan, surface open questions, get explicit approval before implementing. Runs *after* the worktree is claimed so you plan with full context. Rejection routes to Phase 7 cleanup exactly like a Phase 3 skip. **Ignored in `--swarm` mode when more than one issue actually runs** (parallel agents can't each hold an interactive plan session; state the skip). When swarm degenerates to the single-issue flow — one target, one surviving named issue, or one eligible issue — `--plan` is honored as normal: there's only one agent, so there's nothing to skip it for. - **`--review-with` / `--review-iterations` / `--review-mode` / `--review-stop-on-findings` / `--review-stop-on-clean` / `--reviewer-applies` / `--no-review`** — **passed through to `/do:pr`** in Phase 6, which owns the review/ship machinery. (`--review-mode series|parallel` selects how `/do:pr`'s multi-reviewer loop dispatches the reviewers; series is the default.) Same grammar as every other slashdo command (see `/do:pr`). `--no-review` opts out of both `/simplify` and the external pass. When neither `--review-with` nor `--no-review` is given, you decide in Phase 6 whether the diff warrants `/simplify` and/or an external review (a value swap doesn't; a multi-file change does). -## Swarm mode (`--swarm`) — drain several independent issues in parallel +## Swarm mode (`--swarm`) -**When `SWARM` is true, this section replaces Phases 1–7 for the run.** It claims and ships up to `SWARM_N` independent open issues at once — each in its own worktree subagent running the normal single-issue flow — then serializes only the merge. When `--swarm` is absent, skip this section entirely and run Phases 1–7 below. Swarm reuses the single-issue phases wholesale: each agent runs the single-issue **Phases 2–6** for one issue (claim → implement → changelog → review, **no merge, no Phase 7 cleanup** — the orchestrator owns those), so the claim/lease, worktree, implement, changelog, and review-gate semantics are exactly the single-issue ones. The only new logic is partitioning the batch up front (Phase A) and serializing the merges at the end (Phase C). +**When `--swarm` is absent — the default — skip this section entirely and run Phases 1–7 below.** -**Preconditions — check first; abort cleanly if any fails (do not partially claim):** -- **Issues mode only.** Swarm's claim/lease is the tracker's issue-assignee marker (GitHub or GitLab), and partitioning by dependency needs the tracker. **Resolve `ISSUE_MODE` here first, including Phase 1's auto-redirect** — because swarm replaces Phases 1–7, that redirect won't fire on its own: if `--issues`/a saved default didn't already set it, apply the same structural check Phase 1 does — a repo with **no PLAN.md, or only the issue-mode stub**, *is* issue-tracked, so set `ISSUE_MODE=true` (state the switch). **An explicit numeric target settles this too** — issue numbers are inherently tracker references, so **any** numeric target (one or several) sets `ISSUE_MODE=true` (state the switch) even in a repo with a real PLAN.md backlog — **unless the user explicitly typed `--no-issues`**, which wins per this file's usual typed-flag-beats-inference rule and routes straight to the abort below. One target matters as much as several here: a lone `#12` hands off to the single-issue Phases 1–7, which must run in *issues* mode or Phase 1 would go looking for a PLAN.md slug named `12`. Abort when it still resolves to PLAN.md mode — a real PLAN.md backlog with either (no `--issues` and no numeric targets) or an explicit `--no-issues`: ``--swarm works in issues mode only — pass --issues (or run in an issue-tracked repo). PLAN.md-mode swarm is a future enhancement.`` +When `SWARM` is true the swarm flow **replaces Phases 1–7** for the run: it claims and ships up to `SWARM_N` independent open issues at once, each in its own worktree subagent running the normal single-issue flow, and serializes only the merge. Its preconditions, the four swarm phases (A triage/partition, B fan-out, C merge queue, D reconcile), and the batch-abort rules all live in one file: - **Then probe for `jq` on GitLab, right here.** Swarm replaces Phases 1–7, so the - identical probe at the top of "Phase 1 — issues mode" never runs on this path — yet - A1e/A2e's native blocked-by check calls plain `glab api ... | jq` just like the picker - does. Without this, an explicit GitLab swarm on a host with `glab` but no `jq` skips the - documented install check and cannot validate dependencies. Run it once `ISSUE_MODE` is - settled above (never before — the probe is issue-mode-only, for the same PLAN.md reason): - ```bash - if [ "$CLI_TOOL" = glab ]; then - command -v jq >/dev/null 2>&1 || { - echo "/do:next's GitLab issue mode pipes 'glab api' output through jq, which is not installed. Install it (e.g. 'brew install jq' or 'apt-get install jq') and re-run."; exit 1; } - fi - ``` -- **GitHub or GitLab, with the matching CLI authenticated** — the same Phase 1 pre-flight (it ships through `/do:pr`, which supports both). -- **A subagent-capable harness.** Swarm fans out parallel agents via the harness's subagent mechanism (Claude Code's `Agent`/Task tool, or the equivalent). **If the environment cannot spawn parallel subagents, fall back to sequential** — run Phase B's per-issue task (Phases 2–6, **no merge**) for each partitioned issue one after another in this same session, then proceed to Phase C so the merge stays owned by the serialized queue, not each iteration (still useful: it drains `SWARM_N` items in one invocation, just not concurrently). State that you're doing so. -- **Targets are optional — and may be an explicit list.** **Check target *shape* first, before mode resolution or any claim:** every target must be an **issue number** (bare or `#`-prefixed), because a PLAN.md slug can never be a swarm member — abort on one, and let this abort win over the issues-mode abort above so the message names the real problem: ``--swarm works on issue numbers only — "" looks like a PLAN.md item. Drop --swarm to claim it, or pass issue numbers.`` Then route by count: **no target** → Phase A auto-picks the batch; **two or more** → that list IS the batch (Phase A's explicit-list path; the same `#` cherry-pick semantics, `SWARM_N` at a time); **exactly one** → this isn't a swarm: run the single-issue **Phases 1–7** for it (in issues mode, per the bullet above) and say so. - -**Concurrency & cost.** `SWARM_N` parallel agents multiply token spend roughly N×. State the resolved N and that implication up front (e.g. `launching 3 parallel agents — ≈3× the tokens of a single /do:next`). The `1..6` clamp (Parse Arguments) is deliberate: beyond ~6 concurrent worktrees/PRs, git-index-lock contention and merge-queue churn outweigh the throughput gain. **`SWARM_N` caps concurrency, not batch size** — an explicit list of 9 issues costs ≈9× regardless of how many waves it takes, so state the total (`9 issues named — ≈9× the tokens, 3 at a time`) and let the user cut the list if that's more than they meant: **for a named list of more than 8 members, stop and confirm before launching wave 1** — print the total cost and the wave plan, and proceed only on an explicit go-ahead. Below that threshold, state the cost and continue. (If the caller genuinely can't be asked — a non-interactive/subagent context — proceed, but log the total prominently rather than burying it.) - -### Swarm Phase A — Triage & partition (orchestrator, in the main repo) - -**Two paths in.** With **no target**, run **A1–A2** (auto-pick). With an **explicit list of two or more issue numbers**, skip the picker and run **A1e–A2e** instead. Both paths converge on **A3** and hand Phase B an ordered batch, split into waves of at most `SWARM_N`. - -1. **A1 — Build the eligible queue** exactly as **Phase 1 — issues mode** below: the priority-then-oldest walk with EVERY skip applied (in-flight, already-assigned, parking-labelled, `epic-open`/`epic-done` epics, blocked-by an open declared dependency), the **dispatch-hint filter** when `MODEL_FILTER`/`EFFORT_FILTER` is active (so `/do:next --swarm --model light` drains a wave of cheap work), and — **when `SELF_MODE` is on** — the `--author "@me"` filter so the batch only ever contains issues you filed (same security boundary as the single-issue flow). An `epic-wrapup` epic is eligible like any issue. Reuse that logic verbatim — do not invent a second picker. -2. **A2 — Select the first `SWARM_N` *independent* eligible issues** off the top of that ordered queue: - - **Intra-batch dependency.** If a candidate declares `Depends on #N` / `Blocked by #N` (or native blocked-by) on **another candidate in the batch**, keep only the predecessor this round — the successor self-clears and is picked next run once the predecessor merges. (Blockers *outside* the batch were already handled by the Phase 1 skip.) - - **File-overlap avoidance (best-effort).** From each issue's title/body, predict the rough files/paths/components it touches. When two candidates obviously target the same file(s), keep the higher-priority one and skip the other **this round** — not for correctness (the serialized merge + re-sync handles that) but to avoid two agents thrashing or duplicating the same file. This is a cheap heuristic, not a guarantee; note when you apply it. - - **Under-fill is fine.** If fewer than `SWARM_N` independent issues exist, run the swarm at the smaller size and say so. **If only one is eligible, run the normal single-issue flow instead** (Phases 1–7) and say so — a one-agent swarm is just `/do:next` with overhead. - - Auto-pick never selects more than `SWARM_N`, so it always yields exactly **one wave**. - -**A1e — Vet each named issue; no picker, no substitutions.** The list is a deliberate cherry-pick, so it **bypasses the auto-pick skips exactly as a single explicit `#` does**: parking labels (`future`/`blocked`/`discussion`/…), an active `LABEL_FILTER`, an active `MODEL_FILTER`/`EFFORT_FILTER`, and an open declared blocker that was **never named in the list** are all overridden — state each override as you apply it (e.g. `claiming future-labelled #123 by explicit request`). A named member still keeps its own dispatch hint for Phase B: overriding the *filter* selects the issue, it does not restate what the issue needs. **A blocker that *was* named and is then removed from the batch while still OPEN is a different case** — removed as in-flight/already-assigned, removed by *this very rule* as a hold, or removed by A2e as part of an unorderable dependency cycle: it still blocks its dependent, so **hold that dependent here, before A2e orders anything** — drop it from the batch with a note (`#21 held: depends on #17, which is claimed elsewhere and won't merge this run`) and carry it into Phase D's summary as `held`. **Apply this rule repeatedly until no new holds appear** — a member you just held is itself a still-open removed blocker, so a chain (`#23` depends on `#21` depends on `#17`) must hold `#21` **and** `#23`, never just the first link. If A2e later drops cycle members, re-run this hold pass over what remains and re-apply the survivor routing before ordering waves. Don't defer any of this to Phase B's wave rule: A2e only defers a member behind *another surviving member*, so a removed blocker produces no deferral for that rule to act on, and the dependent would otherwise launch in wave 1 anyway. Holding here also matters because of what comes next — if the hold leaves a single survivor, the single-issue hand-off below would claim it through Phase 1 step 5's explicit-`#num` path, which *overrides* the open-blocker skip (`claiming #21 despite open blocker #17 by explicit request`) — the exact opposite of the hold. **Count survivors *after* these holds** when applying the "nothing survives / exactly one survives" rules below. A blocker dropped because it is **closed or doesn't exist doesn't block at all** (Phase 1 step 4), so its dependent runs normally — never hold work behind a dependency that's already satisfied. Do still check each named issue, in this order: - - **`--self` is the one gate a list cannot cross.** When `SELF_MODE` is on, verify every named issue's author is the running account (the same check as Phase 1 step 5) — an issue that **doesn't exist or can't be read** falls through to the drop rule below rather than the refusal, so a typo'd number is a skip, not a bogus "filed by someone else". Refuse the whole run only when a real issue resolves to another author: ``Issue # was filed by , not you — /do:next --self only works on issues you filed. Drop --self, or drop # from the list.`` Don't quietly drop it and swarm the rest — a silently shrunk batch hides a refused security gate. - - **Closed, or no such issue → drop with a note** (`#: already closed — skipping`). - - **In flight or already assigned → drop with a note** (`#: already claimed (branch/assignee) — skipping`). **Never substitute another issue for a dropped one** — the user named these; auto-pick is not in play. - - **Epic → resolve with Phase 1 step 3** and act on the state: an `epic-wrapup` joins the batch like any issue; an `epic-done` is closed inline and dropped from the batch; an `epic-open` joins the batch only as an explicit override (say that children are still open). - - **Nothing survives → stop** and list why per issue. **Exactly one survives → run the single-issue flow** (Phases 1–7) for it and say so. -**A2e — Order the batch and split it into waves.** Keep the **user's order** as the *default* sequence — a named list is a stated preference, so never re-sort it by priority. The two placement constraints below **outrank that order**: sort the members topologically with respect to intra-list dependencies first (a predecessor always precedes its dependent, even when the user named the dependent first), using the user's order only to break ties. Then: - - **Intra-list dependency ⇒ a later wave, not a drop.** If a member declares `Depends on #N` / `Blocked by #N` (or native blocked-by) on **another member**, place the dependent in a **later wave than its predecessor** instead of dropping it: Phase C merges each wave before the next one starts, so the dependency is satisfied by the time the successor's agent branches. A true cycle among members can't be ordered — drop those members with a note so a human can break it, then **re-run A1e's hold pass** over what remains: a dropped cycle member is a still-open removed blocker for anything that depends on it. - - **File-overlap ⇒ a later wave, not a drop.** Same best-effort title/body prediction as A2, but for a named list the remedy is separation in time rather than exclusion: put members that obviously touch the same file(s) in different waves, and note it. - - **Chunk into waves of at most `SWARM_N`,** preserving that (dependency-topological, else user) order **and the two placement constraints above** — which outrank both the chunk size and the stated order: a member deferred behind a predecessor or an overlap peer must land in a **strictly later** wave than it, so a wave may come out smaller than `SWARM_N`. Only when **nothing was deferred** and the batch that **survived A1e** is `<= SWARM_N` is this a single wave (the common case) — count survivors, not named targets, since A1e may have dropped some. State the plan up front: `9 issues survived vetting, 3 at a time — 3 waves (≈3× the tokens of a single /do:next per wave)`. -**A3 — Do NOT claim here (both paths).** Each issue is claimed *inside* its own subagent so the assignee-marker + race read-back runs per issue, atomically. The orchestrator only hands each agent one specific issue number. - -### Swarm Phase B — Fan out (one subagent per issue) - -Launch the current wave's agents **in parallel** (all Agent/Task calls in a single response) and wait for all to return. - -**Apply each issue's dispatch hint to its own agent.** Read `model:` / `effort:` off the issue you're about to hand out and configure that agent from it. **A1's auto-pick queue already carries `labels` (its issue-list call requests them); A1e's explicit-list path does not** — it vets each named issue individually — so for a named batch, fetch them per issue before dispatching rather than assuming they're in hand: GitHub `gh issue view --json labels -q '[.labels[].name]'`; GitLab `glab issue view --output json --jq '.labels'` (labels already come back as a flat string array, no `.name` needed). Skipping that is how a named swarm silently runs every worker at the session default despite A1e promising each member keeps its own hint — this is the payoff for the labels, and it's why a mixed batch shouldn't cost one flat rate per agent. - -**The labels name tiers, not models — resolve each against the host you're running on** using the shared rules in [lib/model-tiers.md](../../lib/model-tiers.md): `light` → this host's cheapest capable coding model, `medium` → its workhorse, `heavy` → **its strongest available model, named by alias** (`model: "opus"` on Claude Code — an alias, never a pinned version ID), and `effort:` → **advisory**: it tells the worker how careful this issue needs to be, and you **may** additionally set the sub-agent's reasoning-effort control from it (clamped to the host's nearest level) where the dispatch API has one — Claude Code's `Agent` tool does not, so pass the level in the worker's brief instead and let it run at session effort. An issue missing either label inherits the session's setting for that axis. That file also carries the two consequences that matter here: - -- **`model:heavy` reaches up, not just sideways** — it names this host's strongest alias, so a `heavy` issue runs on the strongest model even when the orchestrating session is mid-tier. That is the point of the tier: `heavy` marks work where a weaker model produces confident wrong answers. If the dispatch is rejected for lack of entitlement to that tier, retry that agent once with `model` omitted (inherit), note the degrade, and proceed. A stale advisory label must never block a swarm. -- **Degrade, never abort.** If this host can't set a per-agent model or effort (or can't spawn sub-agents at all — see the precondition), run each agent at the session default and report the hint in the Phase D summary instead. Dispatch is an optimization; the swarm is the feature. - -Two things this is **not**: it is not the `--model`/`--effort` *filter* (that chose the batch; this runs it), and it is not `--review-models` (that pins each *reviewer*, and passes through to `/do:pr` untouched — a worker's own model has no bearing on who reviews its PR). - -Give each subagent exactly one issue number and this task: - -> Run the `/do:next` single-issue flow for issue **#``** — the **Phase 1 explicit-`#` path** (for validation + variable setup) followed by **Phases 2 through 6 only** — in your own sibling worktree, with these adjustments: -> - **Validate & set up via Phase 1's explicit-number path first:** run Phase 1's `#` branch to confirm the issue is open and not in flight, resolve it if it's an epic, and set `ISSUE_NUM=` / `SLUG=issue-` — Phase 2's worktree/branch and every later `gh issue`/`glab issue` call depend on those variables. (Skip the auto-pick walk; the orchestrator already vetted this issue in Phase A, and an explicit number deliberately bypasses the auto-pick skips.) **Map this validation's failures to the structured result instead of the explicit-path's normal print-and-stop:** if it fails because the issue is now **in flight or already assigned** (a sibling `/do:next` claimed it between Phase A and now — a lost race, not an error), return `{ issue, status: "yielded" }`; if it fails because the issue is **closed or no longer exists**, return `{ issue, status: "skipped", reason }`. Either way, do NOT pick a different issue. -> - **Then claim via that same explicit-number path:** it performs the Phase 2 worktree creation + assignee-marker claim with the race read-back. If the read-back shows a sibling won the race, **yield** (release the marker, retract the claim branch, clean up your worktree) and return `{ issue, status: "yielded" }` — do NOT pick a different issue. -> - **Phase 3 still applies:** if the issue is stale/superseded/awaiting-input, skip it (release the marker, clean up) and return `{ issue, status: "skipped", reason }`. -> - Implement (Phase 4) and record completion + changelog (Phase 5) as normal. -> - **Ship via `/do:pr --no-merge`** with the run's review flags (Phase 6) and **STOP before the merge** — open the PR, run the review gate, but DO NOT merge and DO NOT run Phase 7 cleanup. The orchestrator owns the merge and the cleanup. -> - **Wait on external reviewers/CI ACTIVELY — never end your turn to "wait for a notification."** You are a subagent: if you stop while a background reviewer (`codex review`, `claude -p`, `agy`) or a CI watch is still running, your run is over — completion notifications are not guaranteed to reach a stopped subagent, and the orchestrator will read your premature last words as your final result while the reviewer's findings are lost. Wait with bounded blocking-chunk foreground Bash calls, each safely under the host's ~10-minute foreground cap, repeated until the reviewer's done-marker exists (the local-agent review loop's `$DONE_FILE` pattern): `for i in $(seq 1 55); do [ -f "$DONE_FILE" ] && break; sleep 10; done` — then immediately issue the same call again if it's still running. End your turn only to return the structured result below. -> - **Return a structured result**, one of two shapes Phase C dispatches on by whether `pr_number` is present: -> - **PR opened:** `{ issue, pr_number, branch, worktree, review_status, notes }`, where `review_status` is `/do:pr`'s aggregate (`clean` / `partial` / `inconclusive` / `dirty`) or `opened-no-review` when no external reviewer ran and the Local Code Review gate passed. -> - **No PR** (claim yielded to a race winner, or Phase 3 skipped it as stale): `{ issue, status: "yielded" | "skipped", reason }` — no `pr_number`. - -Pass each agent the review flags verbatim (`--review-with` / `--review-iterations` / `--review-mode` / stop-mode / `--reviewer-applies` / `--no-review`). **Also pass the orchestrator's resolved self decision as an explicit `--self` or `--no-self`** so each worker honors *this run's* mode instead of re-resolving the saved `self` default. This matters because the worker runs a fresh `/do:next #` whose Phase 1 would otherwise re-read the saved default: with a saved `self=true` and a per-run `--no-self`, Phase A correctly selected third-party issues but a worker re-resolving `self=true` would refuse them at the explicit-#num gate. Passing the typed flag makes the worker's gate use the orchestrator's mode (typed wins over saved default) — a redundant re-check when `--self` is on (Phase A already filtered the batch to your issues) and correctly any-author when `--no-self`. The **fix regression guard** and **CI flake handling** apply inside each agent automatically (they live in `/do:pr`'s loop and merge gate). Concurrent `git worktree add` against the shared repo can briefly contend on `.git` index locks — an agent that hits a transient lock retries once before failing. - -**Harness without parallel subagents?** Per the precondition, run this same per-issue task **sequentially** in the current session — one issue at a time, identical task body — collecting each result, then proceed to Phase C unchanged. The merge queue is already serialized, so sequential fan-out only loses the concurrency, not any correctness. **The dispatch hints degrade to reports here**, exactly as in the single-issue flow (Phase 1 step 6): there's no fresh agent to configure, so note each issue's hint and run in the current session. - -**Multi-wave batches (any batch A2e split into more than one wave).** Run **B → C once per wave, in wave order** — merge wave *K*'s PRs before launching wave *K+1*, so the next wave's agents branch off a default branch that already carries the previous wave's merged work. That ordering is what makes A2e's deferral correct: a member deferred for a dependency or a file overlap starts from a base that already contains what it was waiting on, so it needs no re-sync gymnastics. Phase D runs **once, after the last wave**, over the accumulated results. Re-state progress between waves (`wave 2/3 — #17 #21 #23`). If a wave leaves an issue **unmerged for ANY reason** — PR left open (unmergeable or review gate unsatisfied), agent `yielded`, Phase 3 `skipped` it as stale, or the agent never returned (Phase D reconciles it, but not until after the last wave) — continue to the next wave anyway, but **hold back any later member A2e deferred behind that issue**: its declared blocker is genuinely still open, so skip it with a note (`#21 held: depends on #17, which did not merge this run`), exactly as Phase 1's blocked-by skip would. Gate the hold on *not merged*, never on *PR left open* — a yielded or dead-agent predecessor leaves the dependent just as unsupported as an open PR does. Members deferred only for file overlap still run. - -### Swarm Phase C — Serialized merge queue (orchestrator) - -After the barrier, merge the wave's returned PRs **one at a time, never concurrently** — each merge advances the default branch, so the next PR may need a re-sync. Walk the results in the batch's own order: priority/oldest for an auto-picked batch, the **user's order** for an explicit list (A2e). For each: - -1. **Skip non-mergeable results.** A result with `status: "yielded"`/`"skipped"` (no `pr_number`) has nothing to merge — record it. For the rest, apply **single-issue Phase 6's merge gate** to `review_status`: never merge `dirty` (build/test broken) or `inconclusive` (a requested reviewer missing/timed-out/errored) — leave that PR open and record why; merge only `clean`, `opened-no-review` (Local gate passed, no external reviewer requested), or `partial` **with** an explicit `--review-stop-on-*` flag. (Agents that never returned are reconciled in Phase D, not here.) -2. **Re-sync onto the advanced default branch** from the PR's worktree — `git fetch origin ` then `git merge --no-edit origin/` — resolving any PLAN.md/changelog conflict **deletions-win** (a line removed on either side stays removed; keep additions from both). If the merge **can't be resolved cleanly**, leave that PR open, record it for human follow-up, and move to the next — **never force it**. -3. **Gate on required CI, then merge.** Because each swarm agent opened its PR with `/do:pr --no-merge`, `/do:pr`'s own CI merge gate never ran — and the re-sync in step 2 just pushed a new SHA whose checks are pending — so the orchestrator must run the CI gate here, exactly as the single-issue Phase 6 / `/do:pr` merge does, before merging. Push the re-synced SHA, wait for CI, and only then merge; on a check failure apply **CI flake handling** (one re-run on the same commit; flake → proceed, real → leave that PR open, record it, and move to the next — see `~/.claude/lib/ci-flake-handling.md`). - - GitHub (`gh`) — scope the watch to **required** checks only so an optional/non-required job can't block a merge branch protection would allow (vacuously satisfied when no required checks exist): - ```bash - git -C "" push - # &&, not three separate lines: `--fail-fast` makes `gh pr checks` exit non-zero on - # a failing required check, but an unchained next line merges anyway — which is the - # opposite of what this step's own prose promises. Chain it so a red gate stops here. - gh pr checks --required --watch --fail-fast && \ - gh pr merge --merge - # Delete the head branch ONLY once the PR really reads MERGED. - if [ "$(gh pr view --json state -q .state)" = "MERGED" ]; then - if ! git push origin --delete ""; then - # rc 2 means "no such ref" — already gone, which is success. Any other rc is - # a transport/auth failure that proves nothing about the branch. - git ls-remote --exit-code --heads origin "" >/dev/null 2>&1; RC=$? - [ "$RC" -eq 2 ] || echo "ERROR: could not confirm is gone (ls-remote rc=$RC) — record this PR for follow-up" - fi - else - echo "PR is not MERGED — keeping " - fi - ``` - **No `--delete-branch`** — it deletes the *local* branch too, and `` (the `branch` field the worker returned, normally `next/issue-`) is checked out in the agent's worktree, so git refuses (`cannot delete branch 'next/issue-' used by worktree at …`) and **`gh` exits non-zero after the merge already succeeded**. That reads as a merge failure and fires any `||` fallback wrapped around the merge. Delete the remote branch with the explicit `git push origin --delete` above — it needs no local checkout — and let Phase D remove the worktree and the local branch from the main repo, where that works. **The `MERGED` read-back is load-bearing**: `--delete-branch` only ever deleted the head branch *because* the merge had happened, and an ungated delete would retract the head of a PR that is still open — either the merge failed (unmergeable, branch protection, a lost race) or, on a repo with a **merge queue**, `gh pr merge` returned success having merely *queued* it. GitHub auto-closes a PR whose head branch disappears, which destroys both the "leave that PR open, record it, and move to the next" outcome step 3 requires and the queued merge itself. Read the state back rather than trusting the merge command's exit status. - - GitLab (`glab`) — there's no discrete "required checks" list to scope to; the project's own merge/pipeline-success requirement governs, so wait on the pipeline explicitly and merge only then: - ```bash - git -C "" push - glab ci status --wait && glab mr merge --yes --remove-source-branch - # Read the state back for the same reason the gh path does: --auto-merge returns - # while the MR is still queued behind the pipeline, and step 4 gates issue closure - # on this answer. - if [ "$(glab mr view --output json --jq .state)" = "merged" ]; then - echo "MR merged" - else - echo "MR is not merged (queued on the pipeline) — leaving its issue open and keeping " - fi - ``` -**Why `glab ci status --wait` and not `--auto-merge`:** `--auto-merge` does not wait — it sets merge-when-pipeline-succeeds server-side and returns while the MR is still `opened`. The read-back above would then be `opened` on every run, so step 4 would never close an issue and Phase D would never reclaim a worktree: the guard against closing unlanded work would become a guard against closing anything. Waiting first makes one read authoritative. If you must use `--auto-merge` (a very long pipeline, say), replace the single read with a bounded poll of `glab mr view --output json --jq .state` and treat "still `opened` at the deadline" as queued, not merged. -4. **Close out the issue — only when step 3 read back `MERGED`.** A PR the merge left open or queued has shipped nothing, so closing its issue would drop live work out of the queue: record it as left-open with the reason instead, and let Phase D keep its worktree and branch. For a genuinely merged PR, apply single-issue Phase 7's closure step: confirm `Closes #` auto-closed it on merge to the default branch; if still open, close it explicitly (GitHub: `gh issue close --comment "Shipped in PR #."`; GitLab: `glab issue note -m "Shipped in PR #." && glab issue close `). Drop the `in-progress` label. - -### Swarm Phase D — Reconcile, clean up, report - -Runs **once per invocation**, after the last wave, over every result the batch produced. - -1. **Sweep worktrees & branches** from the main repo: `git worktree remove` + `git branch -d`/`-D` each merged agent's worktree/branch, then `git worktree prune`. **Keep** the worktree/branch of any PR left open so the work can be finished. -2. **Handle agent death / spend-limit.** If an agent never returned (crash, monthly spend cap), reconcile from **tracker state, not its last words** — and **never assume a review status survived the death** (slashdo's local-reviewer verdicts aren't persisted to the PR, so there's no stored `review_status` to read back). A worktree/branch/PR may exist: - - **A PR opened:** recompute its gate from scratch before merging — re-run the run's review flags against it (`/do:pr --no-merge` with the same `--review-with`), or, for a no-external-review run, treat green required-CI + a mergeable state as `opened-no-review`-eligible. Merge it through the Phase C queue only on a clean recompute; if requested external reviewers can't be re-run to clean, leave the PR open and flag it for a human. - - **No PR (or an unclean recompute you won't finish):** **release the claim** — remove the assignee (GitHub: `gh issue edit --remove-assignee @me`; GitLab: `glab issue update --assignee "-$ME"`), delete the local+remote `next/issue-` branch, drop `in-progress` — so the issue returns to the queue, and flag it for human follow-up. -3. **Re-evaluate parent epics** for every issue that closed — a shipped issue may have been an epic's last child (the Phase 7 "re-evaluate the parent epic" step, run once per closed child). -4. **Reconcile changelog/PLAN churn.** Parallel claims all touch the same changelog file, when the project has one; the deletions-win re-syncs in Phase C should have kept it consistent — confirm the merged default branch's changelog carries every shipped issue's entry with no duplicate or resurrected lines. Skip this check for a project whose release notes come from commit messages. -5. **Print a summary table** — one row per batch issue, including the ones dropped in A1e and any held in a later wave: `issue · wave · dispatch (model/effort actually used, or `session` when the issue carried no hint — or when this host couldn't set them) · PR · result (merged / open: why / yielded / skipped / held / needs-input) · review status`. The dispatch column is what makes a wrong hint visible: a `model:light` issue whose agent needed three review iterations is a label to correct, not a mystery. For an explicit list, **account for every number the user named** — a named issue that never ran must appear with its reason, never be silently absent. +!`cat ~/.claude/lib/next-swarm.md` ## Phase 1: Pick diff --git a/install.sh b/install.sh index 4906a5a2..b38f7dcb 100755 --- a/install.sh +++ b/install.sh @@ -176,7 +176,7 @@ LIBS=( empty-array-expansion enhance-loop epic-children finding-disposition fix-regression-guard gh-host github-reviewer-loop graphql-escaping - local-agent-review-loop model-tiers multi-reviewer-loop ollama-review-loop + local-agent-review-loop model-tiers multi-reviewer-loop next-swarm ollama-review-loop per-finding-root-cause plan-id-format plan-issue-mode post-review-doc-recommendations remediation-agent-template review-agent-selection review-config-defaults review-convergence-gate diff --git a/lib/next-swarm.md b/lib/next-swarm.md new file mode 100644 index 00000000..ce4a74de --- /dev/null +++ b/lib/next-swarm.md @@ -0,0 +1,142 @@ + + +## Swarm mode (`--swarm`) — drain several independent issues in parallel + +**You are here because `SWARM` is true — this file replaces Phases 1–7 for the run.** It claims and ships up to `SWARM_N` independent open issues at once, each in its own worktree subagent running the normal single-issue flow, then serializes only the merge. Swarm reuses the single-issue phases wholesale: each agent runs the single-issue **Phases 2–6** (in the skill body) for one issue (claim → implement → changelog → review, **no merge, no Phase 7 cleanup** — the orchestrator owns those), so the claim/lease, worktree, implement, changelog, and review-gate semantics are exactly the single-issue ones. The only new logic is partitioning the batch up front (Phase A) and serializing the merges at the end (Phase C). + +**Preconditions — check first; abort cleanly if any fails (do not partially claim):** +- **Issues mode only.** Swarm's claim/lease is the tracker's issue-assignee marker (GitHub or GitLab), and partitioning by dependency needs the tracker. **Resolve `ISSUE_MODE` here first, including Phase 1's auto-redirect** — because swarm replaces Phases 1–7, that redirect won't fire on its own: if `--issues`/a saved default didn't already set it, apply the same structural check Phase 1 does — a repo with **no PLAN.md, or only the issue-mode stub**, *is* issue-tracked, so set `ISSUE_MODE=true` (state the switch). **An explicit numeric target settles this too** — issue numbers are inherently tracker references, so **any** numeric target (one or several) sets `ISSUE_MODE=true` (state the switch) even in a repo with a real PLAN.md backlog — **unless the user explicitly typed `--no-issues`**, which wins per this file's usual typed-flag-beats-inference rule and routes straight to the abort below. One target matters as much as several here: a lone `#12` hands off to the single-issue Phases 1–7, which must run in *issues* mode or Phase 1 would go looking for a PLAN.md slug named `12`. Abort when it still resolves to PLAN.md mode — a real PLAN.md backlog with either (no `--issues` and no numeric targets) or an explicit `--no-issues`: ``--swarm works in issues mode only — pass --issues (or run in an issue-tracked repo). PLAN.md-mode swarm is a future enhancement.`` + + **Then probe for `jq` on GitLab, right here.** Swarm replaces Phases 1–7, so the + identical probe at the top of "Phase 1 — issues mode" never runs on this path — yet + A1e/A2e's native blocked-by check calls plain `glab api ... | jq` just like the picker + does. Without this, an explicit GitLab swarm on a host with `glab` but no `jq` skips the + documented install check and cannot validate dependencies. Run it once `ISSUE_MODE` is + settled above (never before — the probe is issue-mode-only, for the same PLAN.md reason): + ```bash + if [ "$CLI_TOOL" = glab ]; then + command -v jq >/dev/null 2>&1 || { + echo "/do:next's GitLab issue mode pipes 'glab api' output through jq, which is not installed. Install it (e.g. 'brew install jq' or 'apt-get install jq') and re-run."; exit 1; } + fi + ``` +- **GitHub or GitLab, with the matching CLI authenticated** — the same Phase 1 pre-flight (it ships through `/do:pr`, which supports both). +- **A subagent-capable harness.** Swarm fans out parallel agents via the harness's subagent mechanism (Claude Code's `Agent`/Task tool, or the equivalent). **If the environment cannot spawn parallel subagents, fall back to sequential** — run Phase B's per-issue task (Phases 2–6, **no merge**) for each partitioned issue one after another in this same session, then proceed to Phase C so the merge stays owned by the serialized queue, not each iteration (still useful: it drains `SWARM_N` items in one invocation, just not concurrently). State that you're doing so. +- **Targets are optional — and may be an explicit list.** **Check target *shape* first, before mode resolution or any claim:** every target must be an **issue number** (bare or `#`-prefixed), because a PLAN.md slug can never be a swarm member — abort on one, and let this abort win over the issues-mode abort above so the message names the real problem: ``--swarm works on issue numbers only — "" looks like a PLAN.md item. Drop --swarm to claim it, or pass issue numbers.`` Then route by count: **no target** → Phase A auto-picks the batch; **two or more** → that list IS the batch (Phase A's explicit-list path; the same `#` cherry-pick semantics, `SWARM_N` at a time); **exactly one** → this isn't a swarm: run the single-issue **Phases 1–7** for it (in issues mode, per the bullet above) and say so. + +**Concurrency & cost.** `SWARM_N` parallel agents multiply token spend roughly N×. State the resolved N and that implication up front (e.g. `launching 3 parallel agents — ≈3× the tokens of a single /do:next`). The `1..6` clamp (Parse Arguments) is deliberate: beyond ~6 concurrent worktrees/PRs, git-index-lock contention and merge-queue churn outweigh the throughput gain. **`SWARM_N` caps concurrency, not batch size** — an explicit list of 9 issues costs ≈9× regardless of how many waves it takes, so state the total (`9 issues named — ≈9× the tokens, 3 at a time`) and let the user cut the list if that's more than they meant: **for a named list of more than 8 members, stop and confirm before launching wave 1** — print the total cost and the wave plan, and proceed only on an explicit go-ahead. Below that threshold, state the cost and continue. (If the caller genuinely can't be asked — a non-interactive/subagent context — proceed, but log the total prominently rather than burying it.) + +### Swarm Phase A — Triage & partition (orchestrator, in the main repo) + +**Two paths in.** With **no target**, run **A1–A2** (auto-pick). With an **explicit list of two or more issue numbers**, skip the picker and run **A1e–A2e** instead. Both paths converge on **A3** and hand Phase B an ordered batch, split into waves of at most `SWARM_N`. + +1. **A1 — Build the eligible queue** exactly as **Phase 1 — issues mode** below: the priority-then-oldest walk with EVERY skip applied (in-flight, already-assigned, parking-labelled, `epic-open`/`epic-done` epics, blocked-by an open declared dependency), the **dispatch-hint filter** when `MODEL_FILTER`/`EFFORT_FILTER` is active (so `/do:next --swarm --model light` drains a wave of cheap work), and — **when `SELF_MODE` is on** — the `--author "@me"` filter so the batch only ever contains issues you filed (same security boundary as the single-issue flow). An `epic-wrapup` epic is eligible like any issue. Reuse that logic verbatim — do not invent a second picker. +2. **A2 — Select the first `SWARM_N` *independent* eligible issues** off the top of that ordered queue: + - **Intra-batch dependency.** If a candidate declares `Depends on #N` / `Blocked by #N` (or native blocked-by) on **another candidate in the batch**, keep only the predecessor this round — the successor self-clears and is picked next run once the predecessor merges. (Blockers *outside* the batch were already handled by the Phase 1 skip.) + - **File-overlap avoidance (best-effort).** From each issue's title/body, predict the rough files/paths/components it touches. When two candidates obviously target the same file(s), keep the higher-priority one and skip the other **this round** — not for correctness (the serialized merge + re-sync handles that) but to avoid two agents thrashing or duplicating the same file. This is a cheap heuristic, not a guarantee; note when you apply it. + - **Under-fill is fine.** If fewer than `SWARM_N` independent issues exist, run the swarm at the smaller size and say so. **If only one is eligible, run the normal single-issue flow instead** (Phases 1–7) and say so — a one-agent swarm is just `/do:next` with overhead. + + Auto-pick never selects more than `SWARM_N`, so it always yields exactly **one wave**. + +**A1e — Vet each named issue; no picker, no substitutions.** The list is a deliberate cherry-pick, so it **bypasses the auto-pick skips exactly as a single explicit `#` does**: parking labels (`future`/`blocked`/`discussion`/…), an active `LABEL_FILTER`, an active `MODEL_FILTER`/`EFFORT_FILTER`, and an open declared blocker that was **never named in the list** are all overridden — state each override as you apply it (e.g. `claiming future-labelled #123 by explicit request`). A named member still keeps its own dispatch hint for Phase B: overriding the *filter* selects the issue, it does not restate what the issue needs. **A blocker that *was* named and is then removed from the batch while still OPEN is a different case** — removed as in-flight/already-assigned, removed by *this very rule* as a hold, or removed by A2e as part of an unorderable dependency cycle: it still blocks its dependent, so **hold that dependent here, before A2e orders anything** — drop it from the batch with a note (`#21 held: depends on #17, which is claimed elsewhere and won't merge this run`) and carry it into Phase D's summary as `held`. **Apply this rule repeatedly until no new holds appear** — a member you just held is itself a still-open removed blocker, so a chain (`#23` depends on `#21` depends on `#17`) must hold `#21` **and** `#23`, never just the first link. If A2e later drops cycle members, re-run this hold pass over what remains and re-apply the survivor routing before ordering waves. Don't defer any of this to Phase B's wave rule: A2e only defers a member behind *another surviving member*, so a removed blocker produces no deferral for that rule to act on, and the dependent would otherwise launch in wave 1 anyway. Holding here also matters because of what comes next — if the hold leaves a single survivor, the single-issue hand-off below would claim it through Phase 1 step 5's explicit-`#num` path, which *overrides* the open-blocker skip (`claiming #21 despite open blocker #17 by explicit request`) — the exact opposite of the hold. **Count survivors *after* these holds** when applying the "nothing survives / exactly one survives" rules below. A blocker dropped because it is **closed or doesn't exist doesn't block at all** (Phase 1 step 4), so its dependent runs normally — never hold work behind a dependency that's already satisfied. Do still check each named issue, in this order: + - **`--self` is the one gate a list cannot cross.** When `SELF_MODE` is on, verify every named issue's author is the running account (the same check as Phase 1 step 5) — an issue that **doesn't exist or can't be read** falls through to the drop rule below rather than the refusal, so a typo'd number is a skip, not a bogus "filed by someone else". Refuse the whole run only when a real issue resolves to another author: ``Issue # was filed by , not you — /do:next --self only works on issues you filed. Drop --self, or drop # from the list.`` Don't quietly drop it and swarm the rest — a silently shrunk batch hides a refused security gate. + - **Closed, or no such issue → drop with a note** (`#: already closed — skipping`). + - **In flight or already assigned → drop with a note** (`#: already claimed (branch/assignee) — skipping`). **Never substitute another issue for a dropped one** — the user named these; auto-pick is not in play. + - **Epic → resolve with Phase 1 step 3** and act on the state: an `epic-wrapup` joins the batch like any issue; an `epic-done` is closed inline and dropped from the batch; an `epic-open` joins the batch only as an explicit override (say that children are still open). + - **Nothing survives → stop** and list why per issue. **Exactly one survives → run the single-issue flow** (Phases 1–7) for it and say so. +**A2e — Order the batch and split it into waves.** Keep the **user's order** as the *default* sequence — a named list is a stated preference, so never re-sort it by priority. The two placement constraints below **outrank that order**: sort the members topologically with respect to intra-list dependencies first (a predecessor always precedes its dependent, even when the user named the dependent first), using the user's order only to break ties. Then: + - **Intra-list dependency ⇒ a later wave, not a drop.** If a member declares `Depends on #N` / `Blocked by #N` (or native blocked-by) on **another member**, place the dependent in a **later wave than its predecessor** instead of dropping it: Phase C merges each wave before the next one starts, so the dependency is satisfied by the time the successor's agent branches. A true cycle among members can't be ordered — drop those members with a note so a human can break it, then **re-run A1e's hold pass** over what remains: a dropped cycle member is a still-open removed blocker for anything that depends on it. + - **File-overlap ⇒ a later wave, not a drop.** Same best-effort title/body prediction as A2, but for a named list the remedy is separation in time rather than exclusion: put members that obviously touch the same file(s) in different waves, and note it. + - **Chunk into waves of at most `SWARM_N`,** preserving that (dependency-topological, else user) order **and the two placement constraints above** — which outrank both the chunk size and the stated order: a member deferred behind a predecessor or an overlap peer must land in a **strictly later** wave than it, so a wave may come out smaller than `SWARM_N`. Only when **nothing was deferred** and the batch that **survived A1e** is `<= SWARM_N` is this a single wave (the common case) — count survivors, not named targets, since A1e may have dropped some. State the plan up front: `9 issues survived vetting, 3 at a time — 3 waves (≈3× the tokens of a single /do:next per wave)`. +**A3 — Do NOT claim here (both paths).** Each issue is claimed *inside* its own subagent so the assignee-marker + race read-back runs per issue, atomically. The orchestrator only hands each agent one specific issue number. + +### Swarm Phase B — Fan out (one subagent per issue) + +Launch the current wave's agents **in parallel** (all Agent/Task calls in a single response) and wait for all to return. + +**Apply each issue's dispatch hint to its own agent.** Read `model:` / `effort:` off the issue you're about to hand out and configure that agent from it. **A1's auto-pick queue already carries `labels` (its issue-list call requests them); A1e's explicit-list path does not** — it vets each named issue individually — so for a named batch, fetch them per issue before dispatching rather than assuming they're in hand: GitHub `gh issue view --json labels -q '[.labels[].name]'`; GitLab `glab issue view --output json --jq '.labels'` (labels already come back as a flat string array, no `.name` needed). Skipping that is how a named swarm silently runs every worker at the session default despite A1e promising each member keeps its own hint — this is the payoff for the labels, and it's why a mixed batch shouldn't cost one flat rate per agent. + +**The labels name tiers, not models — resolve each against the host you're running on** using the shared rules in [lib/model-tiers.md](../../lib/model-tiers.md): `light` → this host's cheapest capable coding model, `medium` → its workhorse, `heavy` → **its strongest available model, named by alias** (`model: "opus"` on Claude Code — an alias, never a pinned version ID), and `effort:` → **advisory**: it tells the worker how careful this issue needs to be, and you **may** additionally set the sub-agent's reasoning-effort control from it (clamped to the host's nearest level) where the dispatch API has one — Claude Code's `Agent` tool does not, so pass the level in the worker's brief instead and let it run at session effort. An issue missing either label inherits the session's setting for that axis. That file also carries the two consequences that matter here: + +- **`model:heavy` reaches up, not just sideways** — it names this host's strongest alias, so a `heavy` issue runs on the strongest model even when the orchestrating session is mid-tier. That is the point of the tier: `heavy` marks work where a weaker model produces confident wrong answers. If the dispatch is rejected for lack of entitlement to that tier, retry that agent once with `model` omitted (inherit), note the degrade, and proceed. A stale advisory label must never block a swarm. +- **Degrade, never abort.** If this host can't set a per-agent model or effort (or can't spawn sub-agents at all — see the precondition), run each agent at the session default and report the hint in the Phase D summary instead. Dispatch is an optimization; the swarm is the feature. + +Two things this is **not**: it is not the `--model`/`--effort` *filter* (that chose the batch; this runs it), and it is not `--review-models` (that pins each *reviewer*, and passes through to `/do:pr` untouched — a worker's own model has no bearing on who reviews its PR). + +Give each subagent exactly one issue number and this task: + +> Run the `/do:next` single-issue flow for issue **#``** — the **Phase 1 explicit-`#` path** (for validation + variable setup) followed by **Phases 2 through 6 only** — in your own sibling worktree, with these adjustments: +> - **Validate & set up via Phase 1's explicit-number path first:** run Phase 1's `#` branch to confirm the issue is open and not in flight, resolve it if it's an epic, and set `ISSUE_NUM=` / `SLUG=issue-` — Phase 2's worktree/branch and every later `gh issue`/`glab issue` call depend on those variables. (Skip the auto-pick walk; the orchestrator already vetted this issue in Phase A, and an explicit number deliberately bypasses the auto-pick skips.) **Map this validation's failures to the structured result instead of the explicit-path's normal print-and-stop:** if it fails because the issue is now **in flight or already assigned** (a sibling `/do:next` claimed it between Phase A and now — a lost race, not an error), return `{ issue, status: "yielded" }`; if it fails because the issue is **closed or no longer exists**, return `{ issue, status: "skipped", reason }`. Either way, do NOT pick a different issue. +> - **Then claim via that same explicit-number path:** it performs the Phase 2 worktree creation + assignee-marker claim with the race read-back. If the read-back shows a sibling won the race, **yield** (release the marker, retract the claim branch, clean up your worktree) and return `{ issue, status: "yielded" }` — do NOT pick a different issue. +> - **Phase 3 still applies:** if the issue is stale/superseded/awaiting-input, skip it (release the marker, clean up) and return `{ issue, status: "skipped", reason }`. +> - Implement (Phase 4) and record completion + changelog (Phase 5) as normal. +> - **Ship via `/do:pr --no-merge`** with the run's review flags (Phase 6) and **STOP before the merge** — open the PR, run the review gate, but DO NOT merge and DO NOT run Phase 7 cleanup. The orchestrator owns the merge and the cleanup. +> - **Wait on external reviewers/CI ACTIVELY — never end your turn to "wait for a notification."** You are a subagent: if you stop while a background reviewer (`codex review`, `claude -p`, `agy`) or a CI watch is still running, your run is over — completion notifications are not guaranteed to reach a stopped subagent, and the orchestrator will read your premature last words as your final result while the reviewer's findings are lost. Wait with bounded blocking-chunk foreground Bash calls, each safely under the host's ~10-minute foreground cap, repeated until the reviewer's done-marker exists (the local-agent review loop's `$DONE_FILE` pattern): `for i in $(seq 1 55); do [ -f "$DONE_FILE" ] && break; sleep 10; done` — then immediately issue the same call again if it's still running. End your turn only to return the structured result below. +> - **Return a structured result**, one of two shapes Phase C dispatches on by whether `pr_number` is present: +> - **PR opened:** `{ issue, pr_number, branch, worktree, review_status, notes }`, where `review_status` is `/do:pr`'s aggregate (`clean` / `partial` / `inconclusive` / `dirty`) or `opened-no-review` when no external reviewer ran and the Local Code Review gate passed. +> - **No PR** (claim yielded to a race winner, or Phase 3 skipped it as stale): `{ issue, status: "yielded" | "skipped", reason }` — no `pr_number`. + +Pass each agent the review flags verbatim (`--review-with` / `--review-iterations` / `--review-mode` / stop-mode / `--reviewer-applies` / `--no-review`). **Also pass the orchestrator's resolved self decision as an explicit `--self` or `--no-self`** so each worker honors *this run's* mode instead of re-resolving the saved `self` default. This matters because the worker runs a fresh `/do:next #` whose Phase 1 would otherwise re-read the saved default: with a saved `self=true` and a per-run `--no-self`, Phase A correctly selected third-party issues but a worker re-resolving `self=true` would refuse them at the explicit-#num gate. Passing the typed flag makes the worker's gate use the orchestrator's mode (typed wins over saved default) — a redundant re-check when `--self` is on (Phase A already filtered the batch to your issues) and correctly any-author when `--no-self`. The **fix regression guard** and **CI flake handling** apply inside each agent automatically (they live in `/do:pr`'s loop and merge gate). Concurrent `git worktree add` against the shared repo can briefly contend on `.git` index locks — an agent that hits a transient lock retries once before failing. + +**Harness without parallel subagents?** Per the precondition, run this same per-issue task **sequentially** in the current session — one issue at a time, identical task body — collecting each result, then proceed to Phase C unchanged. The merge queue is already serialized, so sequential fan-out only loses the concurrency, not any correctness. **The dispatch hints degrade to reports here**, exactly as in the single-issue flow (Phase 1 step 6): there's no fresh agent to configure, so note each issue's hint and run in the current session. + +**Multi-wave batches (any batch A2e split into more than one wave).** Run **B → C once per wave, in wave order** — merge wave *K*'s PRs before launching wave *K+1*, so the next wave's agents branch off a default branch that already carries the previous wave's merged work. That ordering is what makes A2e's deferral correct: a member deferred for a dependency or a file overlap starts from a base that already contains what it was waiting on, so it needs no re-sync gymnastics. Phase D runs **once, after the last wave**, over the accumulated results. Re-state progress between waves (`wave 2/3 — #17 #21 #23`). If a wave leaves an issue **unmerged for ANY reason** — PR left open (unmergeable or review gate unsatisfied), agent `yielded`, Phase 3 `skipped` it as stale, or the agent never returned (Phase D reconciles it, but not until after the last wave) — continue to the next wave anyway, but **hold back any later member A2e deferred behind that issue**: its declared blocker is genuinely still open, so skip it with a note (`#21 held: depends on #17, which did not merge this run`), exactly as Phase 1's blocked-by skip would. Gate the hold on *not merged*, never on *PR left open* — a yielded or dead-agent predecessor leaves the dependent just as unsupported as an open PR does. Members deferred only for file overlap still run. + +### Swarm Phase C — Serialized merge queue (orchestrator) + +After the barrier, merge the wave's returned PRs **one at a time, never concurrently** — each merge advances the default branch, so the next PR may need a re-sync. Walk the results in the batch's own order: priority/oldest for an auto-picked batch, the **user's order** for an explicit list (A2e). For each: + +1. **Skip non-mergeable results.** A result with `status: "yielded"`/`"skipped"` (no `pr_number`) has nothing to merge — record it. For the rest, apply **single-issue Phase 6's merge gate** to `review_status`: never merge `dirty` (build/test broken) or `inconclusive` (a requested reviewer missing/timed-out/errored) — leave that PR open and record why; merge only `clean`, `opened-no-review` (Local gate passed, no external reviewer requested), or `partial` **with** an explicit `--review-stop-on-*` flag. (Agents that never returned are reconciled in Phase D, not here.) +2. **Re-sync onto the advanced default branch** from the PR's worktree — `git fetch origin ` then `git merge --no-edit origin/` — resolving any PLAN.md/changelog conflict **deletions-win** (a line removed on either side stays removed; keep additions from both). If the merge **can't be resolved cleanly**, leave that PR open, record it for human follow-up, and move to the next — **never force it**. +3. **Gate on required CI, then merge.** Because each swarm agent opened its PR with `/do:pr --no-merge`, `/do:pr`'s own CI merge gate never ran — and the re-sync in step 2 just pushed a new SHA whose checks are pending — so the orchestrator must run the CI gate here, exactly as the single-issue Phase 6 / `/do:pr` merge does, before merging. Push the re-synced SHA, wait for CI, and only then merge; on a check failure apply **CI flake handling** (one re-run on the same commit; flake → proceed, real → leave that PR open, record it, and move to the next — see `~/.claude/lib/ci-flake-handling.md`). + - GitHub (`gh`) — scope the watch to **required** checks only so an optional/non-required job can't block a merge branch protection would allow (vacuously satisfied when no required checks exist): + ```bash + git -C "" push + # &&, not three separate lines: `--fail-fast` makes `gh pr checks` exit non-zero on + # a failing required check, but an unchained next line merges anyway — which is the + # opposite of what this step's own prose promises. Chain it so a red gate stops here. + gh pr checks --required --watch --fail-fast && \ + gh pr merge --merge + # Delete the head branch ONLY once the PR really reads MERGED. + if [ "$(gh pr view --json state -q .state)" = "MERGED" ]; then + if ! git push origin --delete ""; then + # rc 2 means "no such ref" — already gone, which is success. Any other rc is + # a transport/auth failure that proves nothing about the branch. + git ls-remote --exit-code --heads origin "" >/dev/null 2>&1; RC=$? + [ "$RC" -eq 2 ] || echo "ERROR: could not confirm is gone (ls-remote rc=$RC) — record this PR for follow-up" + fi + else + echo "PR is not MERGED — keeping " + fi + ``` + **No `--delete-branch`** — it deletes the *local* branch too, and `` (the `branch` field the worker returned, normally `next/issue-`) is checked out in the agent's worktree, so git refuses (`cannot delete branch 'next/issue-' used by worktree at …`) and **`gh` exits non-zero after the merge already succeeded**. That reads as a merge failure and fires any `||` fallback wrapped around the merge. Delete the remote branch with the explicit `git push origin --delete` above — it needs no local checkout — and let Phase D remove the worktree and the local branch from the main repo, where that works. **The `MERGED` read-back is load-bearing**: `--delete-branch` only ever deleted the head branch *because* the merge had happened, and an ungated delete would retract the head of a PR that is still open — either the merge failed (unmergeable, branch protection, a lost race) or, on a repo with a **merge queue**, `gh pr merge` returned success having merely *queued* it. GitHub auto-closes a PR whose head branch disappears, which destroys both the "leave that PR open, record it, and move to the next" outcome step 3 requires and the queued merge itself. Read the state back rather than trusting the merge command's exit status. + - GitLab (`glab`) — there's no discrete "required checks" list to scope to; the project's own merge/pipeline-success requirement governs, so wait on the pipeline explicitly and merge only then: + ```bash + git -C "" push + glab ci status --wait && glab mr merge --yes --remove-source-branch + # Read the state back for the same reason the gh path does: --auto-merge returns + # while the MR is still queued behind the pipeline, and step 4 gates issue closure + # on this answer. + if [ "$(glab mr view --output json --jq .state)" = "merged" ]; then + echo "MR merged" + else + echo "MR is not merged (queued on the pipeline) — leaving its issue open and keeping " + fi + ``` +**Why `glab ci status --wait` and not `--auto-merge`:** `--auto-merge` does not wait — it sets merge-when-pipeline-succeeds server-side and returns while the MR is still `opened`. The read-back above would then be `opened` on every run, so step 4 would never close an issue and Phase D would never reclaim a worktree: the guard against closing unlanded work would become a guard against closing anything. Waiting first makes one read authoritative. If you must use `--auto-merge` (a very long pipeline, say), replace the single read with a bounded poll of `glab mr view --output json --jq .state` and treat "still `opened` at the deadline" as queued, not merged. +4. **Close out the issue — only when step 3 read back `MERGED`.** A PR the merge left open or queued has shipped nothing, so closing its issue would drop live work out of the queue: record it as left-open with the reason instead, and let Phase D keep its worktree and branch. For a genuinely merged PR, apply single-issue Phase 7's closure step: confirm `Closes #` auto-closed it on merge to the default branch; if still open, close it explicitly (GitHub: `gh issue close --comment "Shipped in PR #."`; GitLab: `glab issue note -m "Shipped in PR #." && glab issue close `). Drop the `in-progress` label. + +### Swarm Phase D — Reconcile, clean up, report + +Runs **once per invocation**, after the last wave, over every result the batch produced. + +1. **Sweep worktrees & branches** from the main repo: `git worktree remove` + `git branch -d`/`-D` each merged agent's worktree/branch, then `git worktree prune`. **Keep** the worktree/branch of any PR left open so the work can be finished. +2. **Handle agent death / spend-limit.** If an agent never returned (crash, monthly spend cap), reconcile from **tracker state, not its last words** — and **never assume a review status survived the death** (slashdo's local-reviewer verdicts aren't persisted to the PR, so there's no stored `review_status` to read back). A worktree/branch/PR may exist: + - **A PR opened:** recompute its gate from scratch before merging — re-run the run's review flags against it (`/do:pr --no-merge` with the same `--review-with`), or, for a no-external-review run, treat green required-CI + a mergeable state as `opened-no-review`-eligible. Merge it through the Phase C queue only on a clean recompute; if requested external reviewers can't be re-run to clean, leave the PR open and flag it for a human. + - **No PR (or an unclean recompute you won't finish):** **release the claim** — remove the assignee (GitHub: `gh issue edit --remove-assignee @me`; GitLab: `glab issue update --assignee "-$ME"`), delete the local+remote `next/issue-` branch, drop `in-progress` — so the issue returns to the queue, and flag it for human follow-up. +3. **Re-evaluate parent epics** for every issue that closed — a shipped issue may have been an epic's last child (the Phase 7 "re-evaluate the parent epic" step, run once per closed child). +4. **Reconcile changelog/PLAN churn.** Parallel claims all touch the same changelog file, when the project has one; the deletions-win re-syncs in Phase C should have kept it consistent — confirm the merged default branch's changelog carries every shipped issue's entry with no duplicate or resurrected lines. Skip this check for a project whose release notes come from commit messages. +5. **Print a summary table** — one row per batch issue, including the ones dropped in A1e and any held in a later wave: `issue · wave · dispatch (model/effort actually used, or `session` when the issue carried no hint — or when this host couldn't set them) · PR · result (merged / open: why / yielded / skipped / held / needs-input) · review status`. The dispatch column is what makes a wrong hint visible: a `model:light` issue whose agent needed three review iterations is a label to correct, not a mystery. For an explicit list, **account for every number the user named** — a named issue that never ran must appear with its reason, never be silently absent. diff --git a/src/transformer.js b/src/transformer.js index 44867fac..0348de9a 100644 --- a/src/transformer.js +++ b/src/transformer.js @@ -118,36 +118,80 @@ const LIB_SIBLING_LINK_RE = /\[[^\]]*\]\(\.\/([A-Za-z0-9._-]+\.md)\)/g; // left alone. const LIB_BACKTICK_RE = /`lib\/([A-Za-z0-9._-]+\.md)`/g; -// Reviewer BACKEND loops are mutually exclusive: one `--review-with` entry -// dispatches to exactly ONE of these per reviewer, yet inlining all four costs -// ~130KB (~33K tokens) in every command that runs a review — `/do:better`, -// `/do:better-swift`, `/do:review`, `/do:pr`, `/do:release`, `/do:depfree`, -// `/do:rpr`. The dispatcher (`multi-reviewer-loop.md`) stays inline because it is -// always on the taken path and is what names the backend to load; the backends -// themselves are written as sibling docs beside SKILL.md and cited by path, for -// the agent to read on demand. Environments with runtime `!cat` (Claude/OpenCode) -// never reach this path and are unaffected. -const DEFERRED_LIBS = new Set([ - 'copilot-review-loop.md', - 'github-reviewer-loop.md', - 'local-agent-review-loop.md', - 'ollama-review-loop.md', +// Libs that sit on a CONDITIONAL path — content a given run needs only when it +// takes a particular branch. Inlining them puts every branch in every SKILL.md at +// once; for environments that bundle lib docs beside SKILL.md they are written as +// sibling files and cited with a read directive instead. +// +// The bar for an entry is that a real run can finish WITHOUT it. A lib the command +// always needs (`code-review-checklist.md` under a REQUIRED GATE, `swift-gotchas.md` +// which Phase 1 says to "load into your context") must stay inline: deferring it +// only buys an extra read, and risks the agent skipping content it always needed. +// +// `when` states the branch that makes the read required; `what` names the content. +// Both are rendered into the directive, so the agent is told when it must read the +// file rather than being left to infer it. Environments with runtime `!cat` +// (Claude/OpenCode) never reach this path and are unaffected. +const ON_DEMAND_LIBS = new Map([ + // Reviewer backends: `--review-with` dispatches to exactly one of these per + // reviewer, so at most one of the four is ever live. The dispatcher + // (multi-reviewer-loop.md) deliberately stays inline — it is always on the taken + // path and is what names which backend to load. + ['copilot-review-loop.md', + { what: 'Copilot reviewer loop', when: 'the reviewer list includes `copilot`' }], + ['github-reviewer-loop.md', + { what: 'GitHub-reviewer loop', when: 'the reviewer list includes an `@` reviewer' }], + ['local-agent-review-loop.md', + { what: 'local-agent reviewer loop', when: 'the reviewer list includes `codex`, `claude`, `agy`, `grok`, or `cursor`' }], + ['ollama-review-loop.md', + { what: 'Ollama reviewer loop', when: 'the reviewer list includes `ollama`' }], + + // Issue-tracker machinery: only reached in issues mode. PLAN.md mode — the + // default — never opens the tracker at all. + ['plan-issue-mode.md', + { what: 'issue-mode setup and filing rules', when: 'this run is in issues mode' }], + ['epic-children.md', + { what: 'epic/child issue resolution rules', when: 'a candidate issue is an epic or carries children' }], + + // Explicitly flag-gated or situational paths. + ['next-swarm.md', + { what: 'parallel swarm flow (phases A-D)', when: '`--swarm` was passed' }], + ['enhance-loop.md', + { what: 'draft-enhancement loop', when: '`--enhance-with` was passed' }], + ['ci-flake-handling.md', + { what: 'CI flake triage rules', when: 'a CI check fails in a way that looks like a flake' }], + + // Review lenses: review-agent-selection.md dispatches only the lenses a diff + // actually signals — often one or two, sometimes none. + ['review-surface-scan.md', + { what: 'Surface Scan (Runtime) lens', when: 'you dispatch that lens' }], + ['review-surface-quality.md', + { what: 'Surface Quality lens', when: 'you dispatch that lens' }], + ['review-security-audit.md', + { what: 'Security Audit lens', when: 'you dispatch that lens' }], + ['review-cross-file-tracing.md', + { what: 'Cross-File Tracing (State) lens', when: 'you dispatch that lens' }], + ['review-cross-file-contract.md', + { what: 'Cross-File Contract lens', when: 'you dispatch that lens' }], + ['review-structural-ambition.md', + { what: 'Structural Ambition lens', when: 'you dispatch that lens (strict mode only)' }], ]); -// Subdirectory, relative to a skill's own directory, that bundled lib docs are -// written into by the installer. Cited from SKILL.md as `lib/.md`. +const DEFERRED_LIBS = new Set(ON_DEMAND_LIBS.keys()); + const BUNDLED_LIB_DIR = 'lib'; // The read directive that replaces a deferred lib's inline content. Written as an -// imperative instruction rather than a passive link: the agent must treat it as a -// required read, not an optional reference, or the loop it names is lost. -function deferredLibDirective(ref, name) { +// imperative instruction naming the branch that makes it required, rather than a +// passive link: the agent must treat it as a required read on that path, not an +// optional reference, or the content it names is silently lost. +function deferredLibDirective(ref, filename) { + const { what, when } = ON_DEMAND_LIBS.get(filename); return [ - `> **Read \`${ref}\` now — required.** The full ${name} procedure lives in that`, - '> file, bundled alongside this skill. Read it in full before running this loop and', - '> follow it exactly. Do NOT improvise the loop from the summary above: the file', - '> carries the load-bearing detail (exit codes, verdict parsing, iteration caps,', - '> convergence and push rules) that the summary deliberately omits.', + `> **Read \`${ref}\` now — required when ${when}.** The full ${what} lives in`, + '> that file, bundled alongside this skill. Read it in full before acting on this', + '> step and follow it exactly. Do NOT improvise from the summary above: the file', + '> carries the load-bearing detail this summary deliberately omits.', ].join('\n'); } @@ -197,7 +241,7 @@ function inlineLibReferences(body, libDir, opts = {}) { // A deferred backend is bundled as its own file and cited, never inlined. if (deferred.has(filename)) { bundled.add(filename); - return deferredLibDirective(bundledRef(filename), bareName(filename)); + return deferredLibDirective(bundledRef(filename), filename); } inlined.add(filename); return content; @@ -404,6 +448,7 @@ function transformLib(content, env, sourceLibDir, opts = {}) { } module.exports = { + ON_DEMAND_LIBS, DEFERRED_LIBS, BUNDLED_LIB_DIR, parseFrontmatter, diff --git a/test/glab-jq-contract.test.js b/test/glab-jq-contract.test.js index 7519f23c..3398796c 100644 --- a/test/glab-jq-contract.test.js +++ b/test/glab-jq-contract.test.js @@ -6,7 +6,19 @@ const fs = require('fs'); const path = require('path'); const root = path.join(__dirname, '..'); -const next = fs.readFileSync(path.join(root, 'commands', 'do', 'next.md'), 'utf8'); +// A command's contract spans the file plus the lib docs it `!cat`-includes: those +// are one document to the agent, and splitting a section into lib/ must not move it +// out of a contract's reach. Resolve includes so these assertions scan what actually +// reaches the agent, not just the top-level file. +const resolveIncludes = (body) => body.replace( + /!`cat ~\/\.claude\/lib\/(.+?)`/g, + (match, name) => { + const libFile = path.join(__dirname, '..', 'lib', name); + return fs.existsSync(libFile) ? fs.readFileSync(libFile, 'utf8') : match; + }); + +const next = resolveIncludes( + fs.readFileSync(path.join(root, 'commands', 'do', 'next.md'), 'utf8')); // `glab api` — unlike the `glab issue` / `glab mr` subcommands — has no built-in // `--jq` flag and exits with "Unknown flag: --jq", so every `glab api` call pipes to diff --git a/test/installer.test.js b/test/installer.test.js index 5fb372a3..2be43efe 100644 --- a/test/installer.test.js +++ b/test/installer.test.js @@ -794,18 +794,41 @@ describe('bundled lib docs', () => { } it('writes each deferred lib beside the SKILL.md that cites it', () => { + // Only the libs a command actually cites — /do:pr runs reviewers, so it bundles + // the four backends; it never opens the tracker, so it gets no issue-mode doc. const { tmpDir, env } = makeSkillEnv(); try { install({ env, packageDir: PACKAGE_DIR, dryRun: false }); const bundleDir = path.join(env.commandsDir, 'do-pr', BUNDLED_LIB_DIR); assert.ok(fs.existsSync(bundleDir), 'do-pr must get a bundle dir'); const written = fs.readdirSync(bundleDir); - for (const name of DEFERRED_LIBS) { + for (const name of ['copilot-review-loop.md', 'github-reviewer-loop.md', + 'local-agent-review-loop.md', 'ollama-review-loop.md']) { assert.ok(written.includes(name), `${name} must be bundled with /do:pr`); } + assert.ok(!written.includes('plan-issue-mode.md'), '/do:pr does not use issue mode'); } finally { cleanup(tmpDir); } }); + it('bundles a deferred lib only into the commands that cite it', () => { + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + const bundled = (skill, name) => + fs.existsSync(path.join(env.commandsDir, skill, BUNDLED_LIB_DIR, name)); + assert.ok(bundled('do-next', 'plan-issue-mode.md'), '/do:next has an issues mode'); + assert.ok(bundled('do-review', 'review-security-audit.md'), '/do:review has lenses'); + assert.ok(!bundled('do-next', 'review-security-audit.md'), '/do:next has no lenses'); + } finally { cleanup(tmpDir); } + }); + + it('gives every deferred lib a when/what for its directive', () => { + const { ON_DEMAND_LIBS } = require('../src/transformer'); + for (const [name, meta] of ON_DEMAND_LIBS) { + assert.ok(meta && meta.when && meta.what, `${name} needs both when and what`); + } + }); + it('every cited bundle path resolves to a file on disk', () => { // The read directive is only as good as the path it names — a dangling one // silently drops the whole reviewer loop. diff --git a/test/worktree-merge-contract.test.js b/test/worktree-merge-contract.test.js index 74d5f487..d9c1d01b 100644 --- a/test/worktree-merge-contract.test.js +++ b/test/worktree-merge-contract.test.js @@ -5,7 +5,19 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const readCommand = (name) => fs.readFileSync(path.join(__dirname, '..', 'commands', 'do', name), 'utf8'); +// A command's contract spans the file plus the lib docs it `!cat`-includes: those +// are one document to the agent, and splitting a section into lib/ must not move it +// out of a contract's reach. Resolve includes so these assertions scan what actually +// reaches the agent, not just the top-level file. +const resolveIncludes = (body) => body.replace( + /!`cat ~\/\.claude\/lib\/(.+?)`/g, + (match, name) => { + const libFile = path.join(__dirname, '..', 'lib', name); + return fs.existsSync(libFile) ? fs.readFileSync(libFile, 'utf8') : match; + }); + +const readCommand = (name) => resolveIncludes( + fs.readFileSync(path.join(__dirname, '..', 'commands', 'do', name), 'utf8')); // Command lines only — the surrounding prose explains why `--delete-branch` is // absent, so a naive whole-file scan would flag its own rationale. diff --git a/uninstall.sh b/uninstall.sh index 3c37bf9a..9ad2c4cb 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -101,7 +101,7 @@ LIBS=( empty-array-expansion enhance-loop epic-children finding-disposition fix-regression-guard gh-host github-reviewer-loop graphql-escaping - local-agent-review-loop model-tiers multi-reviewer-loop ollama-review-loop + local-agent-review-loop model-tiers multi-reviewer-loop next-swarm ollama-review-loop per-finding-root-cause plan-id-format plan-issue-mode post-review-doc-recommendations remediation-agent-template review-agent-selection review-config-defaults review-convergence-gate From 8234401cbc86ecc6ff644fbd3692f0d1bea41516 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 28 Aug 2026 22:40:20 -0700 Subject: [PATCH 4/7] fix: refuse symlink traversal in bundled skill libs --- src/installer.js | 47 ++++++++++++++++++++++++++++++++++++------ test/installer.test.js | 43 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 6 deletions(-) diff --git a/src/installer.js b/src/installer.js index 39d0f122..1633c5f4 100644 --- a/src/installer.js +++ b/src/installer.js @@ -130,6 +130,22 @@ function removeFileSet(items, { getTargetPath, getLabel, dryRun, results }) { } } +function assertSafeBundlePath(targetPath, expectedType) { + let stat; + try { + stat = fs.lstatSync(targetPath); + } catch (error) { + if (error.code === 'ENOENT') return false; + throw error; + } + + const isExpectedType = expectedType === 'directory' ? stat.isDirectory() : stat.isFile(); + if (stat.isSymbolicLink() || !isExpectedType) { + throw new Error(`Refusing to traverse unsafe bundled lib ${expectedType}: ${targetPath}`); + } + return true; +} + const RENAMED_COMMANDS = { cam: 'push', makegoals: 'goals', @@ -234,6 +250,9 @@ function syncBundledLibs(commands, bundledByCommand, libDir, env, dryRun, result const { bundled: pending, present } = entry; const skillDir = path.dirname(path.join(env.commandsDir, getTargetFilename(cmd.relPath, env))); + assertSafeBundlePath(skillDir, 'directory'); + const bundleDir = path.join(skillDir, BUNDLED_LIB_DIR); + assertSafeBundlePath(bundleDir, 'directory'); const written = new Set(); // `pending` grows while draining when a bundled lib defers another. while (written.size < pending.size) { @@ -242,10 +261,12 @@ function syncBundledLibs(commands, bundledByCommand, libDir, env, dryRun, result written.add(filename); const absPath = path.join(libDir, filename); if (!fs.existsSync(absPath)) continue; + const targetPath = path.join(bundleDir, filename); + assertSafeBundlePath(targetPath, 'file'); syncFile({ label: `/do:${cmd.name} ${BUNDLED_LIB_DIR}/${filename}`, content: transformLib(fs.readFileSync(absPath, 'utf8'), env, libDir, { bundled: pending, present }), - targetPath: path.join(skillDir, BUNDLED_LIB_DIR, filename), + targetPath, dryRun, results, }); @@ -322,6 +343,23 @@ function install({ env, packageDir, filterNames, dryRun, uninstall, autoUpdate } } function doUninstall(commands, libFiles, hookFiles, env, results, dryRun, filterNames) { + const bundledToRemove = []; + if (env.bundlesLibs) { + for (const cmd of commands) { + const skillDir = path.dirname(path.join(env.commandsDir, getTargetFilename(cmd.relPath, env))); + if (!assertSafeBundlePath(skillDir, 'directory')) continue; + const bundleDir = path.join(skillDir, BUNDLED_LIB_DIR); + if (!assertSafeBundlePath(bundleDir, 'directory')) continue; + const names = fs.readdirSync(bundleDir); + // Validate the whole set before uninstall removes anything. This keeps an + // unexpected entry from causing a partial uninstall or escaping the skill. + for (const name of names) { + assertSafeBundlePath(path.join(bundleDir, name), 'file'); + } + bundledToRemove.push({ cmd, bundleDir, names }); + } + } + removeFileSet(commands, { getTargetPath: cmd => path.join(env.commandsDir, getTargetFilename(cmd.relPath, env)), getLabel: cmd => `/do:${cmd.name}`, @@ -333,11 +371,8 @@ function doUninstall(commands, libFiles, hookFiles, env, results, dryRun, filter // would strand them (and leave the directory behind). Remove every file the // bundle dir holds, then the now-empty dir. if (env.bundlesLibs) { - for (const cmd of commands) { - const skillDir = path.dirname(path.join(env.commandsDir, getTargetFilename(cmd.relPath, env))); - const bundleDir = path.join(skillDir, BUNDLED_LIB_DIR); - if (!fs.existsSync(bundleDir)) continue; - for (const name of fs.readdirSync(bundleDir)) { + for (const { cmd, bundleDir, names } of bundledToRemove) { + for (const name of names) { removeFile({ label: `/do:${cmd.name} ${BUNDLED_LIB_DIR}/${name}`, targetPath: path.join(bundleDir, name), diff --git a/test/installer.test.js b/test/installer.test.js index 2be43efe..2ef1e8dc 100644 --- a/test/installer.test.js +++ b/test/installer.test.js @@ -894,4 +894,47 @@ describe('bundled lib docs', () => { assert.equal(second.updated, 0); } finally { cleanup(tmpDir); } }); + + it('refuses to install through a symlinked bundled lib file', () => { + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + const target = path.join(env.commandsDir, 'do-pr', BUNDLED_LIB_DIR, + 'ollama-review-loop.md'); + const victim = path.join(tmpDir, 'victim.md'); + fs.writeFileSync(victim, 'must survive', 'utf8'); + fs.unlinkSync(target); + fs.symlinkSync(victim, target); + + assert.throws( + () => install({ + env, packageDir: PACKAGE_DIR, filterNames: ['pr'], dryRun: false, + }), + /Refusing to traverse unsafe bundled lib file/); + assert.equal(fs.readFileSync(victim, 'utf8'), 'must survive'); + } finally { cleanup(tmpDir); } + }); + + it('refuses to traverse a symlinked bundle directory on uninstall', () => { + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, dryRun: false }); + const bundleDir = path.join(env.commandsDir, 'do-pr', BUNDLED_LIB_DIR); + const victimDir = path.join(tmpDir, 'victim'); + const victim = path.join(victimDir, 'important.txt'); + fs.mkdirSync(victimDir); + fs.writeFileSync(victim, 'must survive', 'utf8'); + fs.rmSync(bundleDir, { recursive: true }); + fs.symlinkSync(victimDir, bundleDir, 'dir'); + + assert.throws( + () => install({ + env, packageDir: PACKAGE_DIR, filterNames: ['pr'], dryRun: false, uninstall: true, + }), + /Refusing to traverse unsafe bundled lib directory/); + assert.equal(fs.readFileSync(victim, 'utf8'), 'must survive'); + assert.ok(fs.existsSync(path.join(env.commandsDir, 'do-pr', 'SKILL.md')), + 'validation must fail before uninstall removes anything'); + } finally { cleanup(tmpDir); } + }); }); From 4ad6b9dc8901bb29212f06760b91bd0051a36e08 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 28 Aug 2026 22:48:25 -0700 Subject: [PATCH 5/7] fix: synchronize bundled skill libraries --- src/installer.js | 127 +++++++++++++++++++++++++++++++++-------- test/installer.test.js | 53 +++++++++++++++++ 2 files changed, 156 insertions(+), 24 deletions(-) diff --git a/src/installer.js b/src/installer.js index 1633c5f4..757981ef 100644 --- a/src/installer.js +++ b/src/installer.js @@ -146,6 +146,62 @@ function assertSafeBundlePath(targetPath, expectedType) { return true; } +function collectBundledLibContents(entry, libDir, env) { + const contents = new Map(); + if (!entry) return contents; + + const { bundled: pending, present } = entry; + const processed = new Set(); + // `pending` grows while draining when a bundled lib defers another. + while (processed.size < pending.size) { + for (const filename of Array.from(pending)) { + if (processed.has(filename)) continue; + processed.add(filename); + const absPath = path.join(libDir, filename); + if (!fs.existsSync(absPath)) continue; + contents.set(filename, transformLib( + fs.readFileSync(absPath, 'utf8'), env, libDir, { bundled: pending, present })); + } + } + return contents; +} + +function bundledLibsAreEqual(skillDir, expected) { + const bundleDir = path.join(skillDir, BUNDLED_LIB_DIR); + let bundleStat; + try { + bundleStat = fs.lstatSync(bundleDir); + } catch (error) { + return error.code === 'ENOENT' && expected.size === 0; + } + if (bundleStat.isSymbolicLink() || !bundleStat.isDirectory()) return false; + + let names; + try { + names = fs.readdirSync(bundleDir); + } catch { + return false; + } + if (names.length !== expected.size) return false; + + for (const [filename, content] of expected) { + const targetPath = path.join(bundleDir, filename); + let targetStat; + try { + targetStat = fs.lstatSync(targetPath); + } catch { + return false; + } + if (targetStat.isSymbolicLink() || !targetStat.isFile()) return false; + try { + if (fs.readFileSync(targetPath, 'utf8') !== content) return false; + } catch { + return false; + } + } + return true; +} + const RENAMED_COMMANDS = { cam: 'push', makegoals: 'goals', @@ -246,31 +302,45 @@ function finalizeInstall(env, hookFiles, packageDir, dryRun, filterNames, autoUp function syncBundledLibs(commands, bundledByCommand, libDir, env, dryRun, results) { for (const cmd of commands) { const entry = bundledByCommand.get(cmd.relPath); - if (!entry || entry.bundled.size === 0) continue; - const { bundled: pending, present } = entry; + if (!entry) continue; + const contents = collectBundledLibContents(entry, libDir, env); const skillDir = path.dirname(path.join(env.commandsDir, getTargetFilename(cmd.relPath, env))); - assertSafeBundlePath(skillDir, 'directory'); + const skillExists = assertSafeBundlePath(skillDir, 'directory'); const bundleDir = path.join(skillDir, BUNDLED_LIB_DIR); - assertSafeBundlePath(bundleDir, 'directory'); - const written = new Set(); - // `pending` grows while draining when a bundled lib defers another. - while (written.size < pending.size) { - for (const filename of Array.from(pending)) { - if (written.has(filename)) continue; - written.add(filename); - const absPath = path.join(libDir, filename); - if (!fs.existsSync(absPath)) continue; - const targetPath = path.join(bundleDir, filename); - assertSafeBundlePath(targetPath, 'file'); - syncFile({ - label: `/do:${cmd.name} ${BUNDLED_LIB_DIR}/${filename}`, - content: transformLib(fs.readFileSync(absPath, 'utf8'), env, libDir, { bundled: pending, present }), - targetPath, - dryRun, - results, - }); - } + const bundleExists = skillExists && assertSafeBundlePath(bundleDir, 'directory'); + const existingNames = bundleExists ? fs.readdirSync(bundleDir) : []; + + // Validate the existing set before changing anything. The whole directory is + // installer-owned, so a safe regular file not in `contents` is stale. + for (const name of existingNames) { + assertSafeBundlePath(path.join(bundleDir, name), 'file'); + } + + for (const [filename, content] of contents) { + const targetPath = path.join(bundleDir, filename); + assertSafeBundlePath(targetPath, 'file'); + syncFile({ + label: `/do:${cmd.name} ${BUNDLED_LIB_DIR}/${filename}`, + content, + targetPath, + dryRun, + results, + }); + } + + for (const name of existingNames) { + if (contents.has(name)) continue; + removeFile({ + label: `/do:${cmd.name} ${BUNDLED_LIB_DIR}/${name}`, + targetPath: path.join(bundleDir, name), + dryRun, + results, + }); + } + + if (!dryRun && bundleExists && fs.readdirSync(bundleDir).length === 0) { + fs.rmdirSync(bundleDir); } } } @@ -450,19 +520,28 @@ function doUninstall(commands, libFiles, hookFiles, env, results, dryRun, filter function list({ env, packageDir }) { const commandsDir = path.join(packageDir, 'commands'); + const libDir = path.join(packageDir, 'lib'); const commands = collectCommands(commandsDir); const items = []; for (const cmd of commands) { const content = fs.readFileSync(cmd.absPath, 'utf8'); - const transformed = transformCommand(content, env, path.join(packageDir, 'lib'), cmd.relPath); + const bundled = new Set(); + const present = new Set(); + const transformed = transformCommand( + content, env, libDir, cmd.relPath, { bundled, present }); + const expectedBundles = env.bundlesLibs + ? collectBundledLibContents({ bundled, present }, libDir, env) + : null; const targetRel = getTargetFilename(cmd.relPath, env); const targetPath = path.join(env.commandsDir, targetRel); let status; if (!fs.existsSync(targetPath)) { status = 'not installed'; - } else if (filesAreEqual(targetPath, transformed)) { + } else if (filesAreEqual(targetPath, transformed) + && (!expectedBundles + || bundledLibsAreEqual(path.dirname(targetPath), expectedBundles))) { status = 'up to date'; } else { status = 'changed'; diff --git a/test/installer.test.js b/test/installer.test.js index 2ef1e8dc..02790467 100644 --- a/test/installer.test.js +++ b/test/installer.test.js @@ -895,6 +895,59 @@ describe('bundled lib docs', () => { } finally { cleanup(tmpDir); } }); + it('prunes stale bundled libs, including a transition to no bundle', () => { + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, filterNames: ['pr'], dryRun: false }); + const bundleDir = path.join(env.commandsDir, 'do-pr', BUNDLED_LIB_DIR); + const stale = path.join(bundleDir, 'obsolete-backend.md'); + fs.writeFileSync(stale, 'obsolete', 'utf8'); + + const synchronized = install({ + env, packageDir: PACKAGE_DIR, filterNames: ['pr'], dryRun: false, + }); + assert.ok(!fs.existsSync(stale), 'an obsolete bundle member must be pruned'); + assert.ok(synchronized.actions.some(action => + action.name.endsWith(`${BUNDLED_LIB_DIR}/obsolete-backend.md`) + && action.status === 'removed')); + + const nextPackage = path.join(tmpDir, 'next-package'); + fs.mkdirSync(path.join(nextPackage, 'commands', 'do'), { recursive: true }); + fs.mkdirSync(path.join(nextPackage, 'lib')); + fs.writeFileSync(path.join(nextPackage, 'commands', 'do', 'pr.md'), + '---\ndescription: No bundled runtime\n---\n\nNothing deferred.\n', 'utf8'); + fs.writeFileSync(path.join(nextPackage, 'package.json'), + JSON.stringify({ version: '1.0.0' }), 'utf8'); + + install({ env, packageDir: nextPackage, filterNames: ['pr'], dryRun: false }); + assert.ok(!fs.existsSync(bundleDir), + 'the bundle directory must be removed when the current set becomes empty'); + } finally { cleanup(tmpDir); } + }); + + it('includes missing, changed, and stale bundled libs in list health', () => { + const { tmpDir, env } = makeSkillEnv(); + try { + install({ env, packageDir: PACKAGE_DIR, filterNames: ['next'], dryRun: false }); + const nextItem = () => list({ env, packageDir: PACKAGE_DIR }) + .find(item => item.name === '/do:next'); + const bundleDir = path.join(env.commandsDir, 'do-next', BUNDLED_LIB_DIR); + const bundlePath = path.join(bundleDir, 'next-swarm.md'); + + assert.equal(nextItem().status, 'up to date'); + fs.unlinkSync(bundlePath); + assert.equal(nextItem().status, 'changed', 'a missing required bundle is unhealthy'); + + install({ env, packageDir: PACKAGE_DIR, filterNames: ['next'], dryRun: false }); + fs.writeFileSync(bundlePath, 'changed', 'utf8'); + assert.equal(nextItem().status, 'changed', 'a modified required bundle is unhealthy'); + + install({ env, packageDir: PACKAGE_DIR, filterNames: ['next'], dryRun: false }); + fs.writeFileSync(path.join(bundleDir, 'obsolete.md'), 'stale', 'utf8'); + assert.equal(nextItem().status, 'changed', 'an extra stale bundle is unhealthy'); + } finally { cleanup(tmpDir); } + }); + it('refuses to install through a symlinked bundled lib file', () => { const { tmpDir, env } = makeSkillEnv(); try { From bce2f17bda3b7f256240300c9f869ee35caeedfc Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 28 Aug 2026 22:56:26 -0700 Subject: [PATCH 6/7] fix(installer): clean up empty skill directory on uninstall and update multi-reviewer-loop docs --- lib/multi-reviewer-loop.md | 2 +- src/installer.js | 9 +++++++++ test/installer.test.js | 3 ++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/multi-reviewer-loop.md b/lib/multi-reviewer-loop.md index 85f2faee..5346cafa 100644 --- a/lib/multi-reviewer-loop.md +++ b/lib/multi-reviewer-loop.md @@ -59,7 +59,7 @@ This is the default path. Iterate `REVIEW_AGENTS` in order, running each reviewe 1. **Print a banner**: `--- Review pass {n}/{N}: {REVIEW_AGENT} ---` 2. **Capture baseline**: `PASS_START_SHA=$(git rev-parse HEAD)` so the wrapper can tell whether this reviewer changed anything (independent of the inner loop's own tracking). -3. **Dispatch** to the matching single-reviewer loop. The loop file lives under the host CLI's lib directory (`~/.claude/lib/` for Claude, `~/.config/opencode/lib/` for OpenCode — slashdo's installer rewrites command-spec `!cat` references for each env, so use the same lib basename in whichever env this wrapper executes. For Antigravity and Codex there is no separate lib path: slashdo inlines the loop bodies directly into the installed skill, so the dispatch targets below are already present in-context rather than at a file path): +3. **Dispatch** to the matching single-reviewer loop. The loop file lives under the host CLI's lib directory (`~/.claude/lib/` for Claude, `~/.config/opencode/lib/` for OpenCode) or bundled alongside the skill under `lib/` (for Antigravity, Codex, Grok — read on demand per the directive in this skill): Forward this entry's resolved `{MAX_ITERATIONS}` and `{MAX_EXPLICIT}` (from the cap-resolution step) to **every** target below — that pair is what carries a `~max=` into the reviewer; forward `{ENTRY_EFFORT}` as `{REVIEW_EFFORT}` / `{OLLAMA_EFFORT}` (empty if unset): - `copilot` → `{LIB_DIR}/copilot-review-loop.md` (forward `{GH_HOST}`, and this entry's `{MAX_ITERATIONS}` as its `{REVIEW_ITERATIONS}` iteration cap) - `@` → `{LIB_DIR}/github-reviewer-loop.md` (forward `{GH_HOST}`, this entry's `{REVIEWER_LOGIN}`, and this entry's `{MAX_ITERATIONS}` as its `{REVIEW_ITERATIONS}` iteration cap) diff --git a/src/installer.js b/src/installer.js index 757981ef..22803f76 100644 --- a/src/installer.js +++ b/src/installer.js @@ -454,6 +454,15 @@ function doUninstall(commands, libFiles, hookFiles, env, results, dryRun, filter } } + if (env.namespacing === 'directory') { + for (const cmd of commands) { + const skillDir = path.dirname(path.join(env.commandsDir, getTargetFilename(cmd.relPath, env))); + if (!dryRun && fs.existsSync(skillDir) && fs.readdirSync(skillDir).length === 0) { + fs.rmdirSync(skillDir); + } + } + } + if (env.libDir) { removeFileSet(libFiles, { getTargetPath: lib => path.join(env.libDir, lib.relPath), diff --git a/test/installer.test.js b/test/installer.test.js index 02790467..ef564f84 100644 --- a/test/installer.test.js +++ b/test/installer.test.js @@ -874,7 +874,7 @@ describe('bundled lib docs', () => { } finally { cleanup(tmpDir); } }); - it('removes bundled libs on uninstall', () => { + it('removes bundled libs and skill directory on uninstall', () => { const { tmpDir, env } = makeSkillEnv(); try { install({ env, packageDir: PACKAGE_DIR, dryRun: false }); @@ -882,6 +882,7 @@ describe('bundled lib docs', () => { assert.ok(fs.existsSync(bundleDir)); install({ env, packageDir: PACKAGE_DIR, dryRun: false, uninstall: true }); assert.ok(!fs.existsSync(bundleDir), 'bundle dir must not be stranded'); + assert.ok(!fs.existsSync(path.join(env.commandsDir, 'do-pr')), 'skill dir must not be stranded'); } finally { cleanup(tmpDir); } }); From 3e0655831424eca7d280efd9974dcd979c520653 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 28 Aug 2026 23:02:39 -0700 Subject: [PATCH 7/7] chore: release v3.35.2 --- .changelogs/v3.35.2.md | 31 +++++++++++++++++++++++++++++++ package.json | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 .changelogs/v3.35.2.md diff --git a/.changelogs/v3.35.2.md b/.changelogs/v3.35.2.md new file mode 100644 index 00000000..9346ea32 --- /dev/null +++ b/.changelogs/v3.35.2.md @@ -0,0 +1,31 @@ +# Release v3.35.2 + +Released: 2026-08-28 + +## Highlights + +- **On-demand bundled libraries for Agent Skills environments.** For Antigravity CLI (`agy`), Codex, and Grok Build, slashdo now bundles conditional-path libraries into a local `lib/` directory alongside `SKILL.md` and loads them on demand with an explicit read directive, rather than inlining every branch into `SKILL.md`. This significantly trims the token footprint of installed skills like `/do:review`, `/do:pr`, and `/do:next`. +- **`/do:next` swarm flow extracted.** The parallel swarm dispatch logic (Phases A–D) is now split into `lib/next-swarm.md`, keeping single-issue `/do:next` streamlined while providing the full swarm orchestration when `--swarm` is invoked. +- **Installer and uninstaller hardening.** Bundled libraries are validated against symlink traversal and arbitrary path escaping (`assertSafeBundlePath`) during install, update, and uninstall. Empty skill directories are now cleanly removed upon uninstallation for directory-namespaced environments. +- **Full multi-environment health and parity.** All 21 commands and bundled libraries maintain zero dangling link references and seamless AST-free transformation across Claude Code, OpenCode, Antigravity CLI, Codex, and Grok Build. + +## Added + +- `lib/next-swarm.md`: standalone library containing the complete parallel swarm workflow (Phases A–D) for `/do:next --swarm`. +- `test/installer.test.js` & `test/transformer.test.js`: new test suites covering deferred lib bundling, cycle termination, list health with bundled dependencies, symlink traversal security guards, and uninstallation cleanup. + +## Changed + +- `src/transformer.js`: added `ON_DEMAND_LIBS`, `DEFERRED_LIBS`, `deferredLibDirective`, and AST-free markdown path transformation supporting sibling bundle references and relative linking. +- `src/installer.js`: added `syncBundledLibs`, `bundledLibsAreEqual`, stale bundle pruning, and `assertSafeBundlePath` security checks. +- `commands/do/next.md`: replaced inline swarm execution phases with a reference to `lib/next-swarm.md`. +- `install.sh` & `uninstall.sh`: added `next-swarm` to the curl installer's library allowlist. + +## Fixed + +- `lib/multi-reviewer-loop.md`: updated documentation to reflect on-demand library resolution for Agent Skills hosts instead of legacy full inlining. +- `src/installer.js`: ensured empty parent skill directories are removed during uninstallation of directory-namespaced skills. + +## Full Changelog + +**Full Diff**: https://github.com/atomantic/slashdo/compare/v3.35.1...v3.35.2 diff --git a/package.json b/package.json index 5f8fc9f6..829b1a5c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "slash-do", - "version": "3.35.1", + "version": "3.35.2", "description": "Curated slash commands for AI coding assistants — Claude Code, OpenCode, Antigravity CLI, Codex, and Grok Build", "author": "Adam Eivy ", "license": "MIT",