Skip to content

fix(pm): guard-governed-enqueue reads a rename's old path too - #17531

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-17503-enqueue-hook-previous-filename
Sep 11, 2026
Merged

fix(pm): guard-governed-enqueue reads a rename's old path too#17531
os-zhuang merged 1 commit into
mainfrom
claude/issue-17503-enqueue-hook-previous-filename

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #17503

guard-governed-enqueue.sh handed the register's --test predicate only the NEW path of a renamed entry, so a rename OUT of a governed path 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 — the one direction a governed-surface reading must never be wrong in.

The hook still decides nothing of its own. This changes what it HANDS the first predicate, never how either predicate answers.

The reader line, before and after

before — .claude/hooks/guard-governed-enqueue.sh, the filenames mode of the embedded node payload reader:

else if (mode === "filenames") { if (!Array.isArray(d)) process.exit(1); for (const f of d) if (f && f.filename) console.log(f.filename); }

after:

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); } }

The old path is printed as its own line, so the register receives it as a path argument like any other. Nothing else moved. The count mode still reads d.length, so the pagination arithmetic (10 pages of 100) still counts ENTRIES and is untouched by the extra printed lines. The header was left alone deliberately: its "Known boundaries" block describes this read as the PR's "changed files" and names no field, so nothing in it became false.

The measured rename entry (the shape the fixture copies)

One REST read, GET /repos/objectstack-ai/objectstack/pulls/17372/files?per_page=100 — HTTP 200, 100 entries (modified 50, removed 28, added 10, renamed 12). Its first renamed entry:

{
  "filename": "content/docs/references/marketplace/marketplace.mdx",
  "previous_filename": "content/docs/references/cloud/marketplace.mdx",
  "status": "renamed",
  "changes": 6
}

filename is the NEW path, previous_filename the OLD one, and previous_filename is present in the entry's key list only on a renamed entry — which is why the reader guards on its presence rather than on status.

The register's answer, with and without the old path (re-run on this branch)

