Skip to content

skills: declare and enforce which skills the model may auto-fire - #303

Merged
EdbertChan merged 3 commits into
mainfrom
skill-trigger-policy
Sep 9, 2026
Merged

skills: declare and enforce which skills the model may auto-fire#303
EdbertChan merged 3 commits into
mainfrom
skill-trigger-policy

Conversation

@EdbertChan

@EdbertChan EdbertChan commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Summary

A skill cannot opt out of auto-invocation by asking. admin-bypass-sweep force-merges every open admin-bypass PR to trunk past required checks, and its description told the model not to auto-invoke it from a description match — with no flag behind that. The model reads the description to decide whether to fire, so the sentence asking it not to sat inside the thing it matches on.

This adds the flag, and a gate so the next one cannot happen: a skill declaring itself manual or human-only in its own frontmatter must carry disable-model-invocation: true.

docs/skill-triggers.md records which skills may fire on their own (17) and which need a typed slash command (30). The inventory is generated, because a hand-written list of 47 skills is stale by the next PR.

Review Claim

A skill that declares itself manual must be mechanically prevented from auto-firing, and the auto-fire inventory should be generated rather than maintained by hand.

Review Lane

policy

Review Unit

engine-runtime (dominant: scripts/check_skill_trigger_policy.py, engine/skills/make-pr/scripts/preflight.py)

Safety Invariant

Narrows what the model can reach, never widens it. admin-bypass-sweep gains disable-model-invocation: true, so a force-merge past required checks now requires a typed /admin-bypass-sweep — which is what the skill's own description already claimed. Its fixtures already asserted that behavior and needed no change. The new gate is additive; preflight.py gains one command. No other skill's trigger state changes.

Slice Rationale

One product-skill file rides along with the engine-runtime change because it is the violation the gate exists to catch: landing the gate without the fix leaves main red, and landing the fix without the gate leaves the next dangerous skill unprotected.

Split from #302 because that slice adds skills and this one changes what the model may invoke. A reviewer checking "are these five skills right" and a reviewer checking "should a force-merge skill be reachable by description match" are asking different questions.

Non-goals

  • Does not change any other skill's trigger state. The 17/30 split is the existing state, recorded.
  • Does not add a hardcoded allowlist of dangerous skill names. That needs someone to remember the next one, which is the failure being fixed.
  • Does not match manual-sounding wording in skill bodies, only frontmatter — a doc explaining this policy necessarily contains the phrases.
  • Does not run ./install.sh.

Test Plan

Test Plan

Fail-before / pass-after on the real case. With the flag removed (main's state today):

fail	product/skills/admin-bypass-sweep: frontmatter declares itself manual/human-only but has no 'disable-model-invocation: true', so the model can still fire it from a description match
fail	docs/skill-triggers.md: generated block is stale; run python3 scripts/check_skill_trigger_policy.py --write
exit=1

With the flag present:

ok	skill trigger policy
exit=0
  • python3 -m unittest tests.test_skill_trigger_policy -vRan 8 tests ... OK
  • python3 -m unittest discover -s engine/skills/make-pr/testsRan 12 tests ... OK
  • python3 engine/skills/make-pr/scripts/preflight.py --base prove-it-decomposeok preflight passed (8 gates, including the new one)

Revert Plan

Revert Plan
  • Safe to revert? Yes, but it is the unsafe direction — reverting restores admin-bypass-sweep's auto-invocability. Prefer fixing forward.
  • Revert command: git revert <sha>
  • Post-revert steps: rerun ./install.sh from the main checkout
  • Data migration? No

🤖 Generated with Claude Code

https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD


Note

Medium Risk
Narrows model reach for a dangerous admin merge skill and adds policy CI; the no-comments change could miss real # comments inside triple-quoted Python strings.

Overview
Mechanically enforces which skills the model may auto-invoke: frontmatter that claims a skill is manual/human-only must include disable-model-invocation: true, or CI fails. admin-bypass-sweep gets that flag so force-merge past checks is slash-command-only, matching what its description already said.

Adds scripts/check_skill_trigger_policy.py (verify + --write to refresh inventory) and docs/skill-triggers.md with a generated auto-fire vs explicit-only list. make-pr preflight runs the new gate on skill-touched diffs; unit tests cover the policy script and preflight wiring.

no-comments hook now treats Python triple-quoted strings as out of scope so # lines inside docstrings/fixtures (e.g. markdown headings in test literals) are not blocked as comments.

Reviewed by Cursor Bugbot for commit be22fbf. Bugbot is set up for automated code reviews on this repo. Configure here.

## Context
- Problem: catstack ships three hooks that enforce prove-it
  (prove-it-ship-gate, hedge-runs-prove-it, diu-stop) and eight skills that
  cite it, but does not own the rule. diu-stop tells the agent to read
  "skills/prove-it/SKILL.md in the Invoker repo"; ~/.claude/skills/
  invoker-prove-it is a real directory written by Invoker's installer, so
  ./install.sh cannot update it. Engine-only mode cites a file it does not
  install.
- Trigger: reading lauren (@poteto)'s pstack guide (mirrored at
  github.com/cursor/plugins/pstack/docs/guide) surfaced /how and /why as
  understand-before-edit skills catstack has no equivalent of. The gap is
  real: our only git-history rule is one bullet in
  principle-fix-root-causes:22, not a procedure.

## Considerations
- Chose: split the rule from the procedures. principle-prove-it holds the
  claim gate (corpus, because check_ecosystem_boundaries.py:206 requires
  principle-* live there). how / why / alternatives-considered /
  spike-and-validate hold the fan-outs (product, human-authored portable
  workflows per docs/ecosystem.md).
