Skip to content

feat(hooks): guard the shared process table — kill only a PID you recorded - #16843

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-16182-process-kill-guard
Sep 8, 2026
Merged

feat(hooks): guard the shared process table — kill only a PID you recorded#16843
os-zhuang merged 3 commits into
mainfrom
claude/issue-16182-process-kill-guard

Conversation

@hotlong

@hotlong hotlong commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #16182

Worktree isolation gives each agent its own checkout, index and HEAD. It gives it nothing
over the process table: there is one per container, so a name-matched kill reaches whatever a
parallel agent happens to be running under that name — and it reports success either way.
The agent that fired it observes nothing; the agent whose run died sees a killed process and a
truncated log with no signal tying it back, and re-runs blaming a flake. That is the same
property that makes the git stash rule a rule rather than advice, which is why this is
guard-shared-stash.sh's shape and not a sixth one.

1. The sweep — population fixed BEFORE the rule (triage's ⚠️)

Swept over all tracked files at d4401f75b: pkill, killall, killall5, taskkill,
fuser -k, xargs kill, pgrep, and ps … | grep … pipelines.

shape tracked occurrences verdict
pkill (any form) 0 command sites (2 prose sites: .claude/agents/os-dev.md L92, skills/objectstack-pm-dispatch/rules/dev-template.md L64 — both already state the rule) blocked
killall / killall5 / taskkill 0 blocked
xargs kill / fuser -k 0 blocked (first)
ps … | grep … pipeline 0 blocked only when a kill is in the same command
pgrep -s SID / pgrep -P PID 6scripts/gen-sdui-manifest.sh L71/L76, scripts/publish-smoke.sh L154, packages/spec/scripts/gen-sdui-manifest-cleanup.test.ts L104/L112/L149 allowed — selection by a session/parent handle the caller owns, not by name
kill PID / kill -0 PID / kill $(lsof -ti tcp:PORT) many, incl. AGENTS.md L144 and L527 and .github/workflows/scaffold-e2e.yml allowed — this repo prescribes them

So the class the rule names is selection by NAME, not "the kill verb": every live kill
site in the tree is PID-scoped and stays green. Nothing in the tree had to be rewritten, and
the stop-condition (a legitimate name-pattern kill that cannot be rewritten PID-scoped) did
not fire.

2. The rule the guard enforces

Positive form first, because a rule that only forbids gets routed into another spelling:

Kill only a PID you recorded.

Blockedpkill with any pattern operand or any unrecognised option (with or without
-f); killall in every form but --help/-l/-V; a pgrep pattern substituted or
piped into a kill; ps piped through grep in a command that also kills.

Allowedkill / kill -0 / kill -9 on a pid, a job spec or your own group;
kill $(lsof -ti tcp:PORT); pgrep -P / pgrep -s even piped into a kill; pkill -P /
pkill -s with no pattern operand (add a pattern and it is blocked again); every read
(pgrep -f PATTERN, ps aux | grep name) — nothing dies, so nothing is blocked.

Fail-CLOSED once the command word is confidently pkill/killall (an option it does not
recognise is refused, not waved through), fail-OPEN on anything it cannot parse — the
guard-shared-stash.sh contract, exit 0 allow / exit 2 block. Escape: OS_ALLOW_PROCESS_KILL=1.
Two boundaries are stated in the header rather than left to be rediscovered: wrapped
invocations (bash -c, a script file) are not caught, and the pgrep/ps coupling is
whole-command rather than pipeline-exact.

3. The two governed lines, before and after

AGENTS.md — one line, appended to Multi-agent working discipline item 8, the existing
"never stop someone else's server" rule this generalises. Nothing removed:

+   ⛔ One process table per container: **kill only a PID you recorded, never a name** (`guard-process-kill.sh`).

CLAUDE.md — the ⛔ stash section extended in place, line count unchanged. Before:

## ⛔ Never `git stash` — the stash stack is NOT covered by worktree isolation

`refs/stash` lives in the common `.git` dir, so all worktrees share one LIFO stack: your
`pop` takes another agent's entry and reports **success** — use a patch or a wip commit.
Hook `guard-shared-stash.sh` enforces it (override `OS_ALLOW_STASH=1`; re-run its
`.selftest.sh` if you change it). Full rule: AGENTS.md → **Multi-agent working discipline**.

After (same 6 lines, every clause of the old text still present):

## ⛔ Never `git stash`, never kill by name — worktree isolation covers neither

`refs/stash` lives in the common `.git` dir and the process table is one per container, neither isolated: your `pop`
takes another agent's entry, a name-matched kill takes their run, and both report **success**. Use a patch or a wip
commit; kill only a PID you recorded. Hooks `guard-shared-stash.sh` / `guard-process-kill.sh` (`OS_ALLOW_STASH=1`,
`OS_ALLOW_PROCESS_KILL=1`; re-run each `.selftest.sh`). Full rule: AGENTS.md → **Multi-agent working discipline**.

The intro's hard-coded "the four rules" became "the rules" (one line, one word deleted):
the section now carries two rules and two hooks, so the count was about to become false.

4. Line budgets — measured first, no ceiling touched

pnpm check:pm-skill-ratchet at 089173a8a, exit 0, its own verdict lines:

✓ check-skill-line-ratchet: AGENTS.md is 1068 lines (ceiling 1068; headroom 0).
✓ check-skill-line-ratchet: CLAUDE.md is 41 lines (ceiling 41; headroom 0).
✓ check-skill-line-ratchet: AGENTS.md: widest table row is 768 bytes (pin 768; headroom 0).
✓ check-skill-line-ratchet: CLAUDE.md: widest table row is 0 bytes (pin 0; headroom 0).

AGENTS.md 1067 → 1068 spends its one line of headroom. CLAUDE.md stays at 41: the new
content is absorbed into the stash section's existing six lines, which carried 166 bytes of
slack under the 120-byte cap. No ceiling is raised, no ruled clause deleted, and no line was
bought by re-wrapping
— the mechanism, the class and the escape live in the hook header,
which carries no ceiling. That is the ratchet header's own prescribed remedy: "pays its way
by moving narrative out … instead of raising the roof."

⚠️ The cost of that discipline, stated rather than hidden: AGENTS.md gets one dense line and
CLAUDE.md one clause, so the failure mode ("reports success either way") is spelled out only
in CLAUDE.md and the hook header, not in AGENTS.md. Buying a second AGENTS.md line needs
a maintainer ruling, and this PR does not take one.

5. Tests

.claude/hooks/guard-process-kill.selftest.sh69 passed, 0 failed, exit 0. Discovered
automatically by lint.yml's hook self-test collector (it globs .claude/hooks/**/*.selftest.sh),
so no workflow edit. All five sibling self-tests still pass: governed-enqueue 52/0,
main-checkout-bash 130/0, main-checkout 120/0, shared-stash 53/0, tree-enum 38/0.

Ablation — two legs, each proved to land on disk and each restored by blob hash:

leg mutation self-test
name-pattern branch is_name_selector's *) return 0 (a bare operand IS the name pattern) → *) i=$((i + 1)) 64 passed, 5 failed — every pgrep-pattern-into-kill row flipped to allow
pkill operand branch check_pkill's [ "$pidscoped" -eq 1 ] && return 0 → unconditional allow 68 passed, 1 failed

⚠️ The second leg is the interesting one and it is reported as it happened, not as the template
expects: on its first run it left the self-test green at 68/0. Bare pkill is the only
input that reaches that gate — every other spelling returns earlier — so the branch had no case
standing on it. The pin was added (commit 2, expect block 'pkill') and the leg then reds. The
ablation found the hole instead of shipping it.

Restore evidence per leg: anchor line count 1 → 0 with the injected line present (so the edit
was not a no-op), mutated blob 8acbd348… / f8d384ff… ≠ HEAD blob c59489df…, restored blob
equals c59489df…, and git diff HEAD names no path. The mutation script carries a
trap … EXIT INT TERM on absolute paths and restores with git checkout HEAD -- PATH, never a
bare checkout.

6. Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack re-derived in
the worktree after the files existed and after merging origin/main (acf4d3833): 18
families. All 18 run at 089173a8a, every exit code captured by redirect before any pipe, all
exit 0. Reconciliation with --ran:

✓ dispatch-gates --ran: 18 derived famil(ies) accounted for — 18 run, 0 NOT-MEASURED.

pnpm --filter @objectstack/lint run check:doc-formula-expressions first returned exit 3 —
PREREQUISITE NOT MET
(unbuilt @objectstack/formula / @objectstack/lint), which is not a
finding; after turbo run build for those two packages under the shared verify lock it is exit 0.

node scripts/pm/check-governed-merges.mjs --test …exit 3, GOVERNED (5 of 5 paths hit
the register). Draft PR, human merge, no ready-flip, no enqueue, no auto-merge.

ESLint narrowing, three readings together: the universe is eslint --no-inline-config over the
diff's own paths and it accepted all five (they are inside the configured universe);
--format json reports 5 files linted, 0 errors; and eslint.config.mjs L328 states the
config has "no parserOptions.project, no typed @typescript-eslint rules", so nothing in
this diff can move a verdict on an untouched file. Measured at 089173a8a, the final commit.

7. skip-changeset — measured, not assumed

Of the 70 published packages carrying a files[] array, zero name AGENTS.md,
CLAUDE.md or .claude in it (positive control: packages/spec's files[] reads back as
["dist","json-schema","liveness","prompts","llms.txt","README.md","src/**/*.zod.ts",…], so the
scan is reading real arrays). Nothing published ships. Label applied.

验收备注

Out-of-scope observations, filed nowhere and deliberately:

  • skills/objectstack-pm-dispatch/rules/dev-template.md L64-66 already states this rule in
    English for dispatched devs, and .claude/agents/os-dev.md L92 states it in Chinese. Both are
    now backed by a hook rather than by convention alone. Neither file is touched — the published
    skills catalogue is untouched in full, and nothing here needs a twin there. Successor: any PR
    that next revises the dev template. noted, not filed.
  • The pgrep/ps coupling is whole-command, so kill "$PID" && pgrep -f node blocks though the two
    halves are unrelated. Pipeline-exact coupling needs separator bookkeeping the splitter does not
    carry; the over-block errs toward the rule and has a one-variable way out. Stated in the hook
    header as a known boundary. noted, not filed — successor: whoever hits it.
  • .claude/hooks/ and .claude/settings.json carry no line ceiling by design
    (check-skill-line-ratchet.mjs says so explicitly), which is what makes "move the narrative
    into the hook header" a real payment rather than a dodge. noted, not filed.

维护者速读(草稿)

改了什么 —— 新增一个 PreToolUse 钩子 guard-process-kill.sh(带同名 .selftest.sh
OS_ALLOW_PROCESS_KILL=1 覆盖开关、在 .claude/settings.jsonBash 匹配器里注册),它拦截
"按名字杀进程"这一类命令,放行所有"按你自己记下的 PID 杀"的写法。配套 AGENTS.md 加一行、
CLAUDE.md 把原来的 ⛔ stash 小节就地扩成"stash 栈与进程表都不受 worktree 隔离"。

为什么改 —— 容器里只有一张进程表。一次 pkill -f 会打到隔壁 agent 正在跑的门禁,而且
两边都看不见:开枪的人拿到 exit 0,中枪的人只看到进程被杀、日志被截断,没有任何线索指向邻居,
于是当成 flake 重跑。这种既不被肇事者察觉、也不被受害者归因的故障不会随经验减少。卡片来自一次
自报:PR #16120 的实现席在自己已经全绿的交付报告里主动记下这次滑坡,当时同容器有四个 agent 存活。

风险与代价(含回滚) —— 风险是误拦:钩子对 pkill/killall 是 fail-closed(不认识的选项一律拦),
且 pgrep/ps 与 kill 的耦合判断是"整条命令"而非"同一管道",所以 kill "$PID" && pgrep -f node
这种不相干的组合也会被拦。代价方向是刻意选的:宁可偶尔误拦一条,也不放过一次跨 agent 的误杀,
出口是 OS_ALLOW_PROCESS_KILL=1。自测把仓库里现存的全部 PID-scoped 写法都钉成"必须放行",
所以现有脚本不会变红。回滚成本极低:钩子只要从 .claude/settings.json 摘掉即刻失效,
两个文档改动是纯文本、可单独 revert。⚠️ 行数天花板一格不动,AGENTS.md 用掉了它仅剩的一行余量,
下一张动 AGENTS.md 的卡(#16814)将从 0 余量起步。

席位意见 ——

你要做的 —— 请只看两处:① CLAUDE.md 那 6 行的新措辞是否仍然把 stash 规则说清楚了
(合并两条规则是为了不加第五个小节、不抬天花板;若你认为进程表值得单独一节,那需要一次抬 ceiling 的裁决);
AGENTS.md 新增的那一行是否放在了对的位置(挂在"别停别人的服务器"那条之后,而不是挂在 stash 段旁)。
其余是机械可核的:门禁 18/18 全绿、消融两条腿、天花板 0 余量未抬。治理面 ⇒ 请人工合并,
⛔ 本席不翻 ready、不入队、不挂 auto-merge。


Generated by Claude Code

…orded

Worktree isolation gives each agent its own checkout, index and HEAD; it gives
it nothing over the process table, which is one table per container. A
name-matched kill reaches whatever a parallel agent is running under that name,
and it exits 0 either way — the agent that fired it observes nothing, the agent
whose run died cannot attribute it, and the loss is recorded as a flake.

The same shape as guard-shared-stash.sh, deliberately: hook + same-named
.selftest.sh + an OS_ALLOW_* escape + the Bash PreToolUse registration, with the
AGENTS.md line and the CLAUDE.md inline beside the stash rule they generalise.

Swept first, so the rule covers the class rather than one spelling: the tree
carries no pkill, no killall, no `xargs kill`, no `ps | grep | kill` and no
`fuser -k`. Every live kill site is PID-scoped — `kill "$SERVER_PID"`,
`kill $(lsof -ti tcp:PORT)`, `pgrep -s`/`pgrep -P` in publish-smoke.sh and
gen-sdui-manifest.sh — and the self-test pins all of them ALLOWED alongside the
blocked shapes, including the card's own specimen.

Neither ceiling moves: AGENTS.md 1067 -> 1068 (its one line of headroom) and
CLAUDE.md stays at 41, the stash section absorbing the second rule inside its
existing six lines. The mechanism, the class and the escape live in the hook
header, which carries no ceiling — the ratchet's own prescribed remedy.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P58euzUXCVJNwmhuPC9DXY
… unpinned

Second ablation leg: replacing check_pkill's `[ "$pidscoped" -eq 1 ] && return 0`
with an unconditional allow left the self-test green at 68/0. Bare `pkill` is
the only input that reaches that gate — every other spelling returns earlier —
so the branch had no case standing on it. It does now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P58euzUXCVJNwmhuPC9DXY
@github-actions github-actions Bot added the size/l label Sep 8, 2026
@hotlong hotlong added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 8, 2026 — with Claude
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 8, 2026
@hotlong hotlong self-assigned this Sep 8, 2026

hotlong commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

维护者速读(终稿)

席位复核记录:#16182 上的 ACCEPT 评论(skills 席,hotlong,session session_01P58euzUXCVJNwmhuPC9DXY,2026-09-08T10:18Z)。head 089173a8a

改了什么

五个文件。新增 PreToolUse 钩子 .claude/hooks/guard-process-kill.sh 及同名 .selftest.sh(69 例全绿);.claude/settings.jsonBash 匹配器里紧随 guard-shared-stash.sh 注册它;AGENTS.md 在「多 agent 协作纪律」第 8 条(别停别人的服务器)之后加一行——「一个容器一张进程表:只杀你自己记下的 PID,永不按名字」;CLAUDE.md 把 ⛔ stash 小节就地扩成「stash 栈与进程表都不受 worktree 隔离」,行数不变(41/41,最长行 116 字节),导语「四条规则」改成「这些规则」。钩子拦的是按名字选进程再杀这一类——pkillkillallpgrep 模式 | xargs killps | grep | kill;放行所有按 PID、端口、进程组、父进程、会话的写法;出口 OS_ALLOW_PROCESS_KILL=1

为什么改

容器只有一张进程表,worktree 隔离对它无效;pkill -f 不管命中谁都返回 0——肇事者看不见,受害者归不了因,只会当 flake 重跑。这与 git stash 是同一种共享对象、同一种「两边都报成功」的失败,所以照 stash 守卫的形状做,不发明第六种。卡片来自一次自报:PR #16120 的实现席在全绿之后主动记下这次滑坡,当时同一容器里有四个 agent 存活。

风险与代价(含回滚)

风险是误拦:对 pkill/killall 是 fail-closed(不认识的选项一律拦);pgrep/ps 与 kill 的耦合按整条命令判,kill "$PID" && pgrep -f node 这种不相干组合也会拦——方向刻意偏严,出口是一个环境变量。自测把树里现存 6 处 pgrep -s/-P 与所有 kill $(lsof -ti tcp:PORT) 钉为必放行,现有脚本不会变红;五个兄弟守卫的自测全绿。AGENTS.md 用掉最后 1 行余量(1068/1068),CLAUDE.md 零新增;两个上限一格不动。回滚:从 settings.json 摘掉那一条即刻失效,两处文档可单独 revert。⚠️ 下一张动 AGENTS.md 的卡(#16814)将从零余量起步。

席位意见

同意合并。席位对照树逐项核过:分支 AGENTS.md 1068 行、第 528 行即新句;CLAUDE.md 41 行、stash 小节四行 116/115/114/116 字节全在 120 上限内,原句每个子句仍在;钩子与自测在树上、settings.json 第 82 行已注册;check-governed-merges --test 五路径 exit 3;18/18 门禁绿,check:pm-skill-ratchet 两个上限未动;消融两条腿——第二条腿首跑仍绿,暴露了裸 pkill 没有用例,dev 补了钉子后才红,按发生的样子报了。⚠️ 10:16Z 读 head 的 check run:28 项 success/skipped、1 项进行中(Lint & Repo Gates),无红;合并队列会再跑一遍。

一处派发假设被 dev 依 os-dev 标准合同证伪:席位派发令写了「可用折并(fold)付行」,而 os-dev.md 规定行棘轮唯一合法货币是删除内容、折并不得为新内容买行;dev 据此拒绝折并、只用现有余量——做得对,记为席位错误。同一条也说明 #16516 决策卡的选项 B 应读作「把重复陈述折成指针(删除)」,不是重排。

你要做的

受管面(.claude/** + AGENTS.md + CLAUDE.md),按 #9495 规矩由人合:请看两处——CLAUDE.md 合并后那 6 行是否仍把 stash 规则说清;AGENTS.md 新行挂在第 8 条之后是否合适。认可就批准或直接合并——席位在授权批准落地且全绿时挂 auto-merge,⛔ 不翻 ready、不入队;本席即将收班,若您照今天的做法自己 ready + 入队,落地记录由下一位 skills 席补。只问一字:一个钩子管住按名字杀进程、两处文档各一句、上限不动,你认吗——


Generated by Claude Code

@hotlong
hotlong requested a review from os-zhuang September 8, 2026 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation needs-user-decision size/l skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

3 participants