run exit the register's own verdict line
node scripts/pm/check-governed-merges.mjs --test docs/AGENTS.md 0 ✅ NOT governed — ordinary queue landing applies to a PR with exactly this file list.
node scripts/pm/check-governed-merges.mjs --test docs/AGENTS.md AGENTS.md 3 ⛔ GOVERNED — a human merge is the review record for this PR (#9495 regime). · AGENTS.md ×1

That is the whole defect in two runs: for a rename off the surface the hook produced the first list. It now produces the second.

The refusal that follows names the OLD path as the hit, measured on the fixture directly:

   governed:  AGENTS.md

Self-test: before / after / ablation

.claude/hooks/guard-governed-enqueue.selftest.sh — no network, both predicates run for real.

run result exit
before the edit (branch point) 52 passed, 0 failed 0
after the edit 55 passed, 0 failed 0
ablation: the reader line reverted, the selftest kept 53 passed, 2 failed 1
restored 55 passed, 0 failed 0

Two cases were added, spelled as three assertions (expect block, expect_says, expect allow), so the count moves by +3.

The ablation ran from the committed state and mutated the file on disk, not a copy:

  • the mutation is proved on disk before anything is read: grep -c 'f.previous_filename' moved 1 to 0, the pre-fix spelling counted 1, and the file's blob hash moved 092b5818 to 961088ab;
  • the two red rows are exactly the new governed case, and the non-governed rename stayed green:
== a RENAME is a change to BOTH paths, so the OLD one is read too ==
  FAIL want=block got=allow  a governed file renamed OFF the governed surface is still governed
  FAIL missing "AGENTS.md"  the OLD path is the governed hit the refusal names
  ok   allow  a rename inside a non-governed prefix changes no verdict
  • the restore leg is a git checkout HEAD -- of the absolute path, inside a trap, and it is proved by state rather than by an exit code: the restored blob hash equals the HEAD blob 092b5818bdcb0cc26b068f18ee7bc47852d4d0c7 and git diff HEAD is empty.

One honest wrinkle: the matrix's pure-regeneration row is an AGREEMENT assertion, and it took the NOT lifted branch in the first two runs (no node_modules yet) and the LIFTED branch in the last two (installed). Either branch passes by construction, so the counts above are comparable; nothing in this card touches that row.

The one judgement, on the four axes

The choice was to emit the old path as a second ordinary path versus adding a reader mode for renames or teaching a predicate about renames. 实际业务需求: measured, not speculative — the one REST read above found 12 renamed entries in a single real PR of this repo, and a docs reorganisation moving a governed file is exactly the shape the card names; the other two 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, landed in PR #17504), so this option makes three readers agree rather than inventing a fourth reading. 项目长远合理性: a rename's old path IS a changed path, so the correct fix is inside the existing "hand the register the changed paths" contract; a new mode would fork one reading into two lists that can disagree, and a rename rule inside a predicate would be the second mechanism this hook's header explicitly refuses. 防 AI 写错: the failure direction here is silent under-enumeration — the hook allows and prints nothing — so the fix must make the strict answer arrive by default; widening what the caller HANDS the predicate does that, whereas any tolerance added on the reading side would be the ??-fallback shape this repo rejects. 创业阶段不扩散需求: zero new surface — one reader line, no new mode, no new env var, no new register row, no new predicate, and the selftest's files_of keeps its shape (the rename twin is its own three-line helper). All four axes point the same way, so there is no trade-off to hand up.

Verification

Gate families derived from the final diff by node scripts/pm/dispatch-gates.mjs --commands (no paths — the script derives its own changeset: 2 committed paths against merge base cac62d5f9), every one run, each exit code captured before any pipe:

family exit
node scripts/check-closing-keyword-parity.mjs 0
node scripts/check-closing-keyword-parity.mjs --self-test 0
node scripts/check-comment-mask-corpus.mjs 0
pnpm --filter @objectstack/lint run check:doc-formula-expressions 0 (see below)
pnpm check:agent-test-spelling 0
pnpm check:bash32-floor 0
pnpm check:doc-authoring 0
pnpm check:driver-memory-census 0
pnpm check:nul-bytes 0
pnpm check:pm-governed-merges 0
pnpm check:refd-timer-probe 0
pnpm check:skill-frame-sync 0
pnpm check:watch-hint-literal 0

Their own verdict lines, for the three that speak to this diff:

✓ check-bash32-floor: 33 tracked shell file(s) under scripts/**, .claude/hooks/**, .githooks/** name no bash 4+ construct outside a comment, a guarded ${VAR:-} read, or a non-command position.
check-nul-bytes: OK (scanned 8315 text file(s) -- 8315 tracked, 0 untracked-not-ignored; skipped 7 binary; no raw ASCII control bytes).
✓ check-governed-merges --self-test: 317 assertions (…) — and the #17003 derivation set: … a rename reaching the predicate as BOTH of its paths …

check:doc-formula-expressions first exited 3 with PREREQUISITE NOT MET — the workspace package @objectstack/formula is not built, which is NOT MEASURED rather than a finding. Its own remedy was run through the shared verify lock (os-verify-lock: VERDICT command-exit 0 · held the lock 157s · waited 0s, Tasks: 4 successful, 4 total) and the gate then exited 0: ✓ check:doc-formula-expressions: 22 record-scoped formula example(s) across 438 files / 1375 TS blocks judged clean.

Reconciliation: node scripts/pm/dispatch-gates.mjs --ran over the list with exit codes — ✓ dispatch-gates --ran: 13 derived famil(ies) accounted for — 13 run, 0 NOT-MEASURED (a DERIVED zero — all 13 recorded an exit code and none of them is 3).

Control bytes, beyond check:nul-bytes: grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' over both edited files matched nothing.

Path face — node scripts/pm/check-governed-merges.mjs --branch claude/issue-17503-enqueue-hook-previous-filename, exit 3:

⛔  GOVERNED — a human merge is the review record for this PR (#9495 regime).
      .claude/** ×2 — the agent instruction tree (skills, agents, hooks, settings)

So this PR stays DRAFT and the maintainer merges it. No seat flips it ready, enqueues it, arms auto-merge, or approves it.

Changeset: none, and the skip-changeset label is applied. .claude/** ships in no package's files[] — the whole diff is two agent-side hook scripts, so nothing published moves.

Acceptance notes

  • noted, not filed: the guard's read cap (10 pages of 100 entries) still warns-and-allows on a longer diff. It is unchanged here and already stated in the hook's own "Known boundaries" block; the extra printed lines do not touch the arithmetic, which counts entries. Carrier: the next PR that touches this hook's read path.
  • noted, not filed: files_of in the self-test still builds filename-only entries by design, so any future status-specific entry shape needs its own twin helper beside renamed_of rather than a widened files_of. Carrier: none queued.
  • noted, not filed: expect_says 'AGENTS.md' is a substring match, and docs/AGENTS.md contains it — the assertion is nevertheless discriminating here, because the refusal prints only the register's hit list (governed: AGENTS.md) and under the ablation there is no refusal at all, which is what the second FAIL row above shows.

Clause-②: no

维护者速读(草稿)

改了什么 —— 入队前置钩子在读 GitHub 的「变更文件」列表时,只取了每条记录的 filename。重命名记录里 filename 是新路径,旧路径在 previous_filename 里。现在两个路径都交给受管面判定器,一行 reader 的改动,外加自测里两个用例。

为什么改 —— 把一个受管文件重命名出受管目录(例如 AGENTS.md 移到 docs/AGENTS.md),旧路径被丢掉,钩子读成「不受管」,于是这类 PR 可以被 agent 直接入队。漏掉一个路径只会减少治理,不会增加,所以这是缺陷不是小瑕疵。同一份 diff 的另外两个读取方(合并队列守卫、--pr 推导)本来就能看见两个路径,这次是钩子补齐,不是新增判定。

风险与代价(含回滚) —— 风险很小:钩子自身不做任何判断,只是多递一个路径参数;失效方向是「多报受管」,而多报只会让 PR 停在人工合并这条既定路线上。回滚就是把那一行 reader 改回去(消融实验已经演练过一次:改回去 → 新用例转红 → 恢复 → 全绿)。无发布影响,不需要 changeset。

席位意见 ——(留给 skills 席位定稿)

你要做的 —— 这是 .claude/** 受管面,PR 保持 draft,由你手动合并;不需要任何 agent 审批。


Generated by Claude Code

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>

Copy link
Copy Markdown
Collaborator Author

维护者速读

改了什么 —— .claude/hooks/guard-governed-enqueue.sh(入队前置钩子)读 GitHub「变更文件」列表的那一行:重命名记录除了 filename(新路径),现在把 previous_filename(旧路径)也作为一条路径交给受管面判定器 check-governed-merges.mjs --test。自测 guard-governed-enqueue.selftest.sh 加一对重命名用例(受管文件改名移出受管目录 ⇒ 拦并点名旧路径;普通前缀内改名 ⇒ 放行),52 → 55 条断言。

为什么改 —— 只读新路径时,把 AGENTS.md 改名成 docs/AGENTS.md 这样的 diff 在钩子处读成「不受管」,agent 可以把它直接入队;丢路径只会减少治理。合并队列守卫(objectstack main 的必需检查)和 --pr 推导本来就读两个路径,这次是钩子补齐;objectui 的队列守卫还没进必需集(objectui#6596 等您开关),在那之前这个钩子是 objectui 受管 PR 入队前唯一的自动读取,所以定 p2。

风险与代价(含回滚) —— 钩子不判断,只多递一个路径;失效方向是「多报受管」,后果是 PR 停在人工合并这条既有路线。本席在 head cedf38c4 上跑自测 55/0,把 reader 行换回 main 的版本做消融:53/2,红的恰是两条新断言,恢复后 55/0。回滚 = revert 这一行和两个用例。CI 在 head 上 12 过 / 12 跳 / 4 在跑,无失败。

席位意见 —— 通过。受管面(.claude/**):本席不翻 ready、不入队、不批准。

你要做的 —— 一个动作:人工合并本 draft PR。

skills 席,session session_01YKEjmbYNvYWJvWGSWx26zK,2026-09-10T20:00Z。


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review September 11, 2026 06:29
@os-zhuang
os-zhuang enabled auto-merge September 11, 2026 06:29
@os-zhuang
os-zhuang added this pull request to the merge queue Sep 11, 2026
Merged via the queue into main with commit f8ee491 Sep 11, 2026
35 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-17503-enqueue-hook-previous-filename branch September 11, 2026 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-user-decision size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

3 participants