Carry the Permission Rules the Review Loop Needs in the Installer

host-setup/agent-safety/README.md

@@ -10,6 +10,7 @@ Into `~/.claude/` (or `%USERPROFILE%\.claude\` on Windows):
 - **Two marker-delimited blocks of host-wide content in `CLAUDE.md`**, loaded into every session on the machine (including ad-hoc work outside any project), installed and updated independently so one can change without rewriting the other.
   - The `agent-safety` block carries restrictions alone, so nothing in it can widen a permission. `Repository Boundaries and Write Safety` mirrors the committed `GOVERNANCE.md` section of the same name, carrying its write rules and its rule that a task works in its own checkout as behavioral guidance. `Data in Agent-Authored Text` mirrors `GOVERNANCE.md` "Representative Data in Agent-Authored Text", so an agent illustrates with constructed data rather than data observed on the machine. `Authorization Scope and Memory Hygiene` bounds how a granted permission is recorded and read. Each mirrors a rule that otherwise reaches fleet repos only.
   - The `fleet-bootstrap` block carries `Fleet Governance Entry Point`, which names the template repository and routes by the state a repository is actually in. It is separate precisely because it enables rather than restricts, and the safety block's own text says nothing in it widens a permission, so merging the two would contradict that. It is host-wide rather than per repository because the repositories that most need it are the ones carrying no instruction set to point the way, and it mirrors the byte-locked `AGENTS.md` "Fleet Bootstrap" section that a conformant repository carries.
+- **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.
 
 The hook is the mechanical backstop. The CLAUDE.md rules and the carried GOVERNANCE.md rules are the behavioral layer. Prose alone is not enough, since the incident happened under prose rules, so both ship. The GitHub write rules have a hook behind them, as do the git operations that bypass a repository protection, while which checkout a command belongs in, the data a line of text quotes, and how an authorization is recorded are prose only, since no hook can see any of the three.
 
@@ -25,7 +26,7 @@ host-setup/agent-safety/install.sh
 .\host-setup\agent-safety\install.ps1
 ```
 
-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 entry 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.
+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.
 
 **Restart Claude Code sessions on the machine afterward** so the new hook and CLAUDE.md load.
 
@@ -39,6 +40,7 @@ The deployed copy on each machine is a snapshot, so when the guard changes upstr
 python3 ~/.claude/hooks/gh-write-guard.py --selftest    # decision matrix: all cases pass
 grep -c 'agent-safety v' ~/.claude/CLAUDE.md            # expect 2 (start + end marker)
 grep -c 'fleet-bootstrap v' ~/.claude/CLAUDE.md         # expect 2 (start + end marker)
+grep -cF 'Bash(python3 scripts/pr_review.py:*)' ~/.claude/settings.json   # expect 1 (never duplicated)
 ```
 
 On Windows PowerShell:
@@ -47,6 +49,7 @@ On Windows PowerShell:
 py -3 "$env:USERPROFILE\.claude\hooks\gh-write-guard.py" --selftest    # all cases pass
 (Select-String 'agent-safety v' "$env:USERPROFILE\.claude\CLAUDE.md").Count      # expect 2
 (Select-String 'fleet-bootstrap v' "$env:USERPROFILE\.claude\CLAUDE.md").Count   # expect 2
+(Select-String -SimpleMatch 'Bash(python3 scripts/pr_review.py:*)' "$env:USERPROFILE\.claude\settings.json").Count   # expect 1
 ```
 
 Live end-to-end (in any repo): attempt a discarded-output write and confirm the Bash tool is blocked:
@@ -65,10 +68,15 @@ The installer writes this. It is here so you can inspect or hand-place it:
     "PreToolUse": [
       { "matcher": "Bash", "hooks": [ { "type": "command", "command": "\"python3\" \"<home>/.claude/hooks/gh-write-guard.py\"" } ] }
     ]
+  },
+  "permissions": {
+    "allow": [ "Bash(python3 scripts/pr_review.py:*)" ]
   }
 }
 ```
 
+Every other key in the file is left as it stands, `permissions.allow` included, apart from the rules whose prefix this kit owns.
+
 ## Scope and Limits
 
 - **Per-machine.** `~/.claude/` does not travel, so run the installer on each box. This is the rollout that [#365][issue-365] tracks.

host-setup/agent-safety/install.py

@@ -1,8 +1,9 @@
 #!/usr/bin/env python3
 """Install the agent write-safety kit for the current user account. Cross-platform, idempotent.
 
-Deploys the PreToolUse hook, registers it in the user settings.json, adds the safety rules to the user
-CLAUDE.md (marker-delimited so re-runs update in place), and self-tests the hook before registering it.
+Deploys the PreToolUse hook, registers it in the user settings.json, merges the permission rules this
+kit owns into the same file, adds the safety rules to the user CLAUDE.md (marker-delimited so re-runs
+update in place), and self-tests the hook before registering it.
 The bash and PowerShell wrappers both call this, so every OS runs one tested code path.
 
 Usage: python3 install.py            (installs to ~/.claude)
@@ -18,6 +19,32 @@
 
 HERE = pathlib.Path(__file__).resolve().parent
 
+# 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()
+
+
+def at(data, path):
+    """The value at a slash-separated key path, or MISSING where any step of it is absent."""
+    node = data
+    for part in path.split("/"):
+        if not isinstance(node, dict) or part not in node:
+            return MISSING
+        node = node[part]
+    return node
+
+# Permission rules this kit installs, each as (owned prefix, rule).
+# A re-run drops every rule under the prefix before adding the current one, so a changed rule updates in place.
+# The prefix bounds what the installer owns, so a rule written by hand outside it is never touched.
+# These widen rather than restrict, so they stay their own step for the reason the two CLAUDE.md blocks stay separate.
+MANAGED_PERMISSIONS = [
+    # 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:*)"),
+]
+
 
 def hook_launcher():
     """A python invocation for the settings.json command. Prefer a bare `python3` (portable and
@@ -70,6 +97,45 @@ def main():
                 f"{settings} exists but is not valid JSON ({e}). Fix or remove it, then re-run.\n"
             )
             return 1
+    # A settings file is an object, and any other JSON value parses cleanly and breaks every lookup below.
+    # The root is therefore checked before the keys under it are.
+    if not isinstance(data, dict):
+        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"
+        )
+        return 1
+    # 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.
+    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"
+        )
+
+    for path, want in (("hooks", dict), ("hooks/PreToolUse", list),
+                       ("permissions", dict), ("permissions/allow", list)):
+        held = at(data, path)
+        # An explicit null is present rather than absent, and `setdefault` hands back the null it found.
+        # It is therefore rejected here rather than read as a gap the default fills.
+        if held is not MISSING and not isinstance(held, want):
+            reject(path.replace("/", "."), held, want)
+            return 1
+
+    # A list of the right type can still hold the wrong elements.
+    # The registration below reads each group as an object, and each group's `hooks` as a list it appends to.
+    groups = at(data, "hooks/PreToolUse")
+    if groups is not MISSING:
+        for i, g in enumerate(groups):
+            if not isinstance(g, dict):
+                reject(f"hooks.PreToolUse[{i}]", g, dict)
+                return 1
+            if "hooks" in g and not isinstance(g["hooks"], list):
+                reject(f"hooks.PreToolUse[{i}].hooks", g["hooks"], list)
+                return 1
+
     pre = data.setdefault("hooks", {}).setdefault("PreToolUse", [])
     # Strip the hook from every existing group first, so a re-run leaves no duplicate behind.
     # That matters when settings.json already carries more than one Bash group.
@@ -83,10 +149,34 @@ def main():
         group = {"matcher": "Bash", "hooks": []}
         pre.append(group)
     group.setdefault("hooks", []).append({"type": "command", "command": hook_cmd})
+    done = ["PreToolUse/Bash hook registered"]
+
+    # 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", [])
+    for prefix, rule in MANAGED_PERMISSIONS:
+        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]
+        if not matched:
+            action = "added"
+        elif not older:
+            action = "already current"
+        else:
+            action = f"updated, superseding {len(older)}"
+        done.append(f"permission {rule}: {action}")
+
+    # Reported after the write rather than as each edit is made, since both edits share one write.
+    # A line printed before it claims a change that a later failure would leave unmade.
     settings.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8")
-    print(f"  settings -> {settings} (PreToolUse/Bash hook registered)")
+    for line in done:
+        print(f"  settings -> {settings} ({line})")
 
-    # 3. CLAUDE.md carries one marker block per snippet, replaced where present and appended where not.
+    # 4. CLAUDE.md carries one marker block per snippet, replaced where present and appended where not.
     # The two blocks install and update independently, so one can change without rewriting the other.
     # The safety block states restrictions only.
     # The fleet block enables, so it stays separate from a block whose own text says nothing in it widens a permission.
@@ -113,6 +203,10 @@ def main():
     print(f"  {launcher} \"{hook_dst}\" --selftest")
     print(f"  grep -c 'agent-safety v' \"{claude_md}\"      # expect 2")
     print(f"  grep -c 'fleet-bootstrap v' \"{claude_md}\"   # expect 2")
+    # One line per rule, matching the rule itself rather than a word inside it.
+    # A hint naming a fixed word would stop matching the moment a rule that lacks it is added.
+    for _, rule in MANAGED_PERMISSIONS:
+        print(f"  grep -cF '{rule}' \"{settings}\"   # expect 1")
     print("Restart Claude Code sessions on this machine so the hook and CLAUDE.md load.")
     return 0