diff --git a/corpus/skills/principle-prove-it/SKILL.md b/corpus/skills/principle-prove-it/SKILL.md new file mode 100644 index 0000000..c3fe5bf --- /dev/null +++ b/corpus/skills/principle-prove-it/SKILL.md @@ -0,0 +1,66 @@ +--- +name: principle-prove-it +description: "Apply before any claim about code, systems, or history. No claim without same-turn evidence; a hedge is a trigger to verify, never a place to stop. Routes the four investigation products that build the evidence. Unlike the other principles this one auto-fires: it must intercept a claim as it forms, not be named after the fact." +--- + +# Prove It + +A claim is settled only when the evidence for it is in the same message. +Everything else is a hypothesis and must say so. + +**Why:** the cheap signal always stands in for the expensive one. A build +passes and gets reported as "it works." A file exists and gets reported as +"it's wired up." A name matches and gets reported as "that's the bug." Each +substitution is individually reasonable and collectively how wrong answers +reach the user with confidence attached. + +## The gate + +Before writing any claim of the form "this is fixed," "this works," "the +cause is X," "it's merged," "N are running" — you must already have, in the +same message, one of: + +1. A command run this turn, with its real pasted output. Not summarized. +2. A `file:line` read this turn, named with the ref it was read at. +3. A test name plus its real pass/fail line from the runner. + +Otherwise write `UNVERIFIED:` immediately before the claim. There is no +softer wording. The full evidence rules live in `engine/CLAUDE.core.md` and +are always loaded; this file is the judgment half plus the routing below. + +**A hedge is a trigger.** "I think," "probably," "should work" mean run the +check now, not lower the confidence and continue. + +**Absence of output is not proof of success.** A command that printed +nothing needs its exit code shown. + +## What builds the evidence + +The gate says what counts. It does not gather anything. Four product skills +do that, each fanning out to parallel subagents and returning the shape in +`corpus/skills/principle-prove-it/references/finding-shape.md`: + +| Product | Question | Returns | +| --- | --- | --- | +| `how` | How does this work now? | The mechanism you must exercise | +| `why` | Why is it shaped this way? | The constraint you must not break | +| `alternatives-considered` | What else could this be? | Options, each labeled considered or invented | +| `spike-and-validate` | Does the invented one hold? | A throwaway that ran, and its real output | + +They compose in that order and each is optional. A one-line fix needs none +of them. A claim about a subsystem nobody on the team has read needs `how` +before anything else. + +The pairing that matters: `alternatives-considered` **generates**, so it can +always produce three plausible options and none of them are evidence. +`spike-and-validate` is its gate. An invented option that reaches a decision +without a spike is exactly the manufactured evidence this principle exists +to stop. + +## Related + +- `prove-it-ship-gate` — the done/shipped auto-route for live side effects. +- `principle-fix-root-causes` — reproduce before explaining. +- `principle-sequence-verifiable-units` — end each unit in a check. +- Mechanical enforcement: `engine/hooks/prove-it-ship-gate`, + `engine/hooks/hedge-runs-prove-it`, `engine/hooks/diu-stop`. diff --git a/corpus/skills/principle-prove-it/references/finding-shape.md b/corpus/skills/principle-prove-it/references/finding-shape.md new file mode 100644 index 0000000..28379a1 --- /dev/null +++ b/corpus/skills/principle-prove-it/references/finding-shape.md @@ -0,0 +1,45 @@ +# Shared finding shape + +Every investigation product (`how`, `why`, `alternatives-considered`, +`spike-and-validate`) fans out to parallel subagents and merges their +returns. They share this shape so `principle-prove-it` can consume any of +them without a per-skill adapter. + +This is **not** the judge board in `product/skills/independent-judge-swarm`. +That board grades a finished artifact (`verdict`, `score`, +`blocking_issues`). These skills return findings about work that has not +happened yet, so there is nothing to grade. + +## Fields + +| Field | Shape | +| --- | --- | +| `lens` | string — which angle this subagent owned | +| `claim` | string — one sentence, the thing found | +| `grounding` | `read-confirmed` \| `name-matched` \| `inferred` \| `invented` | +| `evidence` | `file:line` + ref, a command + its real output, or a URL | +| `ref` | which ref `evidence` was read at: working tree, `HEAD`, `origin/`, installed bundle | +| `null_result` | string — what was searched and came back empty | + +## Rules + +- A subagent that found nothing still returns a row with `null_result` set. + Silence is not the same as absence, and a skipped source is not a null — + see the Evidence rules in `engine/CLAUDE.core.md`. +- `grounding: read-confirmed` means the subagent opened the actual + reference and traced it. `name-matched` means the path or symbol name + looked right. A working-tree read under a dirty path is `name-matched`. +- `grounding: inferred` and `grounding: invented` rows carry no authority + on their own. They are hypotheses. Route an `invented` row that matters + to `spike-and-validate` before it reaches a decision. +- The merge step reports disagreement rather than resolving it silently. + +## Merge + +1. Group rows by `claim`. +2. Two subagents reaching the same `read-confirmed` claim independently is + the strongest signal available here. Check they were not handed the same + anchor and prompt first — template-identical attempts converge on the + same wrong answer exactly like independent ones agree. +3. Any `read-confirmed` row beats any number of `inferred` rows. +4. Report every `null_result` in the output. Do not drop it for brevity. diff --git a/corpus/skills/principle-prove-it/tests/fires_example.md b/corpus/skills/principle-prove-it/tests/fires_example.md new file mode 100644 index 0000000..69bdc4a --- /dev/null +++ b/corpus/skills/principle-prove-it/tests/fires_example.md @@ -0,0 +1,15 @@ +An agent has just edited a retry handler and is about to write "this fixes +the duplicate-write bug" into a PR body. Nothing ran this turn: no repro, no +test, no command output. The agent never types a slash command. + +This skill fires on the claim shape itself. Unlike the other `principle-*` +skills, it does not carry `disable-model-invocation: true`, so its +`description:` is loaded and the model can match it — which is the whole +point: a gate that only works when someone remembers to name it is not a +gate. A fix claim with no evidence in the same message is exactly what the +description targets, and once loaded the skill supplies both the three +accepted evidence forms and the routing table that sends the agent to `how` +to find which artifact to exercise. + +The same auto-fire path covers the hedge case: "this should work" is a +trigger to run the check, not a softer way to state the claim. diff --git a/corpus/skills/principle-prove-it/tests/stays_silent_example.md b/corpus/skills/principle-prove-it/tests/stays_silent_example.md new file mode 100644 index 0000000..a2a3226 --- /dev/null +++ b/corpus/skills/principle-prove-it/tests/stays_silent_example.md @@ -0,0 +1,12 @@ +A user asks which name reads better for a local variable in a helper they are +writing, `retryCount` or `attempts`, and the agent gives an opinion. + +This skill stays silent. Auto-firing is driven by the presence of a claim to +hold evidence against, and there is none here: a naming preference asserts +nothing about behavior, system state, history, or a cause. There is no +"this is fixed", no "the cause is X", no hedge about code that could be +checked by running something. + +Firing here would be the skill misbehaving rather than working — demanding a +command run and pasted output before answering a style question is the +false-positive this fixture exists to pin down. diff --git a/docs/skill-triggers.md b/docs/skill-triggers.md new file mode 100644 index 0000000..094d147 --- /dev/null +++ b/docs/skill-triggers.md @@ -0,0 +1,72 @@ +# Skill triggers: what the model may fire on its own + +Every skill in this repo is in one of two states, set by one line of +frontmatter. + +**Auto-fire** (no flag). Claude Code loads the skill's `description:` and the +model may invoke the skill when a turn matches it. Use this for a gate or a +procedure that is worthless if it only runs when someone remembers to name +it — `principle-prove-it` fires on a claim taking shape, `narrow-the-scope` +on a loop that is not converging. + +**Explicit only** (`disable-model-invocation: true`). The `description:` is +never loaded into context, so no description match can reach the skill. The +only way in is a typed `/`. Use this for steering vocabulary you reach +for deliberately (the `principle-*` set), personal convention files +(`*-mode`), and anything whose blast radius demands a human in the loop. + +## How to choose + +Ask what happens when the model guesses wrong in each direction. + +| | A false fire costs | A missed fire costs | +| --- | --- | --- | +| Gate / evidence rule | A little context | A wrong claim reaching the user | +| Steering vocabulary | Context on 25 principles every task | Nothing — you name it when you want it | +| Irreversible action | A force-merge nobody asked for | A human types six more words | + +Auto-fire when a missed fire is the expensive direction. Flag it when a +false fire is. + +## Prose is not a trigger mechanism + +A skill cannot opt out of auto-invocation by asking. `admin-bypass-sweep` +force-merges PRs past required checks, and its description said "MANUAL, +HUMAN-ONLY ... Do not auto-invoke this skill from a natural-language +request, a description match, or another agent's delegation" — while +remaining fully auto-invocable, because the flag was missing. The model reads +that description to decide whether to fire; the sentence asking it not to is +inside the thing it is matching on. + +`scripts/check_skill_trigger_policy.py` fails closed on that contradiction: +a skill that declares itself manual in its own frontmatter must carry the +flag. It also regenerates the inventory below, because a hand-maintained +list of 35+ skills is stale by the next PR. + +```sh +python3 scripts/check_skill_trigger_policy.py # verify +python3 scripts/check_skill_trigger_policy.py --write # regenerate +``` + +Fixture contracts follow from the same line — see +`scripts/check_skill_trigger_mechanism.py`. A flagged skill's +`fires_*.md` must contain its literal `/`; an auto-fire skill's must +instead share vocabulary with its `description:`. + +## Inventory + + + +### Auto-fire (17) + +The model may invoke these from a description match. Everything here is a gate or a procedure that is useless if it only runs when named. + +`create-skill`, `draft-pr`, `make-pr`, `thrash-reflect-automate`, `principle-prove-it`, `prove-it-ship-gate`, `alternatives-considered`, `diu`, `how`, `land-stack`, `loop-generator`, `narrow-the-scope`, `show-me-your-work`, `spike-and-validate`, `split-scope`, `visual-proof`, `why` + +### Explicit invocation only (30) + +These carry `disable-model-invocation: true`. Claude Code does not load their `description:` at all, so the only way in is a typed `/`. + +`automate-me`, `reflect`, `cat-mode`, `principle-assert-invariants-not-last-bug`, `principle-bind-to-named-inventory`, `principle-build-the-lever`, `principle-encode-lessons-in-structure`, `principle-experience-first`, `principle-explicit-errors`, `principle-fix-root-causes`, `principle-flag-your-own-corrections`, `principle-foundational-thinking`, `principle-generalize-from-rejection`, `principle-guard-the-context-window`, `principle-laziness-protocol`, `principle-manage-idle-resumption`, `principle-minimize-reader-load`, `principle-name-the-scorer`, `principle-never-block-on-the-human`, `principle-outcome-oriented-execution`, `principle-push-not-poll`, `principle-scope-the-session`, `principle-separate-before-serializing-shared-state`, `principle-sequence-verifiable-units`, `principle-subtract-before-you-add`, `principle-trace-token-burn-loop`, `principle-type-system-discipline`, `admin-bypass-sweep`, `i-have-adhd`, `independent-judge-swarm` + + diff --git a/engine/hooks/no-comments/detect.py b/engine/hooks/no-comments/detect.py index fb4222f..43c5b16 100644 --- a/engine/hooks/no-comments/detect.py +++ b/engine/hooks/no-comments/detect.py @@ -5,7 +5,9 @@ comments: shebangs, encoding lines, noqa, type:, pragma, pylint, mypy, eslint, prettier, ts-ignore, ts-expect-error, istanbul, nosec, ruff, fmt, and SPDX or license headers. Markdown, JSON, YAML, TOML and other -non-code files are out of scope. Docstrings are out of scope. +non-code files are out of scope. Python triple-quoted strings are out +of scope: a docstring's usage examples and a markdown fixture's '#' +headings are string content, not comments. """ from __future__ import annotations @@ -34,6 +36,7 @@ HASH_LINE_RE = re.compile(r"^\s*#") SLASH_LINE_RE = re.compile(r"^\s*(?://|/\*|\*(?!/)|\n
\n"))) + def test_blocks_a_real_comment_after_a_closed_triple_quoted_string(self): + hits = detect.comment_lines("/r/t.py", COMMENT_AFTER_A_CLOSED_TRIPLE_QUOTED_STRING) + self.assertEqual(hits, ["# real shape: the flag was missing here"]) + def test_blocks_write_and_multiedit_shapes(self): w = {"tool_name": "Write", "tool_input": {"file_path": "/r/x.sh", "content": "#!/bin/bash\n# step one\nls\n"}} m = {"tool_name": "MultiEdit", "tool_input": {"file_path": "/r/x.py", "edits": [{"new_string": "a = 1\n"}, {"new_string": "# TODO\n"}]}} @@ -77,6 +100,12 @@ def test_allows_hash_and_slashes_inside_strings_and_urls(self): def test_allows_docstrings_and_plain_code(self): self.assertEqual(detect.comment_lines("/r/a.py", '"""Module doc.\n\nMore prose.\n"""\ndef f():\n return 1\n'), []) + def test_allows_markdown_headings_inside_a_python_triple_quoted_fixture(self): + self.assertEqual(detect.comment_lines("/r/t.py", MARKDOWN_FIXTURE_IN_TRIPLE_QUOTES), []) + + def test_allows_trailing_hash_usage_lines_inside_a_module_docstring(self): + self.assertEqual(detect.comment_lines("/r/a.py", DOCSTRING_WITH_TRAILING_HASH_USAGE), []) + def test_allows_non_code_files(self): self.assertIsNone(detect.decide(edit("/r/README.md", "# Heading\n"))) self.assertIsNone(detect.decide(edit("/r/ci.yml", "# comment in yaml\n"))) diff --git a/engine/skills/make-pr/scripts/preflight.py b/engine/skills/make-pr/scripts/preflight.py index b7ce7f5..3a2a5b5 100644 --- a/engine/skills/make-pr/scripts/preflight.py +++ b/engine/skills/make-pr/scripts/preflight.py @@ -102,6 +102,7 @@ def gates_for(paths: list[str], base: str | None = None) -> list[list[str]]: ["python3", "scripts/check_skill_file_refs.py"], ["python3", "scripts/check_skill_test_coverage.py"], ["python3", "scripts/check_skill_trigger_mechanism.py"], + ["python3", "scripts/check_skill_trigger_policy.py"], ] return cmds diff --git a/engine/skills/make-pr/tests/test_preflight.py b/engine/skills/make-pr/tests/test_preflight.py index 4abb8a4..f9b6d28 100644 --- a/engine/skills/make-pr/tests/test_preflight.py +++ b/engine/skills/make-pr/tests/test_preflight.py @@ -55,6 +55,16 @@ def test_gates_for_rule_prose_include_codify_check(self): def test_gates_for_neutral_only_are_empty(self): self.assertEqual(pf.gates_for(["docs/ecosystem.md", "README.md"]), []) + def test_gates_for_skill_slice_include_trigger_policy(self): + """A skill slice must run the trigger-policy gate. + + It catches a skill declaring itself human-only with no + disable-model-invocation behind it -- the admin-bypass-sweep shape, + where a force-merge skill was reachable by description match. + """ + cmds = pf.gates_for(["product/skills/how/SKILL.md"]) + self.assertIn(["python3", "scripts/check_skill_trigger_policy.py"], cmds) + def test_gates_for_rule_prose_with_base_includes_dated_provenance_check(self): self.assertIn( ["python3", "scripts/check_no_dated_provenance.py", "--base", "origin/main"], diff --git a/product/skills/admin-bypass-sweep/SKILL.md b/product/skills/admin-bypass-sweep/SKILL.md index f3b0504..becb9df 100644 --- a/product/skills/admin-bypass-sweep/SKILL.md +++ b/product/skills/admin-bypass-sweep/SKILL.md @@ -9,6 +9,7 @@ description: > has typed its literal slash command themselves in the current turn (the exact form depends on how it was installed — see the "STOP" section). See that section before doing anything. +disable-model-invocation: true --- # admin-bypass-sweep diff --git a/product/skills/alternatives-considered/SKILL.md b/product/skills/alternatives-considered/SKILL.md new file mode 100644 index 0000000..ee64b39 --- /dev/null +++ b/product/skills/alternatives-considered/SKILL.md @@ -0,0 +1,87 @@ +--- +name: alternatives-considered +description: >- + Produce the real option set for a decision before committing to the first + shape that came to mind. Use before a design, a schema, a naming or format + choice, or any change expensive to reverse. Every option is labeled + considered (cited) or invented (untested); invented options route to + `spike-and-validate` before they can decide anything. +--- + +# Alternatives considered + +One attempt at a hard design locks in the first shape the model thought of. +This produces the option set, with each option honest about where it came +from. + +## The asymmetry that governs this skill + +`how` and `why` read what exists. They can come back empty, and an empty +result is informative. + +This skill **generates**. It can always produce three plausible options, and +plausibility is free. Left ungated it manufactures evidence for +`principle-prove-it` — which is the exact failure that principle exists to +stop. + +So every option carries a label and the label is load-bearing: + +| Label | Means | Authority | +| --- | --- | --- | +| `considered` | Someone actually weighed this — cite the PR comment, commit, doc, or rejected design | Evidence | +| `invented` | Produced here, never tried | Hypothesis only | + +An `invented` option may not decide anything until `spike-and-validate` +turns it into a run with real output. Saying "we considered X and rejected +it" about an option nobody ran is a false claim about the record. + +## Step 1. Bound the decision + +State in one line: what is being chosen, and what makes it expensive to +reverse. A decision that is cheap to reverse does not need this skill — +make it, and move on. + +Pull **Avoid** from `why` if it ran. Options already tried and failed are +`considered` with a citation, not fresh ideas. + +## Step 2. Fan out + +2–4 subagents, spawned in **one message**, each given the same brief and a +different constraint to optimize for (fewest moving parts, easiest to +delete later, best for the caller, cheapest to migrate). Different +constraints, not different phrasings — same-brief subagents converge. + +Each returns rows in the shape at +`corpus/skills/principle-prove-it/references/finding-shape.md`, with +`grounding` set to `invented` unless it can cite where the option was +already weighed. + +Give each one its own worktree if it writes anything. Scope wording is not +filesystem isolation. + +## Step 3. Merge into an option set + +For each surviving option: + +- **Shape** — what the caller writes, first. Then types and boundaries. +- **Label** — `considered` with citation, or `invented`. +- **Cost to reverse** — the real question behind most of these decisions. +- **What would kill it** — the check that would rule it out. This line is + the spike brief. + +Include the status quo as an option. It is `considered` by definition and +frequently wins. + +## Step 4. Route + +- All options `considered` → decide now, cite the record. +- The leading option is `invented` and expensive to reverse → + `spike-and-validate` before committing. +- Subagents disagree on which constraint matters → that is the real + decision. Surface it instead of picking quietly. + +## Do not + +- Pad the set. Two real options beat four where two are strawmen. +- Present an `invented` option as prior art. +- Run this on a reversible decision. diff --git a/product/skills/alternatives-considered/tests/fires_example.md b/product/skills/alternatives-considered/tests/fires_example.md new file mode 100644 index 0000000..d0940ca --- /dev/null +++ b/product/skills/alternatives-considered/tests/fires_example.md @@ -0,0 +1,12 @@ +A user is designing a cache key format for a service and says: "this is +expensive to change once it's in production — what are our real options +here?" + +This skill fires. It is a design decision that is expensive to reverse, +which is the stated trigger, and the user is explicitly asking for the +option set rather than a recommendation. Step 2 fans out subagents each +optimizing a different constraint (fewest moving parts, cheapest to migrate, +best for the caller), and Step 3 labels every returned option `considered` +with a citation or `invented`. Because the leading option is expensive to +reverse, Step 4 routes it to `spike-and-validate` before it can decide +anything. diff --git a/product/skills/alternatives-considered/tests/stays_silent_example.md b/product/skills/alternatives-considered/tests/stays_silent_example.md new file mode 100644 index 0000000..0c66869 --- /dev/null +++ b/product/skills/alternatives-considered/tests/stays_silent_example.md @@ -0,0 +1,8 @@ +A user asks the agent to rename a local variable inside one function from +`res` to `response`, and confirms the function is not exported. + +This skill stays silent. The decision is cheap to reverse — a rename inside +one unexported function is a single edit undone by a single edit — and the +skill's Step 1 says a cheap-to-reverse decision does not need it. There is +no option set worth generating, and producing three labeled alternatives for +a local rename would be the padding its own "do not" list forbids. diff --git a/product/skills/how/SKILL.md b/product/skills/how/SKILL.md new file mode 100644 index 0000000..44ebc6f --- /dev/null +++ b/product/skills/how/SKILL.md @@ -0,0 +1,77 @@ +--- +name: how +description: >- + Trace how a subsystem actually works before changing it. Use for "how does + X work", a walkthrough before an edit, or placement questions ("where + should this live", "which package owns this"). Fans out read-only + explorers for a subsystem, one explainer for a narrow question. Use `why` + for motivation, `alternatives-considered` for options. +--- + +# How + +Build a working mental model of code you are about to change. The output is +the mechanism, not annotated source. + +Feeds `principle-prove-it`: you cannot exercise the real artifact until you +know which artifact is real. + +## Step 1. Size the question + +State your reading of the scope in one line, then proceed. The user can +redirect. + +- **Narrow** — one module, one function, one file. No fan-out. Read and + explain in a single pass. Go to Step 3. +- **Broad** — a subsystem across files or packages, a cross-cutting flow, an + architectural overview. Fan out first. Go to Step 2. + +When it is ambiguous, take the narrow path. A second pass is cheaper than +four subagents answering a question that had one file in it. + +## Step 2. Fan out (broad only) + +Split the question into 2–4 angles that do not overlap. Spawn all explorers +in **one message** so they run concurrently: + +- `subagent_type: Explore` (or `general-purpose` where Explore is absent) +- read-only; no edits, no worktree needed + +Each explorer owns one angle and returns rows in the shape at +`corpus/skills/principle-prove-it/references/finding-shape.md`. + +**Prompt hygiene:** give each explorer the question and the entry points, +not your working theory. A primed explorer finds roughly what you already +suspected. + +**Every explorer states grounding per row.** `read-confirmed` means it +opened the actual import or call site and traced it. `name-matched` means +the path looked right. Two files can hold the same symbol name; a subagent +reasoning by name-proximity hands back a confident wrong file. + +## Step 3. Explain + +Merge the rows and write the explanation yourself. Sections, dropping any +that do not apply: + +- **Overview** — what this does, in two sentences. +- **Runtime flow** — the actual call path, entry point to effect. +- **Key types** — the data structures the logic is written against. +- **Where things live** — `file:line` per claim, with the ref. +- **Gotchas** — the non-obvious parts, and what looks wrong but isn't. +- **Not traced** — every angle that came back empty or was not covered. + +Do not drop the last section for brevity. A gap you name is a gap the reader +can fill; a gap you omit reads as coverage. + +## Hand-off + +When `how` precedes a change, end with the mechanism that must still work +afterward, phrased as a command or check. That line is what +`principle-prove-it` will demand output from later. + +## Do not + +- Explain from a grep hit. A name match is not a read. +- Fan out on a question one file answers. +- Report a flow you inferred from function names as if you traced it. diff --git a/product/skills/how/tests/fires_example.md b/product/skills/how/tests/fires_example.md new file mode 100644 index 0000000..682c0f2 --- /dev/null +++ b/product/skills/how/tests/fires_example.md @@ -0,0 +1,11 @@ +A user is about to add a new notification channel and asks: "how does the +dedupe path work before I touch it? is there an n+1 when we look up +subscribers?" + +This skill fires. The question is a mechanism question about a subsystem the +agent has not read, asked ahead of an edit — the skill's stated trigger. It +spans more than one file (dedupe plus subscriber lookup), so Step 1 sizes it +broad and Step 2 fans out read-only explorers on non-overlapping angles. +Each returned row carries `read-confirmed` or `name-matched` grounding, and +the merged explanation ends with the mechanism that must still work after +the change — the line `principle-prove-it` will later demand output from. diff --git a/product/skills/how/tests/stays_silent_example.md b/product/skills/how/tests/stays_silent_example.md new file mode 100644 index 0000000..0a3f2a0 --- /dev/null +++ b/product/skills/how/tests/stays_silent_example.md @@ -0,0 +1,10 @@ +A user asks to bump a timeout constant from 30 to 60 seconds in a file they +are currently looking at, and pastes the line. The agent makes the one-token +edit. + +This skill stays silent. There is no mechanism to trace: the target is a +single literal in a file already in context, the user supplied the code, and +nothing about the change depends on understanding a call path. Step 1 would +size this narrow and the skill's own "do not" list rules out fanning +subagents at a question one file answers. Running `how` here would spend +subagents to re-explain a line the user just read. diff --git a/product/skills/spike-and-validate/SKILL.md b/product/skills/spike-and-validate/SKILL.md new file mode 100644 index 0000000..fc0e71b --- /dev/null +++ b/product/skills/spike-and-validate/SKILL.md @@ -0,0 +1,80 @@ +--- +name: spike-and-validate +description: >- + Turn an untested assumption into real output by building the smallest + throwaway that would kill it, running it, then discarding the code and + keeping the finding. Use before committing to an invented option, an + unfamiliar library or API, or a performance assumption. The gate on + `alternatives-considered`. +--- + +# Spike and validate + +A spike is an experiment, not a draft of the real change. Its only product +is a run with real output. The code goes in the bin. + +This is the gate that makes `alternatives-considered` safe: an option nobody +ran is a hypothesis, and a spike is the cheapest way to stop it from +becoming a decision. + +## Step 1. Write the kill condition first + +Before any code, in one line: **what result would rule this out?** + +If you cannot write that line, you are not spiking, you are starting the +implementation. Stop and say so. + +The kill condition is falsifiable and specific. "See if the library works" +is not one. "Parses our 40MB fixture in under 2s on this machine" is. + +## Step 2. Build the smallest thing that could fail + +- Own worktree or scratch directory. Never the live checkout. +- Hardcode everything not under test. Config, auth, error handling, and + edge cases are not what you are learning. +- No tests, no cleanup, no comments. This code is not going to review. +- Timebox it and say the box out loud. A spike that outgrows its box has + become the implementation without anyone deciding that. + +## Step 3. Run it and paste the output + +The whole point. Real command, real output, in the same message as the +verdict — `principle-prove-it`, applied to your own experiment. + +A spike that only passes proves less than you think. Where the assumption +has a failing side, show both: the case that works and the case that +breaks. A run that could not fail did not test anything. + +## Step 4. Discard the code, keep the finding + +**Delete the spike.** A spike promoted to production carries every shortcut +in Step 2 with it, and nobody remembers which lines were deliberate. + +What survives is one row in the shape at +`corpus/skills/principle-prove-it/references/finding-shape.md`: + +- `claim` — the assumption, now resolved. +- `grounding` — `read-confirmed`, because you ran it. +- `evidence` — the command and its real output. +- `null_result` — what the spike did **not** cover. Always fill this in. A + spike proves one thing on one machine with one fixture. + +Then say plainly: **validated**, **killed**, or **inconclusive**. +Inconclusive is a real outcome and much better than a spike quietly +reported as a pass. + +## When not to spike + +- The answer is in the code — that is `how`. +- The answer is in the history — that is `why`. +- The decision is cheap to reverse. Build it and change it later. +- You already know the answer and want cover for it. That is not an + experiment. + +## Do not + +- Spike in the live checkout. +- Skip the kill condition and decide after the fact what the run showed. +- Ship the spike. If it turns out to be the right shape, rebuild it with + the shortcuts removed. +- Report a spike that never ran as evidence for anything. diff --git a/product/skills/spike-and-validate/tests/fires_example.md b/product/skills/spike-and-validate/tests/fires_example.md new file mode 100644 index 0000000..edfa935 --- /dev/null +++ b/product/skills/spike-and-validate/tests/fires_example.md @@ -0,0 +1,12 @@ +A plan needs a streaming ZIP reader for uploads up to 2GB. Nobody on the +team has used the candidate library, and `alternatives-considered` returned +it labeled `invented` — plausible, never run here. + +This skill fires. An untested assumption is about to decide an expensive +choice, which is the stated trigger and the gate +`alternatives-considered` routes to. Step 1 writes the kill condition first +("streams a 2GB fixture without loading it into memory, under 4GB RSS"), +Step 2 builds a hardcoded throwaway in its own scratch directory, Step 3 +runs it and pastes the real output including the case that breaks, and Step +4 deletes the code, keeps one finding row with `null_result` naming what the +spike did not cover, and reports validated, killed, or inconclusive. diff --git a/product/skills/spike-and-validate/tests/stays_silent_example.md b/product/skills/spike-and-validate/tests/stays_silent_example.md new file mode 100644 index 0000000..1343dfc --- /dev/null +++ b/product/skills/spike-and-validate/tests/stays_silent_example.md @@ -0,0 +1,10 @@ +A user asks why an existing export job writes duplicate rows after a retry. +The behavior is already implemented, already reproducible, and the answer is +in the code. + +This skill stays silent. Its own "when not to spike" section rules this out +directly: the answer is in the code, so this is `how`, and the causal +question about an existing defect belongs to the repro-then-fix path in +`principle-fix-root-causes`. There is no untested assumption about something +that does not exist yet, and building a throwaway would not answer a +question the real code already answers. diff --git a/product/skills/why/SKILL.md b/product/skills/why/SKILL.md new file mode 100644 index 0000000..85c6a37 --- /dev/null +++ b/product/skills/why/SKILL.md @@ -0,0 +1,94 @@ +--- +name: why +description: >- + Recover why code is shaped the way it is before changing it. Use for "why + does X work this way", "why was this threshold picked", design rationale, + and dead-code questions. Anchors in git history and PR bodies first, then + fans out one investigator per available evidence source and reports every + null. Use `how` for runtime behavior. +--- + +# Why + +`how` answers what the code does. `why` answers what forces produced it. + +Feeds `principle-prove-it`: a constraint you cannot see is a constraint you +will delete. Most "we changed it back a week later" bugs are this. + +## Operating posture + +Careful and explicit about the line between what the record says and what +you are inferring from it. Never smooth an inference into a fact because it +reads better. + +## Step 1. Anchor in the code + +Build this inline **before** spawning anything. Every investigator starts +from it, so a weak anchor wastes the whole fan-out. + +```sh +git blame -L , -- # last-touch commits +git log --oneline -20 -- # recent commits, PR numbers visible +git log --follow -p -- # full history through renames +gh pr view --json title,body,author,mergedAt,comments,reviews +``` + +Capture: file paths, line ranges, key symbols, commit SHAs, PR numbers, and +any ticket IDs those PR bodies reference. Read the PR bodies yourself. Review +discussion is where implementation-time rationale actually lives, and it is +usually the answer. + +## Step 2. Fan out over the sources that exist + +List the evidence sources actually available in this environment before +assigning any. Typical set: git and `gh` (always), the issue tracker, +long-form docs, team chat, error tracking, observability, analytics — +whichever have a working MCP or CLI here. + +One investigator per source, spawned in **one message**. Do not hand one +subagent two sources; it will search the easy one and summarize the other. + +Each returns rows in the shape at +`corpus/skills/principle-prove-it/references/finding-shape.md`. + +**Document the null.** A source that returned nothing gets a row with +`null_result` set and a line in Sources Consulted. Skipping a source needs a +written reason in the output — "no MCP available here" or "provably +irrelevant, this is a build-time script with no runtime path." "Probably +irrelevant" is not a reason. + +This is the same rule as the projection rule in `engine/CLAUDE.core.md`: +absence from a projection is not proof of absent state. + +## Step 3. Report with the confidence separated + +- **The question** — restated, and the code it points at. +- **What the record says** — cited. Commit, PR, ticket, message, with the ref. +- **What we can reasonably infer** — clearly marked as inference. +- **Competing explanations** — when two readings both fit, give both. +- **What we don't know** — including every null and every skipped source. +- **Sources consulted** — one line each, including the empty ones. + +"Nobody wrote it down" is a real answer and often the most useful one: it +means the constraint is unprotected and the next person will break it too. + +## Step 4. Convert to constraints (when a change follows) + +If the `why` precedes an edit, end with: + +- **Preserve** — behavior the history says was paid for. +- **Change** — what the original reason no longer justifies. +- **Avoid** — approaches the record shows were already tried and failed. +- **Risk** — what breaks if the recovered reason is wrong. + +Hand that set to the plan. `alternatives-considered` treats **Avoid** as +already-explored ground. + +## Do not + +- Treat the most recent commit as authoritative. The current shape is + usually accretion, not a decision. +- Report a PR body's claim as a fact about today's code without checking the + code still matches it. +- Answer inline from one commit unless you can say why every other available + source would have been redundant. diff --git a/product/skills/why/tests/fires_example.md b/product/skills/why/tests/fires_example.md new file mode 100644 index 0000000..02e9a3c --- /dev/null +++ b/product/skills/why/tests/fires_example.md @@ -0,0 +1,12 @@ +A user is cleaning up a retry helper and asks: "why is the retry limit set +to five? does that reason still hold, or can I raise it?" + +This skill fires. It is a rationale question about an existing constant, not +a behavior question, and it precedes a change — the skill's stated trigger. +Step 1 anchors inline with `git blame -L` on the constant, `git log +--oneline` for the PR number, and `gh pr view` on the merge commit before +any subagent spawns. Step 2 fans out one investigator per source actually +available here, and a source with no MCP gets a null row plus a written +reason rather than being dropped. The report separates what the record says +from what is inferred, and Step 4 converts the answer into +Preserve / Change / Avoid / Risk for the edit that follows. diff --git a/product/skills/why/tests/stays_silent_example.md b/product/skills/why/tests/stays_silent_example.md new file mode 100644 index 0000000..a7f6ce9 --- /dev/null +++ b/product/skills/why/tests/stays_silent_example.md @@ -0,0 +1,10 @@ +A user asks the agent to add a `--json` flag to a CLI command that has no +existing output-format handling, on a file created in this same session. + +This skill stays silent. There is no lineage to recover: the code was +written minutes ago in the current session, there is no blame history, no +merged PR, and no prior decision anyone could have written down. Step 1's +anchor commands would return the session's own commit or nothing at all, and +fanning investigators across an issue tracker and team chat for a file that +never existed before today would produce nothing but null rows. The +motivation is in the user's request, already stated. diff --git a/scripts/check_skill_trigger_policy.py b/scripts/check_skill_trigger_policy.py new file mode 100755 index 0000000..7f8a7d4 --- /dev/null +++ b/scripts/check_skill_trigger_policy.py @@ -0,0 +1,157 @@ +#!/usr/bin/env python3 +"""Which skills may the model fire on its own? Declare it, then keep it true. + +Two failures this catches, both real: + +1. A skill whose own prose asks not to be auto-invoked while remaining + auto-invocable. Found in this repo: product/skills/admin-bypass-sweep + force-merges PRs past required checks via `gh pr merge --admin`, and its + description said "MANUAL, HUMAN-ONLY ... Do not auto-invoke this skill + from a natural-language request, a description match, or another agent's + delegation" -- with no `disable-model-invocation: true` to enforce it. + Prose asking a model not to match a description it can read is tier-4 + where tier-3 applies (see reflect/references/lenses.md's fix hierarchy). + +2. docs/skill-triggers.md drifting from reality. A hand-maintained + inventory of 35+ skills rots by the second PR, so the doc carries a + generated block and this script is what proves it still matches. + + python3 scripts/check_skill_trigger_policy.py # verify + python3 scripts/check_skill_trigger_policy.py --write # regenerate doc +""" +from __future__ import annotations + +import argparse +import re +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +SKILL_BUCKETS = ("engine/skills", "corpus/skills", "product/skills") +DOC_PATH = REPO_ROOT / "docs" / "skill-triggers.md" + +BEGIN_MARK = "" +END_MARK = "" + +FLAG = "disable-model-invocation: true" + +MANUAL_DECL_RE = re.compile( + r"human-only|do not auto-invoke|never auto-invoke|manual,\s*human|" + r"only run this skill when a human|do not invoke (?:this )?automatically|" + r"must not be auto-invoked", + re.IGNORECASE, +) + + +def frontmatter(text: str) -> str: + """The YAML block between the opening and closing '---' lines, or ''.""" + if not text.startswith("---"): + return "" + end = text.find("\n---", 3) + return text[3:end] if end != -1 else "" + + +def frontmatter_declares_manual(frontmatter_block: str) -> bool: + """Whether the skill's own frontmatter asks not to be model-invoked. + + A skill declaring itself manual/human-only there must back it with FLAG. + Reads the frontmatter block alone, never the body, so prose describing + some other skill's manual policy does not trip the rule. + """ + return bool(MANUAL_DECL_RE.search(frontmatter_block)) + + +def skills(repo_root: Path) -> list[tuple[str, str, bool, bool]]: + """(bucket, name, auto_fires, self_declared_manual), sorted.""" + out: list[tuple[str, str, bool, bool]] = [] + for bucket in SKILL_BUCKETS: + root = repo_root / bucket + if not root.is_dir(): + continue + for skill_dir in sorted(p for p in root.iterdir() if p.is_dir()): + md = skill_dir / "SKILL.md" + if not md.is_file(): + continue + fm = frontmatter(md.read_text(encoding="utf-8")) + out.append( + ( + bucket, + skill_dir.name, + FLAG not in fm, + frontmatter_declares_manual(fm), + ) + ) + return out + + +def violations(rows: list[tuple[str, str, bool, bool]]) -> list[str]: + return [ + f"{bucket}/{name}: frontmatter declares itself manual/human-only but " + f"has no '{FLAG}', so the model can still fire it from a description match" + for bucket, name, auto, manual in rows + if auto and manual + ] + + +def render(rows: list[tuple[str, str, bool, bool]]) -> str: + auto = [f"`{n}`" for b, n, a, _ in rows if a] + explicit = [f"`{n}`" for b, n, a, _ in rows if not a] + lines = [ + BEGIN_MARK, + "", + f"### Auto-fire ({len(auto)})", + "", + "The model may invoke these from a description match. Everything here " + "is a gate or a procedure that is useless if it only runs when named.", + "", + ", ".join(auto) if auto else "_none_", + "", + f"### Explicit invocation only ({len(explicit)})", + "", + f"These carry `{FLAG}`. Claude Code does not load their " + "`description:` at all, so the only way in is a typed `/`.", + "", + ", ".join(explicit) if explicit else "_none_", + "", + END_MARK, + ] + return "\n".join(lines) + + +def splice(doc: str, block: str) -> str: + start, end = doc.find(BEGIN_MARK), doc.find(END_MARK) + if start == -1 or end == -1: + raise SystemExit(f"fail\t{DOC_PATH}: missing generated-block markers") + return doc[:start] + block + doc[end + len(END_MARK):] + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("--write", action="store_true", help="regenerate the doc block") + args = ap.parse_args() + + rows = skills(REPO_ROOT) + errors = violations(rows) + block = render(rows) + + if args.write: + DOC_PATH.write_text(splice(DOC_PATH.read_text(encoding="utf-8"), block), encoding="utf-8") + print(f"wrote\t{DOC_PATH.relative_to(REPO_ROOT)}") + elif not DOC_PATH.is_file(): + errors.append("docs/skill-triggers.md: missing; run --write") + elif block not in DOC_PATH.read_text(encoding="utf-8"): + errors.append( + "docs/skill-triggers.md: generated block is stale; run " + "python3 scripts/check_skill_trigger_policy.py --write" + ) + + if errors: + for e in errors: + print(f"fail\t{e}", file=sys.stderr) + return 1 + print("ok\tskill trigger policy") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_skill_trigger_policy.py b/tests/test_skill_trigger_policy.py new file mode 100644 index 0000000..a5ad9c2 --- /dev/null +++ b/tests/test_skill_trigger_policy.py @@ -0,0 +1,120 @@ +#!/usr/bin/env python3 +"""Tests for scripts/check_skill_trigger_policy.py. + +The manual-without-flag fixture is the real one: product/skills/ +admin-bypass-sweep shipped that exact frontmatter shape -- a description +telling the model not to auto-invoke it, with no flag to stop it. +""" +from __future__ import annotations + +import sys +import tempfile +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO / "scripts")) +import check_skill_trigger_policy as pol # noqa: E402 + +MANUAL_NO_FLAG = """--- +name: admin-bypass-sweep +description: > + MANUAL, HUMAN-ONLY skill. Force-merges every open PR labeled admin-bypass + directly to the trunk branch. Do not auto-invoke this skill from a + natural-language request, a description match, or another agent's + delegation. +--- + +# admin-bypass-sweep +""" + +MANUAL_WITH_FLAG = MANUAL_NO_FLAG.replace( + "---\n\n# admin", "disable-model-invocation: true\n---\n\n# admin" +) + +AUTO_GATE = """--- +name: principle-prove-it +description: "No claim without same-turn evidence." +--- + +# Prove It +""" + +BODY_MENTION_ONLY = """--- +name: docs-about-triggers +description: "Explains the trigger policy." +--- + +# Docs + +A skill that says human-only must not be auto-invoked from a description +match; see admin-bypass-sweep. +""" + + +def _repo(skills: dict[str, str]) -> Path: + root = Path(tempfile.mkdtemp()) + for rel, text in skills.items(): + md = root / rel / "SKILL.md" + md.parent.mkdir(parents=True, exist_ok=True) + md.write_text(text, encoding="utf-8") + return root + + +class TestManualDeclarationRule(unittest.TestCase): + def test_manual_declaration_without_flag_is_a_violation(self): + rows = pol.skills(_repo({"product/skills/admin-bypass-sweep": MANUAL_NO_FLAG})) + errs = pol.violations(rows) + self.assertEqual(len(errs), 1, errs) + self.assertIn("admin-bypass-sweep", errs[0]) + self.assertIn("description match", errs[0]) + + def test_manual_declaration_with_flag_passes(self): + rows = pol.skills(_repo({"product/skills/admin-bypass-sweep": MANUAL_WITH_FLAG})) + self.assertEqual(pol.violations(rows), []) + + def test_auto_fire_gate_without_manual_wording_passes(self): + rows = pol.skills(_repo({"corpus/skills/principle-prove-it": AUTO_GATE})) + self.assertEqual(pol.violations(rows), []) + + def test_manual_wording_in_body_only_does_not_trip_the_rule(self): + """The rule reads frontmatter, not prose describing another skill.""" + rows = pol.skills(_repo({"product/skills/docs-about-triggers": BODY_MENTION_ONLY})) + self.assertEqual(pol.violations(rows), []) + + +class TestInventoryRendering(unittest.TestCase): + def test_render_splits_auto_from_explicit(self): + rows = pol.skills( + _repo( + { + "corpus/skills/principle-prove-it": AUTO_GATE, + "product/skills/admin-bypass-sweep": MANUAL_WITH_FLAG, + } + ) + ) + block = pol.render(rows) + auto, explicit = block.split("### Explicit invocation only") + self.assertIn("principle-prove-it", auto) + self.assertNotIn("admin-bypass-sweep", auto) + self.assertIn("admin-bypass-sweep", explicit) + + def test_splice_replaces_only_the_generated_block(self): + doc = f"keep above\n\n{pol.BEGIN_MARK}\nstale\n{pol.END_MARK}\n\nkeep below\n" + out = pol.splice(doc, pol.render([("corpus/skills", "x", True, False)])) + self.assertIn("keep above", out) + self.assertIn("keep below", out) + self.assertNotIn("stale", out) + + def test_splice_fails_closed_without_markers(self): + with self.assertRaises(SystemExit): + pol.splice("no markers here\n", "block") + + +class TestRealRepoState(unittest.TestCase): + def test_this_repo_has_no_manual_without_flag(self): + self.assertEqual(pol.violations(pol.skills(REPO)), []) + + +if __name__ == "__main__": + unittest.main()