- Chose: principle-prove-it is the one principle that does NOT carry
  disable-model-invocation. The other 25 are steering vocabulary you invoke
  by name; this one is a gate whose job is to intercept a claim as it forms.
  A gate that only fires when someone remembers to name it is not a gate.
  prove-it-ship-gate already auto-fires from corpus, so there is precedent.
- Rejected: reusing independent-judge-swarm's board shape. That grades a
  finished artifact (verdict/score/blocking_issues); these four return
  findings about work that has not happened yet, so there is nothing to
  grade. Shared shape lives in principle-prove-it/references/
  finding-shape.md instead, reusing the fan-out mechanic and the
  document-the-null rule from reflect/references/lenses.md.
- Rejected: treating alternatives-considered as symmetric with how/why.
  how and why read what exists and can return an honest null;
  alternatives generates, so it can always produce three plausible options
  and manufacture evidence for the very gate it feeds. Every option is
  labeled considered (cited) or invented (untested), and an invented option
  may not decide anything until spike-and-validate turns it into a run with
  real output.
- Not done here: retiring invoker-prove-it, repointing diu-stop's message at
  the new principle, and documenting/enforcing which skills may auto-fire.
  The last one is the next slice in this stack — it found that
  product/skills/admin-bypass-sweep (force-merge past required checks) is
  auto-invocable today and held back only by prose in its description.

## Blast Radius
- Adds only. No existing file modified, no hook logic touched.
- New: corpus/skills/principle-prove-it/ (SKILL.md, references/
  finding-shape.md, 2 test fixtures); product/skills/{how,why,
  alternatives-considered,spike-and-validate}/ (SKILL.md + 2 fixtures each).
- principle-prove-it auto-fires, so it can now load on claim-shaped turns
  that previously loaded nothing. Cost is context on exactly the turns where
  the rule applies.
- Risk: `how` and `why` are generic names in a flat installed skill
  namespace. Verified no collision in ~/.claude/skills at authoring time.
- Revertable with git revert; ./install.sh prunes the symlinks on rerun.

## Verification
- python3 scripts/check_ecosystem_boundaries.py -> ok
- python3 scripts/check_skill_file_refs.py -> ok
- python3 scripts/check_skills_three_harnesses.py -> ok
- python3 scripts/check_skill_trigger_mechanism.py -> ok
- python3 scripts/check_skill_test_coverage.py --base origin/main --head HEAD
  -> ok (rerun post-commit; the pre-commit run compared origin/main to an
  unchanged HEAD and passed vacuously)
- check_codify_has_code.py run with --allow-prose-only: this slice adds rule
  prose whose enforcement already ships in engine/hooks/prove-it-ship-gate,
  engine/hooks/hedge-runs-prove-it, and engine/hooks/diu-stop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
@cursor

cursor Bot commented Sep 9, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_b9f13e86-75c9-4f45-809d-2bdb7b99d7e0)

