fix: five defects found dogfooding vigiles on a real consumer repo, plus CI job bounds (#162)

Five defects found dogfooding vigiles on a real consumer repo (zernie/mine — 43 skills,
48 harness files), each measured rather than reasoned about:

1. `exclude` never reached the markdown-refs pass. Measured: 9 broken refs, 8 of them
   inside a directory explicitly listed in `exclude`, all 9 still reported. After: 9 → 0.
   The consequence was larger than the cause — with no way to reach exit 0 the consumer
   set `continue-on-error: true`, and a lint whose exit code is discarded gates nothing.

2. Refs were found by TEXT, per line. Four of six probe inputs classified wrongly
   (`ctx.file("OUT")`, `obj.cmd(...)`, a comment, a string), and because the regex ran
   per line, any call split across lines was invisible — which had hidden two genuinely
   broken refs in this repo's own docs. Now parsed: a ref is a call whose callee is a
   bare identifier with a string-literal first argument. `typescript` was already a
   runtime dependency. Net: 21 refs → 25, all valid.

3. The script runner was a strict queue. Measured on 48 harness files: 270s → 77s
   (3.5x), byte-identical results. The default follows `entry` rather than a new flag:
   `test` fans out (free and deterministic by construction — mock model, no API key),
   `eval` stays at 1 (real model quota). `stdio: "inherit"` was the actual blocker.

4. Agent path attribution was keyed by name, so with `agents/foo.md` and
   `.claude/agents/foo.md` a lethal-trifecta finding was displayed against the wrong
   file. Ambiguous names now yield no path instead of an arbitrary one.

5. `compile` emitted invalid YAML. A description containing `: ` produced frontmatter
   where `skillContract()` reports `malformed: true, declared: []` — the blessed path
   producing the exact defect `frontmatter-valid` exists to report, and punishing anyone
   who fixed it by hand with a hash mismatch. Fixed by round-tripping through the same
   loader the linter uses, so already-safe values stay byte-identical and no integrity
   hash moves.

Plus: all 10 jobs across 6 workflows were unbounded and now carry `timeout-minutes`
sized to measured durations; docs updated in the pages that own each defect.

Gates: vitest 3186 passed - lint 0 errors - prettier clean - api:check no drift - build.
CI: all 7 jobs green.
This commit is contained in:
zernie
2026-08-19 22:48:28 +05:00
committed by GitHub
parent 04225f9980
commit 6532dbf48d
37 changed files with 2870 additions and 270 deletions
+15
View File
@@ -9,6 +9,13 @@ on:
jobs:
test:
runs-on: ubuntu-latest
# Bounded 2026-08-19. Every job in this repository was unbounded, so a hung one
# ran to GitHub's six-hour ceiling — the one failure mode a green checkmark
# cannot show you, because it does not fail, it bills. Sizes are ~3-4x the
# MEASURED duration on this repo's own runs, quoted per job so a later edit can
# see where the number came from instead of guessing again.
# measured 5m49s.
timeout-minutes: 20
env:
# JVM/Go linter pins for the gated real-binary catalog tests
# (src/core/linters.test.ts — describe.skipIf(!hasBinary(...))). Installing
@@ -219,6 +226,8 @@ jobs:
check:
runs-on: ubuntu-latest
# measured 2m36s.
timeout-minutes: 10
# pull-requests: write lets the dogfooded action post its sticky PR comment.
permissions:
contents: read
@@ -306,6 +315,8 @@ jobs:
# no API key and costs nothing. The eval tier (real model) is run manually
# via `npm run test:eval`.
runs-on: ubuntu-latest
# measured 45s.
timeout-minutes: 10
steps:
- uses: actions/checkout@v4
@@ -338,6 +349,8 @@ jobs:
# catches layout bleed a component-level test can't (no viewport in @vitest/browser).
site:
runs-on: ubuntu-latest
# measured 3m35s.
timeout-minutes: 15
steps:
- uses: actions/checkout@v4
@@ -377,6 +390,8 @@ jobs:
# route, so the job never fails red. See research/egress-sandbox-tooling.md.
e2e:
runs-on: ubuntu-latest
# measured 1m16s.
timeout-minutes: 10
container:
image: node:20-bookworm
options: --privileged --device /dev/net/tun
@@ -12,6 +12,13 @@ permissions:
jobs:
cleanup:
runs-on: ubuntu-latest
# Bounded 2026-08-19. Every job in this repository was unbounded, so a hung one
# ran to GitHub's six-hour ceiling — the one failure mode a green checkmark
# cannot show you, because it does not fail, it bills. Sizes are ~3-4x the
# MEASURED duration on this repo's own runs, quoted per job so a later edit can
# see where the number came from instead of guessing again.
# not measured — a branch delete; if it ever needs 5m something is wrong.
timeout-minutes: 5
steps:
- name: Delete branches whose pull request was merged
env:
+7
View File
@@ -32,6 +32,13 @@ concurrency:
jobs:
build-and-deploy:
runs-on: ubuntu-latest
# Bounded 2026-08-19. Every job in this repository was unbounded, so a hung one
# ran to GitHub's six-hour ceiling — the one failure mode a green checkmark
# cannot show you, because it does not fail, it bills. Sizes are ~3-4x the
# MEASURED duration on this repo's own runs, quoted per job so a later edit can
# see where the number came from instead of guessing again.
# not measured here; sized like `site`, which builds the same assets.
timeout-minutes: 15
environment:
name: github-pages
url: ${{ steps.deploy.outputs.page_url }}
+7
View File
@@ -30,6 +30,13 @@ concurrency:
jobs:
describe:
runs-on: ubuntu-latest
# Bounded 2026-08-19. Every job in this repository was unbounded, so a hung one
# ran to GitHub's six-hour ceiling — the one failure mode a green checkmark
# cannot show you, because it does not fail, it bills. Sizes are ~3-4x the
# MEASURED duration on this repo's own runs, quoted per job so a later edit can
# see where the number came from instead of guessing again.
# measured 8s.
timeout-minutes: 5
# Fork PRs get a read-only GITHUB_TOKEN, so the write would fail — skip them.
if: github.event.pull_request.head.repo.full_name == github.repository
steps:
+7
View File
@@ -10,6 +10,13 @@ permissions:
jobs:
validate:
runs-on: ubuntu-latest
# Bounded 2026-08-19. Every job in this repository was unbounded, so a hung one
# ran to GitHub's six-hour ceiling — the one failure mode a green checkmark
# cannot show you, because it does not fail, it bills. Sizes are ~3-4x the
# MEASURED duration on this repo's own runs, quoted per job so a later edit can
# see where the number came from instead of guessing again.
# measured 5s.
timeout-minutes: 5
steps:
- uses: amannn/action-semantic-pull-request@v5
env:
+7
View File
@@ -12,6 +12,13 @@ permissions:
jobs:
release:
runs-on: ubuntu-latest
# Bounded 2026-08-19. Every job in this repository was unbounded, so a hung one
# ran to GitHub's six-hour ceiling — the one failure mode a green checkmark
# cannot show you, because it does not fail, it bills. Sizes are ~3-4x the
# MEASURED duration on this repo's own runs, quoted per job so a later edit can
# see where the number came from instead of guessing again.
# not measured; publishes to npm, so bounded generously but bounded.
timeout-minutes: 20
# Skip release on commits that don't affect the package
if: "!contains(github.event.head_commit.message, '[skip ci]')"
steps:
+5 -1
View File
@@ -101,7 +101,7 @@ declare module "vigiles/generated" {
| "experimental:check"
| "docs:api";
/** 387 project files. */
/** 389 project files. */
export type ProjectFile =
| "src/CLAUDE.md"
| "src/CLAUDE.md.spec.ts"
@@ -324,6 +324,8 @@ declare module "vigiles/generated" {
| "src/core/source-refs.ts"
| "src/core/spec.test.ts"
| "src/core/spec.ts"
| "src/core/surface-scopes.test.ts"
| "src/core/surface-scopes.ts"
| "src/core/symbols.test.ts"
| "src/core/symbols.ts"
| "src/core/test-file-ext.test.ts"
@@ -783,6 +785,8 @@ declare module "vigiles/spec" {
| "src/core/source-refs.ts"
| "src/core/spec.test.ts"
| "src/core/spec.ts"
| "src/core/surface-scopes.test.ts"
| "src/core/surface-scopes.ts"
| "src/core/symbols.test.ts"
| "src/core/symbols.ts"
| "src/core/test-file-ext.test.ts"
+13
View File
@@ -88,6 +88,19 @@ answer the prompt at a terminal. Run headless with none of those and it **refuse
always runs everything it finds. (Discovery is by name — `*.harness.*` / `*.eval.*`,
never `*.test.*` / `*.spec.*` — so it won't pick up your vitest/jest files.)
**`test` runs several scripts at once; `eval` runs them one at a time.** The two share a runner
and want opposite defaults, so the default follows the tier rather than a flag. Harness scripts are
free and deterministic by construction — the tier drives the agent CLI against a mock model with no
API key — so running them concurrently cannot cost anything, and on one real repository of 48
harness files it took the suite from **270s to 77s with byte-identical results**. Evals spend real
model quota, where concurrency means simultaneous billed calls and rate limits, so they stay
strictly serial.
Each script's output is buffered and printed whole when that script finishes, rather than streamed
live, because concurrent children writing to one terminal interleave into an unreadable mess.
Results are reported in discovery order regardless of which finished first — a run that reorders
its own output between invocations reads as flaky even when every result is stable.
**Already have vitest or jest? You don't need a second runner for the
deterministic tier.** The testing API — `runHook`, `runHarnessTest`, the check
vocabulary, and the matchers (via `vigiles/vitest` / `vigiles/jest`) — are plain
+15
View File
@@ -61,6 +61,21 @@ Out of scope — use other tools:
| Markdown link validity (URLs, paths) | [markdown-link-check](https://github.com/tcort/markdown-link-check) |
| Spell / prose / grammar | [Vale](https://vale.sh/), [alex](https://github.com/get-alex/alex) |
**What counts as a ref (2026-08-19).** Blocks are PARSED, not pattern-matched. A ref is a call
whose callee is a bare `enforce` / `file` / `cmd` / `ref` identifier with a string-literal first
argument. That means a method call on some other object (`ctx.file("OUT")`), a mention inside a
comment (`// cmd("npm test")`), and a string containing one (`'cmd("x")'`) are all NOT refs — none
of them is a call expression to the builder. Until this release those three were matched by a
regex and reported as broken refs; `\b` sits happily after a `.`, and a regex has no notion of a
comment or a string literal. Parsing makes all three inexpressible rather than individually
excused.
**Scope.** The pass reads `**/*.md` from the repo root and honours the top-level `exclude` in
`.vigilesrc.json`, so vendored or benchmark markdown (a third-party `CLAUDE.md` captured verbatim
as test data) can be kept out of it. Before 2026-08-19 `exclude` was not applied here at all, which
meant a repository vendoring other people's markdown had no way to reach a clean `lint` — and a
lint that cannot exit 0 gets its exit code discarded, at which point it gates nothing.
Illustrative code blocks (typo demos, template placeholders, speculative refs in design docs) opt out via `<!-- vigiles:ignore -->` immediately before the fence, or `<!-- vigiles:ignore-file -->` anywhere in a file that's entirely illustrative. Placeholders containing `<` or `>` are auto-skipped. Refs that can't be verified because the underlying tool isn't installed (e.g. `pylint/X` on a machine without pylint) are reported separately from real errors.
## What vigiles composes with
+15
View File
@@ -108,6 +108,21 @@ in isolation. A linter that checks one unit at a time never sees it; the
**combination across the tree** is the risk, and it's decidable from the declared
contracts — for free, no model.
## When two agents share a name
Claude registers `agents/foo.md` and `.claude/agents/foo.md` in DIFFERENT namespaces, so a
repository can legitimately hold two agents with the same `name:`. A finding carries the agent's
name, not its path, and the path shown alongside it is looked up from that name.
For an ambiguous name the finding is therefore reported **with no path** rather than with one of
the two files picked arbitrarily. Until 2026-08-19 the lookup kept whichever entry came last, so a
finding about the first agent was displayed against the second agent's file — which is worse than
showing nothing: it sends the reader to audit innocent code while the surface that actually holds
the trifecta goes unexamined.
The finding itself is unaffected; only the file attribution is withheld. Giving each scope its own
qualified identity end to end is the complete fix and is not done yet.
## See also
- [lethal-trifecta](lethal-trifecta.md) — the per-unit check this one extends
+19 -5
View File
@@ -16,7 +16,7 @@ these as first-class:
| **Skills library** | `skills/<name>/SKILL.md` at the repo root, no manifest | a CI-tested skill monorepo |
| **Plain Claude Code repo** | `.claude/skills/<name>/SKILL.md` (+ `.claude/agents`, `.claude/settings.json`, `CLAUDE.md`) | a normal user repo, not a plugin |
Point `audit`/`lint` at the repo root and vigiles reads whichever shape it finds.
Point `audit`/`lint` at the repo root and vigiles reads every shape it finds.
You can also point it at a **single skill directory** (the dir holding one
`SKILL.md`) and it scans just that skill.
@@ -25,10 +25,24 @@ npx vigiles audit . # the whole repo
npx vigiles audit skills/rca-investigation # one skill dir
```
**Repo-root `skills/` wins over `.claude/skills/`.** If a repo has both, the
root `skills/` is used — so a plugin author's own local `.claude/skills` dev skills
never pollute the audit of what the plugin ships. A plain user (no root `skills/`)
is read from `.claude/skills` as expected.
**A repo with BOTH `skills/` and `.claude/skills/` is audited as both**, because
Claude Code loads both:
> Plugin skills use a `plugin-name:skill-name` namespace, so they can't conflict
> with other levels. For example, `my-plugin/skills/deploy/SKILL.md` becomes
> `/my-plugin:deploy` and loads alongside a `deploy` skill in your project's
> `.claude/skills/`.
> — [Claude Code docs, "Where skills live"](https://code.claude.com/docs/en/skills)
So the same NAME in both places is **two skills**, not one, and vigiles reports two
surfaces at their two real paths plus a warning that the repo carries two discovery
levels. (Until 2026-08, vigiles read one location and reported it under the other
one's path — measured on a real plugin, 50 skill names existed in both trees and
all 50 pairs differed, so fifty audited "skills" named files that were never opened.)
The deterministic sandbox is a project directory, so it registers the **project**
scope only; exercise a plugin scope through `pluginDir` (a real `--plugin-dir`
install), which is what a session does with an installed plugin anyway.
⚠️ vigiles reads your **project** `.claude/` only. It never scans the machine-global
`~/.claude/` install — CI results stay reproducible and independent of the runner's
@@ -100,10 +100,10 @@
"confidence": "likely"
},
{
"surface": "deploy ↔ release",
"surface": "\"deploy\" (skills/deploy/SKILL.md)\"release\" (skills/release/SKILL.md)",
"action": "differentiate",
"rationale": "skills \"deploy\" and \"release\" have near-identical descriptions (100% alike) — the model can't reliably tell them apart, so the wrong one may fire. Differentiate their descriptions.",
"fix": "Differentiate the descriptions of \"deploy\" and \"release\" (1 similar) — the selector picks by description, so near-identical text makes it fire the wrong one.",
"rationale": "skills \"deploy\" (skills/deploy/SKILL.md) and \"release\" (skills/release/SKILL.md) have near-identical descriptions (100% alike) — the model can't reliably tell them apart, so the wrong one may fire. Differentiate their descriptions.",
"fix": "Differentiate the descriptions of \"deploy\" (skills/deploy/SKILL.md) and \"release\" (skills/release/SKILL.md) (1 similar) — the selector picks by description, so near-identical text makes it fire the wrong one.",
"detector": "description-overlap",
"confidence": "possible"
}
+2
View File
@@ -209,6 +209,8 @@ The module must be importable from `PYTHONPATH`. For a local checker, put it in
### vigiles enforce() reference
<!-- vigiles:ignore -->
```typescript
enforce(
"pylint/no-direct-db-query",
+2
View File
@@ -209,6 +209,8 @@ Custom/NoDirectDbQuery:
### vigiles enforce() reference
<!-- vigiles:ignore -->
```typescript
enforce(
"rubocop/Custom/NoDirectDbQuery",
+94 -26
View File
@@ -5,10 +5,11 @@
*/
import { test } from "vitest";
import assert from "node:assert/strict";
import { writeFileSync, mkdirSync } from "node:fs";
import { writeFileSync, mkdirSync, readFileSync } from "node:fs";
import { join, basename } from "node:path";
import { loadPlugin, resolveHarness } from "./plugin-loader.js";
import { claudeCodeLayout } from "./layout.js";
import { makeTmpDir, cleanupTmpDir } from "../../core/test-utils.js";
function makePlugin(): string {
@@ -533,9 +534,11 @@ test("loadPlugin loads an end-user repo's .claude/skills (no plugin.json)", () =
}
});
test("loadPlugin prefers a repo-root skills/ over .claude/skills when both exist", () => {
// A plugin/library author's OWN local `.claude/skills` dev skills must not
// pollute the audit of what the plugin ships — the repo-root `skills/` wins.
test("loadPlugin reads BOTH a repo-root skills/ and .claude/skills when both exist", () => {
// Claude Code loads both — the plugin copy as `/<plugin>:<name>`, the project
// copy as `/<name>` (docs: "loads alongside"). The loader used to read one and
// materialize it under the OTHER's canonical key, so a real file was never
// opened and its name carried someone else's bytes.
const root = makeTmpDir("bothshapes");
try {
mkdirSync(join(root, "skills", "lib-skill"), { recursive: true });
@@ -547,16 +550,78 @@ test("loadPlugin prefers a repo-root skills/ over .claude/skills when both exist
join(root, ".claude", "skills", "user-skill", "SKILL.md"),
"# user\n",
);
const { files } = loadPlugin(root);
const { files, sources } = loadPlugin(root);
assert.equal(
files[join(".claude", "skills", "lib-skill", "SKILL.md")],
files["skills/lib-skill/SKILL.md"],
"# lib\n",
"the repo-root skill is loaded",
"the plugin-scope skill is loaded, keyed where it really lives",
);
assert.equal(
files[join(".claude", "skills", "user-skill", "SKILL.md")],
undefined,
"the .claude/skills fallback is skipped when repo-root skills/ exists",
"# user\n",
"the project-scope skill is loaded too — no longer shadowed",
);
assert.equal(
sources["skills/lib-skill/SKILL.md"],
join(root, "skills", "lib-skill", "SKILL.md"),
);
} finally {
cleanupTmpDir(root);
}
});
test("loadPlugin: a name in BOTH scopes yields two surfaces, neither overwritten", () => {
// The measured case (nyldn/claude-octopus, 2026-08-18): 50 skill names live in
// both `skills/` and `.claude/skills/`, and all 50 pairs DIFFER. Under the old
// one-or-the-other rule the audit named all fifty and had opened none of them.
const root = makeTmpDir("dupname");
try {
mkdirSync(join(root, "skills", "dup"), { recursive: true });
writeFileSync(join(root, "skills", "dup", "SKILL.md"), "# plugin copy\n");
mkdirSync(join(root, ".claude", "skills", "dup"), { recursive: true });
writeFileSync(
join(root, ".claude", "skills", "dup", "SKILL.md"),
"# project copy\n",
);
const { files, sources, warnings } = loadPlugin(root);
assert.equal(files["skills/dup/SKILL.md"], "# plugin copy\n");
assert.equal(
files[join(".claude", "skills", "dup", "SKILL.md")],
"# project copy\n",
);
// Every key names the file it was actually read from — the property whose
// absence was the whole defect.
for (const [key, onDisk] of Object.entries(sources)) {
assert.equal(
readFileSync(onDisk, "utf-8"),
files[key],
`sources[${key}] must be the file the content came from`,
);
}
assert.ok(
warnings.some((w) => w.includes("TWO discovery levels")),
"a two-scope repo is called out, not silently merged",
);
} finally {
cleanupTmpDir(root);
}
});
test("loadPlugin THROWS on a layout whose scopes would collide, rather than dropping one", () => {
// The call-site half of `assertDistinctScopeKeys`: the unit test proves the
// function fires, this proves the loader actually calls it. A layout with an
// empty `materializeRoot` makes the project and plugin scopes mint the same
// prefix — the exact silent overwrite this whole change removed. No shipped
// layout can reach it, so without this the call could be deleted unnoticed.
const root = makeTmpDir("collide-layout");
try {
mkdirSync(join(root, "skills", "a"), { recursive: true });
writeFileSync(join(root, "skills", "a", "SKILL.md"), "# a\n");
mkdirSync(join(root, ".claude", "skills", "b"), { recursive: true });
writeFileSync(join(root, ".claude", "skills", "b", "SKILL.md"), "# b\n");
assert.throws(
() => loadPlugin(root, { ...claudeCodeLayout, materializeRoot: "" }),
/silently shadow/,
);
} finally {
cleanupTmpDir(root);
@@ -643,9 +708,10 @@ test("loadPlugin falls back to .claude/skills when the root skills/ is EMPTY", (
}
});
test("loadPlugin does NOT import project-local .claude/agents into a plugin", () => {
// A plugin/library repo (has a shipped root `skills/`) must not materialize a
// developer's local `.claude/agents` as if the plugin ships them.
test("loadPlugin materializes a plugin's project-local .claude/agents too", () => {
// A repo with a shipped root `skills/` AND a local `.claude/agents` runs BOTH
// in a real session: the subagent under `.claude/agents` is dispatchable by
// name. Dropping it audited a machine the author never runs.
const root = makeTmpDir("plugin-devagents");
try {
mkdirSync(join(root, "skills", "foo"), { recursive: true });
@@ -660,22 +726,23 @@ test("loadPlugin does NOT import project-local .claude/agents into a plugin", ()
);
const { files } = loadPlugin(root);
assert.ok(
files[join(".claude", "skills", "foo", "SKILL.md")],
"the shipped root skill is loaded",
files["skills/foo/SKILL.md"],
"the shipped root skill is loaded, keyed where it lives",
);
assert.ok(
!files[join(".claude", "agents", "dev.md")],
"a plugin's project-local .claude/agents dev agent is NOT materialized",
files[join(".claude", "agents", "dev.md")],
"the project-local .claude/agents subagent is materialized too",
);
} finally {
cleanupTmpDir(root);
}
});
test("loadPlugin does NOT fall back to .claude for a manifest-backed (hook-only) plugin", () => {
// A hook-only plugin has a manifest + hooks but no root surface dirs; its
// project-local `.claude/skills` is dev-only and must not be imported as shipped
// just because there's no root `skills/`.
test("loadPlugin reads .claude/skills for a manifest-backed (hook-only) plugin", () => {
// A hook-only plugin has a manifest + hooks but no root surface dirs. Its
// project-local `.claude/skills` is still a real project skill that loads in a
// session, so it is read — and since the root scope contributes no surface
// files, nothing competes for the canonical key.
const root = makeTmpDir("hookonly-plugin");
try {
mkdirSync(join(root, ".claude-plugin"), { recursive: true });
@@ -693,17 +760,18 @@ test("loadPlugin does NOT fall back to .claude for a manifest-backed (hook-only)
);
const { files } = loadPlugin(root);
assert.ok(
!files[join(".claude", "skills", "dev", "SKILL.md")],
"a manifest-backed plugin's project-local .claude/skills is NOT materialized",
files[join(".claude", "skills", "dev", "SKILL.md")],
"a manifest-backed plugin's project-local .claude/skills IS materialized",
);
} finally {
cleanupTmpDir(root);
}
});
test("loadPlugin treats a hooks/hooks.json convention plugin as plugin-shaped (no .claude fallback)", () => {
test("loadPlugin reads .claude/skills for a hooks/hooks.json convention plugin", () => {
// A hook-only plugin via the `hooks/hooks.json` convention (no manifest, no root
// surfaces) is still a plugin — its project-local `.claude/skills` is dev-only.
// surfaces) is still a plugin — and its project-local `.claude/skills` is still
// a project skill the session loads.
const root = makeTmpDir("hooksconv-plugin");
try {
mkdirSync(join(root, "hooks"), { recursive: true });
@@ -720,8 +788,8 @@ test("loadPlugin treats a hooks/hooks.json convention plugin as plugin-shaped (n
);
const { files } = loadPlugin(root);
assert.ok(
!files[join(".claude", "skills", "dev", "SKILL.md")],
"a hooks-convention plugin's project-local .claude/skills is NOT materialized",
files[join(".claude", "skills", "dev", "SKILL.md")],
"a hooks-convention plugin's project-local .claude/skills IS materialized",
);
} finally {
cleanupTmpDir(root);
+111 -15
View File
@@ -5,7 +5,7 @@
*/
import { test } from "vitest";
import assert from "node:assert/strict";
import { writeFileSync, mkdirSync } from "node:fs";
import { writeFileSync, mkdirSync, readFileSync } from "node:fs";
import { join, resolve } from "node:path";
import { pathToFileURL } from "node:url";
@@ -80,7 +80,7 @@ test("discoverScripts passes an explicit file path through", () => {
}
});
test("runScripts reports per-file exit codes and forwards env", () => {
test("runScripts reports per-file exit codes and forwards env", async () => {
const dir = makeTmpDir("run-scripts");
try {
writeFileSync(join(dir, "ok.mjs"), "process.exit(0);\n");
@@ -90,7 +90,7 @@ test("runScripts reports per-file exit codes and forwards env", () => {
"process.exit(process.env.VIGILES_TRIALS === '7' ? 0 : 9);\n",
);
const results = runScripts(["ok.mjs", "bad.mjs", "env.mjs"], dir, {
const results = await runScripts(["ok.mjs", "bad.mjs", "env.mjs"], dir, {
VIGILES_TRIALS: "7",
});
assert.deepEqual(
@@ -176,7 +176,7 @@ test("interpreterArgs interposes an ENTRY, keeping the loader flags for the SCRI
);
});
test("runScripts passes the script to the entry as an argument", () => {
test("runScripts passes the script to the entry as an argument", async () => {
const dir = makeTmpDir("entry");
try {
writeFileSync(
@@ -184,7 +184,7 @@ test("runScripts passes the script to the entry as an argument", () => {
"console.log('ENTRY GOT ' + process.argv[2]);\n",
);
writeFileSync(join(dir, "x.eval.mjs"), "process.exit(3);\n"); // must NOT run
const [r] = runScripts(
const [r] = await runScripts(
["x.eval.mjs"],
dir,
{},
@@ -215,13 +215,13 @@ test("detectNodeCaps reports tsx presence from node_modules", () => {
}
});
test("runScripts surfaces an error code for an unrunnable TS script", () => {
test("runScripts surfaces an error code for an unrunnable TS script", async () => {
const dir = makeTmpDir("run-scripts");
try {
// A .ts file with no tsx and (on older node) no strip-types still yields a
// non-zero result rather than throwing out of runScripts.
writeFileSync(join(dir, "t.harness.ts"), "export {};\n");
const results = runScripts(["t.harness.ts"], dir);
const results = await runScripts(["t.harness.ts"], dir);
assert.equal(results.length, 1);
assert.equal(typeof results[0]?.code, "number");
} finally {
@@ -258,7 +258,7 @@ function countModuleUrl(): string {
return pathToFileURL(resolve(process.cwd(), "dist/check-count.js")).href;
}
test("runScripts reports 0 checks for a script that loads the API and runs nothing", () => {
test("runScripts reports 0 checks for a script that loads the API and runs nothing", async () => {
const dir = makeTmpDir("run-scripts-vacuous");
try {
const mod = JSON.stringify(countModuleUrl());
@@ -276,7 +276,7 @@ test("runScripts reports 0 checks for a script that loads the API and runs nothi
// The legacy shape: never touches vigiles, so it cannot report. Unchanged.
writeFileSync(join(dir, "legacy.harness.mjs"), "process.exit(0);\n");
const r = runScripts(
const r = await runScripts(
["vacuous.harness.mjs", "real.harness.mjs", "legacy.harness.mjs"],
dir,
);
@@ -295,7 +295,7 @@ test("runScripts reports 0 checks for a script that loads the API and runs nothi
}
});
test("a script's spawned CHILD does not inherit the report path", () => {
test("a script's spawned CHILD does not inherit the report path", async () => {
// A harness spawns processes for a living. A child that inherited the count
// path would write ITS count — usually zero — over the parent's, reporting a
// sub-process's activity as the file's. The variable is read once and dropped,
@@ -313,7 +313,7 @@ test("a script's spawned CHILD does not inherit the report path", () => {
`const r = spawnSync(process.execPath, ["-e", probe]);\n` +
`if (r.status !== 0) process.exit(9); // the child could see the path\n`,
);
const [r] = runScripts(["parent.harness.mjs"], dir);
const [r] = await runScripts(["parent.harness.mjs"], dir);
assert.equal(r?.code, 0, "the spawned child must not see the report path");
assert.equal(r?.checks, 4, "the parent's own count is what gets reported");
assert.equal(r?.status, "pass");
@@ -362,13 +362,13 @@ test("anyFailed: a skip never counts as a failure", () => {
assert.equal(anyFailed([{ file: "c.mjs", code: 2, status: "fail" }]), true);
});
test("runScripts classifies exit 77 as skip, 0 as pass, else fail", () => {
test("runScripts classifies exit 77 as skip, 0 as pass, else fail", async () => {
const dir = makeTmpDir("run-scripts");
try {
writeFileSync(join(dir, "ok.mjs"), "process.exit(0);\n");
writeFileSync(join(dir, "skip.mjs"), "process.exit(77);\n");
writeFileSync(join(dir, "bad.mjs"), "process.exit(1);\n");
const r = runScripts(["ok.mjs", "skip.mjs", "bad.mjs"], dir);
const r = await runScripts(["ok.mjs", "skip.mjs", "bad.mjs"], dir);
assert.deepEqual(
r.map((x) => x.status),
["pass", "skip", "fail"],
@@ -442,7 +442,7 @@ test("decideRunScripts: bare eval over many, at a TTY → CONFIRM", () => {
);
});
test("the runner reads back WHICH surfaces a script exercised", () => {
test("the runner reads back WHICH surfaces a script exercised", async () => {
// The channel's second job: coverage answers "tested?" from execution, and
// this is the wire it travels on. The fixture attributes through the tier
// (runHook derives the hook from the command), not by declaring anything.
@@ -467,7 +467,10 @@ test("the runner reads back WHICH surfaces a script exercised", () => {
join(dir, "plain.harness.mjs"),
`import { recordCheck } from ${mod};\nrecordCheck();\n`,
);
const r = runScripts(["attributes.harness.mjs", "plain.harness.mjs"], dir);
const r = await runScripts(
["attributes.harness.mjs", "plain.harness.mjs"],
dir,
);
assert.deepEqual(r[0].surfaces, [
{ how: "command", ref: "hooks/guard.sh" },
]);
@@ -478,3 +481,96 @@ test("the runner reads back WHICH surfaces a script exercised", () => {
cleanupTmpDir(dir);
}
});
// ── the pool: it must actually overlap, and only where overlap is safe ─────────
// Both halves, because either alone is worthless here. "It got faster" would not
// prove overlap (a machine hiccup does that), and "it produced the right results"
// would not prove it stayed SERIAL for `eval` — where overlap means simultaneous
// billed model calls. So each script records the number of peers running when it
// starts, and the assertion is on that number, not on a clock.
function poolFixture(dir: string, n: number): void {
for (let i = 0; i < n; i++) {
writeFileSync(
join(dir, `s${String(i)}.harness.mjs`),
[
`import { writeFileSync, readdirSync, mkdirSync, rmSync } from "node:fs";`,
`import { join } from "node:path";`,
`const live = join(process.cwd(), "live");`,
`mkdirSync(live, { recursive: true });`,
// 🔴 THE SLOT IS RELEASED ON EXIT, and that is the whole measurement. A
// first version only ever CREATED markers, so the count answered "how many
// have started so far" — which reaches n under perfectly serial execution
// too. It made the concurrency test pass for a reason unrelated to
// concurrency. Holding a slot only while running is what makes the number
// mean "peers running AT THE SAME MOMENT".
`writeFileSync(join(live, "${String(i)}"), "");`,
`const peers = readdirSync(live).length;`,
`const until = Date.now() + 150;`,
`while (Date.now() < until) {}`,
`rmSync(join(live, "${String(i)}"));`,
`writeFileSync(join(process.cwd(), "peers-${String(i)}"), String(peers));`,
].join("\n"),
);
}
}
const peakPeers = (dir: string, n: number): number =>
Math.max(
...Array.from({ length: n }, (_, i) =>
Number(readFileSync(join(dir, `peers-${String(i)}`), "utf8")),
),
);
test("test tier (no entry) runs scripts concurrently", async () => {
const dir = makeTmpDir("run-scripts-pool");
try {
poolFixture(dir, 4);
const files = ["s0", "s1", "s2", "s3"].map((s) => `${s}.harness.mjs`);
const results = await runScripts(files, dir, {}, { concurrency: 4 });
assert.equal(results.length, 4);
assert.ok(
results.every((r) => r.code === 0),
"every script must still succeed under the pool",
);
assert.ok(
peakPeers(dir, 4) > 1,
"at least one script must have observed a peer running — otherwise the pool is serial",
);
// Discovery order, not completion order: a run that reorders its own output
// between invocations reads as flaky even when every result is stable.
assert.deepEqual(
results.map((r) => r.file),
files,
"results must stay in discovery order",
);
} finally {
cleanupTmpDir(dir);
}
});
test("eval tier (entry set) stays strictly serial — overlap there is billed twice", async () => {
const dir = makeTmpDir("run-scripts-serial");
try {
poolFixture(dir, 3);
// An entry that simply runs the script it is handed, so the only difference
// from the tier above is the presence of `entry` itself.
writeFileSync(
join(dir, "entry.mjs"),
`await import(new URL(process.argv[2], "file://" + process.cwd() + "/").href);`,
);
const files = ["s0", "s1", "s2"].map((s) => `${s}.harness.mjs`);
const results = await runScripts(
files,
dir,
{},
{ entry: join(dir, "entry.mjs") },
);
assert.equal(results.length, 3);
assert.equal(
peakPeers(dir, 3),
1,
"no script may see a peer: `eval` spends real model quota, so the default must be 1",
);
} finally {
cleanupTmpDir(dir);
}
});
+85 -18
View File
@@ -10,7 +10,8 @@
* stripping (Node >= 22.6). The scripts import from the built `dist/`, so they
* also run standalone the CLI just discovers, runs, and aggregates exit codes.
*/
import { spawnSync } from "node:child_process";
import { spawn } from "node:child_process";
import { availableParallelism } from "node:os";
import { resolve, join } from "node:path";
import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
@@ -210,6 +211,11 @@ function readCheckReport(
/** Extra wiring for {@link runScripts}. */
export interface RunScriptsOptions {
/**
* How many scripts may run at once. Omitted decided from `entry`: `test`
* fans out across the cores, `eval` stays at 1. See {@link runScripts}.
*/
readonly concurrency?: number;
/**
* A program to run INSTEAD of each script, with the script's path as its one
* argument. `vigiles eval` passes `dist/eval-entry.js`; `vigiles test` passes
@@ -234,45 +240,106 @@ export interface RunScriptsOptions {
* record them (`.vigiles/coverage.json`) and coverage can answer "tested?" from
* execution rather than from a matching file name.
*/
export function runScripts(
export async function runScripts(
files: readonly string[],
cwd: string,
env: NodeJS.ProcessEnv = {},
opts: RunScriptsOptions = {},
): ScriptRunResult[] {
): Promise<ScriptRunResult[]> {
const caps = detectNodeCaps(cwd);
const results: ScriptRunResult[] = [];
const countDir = mkdtempSync(join(tmpdir(), "vigiles-checks-"));
try {
files.forEach((file, i) => {
// 🔴 THE DEFAULT IS DECIDED BY `entry`, NOT BY A FLAG, because the two commands
// that share this runner have OPPOSITE right answers and the caller already
// distinguishes them:
//
// `test` passes no entry — every script is a `*.harness.*` file, which is free
// and deterministic BY CONSTRUCTION (the harness tier drives the agent CLI
// against a mock model with no API key, and anything that spends money lives
// behind `vigiles/eval` with a `paid_` prefix). Nothing here can bill, so the
// only reason to serialize was that we always had.
//
// `eval` passes an entry — every script spends real model quota. Running those
// N-at-a-time multiplies spend and collides with rate limits, so it stays at 1.
//
// Measured motivation: 48 harness files in one consumer repo took 1m48s in CI as
// a strict queue, on a runner with cores sitting idle.
const parallel = Math.max(
1,
opts.concurrency ?? (opts.entry ? 1 : Math.min(8, availableParallelism())),
);
// Output is BUFFERED per child and printed when that child exits, rather than
// inherited. This is the real cost of concurrency and the reason it was not
// free: with `stdio: "inherit"` two children write to the same terminal at once
// and 48 reports shred into each other. Buffering keeps each report whole and
// attributable; what it gives up is live streaming, which only matters when one
// script is slow AND alone — i.e. exactly the `eval` case, where parallel is 1
// and the buffer is flushed as soon as the single child ends anyway.
const runOne = (file: string, i: number): Promise<ScriptRunResult> =>
new Promise((resolveRun) => {
let argv: string[];
try {
argv = interpreterArgs(file, caps, opts.entry);
} catch (e) {
console.error(`${file}: ${(e as Error).message}`);
results.push({ file, code: 1, status: "fail" });
resolveRun({ file, code: 1, status: "fail" });
return;
}
const countFile = join(countDir, `${String(i)}.count`);
const res = spawnSync("node", argv, {
const child = spawn("node", argv, {
cwd,
stdio: "inherit",
stdio: ["ignore", "pipe", "pipe"],
env: { ...process.env, ...env, [CHECK_COUNT_ENV]: countFile },
});
const code = res.status ?? 1;
const report = readCheckReport(countFile);
results.push({
file,
code,
status: statusFor(code, report?.checks),
checks: report?.checks,
...(report ? { surfaces: report.surfaces } : {}),
const chunks: Buffer[] = [];
child.stdout.on("data", (c: Buffer) => chunks.push(c));
child.stderr.on("data", (c: Buffer) => chunks.push(c));
const finish = (code: number): void => {
process.stdout.write(Buffer.concat(chunks));
const report = readCheckReport(countFile);
resolveRun({
file,
code,
status: statusFor(code, report?.checks),
checks: report?.checks,
...(report ? { surfaces: report.surfaces } : {}),
});
};
// `error` fires when the process could not be spawned at all; without this
// the promise would never settle and the whole run would hang silently —
// which is worse than any failure it could report.
child.on("error", (e) => {
chunks.push(Buffer.from(`${file}: ${e.message}\n`));
finish(1);
});
child.on("close", (code) => {
finish(code ?? 1);
});
});
try {
// Results are stored BY INDEX so the reported order is the discovery order,
// whatever order the children happen to finish in. A run whose output reorders
// itself between invocations reads as flaky even when every result is stable.
const results: ScriptRunResult[] = new Array<ScriptRunResult>(files.length);
let next = 0;
const worker = async (): Promise<void> => {
for (;;) {
const i = next++;
if (i >= files.length) return;
results[i] = await runOne(files[i], i);
}
};
await Promise.all(
Array.from({ length: Math.min(parallel, files.length) }, () => {
return worker();
}),
);
return results;
} finally {
rmSync(countDir, { recursive: true, force: true });
}
return results;
}
/**
@@ -52,14 +52,42 @@ test("vigiles plugin: loadPlugin materializes every shipped skill", () => {
// every skills/<name>/ directory that actually IS a skill (has a SKILL.md;
// skills/linter-docs/ is a reference-doc dir, not a skill) shows up
// materialized — no skill silently dropped by a bad manifest path.
const onDisk = readdirSync(join(ROOT, "skills"), { withFileTypes: true })
//
// This repo is itself the two-scope shape (shipped `skills/` + a large local
// `.claude/skills/`), which the loader used to read as ONE. So the assertion is
// per-scope: every shipped skill under its own key, every dev skill under
// `.claude/`, and NEITHER count standing in for the other.
const shippedNames = readdirSync(join(ROOT, "skills"), {
withFileTypes: true,
})
.filter((d) => d.isDirectory())
.filter((d) => existsSync(join(ROOT, "skills", d.name, "SKILL.md"))).length;
assert.ok(onDisk > 0, "repo ships at least one skill");
.filter((d) => existsSync(join(ROOT, "skills", d.name, "SKILL.md")))
.map((d) => d.name);
assert.ok(shippedNames.length > 0, "repo ships at least one skill");
for (const name of shippedNames) {
assert.ok(
loaded.files[`skills/${name}/SKILL.md`] !== undefined,
`shipped skill ${name} materializes under its own repo-relative key`,
);
}
const devNames = existsSync(join(ROOT, ".claude", "skills"))
? readdirSync(join(ROOT, ".claude", "skills"), { withFileTypes: true })
.filter((d) => d.isDirectory())
.filter((d) =>
existsSync(join(ROOT, ".claude", "skills", d.name, "SKILL.md")),
)
.map((d) => d.name)
: [];
for (const name of devNames) {
assert.ok(
loaded.files[join(".claude", "skills", name, "SKILL.md")] !== undefined,
`project skill ${name} is materialized, not shadowed by the shipped scope`,
);
}
assert.equal(
skills.length,
onDisk,
`expected all ${String(onDisk)} skills to materialize, got ${String(skills.length)}`,
shippedNames.length + devNames.length,
`expected ${String(shippedNames.length)} shipped + ${String(devNames.length)} project skills, got ${String(skills.length)}`,
);
// ${CLAUDE_PLUGIN_ROOT} must be expanded in the loaded settings (a skill/hook
+18 -2
View File
@@ -1755,7 +1755,23 @@ async function runLint(
// `<!-- vigiles:ignore -->` (single block) or
// `<!-- vigiles:ignore-file -->` (whole file). Same engine as spec.ts.
if (!silent) console.log("\nMarkdown code block refs:\n");
const docRefReport = findDocRefs({ basePath: process.cwd() });
// 🔴 `config.exclude` REACHES THIS PASS. It did not, and that single omission is
// why `lint` could not exit 0 on a repository that vendors other people's
// markdown: this walker globs `**/*.md` from the repo root, so a third-party
// `CLAUDE.md` captured verbatim as benchmark data was held to the same ref
// validation as the repo's own docs — and the one config field documented to
// stop exactly that ("vendored or benchmark fixtures the repo's own lint
// shouldn't police") was never handed over.
//
// Measured on a consumer repo 2026-08-19: 9 broken refs, 8 of them inside a
// directory the user had explicitly listed in `exclude`, all 9 still reported.
// With no way to reach 0 the step was made `continue-on-error: true`, and a lint
// whose exit code is discarded gates nothing — after which hand-written CI steps
// grew to do the gating instead. One unpassed argument, that whole chain.
const docRefReport = findDocRefs({
basePath: process.cwd(),
ignore: config?.exclude,
});
if (!silent) {
for (const line of formatDocRefReport(docRefReport).split("\n")) {
console.log(` ${line}`);
@@ -5673,7 +5689,7 @@ async function handleRunScripts(
// An eval file DESCRIBES its eval; `dist/eval-entry.js` is what imports the
// description and runs what it declares. A harness script is still its own
// program (it is free, so "import spends money" never applied to it).
const results = runScripts(files, cwd, env, {
const results = await runScripts(files, cwd, env, {
...(kind === "eval" ? { entry: resolve(__dirname, "eval-entry.js") } : {}),
});
// Write down WHAT the run exercised, so `lint`/`audit` can answer "tested?"
+46
View File
@@ -178,3 +178,49 @@ test("a disallowed-tools entry that is a typo of a real tool is an ERROR, not a
`expected a typo report naming both the typo and the real tool, got ${JSON.stringify(errors)}`,
);
});
// ── frontmatter scalars must survive YAML, and only be quoted when they must ──
// Both halves matter and for different reasons. If quoting never fires, `compile`
// keeps emitting frontmatter that `frontmatter-valid` then reports — the blessed
// path producing the defect the product hunts for. If quoting fires when it is not
// needed, every already-compiled file changes bytes and every integrity hash moves,
// which reads to users as "vigiles rewrote my whole repo".
test("compile quotes a description YAML would otherwise mis-read", () => {
// The real shape that broke two shipped skills: a colon-space inside prose.
const withColon =
"Узнать, сколько берут за услугу — ловит ошибки, где замер врёт: неаналоги в выборке.";
const { markdown } = compileSkill({
name: "benchmark-price",
description: withColon,
tools: ["Read", "WebSearch"],
body: "Body.",
} as never);
const fm = readFrontmatter(markdown);
assert.equal(
fm.malformed,
false,
"compiled frontmatter must be valid YAML — it is the input to every other check",
);
assert.equal(
frontmatterScalar(fm, "description"),
withColon,
"and the value must round-trip unchanged, not merely parse",
);
// The tool contract is what silently vanishes when the block is malformed: the
// file still LOOKS declarative while a strict parser reads nothing from it.
assert.match(markdown, /allowed-tools: \[Read, WebSearch\]/);
});
test("compile leaves an already-safe description bare — no hash churn", () => {
const plain = "Search products on ozon.kz and return live listings.";
const { markdown } = compileSkill({
name: "ozon-search-kz",
description: plain,
body: "Body.",
} as never);
assert.ok(
markdown.includes(`description: ${plain}`),
"a safe scalar must stay unquoted, or every compiled file in every repo churns and every integrity hash moves",
);
});
+51 -5
View File
@@ -7,6 +7,7 @@
import { existsSync, readFileSync, statSync } from "node:fs";
import { globSync } from "glob";
import yaml from "js-yaml";
import { resolve, dirname, basename } from "node:path";
import { sha256short, assertNever } from "./hash.js";
@@ -808,6 +809,48 @@ function collectSkillRefs(spec: SkillSpec): InstructionFragment[] {
* argument-hint, tools). Default is `"claude-code"` so callers that pass no
* dialect get byte-identical output to before.
*/
/**
* A frontmatter scalar, quoted only when leaving it bare would not survive YAML.
*
* 🔴 WHY THIS EXISTS. `description: ${spec.description}` interpolated the value
* raw, so a description containing a colon-space `"…измеряет, где замер врёт:
* неаналоги в выборке"` — emitted frontmatter that is not valid YAML. Measured
* 2026-08-19 on two real skills: right after `vigiles compile`, `skillContract()`
* reported `malformed: true` and `declared: []`. The file LOOKS like it declares
* `allowed-tools`; a strict parser reads nothing, so the skill silently inherits
* every tool the session grants.
*
* That is the exact defect `frontmatter-valid` exists to report i.e. the
* blessed path produced the thing the product hunts for. Worse, it PUNISHED the
* fix: a human who quoted the value by hand got a hash mismatch on the next lint
* ("manually edited after compilation"), and recompiling silently reverted them.
*
* The test is a ROUND TRIP rather than a list of dangerous characters: emit it,
* read it back with the same loader the linter uses, and quote only if what comes
* back is not the string that went in. Consequences of that choice:
* - a value that was already safe is emitted byte-identically, so no existing
* compiled file churns and no integrity hash moves;
* - the set of "dangerous" inputs never has to be enumerated or maintained
* YAML itself decides, so `#`, `[`, `&`, `*`, leading/trailing space, `yes`,
* `null` and everything else are covered without being listed.
*/
function yamlScalar(value: string): string {
try {
const parsed = yaml.load(`v: ${value}`);
if (
parsed !== null &&
typeof parsed === "object" &&
(parsed as Record<string, unknown>).v === value
) {
return value;
}
} catch {
// Did not parse at all — definitively needs quoting.
}
// A JSON string IS a YAML double-quoted scalar, escaping included.
return JSON.stringify(value);
}
function renderSkillFrontmatter(
spec: SkillSpec,
profile: SkillFrontmatterProfile = "claude-code",
@@ -815,8 +858,8 @@ function renderSkillFrontmatter(
const fm = [
"---",
"",
`name: ${spec.name}`,
`description: ${spec.description}`,
`name: ${yamlScalar(spec.name)}`,
`description: ${yamlScalar(spec.description)}`,
];
// The CC-only keys below are inert in a minimal (Codex/OpenCode) SKILL.md, so
// they're omitted entirely under that profile.
@@ -831,7 +874,7 @@ function renderSkillFrontmatter(
spec.inputs && spec.inputs.length > 0
? renderArgumentHint(spec.inputs)
: spec.argumentHint;
if (argHint) fm.push(`argument-hint: ${argHint}`);
if (argHint) fm.push(`argument-hint: ${yamlScalar(argHint)}`);
if (spec.tools && spec.tools.length > 0) {
// A Claude Code SKILL declares its tool contract under `allowed-tools`
// (NOT `tools:` — that's the SUBAGENT key), as a real YAML sequence. Flow
@@ -1073,11 +1116,14 @@ function validateAgentTools(
/** Build the subagent YAML frontmatter (name / description / model / tools). */
function renderAgentFrontmatter(spec: AgentSpec): string {
// Same round-trip quoting as the skill renderer — a subagent description is
// written just as freely and breaks its frontmatter the same way. See
// {@link yamlScalar}.
const fm = [
"---",
"",
`name: ${spec.name}`,
`description: ${spec.description}`,
`name: ${yamlScalar(spec.name)}`,
`description: ${yamlScalar(spec.description)}`,
];
if (spec.model !== undefined) fm.push(`model: ${spec.model}`);
if (spec.color !== undefined) fm.push(`color: ${spec.color}`);
+20 -3
View File
@@ -19,10 +19,22 @@ import { ncd } from "./ncd.js";
export interface DescribedSurface {
readonly name: string;
readonly description: string;
/**
* Repo-relative path to report the surface at. Optional for callers that have
* no path, but REQUIRED in practice to keep the message actionable: since the
* loader started reading BOTH discovery levels, a repo that keeps a copy of a
* skill in `skills/` and `.claude/skills/` yields two surfaces with the SAME
* name, and "skills \"dup\" and \"dup\" have near-identical descriptions" names
* neither file. Measured on `nyldn/claude-octopus`: 50 such pairs. The path is
* the only thing that distinguishes them.
*/
readonly where?: string;
}
export interface DescriptionOverlap {
/** Display label for the first skill — `"name"`, or `"name" (path)`. */
readonly a: string;
/** Display label for the second skill — `"name"`, or `"name" (path)`. */
readonly b: string;
/** 01, higher = more alike (1 NCD), rounded to 2 dp. */
readonly similarity: number;
@@ -43,6 +55,11 @@ export const OVERLAP_NCD_CUTOFF = 0.2;
* first. Pure; pass only the surfaces that actually compete for auto-selection
* (model-invocable, described) so a user-invoked pair isn't a false alarm.
*/
/** `"name"`, or `"name" (path)` when the caller supplied one. */
function label(s: DescribedSurface): string {
return s.where === undefined ? `"${s.name}"` : `"${s.name}" (${s.where})`;
}
export function findDescriptionOverlaps(
surfaces: readonly DescribedSurface[],
cutoff: number = OVERLAP_NCD_CUTOFF,
@@ -52,13 +69,13 @@ export function findDescriptionOverlaps(
for (let j = i + 1; j < surfaces.length; j++) {
const d = ncd(surfaces[i].description, surfaces[j].description);
if (d >= cutoff) continue;
const a = surfaces[i].name;
const b = surfaces[j].name;
const a = label(surfaces[i]);
const b = label(surfaces[j]);
overlaps.push({
a,
b,
similarity: Math.round((1 - d) * 100) / 100,
message: `skills "${a}" and "${b}" have near-identical descriptions (${String(Math.round((1 - d) * 100))}% alike) — the model can't reliably tell them apart, so the wrong one may fire. Differentiate their descriptions.`,
message: `skills ${a} and ${b} have near-identical descriptions (${String(Math.round((1 - d) * 100))}% alike) — the model can't reliably tell them apart, so the wrong one may fire. Differentiate their descriptions.`,
});
}
}
+73 -11
View File
@@ -19,6 +19,7 @@
*/
import { existsSync, readFileSync } from "node:fs";
import ts from "typescript";
import { resolve } from "node:path";
import { globSync } from "glob";
@@ -80,7 +81,7 @@ const TS_LANGS = new Set(["ts", "typescript", "js", "javascript"]);
const IGNORE_BLOCK_RE = /^\s{0,3}<!--\s*vigiles:ignore\s*-->\s*$/;
const IGNORE_FILE_RE = /^\s{0,3}<!--\s*vigiles:ignore-file\s*-->\s*$/m;
const CALL_RE = /\b(enforce|file|cmd|ref)\(\s*["']([^"'\n]+)["']/g;
const KINDS = new Set(["enforce", "file", "cmd", "ref"]);
const PLACEHOLDER_RE = /[<>]/;
@@ -105,6 +106,76 @@ interface ExtractResult {
blocksIgnored: number;
}
/**
* The builder calls in one fenced block, found by PARSING it rather than by
* matching text.
*
* 🔴 WHY THIS IS NOT A REGEX ANY MORE. The previous form was
* `/\b(enforce|file|cmd|ref)\(\s*["']([^"'\n]+)["']/g`, and four of these six
* inputs were classified wrongly (measured 2026-08-19):
*
* ctx.file("OUT") matched, though it is a method on some other
* object. A live instance of this sat in a real
* repository's notes and was reported as a broken
* ref for weeks.
* obj.cmd("npm test") matched, same reason
* // cmd("npm test") → matched, though it is a comment
* const s = 'cmd("x")' matched, though it is a string
* myFile("x") correctly skipped
* cmd("npm test") correctly matched
*
* `\b` sits happily after a `.`, so every member call read as a builder call,
* and a regex has no notion of comments or string literals at all. Parsing makes
* all four INEXPRESSIBLE rather than individually patched: the AST only offers a
* call whose callee is a bare identifier, and comments and string bodies are not
* call expressions in the first place.
*
* `typescript` is already a runtime dependency of this package, so this costs no
* new install the parser was in the box the whole time.
*/
function callsIn(
blockLines: readonly { lineNo: number; text: string }[],
file: string,
): DocRef[] {
if (blockLines.length === 0) return [];
const src = blockLines.map((b) => b.text).join("\n");
const firstLine = blockLines[0].lineNo;
const sf = ts.createSourceFile(
"block.ts",
src,
ts.ScriptTarget.Latest,
/* setParentNodes */ true,
ts.ScriptKind.TS,
);
const out: DocRef[] = [];
const visit = (node: ts.Node): void => {
if (ts.isCallExpression(node) && ts.isIdentifier(node.expression)) {
const kind = node.expression.text;
if (KINDS.has(kind)) {
const arg = node.arguments[0];
// Only a plain string literal is a ref we can resolve. A template with
// substitutions, a variable, or a computed value is not something this
// pass can check, and guessing at it is how false reports start.
if (
arg &&
(ts.isStringLiteral(arg) || ts.isNoSubstitutionTemplateLiteral(arg))
) {
const { line } = sf.getLineAndCharacterOfPosition(node.getStart(sf));
out.push({
file,
line: firstLine + line,
kind: kind as DocRefKind,
value: arg.text,
});
}
}
}
ts.forEachChild(node, visit);
};
visit(sf);
return out;
}
/** @internal */ export function extractDocRefs(
content: string,
file: string,
@@ -141,16 +212,7 @@ interface ExtractResult {
if (nextBlockIgnored) {
blocksIgnored++;
} else {
for (const { lineNo, text } of blockLines) {
for (const m of text.matchAll(CALL_RE)) {
refs.push({
file,
line: lineNo,
kind: m[1] as DocRefKind,
value: m[2],
});
}
}
refs.push(...callsIn(blockLines, file));
}
}
fenceChar = null;
+181
View File
@@ -0,0 +1,181 @@
/**
* Unit tests for the surface SCOPING decision the module that replaced "read
* the repo-root surfaces OR the `.claude/` ones, never both".
*
* Both halves, every time: a test that fires on the planted defect AND a test
* that stays silent on the shapes that were already right. A test that only
* checks the new case would pass just as well against code that read `.claude/`
* and dropped the root the mirror-image of the bug.
*/
import { test } from "vitest";
import assert from "node:assert/strict";
import { claudeCodeLayout } from "../adapters/claude-code/layout.js";
import { codexLayout } from "../adapters/codex/layout.js";
import { opencodeLayout } from "../adapters/opencode/layout.js";
import type { PluginLayout } from "./layout.js";
import {
assertDistinctScopeKeys,
multiScopeWarning,
scopeKey,
surfaceSource,
type SurfaceScope,
} from "./surface-scopes.js";
const probe = {
hasRootSkillFile: false,
skillName: "repo",
rootHasLoadable: false,
isPluginShaped: false,
userHasLoadable: false,
};
function scopesOf(p: Partial<typeof probe>, layout = claudeCodeLayout) {
const s = surfaceSource(layout, { ...probe, ...p });
assert.equal(s.kind, "scopes", "expected the multi-scope shape");
return s.kind === "scopes" ? s.scopes : [];
}
// --- THE DEFECT: both levels present, one was dropped ------------------------
test("both levels present → BOTH scopes are read", () => {
const scopes = scopesOf({ rootHasLoadable: true, userHasLoadable: true });
assert.deepEqual(
scopes.map((s) => s.base),
[".claude", ""],
"project scope first, plugin scope second",
);
});
test("both levels present → the two scopes mint DIFFERENT keys", () => {
const scopes = scopesOf({ rootHasLoadable: true, userHasLoadable: true });
const keys = scopes.map((s) => scopeKey(s, "skills", "dup/SKILL.md"));
assert.deepEqual(keys, [
".claude/skills/dup/SKILL.md",
"skills/dup/SKILL.md",
]);
assert.equal(new Set(keys).size, keys.length, "no key is claimed twice");
});
test("a plugin-shaped repo with NO root surfaces still reads .claude/", () => {
// The hook-only plugin: manifest + hooks, no `skills/`, but a real
// `.claude/skills` the session loads. It used to be dropped as "dev-only".
const scopes = scopesOf({ isPluginShaped: true, userHasLoadable: true });
assert.deepEqual(scopes.map((s) => s.base).sort(), ["", ".claude"]);
});
// --- THE OTHER HALF: shapes that were already right must not move ------------
test("plugin-only repo is unchanged — root keeps the canonical key", () => {
const scopes = scopesOf({ rootHasLoadable: true });
assert.deepEqual(scopes, [
{ base: "", materializeUnder: ".claude", label: "plugin" },
]);
assert.equal(
scopeKey(scopes[0], "skills", "x/SKILL.md"),
".claude/skills/x/SKILL.md",
);
});
test("plain-user repo is unchanged — .claude keeps the canonical key", () => {
const scopes = scopesOf({ userHasLoadable: true });
assert.deepEqual(scopes, [
{ base: ".claude", materializeUnder: ".claude", label: "project" },
]);
});
test("an empty repo still points at the project scope, not nothing", () => {
// A repo with no surfaces anywhere must still LOOK at `.claude/` — that
// fallback is why `userSurfaceRoot` exists.
assert.deepEqual(
scopesOf({}).map((s) => s.base),
[".claude"],
);
});
test("a root SKILL.md still wins outright (single-skill target)", () => {
const s = surfaceSource(claudeCodeLayout, {
...probe,
hasRootSkillFile: true,
skillName: "solo",
userHasLoadable: true,
});
assert.deepEqual(s, { kind: "single-skill", skillName: "solo" });
});
test("a layout without a user surface root yields at most the root scope", () => {
// Codex/OpenCode declare no `userSurfaceRoot`; they must not grow a phantom
// second scope, and an empty repo must not synthesize one either.
for (const layout of [codexLayout, opencodeLayout]) {
assert.equal(layout.userSurfaceRoot, undefined, layout.name);
assert.deepEqual(scopesOf({ rootHasLoadable: true }, layout), [
{ base: "", materializeUnder: layout.materializeRoot, label: "plugin" },
]);
assert.deepEqual(scopesOf({}, layout), []);
}
});
// --- The loud backstop -------------------------------------------------------
test("assertDistinctScopeKeys is silent on every scope set the shipped layouts produce", () => {
for (const layout of [claudeCodeLayout, codexLayout, opencodeLayout]) {
for (const p of [
{ rootHasLoadable: true },
{ userHasLoadable: true },
{ rootHasLoadable: true, userHasLoadable: true },
{},
]) {
assertDistinctScopeKeys(scopesOf(p, layout), layout.name);
}
}
});
test("assertDistinctScopeKeys THROWS when two scopes would share a prefix", () => {
// The planted defect: a future layout whose second scope relocates onto the
// first one's prefix — exactly the silent overwrite this module removed.
const colliding: SurfaceScope[] = [
{ base: ".claude", materializeUnder: ".claude", label: "project" },
{ base: "", materializeUnder: ".claude", label: "plugin" },
];
assert.throws(() => {
assertDistinctScopeKeys(colliding, "hypothetical");
}, /silently shadow/);
});
test("a layout naming its materializeRoot as a SECOND scope base is caught", () => {
// Constructed against the real decision function, not a hand-built list: a
// layout whose `materializeRoot` is empty makes both scopes mint "" prefixes.
const bad: PluginLayout = { ...claudeCodeLayout, materializeRoot: "" };
const scopes = scopesOf(
{ rootHasLoadable: true, userHasLoadable: true },
bad,
);
assert.throws(() => {
assertDistinctScopeKeys(scopes, bad.name);
}, /silently shadow/);
});
// --- The warning -------------------------------------------------------------
test("multiScopeWarning fires for two scopes and is silent for one or zero", () => {
const two = scopesOf({ rootHasLoadable: true, userHasLoadable: true });
const w = multiScopeWarning(two, { skills: 4 });
assert.ok(w?.includes("TWO discovery levels"), "names the situation");
assert.ok(w?.includes("4 file(s)"), "counts what was read");
assert.equal(
multiScopeWarning(scopesOf({ rootHasLoadable: true }), {}),
undefined,
);
assert.equal(multiScopeWarning([], {}), undefined);
});
test("scopeKey drops empty segments instead of emitting a leading slash", () => {
assert.equal(
scopeKey(
{ base: "", materializeUnder: "", label: "plugin" },
"skills",
"x/SKILL.md",
),
"skills/x/SKILL.md",
);
});
+185
View File
@@ -0,0 +1,185 @@
/**
* WHERE a repo's model surfaces (skills/agents/commands) live and the key each
* one is materialized under.
*
* 🔴 THIS EXISTS BECAUSE THE LOADER USED TO CHOOSE. It read the repo-root
* `skills/` **or** the project-level `.claude/skills/` never both and
* materialized whichever it picked under the SAME canonical
* `<materializeRoot>/<surface>/…` key. Two real files, one key: the loser was
* never read, and the winner's content sat under the loser's name. Measured
* 2026-08-18 on `nyldn/claude-octopus` (pinned corpus): **50 skill names exist in
* both `skills/` and `.claude/skills/`, and all 50 pairs differ** the `.claude/`
* copies carry multi-line unquoted `description:` blocks that a strict YAML
* loader rejects. vigiles reported those fifty skills as clean without ever
* having opened the files it named.
*
* The vendor settles it Claude Code loads BOTH, in two namespaces:
*
* > Plugin skills use a `plugin-name:skill-name` namespace, so they can't
* > conflict with other levels.
* > For example, `my-plugin/skills/deploy/SKILL.md` becomes `/my-plugin:deploy`
* > and loads alongside a `deploy` skill in your project's `.claude/skills/`.
* > https://code.claude.com/docs/en/skills § "Where skills live"
*
* So "pick one" was never a tie-break to get right; it was a question that has no
* answer, asked because the key shape forced one. The fix removes the question:
* every scope present is read, and **a scope's key prefix is derived from the
* scope, not from a winner**, so two files can no longer claim one key.
*
* Node-free and IO-free on purpose: the disk loader (`src/plugin-loader.ts`) and
* the browser file-map twin (`src/scan-files.ts`) each probe their own storage
* and call THIS for the decision, so the pair that this repo has repeatedly been
* bitten by fixing on one side only cannot disagree about scoping.
*/
import type { PluginLayout } from "./layout.js";
/**
* One discovery level, and the prefix its files are materialized under.
*
* `base` is the repo-relative dir the surfaces really live under (`""` = the repo
* root, the published-plugin shape; `.claude` = the plain-user project shape).
* `materializeUnder` is the prefix prepended to `<surface>/<rel>` to form the
* `LoadedPlugin.files` key.
*/
export interface SurfaceScope {
/** Repo-relative dir holding `<surface>/…`; `""` for the repo root. */
readonly base: string;
/** Key prefix for this scope's files; `""` for none. */
readonly materializeUnder: string;
/** Human label for warnings — `plugin` (root) or `project` (`.claude/`). */
readonly label: string;
}
/** Which shape the audited target is, and every scope to read from it. */
export type SurfaceSource =
| { readonly kind: "single-skill"; readonly skillName: string }
| { readonly kind: "scopes"; readonly scopes: readonly SurfaceScope[] };
/** What the caller must probe on its own storage for {@link surfaceSource}. */
export interface SurfaceProbe {
/** A `<root>/SKILL.md` exists — the target IS one skill dir. */
readonly hasRootSkillFile: boolean;
/** Name to give that single skill (the target dir's basename). */
readonly skillName: string;
/** Some `<root>/<surface>/` holds a loadable file. */
readonly rootHasLoadable: boolean;
/** A plugin manifest or the hooks convention path exists. */
readonly isPluginShaped: boolean;
/** Some `<root>/<userSurfaceRoot>/<surface>/` holds a loadable file. */
readonly userHasLoadable: boolean;
}
/**
* Classify the target and list every scope to read, HIGHEST-PRECEDENCE FIRST.
*
* Precedence decides only one thing: which scope keeps the canonical
* `<materializeRoot>/…` key. The project scope takes it, because that key IS
* where a project skill lives `.claude/skills/deploy/SKILL.md` is loaded from
* exactly that path and answers to `/deploy`. A plugin scope keeps its own real
* location (`skills/deploy/SKILL.md`), which is likewise where the harness reads
* it from, under `/plugin:deploy`. Nothing is relocated on top of something else.
*
* 🔴 THE COLLISION IS STRUCTURAL, NOT CHECKED. Only the FIRST scope is relocated;
* every later one keeps `base` as its prefix. Since the first scope is the only
* one that can produce a `<materializeRoot>/…` key, and every other prefix is a
* distinct real directory, two scopes cannot mint the same key there is no
* ordering, no "if already taken", and no last-write-wins to get wrong.
* {@link assertDistinctScopeKeys} is the LOUD backstop for a future
* `PluginLayout` that breaks the premise (e.g. one naming `.claude` as BOTH its
* `materializeRoot` and a second scope's base).
*/
export function surfaceSource(
layout: PluginLayout,
probe: SurfaceProbe,
): SurfaceSource {
if (layout.skillDir && probe.hasRootSkillFile) {
return { kind: "single-skill", skillName: probe.skillName };
}
const scopes: SurfaceScope[] = [];
if (layout.userSurfaceRoot !== undefined && probe.userHasLoadable) {
scopes.push({
base: layout.userSurfaceRoot,
materializeUnder: layout.materializeRoot,
label: "project",
});
}
if (probe.rootHasLoadable || probe.isPluginShaped) {
scopes.push({
base: "",
materializeUnder: scopes.length === 0 ? layout.materializeRoot : "",
label: "plugin",
});
}
// A layout with no user root and nothing at the root still has to read the
// project shape, or a plain repo would load as an empty machine — the reason
// `userSurfaceRoot` exists. An empty list means genuinely nothing loadable.
if (scopes.length === 0 && layout.userSurfaceRoot !== undefined) {
scopes.push({
base: layout.userSurfaceRoot,
materializeUnder: layout.materializeRoot,
label: "project",
});
}
return { kind: "scopes", scopes };
}
/** The `LoadedPlugin.files` key for one file under one scope. */
export function scopeKey(
scope: SurfaceScope,
surface: string,
rel: string,
): string {
return [scope.materializeUnder, surface, rel]
.filter((s) => s !== "")
.join("/");
}
/**
* Throw when two scopes would mint the same key prefix. Unreachable for every
* shipped layout (see {@link surfaceSource}) it exists so a NEW layout that
* breaks the premise fails loudly at load, rather than silently dropping a
* surface file the way the shadowing bug did for a year.
*/
export function assertDistinctScopeKeys(
scopes: readonly SurfaceScope[],
layoutName: string,
): void {
const seen = new Map<string, string>();
for (const s of scopes) {
const prev = seen.get(s.materializeUnder);
if (prev !== undefined) {
throw new Error(
`layout "${layoutName}": surface scopes "${prev}" and "${s.base}" both materialize under ` +
`"${s.materializeUnder || "<repo root>"}" — one would silently shadow the other. ` +
`Give each scope a distinct materialize prefix (see src/core/surface-scopes.ts).`,
);
}
seen.set(s.materializeUnder, s.base);
}
}
/**
* The warning to emit when more than one scope is present. Both scopes load in a
* real session under DIFFERENT names, but the deterministic sandbox is a project
* dir it registers the project scope only, so a plugin-scope skill sitting at
* `skills/…` in the fixture never activates there (the footgun
* `unregisteredSkillFiles` already warns about for inline arm files). Say so,
* rather than quietly relocating one on top of the other.
*/
export function multiScopeWarning(
scopes: readonly SurfaceScope[],
counts: Record<string, number>,
): string | undefined {
if (scopes.length < 2) return undefined;
const total = Object.values(counts).reduce((a, b) => a + b, 0);
return (
`repo carries surfaces at TWO discovery levels (${scopes
.map((s) => `${s.label}${s.base === "" ? "<repo root>" : s.base}/`)
.join(
", ",
)}); ${String(total)} file(s) were read from both. Claude Code loads both ` +
`a plugin skill as \`/<plugin>:<name>\`, a project skill as \`/<name>\` — so a name in both ` +
`places is TWO surfaces, not one. The deterministic sandbox is a project dir and registers ` +
`the project scope only; install the plugin scope with \`pluginDir\` to exercise it.`
);
}
+91 -95
View File
@@ -41,6 +41,23 @@ import {
startsAtSeparator,
stripFullLineComments,
} from "./core/source-refs.js";
import {
assertDistinctScopeKeys,
multiScopeWarning,
scopeKey,
surfaceSource,
type SurfaceScope,
} from "./core/surface-scopes.js";
/**
* What one materialization pass produced: per-surface file counts, and the
* discovery scopes it actually read from. They travel together because the
* warnings need both how much was read, and from how many levels.
*/
interface MaterializedSurfaces {
readonly counts: Record<string, number>;
readonly scopes: readonly SurfaceScope[];
}
export interface LoadedPlugin {
/** A `.claude/settings.json`-shaped object with hooks resolved. */
@@ -214,53 +231,38 @@ export function loadPlugin(
files[layout.instructionFile] = readFileSync(instructions, "utf-8");
sources[layout.instructionFile] = instructions;
}
const counts = materializeSurfaces(root, layout, files, sources);
const surfaces = materializeSurfaces(root, layout, files, sources);
return {
settings: resolvedHooks ? { hooks: resolvedHooks } : {},
files,
sources,
warnings: pluginWarnings(root, counts, resolvedHooks, files, layout),
warnings: pluginWarnings(root, surfaces, resolvedHooks, files, layout),
};
}
/**
* Materialize every model surface (skills/agents/commands) into `files`, keyed by
* a canonical `<materializeRoot>/<surface>/…` path, and record each file's real
* on-disk path in `sources`. Best-effort (headless activation of plugin
* skills/subagents/commands is not guaranteed; the body is present to read).
* Materialize every model surface (skills/agents/commands) into `files`, and
* record each file's real on-disk path in `sources`. Best-effort (headless
* activation of plugin skills/subagents/commands is not guaranteed; the body is
* present to read).
*
* Each surface is read from ONE of the two locations the layout knows about,
* primary-first: `<root>/<surface>` (the published-plugin / skills-library shape)
* when it exists, ELSE when the layout declares one the project-level
* `<root>/<userSurfaceRoot>/<surface>` (the shape a PLAIN Claude Code user has,
* not a plugin author). Preferring the primary means a plugin author's own local
* `.claude/skills` dev skills don't pollute the audit of what their plugin
* actually ships, while a plain user (no repo-root `skills/`) is still read. Plus
* the single-skill-directory case (`<root>/SKILL.md`), so pointing at one skill
* dir works. Whichever location is used normalizes to the same canonical key, so
* the classifier and every downstream detector see one shape regardless of where
* it lives on disk. Returns the per-surface counts (drives the surface warnings).
* EVERY discovery level present is read the repo-root `<surface>` (the
* published-plugin / skills-library shape) AND the project-level
* `<userSurfaceRoot>/<surface>` (the shape a plain Claude Code user has). The
* loader used to read one OR the other and materialize the winner under the
* loser's canonical key; `src/core/surface-scopes.ts` carries the measurement
* that killed that, and the vendor quote that settles which one the harness
* loads (both, in different namespaces). The KEY now comes from the scope, so
* two files can no longer claim one. Plus the single-skill-directory case
* (`<root>/SKILL.md`), so pointing at one skill dir works. Returns the
* per-surface counts (drives the surface warnings).
*/
/**
* WHERE a repo's model surfaces (skills/agents/commands) live PARSED from the
* repo's shape ONCE (parse-don't-validate) so materialization never re-infers it
* from a pile of ad-hoc booleans (the tangle that spawned repeated edge-case
* bugs: empty dir, stray file, hook-only plugin, single-skill target). A tagged
* union, one variant per real shape; a NEW shape is a new variant the exhaustive
* `switch` below won't compile without handling the whole point.
*/
type SurfaceSource =
| { readonly kind: "single-skill"; readonly skillName: string } // `<root>/SKILL.md` — the target IS one skill
| { readonly kind: "root" } // plugin / library / any root-surface content — read `<root>/<surface>`
| { readonly kind: "user"; readonly sub: string } // plain user repo — read `<root>/<sub>/<surface>`
| { readonly kind: "none" }; // nothing loadable anywhere
/**
* A surface holds a LOADABLE file a `<name>/SKILL.md` for skills, a `.md` for
* agents/commands. A stray non-surface file (`skills/README.md`, `.gitkeep`) does
* NOT count, else it would mark the root populated and shadow a plain user's real
* `.claude/skills`.
* NOT count, else an empty-but-present dir would mark a scope populated.
*/
function surfaceHasLoadable(
layout: PluginLayout,
@@ -273,43 +275,12 @@ function surfaceHasLoadable(
: keys.some((k) => k.endsWith(".md"));
}
/**
* Classify the repo shape from disk, with EXPLICIT precedence:
* 1. a `<root>/SKILL.md` the target IS one skill dir (single-skill).
* 2. any root-surface with LOADABLE content, OR a plugin manifest / hooks
* convention read the ROOT surfaces. A plugin ships from its manifest even
* with no root surface dirs, so its dev `.claude/…` is never a fallback.
* 3. else, if the layout declares a user-surface root a plain user repo.
* 4. else nothing loadable.
* Pure over the pre-read `rootTrees` + a few existence checks one place to test.
*/
function classifySurfaceSource(
root: string,
layout: PluginLayout,
rootTrees: ReadonlyMap<string, Record<string, string>>,
): SurfaceSource {
if (layout.skillDir && existsSync(join(root, "SKILL.md"))) {
return { kind: "single-skill", skillName: basename(root) };
}
const rootHasLoadable = layout.surfaceDirs.some((s) =>
surfaceHasLoadable(layout, s, rootTrees.get(s) ?? {}),
);
const isPluginShaped =
existsSync(join(root, layout.manifestPath)) ||
existsSync(join(root, layout.hooksConventionPath));
if (rootHasLoadable || isPluginShaped) return { kind: "root" };
if (layout.userSurfaceRoot !== undefined) {
return { kind: "user", sub: layout.userSurfaceRoot };
}
return { kind: "none" };
}
function materializeSurfaces(
root: string,
layout: PluginLayout,
files: Record<string, string>,
sources: Record<string, string>,
): Record<string, number> {
): MaterializedSurfaces {
const counts: Record<string, number> = {};
const isDir = (p: string): boolean =>
existsSync(p) && statSync(p).isDirectory();
@@ -324,16 +295,57 @@ function materializeSurfaces(
*/
const surfaceTree = (dir: string): Record<string, string> =>
isDir(dir) && walkableRoot(dir, root) ? readTree(dir, dir) : {};
// Read each ROOT-level surface tree once (keys relative to the surface dir).
const rootTrees = new Map<string, Record<string, string>>();
for (const surface of layout.surfaceDirs)
rootTrees.set(surface, surfaceTree(join(root, surface)));
/** Every surface tree of one scope, read once, keyed by surface dir. */
const scopeTrees = (base: string): Map<string, Record<string, string>> => {
const trees = new Map<string, Record<string, string>>();
for (const surface of layout.surfaceDirs)
trees.set(surface, surfaceTree(join(root, base, surface)));
return trees;
};
const hasLoadable = (trees: ReadonlyMap<string, Record<string, string>>) =>
layout.surfaceDirs.some((s) =>
surfaceHasLoadable(layout, s, trees.get(s) ?? {}),
);
const add = (key: string, content: string, onDisk: string): void => {
files[key] = content;
sources[key] = onDisk;
};
const source = classifySurfaceSource(root, layout, rootTrees);
// Both candidate scopes are read ONCE, up front, because the decision needs to
// know whether each holds anything — and then the same trees are materialized.
const rootTrees = scopeTrees("");
const userTrees =
layout.userSurfaceRoot !== undefined
? scopeTrees(layout.userSurfaceRoot)
: new Map<string, Record<string, string>>();
/** Copy one scope's already-read trees into `files`, keyed by that scope. */
const materializeScope = (
scope: SurfaceScope,
trees: ReadonlyMap<string, Record<string, string>>,
): void => {
for (const surface of layout.surfaceDirs) {
const tree = trees.get(surface) ?? {};
for (const [rel, content] of Object.entries(tree))
add(
scopeKey(scope, surface, rel),
content,
join(root, scope.base, surface, rel),
);
counts[surface] = (counts[surface] ?? 0) + Object.keys(tree).length;
}
};
const source = surfaceSource(layout, {
hasRootSkillFile: existsSync(join(root, "SKILL.md")),
skillName: basename(root),
rootHasLoadable: hasLoadable(rootTrees),
isPluginShaped:
existsSync(join(root, layout.manifestPath)) ||
existsSync(join(root, layout.hooksConventionPath)),
userHasLoadable: hasLoadable(userTrees),
});
switch (source.kind) {
case "single-skill": {
// Materialize the WHOLE skill dir under the canonical skills key, so its
@@ -350,36 +362,18 @@ function materializeSurfaces(
);
}
counts[layout.skillDir] = Object.keys(tree).length;
break;
return { counts, scopes: [] };
}
case "root":
case "user": {
const base = source.kind === "user" ? join(root, source.sub) : root;
for (const surface of layout.surfaceDirs) {
const dir = join(base, surface);
// Root surfaces were pre-read; user surfaces are read fresh here.
const tree =
source.kind === "root"
? (rootTrees.get(surface) ?? {})
: surfaceTree(dir);
for (const [rel, content] of Object.entries(tree)) {
add(
join(layout.materializeRoot, surface, rel),
content,
join(dir, rel),
);
}
counts[surface] = Object.keys(tree).length;
}
break;
case "scopes": {
assertDistinctScopeKeys(source.scopes, layout.name);
for (const scope of source.scopes)
materializeScope(scope, scope.base === "" ? rootTrees : userTrees);
return { counts, scopes: source.scopes };
}
case "none":
break;
/* v8 ignore next 2 -- exhaustiveness guard, unreachable given SurfaceSource */
default:
assertNever(source);
return assertNever(source);
}
return counts;
}
/**
@@ -391,12 +385,14 @@ function materializeSurfaces(
*/
function pluginWarnings(
root: string,
counts: Record<string, number>,
{ counts, scopes }: MaterializedSurfaces,
hooks: unknown,
files: Record<string, string>,
layout: PluginLayout,
): string[] {
const warnings: string[] = [];
const multiScope = multiScopeWarning(scopes, counts);
if (multiScope !== undefined) warnings.push(multiScope);
if (counts.agents) {
warnings.push(
`plugin defines ${String(counts.agents)} subagent file(s) under agents/ — these run only under a real model; test them at the eval tier (runEval), not the deterministic mock.`,
+49 -6
View File
@@ -34,6 +34,7 @@ import { editDistance } from "./core/edit-distance.js";
import { readFrontmatter, frontmatterScalar } from "./core/frontmatter-read.js";
import {
findDescriptionOverlaps,
type DescribedSurface,
type DescriptionOverlap,
} from "./core/description-overlap.js";
import {
@@ -541,24 +542,46 @@ export function skillRefSources(
function modelInvocableSkillSurfaces(
files: Record<string, string>,
cls: SurfaceClassifier,
): { name: string; description: string }[] {
const surfaces: { name: string; description: string }[] = [];
where?: SurfacePathContext,
): DescribedSurface[] {
const surfaces: DescribedSurface[] = [];
for (const [path, md] of Object.entries(files)) {
if (!cls.isSkill(path)) continue;
if (/^\s*disable-model-invocation:\s*true\s*$/m.test(md)) continue;
const fm = frontmatter(md);
const description = fm.description ?? firstBodyParagraph(md);
if (!description || description.length < 20) continue;
surfaces.push({ name: fm.name ?? skillName(path), description });
surfaces.push({
name: fm.name ?? skillName(path),
description,
// The NAME alone stopped identifying a surface once the loader began
// reading both discovery levels: a repo carrying `skills/x` AND
// `.claude/skills/x` has two skills called `x`. Report the real path
// (never the synthetic key) so the pair is actionable.
...(where
? {
where: reportedSurfacePath(path, where.sources?.[path], where.root),
}
: {}),
});
}
return surfaces;
}
/** Where a materialized key really lives — for reporting a surface by path. */
export interface SurfacePathContext {
readonly root: string;
readonly sources?: Record<string, string>;
}
export function descriptionOverlapsFor(
files: Record<string, string>,
cls: SurfaceClassifier,
where?: SurfacePathContext,
): DescriptionOverlap[] {
return findDescriptionOverlaps(modelInvocableSkillSurfaces(files, cls));
return findDescriptionOverlaps(
modelInvocableSkillSurfaces(files, cls, where),
);
}
/**
@@ -1031,9 +1054,29 @@ export function collectDelegationTrifecta(
delegatesTo: canDispatch ? allNames.filter((n) => n !== a.name) : [],
};
});
const pathByName = new Map(agents.map((a) => [a.name, a.path]));
// 🔴 A NAME IS NOT AN IDENTITY HERE, and this map used to assume it was.
// `new Map(agents.map((a) => [a.name, a.path]))` keeps the LAST entry for a
// repeated key, so when the same agent name exists in two discovery scopes —
// `agents/foo.md` and `.claude/agents/foo.md`, which Claude registers in
// DIFFERENT namespaces — a finding about the first was reported against the
// second's file. A lethal-trifecta finding that names the wrong file is worse
// than one that names none: it sends the reader to audit innocent code and
// leaves the real surface unexamined.
//
// Until identity is scope-qualified through the whole delegation pipeline (the
// proper fix, and a larger one — `finding.name` itself carries only the bare
// name), an ambiguous name resolves to NO path rather than to an arbitrary one.
// Nothing changes for the overwhelmingly common case of distinct names.
const pathByName = new Map<string, string>();
const ambiguous = new Set<string>();
for (const a of agents) {
if (pathByName.has(a.name)) ambiguous.add(a.name);
else pathByName.set(a.name, a.path);
}
return delegationTrifectaIssues(nodes, dialect).map((finding) => ({
path: pathByName.get(finding.name) ?? "",
path: ambiguous.has(finding.name)
? ""
: (pathByName.get(finding.name) ?? ""),
finding,
}));
}
+57
View File
@@ -289,6 +289,63 @@ describe("scanFiles parity for NESTED agents (the shape no vendored fixture has)
});
});
/**
* 🔴 THE TWO-SCOPE SHAPE the second shape no vendored fixture has.
*
* Every plugin under test/dogfood/ carries surfaces at exactly ONE level, so the
* both-levels-present branch was executed by neither engine during the byte-parity
* comparison, exactly as the single-skill branch wasn't. The loader used to read
* one level and materialize it under the OTHER level's canonical key; a divergence
* between the two engines here would be invisible for the same reason.
*
* Measured shape (nyldn/claude-octopus @ 2026-08-18): 61 skills at the repo root,
* 57 under `.claude/skills`, 50 names in both and all 50 pairs DIFFER.
*/
describe("scanFiles parity for a TWO-SCOPE repo (root skills/ AND .claude/skills)", () => {
const files = {
".claude-plugin/plugin.json": JSON.stringify({ name: "twoscope" }),
"skills/dup/SKILL.md":
"---\nname: dup\ndescription: The PLUGIN copy, loaded as /twoscope:dup by the harness\n---\n# plugin\n",
".claude/skills/dup/SKILL.md":
"---\nname: dup\ndescription: The PROJECT copy, loaded as /dup by the harness\n---\n# project\n",
".claude/agents/dev.md":
"---\nname: dev\ndescription: A project-level subagent that used to be dropped\n---\nbody\n",
};
it("produces an identical AuditReport on both engines", () => {
const tmp = makeTmpDir("parity-two-scope");
const abs = join(tmp, "two-scope-repo");
for (const [rel, body] of Object.entries(files)) {
mkdirSync(dirname(join(abs, rel)), { recursive: true });
writeFileSync(join(abs, rel), body);
}
const diskReport = scanPlugin(abs);
const fileReport = scanFiles(
readDirToMap(abs),
undefined,
undefined,
basename(abs),
);
const diskAudit = normalizeRoot(buildAuditReport(diskReport, OPTS), abs);
expect(buildAuditReport(fileReport, OPTS)).toEqual(diskAudit);
expect(stabilize(fileReport)).toEqual(
stabilize(normalizeRoot(diskReport, abs)),
);
cleanupTmpDir(tmp);
});
it("…and BOTH copies are actually there — parity is agreement, not correctness", () => {
// Without this the parity assertion above passes while both engines drop the
// same file, which is precisely how the shadowing bug survived.
const report = scanFiles(files, undefined, undefined, "two-scope-repo");
expect(report.skills.map((s) => s.path).sort()).toEqual([
".claude/skills/dup/SKILL.md",
"skills/dup/SKILL.md",
]);
expect(report.agents.map((a) => a.path)).toEqual([".claude/agents/dev.md"]);
});
});
describe("scanFiles — non-plugin and empty inputs", () => {
it("reports an instruction-only repo (CLAUDE.md, no spec) like scanPlugin would", () => {
const map = {
+91 -72
View File
@@ -48,6 +48,19 @@ import { verifyMcpServers } from "./core/mcp-config.js";
import { agentPluginsMcpSources } from "./core/agent-plugins.js";
import { verifyMcpHookTargets } from "./core/mcp-hook.js";
import { pluginDirLayoutIssues } from "./core/plugin-dir-layout.js";
import {
assertDistinctScopeKeys,
multiScopeWarning,
scopeKey,
surfaceSource,
type SurfaceScope,
} from "./core/surface-scopes.js";
/** Mirror of plugin-loader.ts `MaterializedSurfaces`. */
interface MaterializedSurfaces {
readonly counts: Record<string, number>;
readonly scopes: readonly SurfaceScope[];
}
import { hookBlockIssues } from "./core/hook-block-ineffective.js";
import { hookMatcherIssues } from "./core/hook-matcher.js";
import { findUntestedSurfacesInFiles } from "./test-coverage-files.js";
@@ -266,13 +279,12 @@ function hasMcp(files: Record<string, string>, layout: PluginLayout): boolean {
// Surface materialization (mirrors plugin-loader.ts materializeSurfaces)
// ---------------------------------------------------------------------------
type SurfaceSource =
| { readonly kind: "single-skill"; readonly skillName: string }
| { readonly kind: "root" }
| { readonly kind: "user"; readonly sub: string }
| { readonly kind: "none" };
/** Mirror of plugin-loader.ts `surfaceHasLoadable`. */
/**
* Mirror of plugin-loader.ts `surfaceHasLoadable`. The SCOPING DECISION itself is
* not mirrored it lives once, IO-free, in `src/core/surface-scopes.ts`, and both
* engines call it. That pair used to be two copies of the same precedence rules,
* which is exactly the shape this repo keeps getting bitten by fixing on one side.
*/
function surfaceHasLoadable(
layout: PluginLayout,
surface: string,
@@ -284,35 +296,6 @@ function surfaceHasLoadable(
: keys.some((k) => k.endsWith(".md"));
}
/** Mirror of plugin-loader.ts `classifySurfaceSource`, over the file map. */
function classifySurfaceSource(
files: Record<string, string>,
layout: PluginLayout,
rootTrees: ReadonlyMap<string, Record<string, string>>,
repoName?: string,
): SurfaceSource {
if (layout.skillDir && hasFile(files, "SKILL.md")) {
// Disk mirrors the CLI: a nameless root SKILL.md takes the audited dir's
// basename. In-browser there's no real dir, so use the repo name when the
// caller (runAudit) supplies it, else the synthetic BROWSER_ROOT basename.
return {
kind: "single-skill",
skillName: repoName ?? basename(BROWSER_ROOT),
};
}
const rootHasLoadable = layout.surfaceDirs.some((s) =>
surfaceHasLoadable(layout, s, rootTrees.get(s) ?? {}),
);
const isPluginShaped =
hasFile(files, layout.manifestPath) ||
hasFile(files, layout.hooksConventionPath);
if (rootHasLoadable || isPluginShaped) return { kind: "root" };
if (layout.userSurfaceRoot !== undefined) {
return { kind: "user", sub: layout.userSurfaceRoot };
}
return { kind: "none" };
}
/** The materialization accumulator — surface contents + their on-disk source paths. */
interface Materialized {
out: Record<string, string>;
@@ -325,21 +308,66 @@ function materializeSurfaces(
layout: PluginLayout,
acc: Materialized,
repoName?: string,
): Record<string, number> {
): MaterializedSurfaces {
const { out, sources } = acc;
const counts: Record<string, number> = {};
const rootTrees = new Map<string, Record<string, string>>();
for (const surface of layout.surfaceDirs) {
if (isDirRel(files, surface)) {
rootTrees.set(surface, readTreeUnder(files, surface, surface));
const scopeTrees = (base: string): Map<string, Record<string, string>> => {
const trees = new Map<string, Record<string, string>>();
for (const surface of layout.surfaceDirs) {
const dirRel = base === "" ? surface : `${base}/${surface}`;
trees.set(
surface,
isDirRel(files, dirRel) ? readTreeUnder(files, dirRel, dirRel) : {},
);
}
}
return trees;
};
const hasLoadable = (trees: ReadonlyMap<string, Record<string, string>>) =>
layout.surfaceDirs.some((s) =>
surfaceHasLoadable(layout, s, trees.get(s) ?? {}),
);
const add = (key: string, content: string, onDisk: string): void => {
out[key] = content;
sources[key] = onDisk;
};
const source = classifySurfaceSource(files, layout, rootTrees, repoName);
const rootTrees = scopeTrees("");
const userTrees =
layout.userSurfaceRoot !== undefined
? scopeTrees(layout.userSurfaceRoot)
: new Map<string, Record<string, string>>();
/** Mirror of the disk loader's `materializeScope`. */
const materializeScope = (
scope: SurfaceScope,
trees: ReadonlyMap<string, Record<string, string>>,
): void => {
for (const surface of layout.surfaceDirs) {
const tree = trees.get(surface) ?? {};
const dirRel = scope.base === "" ? surface : `${scope.base}/${surface}`;
for (const [rel, content] of Object.entries(tree))
add(
scopeKey(scope, surface, rel),
content,
join(BROWSER_ROOT, dirRel, rel),
);
counts[surface] = (counts[surface] ?? 0) + Object.keys(tree).length;
}
};
const source = surfaceSource(layout, {
hasRootSkillFile: Boolean(layout.skillDir) && hasFile(files, "SKILL.md"),
// Disk mirrors the CLI: a nameless root SKILL.md takes the audited dir's
// basename. In-browser there's no real dir, so use the repo name when the
// caller (runAudit) supplies it, else the synthetic BROWSER_ROOT basename.
skillName: repoName ?? basename(BROWSER_ROOT),
rootHasLoadable: hasLoadable(rootTrees),
isPluginShaped:
hasFile(files, layout.manifestPath) ||
hasFile(files, layout.hooksConventionPath),
userHasLoadable: hasLoadable(userTrees),
});
switch (source.kind) {
case "single-skill": {
const tree = readTreeUnder(files, "", "");
@@ -351,34 +379,15 @@ function materializeSurfaces(
);
}
counts[layout.skillDir] = Object.keys(tree).length;
break;
return { counts, scopes: [] };
}
case "root":
case "user": {
const baseRel = source.kind === "user" ? source.sub : "";
for (const surface of layout.surfaceDirs) {
const dirRel = baseRel === "" ? surface : `${baseRel}/${surface}`;
const tree =
source.kind === "root"
? (rootTrees.get(surface) ?? {})
: isDirRel(files, dirRel)
? readTreeUnder(files, dirRel, dirRel)
: {};
for (const [rel, content] of Object.entries(tree)) {
add(
join(layout.materializeRoot, surface, rel),
content,
join(BROWSER_ROOT, dirRel, rel),
);
}
counts[surface] = Object.keys(tree).length;
}
break;
case "scopes": {
assertDistinctScopeKeys(source.scopes, layout.name);
for (const scope of source.scopes)
materializeScope(scope, scope.base === "" ? rootTrees : userTrees);
return { counts, scopes: source.scopes };
}
case "none":
break;
}
return counts;
}
// ---------------------------------------------------------------------------
@@ -479,12 +488,14 @@ function danglingRefs(
function pluginWarnings(
files: Record<string, string>,
layout: PluginLayout,
counts: Record<string, number>,
{ counts, scopes }: MaterializedSurfaces,
hooks: unknown,
materialized: Record<string, string>,
rootName: string,
): string[] {
const warnings: string[] = [];
const multiScope = multiScopeWarning(scopes, counts);
if (multiScope !== undefined) warnings.push(multiScope);
if (counts.agents) {
warnings.push(
`plugin defines ${String(counts.agents)} subagent file(s) under agents/ — these run only under a real model; test them at the eval tier (runEval), not the deterministic mock.`,
@@ -549,7 +560,12 @@ export function loadPluginFromFiles(
layout.instructionFile,
);
}
const counts = materializeSurfaces(files, layout, { out, sources }, repoName);
const surfaces = materializeSurfaces(
files,
layout,
{ out, sources },
repoName,
);
return {
settings: resolvedHooks ? { hooks: resolvedHooks } : {},
@@ -558,7 +574,7 @@ export function loadPluginFromFiles(
warnings: pluginWarnings(
files,
layout,
counts,
surfaces,
resolvedHooks,
out,
repoName ?? basename(BROWSER_ROOT),
@@ -714,7 +730,10 @@ export function scanFiles(
declaredServers,
dialect,
),
descriptionOverlaps: descriptionOverlapsFor(loaded.files, cls),
descriptionOverlaps: descriptionOverlapsFor(loaded.files, cls, {
root: BROWSER_ROOT,
sources: loaded.sources,
}),
descriptionBudgetIssues: descriptionBudgetFor(loaded.files, cls),
trifectaFindings,
skillResourceIssues: skillResourceFindings,
+27
View File
@@ -103,6 +103,33 @@ test("true-positive: madappgang-frontend tester reproduces AskUserQuestion + mal
);
});
// TRUE-POSITIVE — the SHADOWING fixture (MIT; see test/dogfood/README.md). Both
// copies of one skill, from the same commit of the same repo: the `skills/` copy
// (well-formed) and the `.claude/skills/` copy (a multi-line unquoted
// `description:` that strict YAML rejects). Until 2026-08 the loader read ONE
// discovery level, so the broken copy was never opened — vigiles reported this
// skill clean while naming a file it had not read. Upstream had 50 such pairs and
// every one of them differed.
test("true-positive: claude-octopus flow-define exists in BOTH scopes, and the .claude copy's YAML is broken", () => {
const r = scanPlugin(vendored("claude-octopus"));
assert.deepEqual(
r.skills.map((sk) => sk.path).sort(),
[".claude/skills/flow-define/SKILL.md", "skills/flow-define/SKILL.md"],
"both copies are surfaces — neither shadows the other",
);
assert.ok(
r.malformedFrontmatter.some((m) =>
m.path.startsWith(".claude/skills/flow-define"),
),
"the .claude copy's multi-line unquoted description is invalid YAML",
);
assert.ok(
!r.malformedFrontmatter.some((m) => m.path.startsWith("skills/")),
"…and the root copy is well-formed, so the finding is attributed to the right file",
);
});
// FP-GUARD (calibration) — a real MIT hook component (davila7/claude-code-templates)
// whose description says it "blocks deployments" but is a PostToolUse hook that
// `exit 2`s. On PostToolUse, exit 2 FEEDS stderr back to the model (a legitimate
+4 -1
View File
@@ -815,9 +815,12 @@ test("scanPlugin flags near-duplicate model-invocable skill descriptions, skips
);
const r = scanPlugin(dir);
assert.equal(r.descriptionOverlaps.length, 1);
// The pair is labelled by name AND path: since the loader reads both discovery
// levels, a name alone can name two different files (`skills/x` +
// `.claude/skills/x`), and "x and x" identifies neither.
assert.deepEqual(
[r.descriptionOverlaps[0].a, r.descriptionOverlaps[0].b].sort(),
["a", "b"],
['"a" (skills/a/SKILL.md)', '"b" (skills/b/SKILL.md)'],
);
assert.match(formatScanReport(r), /near-identical/);
cleanupTmpDir(dir);
+4 -1
View File
@@ -651,7 +651,10 @@ export function scanPlugin(
declaredServers,
dialect,
),
descriptionOverlaps: descriptionOverlapsFor(loaded.files, cls),
descriptionOverlaps: descriptionOverlapsFor(loaded.files, cls, {
root: resolve(dir),
sources: loaded.sources,
}),
descriptionBudgetIssues: descriptionBudgetFor(loaded.files, cls),
trifectaFindings,
skillResourceIssues: skillResourceFindings,
+4 -1
View File
@@ -76,7 +76,10 @@ function overlapExplanations(report: ScanReport): ScoreExplanation[] {
symptom: "wrong-skill-fires",
cause: o.message,
detector: "description-overlap",
fix: `Differentiate the descriptions of "${o.a}" and "${o.b}" (${o.similarity} similar) — the selector picks by description, so near-identical text makes it fire the wrong one.`,
// `o.a`/`o.b` are already display LABELS (quoted name, plus the file path
// when the caller had one) — a bare name is ambiguous now that both
// discovery levels are read and the same name can appear twice.
fix: `Differentiate the descriptions of ${o.a} and ${o.b} (${o.similarity} similar) — the selector picks by description, so near-identical text makes it fire the wrong one.`,
confidence: "possible",
}));
}
+1
View File
@@ -54,6 +54,7 @@ their SHA.
| Slice (`dir@sha`) | Upstream | License | Reproduces (verified) |
| ----------------------------- | ---------------------------------------------------------------------------------------------------------------- | -------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `madappgang-frontend@6097ad4` | github.com/MadAppGang/claude-code (`plugins/frontend/agents/tester.md`) | MIT — Copyright (c) 2024 MadAppGang - Jack Rudenko | **True-positive (×3):** `subagent-tool-contract` (`AskUserQuestion` never available to a subagent), `frontmatter-valid` (the one-line `description:` isn't valid YAML), **and** a hard `lethal-trifecta` (Read/Bash + WebFetch/WebSearch + Bash/WebFetch = read-private ∧ ingest-untrusted ∧ exfiltrate) |
| `claude-octopus@5a9cab8` | github.com/nyldn/claude-octopus (`skills/flow-define/SKILL.md` + `.claude/skills/flow-define/SKILL.md`, both from the same commit) | MIT — Copyright (c) 2026 nyldn | **True-positive** for the **surface-scope shadowing** bug (2026-08-18). Upstream ships each skill TWICE — 61 under `skills/`, 57 under `.claude/skills/`, **50 names in both, and all 50 pairs differ**. vigiles read one discovery level and materialized it under the OTHER one's canonical key, so 71 files that exist on disk were never opened and the report named files it had not read. This slice is one such pair: the `skills/` copy is well-formed, the `.claude/skills/` copy has a multi-line unquoted `description:` that strict YAML rejects. Locks (a) both copies load as separate surfaces and (b) `frontmatter-valid` is attributed to the copy that is actually broken. Manifest deliberately NOT vendored — it lists 60+ skills the slice doesn't carry. |
| `davila7-perf-guard@869640b` | github.com/davila7/claude-code-templates (`cli-tool/components/hooks/performance/performance-budget-guard.json`) | MIT — Copyright (c) 2025 Daniel (San) Ávila | **Calibration FP-guard** for `hook-block-ineffective`. Its description says it "blocks deployments" but it's a `PostToolUse` hook that `exit 2`s — which on PostToolUse FEEDS stderr back to the model (a legitimate channel), not a failed block. Since block-vs-feedback intent isn't deterministically separable, the detector must **NOT** flag it (else it cries wolf on every nudge/lint hook, incl. vigiles's own `refs-nudge.sh`). Locks the don't-cry-wolf calibration. Vendored as `.claude/settings.json`. |
## Sweep manifest — broader scans (verdict saved even where files aren't)
@@ -0,0 +1,775 @@
---
name: flow-define
disable-model-invocation: true
aliases:
- define
- define-workflow
- grasp
- grasp-workflow
description: Multi-AI requirements scoping using available external providers (Double Diamond Define phase)
PRIORITY TRIGGERS (always invoke): "octo define", "octo scope", "co-define", "co-scope"
DO NOT use for: implementation tasks (use flow-develop), research (use flow-discover),
review/validation (use flow-deliver), or built-in commands.
# Claude Code v2.1.12+ Integration
agent: Plan
context: fork
task_management: true
task_dependencies:
- flow-discover
execution_mode: enforced
pre_execution_contract:
- visual_indicators_displayed
validation_gates:
- orchestrate_sh_executed
- synthesis_file_exists
trigger: |
EXPLICITLY USE when user requests clarification or scoping:
- "define the requirements for X"
- "clarify the scope of Y"
- "what exactly does X need to do"
- "help me understand the problem with Y"
- "scope out the Z feature"
- "what are the specific requirements for X"
DO NOT activate for:
- Implementation tasks (use tangle-workflow)
- Research tasks (use probe-workflow)
- Review tasks (use ink-workflow)
- Built-in commands (/plugin, /help, etc.)
---
{{PREAMBLE}}
## Pre-Definition: State Check
Before starting definition:
1. Read `.octo/STATE.md` to verify Discover phase complete
2. Update STATE.md:
- current_phase: 2
- phase_position: "Definition"
- status: "in_progress"
```bash
# Verify Discover phase is complete
if [[ -f ".octo/STATE.md" ]]; then
discover_status=$("${HOME}/.claude-octopus/plugin/scripts/octo-state.sh" get_phase_status 1)
if [[ "$discover_status" != "complete" ]]; then
echo "⚠️ Warning: Discover phase not marked complete. Consider running discovery first."
fi
fi
# Update state for Definition phase
"${HOME}/.claude-octopus/plugin/scripts/octo-state.sh" update_state \
--phase 2 \
--position "Definition" \
--status "in_progress"
```
---
## Execution Contract
This skill uses **ENFORCED execution mode**. You MUST follow this exact sequence.
### STEP 1: Display Visual Indicators
**MANDATORY: You MUST use the Bash tool to run this provider check BEFORE displaying the banner. Do NOT skip it. Do NOT assume availability.**
```bash
provider_check_output=$(bash "${HOME}/.claude-octopus/plugin/scripts/helpers/check-providers.sh")
provider_status_lines=$(printf '%s\n' "$provider_check_output" | awk '
/^PROVIDER_CHECK_START$/ { capture=1; next }
/^PROVIDER_CHECK_END$/ { capture=0 }
capture && /^[a-z0-9-]+:(available|missing|degraded)$/ { print }
')
if [[ -z "$provider_status_lines" ]]; then
echo "Provider availability check returned no usable status lines."
exit 1
fi
# Exclude providers intentionally disabled by environment, session, or global
# allowlist policy; show every allowed provider, including missing/degraded.
source "${HOME}/.claude-octopus/plugin/scripts/lib/provider-allowlist.sh"
provider_availability=""
available_provider_count=0
while IFS=: read -r provider status; do
octo_provider_allowed "$provider" || continue
case "$status" in
available)
provider_availability="${provider_availability}🟢 ${provider}: Available ✓"$'\n'
available_provider_count=$((available_provider_count + 1))
;;
degraded)
provider_availability="${provider_availability}🟠 ${provider}: Degraded ⚠"$'\n'
;;
missing)
provider_availability="${provider_availability}🔴 ${provider}: Missing/unavailable ✗"$'\n'
;;
esac
done <<< "$provider_status_lines"
if [[ "$available_provider_count" -eq 0 ]]; then
printf '%s' "$provider_availability"
echo "No external provider is available. Run /octo:setup and retry."
exit 1
fi
# Task status for the banner's Tasks line, if the session has one.
task_status=$("${HOME}/.claude-octopus/plugin/scripts/orchestrate.sh" get-task-status 2>/dev/null || echo "")
```
List every provider the check reports, not only Claude. A banner showing one seat when several ran misrepresents what the user is paying for.
If `OCTO_ALLOWED_PROVIDERS` is set, treat it as the source of truth for which providers may participate. Providers filtered out by that allowlist are intentionally reported as unavailable; do not invoke or recommend them in the workflow.
**Display this banner BEFORE orchestrate.sh execution:**
```
🐙 **CLAUDE OCTOPUS ACTIVATED** - Multi-provider definition mode
🎯 Define Phase: [Brief description of what you're defining/scoping]
📋 Session: ${CLAUDE_SESSION_ID}
📝 Tasks: ${task_status}
Provider Availability:
${provider_availability}
🔵 Claude: Available ✓ - Consensus building and synthesis
💰 Estimated Cost: $0.01-0.05
⏱️ Estimated Time: 2-5 minutes
```
**DO NOT PROCEED TO STEP 2 until banner displayed.** The banner shows users which providers will run and what costs they'll incur — starting API calls without this visibility violates cost transparency.
---
### STEP 2: Read Prior State
**Before executing the workflow, read any prior context:**
```bash
# Initialize state if needed
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" init_state
# Set current workflow
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" set_current_workflow "flow-define" "define"
# Get prior decisions (if any)
prior_decisions=$("${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" get_decisions "all")
# Get context from discover phase
discover_context=$("${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" get_context "discover")
# Display what you found (if any)
if [[ "$discover_context" != "null" ]]; then
echo "📋 Building on discovery findings:"
echo " $discover_context"
fi
if [[ "$prior_decisions" != "[]" && "$prior_decisions" != "null" ]]; then
echo "📋 Respecting prior decisions:"
echo "$prior_decisions" | jq -r '.[] | " - \(.decision) (\(.phase)): \(.rationale)"'
fi
```
**This provides context from:**
- Discovery phase research (if completed)
- Prior architectural decisions
- User vision captured earlier
- If **claude-mem** is installed, its MCP tools (`search`, `timeline`, `get_observations`) are available — use them to find past decisions on similar topics
**DO NOT PROCEED TO STEP 3 until state read.**
---
### STEP 3: Phase Discussion — Capture User Vision
**Before executing expensive multi-AI orchestration, capture the user's vision to scope the work effectively.**
**Ask clarifying questions using AskUserQuestion:**
```
Use AskUserQuestion tool to ask:
1. **User Experience**
Question: "How should users interact with this feature?"
Header: "User Flow"
Options:
- label: "API-first (programmatic access)"
description: "Build API endpoints first, UI later"
- label: "UI-first (user-facing interface)"
description: "Build user interface first, API supports it"
- label: "Both simultaneously"
description: "Develop API and UI in parallel"
- label: "Not applicable"
description: "This feature doesn't have a user interaction"
2. **Implementation Approach**
Question: "What technical approach do you prefer?"
Header: "Approach"
Options:
- label: "Fastest to market"
description: "Prioritize speed, use existing libraries"
- label: "Most maintainable"
description: "Focus on clean architecture, may take longer"
- label: "Best performance"
description: "Optimize for speed and efficiency"
- label: "Multi-LLM debate (Claude + available providers)"
description: "Multiple AI models debate the best approach — may use external provider credits or subscriptions"
3. **Scope Boundaries**
Question: "What's explicitly OUT of scope for this phase?"
Header: "Out of Scope"
Options:
- label: "Testing and QA"
description: "Focus on implementation, test later"
- label: "Performance optimization"
description: "Get it working first, optimize later"
- label: "Edge cases"
description: "Handle happy path only initially"
- label: "Nothing excluded"
description: "Everything is in scope"
multiSelect: true
```
**If user selected "Multi-LLM debate (Claude + available providers)" for approach:**
Before proceeding with orchestrate.sh, run a Multi-LLM debate to determine the technical approach:
```
/octo:debate --rounds 2 --debate-style collaborative "What is the best technical approach for [feature]? Consider: speed to market, maintainability, performance, and the existing codebase patterns."
```
Use the debate synthesis to set the approach context for the Define phase.
**After gathering answers, create context file:**
```bash
# Source context manager
source "${HOME}/.claude-octopus/plugin/scripts/context-manager.sh"
# Extract user answers from AskUserQuestion results
user_flow="[Answer from question 1]"
approach="[Answer from question 2]"
out_of_scope="[Answer from question 3]"
# Create context file with user vision
create_templated_context \
"define" \
"$(echo "$USER_REQUEST" | head -c 50)..." \
"User wants: $user_flow approach with $approach priority" \
"$approach" \
"Implementation of requested feature" \
"$out_of_scope"
echo "📋 Context captured and saved to .claude-octopus/context/define-context.md"
```
**This context will be used to:**
- Scope the multi-AI research (discover phase)
- Focus the requirements definition (define phase)
- Guide implementation decisions (develop phase)
- Validate against user expectations (deliver phase)
**DO NOT PROCEED TO STEP 4 until context captured.** User vision (UX approach, priorities, out-of-scope items) scopes the multi-AI research — without it, providers research too broadly and the definition misses the user's actual intent.
---
### STEP 4: Execute orchestrate.sh define
**You MUST execute this command via the Bash tool:**
```bash
${HOME}/.claude-octopus/plugin/scripts/orchestrate.sh define "<user's clarification request>"
```
**CRITICAL: You are PROHIBITED from:**
- ❌ Defining requirements directly without calling orchestrate.sh — single-model analysis misses the technical, business, and external-model perspective split that provider fanout supplies, producing requirements with blind spots
- ❌ Using direct analysis instead of orchestrate.sh
- ❌ Claiming you're "simulating" the workflow
- ❌ Proceeding to Step 5 without running this command
**You MUST use the Bash tool to invoke orchestrate.sh.**
#### What Users See During Execution (v7.16.0+)
If running in Claude Code v2.1.16+, users will see **real-time progress indicators** in the task spinner:
**Phase 1 - External Provider Execution (Parallel):**
- 🔴 Analyzing technical requirements (Codex)...
- 🟡 Clarifying user needs and context (Antigravity)...
**Phase 2 - Synthesis (Sequential):**
- 🔵 Building consensus on problem definition...
These spinner verb updates happen automatically - orchestrate.sh calls `update_task_progress()` before each agent execution. Users see exactly which provider is working and what it's doing.
**If NOT running in Claude Code v2.1.16+:** Progress indicators are silently skipped, no errors shown.
---
### STEP 5: Verify Execution
**After orchestrate.sh completes, verify it succeeded:**
```bash
# Find the latest synthesis file (created within last 10 minutes)
SYNTHESIS_FILE=$(find ~/.claude-octopus/results -name "grasp-synthesis-*.md" -mmin -10 2>/dev/null | head -n1)
if [[ -z "$SYNTHESIS_FILE" ]]; then
echo "❌ VALIDATION FAILED: No synthesis file found"
echo "orchestrate.sh did not execute properly"
exit 1
fi
echo "✅ VALIDATION PASSED: $SYNTHESIS_FILE"
cat "$SYNTHESIS_FILE"
```
**If validation fails:**
1. Report error to user
2. Show logs from `~/.claude-octopus/logs/`
3. DO NOT proceed with presenting results
4. DO NOT substitute with direct analysis — fallback to single-model analysis defeats the purpose of multi-provider consensus and produces narrower requirements
---
### STEP 6: Update State
**After synthesis is verified, record findings and decisions in state:**
```bash
# Extract key definition from synthesis
key_definition=$(head -50 "$SYNTHESIS_FILE" | grep -A 3 "## Problem Definition\|## Summary" | tail -3 | tr '\n' ' ')
# Record any architectural decisions made
# (You should identify these from the synthesis - e.g., tech stack, approach, patterns)
decision_made=$(echo "$key_definition" | grep -o "decided to\|chose to\|selected\|using [A-Za-z0-9 ]*" | head -1)
if [[ -n "$decision_made" ]]; then
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" write_decision \
"define" \
"$decision_made" \
"Consensus from multi-AI definition phase"
fi
# Update define phase context
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" update_context \
"define" \
"$key_definition"
# Update metrics
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" update_metrics "phases_completed" "1"
```
**DO NOT PROCEED TO STEP 7 until state updated.**
---
### STEP 7: Present Problem Definition (Only After Steps 1-6 Complete)
Read the synthesis file and present:
- Core requirements (must have, should have, nice to have)
- Technical constraints
- User needs
- Edge cases to handle
- Out of scope items
- Perspectives from all providers
- Requirements checklist
- Next steps (usually tangle phase for implementation)
**Include attribution:**
```
---
*Multi-AI Problem Definition powered by Claude Octopus*
*Providers: available external providers + 🔵 Claude*
*Full problem definition: $SYNTHESIS_FILE*
```
---
# Define Workflow - Define Phase 🎯
<!-- Banner requirement lives in Execution Contract STEP 1 above. -->
## What This Workflow Does
The **define** phase clarifies and scopes problems using external CLI providers:
1. **🔴 Codex CLI** - Technical requirements analysis, edge cases, constraints
2. **🟡 Antigravity CLI** - User needs, business requirements, context understanding
3. **🧭 Antigravity CLI** - Additional external-model challenge
4. **🔵 Claude (You)** - Problem synthesis and requirement definition
This is the **convergent** phase after discovery - we narrow down from broad research to specific problem definition.
---
## When to Use Define
Use define when you need:
- **Requirement Definition**: "Define exactly what the auth system needs to do"
- **Problem Clarification**: "Clarify the caching requirements"
- **Scope Definition**: "What's the scope of the notification feature?"
- **Constraint Identification**: "What are the technical constraints for X?"
- **Edge Case Analysis**: "What edge cases do we need to handle for Y?"
- **Requirement Validation**: "Are these requirements complete for Z?"
**Don't use define for:**
- Research and exploration (use probe-workflow)
- Building implementations (use tangle-workflow)
- Code review and validation (use ink-workflow)
- Simple questions Claude can answer
---
## Visual Indicators
Before execution, you'll see a banner in this shape. The provider rows below are illustrative; replace them with every live status from Step 1 and omit only providers excluded by the active allowlist:
```text
🐙 **CLAUDE OCTOPUS ACTIVATED** - Multi-provider problem definition
🎯 Define Phase: Clarifying requirements and scope
📋 Session: ${CLAUDE_SESSION_ID}
📝 Tasks: ${task_status}
Provider Availability:
🟢 codex: Available ✓
🧭 agy: Degraded ⚠
🔴 agy: Missing/unavailable ✗
🔵 Claude: Available ✓ - Consensus building and synthesis
```
---
## How It Works
### Stage A: Invoke Grasp Phase
```bash
${HOME}/.claude-octopus/plugin/scripts/orchestrate.sh define "<user's clarification request>"
```
### Stage B: Multi-Provider Problem Definition
The orchestrate.sh script will:
1. Call **Codex CLI** for technical requirement analysis
2. Call **Antigravity CLI** for business/user need analysis
3. Call **Antigravity CLI** for additional external-model challenge
4. You (Claude) synthesize into clear problem definition
5. Identify gaps and missing requirements
### Stage C: Read Results
Results are saved to:
```text
~/.claude-octopus/results/${SESSION_ID}/grasp-synthesis-<timestamp>.md
```
### Stage D: Present Problem Definition
Read the synthesis and present clear, actionable requirements to the user.
---
## Implementation Instructions
When this skill is invoked, follow the EXECUTION CONTRACT above exactly. The contract includes:
1. **Blocking Step 1**: Check providers and display live visual indicators
2. **Blocking Step 2**: Read prior workflow state
3. **Blocking Step 3**: Capture and persist the user's vision
4. **Blocking Step 4**: Execute orchestrate.sh define via Bash tool
5. **Blocking Step 5**: Verify the synthesis file exists
6. **Blocking Step 6**: Update workflow state
7. **Step 7**: Present the formatted problem definition
Each step is **mandatory and blocking** - you cannot proceed to the next step until the current one completes successfully.
### Task Management Integration
Create tasks to track execution progress:
```javascript
// At start of skill execution
TaskCreate({
subject: "Execute define workflow with multi-AI providers",
description: "Run orchestrate.sh define for problem clarification",
activeForm: "Running multi-AI define workflow"
})
// Mark in_progress when calling orchestrate.sh
TaskUpdate({taskId: "...", status: "in_progress"})
// Mark completed ONLY after synthesis file verified
TaskUpdate({taskId: "...", status: "completed"})
```
### Error Handling
If any step fails:
- **Step 1 (Providers)**: If all external providers are unavailable, suggest `/octo:setup` and STOP
- **Step 4 (orchestrate.sh)**: Show bash error, check logs, report to user
- **Step 5 (Validation)**: If synthesis is missing, show orchestrate.sh logs and do not substitute direct analysis
Never fall back to direct analysis if orchestrate.sh execution fails. Report the failure and let the user decide how to proceed.
### Problem Definition Format
After successful execution, present problem definition with:
```
# Problem Definition: <task>
## Core Requirements
[Essential requirements that must be met]
## Technical Constraints
[Technical limitations and considerations]
## User Needs
[What users actually need from this]
## Edge Cases to Handle
[Specific edge cases identified]
## Out of Scope
[What is explicitly NOT included]
## Perspectives
### Codex Analysis (Technical)
[Technical requirements and constraints]
### Antigravity Analysis (Business/User)
[User needs and business context]
### Claude Synthesis
[Integrated problem definition]
## Requirements Checklist
- [ ] Functional requirements defined
- [ ] Non-functional requirements defined
- [ ] Technical constraints identified
- [ ] Edge cases documented
- [ ] Out-of-scope items clarified
- [ ] Success criteria established
## Next Steps
[Recommended next actions - usually tangle phase for implementation]
Full problem definition saved to: <synthesis file path>
```
---
## Example Usage
### Example 1: Define Auth Requirements
```
User: Define the exact requirements for our user authentication system
Claude:
🐙 **CLAUDE OCTOPUS ACTIVATED** - Multi-provider problem definition
🎯 Define Phase: Clarifying authentication requirements
[Executes: ${HOME}/.claude-octopus/plugin/scripts/orchestrate.sh define "Define exact requirements for user authentication system"]
[After completion, reads synthesis and presents:]
# Problem Definition: User Authentication System
## Core Requirements
### Must Have (P0)
1. User registration with email + password
2. Secure login with JWT token generation
3. Password reset via email
4. Session management with token refresh
5. Logout (token invalidation)
### Should Have (P1)
6. Remember me functionality
7. Account lockout after failed attempts
8. Email verification on signup
9. Password strength requirements
### Nice to Have (P2)
10. OAuth social login (Google, GitHub)
11. Two-factor authentication
12. Single sign-on (SSO)
## Technical Constraints
- Must work with existing Express.js backend
- Token storage: Redis for production, in-memory for dev
- Password hashing: bcrypt (min 12 rounds)
- Token expiry: Access token 15min, refresh token 7 days
- Rate limiting: Max 5 login attempts per 15min per IP
## User Needs
- Simple registration process (< 2 minutes)
- Secure password recovery without support tickets
- Stay logged in across browser sessions (remember me)
- Clear error messages for failed login
- Ability to log out from all devices
## Edge Cases to Handle
1. **Concurrent logins**: Same user, multiple devices
2. **Expired tokens**: Graceful refresh without re-login
3. **Password reset race condition**: Multiple reset requests
4. **Account enumeration**: Don't reveal if email exists
5. **Token theft**: Detect suspicious login locations
6. **Deleted users**: Handle auth for soft-deleted accounts
## Out of Scope
- ❌ Biometric authentication (fingerprint, face ID)
- ❌ LDAP/Active Directory integration
- ❌ Custom OAuth provider implementation
- ❌ Device fingerprinting
- ❌ Passwordless email magic links
## Perspectives
### Codex Analysis (Technical)
- JWT structure: { userId, role, exp, iat, jti }
- Database schema for users, refresh_tokens, password_resets
- API endpoints: POST /register, POST /login, POST /logout, POST /refresh, POST /reset-password
- Middleware for auth verification
- Security: HTTPS only, secure cookies, CSRF protection
### Antigravity Analysis (Business/User)
- User journey: Registration -> Email verification -> Login -> Access app
- Error handling: Clear messages without security leaks
- Performance: Auth checks < 50ms
- Compliance: GDPR (data deletion), password policies
- Analytics: Track signup conversion, failed login rates
### Claude Synthesis
- Hybrid approach: Core auth (P0) first, iterate on P1/P2
- Security-first: All requirements validated against OWASP
- User experience: Balance security with convenience
- Scalable: Design for 100K users, plan for 1M+
## Requirements Checklist
- ✅ Functional requirements defined (registration, login, reset)
- ✅ Non-functional requirements defined (performance, security)
- ✅ Technical constraints identified (Express, Redis, bcrypt)
- ✅ Edge cases documented (6 critical cases)
- ✅ Out-of-scope items clarified (4 items)
- ✅ Success criteria established (< 2min registration, < 50ms auth)
## Next Steps
1. **Immediate**: Review and confirm requirements with stakeholders
2. **Then**: Use **tangle-workflow** to implement the auth system
3. **Finally**: Use **ink-workflow** to validate implementation
Ready to proceed to implementation?
Full problem definition saved to: ~/.claude-octopus/results/abc-123/grasp-synthesis-20250118-144530.md
```
### Example 2: Clarify Feature Scope
```
User: What exactly does the notification feature need to do?
Claude:
🐙 **CLAUDE OCTOPUS ACTIVATED** - Multi-provider problem definition
🎯 Define Phase: Clarifying notification requirements
[Executes grasp workflow]
[Presents detailed problem definition with:]
- Core notification types (email, push, in-app)
- Delivery requirements (real-time vs batched)
- User preferences (opt-in/out, frequency)
- Technical constraints (message queue, delivery tracking)
- Edge cases (offline users, rate limits)
Ready to build once requirements are confirmed.
```
---
## Integration with Other Workflows
Grasp is the **second phase** of the Double Diamond:
```
PROBE (Discover) → GRASP (Define) → TANGLE (Develop) → INK (Deliver)
```
**Typical flow:**
1. **Probe**: "Research authentication best practices" (discover options)
2. **Grasp**: "Define exact requirements for our auth system" (narrow down)
3. **Tangle**: "Implement the auth system" (build it)
4. **Ink**: "Validate the auth implementation" (deliver it)
Or use grasp standalone when requirements are unclear.
---
## Quality Checklist
Before completing grasp workflow, ensure:
- [ ] Core requirements clearly defined (must have, should have, nice to have)
- [ ] Technical constraints documented
- [ ] User needs understood and articulated
- [ ] Edge cases identified and documented
- [ ] Out-of-scope items explicitly listed
- [ ] Success criteria established
- [ ] Next steps recommended to user
- [ ] Full problem definition shared
---
## Cost Awareness
**External API Usage:**
- 🔴 Codex CLI uses your OPENAI_API_KEY (costs apply)
- 🟡 Antigravity CLI uses your AGY_AUTH_TOKEN (costs apply)
- 🧭 Antigravity CLI uses your Antigravity account/model configuration (costs may apply)
- 🔵 Claude analysis included with Claude Code
Grasp workflows typically cost $0.01-0.05 per task depending on complexity.
---
## Post-Definition: State Update
After definition completes:
1. Update `.octo/STATE.md` with completion
2. Populate `.octo/ROADMAP.md` with defined phases and success criteria
```bash
# Update state after Definition completion
"${HOME}/.claude-octopus/plugin/scripts/octo-state.sh" update_state \
--status "complete" \
--history "Define phase completed"
# Populate ROADMAP.md with defined requirements
if [[ -f "$SYNTHESIS_FILE" ]]; then
echo "📝 Updating .octo/ROADMAP.md with defined phases..."
"${HOME}/.claude-octopus/plugin/scripts/octo-state.sh" update_roadmap \
--from-synthesis "$SYNTHESIS_FILE"
fi
```
---
## Terminal State
The Define phase is complete ONLY when requirements are synthesized and the user has
approved the scope. After approval, invoke `flow-develop` if implementation is requested.
Otherwise, deliver the requirements document and stop. Do NOT begin
implementation from here without an approved scope.
**Ready to define!** This skill is used after explicit invocation when users request requirement clarification or problem definition.
@@ -0,0 +1,21 @@
MIT License
Copyright (c) 2026 nyldn
Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
@@ -0,0 +1,725 @@
---
name: flow-define
description: "Multi-AI requirements scoping using available external providers (Double Diamond Define phase)"
disable-model-invocation: true
---
> **Host: Codex CLI** — This skill was designed for Claude Code and adapted for Codex.
> Cross-reference commands use installed skill names in Codex rather than `/octo:*` slash commands.
> Use the active Codex shell and subagent tools. Do not claim a provider, model, or host subagent is available until the current session exposes it.
> For host tool equivalents, see `skills/blocks/codex-host-adapter.md`.
{{PREAMBLE}}
## Pre-Definition: State Check
Before starting definition:
1. Read `.octo/STATE.md` to verify Discover phase complete
2. Update STATE.md:
- current_phase: 2
- phase_position: "Definition"
- status: "in_progress"
```bash
# Verify Discover phase is complete
if [[ -f ".octo/STATE.md" ]]; then
discover_status=$("${HOME}/.claude-octopus/plugin/scripts/octo-state.sh" get_phase_status 1)
if [[ "$discover_status" != "complete" ]]; then
echo "⚠️ Warning: Discover phase not marked complete. Consider running discovery first."
fi
fi
# Update state for Definition phase
"${HOME}/.claude-octopus/plugin/scripts/octo-state.sh" update_state \
--phase 2 \
--position "Definition" \
--status "in_progress"
```
## Execution Contract
This skill uses **ENFORCED execution mode**. You MUST follow this exact sequence.
### STEP 1: Display Visual Indicators
**MANDATORY: You MUST use the native shell command tool to run this provider check BEFORE displaying the banner. Do NOT skip it. Do NOT assume availability.**
```bash
provider_check_output=$(bash "${HOME}/.claude-octopus/plugin/scripts/helpers/check-providers.sh")
provider_status_lines=$(printf '%s\n' "$provider_check_output" | awk '
/^PROVIDER_CHECK_START$/ { capture=1; next }
/^PROVIDER_CHECK_END$/ { capture=0 }
capture && /^[a-z0-9-]+:(available|missing|degraded)$/ { print }
')
if [[ -z "$provider_status_lines" ]]; then
echo "Provider availability check returned no usable status lines."
exit 1
fi
# Exclude providers intentionally disabled by environment, session, or global
# allowlist policy; show every allowed provider, including missing/degraded.
source "${HOME}/.claude-octopus/plugin/scripts/lib/provider-allowlist.sh"
provider_availability=""
available_provider_count=0
while IFS=: read -r provider status; do
octo_provider_allowed "$provider" || continue
case "$status" in
available)
provider_availability="${provider_availability}🟢 ${provider}: Available ✓"$'\n'
available_provider_count=$((available_provider_count + 1))
;;
degraded)
provider_availability="${provider_availability}🟠 ${provider}: Degraded ⚠"$'\n'
;;
missing)
provider_availability="${provider_availability}🔴 ${provider}: Missing/unavailable ✗"$'\n'
;;
esac
done <<< "$provider_status_lines"
if [[ "$available_provider_count" -eq 0 ]]; then
printf '%s' "$provider_availability"
echo "No external provider is available. Run /octo:setup and retry."
exit 1
fi
# Task status for the banner's Tasks line, if the session has one.
task_status=$("${HOME}/.claude-octopus/plugin/scripts/orchestrate.sh" get-task-status 2>/dev/null || echo "")
```
List every provider the check reports, not only Claude. A banner showing one seat when several ran misrepresents what the user is paying for.
If `OCTO_ALLOWED_PROVIDERS` is set, treat it as the source of truth for which providers may participate. Providers filtered out by that allowlist are intentionally reported as unavailable; do not invoke or recommend them in the workflow.
**Display this banner BEFORE orchestrate.sh execution:**
```
🐙 **CLAUDE OCTOPUS ACTIVATED** - Multi-provider definition mode
🎯 Define Phase: [Brief description of what you're defining/scoping]
📋 Session: ${CLAUDE_SESSION_ID}
📝 Tasks: ${task_status}
Provider Availability:
${provider_availability}
🔵 Claude: Available ✓ - Consensus building and synthesis
💰 Estimated Cost: $0.01-0.05
⏱️ Estimated Time: 2-5 minutes
```
**DO NOT PROCEED TO STEP 2 until banner displayed.** The banner shows users which providers will run and what costs they'll incur — starting API calls without this visibility violates cost transparency.
### STEP 2: Read Prior State
**Before executing the workflow, read any prior context:**
```bash
# Initialize state if needed
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" init_state
# Set current workflow
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" set_current_workflow "flow-define" "define"
# Get prior decisions (if any)
prior_decisions=$("${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" get_decisions "all")
# Get context from discover phase
discover_context=$("${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" get_context "discover")
# Display what you found (if any)
if [[ "$discover_context" != "null" ]]; then
echo "📋 Building on discovery findings:"
echo " $discover_context"
fi
if [[ "$prior_decisions" != "[]" && "$prior_decisions" != "null" ]]; then
echo "📋 Respecting prior decisions:"
echo "$prior_decisions" | jq -r '.[] | " - \(.decision) (\(.phase)): \(.rationale)"'
fi
```
**This provides context from:**
- Discovery phase research (if completed)
- Prior architectural decisions
- User vision captured earlier
- If **claude-mem** is installed, its MCP tools (`search`, `timeline`, `get_observations`) are available — use them to find past decisions on similar topics
**DO NOT PROCEED TO STEP 3 until state read.**
### STEP 3: Phase Discussion — Capture User Vision
**Before executing expensive multi-AI orchestration, capture the user's vision to scope the work effectively.**
**Ask clarifying questions using AskUserQuestion:**
```
Use AskUserQuestion tool to ask:
1. **User Experience**
Question: "How should users interact with this feature?"
Header: "User Flow"
Options:
- label: "API-first (programmatic access)"
description: "Build API endpoints first, UI later"
- label: "UI-first (user-facing interface)"
description: "Build user interface first, API supports it"
- label: "Both simultaneously"
description: "Develop API and UI in parallel"
- label: "Not applicable"
description: "This feature doesn't have a user interaction"
2. **Implementation Approach**
Question: "What technical approach do you prefer?"
Header: "Approach"
Options:
- label: "Fastest to market"
description: "Prioritize speed, use existing libraries"
- label: "Most maintainable"
description: "Focus on clean architecture, may take longer"
- label: "Best performance"
description: "Optimize for speed and efficiency"
- label: "Multi-LLM debate (Claude + available providers)"
description: "Multiple AI models debate the best approach — may use external provider credits or subscriptions"
3. **Scope Boundaries**
Question: "What's explicitly OUT of scope for this phase?"
Header: "Out of Scope"
Options:
- label: "Testing and QA"
description: "Focus on implementation, test later"
- label: "Performance optimization"
description: "Get it working first, optimize later"
- label: "Edge cases"
description: "Handle happy path only initially"
- label: "Nothing excluded"
description: "Everything is in scope"
multiSelect: true
```
**If user selected "Multi-LLM debate (Claude + available providers)" for approach:**
Before proceeding with orchestrate.sh, run a Multi-LLM debate to determine the technical approach:
```
/octo:debate --rounds 2 --debate-style collaborative "What is the best technical approach for [feature]? Consider: speed to market, maintainability, performance, and the existing codebase patterns."
```
Use the debate synthesis to set the approach context for the Define phase.
**After gathering answers, create context file:**
```bash
# Source context manager
source "${HOME}/.claude-octopus/plugin/scripts/context-manager.sh"
# Extract user answers from AskUserQuestion results
user_flow="[Answer from question 1]"
approach="[Answer from question 2]"
out_of_scope="[Answer from question 3]"
# Create context file with user vision
create_templated_context \
"define" \
"$(echo "$USER_REQUEST" | head -c 50)..." \
"User wants: $user_flow approach with $approach priority" \
"$approach" \
"Implementation of requested feature" \
"$out_of_scope"
echo "📋 Context captured and saved to .claude-octopus/context/define-context.md"
```
**This context will be used to:**
- Scope the multi-AI research (discover phase)
- Focus the requirements definition (define phase)
- Guide implementation decisions (develop phase)
- Validate against user expectations (deliver phase)
**DO NOT PROCEED TO STEP 4 until context captured.** User vision (UX approach, priorities, out-of-scope items) scopes the multi-AI research — without it, providers research too broadly and the definition misses the user's actual intent.
### STEP 4: Execute orchestrate.sh define
**You MUST execute this command via the native shell command tool:**
```bash
${HOME}/.claude-octopus/plugin/scripts/orchestrate.sh define "<user's clarification request>"
```
**CRITICAL: You are PROHIBITED from:**
- ❌ Defining requirements directly without calling orchestrate.sh — single-model analysis misses the technical, business, and external-model perspective split that provider fanout supplies, producing requirements with blind spots
- ❌ Using direct analysis instead of orchestrate.sh
- ❌ Claiming you're "simulating" the workflow
- ❌ Proceeding to Step 5 without running this command
**You MUST use the native shell command tool to invoke orchestrate.sh.**
#### What Users See During Execution (v7.16.0+)
If running in Claude Code v2.1.16+, users will see **real-time progress indicators** in the task spinner:
**Phase 1 - External Provider Execution (Parallel):**
- 🔴 Analyzing technical requirements (Codex)...
- 🟡 Clarifying user needs and context (Antigravity)...
**Phase 2 - Synthesis (Sequential):**
- 🔵 Building consensus on problem definition...
These spinner verb updates happen automatically - orchestrate.sh calls `update_task_progress()` before each agent execution. Users see exactly which provider is working and what it's doing.
**If NOT running in Claude Code v2.1.16+:** Progress indicators are silently skipped, no errors shown.
### STEP 5: Verify Execution
**After orchestrate.sh completes, verify it succeeded:**
```bash
# Find the latest synthesis file (created within last 10 minutes)
SYNTHESIS_FILE=$(find ~/.claude-octopus/results -name "grasp-synthesis-*.md" -mmin -10 2>/dev/null | head -n1)
if [[ -z "$SYNTHESIS_FILE" ]]; then
echo "❌ VALIDATION FAILED: No synthesis file found"
echo "orchestrate.sh did not execute properly"
exit 1
fi
echo "✅ VALIDATION PASSED: $SYNTHESIS_FILE"
cat "$SYNTHESIS_FILE"
```
**If validation fails:**
1. Report error to user
2. Show logs from `~/.claude-octopus/logs/`
3. DO NOT proceed with presenting results
4. DO NOT substitute with direct analysis — fallback to single-model analysis defeats the purpose of multi-provider consensus and produces narrower requirements
### STEP 6: Update State
**After synthesis is verified, record findings and decisions in state:**
```bash
# Extract key definition from synthesis
key_definition=$(head -50 "$SYNTHESIS_FILE" | grep -A 3 "## Problem Definition\|## Summary" | tail -3 | tr '\n' ' ')
# Record any architectural decisions made
# (You should identify these from the synthesis - e.g., tech stack, approach, patterns)
decision_made=$(echo "$key_definition" | grep -o "decided to\|chose to\|selected\|using [A-Za-z0-9 ]*" | head -1)
if [[ -n "$decision_made" ]]; then
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" write_decision \
"define" \
"$decision_made" \
"Consensus from multi-AI definition phase"
fi
# Update define phase context
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" update_context \
"define" \
"$key_definition"
# Update metrics
"${HOME}/.claude-octopus/plugin/scripts/state-manager.sh" update_metrics "phases_completed" "1"
```
**DO NOT PROCEED TO STEP 7 until state updated.**
### STEP 7: Present Problem Definition (Only After Steps 1-6 Complete)
Read the synthesis file and present:
- Core requirements (must have, should have, nice to have)
- Technical constraints
- User needs
- Edge cases to handle
- Out of scope items
- Perspectives from all providers
- Requirements checklist
- Next steps (usually tangle phase for implementation)
**Include attribution:**
```
*Multi-AI Problem Definition powered by Claude Octopus*
*Providers: available external providers + 🔵 Claude*
*Full problem definition: $SYNTHESIS_FILE*
```
# Define Workflow - Define Phase 🎯
<!-- Banner requirement lives in Execution Contract STEP 1 above. -->
## What This Workflow Does
The **define** phase clarifies and scopes problems using external CLI providers:
1. **🔴 Codex CLI** - Technical requirements analysis, edge cases, constraints
2. **🟡 Antigravity CLI** - User needs, business requirements, context understanding
3. **🧭 Antigravity CLI** - Additional external-model challenge
4. **🔵 Claude (You)** - Problem synthesis and requirement definition
This is the **convergent** phase after discovery - we narrow down from broad research to specific problem definition.
## When to Use Define
Use define when you need:
- **Requirement Definition**: "Define exactly what the auth system needs to do"
- **Problem Clarification**: "Clarify the caching requirements"
- **Scope Definition**: "What's the scope of the notification feature?"
- **Constraint Identification**: "What are the technical constraints for X?"
- **Edge Case Analysis**: "What edge cases do we need to handle for Y?"
- **Requirement Validation**: "Are these requirements complete for Z?"
**Don't use define for:**
- Research and exploration (use probe-workflow)
- Building implementations (use tangle-workflow)
- Code review and validation (use ink-workflow)
- Simple questions Claude can answer
## Visual Indicators
Before execution, you'll see a banner in this shape. The provider rows below are illustrative; replace them with every live status from Step 1 and omit only providers excluded by the active allowlist:
```text
🐙 **CLAUDE OCTOPUS ACTIVATED** - Multi-provider problem definition
🎯 Define Phase: Clarifying requirements and scope
📋 Session: ${CLAUDE_SESSION_ID}
📝 Tasks: ${task_status}
Provider Availability:
🟢 codex: Available ✓
🧭 agy: Degraded ⚠
🔴 agy: Missing/unavailable ✗
🔵 Claude: Available ✓ - Consensus building and synthesis
```
## How It Works
### Stage A: Invoke Grasp Phase
```bash
${HOME}/.claude-octopus/plugin/scripts/orchestrate.sh define "<user's clarification request>"
```
### Stage B: Multi-Provider Problem Definition
The orchestrate.sh script will:
1. Call **Codex CLI** for technical requirement analysis
2. Call **Antigravity CLI** for business/user need analysis
3. Call **Antigravity CLI** for additional external-model challenge
4. You (Claude) synthesize into clear problem definition
5. Identify gaps and missing requirements
### Stage C: Read Results
Results are saved to:
```text
~/.claude-octopus/results/${SESSION_ID}/grasp-synthesis-<timestamp>.md
```
### Stage D: Present Problem Definition
Read the synthesis and present clear, actionable requirements to the user.
## Implementation Instructions
When this skill is invoked, follow the EXECUTION CONTRACT above exactly. The contract includes:
1. **Blocking Step 1**: Check providers and display live visual indicators
2. **Blocking Step 2**: Read prior workflow state
3. **Blocking Step 3**: Capture and persist the user's vision
4. **Blocking Step 4**: Execute orchestrate.sh define via native shell command tool
5. **Blocking Step 5**: Verify the synthesis file exists
6. **Blocking Step 6**: Update workflow state
7. **Step 7**: Present the formatted problem definition
Each step is **mandatory and blocking** - you cannot proceed to the next step until the current one completes successfully.
### Task Management Integration
Create tasks to track execution progress:
```javascript
// At start of skill execution
TaskCreate({
subject: "Execute define workflow with multi-AI providers",
description: "Run orchestrate.sh define for problem clarification",
activeForm: "Running multi-AI define workflow"
})
// Mark in_progress when calling orchestrate.sh
TaskUpdate({taskId: "...", status: "in_progress"})
// Mark completed ONLY after synthesis file verified
TaskUpdate({taskId: "...", status: "completed"})
```
### Error Handling
If any step fails:
- **Step 1 (Providers)**: If all external providers are unavailable, suggest `/octo:setup` and STOP
- **Step 4 (orchestrate.sh)**: Show bash error, check logs, report to user
- **Step 5 (Validation)**: If synthesis is missing, show orchestrate.sh logs and do not substitute direct analysis
Never fall back to direct analysis if orchestrate.sh execution fails. Report the failure and let the user decide how to proceed.
### Problem Definition Format
After successful execution, present problem definition with:
```
# Problem Definition: <task>
## Core Requirements
[Essential requirements that must be met]
## Technical Constraints
[Technical limitations and considerations]
## User Needs
[What users actually need from this]
## Edge Cases to Handle
[Specific edge cases identified]
## Out of Scope
[What is explicitly NOT included]
## Perspectives
### Codex Analysis (Technical)
[Technical requirements and constraints]
### Antigravity Analysis (Business/User)
[User needs and business context]
### Claude Synthesis
[Integrated problem definition]
## Requirements Checklist
- [ ] Functional requirements defined
- [ ] Non-functional requirements defined
- [ ] Technical constraints identified
- [ ] Edge cases documented
- [ ] Out-of-scope items clarified
- [ ] Success criteria established
## Next Steps
[Recommended next actions - usually tangle phase for implementation]
Full problem definition saved to: <synthesis file path>
```
## Example Usage
### Example 1: Define Auth Requirements
```
User: Define the exact requirements for our user authentication system
Claude:
🐙 **CLAUDE OCTOPUS ACTIVATED** - Multi-provider problem definition
🎯 Define Phase: Clarifying authentication requirements
[Executes: ${HOME}/.claude-octopus/plugin/scripts/orchestrate.sh define "Define exact requirements for user authentication system"]
[After completion, reads synthesis and presents:]
# Problem Definition: User Authentication System
## Core Requirements
### Must Have (P0)
1. User registration with email + password
2. Secure login with JWT token generation
3. Password reset via email
4. Session management with token refresh
5. Logout (token invalidation)
### Should Have (P1)
6. Remember me functionality
7. Account lockout after failed attempts
8. Email verification on signup
9. Password strength requirements
### Nice to Have (P2)
10. OAuth social login (Google, GitHub)
11. Two-factor authentication
12. Single sign-on (SSO)
## Technical Constraints
- Must work with existing Express.js backend
- Token storage: Redis for production, in-memory for dev
- Password hashing: bcrypt (min 12 rounds)
- Token expiry: Access token 15min, refresh token 7 days
- Rate limiting: Max 5 login attempts per 15min per IP
## User Needs
- Simple registration process (< 2 minutes)
- Secure password recovery without support tickets
- Stay logged in across browser sessions (remember me)
- Clear error messages for failed login
- Ability to log out from all devices
## Edge Cases to Handle
1. **Concurrent logins**: Same user, multiple devices
2. **Expired tokens**: Graceful refresh without re-login
3. **Password reset race condition**: Multiple reset requests
4. **Account enumeration**: Don't reveal if email exists
5. **Token theft**: Detect suspicious login locations
6. **Deleted users**: Handle auth for soft-deleted accounts
## Out of Scope
- ❌ Biometric authentication (fingerprint, face ID)
- ❌ LDAP/Active Directory integration
- ❌ Custom OAuth provider implementation
- ❌ Device fingerprinting
- ❌ Passwordless email magic links
## Perspectives
### Codex Analysis (Technical)
- JWT structure: { userId, role, exp, iat, jti }
- Database schema for users, refresh_tokens, password_resets
- API endpoints: POST /register, POST /login, POST /logout, POST /refresh, POST /reset-password
- Middleware for auth verification
- Security: HTTPS only, secure cookies, CSRF protection
### Antigravity Analysis (Business/User)
- User journey: Registration -> Email verification -> Login -> Access app
- Error handling: Clear messages without security leaks
- Performance: Auth checks < 50ms
- Compliance: GDPR (data deletion), password policies
- Analytics: Track signup conversion, failed login rates
### Claude Synthesis
- Hybrid approach: Core auth (P0) first, iterate on P1/P2
- Security-first: All requirements validated against OWASP
- User experience: Balance security with convenience
- Scalable: Design for 100K users, plan for 1M+
## Requirements Checklist
- ✅ Functional requirements defined (registration, login, reset)
- ✅ Non-functional requirements defined (performance, security)
- ✅ Technical constraints identified (Express, Redis, bcrypt)
- ✅ Edge cases documented (6 critical cases)
- ✅ Out-of-scope items clarified (4 items)
- ✅ Success criteria established (< 2min registration, < 50ms auth)
## Next Steps
1. **Immediate**: Review and confirm requirements with stakeholders
2. **Then**: Use **tangle-workflow** to implement the auth system
3. **Finally**: Use **ink-workflow** to validate implementation
Ready to proceed to implementation?
Full problem definition saved to: ~/.claude-octopus/results/abc-123/grasp-synthesis-20250118-144530.md
```
### Example 2: Clarify Feature Scope
```
User: What exactly does the notification feature need to do?
Claude:
🐙 **CLAUDE OCTOPUS ACTIVATED** - Multi-provider problem definition
🎯 Define Phase: Clarifying notification requirements
[Executes grasp workflow]
[Presents detailed problem definition with:]
- Core notification types (email, push, in-app)
- Delivery requirements (real-time vs batched)
- User preferences (opt-in/out, frequency)
- Technical constraints (message queue, delivery tracking)
- Edge cases (offline users, rate limits)
Ready to build once requirements are confirmed.
```
## Integration with Other Workflows
Grasp is the **second phase** of the Double Diamond:
```
PROBE (Discover) → GRASP (Define) → TANGLE (Develop) → INK (Deliver)
```
**Typical flow:**
1. **Probe**: "Research authentication best practices" (discover options)
2. **Grasp**: "Define exact requirements for our auth system" (narrow down)
3. **Tangle**: "Implement the auth system" (build it)
4. **Ink**: "Validate the auth implementation" (deliver it)
Or use grasp standalone when requirements are unclear.
## Quality Checklist
Before completing grasp workflow, ensure:
- [ ] Core requirements clearly defined (must have, should have, nice to have)
- [ ] Technical constraints documented
- [ ] User needs understood and articulated
- [ ] Edge cases identified and documented
- [ ] Out-of-scope items explicitly listed
- [ ] Success criteria established
- [ ] Next steps recommended to user
- [ ] Full problem definition shared
## Cost Awareness
**External API Usage:**
- 🔴 Codex CLI uses your OPENAI_API_KEY (costs apply)
- 🟡 Antigravity CLI uses your AGY_AUTH_TOKEN (costs apply)
- 🧭 Antigravity CLI uses your Antigravity account/model configuration (costs may apply)
- 🔵 Claude analysis included with Claude Code
Grasp workflows typically cost $0.01-0.05 per task depending on complexity.
## Post-Definition: State Update
After definition completes:
1. Update `.octo/STATE.md` with completion
2. Populate `.octo/ROADMAP.md` with defined phases and success criteria
```bash
# Update state after Definition completion
"${HOME}/.claude-octopus/plugin/scripts/octo-state.sh" update_state \
--status "complete" \
--history "Define phase completed"
# Populate ROADMAP.md with defined requirements
if [[ -f "$SYNTHESIS_FILE" ]]; then
echo "📝 Updating .octo/ROADMAP.md with defined phases..."
"${HOME}/.claude-octopus/plugin/scripts/octo-state.sh" update_roadmap \
--from-synthesis "$SYNTHESIS_FILE"
fi
```
## Terminal State
The Define phase is complete ONLY when requirements are synthesized and the user has
approved the scope. After approval, invoke `flow-develop` if implementation is requested.
Otherwise, deliver the requirements document and stop. Do NOT begin
implementation from here without an approved scope.
**Ready to define!** This skill is used after explicit invocation when users request requirement clarification or problem definition.