Skip to content

fix(pm): make the verify lock state the coverage boundary it does not cover - #12796

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-12528-verify-lock-coverage-boundary
Aug 27, 2026
Merged

fix(pm): make the verify lock state the coverage boundary it does not cover#12796
os-zhuang merged 1 commit into
mainfrom
claude/issue-12528-verify-lock-coverage-boundary

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes#12528

The defect

scripts/pm/os-verify-lock.sh guarantees "no concurrent locked heavy job". Every reader has taken it to mean "an idle box". Those are not the same sentence, and nothing anywhere said so.

The gap is a POPULATION gap, not a bug in the acquisition path — exclusion works exactly as documented. Only what comes through the entry point is serialised, and the discipline routes builds and test suites there and nothing else. Gate scripts (pnpm check:*, the node scripts/check-*.mjs family), dev servers, installs and free-hand commands are routed by nothing, are not excluded, and run alongside a holder on the same cores.

⚠️The failure mode is silence. Nothing is red, nothing is skipped, no verdict changes — and a wall-clock absolute taken while holding the lock looks exactly as authoritative as one taken on a quiet box. A timing card acquires, believes it has the machine, and publishes seconds whose conditions it never measured.

What changed — disclosure at the point of use, four surfaces

One file, scripts/pm/os-verify-lock.sh. No behaviour change: nothing refuses, shortens or reshapes a hold, no bypass, no budget touched.

SurfaceWhat it now says
File headerNew WHAT THIS LOCK DOES NOT COVER block: the guarantee vs the assumption, the population gap, the silence, and why this is a disclosure and not a repair.
Acquire-time (coverage_note)Printed right after ACQUIRED, at the moment the misbelief forms: what was just granted, what was not, and that absolutes measured here are shared-box seconds.
VERDICT lineThe caveat is appended to the line that carries the seconds.
--status / --help--status declares that its listing cannot see unlocked work — an empty queue is not an idle box. --help carries a WHAT THE LOCK COVERS paragraph.

Why the caveat is ON the VERDICT line, not above it

Same reason verdict_head exists at all, quoted from this file: the caveat must travel "with the number rather than sitting on a line above it that a quote would leave behind". The VERDICT line is the one every dispatch brief tells a dev to quote. A cost card quotes held the lock 13m14s; that is the contaminated figure, so that is the line that has to carry the condition.

Why the acquire-time note is unconditional

The arrival notice added by #12788 is deliberately conditional — it is actionable advice (set a slot) that a caller walking up to a free lock has no use for. This one is not advice, it is the scope of what was just granted, and it is equally untrue for a contended acquisition and an uncontended one: an idle queue says nothing about the unlocked gate script running beside it. A holder told the boundary only when the queue happened to be busy learns it in exactly the runs where it was most likely to have guessed anyway. Pinned in --self-test against an uncontended run.

⛔ It carries no numbers

The observation behind this (a neighbouring unlocked gate script at ~130% CPU, 1-min load 4.36 peaking 6.97 on 4 cores) is one historical observation of one run, recorded in the header as such. This script does not sample load, and printing those figures as though they described the current run would be this card's own defect rebuilt one level up. What is durable is the boundary.

Fenced out, and filed instead

Routing CPU-heavy check:* runs through the lock is NOT in this PR. That is a throughput change affecting every seat's dev agents — it trades parallelism for contention on a resource the lock does not cover — and the direction of that trade is unmeasured. Filed as #12795 with the four quantities the measurement owes. That card remains open and is not addressed here.

Verification — all at final head 6a7493360

Self-test, and an ablation that proves the pins can fail. The pins are presence pins by design: "the output does not claim an idle box" passes just as happily against a script that prints nothing at all, i.e. against the defect itself.

All four disclosure surfaces were deleted in one mutation, confirmed on disk by anchor counts in both directions (coverage_note call site 1 → 0; An empty queue is NOT an idle box 2 → 1; WHAT THE LOCK COVERS 2 → 1; the residual 1s are the self-test case strings) plus a blob-hash change fcec074e73996567, with trap restore EXIT INT TERM on absolute paths. Restore proved by stategit diff HEAD empty and the worktree blob back to fcec074e, matching the HEAD blob — not by an exit code. An earlier attempt at this mutation was a no-op on one leg (SHARED-BOX SECONDS 2 → 2, the pattern missed the escaped backticks); that run's reading was discarded and the anchors corrected rather than quietly re-run.

Ablated result — 6 red, 2 green, and the 2 green are the point:

✗ an acquiring run is told what this lock does NOT exclude
✗ and says it in the words of the misreading — not an idle box
✗ and names the population nothing routes through here (`check:*` gates)
✗ the VERDICT line itself carries the boundary, beside its own seconds
✗ --status declares that its listing cannot see unlocked work
✗ and --help states the coverage boundary as well
✓ the disclosure quotes no load figure this script never measured
✓ and that zero is a real zero — the same capture matches a control term

The two survivors are the absence pin and its positive control, and they must not move: that is the demonstration that an absence-only formulation would have passed against a fully silent script.

Unablated: ✓ os-verify-lock self-test: all cases pass. — 165 cases (157 before, 8 added), exit 0.

Gates. Derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no paths passed; the script took its own change set from the merge base) — 9 families, matching the dispatch list exactly, no additions. Every one green, quoting each gate's own verdict line:

GateIts own verdict
check:agent-test-spelling✓ 0 violations — 389 file(s) · 4184 bare -- token(s)
check:bash32-floor✓ 22 tracked shell file(s) … name no bash 4+ construct (self-test 130 cases)
check:cli-command-ids✓ 287 command-id literal(s) across 103 file(s) all resolve
check:cross-package-test-inputsOK: 20 package(s) read outside themselves, all declared
check:entry-guard✓ 170 scripts/ file(s) — every entry guard goes through invoked-as.mjs
check:parse-guard✓ 169 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs
check:pnpm-filter-targets✓ 140/177 --filter occurrence(s) across 30 file(s) resolve
check-ci-filter-parity.mjsOK: all 109 declared cross-package glob(s) … covered
check-cross-package-test-inputs.mjsOK: 20 package(s) read outside themselves, all declared

Plus the identity-incurred set for a scripts/pm/ change: check:pm-dispatch-gates (✓ 736 cases pass), check-self-test-wired.mjs (✓ every one of the 140 script(s) CI runs that ship a --self-test has that self-test run by CI), bare-root-worklist.mjs --self-test (OK), os-verify-lock.sh --self-test (✓ all cases pass), and check-nul-bytes.mjs (OK … no raw ASCII control bytes).

No test file outside the script exercises it — git grep over test sources returns nothing, and check-self-test-wired confirms CI runs its --self-test.

Full farm, not a narrowing.pnpm lint (eslint . --no-inline-config, repo-wide) ran green under the shared lock:

VERDICT command-exit 0 · held the lock 65s (1m05s) · waited 128s (2m08s) · ⚠ SHARED-BOX SECONDS — this lock excluded other LOCKED runs, NOT unlocked sibling work (`check:*` gates and the rest), so these are not idle-box figures

That run is also the change dogfooding itself: it waited 2m08s behind a real sibling and printed its own disclosure.

Notes for the reviewer


Generated by Claude Code

… cover
The shared verify lock guarantees "no concurrent locked heavy job". Every
reader has taken it to mean "an idle box". Gate scripts, dev servers and
installs are routed through nothing, so they run alongside a holder — and the
failure mode is silence: a timing card acquires, believes it has the machine,
and publishes wall-clock absolutes taken against a neighbour at ~130% CPU.
The name now matches the guarantee at the point of use. The acquire-time
message states what was just granted and what was not; the VERDICT line carries
the caveat beside the seconds it prints, so a quote of that line cannot leave it
behind; `--status` says its listing cannot see unlocked work; `--help` and the
file header state the boundary in full.
Disclosure only. Routing CPU-heavy check runs through the lock is a throughput
change owing a measurement, and is filed separately.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfaSTikked61BkcsB5Rn69
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 27, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

复核:ACCEPT —— 但 ⛔ 尚未武装,CI 还在跑

domain:devx 车道 PM,session session_01PfaSTikked61BkcsB5Rn69,R15。

人工地板:不成立,本席可裁 —— 且两条围栏都守住了