## Context
- Problem: nothing recorded which skills may fire from a description match
  and which need a typed slash command, and one skill was wrong about its
  own state. product/skills/admin-bypass-sweep force-merges every open
  admin-bypass PR to trunk past required checks via `gh pr merge --admin`.
  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` behind it. The model
  reads that description to decide whether to fire, so the sentence asking it
  not to was inside the thing it matches on.
- Trigger: asked to document the auto-fire list while flipping
  principle-prove-it to auto-fire in the parent slice.

## Considerations
- Chose: a generated inventory plus a fail-closed contradiction check. A
  skill declaring itself manual/human-only in its own frontmatter MUST carry
  the flag. This is reflect/references/lenses.md's fix hierarchy applied to
  itself — tier 3 (mechanical check) instead of tier 4 (prose asking nicely).
- Chose: generate docs/skill-triggers.md's inventory from the frontmatter
  rather than hand-maintain it. A hand-written list of 35+ skills is stale by
  the next PR, and a stale list is worse than none.
- Chose: match the frontmatter block only, not the body. A doc explaining the
  policy necessarily contains "human-only" and "do not auto-invoke" in prose;
  matching the whole file would flag every such doc. Covered by
  test_manual_wording_in_body_only_does_not_trip_the_rule.
- Rejected: a hardcoded allowlist of dangerous skill names. It requires
  someone to remember to add the next one, which is the failure being fixed.
- admin-bypass-sweep's fixtures already asserted the right behavior (fires on
  the literal /admin-bypass-sweep, stays silent on "help me land all the prs
  that are labeled admin-bypass"), so only the flag was missing; the fixtures
  needed no change and now match the mechanism.

## Blast Radius
- admin-bypass-sweep can no longer be invoked by description match or agent
  delegation — only a typed /admin-bypass-sweep. That is a deliberate
  reduction in what the model can reach, and what the skill already claimed.
- preflight.py gains one gate, so future skill PRs run it automatically.
- New files: scripts/check_skill_trigger_policy.py, docs/skill-triggers.md,
  tests/test_skill_trigger_policy.py.
- Revertable with git revert. Reverting restores auto-invocability of
  admin-bypass-sweep, which is the unsafe direction — prefer fixing forward.

## Verification
- Fail-before / pass-after on the real case, both outputs captured:
  with the flag removed (main's state today) —
    fail  product/skills/admin-bypass-sweep: frontmatter declares itself
          manual/human-only but has no 'disable-model-invocation: true', so
          the model can still fire it from a description match
    fail  docs/skill-triggers.md: generated block is stale
    exit=1
  with the flag present —
    ok    skill trigger policy
    exit=0
- python3 -m unittest tests.test_skill_trigger_policy -v -> Ran 8 tests, OK
- python3 -m unittest discover -s engine/skills/make-pr/tests -> Ran 12, OK
- python3 engine/skills/make-pr/scripts/preflight.py --base
  prove-it-decompose -> ok preflight passed (8 gates, including the new one)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
@cursor

cursor Bot commented Sep 9, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_143d2c6e-e63f-4dc2-8f5c-a381af28a56c)

EdbertChan pushed a commit that referenced this pull request Sep 9, 2026
…rage pass

## Context
- Two defects, both found by the same incident this session. A claim went out
  ("PR #303: full preflight green") that was never actually checked, and the
  guard that exists to catch that class stayed silent.
- Defect 1, the vacuous pass: preflight.py ran
  check_skill_test_coverage.py with no refs, while `base` was already in scope
  two lines above (used for check_no_dated_provenance). With no refs the gate
  defaults to origin/main, so for a stacked slice it compares the whole stack
  instead of the slice and can print "ok" for a slice it never looked at.
  That is how "full preflight green" was reported for a slice that had
  changed preflight.py with no colocated test.
- Defect 2, the missed admission: wrong-check-reflect exists precisely to fire
  on "a prior check was wrong" and inject /reflect. It stayed silent on
  "Also: a claim I made earlier was wrong." The user had to play the hook's
  role by hand.

## Considerations
- Defect 1 is fixed categorically, not by a rule asking anyone to remember the
  flags: preflight now passes --base <slice base> --head HEAD, so the wider
  default is unreachable from the gate runner. Tier 1 of
  reflect/references/lenses.md's fix hierarchy rather than tier 4.
- Defect 2's root cause is the detector's shape, not a missing phrase.
  ADMISSION_RES enumerates eight sentences someone actually wrote, which
  guarantees it lags the ninth -- exactly what
  principle-assert-invariants-not-last-bug warns about. Adding
  "a claim I made was wrong" as pattern nine would repeat the mistake.
- So the fix matches the SHAPE of a retraction: a first-person marker, a
  reference to something already stated, and a wrongness word, within one
  ~260-char window (a retraction often spans two sentences). The enumerated
  list is kept -- it costs nothing and documents real phrasings -- with the
  structural check as the fallback.
- NEGATIVE_RES still runs first, so hypotheticals ("if my earlier check was
  wrong") and product blame ("the test was wrong") cannot reach the window.
- Deliberately NOT attempted here: enumerating every way a model can admit
  fault. That is not enumerable. The judgment half is the next slice, which
  makes principle-flag-your-own-corrections auto-fire.

## Blast Radius
- wrong-check-reflect now fires on more shapes, so false positives are the
  risk. Guarded by four new negative tests plus the 12 pre-existing ones; the
  whole 36-test suite was re-run unchanged before adding any.
- preflight reports failures it previously hid. Expect previously "green"
  stacked slices to surface real gaps -- that is the point.
- Modified: engine/hooks/wrong-check-reflect/{detect.py,tests/test_hooks.py},
  engine/skills/make-pr/scripts/preflight.py.
- Revertable with git revert.

## Verification
- Structural layer, 9 cases, 0 mismatches, including the 4 real misses and 4
  shapes that must stay silent (present-tense product opinion, no
  prior-statement marker, hypothetical, unrelated bug report).
- python3 -m unittest discover -s engine/hooks/wrong-check-reflect/tests
  -> Ran 36 tests, OK (before adding new tests: no regression)
  -> Ran 43 tests, OK (after)
- python3 scripts/check_hook_test_coverage.py engine/hooks/wrong-check-reflect
  -> check_hook_test_coverage: OK (1 hook(s) checked)
- Vacuous-pass divergence, same tree, one skill touched with no test change:
    check_skill_test_coverage.py (no refs)                 -> ok
    check_skill_test_coverage.py --base <slice> --head HEAD -> fail
  The second is what preflight now runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
The gate's docstring already promised "Docstrings are out of scope," but
the detector never tracked triple-quote parity, so any '#' line inside a
Python string literal was reported as a new comment. This PR's own files
tripped it five times: two usage lines carrying a trailing '# verify' /
'# regenerate doc' inside a module docstring, and three markdown headings
('# admin-bypass-sweep', '# Prove It', '# Docs') inside test fixtures.
Confirmed against tokenize: none of the five is a COMMENT token.

_starts_outside_triple_quotes tracks """/''' parity per line for .py and
skips lines that begin inside a string. Fixture pair added to the hook's
own suite: the markdown-fixture and docstring-usage negatives stay silent,
and a positive proves a real comment after a closed triple-quoted string
still fires.

The five genuine comments this PR added are removed. The frontmatter-only
matching rule is now carried by frontmatter_declares_manual() and its
parameter name instead of a three-line comment. Also fixes an F541
f-string-without-placeholders that was failing the separate lint job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F43CBUnsDEs6J2zEC1r8a8
@cursor

cursor Bot commented Sep 9, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_6d39f047-c010-4cb5-86d3-bcb1b7aed7d0)

@EdbertChan
EdbertChan changed the base branch from prove-it-decompose to main September 9, 2026 20:20
@EdbertChan
EdbertChan merged commit b633a23 into main Sep 9, 2026
4 checks passed
@mergify

mergify Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

EdbertChan pushed a commit that referenced this pull request Sep 9, 2026
- Problem: principle-subagent-inherits-scope (previous slice) is prose, and
  the next fan-out skill someone writes will not remember it. A scope
  boundary that exists only in a principle nobody loads at spawn time is the
  tier-4-where-tier-3-applies failure that reflect/references/lenses.md's fix
  hierarchy warns about.
- Trigger: landing that principle without a check would repeat the
  admin-bypass-sweep shape from PR #303 — prose asking for a behavior with
  nothing enforcing it.

- Chose: a skill whose body instructs delegation must name
  principle-subagent-inherits-scope, or cite the scope contract in
  principle-prove-it/references/finding-shape.md. Two ways to satisfy it
  because the four investigation products already cite the reference and
  should not each repeat the principle's name.
- Chose: grandfather the three pre-existing spawners in a shrink-only
  allowlist (scripts/subagent_scope_debt_allowlist.txt), mirroring
  scripts/skill_test_debt_allowlist.txt exactly. Rewriting reflect,
  independent-judge-swarm, and show-me-your-work to state the contract is a
  different review than adding the rule.
- Rejected: a hardcoded list of known fan-out skills. Same objection as the
  trigger-policy gate — it needs someone to remember the next one.
- Regex iteration, recorded because the first version was wrong in both
  directions. A loose pattern flagged three false positives:
  principle-build-the-lever says "don't fan out delegates",
  principle-trace-token-burn-loop prices "re-spawning the agent", and
  principle-guard-the-context-window costs out parallel Agent calls — all
  prose ABOUT delegation, not instructions to delegate. Tightening the verb
  to require an article ("spawn a|one|all ...") then went too far and lost
  why, alternatives-considered, and independent-judge-swarm, which are real
  spawners — false negatives are worse than false positives in a safety gate.
  The landed version keeps the broad verb patterns and adds a per-line
  negation guard, which drops all three false positives and keeps all seven
  real spawners. Both false positives are regression fixtures in the tests.

- New: scripts/check_subagent_scope_contract.py,
  scripts/subagent_scope_debt_allowlist.txt,
  tests/test_subagent_scope_contract.py.
- Modified: engine/skills/make-pr/scripts/preflight.py gains the gate (so
  future skill PRs run it) plus a colocated test for the gate list.
- No skill prose changes, so no agent behavior changes from this slice alone.
- Revertable with git revert.

- Fail-before / pass-after with the allowlist emptied (the state before this
  slice), both outputs captured:
    fail  engine/skills/reflect: instructs spawning subagents but names
          neither principle-subagent-inherits-scope nor the scope contract in
          finding-shape.md
    fail  product/skills/independent-judge-swarm: ... same
    fail  product/skills/show-me-your-work: ... same
    exit=1
  with the allowlist restored:
    ok    subagent scope contract
    exit=0
- python3 scripts/check_subagent_scope_contract.py --list -> 7 spawners, 4 ok
  (principle-prove-it, alternatives-considered, how, why), 3 grandfathered
- python3 -m unittest tests.test_subagent_scope_contract -> Ran 9 tests, OK
- python3 -m unittest discover -s engine/skills/make-pr/tests -> Ran 13, OK

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
EdbertChan pushed a commit that referenced this pull request Sep 9, 2026
…rage pass

## Context
- Two defects, both found by the same incident this session. A claim went out
  ("PR #303: full preflight green") that was never actually checked, and the
  guard that exists to catch that class stayed silent.
- Defect 1, the vacuous pass: preflight.py ran
  check_skill_test_coverage.py with no refs, while `base` was already in scope
  two lines above (used for check_no_dated_provenance). With no refs the gate
  defaults to origin/main, so for a stacked slice it compares the whole stack
  instead of the slice and can print "ok" for a slice it never looked at.
  That is how "full preflight green" was reported for a slice that had
  changed preflight.py with no colocated test.
- Defect 2, the missed admission: wrong-check-reflect exists precisely to fire
  on "a prior check was wrong" and inject /reflect. It stayed silent on
  "Also: a claim I made earlier was wrong." The user had to play the hook's
  role by hand.

## Considerations
- Defect 1 is fixed categorically, not by a rule asking anyone to remember the
  flags: preflight now passes --base <slice base> --head HEAD, so the wider
  default is unreachable from the gate runner. Tier 1 of
  reflect/references/lenses.md's fix hierarchy rather than tier 4.
- Defect 2's root cause is the detector's shape, not a missing phrase.
  ADMISSION_RES enumerates eight sentences someone actually wrote, which
  guarantees it lags the ninth -- exactly what
  principle-assert-invariants-not-last-bug warns about. Adding
  "a claim I made was wrong" as pattern nine would repeat the mistake.
- So the fix matches the SHAPE of a retraction: a first-person marker, a
  reference to something already stated, and a wrongness word, within one
  ~260-char window (a retraction often spans two sentences). The enumerated
  list is kept -- it costs nothing and documents real phrasings -- with the
  structural check as the fallback.
- NEGATIVE_RES still runs first, so hypotheticals ("if my earlier check was
  wrong") and product blame ("the test was wrong") cannot reach the window.
- Deliberately NOT attempted here: enumerating every way a model can admit
  fault. That is not enumerable. The judgment half is the next slice, which
  makes principle-flag-your-own-corrections auto-fire.

## Blast Radius
- wrong-check-reflect now fires on more shapes, so false positives are the
  risk. Guarded by four new negative tests plus the 12 pre-existing ones; the
  whole 36-test suite was re-run unchanged before adding any.
- preflight reports failures it previously hid. Expect previously "green"
  stacked slices to surface real gaps -- that is the point.
- Modified: engine/hooks/wrong-check-reflect/{detect.py,tests/test_hooks.py},
  engine/skills/make-pr/scripts/preflight.py.
- Revertable with git revert.

## Verification
- Structural layer, 9 cases, 0 mismatches, including the 4 real misses and 4
  shapes that must stay silent (present-tense product opinion, no
  prior-statement marker, hypothetical, unrelated bug report).
- python3 -m unittest discover -s engine/hooks/wrong-check-reflect/tests
  -> Ran 36 tests, OK (before adding new tests: no regression)
  -> Ran 43 tests, OK (after)
- python3 scripts/check_hook_test_coverage.py engine/hooks/wrong-check-reflect
  -> check_hook_test_coverage: OK (1 hook(s) checked)
- Vacuous-pass divergence, same tree, one skill touched with no test change:
    check_skill_test_coverage.py (no refs)                 -> ok
    check_skill_test_coverage.py --base <slice> --head HEAD -> fail
  The second is what preflight now runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
EdbertChan pushed a commit that referenced this pull request Sep 10, 2026
…rage pass

## Context
- Two defects, both found by the same incident this session. A claim went out
  ("PR #303: full preflight green") that was never actually checked, and the
  guard that exists to catch that class stayed silent.
- Defect 1, the vacuous pass: preflight.py ran
  check_skill_test_coverage.py with no refs, while `base` was already in scope
  two lines above (used for check_no_dated_provenance). With no refs the gate
  defaults to origin/main, so for a stacked slice it compares the whole stack
  instead of the slice and can print "ok" for a slice it never looked at.
  That is how "full preflight green" was reported for a slice that had
  changed preflight.py with no colocated test.
- Defect 2, the missed admission: wrong-check-reflect exists precisely to fire
  on "a prior check was wrong" and inject /reflect. It stayed silent on
  "Also: a claim I made earlier was wrong." The user had to play the hook's
  role by hand.

## Considerations
- Defect 1 is fixed categorically, not by a rule asking anyone to remember the
  flags: preflight now passes --base <slice base> --head HEAD, so the wider
  default is unreachable from the gate runner. Tier 1 of
  reflect/references/lenses.md's fix hierarchy rather than tier 4.
- Defect 2's root cause is the detector's shape, not a missing phrase.
  ADMISSION_RES enumerates eight sentences someone actually wrote, which
  guarantees it lags the ninth -- exactly what
  principle-assert-invariants-not-last-bug warns about. Adding
  "a claim I made was wrong" as pattern nine would repeat the mistake.
- So the fix matches the SHAPE of a retraction: a first-person marker, a
  reference to something already stated, and a wrongness word, within one
  ~260-char window (a retraction often spans two sentences). The enumerated
  list is kept -- it costs nothing and documents real phrasings -- with the
  structural check as the fallback.
- NEGATIVE_RES still runs first, so hypotheticals ("if my earlier check was
  wrong") and product blame ("the test was wrong") cannot reach the window.
- Deliberately NOT attempted here: enumerating every way a model can admit
  fault. That is not enumerable. The judgment half is the next slice, which
  makes principle-flag-your-own-corrections auto-fire.

## Blast Radius
- wrong-check-reflect now fires on more shapes, so false positives are the
  risk. Guarded by four new negative tests plus the 12 pre-existing ones; the
  whole 36-test suite was re-run unchanged before adding any.
- preflight reports failures it previously hid. Expect previously "green"
  stacked slices to surface real gaps -- that is the point.
- Modified: engine/hooks/wrong-check-reflect/{detect.py,tests/test_hooks.py},
  engine/skills/make-pr/scripts/preflight.py.
- Revertable with git revert.

## Verification
- Structural layer, 9 cases, 0 mismatches, including the 4 real misses and 4
  shapes that must stay silent (present-tense product opinion, no
  prior-statement marker, hypothetical, unrelated bug report).
- python3 -m unittest discover -s engine/hooks/wrong-check-reflect/tests
  -> Ran 36 tests, OK (before adding new tests: no regression)
  -> Ran 43 tests, OK (after)
- python3 scripts/check_hook_test_coverage.py engine/hooks/wrong-check-reflect
  -> check_hook_test_coverage: OK (1 hook(s) checked)
- Vacuous-pass divergence, same tree, one skill touched with no test change:
    check_skill_test_coverage.py (no refs)                 -> ok
    check_skill_test_coverage.py --base <slice> --head HEAD -> fail
  The second is what preflight now runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
EdbertChan pushed a commit that referenced this pull request Sep 10, 2026
- Problem: principle-subagent-inherits-scope (previous slice) is prose, and
  the next fan-out skill someone writes will not remember it. A scope
  boundary that exists only in a principle nobody loads at spawn time is the
  tier-4-where-tier-3-applies failure that reflect/references/lenses.md's fix
  hierarchy warns about.
- Trigger: landing that principle without a check would repeat the
  admin-bypass-sweep shape from PR #303 — prose asking for a behavior with
  nothing enforcing it.

- Chose: a skill whose body instructs delegation must name
  principle-subagent-inherits-scope, or cite the scope contract in
  principle-prove-it/references/finding-shape.md. Two ways to satisfy it
  because the four investigation products already cite the reference and
  should not each repeat the principle's name.
- Chose: grandfather the three pre-existing spawners in a shrink-only
  allowlist (scripts/subagent_scope_debt_allowlist.txt), mirroring
  scripts/skill_test_debt_allowlist.txt exactly. Rewriting reflect,
  independent-judge-swarm, and show-me-your-work to state the contract is a
  different review than adding the rule.
- Rejected: a hardcoded list of known fan-out skills. Same objection as the
  trigger-policy gate — it needs someone to remember the next one.
- Regex iteration, recorded because the first version was wrong in both
  directions. A loose pattern flagged three false positives:
  principle-build-the-lever says "don't fan out delegates",
  principle-trace-token-burn-loop prices "re-spawning the agent", and
  principle-guard-the-context-window costs out parallel Agent calls — all
  prose ABOUT delegation, not instructions to delegate. Tightening the verb
  to require an article ("spawn a|one|all ...") then went too far and lost
  why, alternatives-considered, and independent-judge-swarm, which are real
  spawners — false negatives are worse than false positives in a safety gate.
  The landed version keeps the broad verb patterns and adds a per-line
  negation guard, which drops all three false positives and keeps all seven
  real spawners. Both false positives are regression fixtures in the tests.

- New: scripts/check_subagent_scope_contract.py,
  scripts/subagent_scope_debt_allowlist.txt,
  tests/test_subagent_scope_contract.py.
- Modified: engine/skills/make-pr/scripts/preflight.py gains the gate (so
  future skill PRs run it) plus a colocated test for the gate list.
- No skill prose changes, so no agent behavior changes from this slice alone.
- Revertable with git revert.

- Fail-before / pass-after with the allowlist emptied (the state before this
  slice), both outputs captured:
    fail  engine/skills/reflect: instructs spawning subagents but names
          neither principle-subagent-inherits-scope nor the scope contract in
          finding-shape.md
    fail  product/skills/independent-judge-swarm: ... same
    fail  product/skills/show-me-your-work: ... same
    exit=1
  with the allowlist restored:
    ok    subagent scope contract
    exit=0
- python3 scripts/check_subagent_scope_contract.py --list -> 7 spawners, 4 ok
  (principle-prove-it, alternatives-considered, how, why), 3 grandfathered
- python3 -m unittest tests.test_subagent_scope_contract -> Ran 9 tests, OK
- python3 -m unittest discover -s engine/skills/make-pr/tests -> Ran 13, OK

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
EdbertChan pushed a commit that referenced this pull request Sep 10, 2026
…rage pass

## Context
- Two defects, both found by the same incident this session. A claim went out
  ("PR #303: full preflight green") that was never actually checked, and the
  guard that exists to catch that class stayed silent.
- Defect 1, the vacuous pass: preflight.py ran
  check_skill_test_coverage.py with no refs, while `base` was already in scope
  two lines above (used for check_no_dated_provenance). With no refs the gate
  defaults to origin/main, so for a stacked slice it compares the whole stack
  instead of the slice and can print "ok" for a slice it never looked at.
  That is how "full preflight green" was reported for a slice that had
  changed preflight.py with no colocated test.
- Defect 2, the missed admission: wrong-check-reflect exists precisely to fire
  on "a prior check was wrong" and inject /reflect. It stayed silent on
  "Also: a claim I made earlier was wrong." The user had to play the hook's
  role by hand.

## Considerations
- Defect 1 is fixed categorically, not by a rule asking anyone to remember the
  flags: preflight now passes --base <slice base> --head HEAD, so the wider
  default is unreachable from the gate runner. Tier 1 of
  reflect/references/lenses.md's fix hierarchy rather than tier 4.
- Defect 2's root cause is the detector's shape, not a missing phrase.
  ADMISSION_RES enumerates eight sentences someone actually wrote, which
  guarantees it lags the ninth -- exactly what
  principle-assert-invariants-not-last-bug warns about. Adding
  "a claim I made was wrong" as pattern nine would repeat the mistake.
- So the fix matches the SHAPE of a retraction: a first-person marker, a
  reference to something already stated, and a wrongness word, within one
  ~260-char window (a retraction often spans two sentences). The enumerated
  list is kept -- it costs nothing and documents real phrasings -- with the
  structural check as the fallback.
- NEGATIVE_RES still runs first, so hypotheticals ("if my earlier check was
  wrong") and product blame ("the test was wrong") cannot reach the window.
- Deliberately NOT attempted here: enumerating every way a model can admit
  fault. That is not enumerable. The judgment half is the next slice, which
  makes principle-flag-your-own-corrections auto-fire.

## Blast Radius
- wrong-check-reflect now fires on more shapes, so false positives are the
  risk. Guarded by four new negative tests plus the 12 pre-existing ones; the
  whole 36-test suite was re-run unchanged before adding any.
- preflight reports failures it previously hid. Expect previously "green"
  stacked slices to surface real gaps -- that is the point.
- Modified: engine/hooks/wrong-check-reflect/{detect.py,tests/test_hooks.py},
  engine/skills/make-pr/scripts/preflight.py.
- Revertable with git revert.

## Verification
- Structural layer, 9 cases, 0 mismatches, including the 4 real misses and 4
  shapes that must stay silent (present-tense product opinion, no
  prior-statement marker, hypothetical, unrelated bug report).
- python3 -m unittest discover -s engine/hooks/wrong-check-reflect/tests
  -> Ran 36 tests, OK (before adding new tests: no regression)
  -> Ran 43 tests, OK (after)
- python3 scripts/check_hook_test_coverage.py engine/hooks/wrong-check-reflect
  -> check_hook_test_coverage: OK (1 hook(s) checked)
- Vacuous-pass divergence, same tree, one skill touched with no test change:
    check_skill_test_coverage.py (no refs)                 -> ok
    check_skill_test_coverage.py --base <slice> --head HEAD -> fail
  The second is what preflight now runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
mergify Bot pushed a commit that referenced this pull request Sep 10, 2026
* gate: a skill that spawns subagents must state the scope contract

- Problem: principle-subagent-inherits-scope (previous slice) is prose, and
  the next fan-out skill someone writes will not remember it. A scope
  boundary that exists only in a principle nobody loads at spawn time is the
  tier-4-where-tier-3-applies failure that reflect/references/lenses.md's fix
  hierarchy warns about.
- Trigger: landing that principle without a check would repeat the
  admin-bypass-sweep shape from PR #303 — prose asking for a behavior with
  nothing enforcing it.

- Chose: a skill whose body instructs delegation must name
  principle-subagent-inherits-scope, or cite the scope contract in
  principle-prove-it/references/finding-shape.md. Two ways to satisfy it
  because the four investigation products already cite the reference and
  should not each repeat the principle's name.
- Chose: grandfather the three pre-existing spawners in a shrink-only
  allowlist (scripts/subagent_scope_debt_allowlist.txt), mirroring
  scripts/skill_test_debt_allowlist.txt exactly. Rewriting reflect,
  independent-judge-swarm, and show-me-your-work to state the contract is a
  different review than adding the rule.
- Rejected: a hardcoded list of known fan-out skills. Same objection as the
  trigger-policy gate — it needs someone to remember the next one.
- Regex iteration, recorded because the first version was wrong in both
  directions. A loose pattern flagged three false positives:
  principle-build-the-lever says "don't fan out delegates",
  principle-trace-token-burn-loop prices "re-spawning the agent", and
  principle-guard-the-context-window costs out parallel Agent calls — all
  prose ABOUT delegation, not instructions to delegate. Tightening the verb
  to require an article ("spawn a|one|all ...") then went too far and lost
  why, alternatives-considered, and independent-judge-swarm, which are real
  spawners — false negatives are worse than false positives in a safety gate.
  The landed version keeps the broad verb patterns and adds a per-line
  negation guard, which drops all three false positives and keeps all seven
  real spawners. Both false positives are regression fixtures in the tests.

- New: scripts/check_subagent_scope_contract.py,
  scripts/subagent_scope_debt_allowlist.txt,
  tests/test_subagent_scope_contract.py.
- Modified: engine/skills/make-pr/scripts/preflight.py gains the gate (so
  future skill PRs run it) plus a colocated test for the gate list.
- No skill prose changes, so no agent behavior changes from this slice alone.
- Revertable with git revert.

- Fail-before / pass-after with the allowlist emptied (the state before this
  slice), both outputs captured:
    fail  engine/skills/reflect: instructs spawning subagents but names
          neither principle-subagent-inherits-scope nor the scope contract in
          finding-shape.md
    fail  product/skills/independent-judge-swarm: ... same
    fail  product/skills/show-me-your-work: ... same
    exit=1
  with the allowlist restored:
    ok    subagent scope contract
    exit=0
- python3 scripts/check_subagent_scope_contract.py --list -> 7 spawners, 4 ok
  (principle-prove-it, alternatives-considered, how, why), 3 grandfathered
- python3 -m unittest tests.test_subagent_scope_contract -> Ran 9 tests, OK
- python3 -m unittest discover -s engine/skills/make-pr/tests -> Ran 13, OK

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD

* fix CI: move the regex rationale out of comments and into the docstring

check_no_new_comments.py rejected nine comment lines this branch added.
Seven explained why SPAWNER_RE and NEGATED_RE are deliberately narrow;
two named the corpus skills the test fixtures were copied from.

The SPAWNER_RE/NEGATED_RE rationale moves into the module docstring,
which the gate exempts. The two fixture-provenance lines are dropped: the
test module docstring already names both skills and why they are there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013mDsvDgGuwaBuyRtYgvktd

---------

Co-authored-by: Edbert Chan <chanedbert@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
EdbertChan pushed a commit that referenced this pull request Sep 10, 2026
…rage pass

## Context
- Two defects, both found by the same incident this session. A claim went out
  ("PR #303: full preflight green") that was never actually checked, and the
  guard that exists to catch that class stayed silent.
- Defect 1, the vacuous pass: preflight.py ran
  check_skill_test_coverage.py with no refs, while `base` was already in scope
  two lines above (used for check_no_dated_provenance). With no refs the gate
  defaults to origin/main, so for a stacked slice it compares the whole stack
  instead of the slice and can print "ok" for a slice it never looked at.
  That is how "full preflight green" was reported for a slice that had
  changed preflight.py with no colocated test.
- Defect 2, the missed admission: wrong-check-reflect exists precisely to fire
  on "a prior check was wrong" and inject /reflect. It stayed silent on
  "Also: a claim I made earlier was wrong." The user had to play the hook's
  role by hand.

## Considerations
- Defect 1 is fixed categorically, not by a rule asking anyone to remember the
  flags: preflight now passes --base <slice base> --head HEAD, so the wider
  default is unreachable from the gate runner. Tier 1 of
  reflect/references/lenses.md's fix hierarchy rather than tier 4.
- Defect 2's root cause is the detector's shape, not a missing phrase.
  ADMISSION_RES enumerates eight sentences someone actually wrote, which
  guarantees it lags the ninth -- exactly what
  principle-assert-invariants-not-last-bug warns about. Adding
  "a claim I made was wrong" as pattern nine would repeat the mistake.
- So the fix matches the SHAPE of a retraction: a first-person marker, a
  reference to something already stated, and a wrongness word, within one
  ~260-char window (a retraction often spans two sentences). The enumerated
  list is kept -- it costs nothing and documents real phrasings -- with the
  structural check as the fallback.
- NEGATIVE_RES still runs first, so hypotheticals ("if my earlier check was
  wrong") and product blame ("the test was wrong") cannot reach the window.
- Deliberately NOT attempted here: enumerating every way a model can admit
  fault. That is not enumerable. The judgment half is the next slice, which
  makes principle-flag-your-own-corrections auto-fire.

## Blast Radius
- wrong-check-reflect now fires on more shapes, so false positives are the
  risk. Guarded by four new negative tests plus the 12 pre-existing ones; the
  whole 36-test suite was re-run unchanged before adding any.
- preflight reports failures it previously hid. Expect previously "green"
  stacked slices to surface real gaps -- that is the point.
- Modified: engine/hooks/wrong-check-reflect/{detect.py,tests/test_hooks.py},
  engine/skills/make-pr/scripts/preflight.py.
- Revertable with git revert.

## Verification
- Structural layer, 9 cases, 0 mismatches, including the 4 real misses and 4
  shapes that must stay silent (present-tense product opinion, no
  prior-statement marker, hypothetical, unrelated bug report).
- python3 -m unittest discover -s engine/hooks/wrong-check-reflect/tests
  -> Ran 36 tests, OK (before adding new tests: no regression)
  -> Ran 43 tests, OK (after)
- python3 scripts/check_hook_test_coverage.py engine/hooks/wrong-check-reflect
  -> check_hook_test_coverage: OK (1 hook(s) checked)
- Vacuous-pass divergence, same tree, one skill touched with no test change:
    check_skill_test_coverage.py (no refs)                 -> ok
    check_skill_test_coverage.py --base <slice> --head HEAD -> fail
  The second is what preflight now runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
mergify Bot pushed a commit that referenced this pull request Sep 10, 2026
…rage pass (#309)

* hook+gate: catch the shape of a retraction, and stop the vacuous coverage pass

## Context
- Two defects, both found by the same incident this session. A claim went out
  ("PR #303: full preflight green") that was never actually checked, and the
  guard that exists to catch that class stayed silent.
- Defect 1, the vacuous pass: preflight.py ran
  check_skill_test_coverage.py with no refs, while `base` was already in scope
  two lines above (used for check_no_dated_provenance). With no refs the gate
  defaults to origin/main, so for a stacked slice it compares the whole stack
  instead of the slice and can print "ok" for a slice it never looked at.
  That is how "full preflight green" was reported for a slice that had
  changed preflight.py with no colocated test.
- Defect 2, the missed admission: wrong-check-reflect exists precisely to fire
  on "a prior check was wrong" and inject /reflect. It stayed silent on
  "Also: a claim I made earlier was wrong." The user had to play the hook's
  role by hand.

## Considerations
- Defect 1 is fixed categorically, not by a rule asking anyone to remember the
  flags: preflight now passes --base <slice base> --head HEAD, so the wider
  default is unreachable from the gate runner. Tier 1 of
  reflect/references/lenses.md's fix hierarchy rather than tier 4.
- Defect 2's root cause is the detector's shape, not a missing phrase.
  ADMISSION_RES enumerates eight sentences someone actually wrote, which
  guarantees it lags the ninth -- exactly what
  principle-assert-invariants-not-last-bug warns about. Adding
  "a claim I made was wrong" as pattern nine would repeat the mistake.
- So the fix matches the SHAPE of a retraction: a first-person marker, a
  reference to something already stated, and a wrongness word, within one
  ~260-char window (a retraction often spans two sentences). The enumerated
  list is kept -- it costs nothing and documents real phrasings -- with the
  structural check as the fallback.
- NEGATIVE_RES still runs first, so hypotheticals ("if my earlier check was
  wrong") and product blame ("the test was wrong") cannot reach the window.
- Deliberately NOT attempted here: enumerating every way a model can admit
  fault. That is not enumerable. The judgment half is the next slice, which
  makes principle-flag-your-own-corrections auto-fire.

## Blast Radius
- wrong-check-reflect now fires on more shapes, so false positives are the
  risk. Guarded by four new negative tests plus the 12 pre-existing ones; the
  whole 36-test suite was re-run unchanged before adding any.
- preflight reports failures it previously hid. Expect previously "green"
  stacked slices to surface real gaps -- that is the point.
- Modified: engine/hooks/wrong-check-reflect/{detect.py,tests/test_hooks.py},
  engine/skills/make-pr/scripts/preflight.py.
- Revertable with git revert.

## Verification
- Structural layer, 9 cases, 0 mismatches, including the 4 real misses and 4
  shapes that must stay silent (present-tense product opinion, no
  prior-statement marker, hypothetical, unrelated bug report).
- python3 -m unittest discover -s engine/hooks/wrong-check-reflect/tests
  -> Ran 36 tests, OK (before adding new tests: no regression)
  -> Ran 43 tests, OK (after)
- python3 scripts/check_hook_test_coverage.py engine/hooks/wrong-check-reflect
  -> check_hook_test_coverage: OK (1 hook(s) checked)
- Vacuous-pass divergence, same tree, one skill touched with no test change:
    check_skill_test_coverage.py (no refs)                 -> ok
    check_skill_test_coverage.py --base <slice> --head HEAD -> fail
  The second is what preflight now runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD

* fix CI: keep engine Python off corpus/ paths and out of comments

Two gates were red on this branch. check_ecosystem_boundaries.py rejects
any engine/ Python that names a corpus/skills or product/skills path, and
wrong-check-reflect/detect.py cited two principle skills by full path.
check_no_new_comments.py rejected four more comment lines here and in
make-pr/scripts/preflight.py.

Both fixes are the same move: the prose goes into the module or function
docstring, which both gates exempt, and the principle names lose their
corpus/skills/ prefix. No behavior changes.

The ecosystem failure was invisible from a worktree under .worktrees/ --
that gate skips those paths and prints ok having read no files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013mDsvDgGuwaBuyRtYgvktd

---------

Co-authored-by: Edbert Chan <chanedbert@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants