Agent write-safety: repo-scope + never-fabricate-id + never-suppress-write rules, runbook fix, and a portable enforcement kit (#364/#365/#366)

.editorconfig

@@ -63,6 +63,12 @@ end_of_line = lf
 [spec/{validate,audit}.py]
 end_of_line = lf
 
+# The agent-safety kit's Python is shebang-executable tooling run by path (the PreToolUse hook and its
+# installer), so pin LF for the same reason as the entry points above - a CRLF shebang breaks direct
+# execution on a Unix host.
+[host-setup/agent-safety/*.py]
+end_of_line = lf
+
 # uv regenerates uv.lock with LF on every platform, so pin it or an EOL check (editorconfig-checker/CI)
 # reds on every `uv lock`/`uv sync` until the file is manually reconverted - same rationale as the
 # shebang/Dockerfile pins (a tool owns the ending). A Python repo on the CRLF default carries this; a repo

.gitattributes

@@ -15,9 +15,12 @@ catalog/snippets/husky/pre-commit text eol=lf
 
 # Vanilla `.py` follows the CRLF default - Python's universal newlines accept CRLF, and it is
 # commonly edited on Windows. Pin LF only for a `.py` executed directly via its shebang, by path -
-# here the CI validation entry point; do not re-add a blanket `*.py text eol=lf`.
+# here the CI validation entry point, the fleet-audit runner, and the agent-safety hook and its
+# installer. Do not re-add a blanket `*.py text eol=lf`.
 spec/validate.py text eol=lf
 spec/audit.py text eol=lf
+host-setup/agent-safety/gh-write-guard.py text eol=lf
+host-setup/agent-safety/install.py text eol=lf
 
 # uv regenerates uv.lock with LF on every platform; pin it so git enforces LF on checkout/renormalize and a
 # CRLF-default repo does not fight the tool on every `uv lock`/`uv sync`. A repo with no lockfile is unaffected.

.github/copilot-instructions.md

@@ -85,6 +85,9 @@ Known non-working request paths (don't rely on them - use the `requestReviews` m
 
 - `POST /requested_reviewers` with `reviewers=[Copilot]` can return 200 but no-op.
 - `copilot-pull-request-reviewer` as a requested reviewer slug returns 422.
+- `requestReviews` with the reviewer's bot node id in **`userIds`** fails with `Could not resolve to User node` - the Copilot reviewer is a **Bot**, so its node id goes in **`botIds`** (as in the mutation above), never `userIds`.
+- `suggestedActors(capabilities: [CAN_BE_ASSIGNED])` lists `copilot-swe-agent` (the coding agent), not `copilot-pull-request-reviewer` - do not source the reviewer's bot node id there. Read it from an existing review per step 1 above.
+- There is no `removePullRequestFromReviewRequest` mutation, and removing the reviewer to force a fresh pass is unnecessary anyway - `requestReviews` with `union: true` re-fires the review on the current head.
 
 ### Verify Review Covered Current Head
 
@@ -119,6 +122,8 @@ If a review did not run on the current head, retry:
 
 ### Reply and Thread Resolution Workflow
 
+Every id below is captured from a live query into a variable and passed from there - never hand-typed, guessed, or pasted as a `PRRT_...` literal. A node id resolves globally, so a fabricated or stale id does not fail, it writes to a real thread on an unrelated repository. This runbook implements [AGENTS.md "Repository Boundaries and Write Safety"](../AGENTS.md#repository-boundaries-and-write-safety): write only to this repo, capture every id from a live query, and never suppress a mutation's output.
+
 List unresolved threads. Use `first: 100` with cursor-based pagination; if `hasNextPage` is true, re-run with `after: "<endCursor>"` to retrieve the next page:
 
 ```sh
@@ -142,20 +147,38 @@ gh api graphql -f query='
 '
 ```
 
-Reply on a thread, then resolve it:
+Reply on a thread, then resolve it. Capture the target thread's id into `$TID` from the listing query above - filter to the thread being answered by its `path`, and guard for an empty result so a mutation never runs on a guessed id. When a file carries more than one unresolved thread, `path` alone is ambiguous and `head -n 1` would pick the wrong one, so narrow by first-comment body - the query already fetches `comments(first: 1)` for this - by adding `and (.comments.nodes[0].body | contains("<SNIPPET>"))` to the `select`:
 
 ```sh
+TID=$(gh api graphql -f query='
+{
+  repository(owner: "<owner>", name: "<repo>") {
+    pullRequest(number: <N>) {
+      reviewThreads(first: 100) {
+        nodes { id isResolved path comments(first: 1) { nodes { body } } }
+      }
+    }
+  }
+}' --jq '.data.repository.pullRequest.reviewThreads.nodes[]
+  | select(.isResolved == false and .path == "<PATH>")
+  | .id' | head -n 1)
+[ -n "$TID" ] || { echo "no matching unresolved thread on <PATH> - do not guess an id" >&2; return 1 2>/dev/null || exit 1; }
+
+# Show the mutation's output. Never append an output-discard or force-success tail
+# (>/dev/null, 2>/dev/null, &>/dev/null, || true, || :, || echo) to a write.
 gh api graphql -f query='
 mutation($threadId: ID!, $body: String!) {
   addPullRequestReviewThreadReply(input: { pullRequestReviewThreadId: $threadId, body: $body }) {
-    comment { id }
+    comment { id url }
   }
-}' -F threadId="PRRT_..." -F body="Fixed in <SHA>: <one-line summary>."
+}' -F threadId="$TID" -F body="Fixed in <SHA>: <one-line summary>."
 
+# Confirm isResolved: true in this response before treating the thread as closed - a write that
+# appears to fail may have taken on the server.
 gh api graphql -f query='
 mutation($threadId: ID!) {
   resolveReviewThread(input: { threadId: $threadId }) { thread { id isResolved } }
-}' -F threadId="PRRT_..."
+}' -F threadId="$TID"
 ```
 
 Issue-level Copilot comments (those in `issues/<N>/comments`) have no resolution action - GitHub provides no API or UI to resolve them. Reply if the finding warrants it; no resolution step is needed or possible.

AGENTS.md

@@ -15,6 +15,14 @@ The specific rules in this file implement a few governing principles. Read these
 - **Two version numbers, two jobs.** The 2-digit `major.minor` in `version.json` carries human meaning - the maintainer raises it only for a functional change (feature, behavior or API change, breaking change), at their discretion - while NBGV owns the patch position and always increments with git height, so every build is uniquely versioned with no edit. Human-facing docs name the 2-digit line; the toolchain guarantees monotonic builds. See "Release Model".
 - **Contracts state what, not how, and favor reuse.** [`WORKFLOW.md`](./WORKFLOW.md) fixes required outcomes, not a required implementation - two repos may satisfy a guarantee with different YAML. Within that freedom, apply good engineering practice: minimize duplication and maximize reuse, which is why the pipeline splits a carried, generic orchestration layer from a repo-owned build layer.
 
+## Repository Boundaries and Write Safety
+
+A state-changing GitHub call is the highest-blast-radius thing an agent does here: it runs under the maintainer's identity, so one wrong target writes to another owner's repository as the maintainer - an outward-facing, hard-to-reverse act. These rules bound every write - a git push, an API mutation, a comment, a label, a merge - on any platform. Reads are unrestricted. The bounds below are on writes.
+
+- **Write only to the current project's own repository.** Every state-changing call targets this project's `origin` and nothing else. A broad or logged-in identity is capability, not permission - a token that *can* reach another repository does not authorize writing to it. Writing to any other repository needs explicit, per-session human permission for that specific repository, and a "harmless test" write is still a write, so there is no probe exception. Reads from anywhere are fine.
+- **Never fabricate, guess, or reuse an identifier passed to a write.** Every id a state-changing call consumes - a node id, a numeric id, a thread or comment id - is captured from a live query in the **same** session into a variable and passed from there. Do not hand-type an id, guess it, recall it from memory or an earlier session, or copy it from documentation or an example. Ids commonly resolve **globally**, so a wrong-but-valid id does not fail - it writes to the wrong target, in someone else's repository. If a query returns no id, stop rather than invent one to proceed.
+- **A write is never a probe, and a write's output is never suppressed.** Never fire a state-changing call to see whether it works: decide it should happen, make it happen, and read the result. Never append output-discarding redirection or a force-success tail to a mutation (for example `>/dev/null`, `2>/dev/null`, `&>/dev/null`, `|| true`, `|| :`, `|| echo`) - the write's output is exactly what must be read. A write that appears to fail is **verified, not assumed harmless** - the operation may have succeeded on the server while the client reported an error - so confirm the actual state before retrying or moving on.
+
 ## Git and Commit Rules
 
 - **Default to staging, not committing.** Stage changes with `git add` and leave `git commit` to the developer unless the developer has explicitly authorized the agent to commit for the current ask ("commit this", "open a PR", etc.). Authorization is scope-bound - it covers the commits needed for that specific task, not a blanket commit license for the rest of the session.

host-setup/agent-safety/.markdownlint-cli2.jsonc

@@ -0,0 +1,8 @@
+{
+    // claude-md-safety.md is a fragment the installer appends into ~/.claude/CLAUDE.md (which already
+    // has its own H1), so it intentionally opens at H2. MD041 (first line must be a top-level heading)
+    // does not apply to an appended snippet. This nested config affects only this directory.
+    "config": {
+        "MD041": false
+    }
+}

host-setup/agent-safety/README.md

@@ -0,0 +1,72 @@
+# Agent Write-Safety Kit
+
+Per-machine, user-account-scoped guards against an agent making a mis-targeted GitHub **write** under the maintainer's identity. Deploy it as the **first thing on any new system** where Claude Code runs with the `gh` credentials logged in (WSL, Linux, macOS, Proxmox, Windows).
+
+## What It Installs
+
+Into `~/.claude/` (or `%USERPROFILE%\.claude\` on Windows):
+
+- **`hooks/gh-write-guard.py`** - a PreToolUse hook that denies the three write footguns behind the cross-repo comment incident: a state-changing `gh` call whose output is discarded, a GraphQL mutation passing a **literal** node id instead of a `$variable`, and a `gh` write whose explicit target is outside the checkout's `origin`. Reads and everything else pass through. It fires even in autonomous / bypass-permissions sessions, which is how the incident happened.
+- **A `## GitHub write safety` section in `CLAUDE.md`** - the same three rules as behavioral guidance, loaded into every session on the machine (including ad-hoc work outside any project). It mirrors the committed `AGENTS.md` "Repository Boundaries and Write Safety" rules, which only reach fleet repos.
+
+The hook is the mechanical backstop. The CLAUDE.md rules and the carried AGENTS.md rules are the behavioral layer. Prose alone is not enough - the incident happened under prose rules - so both ship.
+
+## Install (Idempotent - Safe to Re-Run to Update)
+
+```sh
+# Linux / WSL / macOS / Proxmox
+host-setup/agent-safety/install.sh
+```
+
+```powershell
+# Windows
+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 the CLAUDE.md block in place (marker-delimited) rather than duplicating it.
+
+**Restart Claude Code sessions on the machine afterward** so the new hook and CLAUDE.md load.
+
+## Verify (POSIX Shell)
+
+```sh
+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)
+```
+
+On Windows PowerShell:
+
+```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
+```
+
+Live end-to-end (in any repo): attempt a discarded-output write and confirm the Bash tool is blocked:
+
+```sh
+gh api graphql -f query='mutation{noop}' -F t="PRRT_x" >/dev/null 2>&1 || true   # blocked by the hook
+```
+
+## Manual settings.json Shape (for Reference)
+
+The installer writes this. It is here so you can inspect or hand-place it:
+
+```json
+{
+  "hooks": {
+    "PreToolUse": [
+      { "matcher": "Bash", "hooks": [ { "type": "command", "command": "\"python3\" \"<home>/.claude/hooks/gh-write-guard.py\"" } ] }
+    ]
+  }
+}
+```
+
+## 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.
+- **Precision over recall.** The hook denies the specific dangerous shapes with high confidence rather than gating every write, so it never blocks legitimate work. A shape it does not catch still falls under the behavioral rules.
+- **Opaque targets are unseen.** The hook cannot see the repository behind a GraphQL node id, which is exactly why rule 2 blocks a *literal* id at all - a captured `$variable` is trusted. Likewise, the cross-origin check only runs when an `origin` can be resolved and the write names an explicit `-R`/`repos/<owner>/<repo>` target. A write from a non-git directory, or one whose target is only a node id, is evaluated by rules 1 and 2 alone.
+- **Not a credential control.** A fine-grained PAT limited to owned repositories is a separate, stronger structural guard (a hard `403` on any non-owned repo) and is left to per-machine credential setup, out of this kit.
+
+<!-- Repo -->
+[issue-365]: https://github.com/ptr727/ProjectTemplate/issues/365

host-setup/agent-safety/claude-md-safety.md

@@ -0,0 +1,9 @@
+<!-- agent-safety v1 start -->
+## GitHub Write Safety (Any Project, Every Session)
+
+A `gh` / GitHub API write runs under the logged-in identity, so a mis-targeted write acts publicly as that account on someone else's repository - outward-facing and hard to reverse. These rules bound every write (a git push, an API mutation, a comment, a label, a merge) in every session on this machine, including ad-hoc work outside any project. Reads are unrestricted. A committed repo's `AGENTS.md` "Repository Boundaries and Write Safety" states the same rules for its fleet, and the two are kept in sync deliberately, because this file also covers sessions that `AGENTS.md` never reaches. The `gh-write-guard` PreToolUse hook enforces the mechanical half.
+
+- **Write only to the current project's own repository.** Every state-changing call targets this checkout's `origin` and nothing else. A broad or logged-in identity is capability, not permission. Another repository needs explicit, per-session human permission for that specific repository, and a "harmless test" write is still a write.
+- **Never fabricate, guess, or reuse an identifier passed to a write.** Every id a write consumes (a node id, a numeric id, a thread or comment id) is captured from a live query in the same session into a variable and passed from there. Ids resolve globally, so a wrong-but-valid id does not fail - it writes to the wrong target in another repository. If a query returns no id, stop rather than invent one.
+- **A write is never a probe, and a write's output is never suppressed.** Never fire a state-changing call to see whether it works, and never append an output-discarding or force-success tail (for example `>/dev/null`, `2>/dev/null`, `&>/dev/null`, `|| true`, `|| :`, `|| echo`) to a mutation. A write that appears to fail is verified, not assumed harmless - it may have succeeded on the server.
+<!-- agent-safety v1 end -->