You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Promote Model Tier Config and Shared Installer Hook Registration to Main - #3521
#3516 maps model tiers in one config, spec/model-tiers.json, carries the "Model Tiers" rule into the skills that dispatch, and adds a guard that refuses a highest-tier dispatch the maintainer did not allow.
#3518 has the agent-safety installer's hooks share one registration helper.
#3520 has those hooks share one registration-check loop.
It also carries the fixes for this promotion's own review findings, each landed on develop first:
#3522 requires a non-empty model name per tier and names both edits a model release makes.
#3523 states what the highest-tier allowance covers and where the guard stops.
#3524 rejects a tier sharing the maintainer-only model and states the allowance as conduct.
#3525 matches the guard's alias boundaries regardless of case.
#3526 mirrors the guard's CI self-tests in the local runbook.
#3527 rejects a repeated tier name and names every hook the installer registers.
…Dispatch (#3516)
## Summary
Skills name a model tier and leave its model to the cwd repository's
carried `AGENTS.md`. A repository whose `AGENTS.md` predates the tier
table leaves `frontier` unresolved, and sessions read it as "the
frontier model" and dispatched subagents at the most capable,
maintainer-only model. This closes that from three sides:
- **Data home.** `spec/model-tiers.json` states the tier-to-model
mapping per tool. Only Claude Code has a row today, and Codex or
opencode join as data. `spec/validate.py` holds the `AGENTS.md` table
equal to it, since a carried file cannot hold a generated include
region.
- **Rule text carried with the skills.** `AGENTS.md` moves the tier rule
into a "Model Tiers" subsection. It says `frontier` is not the
industry's frontier model and that an orchestrator names a tier on every
dispatch. `local-strict-review`, `unattended-handoff`, and
`backlog-burndown` carry that subsection as a generated include, so the
mapping resolves even in a repository whose carried `AGENTS.md` is
stale.
- **Mechanical guard.** A `model-tier-guard` PreToolUse hook on
`Agent|Workflow` refuses a dispatch at the maintainer-only tier's model
unless the session was launched with `FLEET_ALLOW_HIGHEST=1`. The
installer deploys it with the mapping and verifies its registration. It
is requirement 10 of the agent-safety spec, and the host safety block
states the same restriction.
## Verification
- `spec/validate.py` passes, and `tests/test_spec_validate.py` covers
the table check (171 tests).
- The agent-safety kit's tests pass: 181 passed, 1 skipped, including 15
new subprocess cases for the hook.
- `test_cli_usage` passes (16), `build_dist.py --check` is current, and
ruff and mypy are clean.
- The prose gate's default checks are clean with comments allowed. This
PR adds code comments, so it carries the `comments` label.
## Local Strict Review
Ten recorded passes ran, the last over 6fdde9b with no finding. Every
introduced finding from the earlier passes was fixed on this branch,
except one the maintainer deferred:
- `host-setup/agent-safety/claude/install.py`: the tier guard's
registration check and its strip-then-register block follow the guard's
blocks rather than sharing a helper. Deferred to #3517. (introduced,
consolidation)
## Pre-existing findings, not filed
- `host-setup/agent-safety/README.md` near lines 6, 168, and 214, and
`host-setup/agent-safety/claude/install.py:196`: sentences over the
prose gate's optional sentence-length bound, on lines this change
touched only incidentally. (pre-existing, style)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…oks (#3518)
## Summary
`host-setup/agent-safety/claude/install.py` registered three hooks:
gh-write-guard and model-tier-guard under PreToolUse, and the
stray-process sweep under SessionEnd. It carried a separate copy of the
registration logic for each, so a later fix to that logic had to be made
in every copy.
- `register_hook` is a new module-level helper. It strips every entry
naming a hook from an event's groups, then registers that hook once in
the group a selector picks, or in a new group carrying the given
matcher. `main` now calls it for all three hooks.
- `registration_problems` now checks both PreToolUse hooks through one
nested `check_pre_tool_hook`, which takes the hook's name, the tools its
matcher must see, and the wording of its messages.
This is a pure refactor. Every message string, its order, and its
deduplication are unchanged, and so is every settings.json shape the
installer writes. That includes the SessionEnd group, which is still
created with no `matcher` key.
The `comments` label is set because the counting comment moved into the
shared helper (re-indented and reworded for both hooks), and the
tool-coverage comment now covers one tool or several.
## Verification
- `python3 -m unittest discover -s host-setup/agent-safety/claude -p
'test_*.py'`: 184 tests, OK
- `uvx ruff@latest check` and `format --check` on
`host-setup/agent-safety/claude`: clean
- `uvx mypy host-setup/agent-safety/claude/install.py`: no issues
- Local strict review (frontier tier), two passes. The first raised one
wording finding on the moved comment, fixed in 208d4a7, and the second
pass raised none.
## Pre-existing findings, not filed
- `host-setup/agent-safety/claude/install.py`, the SessionEnd check loop
in `registration_problems`, design. It still hand-rolls the count loop
that `check_pre_tool_hook` now shares. It is left out because #3517
scopes the shared check to the two PreToolUse loops, and the SessionEnd
loop differs in substance: it checks the timeout, and its matcher note
does not exclude the entry from the count.
Closes#3517
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
`registration_problems` in `host-setup/agent-safety/claude/install.py`
walked an event's groups for a hook's entries in two copies,
`check_pre_tool_hook` and the SessionEnd sweep loop, so each fix to the
walk had to land twice.
- One `check_hook` now owns the walk. It picks out each entry
`names_hook` matches, reports one that does not run the deployed file,
counts named entries apart from sound ones, and closes with the "not
registered" and "registered N times" lines.
- Each caller passes a `judge(group, hook)` that reports the event's own
notes and returns whether the entry counts. For PreToolUse a blind
matcher excludes the entry. For SessionEnd a too-short timeout excludes
it, and a narrowed matcher is noted while still counting.
- Every message, its order, and its `report` or `out.append` choice are
unchanged. A randomized differential run of the old and new function
over 20000 generated settings files gave identical output.
- A new test pins that a sweep under a narrowed matcher still counts
toward the duplicate line.
The `comments` label is on this PR because moving the existing comments
into the judge functions re-indents them, which the prose gate reads as
added lines. No comment text is new.
Local strict review: one pass at the `frontier` tier, no findings.
Closes#3519
🤖 Generated with [Claude Code](https://claude.com/claude-code)
This pull request promotes centralized model-tier configuration, a Claude Code model-dispatch guard, and shared agent-safety installer helpers to main.
Changes:
Adds and validates spec/model-tiers.json, synchronizing it with AGENTS.md and relevant skills.
Deploys the Claude model-tier guard with documentation, tests, installer integration, and CI coverage.
Consolidates hook registration and registration-check logic across agent-safety hooks.
Updates generated skills, digests, and supporting documentation.
Review findings:
host-setup/agent-safety/claude/install.py:436 — P1 / critical, 1 vote: Convert absolute payload paths to repository-relative POSIX paths before the dirty check so local edits cannot be incorrectly stamped clean.
host-setup/agent-safety/claude/model-tier-guard.py:8,99 — P2 / critical, 2 votes: Update release documentation to require changes to both the deployed mapping and AGENTS.md.
spec/validate.py:733 — P2 / moderate, 2 votes: Require mapped model values to be non-empty strings and add validator coverage for invalid values.
tests/test_spec_validate.py:393 — P2 / nit, 1 vote: Replace the unexplained # type: ignore with a narrowed or typed fixture.
File
Description
tests/test_spec_validate.py
Tests model-tier configuration and table validation.
tests/test_cli_usage.py
Classifies the new hook and test entry points.
spec/validate.py
Validates model-tier configuration and its AGENTS.md table.
spec/model-tiers.json
Defines the tier-to-model mapping.
host-setup/agent-safety/README.md
Documents requirement 10 and the model-tier guard.
❌ Patch coverage is 88.88889% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.58%. Comparing base (3dbbe67) to head (f0c1d25). ⚠️ Report is 430 commits behind head on main.
…e Edits (#3522)
## Summary
This answers the review findings on the promotion pull request
[#3521](#3521):
- `spec/validate.py` rejects a tier model value that is not a non-empty
string. The deployed guard's `load_tiers()` reads such a value as an
unreadable mapping and then refuses every named model.
- The guard's docstring names both edits a model release makes, the
hub's `spec/model-tiers.json` and its `AGENTS.md` tier table, rather
than the mapping alone.
- The tier tests hold the `models` mapping as a typed fixture of their
own, which drops an unexplained `# type: ignore[index]`. A new test
covers a `null`, numeric, and empty model value.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
#3523)
## Summary
This answers the second round of review findings on the promotion pull
request [#3521](#3521),
both on the model-tier guard. The code is unchanged. The docs stated
more than it does.
- **The allowance and a missing mapping.** Requirement 10, the Claude
kit README, and the guard's docstring each said an unreadable tier
mapping refuses every dispatch naming a model. `classify()` returns
allow first when `FLEET_ALLOW_HIGHEST=1`. That is correct, since a
session launched with the allowance blocks no model and so needs no
mapping. Each now says so.
- **A nested session an agent launches.** The guard bounds a good-faith
agent's mistake rather than an adversary, so it does not tell such a
session apart, per `GOVERNANCE.md` "Trust Boundaries and Hardening
Effort". The docs now name that bound. They no longer claim an agent
cannot set the allowance for itself, only that a tool call cannot change
its own session's launch environment. Requirement 10 states the agent's
side as a conduct rule. Requirement 3 and the Claude kit README make the
same narrowing for `GH_WRITE_GUARD_ALLOW`.
## Pre-existing findings, not filed
- `host-setup/agent-safety/claude/gh-write-guard.py:1057` and
`.agents/skills/repo-worktree/SKILL.md:155` make the same "an agent
cannot set it for itself" claim about `GH_WRITE_GUARD_ALLOW`, in files
this change does not touch. (pre-existing, wording)
## Dropped findings
- `host-setup/agent-safety/README.md:35` and
`host-setup/agent-safety/claude/README.md:15` carry sentences over 25
words that this change did not write. (style)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
[P2] Reject a maintainer-only model alias that is shared with another tier. The validator currently accepts highest and frontier mapping to the same model, but load_tiers() then gives classify() one blocked model string and every default-tier dispatch using that alias is denied. Require the maintainer-only model to be distinct from the other tier values and cover that invariant in the validator tests.
[P2] Reject a maintainer-only model alias that is shared with another tier. The validator currently accepts highest and frontier mapping to the same model, but load_tiers() then gives classify() one blocked model string and every default-tier dispatch using that alias is denied. Require the maintainer-only model to be distinct from the other tier values and cover that invariant in the validator tests.
Fixed in #3524 (08e3e4a) (spec/validate.py:743). model_tier_errors now rejects a tool mapping where another tier names the maintainer-only model, compared ignoring case as the guard compares. test_rejects_a_tier_sharing_the_maintainer_only_model covers frontier mapped to Fable. This lands on develop and reaches this promotion through it.
…ce as Conduct (#3524)
## Summary
This answers the third round of review findings on the promotion pull
request [#3521](#3521):
- **A tier sharing the maintainer-only model.** `spec/validate.py` now
rejects a tool mapping where another tier names the maintainer-only
model. The comparison ignores case, as the guard's own comparison does.
Without it, the guard would block that name and refuse every dispatch at
the other tier. A new test covers `frontier` mapped to `Fable`.
- **Requirement 10's allowance sentence.** It was read twice as a claim
about what an agent cannot do, against the nested-session bound the same
requirement names. It now says that setting the allowance is the
maintainer's act. It also says that an agent never doing it is conduct
the hook does not enforce.
## Local review, declined
- `spec/validate.py` model_tier_errors: the check matches the guard's
`Agent` path, an equality ignoring case. The guard's `Workflow` path
matches the blocked alias as a word inside a script's model value, so a
lower tier mapped to a variant such as `opus[1m]` beside a reserved
`opus` would pass here. Two things rule it out under `GOVERNANCE.md`
"Trust Boundaries and Hardening Effort". The config is trusted hub
content, and the failure would be a loud refusal with a stated reason,
not a silent one. Mirroring the regex would also put a second copy of
the guard's matching rule in the validator.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
[P2] Make the model-id boundaries case-insensitive. The alias match is case-insensitive, but the adjacent boundary classes only contain lowercase letters, so a non-blocked value such as model: 'fableX' (or Xfable) matches fable because the uppercase adjacent character is treated as a boundary and the workflow is denied. Use an ASCII case-independent alphanumeric boundary for both lookarounds so only a standalone alias or an actual separated model id is matched.
[P2] Make the model-id boundaries case-insensitive. The alias match is case-insensitive, but the adjacent boundary classes only contain lowercase letters, so a non-blocked value such as model: 'fableX' (or Xfable) matches fable because the uppercase adjacent character is treated as a boundary and the workflow is denied. Use an ASCII case-independent alphanumeric boundary for both lookarounds so only a standalone alias or an actual separated model id is matched.
Fixed in #3525 (9925cdc) (host-setup/agent-safety/claude/model-tier-guard.py:88). Both lookarounds are now [A-Za-z0-9]. The tests allow fableX, Xfable, and FABLEX, and they deny claude-Fable-5-1. This lands on develop and reaches this promotion through it.
## Summary
This answers a fourth-round review finding on the promotion pull request
[#3521](#3521). The
model-tier guard's `names_model()` matched the blocked alias inside a
workflow script's model value ignoring case, but its boundaries excluded
only lowercase letters. So `fableX` or `Xfable` matched `fable`, and a
workflow naming either was denied. Both boundaries now exclude any ASCII
letter or digit.
The tests now deny a mixed-case `claude-Fable-5-1`, which holds the
case-insensitive half of the match. They also allow `fableX`, `Xfable`,
and `FABLEX`, each of which the old boundary denied.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
One moderate test issue and three documentation/runbook nits remain unresolved.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Add new CI guard commands to OPERATIONS.md
.github/actions/validate/action.yml:38
[P2] Update the local verification runbook when adding these CI checks. OPERATIONS.md is the repository's documented mirror of this action's self-test block, but its command list still omits both the new model-tier-guard.py --selftest and test_model_tier_guard.py invocations. A maintainer following the prescribed local gates will therefore skip the new guard suite even though CI runs it.
[P2] Update the local verification runbook when adding these CI checks. OPERATIONS.md is the repository's documented mirror of this action's self-test block, but its command list still omits both the new model-tier-guard.py --selftest and test_model_tier_guard.py invocations. A maintainer following the prescribed local gates will therefore skip the new guard suite even though CI runs it.
Fixed in #3526 (7b8a426) (.github/actions/validate/action.yml:38). OPERATIONS.md now carries both lines in the action's order. This lands on develop and reaches this promotion through it.
## Summary
This answers a fifth-round review finding on the promotion pull request
[#3521](#3521).
`OPERATIONS.md`'s local command block mirrors the validate action's
self-test step, but it lacked the two lines that step gained for the
model-tier guard, `model-tier-guard.py --selftest` and
`test_model_tier_guard.py`. The block now carries both, in the action's
order.
Neither line behaves differently locally than in CI, so the runbook's
prose about local-only differences needs no change.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Two unresolved moderate validation findings and three documentation nits remain.
No open issues 🎉
2 previously missed issues
In code that hasn't changed since last review
Reject duplicate tier names in model mappings
spec/validate.py:710
[P2] Reject duplicate tier names. set(models) == set(tiers) still passes a config such as ['highest', 'frontier', 'frontier'], and the renderer then emits two identical table columns even though a tier name should identify one column and one model. Add a distinctness check for tiers and a regression test so malformed tier mappings cannot pass the spec gate.
Update installer documentation to include the third hook
host-setup/agent-safety/claude/install.py:61
[P2] Update the installer documentation for the added hook. The module docstring still says the installer registers two hooks, while this list now deploys and registers a third model-tier-guard.py hook, so the module-level installation description is stale and can mislead operators about what the kit installs. Update that description to include the model-tier hook (and the copied mapping data).
[P2] Reject duplicate tier names. set(models) == set(tiers) still passes a config such as ['highest', 'frontier', 'frontier'], and the renderer then emits two identical table columns even though a tier name should identify one column and one model. Add a distinctness check for tiers and a regression test so malformed tier mappings cannot pass the spec gate.
[P2] Update the installer documentation for the added hook. The module docstring still says the installer registers two hooks, while this list now deploys and registers a third model-tier-guard.py hook, so the module-level installation description is stale and can mislead operators about what the kit installs. Update that description to include the model-tier hook (and the copied mapping data).
Both fixed in #3527 (b2bcafc) (spec/validate.py:710, host-setup/agent-safety/claude/install.py:61). model_tier_errors reports a repeated tier name, and test_rejects_a_repeated_tier_name covers it. The installer docstring names both PreToolUse hooks, the tier mapping, and the three registered hooks. A sweep of the same file also corrected the --report and refusal messages that named a single hook. This lands on develop and reaches this promotion through it.
…rs (#3527)
## Summary
This answers the sixth round of review findings on the promotion pull
request [#3521](#3521):
- **A repeated tier name.** `model_tier_errors` compared each tool's
models against `tiers` as sets, so a `tiers` list that repeats a name
passed. It now reports "tiers must not repeat a name", and a test covers
it.
- **The installer's account of its hooks.** The module docstring named
one PreToolUse hook and two registered hooks. It now names both
PreToolUse hooks, the model-tier guard's mapping, and the three hooks it
registers. A class sweep of the same file found more wording of this
kind, and this fixes it:
- The `--report` check now labels the write guard as such, rather than
as "the PreToolUse hook" or a bare "guard".
- The settings-file messages now say "the hooks" where several hooks go
unregistered.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
[P2] Require the Claude Code mapping consumed by the deployed hook. load_tiers() hard-codes tools["claude-code"], but this validator accepts an empty tools object or a config containing only another tool if AGENTS.md is updated to match. The gate can therefore pass a config that makes every model-naming Agent/Workflow dispatch fail closed as "mapping missing". Reject a missing claude-code entry and cover that case in the validator tests.
[P2] Require the Claude Code mapping consumed by the deployed hook. load_tiers() hard-codes tools["claude-code"], but this validator accepts an empty tools object or a config containing only another tool if AGENTS.md is updated to match. The gate can therefore pass a config that makes every model-naming Agent/Workflow dispatch fail closed as "mapping missing". Reject a missing claude-code entry and cover that case in the validator tests.
Declined, per GOVERNANCE.md "Trust Boundaries and Hardening Effort" (spec/validate.py:729). That section asks this code to guard a realistic mistake only where its failure would be silent or destructive. The finding states the failure itself: every model-naming dispatch fails closed as "mapping missing", and the deny text tells the session to re-run the installer. That is loud, and it blocks rather than leaks, which is the direction the guard was built to fail in. Reaching it also takes two deliberate edits, removing the claude-code entry from spec/model-tiers.json and its row from the AGENTS.md table, since the spec gate holds the two equal. So it is not a stray mistake either. claude-code is the only tool entry today.
Merge Gate item 2, promotion case, at head f0c1d25ebb97131e198a7d3cd48eb6369966d11c. pr_review.py reads coverage=none on this head, because the latest Copilot rounds carried no file table and the first table, on 487ed744, predates OPERATIONS.md joining the diff. Every change this promotion carries had full coverage on its own feature pull request into develop, at its merged head:
git log origin/main..f0c1d25e lists exactly those nine squash commits. On this head, every review thread is resolved, each finding Copilot raised without a thread is answered in this conversation, and 13 of 13 checks pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This promotes three changes to
main:spec/model-tiers.json, carries the "Model Tiers" rule into the skills that dispatch, and adds a guard that refuses a highest-tier dispatch the maintainer did not allow.It also carries the fixes for this promotion's own review findings, each landed on
developfirst:Closes #3517
Closes #3519
🤖 Generated with Claude Code