Skip to content

Commit f8ee491

Browse files
os-litantclaude
andauthored
fix(pm): guard-governed-enqueue reads a rename's old path too (#17531)
The changed-files reader emitted `filename` alone, so a renamed entry contributed only its NEW path to the list handed to the register's `--test`. A rename OUT of a governed path therefore read as NOT governed at the hook: a diff moving AGENTS.md to docs/AGENTS.md, or skills/x.md to docs/x.md, enqueued freely. A dropped path can only REMOVE governance, never add it, which is the one direction a governed reading must never be wrong in. The `filenames` mode now also prints `previous_filename` when it is present and differs, as its own line, so the register receives it as a path argument like any other. The two other readers of the same diff already see both paths — the queue guard decomposes per commit with `--no-renames`, and `check-governed-merges.mjs --pr` derives the list three-dot — so this is the hook catching up to them. No predicate moves: the hook still decides nothing and asks the same two single sources. Claude-Session: https://claude.ai/code/session_01YKEjmbYNvYWJvWGSWx26zK Co-authored-by: Claude <noreply@anthropic.com>
1 parent d8de599 commit f8ee491

2 files changed

Lines changed: 25 additions & 1 deletion

File tree

‎.claude/hooks/guard-governed-enqueue.selftest.sh‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ files_of() { # files_of path... -> the /pulls/{n}/files body shape
6060
printf '%s' "$out"
6161
}
6262

63+
# A RENAME is the one entry shape `files_of` cannot build: every other status
64+
# carries `filename` alone, a renamed one ALSO carries `previous_filename`.
65+
# Measured on PR #17372 (`GET /pulls/17372/files`): `filename` is the NEW path,
66+
# `previous_filename` the OLD one. `files_of` keeps its shape; this is the twin.
67+
renamed_of() { # renamed_of <old path> <new path> -> the /files body for one RENAME
68+
jq -nc --arg o "$1" --arg n "$2" '[{filename:$n,previous_filename:$o,status:"renamed"}]'
69+
}
70+
6371
approved_at() { # approved_at <login> <sha>
6472
jq -nc --arg l "$1" --arg c "$2" '[{state:"APPROVED",user:{login:$l},commit_id:$c}]'
6573
}
@@ -86,6 +94,8 @@ F_DISMISSED="$(fixture governed-dismissed "$GOVERNED_FILES" \
8694
F_CLEAR="$(fixture not-governed "$CLEAR_FILES" "$NO_REVIEWS")"
8795
F_REGEN="$(fixture pure-regeneration "$REGEN_FILES" "$NO_REVIEWS")"
8896
F_EMPTY="$(fixture empty-diff '[]' "$NO_REVIEWS")"
97+
F_RENAMED_OFF="$(fixture governed-renamed-off-the-surface "$(renamed_of AGENTS.md docs/AGENTS.md)" "$NO_REVIEWS")"
98+
F_RENAMED_CLEAR="$(fixture rename-within-an-ordinary-prefix "$(renamed_of packages/spec/src/a.ts packages/spec/src/b.ts)" "$NO_REVIEWS")"
8999

90100
mcp() { # mcp <tool> <pull> [owner] [repo]
91101
jq -nc --arg t "$1" --argjson n "$2" --arg o "${3:-objectstack-ai}" --arg r "${4:-objectstack}" \
@@ -201,6 +211,20 @@ echo "== nothing governed in the diff: allowed, and no review is ever consulted
201211
expect allow 'an ordinary diff enqueues freely' \
202212
"$(mcp $AUTO 14070)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_CLEAR"
203213

214+
echo "== a RENAME is a change to BOTH paths, so the OLD one is read too =="
215+
# Read `filename` alone and the old path is simply absent from the list handed to
216+
# the register — and a dropped path can only REMOVE governance, never add it, so
217+
# a diff that moves AGENTS.md to docs/AGENTS.md would read here as an ordinary
218+
# one. The other two readers of the same diff already see both paths (the queue
219+
# guard decomposes per commit with `--no-renames`, and `--pr` derives the list
220+
# three-dot), so this is the hook catching up to them, not a new predicate.
221+
expect block 'a governed file renamed OFF the governed surface is still governed' \
222+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_RENAMED_OFF"
223+
expect_says 'AGENTS.md' 'the OLD path is the governed hit the refusal names' \
224+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_RENAMED_OFF"
225+
expect allow 'a rename inside a non-governed prefix changes no verdict' \
226+
"$(mcp $AUTO 14070)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_RENAMED_CLEAR"
227+
204228
echo "== PURE REGENERATION: the hook must AGREE with the register, never re-decide =="
205229
# The requirement (maintainer 2026-09-01: 纯生成的指针行 … 不需要我审核吧) is that
206230
# this hook never re-closes a zero-approval path the register clears. Pinned

‎.claude/hooks/guard-governed-enqueue.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ try { d = JSON.parse(fs.readFileSync(process.env.OS_GUARD_FILE, "utf8")); }
400400
catch { process.exit(1); }
401401
const mode = process.env.OS_GUARD_MODE;
402402
if (mode === "head-sha") { const s = d && d.head && d.head.sha; if (!s) process.exit(1); console.log(s); }
403-
else if (mode === "filenames") { if (!Array.isArray(d)) process.exit(1); for (const f of d) if (f && f.filename) console.log(f.filename); }
403+
else if (mode === "filenames") { if (!Array.isArray(d)) process.exit(1); for (const f of d) { if (f && f.filename) console.log(f.filename); if (f && f.previous_filename && f.previous_filename !== f.filename) console.log(f.previous_filename); } }
404404
else if (mode === "count") { if (!Array.isArray(d)) process.exit(1); console.log(d.length); }
405405
else if (mode === "exceptions") console.log(((d || {}).exceptions || []).length);
406406
else if (mode === "hits") console.log((((d || {}).hitPaths) || []).join(", "));

0 commit comments

Comments
 (0)