Carry the Permission Rules the Review Loop Needs in the Installer

[ptr727]

The kit configured settings.json for the hook alone, so a permission rule added on one machine reached none of the others. The installer now carries the rules it owns, which makes re-running it the way those rules reach every host.

What changed

install.py gains a MANAGED_PERMISSIONS list, each entry a prefix and a rule, merged in the same pass that registers the hook. The settings file is still read once and written once, so the hook and the rules land together or not at all.

Step	Before	After
1	self-test the hook	unchanged
2	register the PreToolUse hook	unchanged
3	write the CLAUDE.md blocks	merge permissions.allow
4		write the CLAUDE.md blocks

Why a prefix rather than a plain append

A re-run drops every rule under the prefix before adding the current one, which is the same strip-then-register the hook registration already uses. A rule whose spelling changes therefore updates in place rather than accumulating beside the version it replaced, and the prefix bounds what the installer owns so a rule written by hand outside it is never touched.

The one rule it carries today

Bash(python3 scripts/pr_review.py:*), for the review loop's reply and resolve. Driving that loop by hand needs a raw GraphQL mutation, which is the shape that reached a stranger's repository. scripts/pr_review.py queries the thread id itself, takes no argument an id fits in, and refuses to resolve a thread whose reply did not land, so allowing the script is narrower than allowing the mutation it replaces.

These widen rather than restrict, so they are their own component for the same reason the fleet-bootstrap block is separate from the agent-safety one: the safety half's own text says nothing in it widens a permission. Keeping a grant in its own step preserves that, where folding it into the hook registration would not.

Verification, run against throwaway CLAUDE_HOME trees

Case	Result
Fresh install, no settings.json at all	added, list holds the one rule
Re-run, unchanged	already current, no duplicate rule, hook count still 1
One older spelling present	updated, superseding 1
Two older spellings present	updated, superseding 2, collapsed to one rule
Unrelated rules and other keys	Bash(git -C * log *), additionalDirectories and effortLevel all preserved
settings.json is not valid JSON	exits 1, names the file, writes nothing

Gates on this branch: prose gate 0 whole-tree, gh-write-guard.py --selftest PASS, repo_gate.py clean, editorconfig-checker exit 0, markdownlint-cli2 0 issues.

The prose gate caught two wrapped comment sentences in the new code before the first push, which is the gate promoted to blocking earlier today doing the job it was promoted for.

What this does not do

It installs nothing anywhere. Running it on each host stays the maintainer's action, tracked by the per-machine rollout in #365.

[Copilot]

Pull request overview

This PR updates the per-machine agent safety installer to also manage the Claude Code settings.json permission grants required by the automated PR review loop, ensuring re-running the installer consistently propagates those permissions across hosts.

Changes:

• Add a managed permissions list to host-setup/agent-safety/install.py and merge permissions.allow entries during install/update.
• Document the new managed permission behavior and add verification steps in host-setup/agent-safety/README.md.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
host-setup/agent-safety/README.md	Documents the new managed permissions.allow behavior and adds verification commands/snippet updates.
host-setup/agent-safety/install.py	Implements prefix-managed permissions.allow merging alongside existing hook registration and single-pass settings writes.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

host-setup/agent-safety/install.py:90