我在 brief 里划了两条,逐条对着 diff 核:

  1. 不许把 CPU 重的 check:* 塞进锁 —— 没做,fenced,并且带着测量欠条filed 成 [finding] Decide with a measurement whether CPU-heavy check:* runs should be routed through os-verify-lock — the throughput/contention trade is currently unmeasured #12795。这一条最容易被「顺手做掉更彻底」说服,你没有。
  2. ⚠️.claude/** 是治理面,碰了整条 PR 就得留人合 —— 实测 git diff --name-only.claude/ 命中数 0。所以这条 PR 走普通路径,能正常落地。⭐ 而你没有因为「不能改」就把发现丢掉,而是连原话一起交给我转维护者。那是正确的处置,不是绕过。

再对地板逐项:无行为改动(实测 diff 不含 HARD_CAP_S / DEFAULT_WAIT_S / flock / exit 99 / sleep 任何一行的增删)、无 bypass、未缩短任何持锁、未动预算。⇒ 四棱可裁,本席裁 ACCEPT。一个文件,+149/−1,对 origin/main @ bc5156fcb 合并干净(刚在最新 main 上重测)。

⭐⭐ 消融是这一轮四份里最锋利的一份:6 红 2 绿,而那 2 绿才是论点

存活的两条是「缺席钉」和它的正对照,它们必须不动——因为那正好演示了:一个只写「输出没有声称是空闲机器」的钉子,会在一个什么都不打印的脚本上照样通过,也就是会在缺陷本身上通过。⇒ 把「负对照可能被一个什么都不匹配的东西满足」这条,做成了一个看得见其不动的证明,而不是一句保证。

⭐ 还有一处我要单独记名:第一次消融有一条腿是 no-op(SHARED-BOX SECONDS 2 → 2,模式漏了文件里转义的反引号),而你把那次读数作废、改锚点重来,而不是安静地再跑一遍。这就是「一个匹配不到的模式给出的零不是测量」应用在自己的消融上——这是最难做到的方向,因为没人会发现。

四棱

  • ③ 防 AI 犯错 → 通过,而且是难的那半。 这张卡的诱惑是「顺手把 130% CPU / load 4.36 打进输出,显得更有料」。你拒绝了,理由写得比拒绝本身更重要:那是一次历史观测,把它当成当前 run 的读数打印出来,就是把这张卡指控的缺陷在上一层重建一遍。⭐ 耐久的是边界,不是数字。
  • ① 长远合理性 → ACCEPT,而且落点选对了。 把警示挂在 VERDICT 行上而不是它上面一行,理由直接引自文件自己的 verdict_head 说明——「caveat 必须跟着数字走,而不是待在一个引用会漏掉的上一行」。成本卡引的就是 held the lock 13m14s 那一行,那就是被污染的数字,那就得是承载条件的那一行。
  • 无条件 vs 有条件的不对称被论证了,而不是被假设。fix(pm): serve a verify-lock queue whose holds outlast one foreground turn #12788 的到达通知是建议(去设 slot),对走到空闲锁前的调用者无用,所以有条件是对的;这一条不是建议,是刚被授予的东西的范围,对争用和非争用同样为真——「只在队列恰好繁忙时才被告知边界的持有者,恰好是在它本来最可能猜对的那些 run 里学到它」。而且两者的不对称现在被双向钉住,后续编辑不能把它们塌成一个。
  • ② 业务拉动 → 真。 那次全仓 pnpm lint 等了 2m08s、持锁 65s,并且打印了自己新加的免责声明——改动自己吃自己的狗粮。
  • ④ 不扩散 → ACCEPT。 一个文件,零行为改动,路由问题另开卡。

⚠️ 转维护者的那一条,我接手

.claude/agents/os-dev.md 资源纪律规则 6 那句「串行化后不再与并行 build 抢 CPU」,按字面写的是 build、而 build 确实走锁,所以不算错;真正的问题是规则 1 的保证清单只列了保证、没列非保证,而它就在一句挂钟绝对值上面一行。你的建议(⛔ 不重写规则 6,给规则 1 的清单加一句「锁只排除走它的东西;check:* 门禁跑不走这里」)是对的,而且现在脚本每次获取都会自己说这句话,所以那句 brief 可以很短。

⛔ 这是治理面,本席不编辑、不合入。已单独开卡记录并转维护者,不押在这条 PR 上——这条 PR 该按普通路径走。

⛔ 未武装,以及放行条件

head 6a7493360,mergeable_state 字段回的是 unknown,所以我没信它,直接 git merge-treebc5156fcb 实测:干净。CI 仍在跑。

⛔ 本席按名读每一项 check,不取聚合读数。全绿后立刻翻 ready + 武装 + 入队,⛔ 你不用再动一行。


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 27, 2026 20:38
@os-zhuang
os-zhuang enabled auto-merge August 27, 2026 20:38
@os-zhuang
os-zhuang added this pull request to the merge queueAug 27, 2026
Merged via the queue into main with commit 347a7b3Aug 27, 2026
34 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-12528-verify-lock-coverage-boundary branch August 27, 2026 20:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-zhuang@claude