diff --git a/.agents/invocation.md b/.agents/invocation.md index d5227f4..1181855 100644 --- a/.agents/invocation.md +++ b/.agents/invocation.md @@ -19,6 +19,8 @@ This is about **operative** instructions — a skill's own steps telling the age The Skill tool takes one skill per call. A step that needs two skills is two calls, not one call with two names — say so (`Call the Skill tool twice, for "grilling" and "domain-modeling"`), not "call it with X and Y," which reads as a single call taking both. +This whole convention only holds when the named skill is **model-invoked**. A user-invoked skill can never be reached this way, full stop — per the invariant above, no other skill can call it, including by naming it to the Skill tool. When a step's precondition is a user-invoked skill (e.g. `setup-matt-pocock-skills`), phrase it as an instruction for the human to act on — "tell the user to run `/setup-matt-pocock-skills`" — never as a Skill tool call. + ## Passive vs active domain work Merely _reading_ `CONTEXT.md` for vocabulary is a one-line prose pointer, not the `domain-modeling` skill. Only the active build/sharpen discipline (challenge terms, edge-case scenarios, write ADRs, update `CONTEXT.md` inline) is `domain-modeling`. diff --git a/.changeset/user-invoked-skill-invocation.md b/.changeset/user-invoked-skill-invocation.md new file mode 100644 index 0000000..e7f206b --- /dev/null +++ b/.changeset/user-invoked-skill-invocation.md @@ -0,0 +1,11 @@ +--- +"mattpocock-skills": patch +--- + +Stop skills from trying to reach user-invoked skills through the Skill tool — fix cross-skill references that violated the "no other skill can call it" invariant in `.agents/invocation.md`, in `to-spec`, `wayfinder`, `to-tickets`, `triage`, `code-review`, and `diagnosing-bugs`. + +- `to-spec`, `wayfinder`, `to-tickets`, `triage`, and `code-review` each carried a precondition ("...run `/setup-matt-pocock-skills` if not") that PR #878 rewrote into a literal `Call the Skill tool with "setup-matt-pocock-skills"` instruction. `setup-matt-pocock-skills` is user-invoked, so none of these skills — user-invoked or model-invoked — can call it. Reworded all five as instructions for the agent to tell the human to run it instead. +- `diagnosing-bugs`'s Phase 6 post-mortem hand off to `improve-codebase-architecture` (also user-invoked) the same way, from an autonomous, often-unattended bug-fixing flow with no human in the loop to catch the failed call. Removed the hand-off outright rather than softening it — it rarely fired in practice. Phase 6 is now "Cleanup" only; the mechanical checklist is untouched. +- Added a carve-out paragraph to `.agents/invocation.md`'s "Dependencies between them" section: the `Call the Skill tool with "name"` convention only applies when the named skill is model-invoked. This is the section PR #878 introduced without reconciling it against the user-invoked/model-invoked invariant stated eight lines above it — the gap is most of why this bug reached six call sites instead of one. + +Fixes #453. diff --git a/skills/engineering/code-review/SKILL.md b/skills/engineering/code-review/SKILL.md index 1d98b8a..8ff164a 100644 --- a/skills/engineering/code-review/SKILL.md +++ b/skills/engineering/code-review/SKILL.md @@ -10,7 +10,7 @@ Two-axis review of the diff between `HEAD` and a fixed point the user supplies: Both axes run as **parallel sub-agents** so they don't pollute each other's context, then this skill aggregates their findings. -The issue tracker should have been provided to you — call the Skill tool with "setup-matt-pocock-skills" if `docs/agents/issue-tracker.md` is missing. +The issue tracker should have been provided to you. If `docs/agents/issue-tracker.md` is missing, tell the user to run `/setup-matt-pocock-skills`. ## Process diff --git a/skills/engineering/diagnosing-bugs/SKILL.md b/skills/engineering/diagnosing-bugs/SKILL.md index c2e00af..ad911b0 100644 --- a/skills/engineering/diagnosing-bugs/SKILL.md +++ b/skills/engineering/diagnosing-bugs/SKILL.md @@ -127,7 +127,7 @@ If a correct seam exists: 4. Watch it pass. 5. Re-run the Phase 1 feedback loop against the original (un-minimised) scenario. -## Phase 6 — Cleanup + post-mortem +## Phase 6 — Cleanup Required before declaring done: @@ -136,5 +136,3 @@ Required before declaring done: - [ ] All `[DEBUG-...]` instrumentation removed (`grep` the prefix) - [ ] Throwaway prototypes deleted (or moved to a clearly-marked debug location) - [ ] The hypothesis that turned out correct is stated in the commit / PR message — so the next debugger learns - -**Then ask: what would have prevented this bug?** If the answer involves architectural change (no good test seam, tangled callers, hidden coupling) hand off by calling the Skill tool with "improve-codebase-architecture" and the specifics. Make the recommendation **after** the fix is in, not before — you have more information now than when you started. diff --git a/skills/engineering/to-spec/SKILL.md b/skills/engineering/to-spec/SKILL.md index 128a6c7..54dbc4b 100644 --- a/skills/engineering/to-spec/SKILL.md +++ b/skills/engineering/to-spec/SKILL.md @@ -6,7 +6,7 @@ disable-model-invocation: true This skill takes the current conversation context and codebase understanding and produces a spec. Do NOT interview the user — just synthesize what you already know. -The issue tracker and triage label vocabulary should have been provided to you — call the Skill tool with "setup-matt-pocock-skills" if not. +The issue tracker and triage label vocabulary should have been provided to you. If not, tell the user to run `/setup-matt-pocock-skills`. ## Process diff --git a/skills/engineering/to-tickets/SKILL.md b/skills/engineering/to-tickets/SKILL.md index d234f98..4a21285 100644 --- a/skills/engineering/to-tickets/SKILL.md +++ b/skills/engineering/to-tickets/SKILL.md @@ -8,7 +8,7 @@ disable-model-invocation: true Break a plan, spec, or conversation into a set of **tickets** — tracer-bullet vertical slices, each declaring the tickets that **block** it. -The issue tracker and triage label vocabulary should have been provided to you — call the Skill tool with "setup-matt-pocock-skills" if not. +The issue tracker and triage label vocabulary should have been provided to you. If not, tell the user to run `/setup-matt-pocock-skills`. ## Process diff --git a/skills/engineering/triage/SKILL.md b/skills/engineering/triage/SKILL.md index 407c04d..41749a9 100644 --- a/skills/engineering/triage/SKILL.md +++ b/skills/engineering/triage/SKILL.md @@ -40,7 +40,7 @@ For a PR, the same states read against the attached code: `ready-for-agent` mean Every triaged issue should carry exactly one category role and one state role. If state roles conflict, flag it and ask the maintainer before doing anything else. -These are canonical role names — the actual label strings used in the issue tracker may differ. The mapping should have been provided to you - call the Skill tool with "setup-matt-pocock-skills" if not. +These are canonical role names — the actual label strings used in the issue tracker may differ. The mapping should have been provided to you. If not, tell the user to run `/setup-matt-pocock-skills`. State transitions: an unlabeled issue normally goes to `needs-triage` first; from there it moves to `needs-info`, `ready-for-agent`, `ready-for-human`, or `wontfix`. `needs-info` returns to `needs-triage` once the reporter replies. The maintainer can override at any time — flag transitions that look unusual and ask before proceeding. diff --git a/skills/engineering/wayfinder/SKILL.md b/skills/engineering/wayfinder/SKILL.md index 2a10850..7fe64c1 100644 --- a/skills/engineering/wayfinder/SKILL.md +++ b/skills/engineering/wayfinder/SKILL.md @@ -22,7 +22,7 @@ The map is a single issue on this repo's issue tracker, labelled `wayfinder:map` The map is an **index**, not a store. It lists the decisions made and points at the tickets that hold their detail; a decision lives in exactly one place — its ticket — so the map never restates it, only gists it and links. -**Where the map, its child tickets, blocking, and frontier queries physically live is tracker-specific.** The issue tracker should have been provided to you — call the Skill tool with "setup-matt-pocock-skills" if not. Consult the tracker doc's "Wayfinding operations" section for how _this_ repo expresses them. If no tracker has been provided, default to the local-markdown tracker. +**Where the map, its child tickets, blocking, and frontier queries physically live is tracker-specific.** The issue tracker should have been provided to you. If not, tell the user to run `/setup-matt-pocock-skills`. Consult the tracker doc's "Wayfinding operations" section for how _this_ repo expresses them. If no tracker has been provided, default to the local-markdown tracker. ### The map body