• The settings.json type validation checks nested containers, but still assumes the JSON root is a dict. If settings.json contains a valid non-object JSON value (e.g., a list), the later setdefault calls will raise an AttributeError and produce a traceback instead of the intended "nothing was written" error path.

    # Every container this installer descends into is checked before it is used.
    # A key holding an unexpected type would otherwise raise a traceback mid-edit.
    # That reads as a crash rather than as the settings problem it is.
    # The invalid-JSON refusal above is the shape this file already answers a malformed file with.
    for path, want in (("hooks", dict), ("hooks/PreToolUse", list),

host-setup/agent-safety/install.py:168

• The final verification hint hardcodes grep -c 'pr_review.py' but uses len(MANAGED_PERMISSIONS) for the expected count. If MANAGED_PERMISSIONS ever adds a rule that doesn't include pr_review.py, the printed command and expected count will diverge.

    print(f"  grep -c 'agent-safety v' \"{claude_md}\"      # expect 2")
    print(f"  grep -c 'fleet-bootstrap v' \"{claude_md}\"   # expect 2")
    print(f"  grep -c 'pr_review.py' \"{settings}\"         # expect {len(MANAGED_PERMISSIONS)}")

host-setup/agent-safety/install.py:133

• The "updated, superseding N" status counts every matched entry, including the current canonical rule when it is already present. That can over-report how many older spellings were superseded (e.g., when the rule is present plus one older spelling, it prints "superseding 2").

        superseded = [a for a in allow if isinstance(a, str) and a.startswith(prefix)]
        allow[:] = [a for a in allow if a not in superseded] + [rule]
        if superseded == [rule]:
            action = "already current"
        elif superseded:

[ptr727]

Round 2, three suppressed findings accepted and fixed in 645c99f

All three sat in a <details> block, so they reach no thread and are answered here. Each was reproduced before being fixed rather than taken on its description.

install.py:90, the JSON root, accepted. The key checks descend from a root nobody checked, so a valid non-object settings file crashed exactly where the check was meant to catch it:

[1,2,3]     -> AttributeError: 'list' object has no attribute 'get'

Four root shapes now refuse cleanly, and the file is byte-identical afterwards in each:

Root	Result
[1,2,3]	holds list at its root where an object is required
"a string"	holds str
42	holds int
null	holds NoneType

install.py:133, the superseded count, accepted. The current rule matches its own prefix, so counting the match set reported it as one of the rules it replaced. Counted over the rules actually replaced now:

permissions.allow before	Was	Now
current rule plus one older spelling	superseding 2	superseding 1
current rule alone	already current	already current
two older spellings, no current	superseding 2	superseding 2
empty	added	added

This is the same class of defect as the two earlier rounds on the other pull request, a count or a claim broader than what was measured, which is worth naming rather than quietly correcting.

install.py:168, the verification hint, accepted. It grepped a fixed word while counting the managed list, and those agree only while every rule happens to carry that word. It now prints one exact-match line per rule, derived from the rule itself:

grep -cF 'Bash(python3 scripts/pr_review.py:*)' "<home>/.claude/settings.json"   # expect 1

That printed command was run against a real install rather than assumed, and returns 1. The README's POSIX and PowerShell verify blocks were updated to the same exact-match form.

Verification on 645c99f

Check	Result
Prose gate, whole tree	0 violations across 0 files
gh-write-guard.py --selftest	PASS
repo_gate.py	eol 0, sha-pin 0
editorconfig-checker	exit 0
markdownlint-cli2 on the kit README	0 issues
Fresh install, re-run, supersede, unrelated rules preserved	all as before

One aside worth recording. The write guard this kit installs blocked a command of mine during this round, because a >/dev/null on an unrelated install test sat in the same command string as a git push. The guard reads the command rather than the pipeline, so that is the rule working as written, and the remedy was to issue the push as its own command, which the kit's README already asks for.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

host-setup/agent-safety/install.py:105

• The type-check loop treats JSON null the same as a missing key (node.get(...) returns None in both cases). If a user has e.g. "hooks": null or "permissions": null, this validation will pass but setdefault(...) will later raise an AttributeError. Track key presence separately so an explicit null is rejected with the same clear error path.

        node = data
        for p in parts[:-1]:
            node = node.get(p, {})
        held = node.get(parts[-1]) if isinstance(node, dict) else None
        if held is not None and not isinstance(held, want):

host-setup/agent-safety/install.py:112

• hooks.PreToolUse is only validated as a list, but later code assumes each element is an object and calls g.get(...). A user settings file like { "hooks": { "PreToolUse": ["oops"] } } will still crash with a traceback. Add a preflight check to fail with a clear error (and no write) when any PreToolUse entry is not an object.

This issue also appears on line 101 of the same file.

    # Every container this installer descends into is checked before it is used.
    # A key holding an unexpected type would otherwise raise a traceback mid-edit.
    # That reads as a crash rather than as the settings problem it is.
    # The invalid-JSON refusal above is the shape this file already answers a malformed file with.
    for path, want in (("hooks", dict), ("hooks/PreToolUse", list),
                       ("permissions", dict), ("permissions/allow", list)):

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

host-setup/agent-safety/install.py:137

• The new settings.json type validation checks that hooks.PreToolUse is a list and that each group is a dict, but it doesn't validate the elements of each group's hooks list. If a user has e.g. {"hooks":{"PreToolUse":[{"matcher":"Bash","hooks":["oops"]}]}}, the later strip step calls h.get(...) and will still crash with an AttributeError/traceback. Consider validating that every hooks.PreToolUse[i].hooks[j] is a dict (and failing with the same clear "Nothing was written" message) before proceeding.

            if "hooks" in g and not isinstance(g["hooks"], list):
                reject(f"hooks.PreToolUse[{i}].hooks", g["hooks"], list)
                return 1

[ptr727]

Round 4, the suppressed finding accepted and fixed in bdf73b4

Correct, and reproduced first. The strip step reads each entry of a group's hooks list as an object, and a list holding a string crashed it:

{"hooks":{"PreToolUse":[{"matcher":"Bash","hooks":["oops"]}]}}
  -> AttributeError: 'str' object has no attribute 'get'

Enumerated rather than patched a fourth time

Four rounds have now each found this class one level deeper than the last, so the paths were listed rather than waiting for a fifth. The registration and the permission merge touch exactly these, and every one is now either type-checked or read through a guard that cannot raise:

Path	How it is safe
the settings root	checked as an object
hooks	checked
hooks.PreToolUse	checked
hooks.PreToolUse[i]	checked
hooks.PreToolUse[i].hooks	checked
hooks.PreToolUse[i].hooks[j]	checked, this round
hooks.PreToolUse[i].hooks[j].command	read through str(...), so any type is searchable
permissions	checked
permissions.allow	checked
permissions.allow[k]	skipped by an isinstance test where it is not a string, and preserved

There is no deeper level to reach, so this class is closed rather than one round further along.

Twelve malformed shapes, each refused with the file byte-identical

settings.json	Message
[1,2,3]	holds list at its root where an object is required
{"hooks": null}	hooks as NoneType where dict is required
{"hooks": "off"}	hooks as str
{"hooks": {"PreToolUse": {}}}	hooks.PreToolUse as dict where list is required
{"hooks": {"PreToolUse": ["oops"]}}	hooks.PreToolUse[0] as str where dict is required
{"hooks": {"PreToolUse": [{"hooks": "x"}]}}	hooks.PreToolUse[0].hooks as str
{"hooks": {"PreToolUse": [{"hooks": null}]}}	hooks.PreToolUse[0].hooks as NoneType
{"hooks": {"PreToolUse": [{"hooks": ["oops"]}]}}	hooks.PreToolUse[0].hooks[0] as str where dict is required
{"permissions": null}	permissions as NoneType
{"permissions": "none"}	permissions as str
{"permissions": {"allow": null}}	permissions.allow as NoneType
{"permissions": {"allow": "all"}}	permissions.allow as str

Legal but unusual values still install

A settings file carrying a numeric command and non-string entries in allow installs normally, and those values come out exactly as they went in:

allow before: ["Bash(x)", 42, null]
allow after:  ["Bash(x)", 42, null, "Bash(python3 scripts/pr_review.py:*)"]

That matters because a validator that rejects everything unfamiliar would refuse a settings file this kit has no business judging. The rule is that a key the installer writes through is checked, and a key it merely carries is not.

Verification on bdf73b4

Check	Result
Prose gate, whole tree	0 violations across 0 files
gh-write-guard.py --selftest	PASS
repo_gate.py	eol 0, sha-pin 0
editorconfig-checker	exit 0

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

host-setup/agent-safety/README.md:13

• This paragraph claims allowing scripts/pr_review.py is narrower than allowing the raw GraphQL mutation. As written, the installed allow rule is a relative-path pattern (Bash(python3 scripts/pr_review.py:*)), which would also allow running an arbitrary scripts/pr_review.py in any other checkout; that is not necessarily narrower than allowing the specific gh api graphql mutation shape.

- **The permission rules this kit owns, merged into `settings.json`** beside the hook registration. Each is declared as a prefix and a rule, and a re-run drops every rule under the prefix before adding the current one, so a rule whose spelling changes updates in place rather than accumulating beside the version it replaced. The prefix bounds what the installer owns, so a rule written by hand outside it is never touched. These widen rather than restrict, which is why they are their own component for the same reason the `fleet-bootstrap` block is separate from the `agent-safety` one. Today the list holds one rule, for `scripts/pr_review.py`, the review loop's reply and resolve. Driving that loop by hand needs a raw GraphQL mutation, which is the shape that reached a stranger's repository, and the script queries the thread id itself and takes no argument an id fits in, so allowing the script is narrower than allowing the mutation it replaces.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (4)

host-setup/agent-safety/install.py:24

• This comment wraps a single sentence across multiple # lines. GOVERNANCE.md requires one sentence per line and forbids wrapping sentences across lines in comments.

# Distinguishes an absent key from one holding an explicit null, which `dict.get` reports alike.
# The two need different answers, since a gap is filled and a null is a settings error.
MISSING = object()

host-setup/agent-safety/install.py:45

• This comment block includes historical incident narrative ("reached a stranger's repository") and is longer than needed. The repo's comment conventions require comments to be short, state only the non-obvious why, and avoid historic/design narrative.

    # The review loop's reply and resolve, the one write in that loop an agent performs.
    # Driving it by hand needs a raw GraphQL mutation, which is the shape that reached a stranger's repository.
    # `pr_review.py` queries the thread id itself and takes no argument an id fits in.
    # Allowing the script is therefore narrower than allowing the mutation it replaces.
    ("Bash(python3 scripts/pr_review.py", "Bash(python3 scripts/pr_review.py:*)"),

host-setup/agent-safety/README.md:13

• This introduces an inline Markdown link ([text](url)) in a non-exempt doc. GOVERNANCE.md requires reference-style links in Markdown docs other than the four instruction files, so host-setup/agent-safety/README.md should avoid inline link targets.

- **The permission rules this kit owns, merged into `settings.json`** beside the hook registration. Each is declared as a prefix and a rule, and a re-run drops every rule under the prefix before adding the current one, so a rule whose spelling changes updates in place rather than accumulating beside the version it replaced. The prefix bounds what the installer owns, so a rule written by hand outside it is never touched. These widen rather than restrict, which is why they are their own component for the same reason the `fleet-bootstrap` block is separate from the `agent-safety` one. Today the list holds one rule, for `scripts/pr_review.py`, the review loop's reply and resolve. Driving that loop by hand needs a raw GraphQL mutation carrying a node id, which is the shape that reached a stranger's repository, where the script queries the id itself and takes no argument an id fits in. What the rule decides is which command runs without a prompt, and it matches the command text rather than the directory the command runs in, so it reaches a `scripts/pr_review.py` in any checkout that carries one. An absolute path would not narrow that, since the hub is reached as a checkout of the caller's own and its location differs per task, so pinning one path would name a checkout the next task does not use. What bounds it is the rule that an agent reaches the hub as a checkout of its own, fetched immediately before it is read, rather than a copy it happens to find on disk, which the `fleet-bootstrap` block beside this carries and [`GOVERNANCE.md`](../../GOVERNANCE.md) "Hub-Hosted Tooling" states in full.

host-setup/agent-safety/install.py:175

• The action message can be incorrect when permissions.allow already contains duplicate copies of the current rule. In that case the installer will still modify the list (deduplicate) but will report "already current" because older only counts non-identical matches.

        matched = [a for a in allow if isinstance(a, str) and a.startswith(prefix)]
        allow[:] = [a for a in allow if a not in matched] + [rule]
        # Counted over the rules actually replaced rather than over everything the prefix matched.
        # The current rule matches its own prefix, so counting the match set reports it as superseded.
        older = [a for a in matched if a != rule]

[ptr727]

Round 6, three of four accepted and fixed in ee9a8e0, one declined

install.py:45, accepted, and it is the round-five finding again. The claim that allowing the script is narrower than allowing the mutation was removed from the README last round and was still sitting in the comment beside the rule, because that sweep fixed the instance the finding named rather than the claim itself. The comment now says what the rule decides and that it bounds no checkout, matching the README.

That is the third time on this branch that fixing a named line left the same statement standing somewhere else, so it is worth recording rather than quietly repairing.

README.md:13, accepted. GOVERNANCE.md "Documentation Style Conventions" allows inline targets in four files only, AGENTS.md, GOVERNANCE.md, OPERATIONS.md and .github/copilot-instructions.md, and this README is none of them. It is a reference definition in the existing <!-- Repo --> group now, sorted by reference name ahead of issue-365.

install.py:175, accepted. A list holding the current rule twice was collapsed while the line reported already current, so the file changed and the output denied it. Verified across every action:

permissions.allow before	Reported	After
empty	added	one rule
the rule once	already current	unchanged
the rule twice	deduplicated, collapsing 2 copies	one rule
the rule plus one older spelling	updated, superseding 1	one rule
two older spellings	updated, superseding 2	one rule

install.py:24, declined. It reads the MISSING comment as one sentence wrapped across two lines. Each line is a complete sentence carrying its own terminator:

# Distinguishes an absent key from one holding an explicit null, which `dict.get` reports alike.
# The two need different answers, since a gap is filled and a null is a settings error.

scripts/prose_lint.py implements that rule, gates CI on it, and exits 0 over this file:

python3 scripts/prose_lint.py host-setup/agent-safety/install.py --check comment-wrap --check comment-case
0 violation(s) across 0 file(s)

The gate is the rule's definition here, so a finding it contradicts is declined rather than applied. Nothing is changed for it.

Verification on ee9a8e0

Check	Result
Prose gate, whole tree	0 violations across 0 files
gh-write-guard.py --selftest	PASS
repo_gate.py	eol 0, sha-pin 0
markdownlint-cli2 on this README	0 issues
editorconfig-checker	exit 0

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

host-setup/agent-safety/README.md:29

• This sentence says the settings file is read once, but install.py reads it twice when it exists and is non-empty (settings.read_text(...) is called for the .strip() check and again for json.loads(...)). Either cache the file contents or adjust the wording so it only promises the single write / single update pass.

Both are thin wrappers around `install.py`, so every OS runs one tested code path. The installer self-tests the hook before registering it, merges the settings.json hook entry and the permission rules without clobbering other keys, and updates each CLAUDE.md block in place by its own markers rather than duplicating it, so the two blocks move independently. The settings file is read once and written once, so the hook and the permission rules land together or not at all.

host-setup/agent-safety/install.py:161

• This comment claims settings.json is "read once and written once", but the file is currently read twice on the non-empty path (see the two settings.read_text(...) calls before parsing). To avoid misleading future maintainers, either cache the read contents or update this comment to only describe the single-write behavior.

    # 3. Permission rules, merged under the prefixes this installer owns.
    # Dropping the prefix before appending is the same strip-then-register the hook above uses.
    # That is what makes a re-run update a changed rule rather than leave both spellings in the list.
    # Written in the same pass as the hook, so the file is read once and written once.
    allow = data.setdefault("permissions", {}).setdefault("allow", [])

[ptr727]

Round 7, both suppressed findings accepted and fixed in 857f351

They are one defect reported in the two places that carried it, the comment and the kit README, and both are correct. The file was read twice on the non-empty path, once to test it for content and once to parse it, while both places said it is read once and written once. The claim is mine, from the commit that moved the write so the hook and the permission rules share one.

Fixed by making the code true rather than by weakening the sentence. The property is worth having, and the second read is not merely redundant. Two reads can disagree when another process writes between them, so the emptiness the first read measured would not be the content the second parsed. One read into a variable now serves both.

settings.read_text  occurrences: 1
settings.write_text occurrences: 1

Verified across every path into that read

settings.json	Result
absent	installs, rule added
present and empty	installs, rule added
present and whitespace only	installs, rule added
present and valid, carrying effortLevel	installs, effortLevel preserved
present and not valid JSON	exits 1, names the file, writes nothing

Verification on 857f351

Check	Result
Prose gate, whole tree	0 violations across 0 files
gh-write-guard.py --selftest	PASS
repo_gate.py	eol 0, sha-pin 0
editorconfig-checker	exit 0

Worth stating plainly, since this is the seventh round on a two-file change: every finding after the first has been in something the change itself asserted rather than in what it does. A count, a claim about scope, a claim about narrowness, and now a claim about reads. The code has been correct for several rounds and the prose around it has not, which is the failure mode this repository already names, and it is being corrected here rather than argued with.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

host-setup/agent-safety/install.py:45

• MANAGED_PERMISSIONS uses a prefix that does not include the : delimiter, so it can match and overwrite unrelated allow-rules that merely start with Bash(python3 scripts/pr_review.py (e.g., ...pr_review.py-custom:*)). Since the prefix is the boundary for what the installer “owns”, it should be as specific as possible to the intended rule namespace.

    # The review loop's reply and resolve, the one write in that loop an agent performs.
    # Driving it by hand needs a raw GraphQL mutation carrying a node id, which is the shape to avoid.
    # The rule decides which command skips a prompt, and it bounds no checkout, since it matches the text.
    ("Bash(python3 scripts/pr_review.py", "Bash(python3 scripts/pr_review.py:*)"),
]

