From cedf38c437d088641d04c7e57c38e492f13443ab Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 19:43:20 +0000 Subject: [PATCH] fix(pm): guard-governed-enqueue reads a rename's old path too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../hooks/guard-governed-enqueue.selftest.sh | 24 +++++++++++++++++++ .claude/hooks/guard-governed-enqueue.sh | 2 +- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/.claude/hooks/guard-governed-enqueue.selftest.sh b/.claude/hooks/guard-governed-enqueue.selftest.sh index 46a88dea88..40ca439f93 100755 --- a/.claude/hooks/guard-governed-enqueue.selftest.sh +++ b/.claude/hooks/guard-governed-enqueue.selftest.sh @@ -60,6 +60,14 @@ files_of() { # files_of path... -> the /pulls/{n}/files body shape printf '%s' "$out" } +# A RENAME is the one entry shape `files_of` cannot build: every other status +# carries `filename` alone, a renamed one ALSO carries `previous_filename`. +# Measured on PR #17372 (`GET /pulls/17372/files`): `filename` is the NEW path, +# `previous_filename` the OLD one. `files_of` keeps its shape; this is the twin. +renamed_of() { # renamed_of -> the /files body for one RENAME + jq -nc --arg o "$1" --arg n "$2" '[{filename:$n,previous_filename:$o,status:"renamed"}]' +} + approved_at() { # approved_at jq -nc --arg l "$1" --arg c "$2" '[{state:"APPROVED",user:{login:$l},commit_id:$c}]' } @@ -86,6 +94,8 @@ F_DISMISSED="$(fixture governed-dismissed "$GOVERNED_FILES" \ F_CLEAR="$(fixture not-governed "$CLEAR_FILES" "$NO_REVIEWS")" F_REGEN="$(fixture pure-regeneration "$REGEN_FILES" "$NO_REVIEWS")" F_EMPTY="$(fixture empty-diff '[]' "$NO_REVIEWS")" +F_RENAMED_OFF="$(fixture governed-renamed-off-the-surface "$(renamed_of AGENTS.md docs/AGENTS.md)" "$NO_REVIEWS")" +F_RENAMED_CLEAR="$(fixture rename-within-an-ordinary-prefix "$(renamed_of packages/spec/src/a.ts packages/spec/src/b.ts)" "$NO_REVIEWS")" mcp() { # mcp [owner] [repo] 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 expect allow 'an ordinary diff enqueues freely' \ "$(mcp $AUTO 14070)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_CLEAR" +echo "== a RENAME is a change to BOTH paths, so the OLD one is read too ==" +# Read `filename` alone and the old path is simply absent from the list handed to +# the register — and a dropped path can only REMOVE governance, never add it, so +# a diff that moves AGENTS.md to docs/AGENTS.md would read here as an ordinary +# one. The other two readers of the same diff already see both paths (the queue +# guard decomposes per commit with `--no-renames`, and `--pr` derives the list +# three-dot), so this is the hook catching up to them, not a new predicate. +expect block 'a governed file renamed OFF the governed surface is still governed' \ + "$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_RENAMED_OFF" +expect_says 'AGENTS.md' 'the OLD path is the governed hit the refusal names' \ + "$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_RENAMED_OFF" +expect allow 'a rename inside a non-governed prefix changes no verdict' \ + "$(mcp $AUTO 14070)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_RENAMED_CLEAR" + echo "== PURE REGENERATION: the hook must AGREE with the register, never re-decide ==" # The requirement (maintainer 2026-09-01: 纯生成的指针行 … 不需要我审核吧) is that # this hook never re-closes a zero-approval path the register clears. Pinned diff --git a/.claude/hooks/guard-governed-enqueue.sh b/.claude/hooks/guard-governed-enqueue.sh index 961088ab3d..092b5818bd 100755 --- a/.claude/hooks/guard-governed-enqueue.sh +++ b/.claude/hooks/guard-governed-enqueue.sh @@ -400,7 +400,7 @@ try { d = JSON.parse(fs.readFileSync(process.env.OS_GUARD_FILE, "utf8")); } catch { process.exit(1); } const mode = process.env.OS_GUARD_MODE; if (mode === "head-sha") { const s = d && d.head && d.head.sha; if (!s) process.exit(1); console.log(s); } -else if (mode === "filenames") { if (!Array.isArray(d)) process.exit(1); for (const f of d) if (f && f.filename) console.log(f.filename); } +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); } } else if (mode === "count") { if (!Array.isArray(d)) process.exit(1); console.log(d.length); } else if (mode === "exceptions") console.log(((d || {}).exceptions || []).length); else if (mode === "hits") console.log((((d || {}).hitPaths) || []).join(", "));