diff --git a/.claude/hooks/check-architecture-rules.js b/.claude/hooks/check-architecture-rules.js new file mode 100755 index 000000000..b50d0867b --- /dev/null +++ b/.claude/hooks/check-architecture-rules.js @@ -0,0 +1,135 @@ +#!/usr/bin/env node + +/** + * PreToolUse guard: refuses a Write, Edit or MultiEdit that would leave a plugin's dispatch + * surface breaking one of the two architecture rules of issue #250. The rules live in + * `scripts/lib/architecture-rules.js` and their filesystem layer in `architecture-scan.js`; + * this script only reconstructs the unwritten content and speaks the host's refusal. + * + * It is the fast path, not the gate: `scripts/check-architecture-rules.js` runs on every commit, + * whichever tool made the edit. This one only ever sees Claude Code, and it fails open at every + * step — an unreadable payload, an unknown tool, an edit it cannot reconstruct or a crash lets + * the write through rather than halting unrelated work. + */ + +const fs = require("node:fs"); +const path = require("node:path"); + +const WRITE_TOOLS = new Set(["Write", "Edit", "MultiEdit"]); +const PROCEED = 0; + +function architecture() { + try { + const lib = path.resolve(__dirname, "..", "..", "scripts", "lib"); + return { + ...require(path.join(lib, "architecture-scan.js")), + classifyFile: require(path.join(lib, "architecture-rules.js")).classifyFile, + }; + } catch { + return null; + } +} + +function payloadFromStdin() { + try { + const parsed = JSON.parse(fs.readFileSync(0, "utf8")); + return parsed && typeof parsed === "object" ? parsed : null; + } catch { + return null; + } +} + +function repoRelative(absolutePath, root) { + const relative = path.relative(root, absolutePath); + if (relative === "" || relative.startsWith("..") || path.isAbsolute(relative)) return null; + return relative.split(path.sep).join("/"); +} + +function readFile(absolutePath) { + try { + return fs.readFileSync(absolutePath, "utf8"); + } catch { + return null; + } +} + +/** + * Splices literally. `String.prototype.replace` reads `$&`, `` $` ``, `$'` and `$1` in its + * replacement even when the pattern is a plain string, which corrupts a `new_string` holding one. + */ +function spliced(content, { old_string: oldString, new_string: newString, replace_all: replaceAll }) { + if (typeof oldString !== "string" || typeof newString !== "string") return null; + if (!content.includes(oldString)) return null; + if (replaceAll) return content.split(oldString).join(newString); + + const at = content.indexOf(oldString); + return content.slice(0, at) + newString + content.slice(at + oldString.length); +} + +function editedContent(absolutePath, edits) { + let content = readFile(absolutePath); + if (content === null) return null; + + for (const edit of edits) { + if (!edit || typeof edit !== "object") return null; + content = spliced(content, edit); + if (content === null) return null; + } + return content; +} + +/** What the file will hold if this call goes through, or null when that cannot be determined. */ +function prospectiveContent(toolName, toolInput, absolutePath) { + if (toolName === "Write") { + return typeof toolInput.content === "string" ? toolInput.content : null; + } + if (toolName === "Edit") return editedContent(absolutePath, [toolInput]); + + const { edits } = toolInput; + if (!Array.isArray(edits) || edits.length === 0) return null; + return editedContent(absolutePath, edits); +} + +function refuse(violations, describeFix) { + const reason = violations.map((v) => `${v.message}. Fix: ${describeFix(v)}.`).join("\n"); + process.stdout.write( + JSON.stringify({ + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: reason, + }, + }) + ); +} + +function main() { + const rules = architecture(); + const payload = payloadFromStdin(); + if (!rules || !payload) return PROCEED; + + const { tool_name: toolName, tool_input: toolInput } = payload; + if (!WRITE_TOOLS.has(toolName)) return PROCEED; + if (!toolInput || typeof toolInput.file_path !== "string" || toolInput.file_path === "") return PROCEED; + + // Resolved against the project root, never the process cwd: the two can differ, and a readdir + // against the wrong one comes back empty instead of failing. + const root = process.env.CLAUDE_PROJECT_DIR || process.cwd(); + const absolutePath = path.resolve(root, toolInput.file_path); + const relativePath = repoRelative(absolutePath, root); + if (!relativePath || !rules.classifyFile(relativePath)) return PROCEED; + + const content = prospectiveContent(toolName, toolInput, absolutePath); + if (content === null) return PROCEED; + + const violations = rules.violationsForFile(relativePath, content, absolutePath); + if (violations.length > 0) refuse(violations, rules.describeFix); + + return PROCEED; +} + +try { + process.exitCode = main(); +} catch { + process.exitCode = PROCEED; +} diff --git a/.claude/settings.json b/.claude/settings.json index 1850d4990..61f2e989b 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -11,6 +11,18 @@ ] } ], + "PreToolUse": [ + { + "matcher": "Write|Edit|MultiEdit", + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/check-architecture-rules.js\"", + "timeout": 60 + } + ] + } + ], "PostToolUse": [ { "matcher": "Edit|Write|MultiEdit", diff --git a/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/backlog-link.json b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/backlog-link.json new file mode 100644 index 000000000..75e33474d --- /dev/null +++ b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/backlog-link.json @@ -0,0 +1,5 @@ +{ + "backlog": "ai-driven-dev/framework#250", + "written_at": "2026-09-18T09:24:15Z", + "written_by": "aidd-pm:04-spec" +} diff --git a/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/phase-1.md b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/phase-1.md new file mode 100644 index 000000000..61e8d98f7 --- /dev/null +++ b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/phase-1.md @@ -0,0 +1,106 @@ + +# Instruction: The two rules, as a tested engine + +## Architecture projection + +> Tree of the final files. ✅ create · ✏️ modify · ❌ delete + +```txt +. +├── scripts +│ ├── lib +│ │ └── architecture-rules.js ✅ the two rules, as pure functions +│ └── __tests__ +│ ├── architecture-rules.test.js ✅ both rules, both directions, plus the clean-tree sweep +│ └── fixtures +│ └── architecture-rules ✅ synthetic plugin trees, written for this task +└── docs + └── ARCHITECTURE.md ✏️ one line pointing at the guard that now enforces the rule +``` + +## User Journey + +```mermaid +flowchart TD + A[A contributor edits a plugin source] --> B{Does the prospective content break a rule?} + B -- no --> C[Nothing is said] + B -- "a sibling plugin is addressed" --> D[File, line, owning plugin] + B -- "the Actions section is out of step" --> E[File, line, the action that does not match] + D --> F[The contributor corrects it and edits again] + E --> F +``` + +## Test Scope + +```mermaid +--- +title: Test scope +--- +journey + section Setup + Write the synthetic plugin fixtures under the test fixtures directory => fixtures on disk: 5: system + section Happy path + Run the rule engine over a fixture whose skill addresses only its own plugin => no violation: 5: cli + Run the rule engine over the repository's own plugins directory => no violation: 5: cli + section Edge case - a recipe skill addresses a sibling + A skill file names another plugin's skill => run the engine over it => one violation naming the file, the line and the owning plugin: 1: cli + section Edge case - a permission list addresses a sibling + An agent lists a sibling skill under its permission heading => run the engine over it => no violation: 1: cli + section Edge case - an orchestration reference addresses a sibling + An orchestrator reference names a provider => run the engine over it => no violation: 1: cli + section Edge case - an action file no section names + A skill gains an action file its Actions section never mentions => run the engine over it => one violation naming the file: 1: cli + section Edge case - a section whose mention is only prose + A word in the section's prose matches an action's stem but no row cites it => run the engine over it => one violation naming the file: 1: cli +``` + +## Tasks to do + +### `1)` The failing tests come first + +> Every rule gets its red before it gets its engine. + +1. Write the synthetic fixtures: a plugin tree with a clean skill, a skill addressing a sibling, an agent permission list addressing a sibling, an orchestrator reference addressing a sibling, a skill with an unnamed action file, a skill citing an absent action. +2. Write `architecture-rules.test.js` covering each fixture and each expected verdict, plus one case that sweeps the repository's real `plugins/` and expects zero violations. +3. Run the suite and watch every case fail for the absence of the module, not for a typo. + +### `2)` The orthogonality rule + +> A dispatch surface never names a sibling plugin. + +1. Expose a function taking a repository-relative path and the prospective content, returning violations with a line, a 1-indexed number, the address found and the plugin that owns it. +2. Match `/:` and `@:` addresses; a match whose plugin equals the file's own owner is not a violation. +3. Govern only `SKILL.md`, `actions/*.md`, `references/*.md` and `agents/*.md`. Anything under `assets/` returns nothing. +4. Exempt a file owned by an orchestrator plugin, and exempt the lines under an agent's `# Skills you may invoke` heading. + +### `3)` The router coherence rule + +> An Actions section cites every action that exists. + +1. Take the prospective `SKILL.md` content and the names of the skill's action files. +2. Isolate the `## Actions` section, up to the next second-level heading. +3. Report an action file the section cites by neither its stem, its file name, nor an `actions/.md` path. Collect a citation from the table's action column, never from running prose. +4. Report the line of the section heading. A citation with no file behind it is deliberately not reported — it cannot be told apart from one written just before the file it names. + +### `4)` The rule the repository already follows + +> A guard that is red on a clean tree is a guard nobody keeps. + +1. Run the sweep case over the real `plugins/` tree. +2. When a rule fires there, the rule is wrong, not the tree. Narrow it and record why in `plan.md`'s decisions. + +### `5)` The rule document points at its enforcement + +> `docs/ARCHITECTURE.md` states the rule; say where it is now checked. + +1. Add one sentence naming the guard, plugin-relative and in backticks, never as a link. + +## Test acceptance criteria + +| Task | Acceptance criteria | +| --- | --- | +| 1 | Every case in the suite fails before the engine exists, each for the missing module | +| 2 | A skill addressing a sibling yields a violation naming file, line and owning plugin; an agent permission list and an orchestrator reference yield none; a file under `assets/` yields none | +| 3 | An action file the section never cites yields one violation; a stem appearing only in prose does not count as a citation; a skill whose section and files agree yields none | +| 4 | Sweeping the repository's own `plugins/` yields zero violations | +| 5 | `docs/ARCHITECTURE.md` names the guard, and the markdown-link check still passes | diff --git a/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/phase-2.md b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/phase-2.md new file mode 100644 index 000000000..27c71e1f2 --- /dev/null +++ b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/phase-2.md @@ -0,0 +1,103 @@ + +# Instruction: The refusal, at the AI host's write moment + +## Architecture projection + +> Tree of the final files. ✅ create · ✏️ modify · ❌ delete + +```txt +. +├── .claude +│ ├── hooks +│ │ └── check-architecture-rules.js ✅ reads the pending edit, calls the engine, refuses +│ └── settings.json ✏️ one PreToolUse entry matching Write and Edit +└── scripts + └── __tests__ + └── architecture-hook.test.js ✅ the payload shapes and the refusal contract +``` + +## User Journey + +```mermaid +flowchart TD + A[An agent calls Write or Edit on a plugin source] --> B[PreToolUse hands the hook the pending content] + B --> C{Does the prospective content break a rule?} + C -- no --> D[The write proceeds] + C -- yes --> E[The call is denied, with file, line and owning plugin] + E --> F[The agent corrects the content and calls again] + F --> B +``` + +## Test Scope + +```mermaid +--- +title: Test scope +--- +journey + section Setup + Build a PreToolUse payload for a plugin file => payload on stdin: 5: system + section Happy path + Run the hook on a payload whose content breaks no rule => exit zero and no output: 5: cli + section Edge case - a Write that introduces a sibling address + The payload carries content addressing another plugin => run the hook => a deny decision naming file, line and owning plugin: 1: cli + section Edge case - an Edit that introduces a sibling address + The payload carries an old and a new string => run the hook => the reconstructed content is judged, and the call is denied: 1: cli + section Edge case - an Edit whose old string is absent + The payload cannot be reconstructed => run the hook => exit zero, because the edit will fail on its own: 1: cli + section Edge case - a file outside the governed surface + The payload names a file under assets => run the hook => exit zero: 1: cli + section Teardown + Remove the temporary payload files => baseline restored: 5: system +``` + +## Tasks to do + +### `1)` The hook, generated by the capability that owns hooks + +> The decider asked for a host-native lifecycle hook produced by the hook capability, not a hand-placed script. + +1. Run `/aidd-context:08-hook-generate` for a `PreToolUse` hook matching `Write|Edit`, at the project scope, for Claude Code — the one host this repository configures hooks for, per the plan's decisions. +2. Keep the generated entry and script; give the script the name the projection states. + +### `2)` The prospective content + +> Judge what the file will hold, not what it holds now. + +1. Read the payload from standard input. Take `tool_input.file_path`. +2. For a `Write`, the prospective content is `tool_input.content`. +3. For an `Edit`, read the file and apply `old_string` to `new_string`, once or everywhere per `replace_all`. +4. When the file path is outside the governed surface, or the reconstruction fails, exit zero and say nothing. + +### `3)` The refusal + +> A refusal a reader cannot act on is noise. + +1. Call the engine with the path, the prospective content, and the skill's action file names when the path is a `SKILL.md`. +2. On a violation, emit the `PreToolUse` deny decision, with a reason naming each file, line, and the plugin that owns the address, and what to write instead. +3. On no violation, exit zero with no output. + +### `4)` The failing tests come first + +> The hook is a contract with the host; test it as one. + +1. Write `architecture-hook.test.js` driving the hook as a subprocess with each payload shape. +2. Watch each case fail before the hook exists. +3. Assert the decision is a deny and the reason carries the file and the line, never only the rule. + +### `5)` The proof + +> A guard nobody watched fire is a comment. + +1. Write a plugin file addressing a sibling, through the agent's own Write tool, and record that the call was refused and what it said. +2. Write an agent permission list naming a sibling the same way, and record that it was applied. + +## Test acceptance criteria + +| Task | Acceptance criteria | +| --- | --- | +| 1 | The hook entry exists in `.claude/settings.json` at the project scope and fires on `Write` and `Edit` | +| 2 | A `Write` payload and an `Edit` payload over the same file yield the same verdict for the same resulting content | +| 3 | A denied call returns a reason naming file, line and owning plugin; a clean call returns nothing at all | +| 4 | Every case fails before the hook exists, then passes | +| 5 | An attempted write of a sibling address is refused in the same turn; an attempted write of a permission list is applied | diff --git a/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/plan.md b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/plan.md new file mode 100644 index 000000000..03e781163 --- /dev/null +++ b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/plan.md @@ -0,0 +1,48 @@ + +# Plan: Architecture guard on AI-authored edits + +## Overview + +| Field | Value | +| --- | --- | +| **Goal** | Refuse an AI edit that would hardcode a sibling plugin's address, or leave a skill's `## Actions` section out of step with its action files, before the edit lands. | +| **Source** | [`spec.md`](./spec.md), from [ai-driven-dev/framework#250](https://github.com/ai-driven-dev/framework/issues/250) | + +## Phases + +| # | Phase | File | +| --- | --- | --- | +| 1 | The two rules, as a tested engine | [`phase-1.md`](./phase-1.md) | +| 2 | The refusal, at the AI host's write moment | [`phase-2.md`](./phase-2.md) | + +## Resources + +| Source | Verified | +| --- | --- | +| `docs/ARCHITECTURE.md`, capability addressing | A capability is addressed only where dispatch is declared: a router's `## Actions` table, an agent's `# Skills you may invoke` list. Agent permission lists and orchestration references legitimately name a provider; recipe skills never do. | +| | `PreToolUse` receives `tool_input` before the tool runs — `content` for `Write` — and refuses the call with `hookSpecificOutput.permissionDecision: "deny"` plus a `permissionDecisionReason` the model reads. `PostToolUse` cannot refuse, it only reports after the fact. | +| `.claude/hooks/check-written-file.js` | The in-repo precedent for a hook that reads the payload from stdin, resolves the written file, and hands a report back in the same turn. | +| Issue #250, comment of 2026-09-14 | Supersedes the issue body's "Guardrail local et CI": no Git hook, no CI gate, synthetic fixtures, #406 neither blocker nor fixture. | +| Probe over the 51 skills and 8 plugins in the tree | Both rules, as scoped below, report zero violations on the repository as it stands — orthogonality across 37 real cross-plugin addresses, router coherence across 48 skills that hold action files. The probe measured both directions; only the decidable one ships, per the decision below. | + +## Decisions + +| Decision | Why | +| --- | --- | +| `PreToolUse`, not `PostToolUse` | The decision is "prevent the write until it is corrected". `PostToolUse` fires after the file is already on disk; Biome gets away with it only because it rewrites in place, and this guard cannot rewrite prose. | +| The engine is a plain module, the hook is a thin caller | A rule that is a pure function of (path, prospective content, action-file listing) is testable without a hook, a host, or a tree. The hook contributes only the payload and the refusal. | +| The governed surface is `SKILL.md`, `actions/`, `references/`, `agents/` | That is where dispatch is declared. `assets/` hold sheets a reader reads — `12-cook`'s recipes name 20 cross-plugin commands on purpose — so they are excluded by a stated rule, never by a quiet path filter. | +| An orchestrator plugin is exempt from rule one, and only rule one | `docs/ARCHITECTURE.md` makes orchestration references responsibility maps. `aidd-orchestrator` holds 14 of the tree's cross-plugin addresses for exactly that reason. Router coherence is a different rule and applies to it like any other plugin. | +| The router rule reads the `## Actions` section, not the table | `10-todo` names its one action as a path in a fenced block rather than a table row. Scoping to the section and matching either the stem or the file name covers both shapes and still reports zero on the tree. | +| Unit tests live under `scripts/__tests__/`, and that is not a CI gate on the rules | Pre-commit runs those tests against synthetic fixtures, proving the engine works. Nothing scans the tree at commit time or in CI, which is what the decider ruled out. | +| Fixtures are written for this task | The spec forbids #406's historical code. Each rule gets a breaking fixture and a legitimate-naming fixture, so a guard that flags a permission list fails its own suite. | +| The hook is wired for Claude Code alone | It is the only host this repository configures hooks for: `.codex/config.toml` carries a sandbox mode and nothing else. Codex, Cursor and Copilot all expose `PreToolUse` and the same deny shape, but Codex delivers a file edit as an `apply_patch` command string rather than a path and a content, which is a different parse. The engine is host-agnostic, so each adapter is additive and none of them touches a rule. | +| Rule two enforces one direction, not two | A citation with no file behind it cannot be told apart from a citation written just before the file it names. Enforcing it deadlocked adding an action: creating the file first was refused for not being named, naming it first was refused for having no file, and `aidd-context:04-skill-generate` documents the second order. The decidable direction is kept. | +| An action is named by a citation, never by a word | Plain containment over the section let ordinary prose pass for a mention: `plan` appears in "the plan is the culmination", so deleting that action's row went unnoticed. Measured over the tree, containment missed 20 of 78 row deletions; citation matching misses none that has a row. | +| One plugin source was repaired, and it is the exception | `plugins/aidd-dev/skills/01-plan/actions/04-plan.md:16` read "declare it the same way `aidd-pm:04-spec` does", which is rule one's own violation in prose. The spec's non-goal keeps existing violations out of scope, and the hard constraint says a rule red on the tree is miscalibrated — so the one line that was genuinely wrong was fixed, and the seven in `00-onboard` were exempted with #883 behind them instead. No other plugin source is touched. | +| A stem speaks for its action only when no sibling shares it | `01-plan.md` and `04-plan.md` both reduce to `plan`, so one row would cover both and deleting either would go unnoticed. A shared stem cites nobody; the numbered name still does. No skill in the tree has a collision today, which is why it had to be reasoned about rather than observed. | +| A fenced block is an example, never a router | The section scan blanks fences before reading headings and tables, so a documented `## Actions`, a `##` inside the section, and an example table all stop steering the verdict. Path citations are read from the unblanked lines, because `10-todo` legitimately cites `actions/01-todo.md` inside a fence. | +| Only the path shape reads through a fence | Both shapes read the unblanked lines at first, which let a backticked `.md` inside a fenced example cite — the very hole blanking fences was for. `10-todo` needs the path shape and nothing needs the other, so the exception is one shape wide. | +| A table resumed after a blank line keeps its column | A run of rows with no `| --- |` beneath it is read as the table above it, resumed. Otherwise inserting one blank line — changing no content — refuses every row below it. The cost: a separator-less glossary written just under the router donates its cells, so a deleted row could hide behind it. Neither shape is a table a renderer accepts, and the false refusal is the worse failure. A column-count check does not separate them: a glossary often has the router's width. | +| A separator row counts from two dashes | `aidd-context:00-onboard` writes `| -- |` and GitHub renders it. Requiring three silently stopped the header being recognised, which unnamed every action in that skill. The real-tree sweep caught it. | +| A fence nobody closed is not a fence | Blanking from an unterminated fence to end of file hides a `## Actions` that is visibly there, and the refusal then reads "has action files but no ## Actions section" — unactionable. The whole document is read as unfenced instead. | diff --git a/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/spec.md b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/spec.md new file mode 100644 index 000000000..9addb4bbf --- /dev/null +++ b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/spec.md @@ -0,0 +1,58 @@ +# Architecture guard on AI-authored edits + +## Target + +An AI-authored edit that would break one of this repository's two named architecture rules is refused before it lands, with the offending file and line named. + +## Hard constraints + +- The guard decides before the edit is applied and refuses it. It is not a commit-time gate and not a continuous-integration job. +- The guard is deterministic: the same prospective file content yields the same verdict, with no dependence on model judgement and none on which AI tool made the edit. +- The guard is produced by the project's own hook-generation capability, so it exists for the AI hosts this repository configures rather than for one. +- Rule one, cross-plugin orthogonality: a plugin's dispatch surface must not name a sibling plugin by a hardcoded address. +- Rule two, router coherence: a skill's `## Actions` section must cite every action file that skill provides. A citation is a table cell, a fenced `actions/.md` path, or a backticked file name — never a bare word in running prose. +- The governed surface is the dispatch surface: a skill's `SKILL.md`, its actions and its references, and an agent's definition. A plugin's `assets/` hold content shown to a reader, not dispatch, and are out of the guard's reach by this rule rather than by an unstated path filter. +- Naming that `docs/ARCHITECTURE.md` declares legitimate stays silent: an agent's permission list and an orchestration reference are responsibility maps and name their provider canonically. Flagging either is a defect of the guard, not of the tree. +- Every refusal names the file, the line, and the plugin that owns the addressed capability, in terms a reader can act on without opening the rule document. +- The guard is silent on the repository as it stands. A fenced block holds an example, never a router: a `## Actions` inside one is not the section, a `##` inside one does not end it, and a table inside one cites nothing. A fenced `actions/.md` path is the one exception, because `aidd-dev:10-todo` cites its only action that way — so a fenced example writing that shape does cite, and can mask a row deleted elsewhere. Two more shapes are read generously and stay that way: a run of table rows with no separator under it is taken for the table above it resumed after a blank line, so a separator-less glossary written just below the router would donate its cells; and rule one reads addresses through fences, so a governed file teaching a counter-example is refused. Neither shape is a table a renderer accepts, and a refusal nobody can act on is the worse failure. A rule that reports a violation in the current tree is miscalibrated, not vindicated. +- The rules are exercised by purpose-built fixtures. No historical code from #406 is used as a fixture. +- Each rule is proved by a fixture that breaks it and turns exactly the test named for that rule red, and by a fixture of legitimate naming that stays green. + +## Non-goals + +- A `lefthook` pre-commit gate for these rules. The decider ruled it out on 2026-09-14. +- A CI job enforcing these rules. +- Judging whether a skill's prose `description` has gone stale. Nothing mechanically separates stale prose from current prose, so the issue's "router/description mismatch" is served by the router half alone. +- Refusing a citation with no action file behind it. A citation written seconds before the file it names is indistinguishable from a stale one, and this project's own skill generator writes the router first, so enforcing that direction makes adding an action impossible in either order. The decidable direction — an action file the section never cites — is the one the issue names, and the one enforced. +- Catching an action file created and never cited. Rule two decides on a write to a `SKILL.md`; an orphan action file is caught at the next write to its skill's router, not at its own creation. Firing on the action file is what broke the order that creates the file first, and dropping it was the cheaper half of breaking the deadlock. The window is unbounded: that next write may never come, so this is a hole, not a delay. +- Reaching into a plugin's `README.md`, which addresses a sibling in two places today. Like `assets/`, it is a document a reader reads, not a dispatch the skill executes. +- Reaching into a plugin's `assets/`. A recipe sheet names the commands a reader types; that is its subject, not a dispatch this rule governs. +- Repairing violations that exist in the tree today. That was #406, now closed. One line is the stated exception, named in the plan's decisions: a prose sentence in `aidd-dev`'s planning action addressed a sibling skill outright, and leaving it would have made the guard red on its own tree. +- Enforcing any architecture rule beyond the two named above. +- Catching a violation introduced outside an AI tool, by a human editing by hand. +- Catching a write performed through a shell command rather than a write tool. Deciding whether a shell line writes a plugin source means parsing arbitrary shell, which is not the deterministic verdict rule one requires. An agent told to edit through `sed` or a heredoc is therefore ungoverned, and that is stated here rather than left to be discovered. +- Making `aidd-context:00-onboard` resolve its providers at runtime. Its reference menus name addresses because those addresses are what it hands a person to type, so it is exempt by a named rule with a follow-up issue behind it, not by silence. +- Shipping the guard into projects that install this marketplace. The rules govern this repository's own plugin sources. + +## Done-when + +- An edit that would leave a skill's dispatch surface holding a sibling plugin's hardcoded address is refused, and the refusal names that file, its line, and the plugin that owns the address. +- An edit that would leave a skill's `## Actions` section not naming an action file the skill provides is refused, and the refusal names the file and the line. The section names an action by citing it, so a word in prose that happens to match a stem never passes for a mention. +- An edit that writes an agent permission list, or an orchestration reference, naming its provider canonically is applied with no complaint. +- Breaking each rule in its fixture turns red exactly the test named for that rule, and no other test. +- The refusal reaches the author in the same turn as the edit that caused it, before any commit, push, or CI run. +- Every plugin source in the repository as it stands passes both rules. +- A contributor who has never read `docs/ARCHITECTURE.md` can correct a refused edit from the refusal text alone. + +## Stakeholders + +- Decider: the repository owner, who ruled out the commit-time and CI mechanisms on 2026-09-14. +- Owner: the framework maintainers. +- Consumer: every contributor and every agent that edits a plugin source in this repository. + +## Context + +- Backlog item: [ai-driven-dev/framework#250](https://github.com/ai-driven-dev/framework/issues/250). +- The `00-onboard` exemption has an expiry, not a pass: [ai-driven-dev/framework#883](https://github.com/ai-driven-dev/framework/issues/883) makes that skill resolve its providers at runtime, and closing it closes the exemption. +- The issue body's "Guardrail local et CI" section predates the 2026-09-14 comment on the same issue and is superseded by it. The comment is the governing statement: no Git hook, no CI gate, synthetic fixtures, #406 neither blocker nor fixture. +- The orthogonality rule and its legitimate exceptions are stated in `docs/ARCHITECTURE.md`, under capability addressing: a capability is addressed only where the dispatch is declared, and elsewhere the concept is named instead of the skill that owns it. diff --git a/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/wiring-proof.md b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/wiring-proof.md new file mode 100644 index 000000000..7d81908d9 --- /dev/null +++ b/aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/wiring-proof.md @@ -0,0 +1,53 @@ +# Wiring proof + +Phase 2's task 5 asks for a recorded live refusal, because everything else about this guard +is verified upstream of the seam that matters: the tests spawn the hook script and pipe it a +payload, which proves the script and never proves the wire. The matcher string, the +`$CLAUDE_PROJECT_DIR` expansion inside the command, and `node` being on `PATH` are only +exercised by a real tool call — and the hook fails open at every one of those steps, so a +broken wire looks exactly like a clean tree. + +Both calls below were made with the Write tool, in this repository, against +`.claude/settings.json` as committed. + +## A refusal + +Writing `plugins/aidd-dev/skills/03-assert/references/wiring-proof.md`, a reference file in a +recipe skill, with this body: + +```md +# Wiring proof + +Hand the result to `/aidd-vcs:01-commit` once every assertion passes. +``` + +The call was refused. Verbatim, as it reached the author: + +```text +plugins/aidd-dev/skills/03-assert/references/wiring-proof.md:3 addresses sibling plugin "aidd-vcs" via "/aidd-vcs:01-commit". Fix: name the concept aidd-vcs owns instead of addressing it directly. +``` + +The file was never created — `ls` on that path answers `No such file or directory`. + +## The same address, legitimately + +Writing `plugins/aidd-dev/agents/wiring-proof.md` with the same address under the heading +`docs/ARCHITECTURE.md` sanctions: + +```md +# Skills you may invoke + +- `/aidd-vcs:01-commit` +``` + +The call was applied, with no output. The probe was removed in the same turn; +`git status --porcelain plugins/` is empty. + +## What this settles + +| Claim | Settled by | +| --- | --- | +| The hook is reachable from the matcher as wired | the refusal arrived at all | +| It decides before the write, not after | the refused path does not exist | +| The refusal names file, line and owning plugin | the verbatim text above | +| A legitimate permission list is not refused | the second call was applied silently | diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1b0f28551..fc2232633 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -155,6 +155,8 @@ Address a capability only where the dispatch is declared: a router's `## Actions Recipe skills never hardcode a sibling provider. They discover cross-plugin capabilities at runtime through description matching. Agent permission lists and orchestration references are responsibility maps, so they name the current provider with its canonical `/plugin:folder` or `@plugin:agent` address. The orchestrator must verify that provider is installed before calling it. +`scripts/lib/architecture-rules.js` is the guard that decides both rules from a path and its content, and `architecture-scan.js` is the only place that reads the disk for it. Two callers share them. `scripts/check-architecture-rules.js` runs as the `architecture-rules` pre-commit job, so every edit that reaches a commit is covered whichever tool or person made it, and `validate.yml` replays it over the whole tree on each pull request. The `PreToolUse` hook on `Write`/`Edit`/`MultiEdit` is the fast path on top: it only ever sees Claude Code, but it refuses the write in the same turn instead of at commit time. `plugins/aidd-context/skills/00-onboard/**` is exempt from rule one: its reference menus name addresses because those are what the skill hands a person to type, not a hardcoded sibling provider. That exemption is temporary and ends with the follow-up issue on making that skill resolve its providers at runtime. The guard refuses an edit leaving a `## Actions` section without a citation for an action file that exists; it never refuses a citation with no file behind it, because that is how a skill legitimately grows one. + This distinction keeps recipe plugins swappable while making orchestration handoffs explicit and auditable. ## 🔎 See also diff --git a/lefthook.yml b/lefthook.yml index 14eefeb00..00d136edb 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -61,6 +61,14 @@ pre-commit: exit 0 fi node scripts/check-skill-argument-hints.mjs + architecture-rules: + glob: "plugins/*/{skills,agents}/**/*.md" + run: | + if ! command -v node >/dev/null 2>&1; then + echo "ℹ️ node not available; skipping architecture-rules" + exit 0 + fi + node scripts/check-architecture-rules.js {files} context-imports: # Two patterns: "**/" needs a literal slash, so it never matches a repository-root file. glob: diff --git a/plugins/aidd-dev/skills/01-plan/actions/04-plan.md b/plugins/aidd-dev/skills/01-plan/actions/04-plan.md index 238b141c8..0803da0ff 100644 --- a/plugins/aidd-dev/skills/01-plan/actions/04-plan.md +++ b/plugins/aidd-dev/skills/01-plan/actions/04-plan.md @@ -13,7 +13,7 @@ A feature folder, always at `aidd_docs/tasks//_ { + const { root, skill } = makeTree(CLEAN_SKILL); + assert.deepEqual(scan(root, [skill]), []); +}); + +test("a skill addressing a sibling plugin is caught, and names the sibling", () => { + const { root, skill } = makeTree(SIBLING_ADDRESS_SKILL); + const found = scan(root, [skill]); + assert.equal(found.length, 1); + assert.equal(found[0].rule, "orthogonality"); + assert.equal(found[0].plugin, "aidd-fixture-b"); +}); + +test("an action file the router never cites is caught, and names the file", () => { + const { root, skill } = makeTree(UNCITED_ACTION_SKILL); + const found = scan(root, [skill]); + assert.equal(found.length, 1); + assert.equal(found[0].rule, "router-coherence"); + assert.match(found[0].message, /01-step\.md/); +}); + +test("a path outside the governed surface contributes nothing", () => { + const { root } = makeTree(SIBLING_ADDRESS_SKILL); + fs.writeFileSync(path.join(root, "README.md"), "See @aidd-fixture-b:02-thing.\n"); + assert.deepEqual(scan(root, ["README.md"]), []); +}); + +test("a path that does not exist is skipped rather than thrown over", () => { + const { root } = makeTree(CLEAN_SKILL); + assert.deepEqual(scan(root, ["plugins/aidd-fixture-a/skills/99-gone/SKILL.md"]), []); +}); + +test("the whole governed tree of this repository is silent, and is not empty", () => { + // A scan that matched nothing would pass every assertion above. Pin both halves. + const paths = governedPaths(REPO_ROOT); + assert.ok(paths.length > 100, `expected the tree to govern more than 100 files, got ${paths.length}`); + assert.deepEqual(scan(REPO_ROOT, paths), []); +}); + +test("the command exits 0 on this repository and says how much it checked", () => { + const result = spawnSync(process.execPath, [CHECK], { encoding: "utf8" }); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /governed file\(s\) checked, no violation/); +}); + +test("the command exits 1 and prints the fix when a governed file breaks a rule", () => { + const { root, skill } = makeTree(SIBLING_ADDRESS_SKILL); + const result = spawnSync(process.execPath, [CHECK, "--root", root, skill], { encoding: "utf8" }); + assert.equal(result.status, 1); + assert.match(result.stderr, /addresses sibling plugin "aidd-fixture-b"/); + assert.match(result.stderr, /Fix: name the concept aidd-fixture-b owns/); +}); + +test("the command scans a whole tree it is pointed at, not only this repository", () => { + const { root } = makeTree(UNCITED_ACTION_SKILL); + const result = spawnSync(process.execPath, [CHECK, "--root", root], { encoding: "utf8" }); + assert.equal(result.status, 1); + assert.match(result.stderr, /never names action file "01-step\.md"/); +}); diff --git a/scripts/__tests__/architecture-hook.test.js b/scripts/__tests__/architecture-hook.test.js new file mode 100644 index 000000000..fadb1ee17 --- /dev/null +++ b/scripts/__tests__/architecture-hook.test.js @@ -0,0 +1,500 @@ +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const test = require("node:test"); +const { spawnSync } = require("node:child_process"); + +const REPO_ROOT = path.resolve(__dirname, "../.."); +const HOOK = path.join(REPO_ROOT, ".claude/hooks/check-architecture-rules.js"); + +const CLEAN_SKILL = [ + "# Clean skill", + "", + "## Actions", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `step` | Do the one thing |", + "", +].join("\n"); + +const SIBLING_ADDRESS_SKILL = [ + "# Clean skill", + "", + "See @aidd-fixture-b:02-thing for the other half.", + "", + "## Actions", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `step` | Do the one thing |", + "", +].join("\n"); + +/** A fresh temp tree with a `plugins/` root, so the hook's own file classifier applies to it + * exactly as it would to this repository, without touching this repository's tracked files. */ +function makeProjectDir() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "architecture-hook-test-")); + return dir; +} + +function writeFixtureFile(projectDir, relPath, content) { + const abs = path.join(projectDir, relPath); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content, "utf8"); + return abs; +} + +function runHook(projectDir, payload) { + const input = typeof payload === "string" ? payload : JSON.stringify(payload); + const result = spawnSync(process.execPath, [HOOK], { + input, + encoding: "utf8", + env: { ...process.env, CLAUDE_PROJECT_DIR: projectDir }, + cwd: projectDir, + }); + return result; +} + +function parseDenyReason(stdout) { + const parsed = JSON.parse(stdout); + return parsed.hookSpecificOutput.permissionDecisionReason; +} + +test("a clean Write over a plugin skill exits zero with no output", () => { + const projectDir = makeProjectDir(); + const filePath = path.join(projectDir, "plugins/aidd-fixture-a/skills/01-clean/SKILL.md"); + + const result = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: filePath, content: CLEAN_SKILL }, + }); + + assert.equal(result.status, 0); + assert.equal(result.stdout, ""); +}); + +test("a Write introducing a sibling address is denied, naming file, line and owning plugin", () => { + const projectDir = makeProjectDir(); + const filePath = path.join(projectDir, "plugins/aidd-fixture-a/skills/01-clean/SKILL.md"); + + const result = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: filePath, content: SIBLING_ADDRESS_SKILL }, + }); + + assert.equal(result.status, 0); + const reason = parseDenyReason(result.stdout); + assert.match(reason, /plugins\/aidd-fixture-a\/skills\/01-clean\/SKILL\.md:3/); + assert.match(reason, /aidd-fixture-b/); +}); + +test("an Edit reconstructing the same content as the denied Write yields the same verdict", () => { + const projectDir = makeProjectDir(); + const filePath = writeFixtureFile( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", + CLEAN_SKILL + ); + + const result = runHook(projectDir, { + tool_name: "Edit", + tool_input: { + file_path: filePath, + old_string: "# Clean skill\n", + new_string: "# Clean skill\n\nSee @aidd-fixture-b:02-thing for the other half.\n", + }, + }); + + assert.equal(result.status, 0); + const writeResult = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: filePath, content: SIBLING_ADDRESS_SKILL }, + }); + const editReason = parseDenyReason(result.stdout); + const writeReason = parseDenyReason(writeResult.stdout); + assert.equal(editReason, writeReason); +}); + +test("an Edit whose new_string is a $-replacement token splices it literally, not specially", () => { + const projectDir = makeProjectDir(); + const AGENT_WITH_PERMISSION_LIST = [ + "# Guardrails", + "", + "- Never delegate to another agent.", + "", + "# Skills you may invoke", + "", + "- `/aidd-dev:02-implement`", + "- `/aidd-vcs:01-commit`", + "", + ].join("\n"); + const filePath = writeFixtureFile( + projectDir, + "plugins/aidd-dev/agents/executor.md", + AGENT_WITH_PERMISSION_LIST + ); + + // Removing the heading unmasks the one real sibling address below it (it is no longer under + // an exempt "# Skills you may invoke" heading) — a single, genuine violation. A `String.replace` + // reconstruction corrupts this: "$'" is a special token even against a plain-string search, so + // it re-inserts (and thereby duplicates) everything after the match, producing a second, + // spurious violation at a line that holds no such content in the real result. + const result = runHook(projectDir, { + tool_name: "Edit", + tool_input: { + file_path: filePath, + old_string: "# Skills you may invoke", + new_string: "$'", + }, + }); + + assert.equal(result.status, 0); + const reason = parseDenyReason(result.stdout); + assert.match(reason, /plugins\/aidd-dev\/agents\/executor\.md:8 /); + // Line 12 only exists in the corrupted (duplicated-tail) reconstruction — its absence here is + // what proves the splice was literal, not merely that some deny happened. + assert.doesNotMatch(reason, /plugins\/aidd-dev\/agents\/executor\.md:12 /); +}); + +test("an Edit whose old_string is absent from the current file exits zero", () => { + const projectDir = makeProjectDir(); + const filePath = writeFixtureFile( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", + CLEAN_SKILL + ); + + const result = runHook(projectDir, { + tool_name: "Edit", + tool_input: { + file_path: filePath, + old_string: "this text is not in the file", + new_string: "See @aidd-fixture-b:02-thing for the other half.", + }, + }); + + assert.equal(result.status, 0); + assert.equal(result.stdout, ""); +}); + +test("a path under assets/ exits zero even when its content addresses a sibling", () => { + const projectDir = makeProjectDir(); + const filePath = path.join( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/assets/notes.md" + ); + + const result = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: filePath, content: SIBLING_ADDRESS_SKILL }, + }); + + assert.equal(result.status, 0); + assert.equal(result.stdout, ""); +}); + +test("writing a new action file is always silent, named or not — rule two fires on the SKILL.md write only", () => { + // Order A of adding an action: the action file lands before the SKILL.md row that names it. + // A sibling-side check here would deny this exact, routine sequence (issue: adding an action + // is impossible in either order); the gap it trades for — an unnamed action file going + // uncaught until the next SKILL.md write — is real and recorded outside this file, not hidden. + const projectDir = makeProjectDir(); + writeFixtureFile(projectDir, "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", CLEAN_SKILL); + writeFixtureFile( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/actions/01-step.md", + "# Step\n" + ); + const filePath = path.join( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/actions/02-extra.md" + ); + + const result = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: filePath, content: "# Extra\n\nDoes something new.\n" }, + }); + + assert.equal(result.status, 0); + assert.equal(result.stdout, ""); +}); + +test("a malformed payload exits zero with no output", () => { + const projectDir = makeProjectDir(); + + const result = runHook(projectDir, "{ not json at all"); + + assert.equal(result.status, 0); + assert.equal(result.stdout, ""); +}); + +test("a path outside plugins/ exits zero even when its content addresses a sibling", () => { + const projectDir = makeProjectDir(); + const filePath = path.join(projectDir, "README.md"); + + const result = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: filePath, content: SIBLING_ADDRESS_SKILL }, + }); + + assert.equal(result.status, 0); + assert.equal(result.stdout, ""); +}); + +test("an agent's permission list naming a sibling canonically is applied with no complaint", () => { + const projectDir = makeProjectDir(); + const filePath = path.join(projectDir, "plugins/aidd-fixture-a/agents/thing.md"); + const content = [ + "# Thing", + "", + "# Skills you may invoke", + "", + "- `/aidd-fixture-b:02-thing`", + "", + ].join("\n"); + + const result = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: filePath, content }, + }); + + assert.equal(result.status, 0); + assert.equal(result.stdout, ""); +}); + +test("an Edit with replace_all denies on both occurrences the reconstruction introduces", () => { + const projectDir = makeProjectDir(); + const twoAnchors = [ + "# Clean skill", + "", + "ANCHOR", + "", + "ANCHOR", + "", + "## Actions", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `step` | Do the one thing |", + "", + ].join("\n"); + const filePath = writeFixtureFile( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", + twoAnchors + ); + + const result = runHook(projectDir, { + tool_name: "Edit", + tool_input: { + file_path: filePath, + old_string: "ANCHOR", + new_string: "See @aidd-fixture-b:02-thing here.", + replace_all: true, + }, + }); + + assert.equal(result.status, 0); + const reason = parseDenyReason(result.stdout); + assert.match(reason, /:3\b/); + assert.match(reason, /:5\b/); +}); + +test("a MultiEdit that introduces a sibling address is denied", () => { + const projectDir = makeProjectDir(); + const rel = "plugins/aidd-fixture-a/skills/01-clean/SKILL.md"; + const filePath = writeFixtureFile(projectDir, rel, CLEAN_SKILL); + + const result = runHook(projectDir, { + tool_name: "MultiEdit", + tool_input: { + file_path: filePath, + edits: [ + { old_string: "# Clean skill", new_string: "# Clean skill\n\nstill clean here" }, + { old_string: "still clean here", new_string: "See @aidd-fixture-b:02-thing for the other half." }, + ], + }, + }); + + assert.equal(result.status, 0); + const reason = parseDenyReason(result.stdout); + assert.match(reason, /aidd-fixture-b/); + assert.match(reason, new RegExp(`${rel}:3`)); +}); + +test("a MultiEdit whose edits stay clean exits zero with no output", () => { + const projectDir = makeProjectDir(); + const rel = "plugins/aidd-fixture-a/skills/01-clean/SKILL.md"; + const filePath = writeFixtureFile(projectDir, rel, CLEAN_SKILL); + + const result = runHook(projectDir, { + tool_name: "MultiEdit", + tool_input: { + file_path: filePath, + edits: [{ old_string: "# Clean skill", new_string: "# Still a clean skill" }], + }, + }); + + assert.equal(result.status, 0); + assert.equal(result.stdout, ""); +}); + +test("a MultiEdit whose first edit cannot be applied exits zero", () => { + const projectDir = makeProjectDir(); + const rel = "plugins/aidd-fixture-a/skills/01-clean/SKILL.md"; + const filePath = writeFixtureFile(projectDir, rel, CLEAN_SKILL); + + const result = runHook(projectDir, { + tool_name: "MultiEdit", + tool_input: { + file_path: filePath, + edits: [ + { old_string: "a string this file never holds", new_string: "@aidd-fixture-b:02-thing" }, + ], + }, + }); + + assert.equal(result.status, 0); + assert.equal(result.stdout, ""); +}); + +test("a relative file_path from a differing cwd still resolves the actions directory against the project root", () => { + // The hook must resolve tool_input.file_path against CLAUDE_PROJECT_DIR exactly once, the same + // way for every filesystem read it performs. Before that fix, actionFileNamesFor derived its + // directory from the raw (relative) file_path, which Node then resolves against the process's + // actual cwd rather than the project root — silently emptying the action-file list, and with + // it, rule two, whenever the two differ. + const projectDir = makeProjectDir(); + const otherCwd = fs.mkdtempSync(path.join(os.tmpdir(), "architecture-hook-othercwd-")); + writeFixtureFile(projectDir, "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", CLEAN_SKILL); + writeFixtureFile( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/actions/01-step.md", + "# Step\n" + ); + writeFixtureFile( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/actions/02-extra.md", + "# Extra\n" + ); + + const result = spawnSync(process.execPath, [HOOK], { + input: JSON.stringify({ + tool_name: "Write", + tool_input: { + file_path: "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", + content: CLEAN_SKILL, + }, + }), + encoding: "utf8", + env: { ...process.env, CLAUDE_PROJECT_DIR: projectDir }, + cwd: otherCwd, + }); + + assert.equal(result.status, 0); + const reason = parseDenyReason(result.stdout); + assert.match(reason, /never names action file "02-extra\.md"/); +}); + +test("adding an action to an existing skill succeeds file-first (order A)", () => { + const projectDir = makeProjectDir(); + writeFixtureFile(projectDir, "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", CLEAN_SKILL); + writeFixtureFile( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/actions/01-step.md", + "# Step\n" + ); + + const actionFilePath = path.join( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/actions/02-refine.md" + ); + const createAction = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: actionFilePath, content: "# Refine\n" }, + }); + assert.equal(createAction.status, 0); + assert.equal(createAction.stdout, ""); + + fs.writeFileSync(actionFilePath, "# Refine\n", "utf8"); + const SKILL_NAMING_BOTH = [ + "# Clean skill", + "", + "## Actions", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `step` | Do the one thing |", + "| 02 | `refine` | Do the new thing |", + "", + ].join("\n"); + const skillFilePath = path.join(projectDir, "plugins/aidd-fixture-a/skills/01-clean/SKILL.md"); + const nameAction = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: skillFilePath, content: SKILL_NAMING_BOTH }, + }); + assert.equal(nameAction.status, 0); + assert.equal(nameAction.stdout, ""); +}); + +test("adding an action to an existing skill succeeds citation-first (order B)", () => { + const projectDir = makeProjectDir(); + writeFixtureFile(projectDir, "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", CLEAN_SKILL); + writeFixtureFile( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/actions/01-step.md", + "# Step\n" + ); + + const SKILL_CITING_REFINE = [ + "# Clean skill", + "", + "## Actions", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `step` | Do the one thing |", + "| 02 | `refine` | Do the new thing |", + "", + ].join("\n"); + const skillFilePath = path.join(projectDir, "plugins/aidd-fixture-a/skills/01-clean/SKILL.md"); + const citeFirst = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: skillFilePath, content: SKILL_CITING_REFINE }, + }); + assert.equal(citeFirst.status, 0); + assert.equal(citeFirst.stdout, ""); + + fs.writeFileSync(skillFilePath, SKILL_CITING_REFINE, "utf8"); + const actionFilePath = path.join( + projectDir, + "plugins/aidd-fixture-a/skills/01-clean/actions/02-refine.md" + ); + const createAction = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: actionFilePath, content: "# Refine\n" }, + }); + assert.equal(createAction.status, 0); + assert.equal(createAction.stdout, ""); +}); + +test("a router refusal says what a citation is, not only that one is missing", () => { + const projectDir = makeProjectDir(); + const rel = "plugins/aidd-fixture-a/skills/01-clean/SKILL.md"; + const filePath = writeFixtureFile(projectDir, rel, CLEAN_SKILL); + writeFixtureFile(projectDir, "plugins/aidd-fixture-a/skills/01-clean/actions/01-step.md", "# step\n"); + writeFixtureFile(projectDir, "plugins/aidd-fixture-a/skills/01-clean/actions/02-refine.md", "# refine\n"); + + const result = runHook(projectDir, { + tool_name: "Write", + tool_input: { file_path: filePath, content: `${CLEAN_SKILL}\nThe refine step tidies up.\n` }, + }); + + assert.equal(result.status, 0); + const reason = parseDenyReason(result.stdout); + assert.match(reason, /02-refine\.md/); + assert.match(reason, /table header that reads Action/); + assert.match(reason, /a word in prose does not count/); +}); diff --git a/scripts/__tests__/architecture-rules.property.test.js b/scripts/__tests__/architecture-rules.property.test.js new file mode 100644 index 000000000..ec8434428 --- /dev/null +++ b/scripts/__tests__/architecture-rules.property.test.js @@ -0,0 +1,248 @@ +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const path = require("node:path"); +const test = require("node:test"); + +const { checkRouterCoherence } = require("../lib/architecture-rules.js"); + +const ROOT = path.resolve(__dirname, "../.."); +const TICK = "`"; + +/** + * Rule two decided by one property, over every shape a router is written in, instead of one + * test per shape someone happened to think of. + * + * The property: a section that cites every action file it provides is silent, and removing any + * one of those citations yields exactly one violation — the one naming that file. + * + * The shapes are generated rather than random. A guard that fails on a seed nobody can reproduce + * is worse than no guard, and the repository's own root holds six dev dependencies, none of them + * a generator library. The matrix below is small enough to enumerate and wide enough to cover + * what six rounds of review found by hand. + */ + +const HEADERS = { + bare: ["| Action | Does |"], + numbered: ["| # | Action | Role |"], + plural: ["| # | Actions | Role |"], +}; + +const SEPARATORS = { + three: "---", + two: "--", + aligned: ":---", +}; + +/** How a row names the action it routes to. Each is a citation shape the engine accepts. */ +const CITATIONS = { + stem: (name) => stemOf(name), + backtickedStem: (name) => `${TICK}${stemOf(name)}${TICK}`, + numberedName: (name) => name.replace(/\.md$/, ""), + backtickedFile: (name) => `${TICK}${name}${TICK}`, +}; + +/** Text placed around the router that must not change any verdict. */ +const NOISE = { + none: () => [], + prose: (names) => [ + "", + `Run them in order. The ${stemOf(names[0])} step is the culmination, and ${stemOf( + names[names.length - 1] + )} closes it.`, + ], + glossaryTable: () => ["", "| Term | Synonym |", "| --- | --- |", "| step | move |"], + fencedExample: (names) => [ + "", + "An example of the shape a router takes:", + "", + "```md", + "| # | Action | Role |", + "| --- | --- | --- |", + `| 99 | ${stemOf(names[names.length - 1])} | an example, not a router |`, + "```", + ], + /** The same example, citing a file name rather than a stem: the shape that reads through a + * fence for the path form and must not for this one. */ + fencedFilename: (names) => [ + "", + "```md", + "| # | Action | Role |", + "| --- | --- | --- |", + `| 99 | ${TICK}${names[names.length - 1]}${TICK} | an example, not a router |`, + "```", + ], + trailingParagraph: () => ["", "Before running an action, read its file in `actions/`."], +}; + +function stemOf(name) { + return name.replace(/\.md$/, "").replace(/^\d+-/, ""); +} + +/** One `SKILL.md` body, built from the matrix. `omit` is the action file left uncited. */ +function buildSkill({ header, separator, citation, noise, names, omit, splitAfter }) { + const headerRow = HEADERS[header][0]; + const columns = headerRow.split("|").filter((cell) => cell.trim() !== "").length; + const separatorRow = `| ${Array.from({ length: columns }, () => SEPARATORS[separator]).join(" | ")} |`; + + const rows = []; + names.forEach((name, index) => { + if (name === omit) return; + if (splitAfter !== undefined && index === splitAfter) rows.push(""); + const cited = CITATIONS[citation](name); + rows.push(columns === 2 ? `| ${cited} | does it |` : `| 0${index + 1} | ${cited} | does it |`); + }); + + return [ + "# Generated skill", + "", + "## Actions", + "", + headerRow, + separatorRow, + ...rows, + ...NOISE[noise](names), + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); +} + +const NAMES = ["01-first.md", "02-second.md", "03-third.md"]; +const FILE = "plugins/aidd-fixture-a/skills/01-generated/SKILL.md"; + +function shapes() { + const out = []; + for (const header of Object.keys(HEADERS)) { + for (const separator of Object.keys(SEPARATORS)) { + for (const citation of Object.keys(CITATIONS)) { + for (const noise of Object.keys(NOISE)) { + for (const splitAfter of [undefined, 1]) { + out.push({ header, separator, citation, noise, splitAfter }); + } + } + } + } + } + return out; +} + +function describe(shape, omit) { + const split = shape.splitAfter === undefined ? "unsplit" : "split by a blank line"; + return `${shape.header} header, ${shape.separator} separator, ${shape.citation} citation, ${shape.noise} noise, ${split}${ + omit ? `, omitting ${omit}` : "" + }`; +} + +test("every shape that cites all its actions is silent", () => { + const failures = []; + for (const shape of shapes()) { + const content = buildSkill({ ...shape, names: NAMES, omit: null }); + const violations = checkRouterCoherence(FILE, content, NAMES); + if (violations.length !== 0) { + failures.push(`${describe(shape)} => ${violations.map((v) => v.message).join(" | ")}`); + } + } + assert.deepEqual(failures, [], `a router shape was refused although it cites every action:\n${failures.join("\n")}`); +}); + +test("every shape that drops one citation yields exactly that one violation", () => { + const failures = []; + for (const shape of shapes()) { + for (const omit of NAMES) { + const content = buildSkill({ ...shape, names: NAMES, omit }); + const violations = checkRouterCoherence(FILE, content, NAMES); + if (violations.length !== 1) { + failures.push(`${describe(shape, omit)} => ${violations.length} violations`); + continue; + } + if (!violations[0].message.includes(omit)) { + failures.push(`${describe(shape, omit)} => named ${violations[0].message}`); + } + } + } + assert.deepEqual(failures, [], `a dropped citation was missed or misattributed:\n${failures.join("\n")}`); +}); + +test("the shape matrix is wide enough to be worth running", () => { + // A generator that silently produces nothing passes every property above. Pin the count so a + // matrix that collapses fails here rather than going quietly green. + // 3 headers x 3 separators x 4 citation shapes x 6 noises x 2 split positions. + assert.equal(shapes().length, 432); +}); + +/** + * The same property against the repository's own routers, which is where it has to hold: every + * action the tree provides is cited, and removing the line that cites it is caught. + */ +function realSkills() { + const skills = []; + const pluginsDir = path.join(ROOT, "plugins"); + for (const owner of fs.readdirSync(pluginsDir)) { + const skillsDir = path.join(pluginsDir, owner, "skills"); + if (!fs.existsSync(skillsDir)) continue; + for (const skill of fs.readdirSync(skillsDir)) { + const skillMd = path.join(skillsDir, skill, "SKILL.md"); + const actionsDir = path.join(skillsDir, skill, "actions"); + if (!fs.existsSync(skillMd) || !fs.existsSync(actionsDir)) continue; + const names = fs.readdirSync(actionsDir).filter((name) => name.endsWith(".md")); + if (names.length === 0) continue; + skills.push({ + relPath: `plugins/${owner}/skills/${skill}/SKILL.md`, + content: fs.readFileSync(skillMd, "utf8"), + names, + }); + } + } + return skills; +} + +/** The skill's content with every table row citing `name` removed. */ +function withoutCitationOf(content, name) { + const stem = stemOf(name).toLowerCase(); + const noExt = name.replace(/\.md$/, "").toLowerCase(); + return content + .split("\n") + .filter((line) => { + if (!/^\s*\|/.test(line)) return true; + const cells = line + .trim() + .replace(/^\|/, "") + .replace(/\|$/, "") + .split("|") + .map((cell) => cell.trim().replace(/^`|`$/g, "").toLowerCase()); + return !cells.includes(stem) && !cells.includes(noExt) && !cells.includes(name.toLowerCase()); + }) + .join("\n"); +} + +test("every router in the tree cites every action it provides", () => { + const failures = []; + for (const { relPath, content, names } of realSkills()) { + const violations = checkRouterCoherence(relPath, content, names); + if (violations.length > 0) failures.push(`${relPath} => ${violations.map((v) => v.message).join(" | ")}`); + } + assert.deepEqual(failures, [], failures.join("\n")); +}); + +test("removing a real router's citation is caught, for every action in the tree", () => { + // `aidd-dev:10-todo` names its one action as a fenced path rather than a table row, so there + // is no row to remove and nothing to catch. It is the one documented exception. + const FENCED_PATH_SKILL = "plugins/aidd-dev/skills/10-todo/SKILL.md"; + const missed = []; + let checked = 0; + + for (const { relPath, content, names } of realSkills()) { + if (relPath === FENCED_PATH_SKILL) continue; + for (const name of names) { + checked += 1; + const violations = checkRouterCoherence(relPath, withoutCitationOf(content, name), names); + if (!violations.some((violation) => violation.message.includes(name))) { + missed.push(`${relPath} ${name}`); + } + } + } + + assert.deepEqual(missed, [], `a deleted citation went unnoticed:\n${missed.join("\n")}`); + assert.ok(checked > 100, `expected the tree to offer more than 100 citations to remove, got ${checked}`); +}); diff --git a/scripts/__tests__/architecture-rules.test.js b/scripts/__tests__/architecture-rules.test.js new file mode 100644 index 000000000..9eb7df1b8 --- /dev/null +++ b/scripts/__tests__/architecture-rules.test.js @@ -0,0 +1,522 @@ +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const path = require("node:path"); +const test = require("node:test"); + +const { checkArchitecture, classifyFile } = require("../lib/architecture-rules.js"); + +const ROOT = path.resolve(__dirname, "../.."); +const FIXTURES = path.join(__dirname, "fixtures/architecture-rules"); + +function fixture(relPath) { + return fs.readFileSync(path.join(FIXTURES, relPath), "utf8"); +} + +/** + * Thin tree walker for the sweep test only. The engine itself never reads the filesystem — + * this is the caller that supplies its inputs from the real `plugins/` tree. + */ +function sweepPlugins() { + const files = []; + const pluginsDir = path.join(ROOT, "plugins"); + + function walk(dir) { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + walk(full); + } else if (entry.isFile() && entry.name.endsWith(".md")) { + files.push(full); + } + } + } + walk(pluginsDir); + + const violations = []; + let skillsWithActions = 0; + for (const absPath of files) { + const relPath = path.relative(ROOT, absPath).split(path.sep).join("/"); + // The engine's own classifier decides what is governed — including `assets/` — so the + // walker never keeps a second copy of that rule that could drift from it. + if (!classifyFile(relPath)) continue; + const content = fs.readFileSync(absPath, "utf8"); + let actionFileNames; + if (path.basename(absPath) === "SKILL.md") { + const actionsDir = path.join(path.dirname(absPath), "actions"); + actionFileNames = fs.existsSync(actionsDir) + ? fs.readdirSync(actionsDir).filter((f) => f.endsWith(".md")) + : []; + if (actionFileNames.length > 0) skillsWithActions += 1; + } + violations.push(...checkArchitecture(relPath, content, actionFileNames)); + } + return { violations, skillsWithActions }; +} + +test("a clean skill with no sibling address and a fully-named action yields no violations", () => { + const content = fixture("clean-skill/SKILL.md"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", + content, + ["01-step.md"] + ); + assert.deepEqual(violations, []); +}); + +test("a skill addressing a sibling plugin yields a violation naming file, line and owning plugin", () => { + const content = fixture("cross-address-skill/SKILL.md"); + const filePath = "plugins/aidd-fixture-a/skills/02-cross-address/SKILL.md"; + const violations = checkArchitecture(filePath, content, ["01-step.md"]); + + assert.equal(violations.length, 1); + const [violation] = violations; + assert.equal(violation.file, filePath); + assert.equal(violation.plugin, "aidd-fixture-b"); + const lines = content.split("\n"); + assert.equal(lines[violation.line - 1].includes("/aidd-fixture-b:01-noop"), true); +}); + +test("an agent's Skills you may invoke list addressing siblings yields no violations", () => { + const content = fixture("agent-permission-list.md"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/agents/reviewer.md", + content + ); + assert.deepEqual(violations, []); +}); + +test("an orchestrator plugin is exempt from orthogonality alone", () => { + const content = fixture("orchestrator-skill/SKILL.md"); + // No actionFileNames: isolates rule one, since rule two short-circuits on an empty list. + const violations = checkArchitecture( + "plugins/aidd-orchestrator/skills/99-fixture/SKILL.md", + content + ); + assert.deepEqual(violations, []); +}); + +test("router coherence still applies to an orchestrator plugin", () => { + const content = fixture("orchestrator-incoherent-skill/SKILL.md"); + const filePath = "plugins/aidd-orchestrator/skills/99-fixture/SKILL.md"; + const violations = checkArchitecture(filePath, content, ["01-route.md"]); + + assert.equal(violations.length, 1); + const [violation] = violations; + assert.equal(violation.rule, "router-coherence"); + assert.equal(violation.plugin, "aidd-orchestrator"); +}); + +test("a file under assets/ yields no violations regardless of content", () => { + const content = fixture("assets-note.md"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-clean/assets/note.md", + content + ); + assert.deepEqual(violations, []); +}); + +test("an action file nested a level deeper than usual is still governed", () => { + const content = fixture("nested-action.md"); + const filePath = "plugins/aidd-fixture-a/skills/01-clean/actions/group/nested-action.md"; + const violations = checkArchitecture(filePath, content); + + assert.equal(violations.length, 1); + const [violation] = violations; + assert.equal(violation.plugin, "aidd-fixture-b"); +}); + +test("a SKILL.md nested a level deeper than usual is still governed", () => { + const content = fixture("nested-skill/SKILL.md"); + const filePath = "plugins/aidd-fixture-a/skills/01-clean/variant/SKILL.md"; + const violations = checkArchitecture(filePath, content); + + assert.equal(violations.length, 1); + const [violation] = violations; + assert.equal(violation.plugin, "aidd-fixture-b"); +}); + +test("a reference file nested two levels under references/ is still governed", () => { + const content = fixture("nested-reference.md"); + const filePath = "plugins/aidd-fixture-a/skills/01-clean/references/state/nested-reference.md"; + const violations = checkArchitecture(filePath, content); + + assert.equal(violations.length, 1); + const [violation] = violations; + assert.equal(violation.plugin, "aidd-fixture-b"); +}); + +test("a SKILL.md nested under assets/ is ungoverned for that reason alone", () => { + const content = fixture("nested-assets-skill/SKILL.md"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-clean/assets/nested/SKILL.md", + content + ); + assert.deepEqual(violations, []); +}); + +test("an action file the Actions section never names yields one violation", () => { + const content = fixture("unnamed-action-skill/SKILL.md"); + const filePath = "plugins/aidd-fixture-a/skills/03-unnamed-action/SKILL.md"; + const violations = checkArchitecture(filePath, content, [ + "01-step-one.md", + "02-step-two.md", + ]); + + assert.equal(violations.length, 1); + const [violation] = violations; + assert.equal(violation.file, filePath); + assert.equal(violation.plugin, "aidd-fixture-a"); + const lines = content.split("\n"); + assert.equal(lines[violation.line - 1].trim(), "## Actions"); +}); + +test("00-onboard's own reference menus are exempt from orthogonality", () => { + const content = fixture("onboard-menu.md"); + const filePath = "plugins/aidd-context/skills/00-onboard/references/order/onboard-menu.md"; + const violations = checkArchitecture(filePath, content); + assert.deepEqual(violations, []); +}); + +test("a bare plugin:skill address is caught, but not one embedded in a longer identifier", () => { + const content = fixture("bare-address-skill/SKILL.md"); + const filePath = "plugins/aidd-fixture-a/skills/07-bare-address/SKILL.md"; + const violations = checkArchitecture(filePath, content, ["01-step.md"]); + + assert.equal(violations.length, 1); + const [violation] = violations; + assert.equal(violation.plugin, "aidd-fixture-b"); + assert.match(violation.message, /"aidd-fixture-b:01-noop"/); +}); + +test("a stem that is a substring of a sibling action's stem is not mistaken for a mention", () => { + const content = fixture("stem-substring-skill/SKILL.md"); + const filePath = "plugins/aidd-fixture-a/skills/05-stem-substring/SKILL.md"; + const violations = checkArchitecture(filePath, content, [ + "01-assert.md", + "02-assert-architecture.md", + ]); + + assert.equal(violations.length, 1); + const [violation] = violations; + assert.match(violation.message, /"01-assert\.md"/); +}); + +test("a deleted table row is still caught when unrelated prose loosely contains its word", () => { + // Regression for the substring-over-prose bug: "Run them in order, `01 → 04`. The plan is + // the culmination." contains the word "plan" in ordinary prose, not as a table cell, an + // actions/ path, or a backticked .md filename — so it must not count as naming 04-plan.md. + const content = fixture("deleted-row-masked-by-prose-skill/SKILL.md"); + const filePath = "plugins/aidd-fixture-a/skills/09-deleted-row/SKILL.md"; + const violations = checkArchitecture(filePath, content, ["01-frame.md", "04-plan.md"]); + + assert.equal(violations.length, 1); + const [violation] = violations; + assert.match(violation.message, /"04-plan\.md"/); +}); + +test("a skill whose Actions section and action files agree yields no violations", () => { + const content = fixture("clean-skill/SKILL.md"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-clean/SKILL.md", + content, + ["01-step.md"] + ); + assert.deepEqual(violations, []); +}); + +test("sweeping the repository's own plugins/ tree yields zero violations", () => { + const { violations, skillsWithActions } = sweepPlugins(); + assert.deepEqual(violations, []); + // A sweep that never actually exercised a skill with action files would pass the same way — + // this pins the sweep to the measured count so it cannot go vacuously green. + assert.equal(skillsWithActions, 48); +}); + +test("a table that declares no action column cites nothing, whatever its cells read", () => { + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-glossary/SKILL.md", + fixture("glossary-column-skill/SKILL.md"), + ["01-step.md"] + ); + assert.equal(violations.length, 1); + assert.equal(violations[0].rule, "router-coherence"); + assert.match(violations[0].message, /never names action file "01-step\.md"/); +}); + +test("a name in a column other than the action column does not cite that action", () => { + const content = [ + "# Two column skill", + "", + "## Actions", + "", + "| # | Action | Next |", + "| --- | --- | --- |", + "| 01 | `first` | second |", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-two-column/SKILL.md", + content, + ["01-first.md", "02-second.md"] + ); + assert.equal(violations.length, 1); + assert.match(violations[0].message, /never names action file "02-second\.md"/); +}); + +test("a skill holding action files with no Actions section at all is refused", () => { + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-no-section/SKILL.md", + fixture("no-actions-section-skill/SKILL.md"), + ["01-step.md"] + ); + assert.equal(violations.length, 1); + assert.equal(violations[0].rule, "router-coherence"); + assert.match(violations[0].message, /no "## Actions" section/); +}); + +test("a glossary table before the router does not blind the router's own column", () => { + const content = [ + "# Two table skill", + "", + "## Actions", + "", + "| Term | Synonym |", + "| --- | --- |", + "| step | move |", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `first` | Do it |", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-two-table/SKILL.md", + content, + ["01-first.md"] + ); + assert.deepEqual(violations, []); +}); + +test("a blank line splitting the router table does not unname the rows below it", () => { + const content = [ + "# Split table skill", + "", + "## Actions", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `first` | Do it |", + "", + "| 02 | `second` | Then this |", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-split/SKILL.md", + content, + ["01-first.md", "02-second.md"] + ); + assert.deepEqual(violations, []); +}); + +test("a two-dash separator row still marks the header it follows", () => { + const content = [ + "# Short rule skill", + "", + "## Actions", + "", + "| # | Action | Does |", + "| -- | --- | --- |", + "| 01 | first | Do it |", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-short-rule/SKILL.md", + content, + ["01-first.md"] + ); + assert.deepEqual(violations, []); +}); + +test("a plural Actions header declares the action column too", () => { + const content = [ + "# Plural header skill", + "", + "## Actions", + "", + "| # | Actions | Role |", + "| --- | --- | --- |", + "| 01 | `first` | Do it |", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-plural/SKILL.md", + content, + ["01-first.md"] + ); + assert.deepEqual(violations, []); +}); + +test("two action files sharing a stem are not both covered by one citation", () => { + const content = [ + "# Shared stem skill", + "", + "## Actions", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `plan` | Do it |", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-shared-stem/SKILL.md", + content, + ["01-plan.md", "04-plan.md"] + ); + assert.equal(violations.length, 2); + assert.match(violations[0].message, /never names action file "01-plan\.md"/); + assert.match(violations[1].message, /never names action file "04-plan\.md"/); +}); + +test("a fenced example table inside the Actions section cites nothing", () => { + const content = [ + "# Fenced example skill", + "", + "## Actions", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `first` | Do it |", + "", + "An example of the shape a router takes:", + "", + "```md", + "| 02 | second | Then this |", + "```", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-fenced/SKILL.md", + content, + ["01-first.md", "02-second.md"] + ); + assert.equal(violations.length, 1); + assert.match(violations[0].message, /never names action file "02-second\.md"/); +}); + +test("a fenced actions/.md path still cites, the way 10-todo does", () => { + const content = [ + "# Fenced path skill", + "", + "## Actions", + "", + "```md", + "actions/01-only.md", + "```", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-fenced-path/SKILL.md", + content, + ["01-only.md"] + ); + assert.deepEqual(violations, []); +}); + +test("a fenced ## heading inside the Actions section does not end it early", () => { + const content = [ + "# Fenced heading skill", + "", + "## Actions", + "", + "```md", + "## Actions", + "```", + "", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `first` | Do it |", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-fenced-heading/SKILL.md", + content, + ["01-first.md"] + ); + assert.deepEqual(violations, []); +}); + +test("a backticked file name inside a fenced example does not cite", () => { + const fence = "```"; + const content = [ + "# Fenced filename skill", + "", + "## Actions", + "", + `${fence}md`, + "| Action | When |", + "| --- | --- |", + `| ${"`"}02-gone.md${"`"} | b |`, + fence, + "", + "| Action | When |", + "| --- | --- |", + "| 01-keep | a |", + "", + "## Transversal rules", + "", + "- Nothing.", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-fenced-name/SKILL.md", + content, + ["01-keep.md", "02-gone.md"] + ); + assert.equal(violations.length, 1); + assert.match(violations[0].message, /never names action file "02-gone\.md"/); +}); + +test("a fence nobody closed is not read as a fence at all", () => { + const content = [ + "# Unterminated fence skill", + "", + "## Actions", + "", + "```md", + "| # | Action | Role |", + "| --- | --- | --- |", + "| 01 | `first` | Do it |", + ].join("\n"); + const violations = checkArchitecture( + "plugins/aidd-fixture-a/skills/01-unterminated/SKILL.md", + content, + ["01-first.md"] + ); + assert.deepEqual(violations, []); +}); diff --git a/scripts/__tests__/fixtures/architecture-rules/agent-permission-list.md b/scripts/__tests__/fixtures/architecture-rules/agent-permission-list.md new file mode 100644 index 000000000..71057c071 --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/agent-permission-list.md @@ -0,0 +1,12 @@ +# Role + +You are the reviewer. + +# Behavior + +- Do your job. + +# Skills you may invoke + +- `/aidd-fixture-b:01-noop` +- `/aidd-fixture-c:02-other` diff --git a/scripts/__tests__/fixtures/architecture-rules/assets-note.md b/scripts/__tests__/fixtures/architecture-rules/assets-note.md new file mode 100644 index 000000000..891150a5d --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/assets-note.md @@ -0,0 +1,3 @@ +# Recipe + +Type `/aidd-fixture-b:01-noop` to trigger the sibling flow from your terminal. diff --git a/scripts/__tests__/fixtures/architecture-rules/bare-address-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/bare-address-skill/SKILL.md new file mode 100644 index 000000000..99822bb50 --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/bare-address-skill/SKILL.md @@ -0,0 +1,10 @@ +# Bare Address + +Declare it the same way `aidd-fixture-b:01-noop` does. Do not confuse this with +some-aidd-fixture-b:02-other, which names no real plugin and must stay silent. + +## Actions + +| Action | Does | +| --- | --- | +| `step` | does the one thing | diff --git a/scripts/__tests__/fixtures/architecture-rules/clean-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/clean-skill/SKILL.md new file mode 100644 index 000000000..1712689ab --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/clean-skill/SKILL.md @@ -0,0 +1,15 @@ +# Clean + +A minimal skill with no sibling address and one action. + +## Actions + +Run `step`. + +| Action | Does | +| --- | --- | +| step | does the one thing | + +## Notes + +Nothing else here. diff --git a/scripts/__tests__/fixtures/architecture-rules/cross-address-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/cross-address-skill/SKILL.md new file mode 100644 index 000000000..ee2e8806a --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/cross-address-skill/SKILL.md @@ -0,0 +1,11 @@ +# Cross Address + +This skill wrongly calls a sibling plugin directly. + +## Actions + +Run `step`, then hand off to `/aidd-fixture-b:01-noop` for the rest. + +| Action | Does | +| --- | --- | +| step | does the one thing | diff --git a/scripts/__tests__/fixtures/architecture-rules/deleted-row-masked-by-prose-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/deleted-row-masked-by-prose-skill/SKILL.md new file mode 100644 index 000000000..4e8fc0967 --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/deleted-row-masked-by-prose-skill/SKILL.md @@ -0,0 +1,9 @@ +# Multi Step + +## Actions + +Run them in order, `01 → 04`. The plan is the culmination. + +| # | Action | Role | +| --- | --- | --- | +| 01 | `frame` | Frame it | diff --git a/scripts/__tests__/fixtures/architecture-rules/glossary-column-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/glossary-column-skill/SKILL.md new file mode 100644 index 000000000..a9959b5f3 --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/glossary-column-skill/SKILL.md @@ -0,0 +1,14 @@ +# Glossary column skill + +## Actions + +Nothing routes here. The table below defines words, it does not dispatch. + +| Term | Synonym | +| --- | --- | +| step | move | +| other | further | + +## Transversal rules + +- Nothing. diff --git a/scripts/__tests__/fixtures/architecture-rules/nested-action.md b/scripts/__tests__/fixtures/architecture-rules/nested-action.md new file mode 100644 index 000000000..701a6a7a6 --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/nested-action.md @@ -0,0 +1 @@ +Nested one level under `actions/`, this still names a sibling directly: `aidd-fixture-b:01-noop`. diff --git a/scripts/__tests__/fixtures/architecture-rules/nested-assets-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/nested-assets-skill/SKILL.md new file mode 100644 index 000000000..47b6d98b3 --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/nested-assets-skill/SKILL.md @@ -0,0 +1,5 @@ +# Nested Under Assets + +This file sits under an `assets/` segment even though it is named `SKILL.md` and would +otherwise be governed at this depth. Naming a sibling here, `aidd-fixture-b:01-noop`, must +stay silent. diff --git a/scripts/__tests__/fixtures/architecture-rules/nested-reference.md b/scripts/__tests__/fixtures/architecture-rules/nested-reference.md new file mode 100644 index 000000000..1875629ed --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/nested-reference.md @@ -0,0 +1,4 @@ +# Nested reference + +Nested two levels under `references/`, this still names a sibling directly: +`aidd-fixture-b:01-noop`. diff --git a/scripts/__tests__/fixtures/architecture-rules/nested-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/nested-skill/SKILL.md new file mode 100644 index 000000000..4b93cd5be --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/nested-skill/SKILL.md @@ -0,0 +1,4 @@ +# Nested Skill + +Nested one level deeper than usual, this still names a sibling directly: +`aidd-fixture-b:01-noop`. diff --git a/scripts/__tests__/fixtures/architecture-rules/no-actions-section-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/no-actions-section-skill/SKILL.md new file mode 100644 index 000000000..a9022d99d --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/no-actions-section-skill/SKILL.md @@ -0,0 +1,7 @@ +# No actions section skill + +This skill ships an action file and never declares a router for it. + +## Transversal rules + +- Nothing. diff --git a/scripts/__tests__/fixtures/architecture-rules/onboard-menu.md b/scripts/__tests__/fixtures/architecture-rules/onboard-menu.md new file mode 100644 index 000000000..2c8cbd610 --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/onboard-menu.md @@ -0,0 +1,5 @@ +# Onboard routing menu + +| Situation | Route to | +| --- | --- | +| start new work | `aidd-orchestrator:01-sdlc` | diff --git a/scripts/__tests__/fixtures/architecture-rules/orchestrator-incoherent-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/orchestrator-incoherent-skill/SKILL.md new file mode 100644 index 000000000..32addd9da --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/orchestrator-incoherent-skill/SKILL.md @@ -0,0 +1,8 @@ +# Orchestrated Router + +Dispatches broadly across plugins. + +## Actions + +Hand off to `/aidd-fixture-a:02-cross-address` depending on intent, and say nothing else about +what this skill itself provides. diff --git a/scripts/__tests__/fixtures/architecture-rules/orchestrator-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/orchestrator-skill/SKILL.md new file mode 100644 index 000000000..6e9d99c02 --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/orchestrator-skill/SKILL.md @@ -0,0 +1,7 @@ +# Orchestrated Router + +Dispatches broadly across plugins. + +## Actions + +Route to `/aidd-fixture-a:02-cross-address` or `/aidd-fixture-b:01-noop` depending on intent. diff --git a/scripts/__tests__/fixtures/architecture-rules/stem-substring-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/stem-substring-skill/SKILL.md new file mode 100644 index 000000000..b2b7bbf35 --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/stem-substring-skill/SKILL.md @@ -0,0 +1,7 @@ +# Two similarly named actions + +## Actions + +| # | Action | Facet | +| --- | --- | --- | +| 02 | `assert-architecture` | Report where the code breaks the documented architecture | diff --git a/scripts/__tests__/fixtures/architecture-rules/unnamed-action-skill/SKILL.md b/scripts/__tests__/fixtures/architecture-rules/unnamed-action-skill/SKILL.md new file mode 100644 index 000000000..1347d874a --- /dev/null +++ b/scripts/__tests__/fixtures/architecture-rules/unnamed-action-skill/SKILL.md @@ -0,0 +1,9 @@ +# Two Steps + +## Actions + +Run `step-one` to begin. + +| Action | Does | +| --- | --- | +| step-one | begins | diff --git a/scripts/check-architecture-rules.js b/scripts/check-architecture-rules.js new file mode 100644 index 000000000..18b2ec53d --- /dev/null +++ b/scripts/check-architecture-rules.js @@ -0,0 +1,49 @@ +#!/usr/bin/env node + +/** + * The two architecture rules of issue #250, at commit time. The write-time hook only ever sees + * Claude Code; this sees every edit that reaches a commit, whichever tool or person made it. + * + * Paths given as arguments are checked; with none, the whole governed tree is. `--root ` + * moves both, which is what lets the failing path be exercised against a tree that is not this + * repository — a gate whose refusal nothing tests is a gate nobody can trust. + */ + +"use strict"; + +const path = require("node:path"); + +const { describeFix, governedPaths, scan } = require("./lib/architecture-scan.js"); + +const DEFAULT_ROOT = path.resolve(__dirname, ".."); + +function parse(argv) { + const at = argv.indexOf("--root"); + if (at === -1) return { root: DEFAULT_ROOT, requested: argv.filter((a) => !a.startsWith("-")) }; + + const root = path.resolve(argv[at + 1] ?? "."); + const rest = [...argv.slice(0, at), ...argv.slice(at + 2)]; + return { root, requested: rest.filter((a) => !a.startsWith("-")) }; +} + +function main(argv) { + const { root, requested } = parse(argv); + const relative = (argument) => + path.relative(root, path.resolve(root, argument)).split(path.sep).join("/"); + const paths = requested.length > 0 ? requested.map(relative) : governedPaths(root); + const violations = scan(root, paths); + + if (violations.length === 0) { + console.log(`✅ Architecture rules: ${paths.length} governed file(s) checked, no violation`); + return 0; + } + + for (const violation of violations) { + console.error(`❌ ${violation.message}`); + console.error(` Fix: ${describeFix(violation)}.`); + } + console.error(`\n${violations.length} violation(s) across ${paths.length} checked file(s).`); + return 1; +} + +process.exitCode = main(process.argv.slice(2)); diff --git a/scripts/lib/architecture-rules.js b/scripts/lib/architecture-rules.js new file mode 100644 index 000000000..3ea9ade7a --- /dev/null +++ b/scripts/lib/architecture-rules.js @@ -0,0 +1,310 @@ +/** + * The two architecture rules of issue #250, as pure functions: the caller supplies the path, the + * prospective content and a SKILL.md's action file names, and nothing here reads the filesystem. + * + * Why each rule is shaped the way it is — the one-directional router check, the exemptions, the + * two table shapes read generously — is in the decisions table of + * `aidd_docs/tasks/2026_09/2026_09_18_cross-plugin-orthogonality-guard/plan.md`. + */ + +"use strict"; + +const ORCHESTRATOR_PLUGIN = "aidd-orchestrator"; + +/** Expires with #883, which makes 00-onboard resolve its providers at runtime. */ +const ONBOARD_EXEMPT_PREFIX = "plugins/aidd-context/skills/00-onboard/"; + +const ANY_HEADING = /^#{1,6}\s/; +const PERMISSION_LIST_HEADING = /^#{1,6}\s+Skills you may invoke\s*$/i; +const ACTIONS_HEADING = /^##\s+Actions\s*$/i; +const SECOND_LEVEL_HEADING = /^##\s+/; +const FENCE_MARKER = /^\s*(`{3,}|~{3,})/; +const TABLE_ROW = /^\s*\|/; + +const PLUGIN_ADDRESS = /(? { + PLUGIN_ADDRESS.lastIndex = 0; + let match; + while ((match = PLUGIN_ADDRESS.exec(line)) !== null) { + addresses.push({ line: index + 1, plugin: match[1], text: match[0] }); + } + }); + return addresses; +} + +/** An agent's `# Skills you may invoke` list names its providers on purpose. */ +function permissionListLines(lines) { + const listed = new Set(); + let inList = false; + lines.forEach((line, index) => { + if (ANY_HEADING.test(line)) inList = PERMISSION_LIST_HEADING.test(line); + else if (inList) listed.add(index + 1); + }); + return listed; +} + +function isExemptFromOrthogonality(filePath, owner) { + return owner === ORCHESTRATOR_PLUGIN || filePath.startsWith(ONBOARD_EXEMPT_PREFIX); +} + +function checkOrthogonality(filePath, content) { + const surface = classifyFile(filePath); + if (!surface || isExemptFromOrthogonality(filePath, surface.owner)) return []; + + const lines = toLines(content); + const exemptLines = surface.kind === "agent" ? permissionListLines(lines) : new Set(); + + return addressesIn(content) + .filter(({ plugin, line }) => plugin !== surface.owner && !exemptLines.has(line)) + .map(({ plugin, line, text }) => + violation( + "orthogonality", + filePath, + line, + plugin, + `${filePath}:${line} addresses sibling plugin "${plugin}" via "${text}"` + ) + ); +} + +// --- Reading a `## Actions` section -------------------------------------------------------- + +function fenceCharacter(line) { + return line.match(FENCE_MARKER)?.[1][0] ?? null; +} + +function hasUnterminatedFence(lines) { + let open = null; + for (const line of lines) { + const marker = fenceCharacter(line); + if (marker === null) continue; + open = open === null ? marker : open === marker ? null : open; + } + return open !== null; +} + +/** + * Every fenced line blanked, line count intact so a reported number still points at its line. A + * fence holds an example, never a router — except an unterminated one, which is not a fence. + */ +function withoutFences(lines) { + if (hasUnterminatedFence(lines)) return lines; + + let open = null; + return lines.map((line) => { + const marker = fenceCharacter(line); + if (open === null && marker === null) return line; + if (open === null) open = marker; + else if (marker === open) open = null; + return ""; + }); +} + +function actionsSection(lines) { + const headingIndex = lines.findIndex((line) => ACTIONS_HEADING.test(line)); + if (headingIndex === -1) return null; + + const after = lines.slice(headingIndex + 1); + const nextHeading = after.findIndex((line) => SECOND_LEVEL_HEADING.test(line)); + + return { + headingLine: headingIndex + 1, + startIndex: headingIndex + 1, + endIndex: nextHeading === -1 ? lines.length : headingIndex + 1 + nextHeading, + }; +} + +function tableCells(line) { + return line.trim().replace(/^\|/, "").replace(/\|$/, "").split("|").map((cell) => cell.trim()); +} + +/** Contiguous runs of table rows: one table's header never speaks for the next one's columns. */ +function tableBlocks(lines) { + const blocks = []; + let current = []; + + for (const line of lines) { + if (TABLE_ROW.test(line)) { + current.push(line); + continue; + } + if (current.length > 0) { + blocks.push(current); + current = []; + } + } + if (current.length > 0) blocks.push(current); + + return blocks; +} + +function isSeparatorRow(cells) { + return cells.length > 0 && cells.every((cell) => SEPARATOR_CELL.test(cell)); +} + +function declaredActionColumn(headerCells) { + return headerCells.findIndex((cell) => ACTION_COLUMN_HEADER.test(stripBackticks(cell))); +} + +// --- Rule two: router coherence ------------------------------------------------------------- + +/** + * Names cited from the column a table calls `Action`. A run of rows with no separator beneath it + * is that table resumed after a blank line, so it keeps the column its header declared. + */ +function citationsFromActionColumns(sectionLines) { + const cited = new Set(); + let actionColumn = -1; + + for (const block of tableBlocks(sectionLines)) { + const rows = block.map(tableCells); + const declaresItsOwnColumns = rows.length > 1 && isSeparatorRow(rows[1]); + + if (declaresItsOwnColumns) actionColumn = declaredActionColumn(rows[0]); + if (actionColumn === -1) continue; + + for (const cells of declaresItsOwnColumns ? rows.slice(1) : rows) { + if (isSeparatorRow(cells)) continue; + const cell = stripBackticks(cells[actionColumn] ?? ""); + if (CITATION_TOKEN.test(cell)) cited.add(cell.toLowerCase()); + } + } + return cited; +} + +function matchesOf(pattern, text) { + pattern.lastIndex = 0; + const found = new Set(); + let match; + while ((match = pattern.exec(text)) !== null) found.add(match[1].toLowerCase()); + return found; +} + +/** + * The three citation shapes. Only the `actions/.md` path is read through a fence, because + * `aidd-dev:10-todo` cites its one action that way; a backticked file name inside a fence is an + * example. A word loose in prose is never a citation. + */ +function citationsIn(blankedLines, rawLines) { + return new Set([ + ...citationsFromActionColumns(blankedLines), + ...matchesOf(ACTION_PATH, rawLines.join("\n")), + ...matchesOf(BACKTICKED_FILE_NAME, blankedLines.join("\n")), + ]); +} + +/** A stem two action files share cites neither: one row would otherwise cover both. */ +function ambiguousStems(actionFileNames) { + const seen = new Set(); + const shared = new Set(); + for (const name of actionFileNames) { + const stem = stemOf(name).toLowerCase(); + if (seen.has(stem)) shared.add(stem); + seen.add(stem); + } + return shared; +} + +function isCited(actionFileName, citations, sharedStems) { + const stem = stemOf(actionFileName).toLowerCase(); + return ( + citations.has(actionFileName.toLowerCase()) || + citations.has(actionFileName.replace(/\.md$/i, "").toLowerCase()) || + (!sharedStems.has(stem) && citations.has(stem)) + ); +} + +function checkRouterCoherence(filePath, content, actionFileNames) { + const surface = classifyFile(filePath); + if (!surface || surface.kind !== "skill") return []; + + const names = actionFileNames ?? []; + if (names.length === 0) return []; + + const rawLines = toLines(content); + const lines = withoutFences(rawLines); + const section = actionsSection(lines); + + if (!section) { + const message = `${filePath} has action files but no "## Actions" section`; + return [violation("router-coherence", filePath, 1, surface.owner, message)]; + } + + const { startIndex, endIndex, headingLine } = section; + const citations = citationsIn(lines.slice(startIndex, endIndex), rawLines.slice(startIndex, endIndex)); + const sharedStems = ambiguousStems(names); + + return names + .filter((name) => !isCited(name, citations, sharedStems)) + .map((name) => + violation( + "router-coherence", + filePath, + headingLine, + surface.owner, + `${filePath}:${headingLine} "## Actions" never names action file "${name}"` + ) + ); +} + +function checkArchitecture(filePath, content, actionFileNames) { + return [ + ...checkOrthogonality(filePath, content), + ...checkRouterCoherence(filePath, content, actionFileNames), + ]; +} + +module.exports = { + checkArchitecture, + checkOrthogonality, + checkRouterCoherence, + classifyFile, +}; diff --git a/scripts/lib/architecture-scan.js b/scripts/lib/architecture-scan.js new file mode 100644 index 000000000..75b8cbafb --- /dev/null +++ b/scripts/lib/architecture-scan.js @@ -0,0 +1,87 @@ +/** + * The filesystem layer the two rules need: `architecture-rules.js` stays pure, and everything + * that reads a directory or a file lives here, so the write-time hook and the commit-time check + * ask the same questions the same way. + */ + +"use strict"; + +const fs = require("node:fs"); +const path = require("node:path"); + +const { checkArchitecture, classifyFile } = require("./architecture-rules.js"); + +const CITATION_SHAPES = + "a cell under a table header that reads Action, a fenced `actions/.md` path, or a " + + "backticked `.md` file name — a word in prose does not count"; + +/** Rule two needs the skill's action files; the engine never reads them itself. */ +function actionFileNames(relativePath, absolutePath) { + if (path.basename(relativePath) !== "SKILL.md") return undefined; + try { + return fs + .readdirSync(path.join(path.dirname(absolutePath), "actions")) + .filter((name) => name.endsWith(".md")); + } catch { + return []; + } +} + +/** Violations for content that may not be on disk yet, which is what the write-time hook holds. */ +function violationsForFile(relativePath, content, absolutePath) { + try { + const found = checkArchitecture(relativePath, content, actionFileNames(relativePath, absolutePath)); + return Array.isArray(found) ? found : []; + } catch { + return []; + } +} + +function describeFix({ rule, plugin }) { + return rule === "orthogonality" + ? `name the concept ${plugin} owns instead of addressing it directly` + : `cite every action file the skill provides in its "## Actions" section. A citation is ${CITATION_SHAPES}`; +} + +function markdownUnder(directory, root, found) { + for (const entry of fs.readdirSync(directory, { withFileTypes: true })) { + const full = path.join(directory, entry.name); + if (entry.isDirectory()) markdownUnder(full, root, found); + else if (entry.name.endsWith(".md")) { + found.push(path.relative(root, full).split(path.sep).join("/")); + } + } + return found; +} + +/** Every governed path in the tree, for the whole-tree run CI does. */ +function governedPaths(root) { + const plugins = path.join(root, "plugins"); + if (!fs.existsSync(plugins)) return []; + return markdownUnder(plugins, root, []).filter((relativePath) => classifyFile(relativePath)); +} + +/** Violations across paths already on disk. A path that is not governed contributes none. */ +function scan(root, relativePaths) { + const violations = []; + for (const relativePath of relativePaths) { + if (!classifyFile(relativePath)) continue; + const absolutePath = path.join(root, relativePath); + let content; + try { + content = fs.readFileSync(absolutePath, "utf8"); + } catch { + continue; + } + violations.push(...violationsForFile(relativePath, content, absolutePath)); + } + return violations; +} + +module.exports = { + actionFileNames, + describeFix, + governedPaths, + scan, + violationsForFile, +};