feat(kyberforge): ADR-0020 context contract for skills and agents #103
16
LESSONS.md
16
LESSONS.md
@@ -245,3 +245,19 @@ noticed — and before describing any defect as pre-existing, run `git log -S` o
|
||||
plugin-content and vale-style drift deterministically and has no equivalent gate asserting tool-owned
|
||||
paths stay out of formatter scope — `.claude/settings.json` was the sixteenth exclude and nothing
|
||||
prevents a seventeenth.
|
||||
|
||||
## 2026-08-16 — A rule reversed inside a retrofit leaves no trace unless someone writes it down
|
||||
|
||||
`skill-author/SKILL.md:204` on `main` said "Keep reference chains one level deep — a reference file
|
||||
that references another reference file is rarely loaded correctly." The ADR-0020 retrofit replaced it
|
||||
with "Two hops from `SKILL.md`, never three" in `references/create.md` and `references/retrofit.md`,
|
||||
which permits exactly the chain the old rule banned. The looser rule is the right one and the
|
||||
retrofit could not have shipped without it: dispatch pushes each flow into its own file, so the
|
||||
shipped structure is `SKILL.md` → `improve.md` → `retrofit.md`, and a one-level ceiling would have
|
||||
made the mandatory dispatch pattern illegal. But ADR-0020 says nothing about chain depth, so the
|
||||
reversal was carried entirely by the diff — the new text asserts the new rule with no sign that a
|
||||
contradicting rule ever existed, and a reader who remembers the old one has no way to tell whether it
|
||||
was overturned or overlooked. Fix: when a change inverts a standing authoring rule rather than
|
||||
tightening or restating it, record the inversion where the rule's rationale lives — the ADR if the
|
||||
ADR is the reason, here otherwise. A rule that quietly flips is indistinguishable from a rule that
|
||||
was forgotten, and the second reading is the one that gets it re-added later.
|
||||
|
||||
@@ -111,9 +111,12 @@ Flag as FAIL if:
|
||||
available). `Kyberforge.VagueWording` catches the known filler; imprecision outside that list is
|
||||
judgment.
|
||||
- **A boundary clause naming a target that does not resolve** to a real skill directory or agent
|
||||
|
|
||||
file in the authoring source. No script checks this for an agent file — `validate.sh` resolves
|
||||
boundary targets for skills only, so resolve the name yourself against `plugins/*/.apm/skills/`
|
||||
and `plugins/*/.apm/agents/`.
|
||||
file in the authoring source. `validate.sh` resolves this for agent files at both scopes and
|
||||
reports each unresolved target itself — take its verdict rather than re-resolving the name by
|
||||
hand, because a hand-walk over a different universe can contradict it. What is left to you is
|
||||
semantic and the script cannot reach it: whether a target that *does* resolve is the right
|
||||
sibling to exclude, and whether a clause naming no target at all ("examine the files manually")
|
||||
should have named one.
|
||||
- **`Use proactively` in a Copilot or vendor-neutral description.**
|
||||
`KyberforgeCopilot.ProactivePhrase` catches it. The phrase steers the Claude Code runtime and
|
||||
does nothing anywhere else, so in a `.agent.md` it is preloaded text that buys no behaviour.
|
||||
|
||||
@@ -53,6 +53,8 @@ Gates `agent-audit` enforces at every scope:
|
||||
- **Body** — no word gate, and a delegation check in its place: name the skill to invoke rather than restating what it does.
|
||||
- **Invocation** — decide whether the agent is model-delegated or reached only by name. Only Copilot's cloud/IDE format expresses that in frontmatter (`disable-model-invocation`, `user-invocable`).
|
||||
|
||||
At every scope, five tools reach no subagent whatever `tools` says — `AskUserQuestion`, `EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup`, `WaitForMcpServers`. Never write a body that has the agent ask the user a question or enter plan mode; it describes a turn the runtime cannot give it.
|
||||
|
||||
## Step 4 — Validate and close
|
||||
|
||||
Invoke `agent-audit` on each file written and resolve every FAIL before reporting done. It checks the field allowlist, name-to-stem match, leftover placeholders and template comments, the description budget and the Copilot body limit — do not hand-check those.
|
||||
|
||||
@@ -25,12 +25,13 @@ description: FILL IN: Use when <trigger>. <One capability clause.> Not <thing> -
|
||||
|
||||
<!-- tools: Read, Bash, Grep
|
||||
Optional. Allowlist of tool names: a comma-separated string or a YAML list.
|
||||
Omit to inherit all tools from parent.
|
||||
Restrict it to what the agent actually needs. Omit only when it needs them
|
||||
all — omitting inherits every tool from the parent.
|
||||
Use Agent(type1,type2) to restrict which subagent types this agent can spawn.
|
||||
Omit Agent entirely to prevent this agent from spawning subagents.
|
||||
Never available to subagents regardless of tools field:
|
||||
AskUserQuestion, EnterPlanMode, ExitPlanMode, ScheduleWakeup, WaitForMcpServers
|
||||
Exception: ExitPlanMode IS available when parent session runs in permissionMode: plan -->
|
||||
Listing any of them is a finding: agent-audit enforces the flat rule. -->
|
||||
|
||||
<!-- model: sonnet
|
||||
Optional. Aliases: sonnet, opus, haiku, fable. Or full model ID.
|
||||
|
||||
@@ -57,6 +57,11 @@ procedure a skill it can invoke already owns is an `agent-audit` FAIL. When a si
|
||||
missing procedure, check first whether an installed skill owns it and name that skill instead of
|
||||
transcribing it. See `references/contract.md`.
|
||||
|
||||
The delegation check is not a length brake — it fires only on procedure an invocable skill already
|
||||
owns, and says nothing about original prose. That brake is judgment, and it is the only one left:
|
||||
for every sentence you add, ask "would the agent get this wrong without it?" and delete it if the
|
||||
answer is no.
|
||||
|
||||
**Explain the why.** Reasoning-based instructions outperform rigid directives. A rule written in
|
||||
all caps (ALWAYS/NEVER) is usually better reframed as why the behaviour matters, so the agent can
|
||||
apply judgment at the edges.
|
||||
@@ -73,4 +78,10 @@ that was already there.
|
||||
If the edit adds or removes research-sourced content, update `source_keys` in the edited file and
|
||||
the matching `sources.md` entry — the create flow's Step 3 has the rules.
|
||||
|
||||
**Check for regressions before handing back.** `SKILL.md` Step 4 tells you to resolve every FAIL,
|
||||
which says nothing about a check that passed *before* these edits and no longer does. Compare the
|
||||
closing `agent-audit` against the agent's pre-edit state — a PASS that has become a SUGGESTION, or
|
||||
a SUGGESTION that has become a FAIL, is damage this flow caused and is in scope for it. Only the
|
||||
improve flow can make that comparison; the create flow has no prior state to compare against.
|
||||
|
||||
Then return to `SKILL.md` Step 4.
|
||||
|
||||
@@ -37,6 +37,11 @@ The rule is about a field's *shape*, not a fixed roster:
|
||||
Claude Code honours it for plugin subagents; the three fields plugin agents do silently ignore
|
||||
are `hooks`, `mcpServers` and `permissionMode`, and this is not one of them. Copilot's handling
|
||||
of the key is unconfirmed, which ADR-0016 accepts as a stated risk.
|
||||
|
||||
Its syntax is the same at every scope, and this is the one scope that cannot reach it anywhere
|
||||
else: MCP tools are denied as `mcp__<server>`, `mcp__<server>__*` or `mcp__*`; both a YAML list
|
||||
and a delimited string are accepted, and this repo writes the comma-separated string form
|
||||
(`disallowedTools: Edit, Write, NotebookEdit`) — match it.
|
||||
- The Claude-only knobs (`isolation`, `maxTurns`, `effort`, `memory`, `permissionMode`, `skills`,
|
||||
`color`, `initialPrompt`, `background`, `hooks`, `mcpServers`) have no Copilot equivalent and
|
||||
are never written to this file at all. "Silently ignored at plugin scope" is the wrong framing:
|
||||
|
||||
@@ -25,7 +25,10 @@ duplicate silently.
|
||||
**`description`** — write it against `references/contract.md`. It is the primary signal for
|
||||
autonomous delegation.
|
||||
|
||||
**`tools`** — an allowlist; omit it to inherit every tool from the parent. Use `Agent(type1,type2)`
|
||||
**`tools`** — an allowlist. Write it, and restrict it to the tools the agent actually needs;
|
||||
omitting it inherits every tool from the parent, which is the right value only when the agent
|
||||
genuinely needs all of them. Least privilege is the default, not the exception. Use
|
||||
`Agent(type1,type2)`
|
||||
to restrict which subagent types this agent may spawn, and omit `Agent` entirely to stop it
|
||||
spawning any. Five tools reach no subagent whatever this field says — `AskUserQuestion`,
|
||||
`EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup` and `WaitForMcpServers` — so listing one buys
|
||||
|
||||
@@ -35,7 +35,7 @@ scripts/vale-wrap.sh <skill-dir>/SKILL.md
|
||||
|
||||
`validate.sh` findings become the `### Structure` dimension — its FAILs and its SUGGESTIONs both.
|
||||
|
||||
If any of the three fails, cannot run, or reports something needing interpretation, read `references/validation-scripts.md` — it carries the manual fallback and the misleading exit codes.
|
||||
If any of the three cannot run, or exits non-zero for a reason other than findings, read `references/validation-scripts.md` — it carries the manual fallback and the misleading exit codes. Ordinary content FAILs are the expected outcome here and need no fallback.
|
||||
|
||||
`validate-provenance.sh` prints nothing on success. Its FAIL and INFO findings become a separate `### Provenance` dimension, and it emits Why and Fix itself — surface those verbatim.
|
||||
|
||||
|
||||
@@ -28,8 +28,10 @@ Include content the agent lacks:
|
||||
- The specific tools or sequences to use — not the full range of options
|
||||
- One default per decision point with one escape hatch
|
||||
|
||||
Move to `references/`, behind an explicit "If X, read `references/file.md`" trigger — the literal
|
||||
conditional form, never a generic pointer:
|
||||
Move to `references/`, behind an explicit "If X, read `references/<file>.md`" trigger — the literal
|
||||
conditional form, never a generic pointer. Write the real filename in the skill under audit; the
|
||||
angle brackets are a placeholder here, and a literal `references/file.md` in a body is an ERROR
|
||||
from the ADR-0020 gate because no such file exists on disk. Move:
|
||||
|
||||
- Lookup tables and spec restatements
|
||||
- Output schemas, templates and example blocks
|
||||
|
||||
@@ -32,8 +32,16 @@ read after the mistake.
|
||||
inner fence as `` \`\`\` ``. An unescaped inner fence terminates the outer block and the remaining
|
||||
instructions render as prose.
|
||||
|
||||
**Conditional references** state a specific trigger: "If the API returns a non-200 status, read
|
||||
`references/api-errors.md`." The generic form — pointing at the directory and hoping — defeats
|
||||
**Conditional references** state a specific trigger, naming a file that exists in the skill's own
|
||||
`references/` directory:
|
||||
|
||||
```text
|
||||
If the API returns a non-200 status, read `references/api-errors.md`.
|
||||
```
|
||||
|
||||
That block is fenced because the filename in it is illustrative — an unfenced `references/` pointer
|
||||
in a `SKILL.md` body must resolve on disk or the ADR-0020 gate reports a hard ERROR. The generic
|
||||
form — pointing at the directory and hoping — defeats
|
||||
progressive disclosure, because the agent either loads everything or loads nothing.
|
||||
`Kyberforge.PaddingPhrase` catches the common generic phrasing deterministically; other malformed
|
||||
forms are judgment.
|
||||
|
||||
@@ -85,4 +85,10 @@ improvise the cuts — four dry runs invented six to ten different answers to th
|
||||
If a signal points to a script or reference file, edit that file directly rather than adding a
|
||||
workaround in SKILL.md.
|
||||
|
||||
**Check for regressions before handing back.** `SKILL.md` Step 4 tells you to resolve every FAIL,
|
||||
which says nothing about a check that passed *before* these edits and no longer does. Compare the
|
||||
closing audit against the skill's pre-edit state — a PASS that has become a SUGGESTION, or a
|
||||
SUGGESTION that has become a FAIL, is damage this flow caused and is in scope for it. Only the
|
||||
improve flow can make that comparison; the create flow has no prior state to compare against.
|
||||
|
||||
Then return to `SKILL.md` Step 4.
|
||||
|
||||
@@ -111,9 +111,12 @@ Flag as FAIL if:
|
||||
available). `Kyberforge.VagueWording` catches the known filler; imprecision outside that list is
|
||||
judgment.
|
||||
- **A boundary clause naming a target that does not resolve** to a real skill directory or agent
|
||||
file in the authoring source. No script checks this for an agent file — `validate.sh` resolves
|
||||
boundary targets for skills only, so resolve the name yourself against `plugins/*/.apm/skills/`
|
||||
and `plugins/*/.apm/agents/`.
|
||||
file in the authoring source. `validate.sh` resolves this for agent files at both scopes and
|
||||
reports each unresolved target itself — take its verdict rather than re-resolving the name by
|
||||
hand, because a hand-walk over a different universe can contradict it. What is left to you is
|
||||
semantic and the script cannot reach it: whether a target that *does* resolve is the right
|
||||
sibling to exclude, and whether a clause naming no target at all ("examine the files manually")
|
||||
should have named one.
|
||||
- **`Use proactively` in a Copilot or vendor-neutral description.**
|
||||
`KyberforgeCopilot.ProactivePhrase` catches it. The phrase steers the Claude Code runtime and
|
||||
does nothing anywhere else, so in a `.agent.md` it is preloaded text that buys no behaviour.
|
||||
|
||||
@@ -53,6 +53,8 @@ Gates `agent-audit` enforces at every scope:
|
||||
- **Body** — no word gate, and a delegation check in its place: name the skill to invoke rather than restating what it does.
|
||||
- **Invocation** — decide whether the agent is model-delegated or reached only by name. Only Copilot's cloud/IDE format expresses that in frontmatter (`disable-model-invocation`, `user-invocable`).
|
||||
|
||||
At every scope, five tools reach no subagent whatever `tools` says — `AskUserQuestion`, `EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup`, `WaitForMcpServers`. Never write a body that has the agent ask the user a question or enter plan mode; it describes a turn the runtime cannot give it.
|
||||
|
||||
## Step 4 — Validate and close
|
||||
|
||||
Invoke `agent-audit` on each file written and resolve every FAIL before reporting done. It checks the field allowlist, name-to-stem match, leftover placeholders and template comments, the description budget and the Copilot body limit — do not hand-check those.
|
||||
|
||||
@@ -25,12 +25,13 @@ description: FILL IN: Use when <trigger>. <One capability clause.> Not <thing> -
|
||||
|
||||
<!-- tools: Read, Bash, Grep
|
||||
Optional. Allowlist of tool names: a comma-separated string or a YAML list.
|
||||
Omit to inherit all tools from parent.
|
||||
Restrict it to what the agent actually needs. Omit only when it needs them
|
||||
all — omitting inherits every tool from the parent.
|
||||
Use Agent(type1,type2) to restrict which subagent types this agent can spawn.
|
||||
Omit Agent entirely to prevent this agent from spawning subagents.
|
||||
Never available to subagents regardless of tools field:
|
||||
AskUserQuestion, EnterPlanMode, ExitPlanMode, ScheduleWakeup, WaitForMcpServers
|
||||
Exception: ExitPlanMode IS available when parent session runs in permissionMode: plan -->
|
||||
Listing any of them is a finding: agent-audit enforces the flat rule. -->
|
||||
|
||||
<!-- model: sonnet
|
||||
Optional. Aliases: sonnet, opus, haiku, fable. Or full model ID.
|
||||
|
||||
@@ -57,6 +57,11 @@ procedure a skill it can invoke already owns is an `agent-audit` FAIL. When a si
|
||||
missing procedure, check first whether an installed skill owns it and name that skill instead of
|
||||
transcribing it. See `references/contract.md`.
|
||||
|
||||
The delegation check is not a length brake — it fires only on procedure an invocable skill already
|
||||
owns, and says nothing about original prose. That brake is judgment, and it is the only one left:
|
||||
for every sentence you add, ask "would the agent get this wrong without it?" and delete it if the
|
||||
answer is no.
|
||||
|
||||
**Explain the why.** Reasoning-based instructions outperform rigid directives. A rule written in
|
||||
all caps (ALWAYS/NEVER) is usually better reframed as why the behaviour matters, so the agent can
|
||||
apply judgment at the edges.
|
||||
@@ -73,4 +78,10 @@ that was already there.
|
||||
If the edit adds or removes research-sourced content, update `source_keys` in the edited file and
|
||||
the matching `sources.md` entry — the create flow's Step 3 has the rules.
|
||||
|
||||
**Check for regressions before handing back.** `SKILL.md` Step 4 tells you to resolve every FAIL,
|
||||
which says nothing about a check that passed *before* these edits and no longer does. Compare the
|
||||
closing `agent-audit` against the agent's pre-edit state — a PASS that has become a SUGGESTION, or
|
||||
a SUGGESTION that has become a FAIL, is damage this flow caused and is in scope for it. Only the
|
||||
improve flow can make that comparison; the create flow has no prior state to compare against.
|
||||
|
||||
Then return to `SKILL.md` Step 4.
|
||||
|
||||
@@ -37,6 +37,11 @@ The rule is about a field's *shape*, not a fixed roster:
|
||||
Claude Code honours it for plugin subagents; the three fields plugin agents do silently ignore
|
||||
are `hooks`, `mcpServers` and `permissionMode`, and this is not one of them. Copilot's handling
|
||||
of the key is unconfirmed, which ADR-0016 accepts as a stated risk.
|
||||
|
||||
Its syntax is the same at every scope, and this is the one scope that cannot reach it anywhere
|
||||
else: MCP tools are denied as `mcp__<server>`, `mcp__<server>__*` or `mcp__*`; both a YAML list
|
||||
and a delimited string are accepted, and this repo writes the comma-separated string form
|
||||
(`disallowedTools: Edit, Write, NotebookEdit`) — match it.
|
||||
- The Claude-only knobs (`isolation`, `maxTurns`, `effort`, `memory`, `permissionMode`, `skills`,
|
||||
`color`, `initialPrompt`, `background`, `hooks`, `mcpServers`) have no Copilot equivalent and
|
||||
are never written to this file at all. "Silently ignored at plugin scope" is the wrong framing:
|
||||
|
||||
@@ -25,7 +25,10 @@ duplicate silently.
|
||||
**`description`** — write it against `references/contract.md`. It is the primary signal for
|
||||
autonomous delegation.
|
||||
|
||||
**`tools`** — an allowlist; omit it to inherit every tool from the parent. Use `Agent(type1,type2)`
|
||||
**`tools`** — an allowlist. Write it, and restrict it to the tools the agent actually needs;
|
||||
omitting it inherits every tool from the parent, which is the right value only when the agent
|
||||
genuinely needs all of them. Least privilege is the default, not the exception. Use
|
||||
`Agent(type1,type2)`
|
||||
to restrict which subagent types this agent may spawn, and omit `Agent` entirely to stop it
|
||||
spawning any. Five tools reach no subagent whatever this field says — `AskUserQuestion`,
|
||||
`EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup` and `WaitForMcpServers` — so listing one buys
|
||||
|
||||
@@ -35,7 +35,7 @@ scripts/vale-wrap.sh <skill-dir>/SKILL.md
|
||||
|
||||
`validate.sh` findings become the `### Structure` dimension — its FAILs and its SUGGESTIONs both.
|
||||
|
||||
If any of the three fails, cannot run, or reports something needing interpretation, read `references/validation-scripts.md` — it carries the manual fallback and the misleading exit codes.
|
||||
If any of the three cannot run, or exits non-zero for a reason other than findings, read `references/validation-scripts.md` — it carries the manual fallback and the misleading exit codes. Ordinary content FAILs are the expected outcome here and need no fallback.
|
||||
|
||||
`validate-provenance.sh` prints nothing on success. Its FAIL and INFO findings become a separate `### Provenance` dimension, and it emits Why and Fix itself — surface those verbatim.
|
||||
|
||||
|
||||
@@ -28,8 +28,10 @@ Include content the agent lacks:
|
||||
- The specific tools or sequences to use — not the full range of options
|
||||
- One default per decision point with one escape hatch
|
||||
|
||||
Move to `references/`, behind an explicit "If X, read `references/file.md`" trigger — the literal
|
||||
conditional form, never a generic pointer:
|
||||
Move to `references/`, behind an explicit "If X, read `references/<file>.md`" trigger — the literal
|
||||
conditional form, never a generic pointer. Write the real filename in the skill under audit; the
|
||||
angle brackets are a placeholder here, and a literal `references/file.md` in a body is an ERROR
|
||||
from the ADR-0020 gate because no such file exists on disk. Move:
|
||||
|
||||
- Lookup tables and spec restatements
|
||||
- Output schemas, templates and example blocks
|
||||
|
||||
@@ -32,8 +32,16 @@ read after the mistake.
|
||||
inner fence as `` \`\`\` ``. An unescaped inner fence terminates the outer block and the remaining
|
||||
instructions render as prose.
|
||||
|
||||
**Conditional references** state a specific trigger: "If the API returns a non-200 status, read
|
||||
`references/api-errors.md`." The generic form — pointing at the directory and hoping — defeats
|
||||
**Conditional references** state a specific trigger, naming a file that exists in the skill's own
|
||||
`references/` directory:
|
||||
|
||||
```text
|
||||
If the API returns a non-200 status, read `references/api-errors.md`.
|
||||
```
|
||||
|
||||
That block is fenced because the filename in it is illustrative — an unfenced `references/` pointer
|
||||
in a `SKILL.md` body must resolve on disk or the ADR-0020 gate reports a hard ERROR. The generic
|
||||
form — pointing at the directory and hoping — defeats
|
||||
progressive disclosure, because the agent either loads everything or loads nothing.
|
||||
`Kyberforge.PaddingPhrase` catches the common generic phrasing deterministically; other malformed
|
||||
forms are judgment.
|
||||
|
||||
@@ -85,4 +85,10 @@ improvise the cuts — four dry runs invented six to ten different answers to th
|
||||
If a signal points to a script or reference file, edit that file directly rather than adding a
|
||||
workaround in SKILL.md.
|
||||
|
||||
**Check for regressions before handing back.** `SKILL.md` Step 4 tells you to resolve every FAIL,
|
||||
which says nothing about a check that passed *before* these edits and no longer does. Compare the
|
||||
closing audit against the skill's pre-edit state — a PASS that has become a SUGGESTION, or a
|
||||
SUGGESTION that has become a FAIL, is damage this flow caused and is in scope for it. Only the
|
||||
improve flow can make that comparison; the create flow has no prior state to compare against.
|
||||
|
||||
Then return to `SKILL.md` Step 4.
|
||||
|
||||
Reference in New Issue
Block a user
This is factually wrong about the validator.
check_boundary()is called at both scopes —agent-audit/scripts/validate.sh:1124(plugin/APM) and:1213(project/user) — and I confirmed it fires on a real agent file (apm-orchestrate.agent.mdemits a boundary SUGGESTION).Consequence: the auditor hand-resolves what the script already resolved, and a hand-derived verdict can contradict the script's on the same file.