[ptr727]

Round 8, the suppressed finding accepted and fixed in e356409

Correct, and it contradicts a property this change states rather than being a refinement of it. The README promises that a rule written by hand outside what the installer owns is never touched, and the prefix test broke that promise.

The prefix ends at the script name, so a bare startswith claimed every longer path:

Allow rule	Claimed by the old test	Actually this script
Bash(python3 scripts/pr_review.py:*)	yes	yes
Bash(python3 scripts/pr_review.py reply *)	yes	yes
Bash(python3 scripts/pr_review.py-custom:*)	yes	no
Bash(python3 scripts/pr_review.py_old:*)	yes	no
Bash(python3 scripts/pr_review.py2:*)	yes	no

A re-run would have deleted the bottom three.

Appending the colon to the prefix is not the fix. It would stop matching pr_review.py reply *, which is exactly the older spelling the supersede exists to replace, so the rule would accumulate beside its predecessor instead. What separates the two cases is the character after the name: a rule invoking this script continues with a rule-syntax delimiter (:, a space, or the closing paren), where a different script continues with more of its own path.

Verified on a list holding all four shapes at once

before: [pr_review.py reply *, pr_review.py-custom, pr_review.py_old, pr_review.py2, git -C * log *]
action: updated, superseding 1
after:  [pr_review.py-custom, pr_review.py_old, pr_review.py2, git -C * log *, pr_review.py:*]

