fix(procedures): pin both gate forks to sonnet — a context:fork skill inherits the session model, not its agent's

[drewdrewthis]

What

Both gate skills now pin their fork's model: model: sonnet in skills/how-do-i/SKILL.md and skills/am-i-done/SKILL.md.

Refs #22.

Why the agent-side declaration was not enough

agents/procedure-scout.md:4 has said model: sonnet all along. It was ignored, because a context: fork skill inherits the parent session's model, not the model its agent: declares.

Measured, three dispatches:

Path	Agent frontmatter	Model that actually ran
Agent(subagent_type: "researcher")	model: sonnet	claude-sonnet-5
Agent(subagent_type: "fast-coder")	model: haiku	claude-haiku-4-5-20251001
/how-do-i → context: fork, agent: procedure-scout	model: sonnet	claude-opus-5 (the parent session's model)

So the declaration works on the Agent() path and is silently dropped on the fork path. That is the whole bug.

Across 167 forked /how-do-i transcripts the split was opus 1,546 / sonnet 1,307 / fable 1,149 turns — all three tiers inside one skill, which is what inheritance from differently-configured parent sessions looks like. ~35% of the skill's weighted spend ran on Opus, for a job that greps record frontmatter and hands back a digest. /how-do-i is ~12% of all token spend measured on this box.

Why this is a declaration and not a rule in the agent body

Prose cannot fix it. The harness resolves the model before the agent body is ever read, so an instruction like "do not run this on Opus" is addressed to a model that has already been chosen and billed. SKILL_TEMPLATE.md:48 makes the same point about dispatch generally — "The harness spawns the subagent — do NOT write prose telling the model to dispatch the X agent. Declare it."

model is a documented SKILL.md frontmatter key (SKILL_TEMPLATE.md:82 lists model and effort). Verified this turn: no skill anywhere on this box sets either. The key was documented and never exercised, which is why nobody noticed the fork path was ignoring the agent-side value.

What I deliberately did not change

effort: is the obvious next lever and the same template line lists it. Not set here — changing two variables at once makes the before/after unattributable. Worth a follow-up once this one is measured.

Human verification

(Evidence below; the probe transcripts are the load-bearing part.)

How I can prove I was successful

proven — hooks/tests/gate-skill-model.bats, 4 assertions. Against origin/main:

not ok 1 how-do-i pins its fork to sonnet in SKILL.md frontmatter
not ok 2 am-i-done pins its fork to sonnet in SKILL.md frontmatter
not ok 3 every context:fork skill declares a model (none may inherit the session tier)
ok  4 the agent-side model declaration is still present (belt and braces)

All 4 pass on this branch. Assertion 3 guards the next fork skill anyone adds, not just today's two. Full suite: 154 → 158, 0 failures.

The frontmatter parser reads the first --- block only, so a mention in the body cannot satisfy the assertion.

missing — the load-bearing check. These tests prove the declaration is present. They cannot prove the harness honours it; that needs a live dispatch after a plugin reload, which I cannot trigger from inside a running session.

After merging and running /reload-plugins, this is the check:

# invoke the gate, then read the model off the fork's own transcript
grep -o '"model":"[^"]*"' /tmp/claude-1000/<project>/<session-id>/tasks/<agentId>.output \
  | sort | uniq -c

Expect claude-sonnet-5. If it still reads claude-opus-5, model: is not honoured on the fork path either, this PR is a no-op, and #22 needs a harness-level answer instead.

⚠ Do not verify from settings.json or from the agent's self-report — settings.json on this box says fable while sessions run opus. Only the transcript model field is authoritative. And never Read/tail a task .output file; it is full subagent JSONL and will overflow context. grep -o only.

Expected saving, if honoured

On the priced subset (19.26M opus+sonnet weighted units, fable excluded — no verified rate): ~$168 at the measured mix vs ~$58 all-Sonnet, ~66% off, with no change to the flow. Rates are assumed list ratios, not verified; and all figures predate PR #9, so re-measure rather than trusting the absolute numbers.

Summary by CodeRabbit

• Enhancements

  • Improved consistency for forked skills by explicitly selecting the Sonnet model.
  • Updated the “How do I?” and “Am I done?” skills with the appropriate model configuration.

• Tests

  • Added coverage to verify model declarations across forked skills and their related agents.

No UI surface. This change is two YAML frontmatter keys plus a bats suite — no runtime UI, no rendered output, no user-facing surface to screenshot. The observable behaviour is which model a forked subagent runs on, and the evidence for that is the two-state transcript probe above (an opus-parent fork moving to claude-haiku-4-5-20251001 when the skill declared model: haiku, with the parent's own turns staying on opus), not a screenshot.

[coderabbitai]

Warning

Review limit reached

@drewdrewthis, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 5 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 38f4e27e-d868-49f3-ad99-1014d3ec49da

📥 Commits

Reviewing files that changed from the base of the PR and between d12b9e2 and c58958c.

📒 Files selected for processing (8)

• .coderabbit.yaml
• CONTRIBUTING.md
• README.md
• plugins/procedures/agents/procedure-scout.md
• plugins/procedures/agents/work-reviewer.md
• plugins/procedures/hooks/tests/gate-skill-model.bats
• plugins/procedures/skills/am-i-done/SKILL.md
• plugins/procedures/skills/how-do-i/SKILL.md

📝 Walkthrough

Walkthrough

The forked how-do-i and am-i-done skills now declare model: sonnet. New Bats tests validate these declarations, scan all forked skills, and verify related agent declarations.

Changes

Forked skill model configuration

Layer / File(s)	Summary
Add Sonnet model declarations plugins/procedures/skills/*/SKILL.md	The how-do-i and am-i-done skill frontmatter now specifies model: sonnet.
Validate model declarations plugins/procedures/hooks/tests/gate-skill-model.bats	Tests extract frontmatter keys, check both skills, scan all forked skills, and verify agent model declarations.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• Issue 22: The changes add and test the model: sonnet declarations for the how-do-i and am-i-done forked skills.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the primary change: pinning both forked gate skills to Sonnet.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/pin-gate-skill-model

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[drewdrewthis]

Two open questions a reviewer should weigh

1. Why frontmatter model: and not converting the gate to an Agent(subagent_type:) dispatch?

Worth stating plainly, because the evidence cuts against the choice made here. The measured facts:

• Agent(subagent_type:) does honour an agent's frontmatter model: — proven on two tiers.
• context: fork does not — it takes the parent session's model.
• SKILL.md model: is documented (SKILL_TEMPLATE.md:82) but used by zero skills on this box.

So this PR bets on an untested key, while the path measured as working was not taken. The reason is that converting the gate to an Agent() dispatch would move the search logic out of the fork contract that three separate "skills are thin gateways" statements require (one lint-enforced, rule D3), and ADR-016 declares the context: fork + agent: shape as architecture rather than incidental. Changing that is an ADR amendment, not a frontmatter edit.

The cheap path first, the architectural one only if it fails. If the post-reload probe shows claude-opus-5, this PR is a no-op and the Agent() conversion becomes the live option — that is the fallback, not a fix to layer on.

2. The tools: allowlists merged in #23 may be a no-op for exactly the same reason.

#23 added tools: to agents/procedure-scout.md and agents/work-reviewer.md — the agent files. That is the same location whose model: the fork path was just measured ignoring. Nothing rules out that tools: is dropped on the fork path too.

One probe answers both. After merging this and running /reload-plugins:

grep -o '"model":"[^"]*"' /tmp/claude-1000/<project>/<session-id>/tasks/<agentId>.output | sort | uniq -c

and in the same transcript, check whether the reviewer fork ever calls Bash (it should not be able to). If model: pins but tools: is still ignored, #23's allowlist needs to move into SKILL.md frontmatter as well — and the work-reviewer boundary it was meant to enforce is still prose-only.

I have not run either check: a plugin reload cannot be triggered from inside a running session, and the session that reviewed this work was still on the pre-#23 plugin.

[drewdrewthis]

⚠️ Converting to draft — model: may not mean what this PR assumes

Read the live field reference directly (code.claude.com/docs/en/skills, line 267 of the fetched page). Verbatim:

| model | No | Model to use when this skill is active. The override applies for the rest of the current turn and is not saved to settings; the session model resumes on your next prompt. Accepts the same values as /model, or inherit to keep the active model. |

That is described as a turn-scoped session override, not a fork-scoped pin. Compare the two neighbouring fields, which are explicitly fork-scoped:

| agent | No | Which subagent type to use when context: fork is set. |
| background | No | Only applies with context: fork. Set to false to wait for the forked subagent's result… |

model carries no such qualifier. On the plain reading, adding model: sonnet to how-do-i/SKILL.md may drop the main session to Sonnet for the remainder of every turn the gate fires on — and the gate fires at the start of essentially every turn. That would silently downgrade the caller doing the actual work, to save money on the scout.

If that is what happens, this PR is not a no-op — it is actively harmful, and worse than the leak it targets.

The probe now has to answer two questions, not one

After /reload-plugins, invoke /how-do-i and check both:

# 1. the fork's own model — did the pin land where intended?
grep -o '"model":"[^"]*"' /tmp/claude-1000/<project>/<session-id>/tasks/<agentId>.output | sort | uniq -c

# 2. the PARENT session's model on the turns AFTER the gate fired — the side effect
grep -o '"model":"[^"]*"' <the parent session .jsonl> | sort | uniq -c

Outcomes:

Fork model	Parent model after gate	Verdict
sonnet	unchanged (opus)	Merge. Does exactly what is wanted.
sonnet	downgraded to sonnet	Close. The saving is real but it is paid by the main agent.
opus	unchanged	No-op; escalate to the Agent() conversion discussed above.

Until someone runs that, this stays a draft. I cannot trigger a plugin reload from inside a running session.

Also worth recording — two things called "fork" behave oppositely

• Skill frontmatter context: fork (what these gates use): "Add context: fork to your frontmatter when you want a skill to run in isolation. The skill content becomes the prompt that drives the subagent. It won't have access to your conversation history." Measured on 197 /how-do-i forks: first-turn context averages 39,699 tok against a parent-session average of 197,769 tok, max fork boot 57,951 — well under the parent mean. Isolation confirmed from both directions.
• The Agent tool's subagent_type: "fork": per its own contract, "forks yourself (the fork inherits your full conversation context and always runs on your model — a model override is ignored)."

Same word, opposite behaviour on both context inheritance and model override. Anyone reasoning about "the fork" needs to say which one.

[drewdrewthis]

✅ Probed — model: pins the fork and does NOT downgrade the parent. Undrafting.

My previous comment worried that model's documented wording ("The override applies for the rest of the current turn") meant this would drop the main session to Sonnet on every gated turn. That is wrong. Measured, not inferred.

Two-state probe, real claude -p processes, parent launched --model opus, skill is context: fork + background: false. The only variable is the frontmatter line; the tier is flipped to haiku so the assertion is that the observed value moves, not merely that it is non-empty:

### STATE A (frontmatter: <none>)
--- PARENT assistant models:  3 claude-opus-5
--- FORK models:              1 claude-opus-5

### STATE B (frontmatter: model: haiku)
--- PARENT assistant models:  3 claude-opus-5          <- UNCHANGED
--- FORK models:              2 claude-haiku-4-5-20251001   <- MOVED

Three findings, each load-bearing:

1. Without model:, the fork inherits the parent — opus parent, opus fork. This is the tier leak (procedure-scout declares model: sonnet but ~35% of /how-do-i spend runs on Opus — tier declaration may not be honoured for forked subagents #22) reproduced from scratch in a clean project.
2. With model:, the fork is pinned — the same fork runs claude-haiku-4-5-20251001. So the key IS honoured on the context: fork path.
3. The parent is untouched — 3 opus assistant turns in both states, including turns after the skill returned. The "rest of the current turn" wording does not manifest as a parent downgrade for a forked skill; the override scopes to the fork.

So the concern that drafted this PR is disproven, and the third row of the decision table in my earlier comment ("fork opus / parent unchanged → no-op") does not apply either. This does what it says.

Method

Project-scoped skill under a scratch dir's .claude/skills/, so the installed plugin was never touched. Parent model read from $CLAUDE_CONFIG_DIR/projects/<slug>/<session-id>.jsonl; fork model from the sibling <session-id>/ subagent transcripts — the fork's model is not in the flat parent jsonl, and grepping only that file reports the parent's model and reads like the pin failed.

A first attempt with a throwaway CLAUDE_CONFIG_DIR returned Not logged in · Please run /login and produced no transcripts; the run above uses the real config with a project-scoped skill instead.

What this does not prove

• Only that model: is honoured. Whether the tools: allowlists merged in fix(procedures): make a bad query loud, and bound the scout's toolset #23 bind on the fork path is still unmeasured — same open question, different key.
• The probe used a trivial one-line skill. It does not show that procedure-scout produces an acceptable digest on Sonnet — only that it will run on Sonnet. Quality at the pinned tier is the thing to watch after this lands.

[drewdrewthis]

Scope of what the probe proved — read before merging

The two-state result above proves the mechanism: model: on a context: fork skill pins the fork, and leaves the parent session alone. It does not prove the change is safe to ship.

The probe used a trivial one-line skill whose whole job was to name its own model. It shows procedure-scout will run on Sonnet. It says nothing about whether the digest it produces on Sonnet is as good as on Opus — and digest quality is the actual risk here, since the whole point of the scout is that the caller trusts its synthesis without re-reading the corpus.

Mechanism proven; output quality at the pinned tier unmeasured. Watch the first few /how-do-i results after this lands — specifically whether it still catches traps and labels standing correctly, which is where a weaker model would degrade first without looking degraded.

One clarification on wording used earlier in this PR

"model: is not honoured on the fork path" has appeared in these comments and is ambiguous. Precisely:

• The AGENT file's model: (agents/procedure-scout.md:4) — not honoured on the fork path. That is the bug this PR works around, measured last turn: an agent declaring model: sonnet, dispatched from an opus session, ran claude-opus-5.
• The SKILL file's model: (what this PR adds) — is honoured, per the two-state probe above.
• The AGENT file's tools: — is honoured, per the probe on fix(procedures): make a bad query loud, and bound the scout's toolset #23.

So within one agent file, tools: binds and model: does not. That asymmetry is measured, not inferred, and it is the reason the fix belongs in SKILL.md rather than in the agent.

Not done on this PR

CONTRIBUTING asks for four repo-local reviewer agents (principles-reviewer, hygiene-reviewer, security-reviewer, test-reviewer) on a change like this. They were not run — this session is under a standing instruction not to dispatch subagents unprompted. With check-runs count 0 on this repo (#20), hand-run bats is the only backstop, so this is worth a deliberate call rather than a silent skip. Say the word and I will run them before merge.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/procedures/hooks/tests/gate-skill-model.bats`:
- Around line 48-53: Update the test “the agent-side model declaration is still
present (belt and braces)” to validate each agent declaration independently
instead of using one multi-file grep. Call frontmatter_key separately for
procedure-scout.md and work-reviewer.md, and assert each returned value equals
sonnet.
- Around line 1-2: Add a `# PLUGIN ADAPTATION: <why>` marker to
`gate-skill-model.bats` at the point where this vendored test diverges,
documenting why the adaptation is required. Keep the existing Bats test content
unchanged.

In `@plugins/procedures/skills/am-i-done/SKILL.md`:
- Line 7: Remove the unmarked model: sonnet frontmatter addition from both
plugins/procedures/skills/am-i-done/SKILL.md (line 7) and
plugins/procedures/skills/how-do-i/SKILL.md (line 7), restoring byte-close
parity with orchard-codex@develop-sweatshop; alternatively, apply the documented
vendor sync or add a valid PLUGIN ADAPTATION marker only if the change fits an
approved README adaptation class.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 036a19eb-0549-48ad-96b6-3e29c034d2c9

📥 Commits

Reviewing files that changed from the base of the PR and between 6327b8c and d12b9e2.

📒 Files selected for processing (3)

• plugins/procedures/hooks/tests/gate-skill-model.bats
• plugins/procedures/skills/am-i-done/SKILL.md
• plugins/procedures/skills/how-do-i/SKILL.md

[drewdrewthis]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[drewdrewthis]

Review verdict: NOT-READY

Reviewed at: c58958c · Run: review (own-PR)

Blocking — must resolve before this PR is done

1. [coderabbit] The named reviewer has never read this PR's current code. Its commit status reads state: success, but the description is Review rate limited and [.[] | select(.commit_id==<head>)] | length returns 0. Green in name only — "rate limited" is pending, not approved. A re-review has been requested and a monitor is armed; this flips to READY when a review lands at head with no unresolved threads. This item is the only thing blocking.

An earlier revision of this comment headlined READY while disclosing this in the body. That was a false green: pr-ready-check.sh is structurally blind to whether CodeRabbit reviewed, so it cannot be the thing that licenses the claim.

Internal fan-out — clean at this SHA

test-reviewer, principles-reviewer, hygiene-reviewer, security-reviewer — all four, not a trimmed subset. Each finding below was raised by exactly one reviewer and missed by the other three, which is why the full set runs even on a diff this small. All are fixed.

1. [test-reviewer] The suite could pass having checked zero files. With no nullglob, a wrong $SKILLS makes "$SKILLS"/*/SKILL.md iterate once over the literal unexpanded pattern; every check falls through and it reports green. Reproduced live against a nonexistent directory.
2. [hygiene-reviewer] A third hand-rolled frontmatter reader. scripts/lib/frontmatter.sh already exports frontmatter_block + fm_value, and its own header records that it exists because byte-identical copies drifted apart. Adding a copy re-created the failure the lib was written to prevent. Now sourced.
3. [principles-reviewer] The docs said "two adaptation classes" while this PR added a third. .coderabbit.yaml is configured to flag any .md divergence outside the documented classes — i.e. configured to flag this PR's own lines. The class is now documented in README (canonical home, carrying the measurement), CONTRIBUTING points at it, and .coderabbit.yaml's claim that .md has "no comment syntax for this" is corrected: YAML frontmatter takes # comments, which is what this PR uses.

The invariant is now agreement between the skill pin and the agent declaration rather than sonnet hardcoded in four places, so a tier change cannot land in one file alone. The vacuity guard derives its expected count from the corpus rather than using a hardcoded floor, which decays silently once a third fork skill is added.

Non-vacuity proven by mutation, not asserted — pasted failures, not a glanced tail: a broken glob path fails via the anchor test; drifting one agent to opus fails with pins 'sonnet' but agents/procedure-scout.md declares 'opus'; deleting a pin fails with declares context:fork with no model: pin; silently dropping a fork skill from the loop fails with corpus has 2 context:fork skills but the loop checked 1. Restored: 1..157, grep -c '^not ok' → 0.

[security-reviewer] No findings

No secrets, PII, host paths, or injection vector.

Accepted, unmeasured risk of this change

Digest quality on Sonnet is not measured. The PR proves the pin binds and quantifies the cost, but does not show that a Sonnet-tier scout digest or done-review is as good as an Opus one. Pinning also means sessions previously running /am-i-done on Opus now get a Sonnet-tier review — a rigor trade, not only a cost win. This is the substantive risk a human reviewer should weigh; it is accepted knowingly, not overlooked.

Why this PR touches docs

README.md, CONTRIBUTING.md, .coderabbit.yaml and the two agent files moved because the fan-out found the model pin was an undocumented third adaptation class in a repo whose vendoring rule permits only documented ones — the bot is literally configured to flag it. The doc churn is the fix for that finding, not unrelated scope.

Non-blocking (Decide / New Issue)

• [test-reviewer] A static suite cannot prove the harness honours the pin — only that the declaration exists. The honouring was measured live (an opus-parent fork moved to claude-haiku-4-5; the parent stayed on opus) and now lives in README rather than only in PR comments. A true regression test needs a live dispatch. (New Issue)
• [hygiene-reviewer] If the fork-model gap is harness behaviour rather than a plugin-hosting quirk, worth filing upstream so this is not permanent plugin-only drift. (New Issue)
• [principles-reviewer] The agreement check lives in bats, not in lint-frontmatter.sh, the pass that runs pre-commit. Worth moving if the fork-skill count grows. (Decide)

---

Verdict is prose, not a GitHub approval. Scope: review findings only. It is not a merge-readiness signal; that is pr-ready-check.sh (8 criteria, of which this verdict is C3), which also gates on CI, PR-body format, and visual proof.