mirror of
https://github.com/mattpocock/skills.git
synced 2026-09-12 18:38:06 +07:00
Stop skills from calling other user-invoked skills
Five skills (to-spec, wayfinder, to-tickets, triage, code-review) told the agent to call the Skill tool on setup-matt-pocock-skills as a missing-config precondition. It's user-invoked, so per .agents/invocation.md no other skill — user- or model-invoked — can ever reach it that way. PR #878 turned the old soft "/skill"-style prose into a literal "Call the Skill tool with ..." instruction without checking invocation type, which made this fire more reliably in the wrong direction. Reworded all five as instructions to tell the human to run it instead. diagnosing-bugs had the same problem handing its post-mortem off to improve-codebase-architecture, with no human anywhere in that loop to catch the failed call. Removed the hand-off outright rather than softening it, since it rarely fired in practice; Phase 6 is now "Cleanup" only. Added a carve-out to .agents/invocation.md's "Dependencies between them" section — the section PR #878 introduced — so future "call the Skill tool" instructions can't repeat this against a user-invoked target. Fixes #453.
This commit is contained in:
@@ -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`.
|
||||
|
||||
@@ -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.
|
||||
@@ -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` — it's user-invoked, so you can't call it yourself.
|
||||
|
||||
## Process
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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` — it's user-invoked, so you can't call it yourself.
|
||||
|
||||
## Process
|
||||
|
||||
|
||||
@@ -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` — it's user-invoked, so you can't call it yourself.
|
||||
|
||||
## Process
|
||||
|
||||
|
||||
@@ -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` — it's user-invoked, so you can't call it yourself.
|
||||
|
||||
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.
|
||||
|
||||
|
||||
@@ -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` — it's user-invoked, so you can't call it yourself. 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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user