One owned rule superseded, three unrelated ones untouched, and the unrelated git rule preserved as before.

Every action still reports correctly:

permissions.allow before	Reported
empty	added
the rule once	already current
the rule twice	deduplicated, collapsing 2 copies
two older spellings	updated, superseding 2
Bash(python3 scripts/pr_review.py), the bare-paren form	updated, superseding 1

Verification on e356409

Check	Result
Prose gate, whole tree	0 violations across 0 files
gh-write-guard.py --selftest	PASS
repo_gate.py	eol 0, sha-pin 0
markdownlint-cli2 on this README	0 issues
editorconfig-checker	exit 0

The README sentence that made the promise now states the delimiter requirement that keeps it.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

host-setup/agent-safety/install.py:192

• The "deduplicated" status message is computed from copies, which currently represents the number of occurrences of the current rule. This can read as if that many entries were removed (e.g., "collapsing 2 copies" even though one copy remains). It also doesn't report deduplication when an update + dedup happen together. Consider computing the number of duplicates removed and reflecting that in the message, optionally alongside the update message.

            action = f"deduplicated, collapsing {copies} copies"

[ptr727]

Round 9, the suppressed finding accepted and fixed in aa65b14

Both halves hold, and the second was reproduced before fixing.

The count named occurrences rather than removals. A list holding the rule twice reported collapsing 2 copies where one entry was removed and one remains.

An update and a deduplication can happen together, and only the update was named. A list holding the rule twice beside one older spelling collapsed three entries into one and reported updated, superseding 1, saying nothing about the duplicate it also removed:

before: [rule, rule, rule-reply-form]        3 entries
report: updated, superseding 1
after:  [rule]                               1 entry

The action is composed from what the write actually removes now, and both causes are named when both apply.

Every message checked against the entry count on each side

This is the test the earlier rounds of this message should have had, since a count is exactly the kind of claim that reads plausibly and measures nothing.

Before	After	Reported
0	1	added
1	1	already current
2	1	updated, removing 1 duplicate
3	1	updated, removing 2 duplicates
2	1	updated, superseding 1
2	1	updated, superseding 2
3	1	updated, superseding 1 and removing 1 duplicate
5	2	updated, superseding 2 and removing 1 duplicate

The last row also carries a rule for a different script, pr_review.py-custom, which survives, so the delimiter ownership from the previous round is exercised here too. Every row's arithmetic is checkable: removed equals before minus after, plus one for the rule that is always written.

Verification on aa65b14

Check	Result
Prose gate, whole tree	0 violations across 0 files
gh-write-guard.py --selftest	PASS
repo_gate.py	eol 0, sha-pin 0
editorconfig-checker	exit 0

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

host-setup/agent-safety/install.py:185

• The inline comments explaining how older/duplicates are computed are currently repetitive (they restate the same rationale twice) which makes this section harder to scan. Consider collapsing them into a single concise comment so the intent is clear without duplication.

        # Counted over the rules actually replaced rather than over everything the prefix matched.
        # The current rule matches its own prefix, so counting the match set reports it as superseded.
        older = [a for a in matched if a != rule]
        # Counted over what the write removes rather than over what the prefix matched.
        # The current rule matches its own prefix, so a match-set count reports it as superseded.

[ptr727]

Round 10, the suppressed finding accepted and fixed in ab6140e

Correct, and the duplicate is mine rather than a style opinion. The previous commit replaced the block starting at the older assignment, and the comment above that line was outside the replacement, so two near-identical pairs ended up around one statement. The stale pair is gone.

The sweep found the same class one step apart

Reading every comment this branch adds, rather than the two lines quoted, turned up a second instance that is not adjacent and so would not have been reported:

Location	What it said
the module constant	a re-run drops every rule the prefix owns before adding the current one, so a changed rule updates in place
step 3	dropping the prefix before appending is what makes a re-run update a changed rule rather than leave both spellings

The constant documents the data, so step 3 now keeps only what is its own: that it borrows the hook registration's shape and shares its single write.

Comments only, and confirmed as such

The action matrix was re-run against entry counts rather than assumed unchanged:

0 -> 1  added
1 -> 1  already current
2 -> 1  updated, removing 1 duplicate
3 -> 1  updated, superseding 1 and removing 1 duplicate
5 -> 2  updated, superseding 2 and removing 1 duplicate

Verification on ab6140e

Check	Result
Prose gate, whole tree	0 violations across 0 files
gh-write-guard.py --selftest	PASS
repo_gate.py	eol 0, sha-pin 0
editorconfig-checker	exit 0

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

host-setup/agent-safety/install.py:119

• The error text says "Nothing was written", but this installer has already copied/self-tested the hook before reaching this validation. Consider narrowing the claim to the settings file so the message stays accurate.

        sys.stderr.write(
            f"{settings} is valid JSON but holds {type(data).__name__} at its root where an object "
            "is required. Fix or remove it, then re-run. Nothing was written.\n"
        )

host-setup/agent-safety/install.py:129

• This validation error also says "Nothing was written", but the hook file may already have been (re)deployed before this check. Rephrase to specify that settings.json was not updated/written to avoid a misleading message.

    def reject(where, held, want):
        sys.stderr.write(
            f"{settings} has `{where}` as {type(held).__name__} where {want.__name__} is required. "
            "Fix or remove that key, then re-run. Nothing was written.\n"
        )

[ptr727]

Round 11, both suppressed findings accepted and fixed in 2680c21

One defect reported at the two lines that carried it, and correct. The refusals claimed nothing was written, where step 1 copies the hook and self-tests it before any of these checks run. The claim is mine, added with the checks.

Confirmed by listing the tree after a refusal rather than by reading the code:

settings.json has `permissions` as NoneType where dict is required. ...
on disk: hooks/gh-write-guard.py
settings.json: byte-identical
exit: 1

The hook file is there, so the message was false about the run as a whole while being true about the file it names.

What the message says now

Each refusal names the settings file as unchanged and states the state the run leaves behind:

... Fix or remove that key, then re-run. This file is unchanged, so the hook is deployed but not registered.

That second clause is the part a reader needs, since it answers whether re-running after the fix is sufficient, which a bare denial does not.

Swept rather than patched at the two quoted lines

The phrase appears four more times, all in scripts/pr_review.py, where nothing was written names a GitHub write on the OUT_OF_SCOPE, NO_MATCH and AMBIGUOUS paths. Each returns before M_REPLY is sent, so that claim is accurate and is left alone. The defect was specific to this installer.

Verification on 2680c21

Check	Result
Three refusal shapes, message and disk state	hook present, settings byte-identical, exit 1
Prose gate, whole tree	0 violations across 0 files
gh-write-guard.py --selftest	PASS
editorconfig-checker	exit 0

One correction to my earlier answers on this pull request, which I cannot edit and would not quietly rewrite: the round two and round three comments quoted the old Nothing was written message as evidence, and that phrasing was inaccurate for the same reason. What those rounds actually verified, and what still holds, is that the settings file is byte-identical after each refusal.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.