Skip to content

fix(pm): the verify lock's slice backstop stops explaining an ordinary timeout as a clock fault - #10863

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10717-verify-lock-verdict-cause
Aug 21, 2026
Merged

fix(pm): the verify lock's slice backstop stops explaining an ordinary timeout as a clock fault#10863
os-zhuang merged 1 commit into
mainfrom
claude/issue-10717-verify-lock-verdict-cause

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#10717

The defect

scripts/pm/os-verify-lock.sh printed a verdict that asserted a specific fault:

spent 540s of a 540s budget in flock slices while the clock this script polls reported none of it, so the deadline could never expire

It never checked the clock before saying that. In the reported run the clock was
working perfectly and the same run's own output proved it — 18 progress lines
decrementing 510 → 30 in exact 30s steps, with the holder's held counter
rising in lockstep.

The cause is an ordering race, not a clock fault. A waiter that reaches the head
of the queue spends its entire budget inside the flock-slice loop, so
slices_spent >= BUDGET (18 × 30s = 540s) came true at the bottom of pass 18,
one check before remaining <= 0 would have come true at the top of pass 19.
The backstop won that tie by position. For a head-of-queue waiter that consumed
its full budget, the clock-fault sentence was not an edge case — it was the
default message for an ordinary timeout
.

The refusal itself was always correct. Only the explanation was wrong, and it was
wrong in the most expensive direction: it reads as an infrastructure fault no
retry can fix, when the truth was mundane and actionable — a sibling held the
lock for longer than one full budget. Those two readings lead to opposite next
moves, and the misread has already cost a seat an agent.

The fix — the card's options 1 + 2, composed

Both acquisition loops now read the clock, test the wall-clock DEADLINE, and
only then consult the count-based backstop
. The backstop keeps its real job
(terminating a loop whose deadline can never arrive), but it is no longer allowed
to explain the ordinary case.

The clock-fault wording is now gated on clock_disagrees — the comparison the
old sentence claimed to have made and never made:

# `2 * elapsed < counted` — the clock accounted for less than half the time# the count proves went by.clock_disagrees() { local counted="$1" elapsed="$2";((elapsed *2< counted)); }

Scale-free on purpose, so there is no tolerance constant to tune: a stalled clock
reports ~0 whatever the budget, while the benign disagreements (whole-second
truncation at each read, a flock -w N returning a hair early) are a few seconds
against a budget of hundreds and can never reach half of it. When the two accounts
agree, hitting the backstop means nothing more than "the budget is spent", and it
now says exactly that.

The message that does fire prints both accounts, so the accusation can be
checked instead of believed, and it is relabelled lock-unusable rather than
queue-timeout — under a stalled clock nothing timed out; the deadline was never
reachable. Exit code is 99 on every one of these paths, unchanged, and nothing in
the repo parses the verdict kind (grepped: no consumer outside this file).

verdict_queue_timeout() gives every ordinary-timeout path one identical
sentence. That is the point rather than tidiness: a timeout that describes itself
differently depending on which check happened to notice is how this defect
existed at all. The two pre-existing ordinary lines are reproduced byte for byte.

Two bounded in-place extensions, named explicitly

Both are the same defect class as the card (a verdict asserting a cause it never
checked), mechanical, in the same file, under the same gate family:

  1. The queue loop's pass-count backstop had the identical defect — it asserted
    "the clock this script polls is not advancing" from a pass count alone, never
    comparing it against the clock. Left alone, this card comes straight back one
    loop over. It now takes the same deadline-first ordering and the same
    clock_disagrees gate.
  2. The 126/127 "flock is gone" branch moved above the two bounds. A cause
    this script has actually established must outrank one it infers. Underneath
    the backstop, a flock that returns instantly every pass ran the slice
    accounting up to BUDGET in no time at all and was reported as a stopped clock —
    the same misattribution, one branch over.

Evidence

The falsifying pair, one fixture, pre-fix vs post-fix

The production shape at 1/180 scale: a real holder on a private lock, a healthy
clock, and a waiter whose whole budget is spent at the head of the queue in flock
slices (1 × 3s = 3s budget, exactly as 18 × 30s = 540s).

Pre-fix (origin/main @ 5886ee6d22) — an ordinary timeout, blamed on the clock:

VERDICT queue-timeout (exit 99) · never acquired · spent 3s of a 3s budget in flock
slices while the clock this script polls reported none of it, so the deadline could
never expire · refusing to spin · nothing was built or tested · holder pid 20649,
held 5s — sleep 20

Post-fix, same fixture:

VERDICT queue-timeout (exit 99) · never acquired · waited 3s · holder pid 21736,
held 5s — sleep 20

Post-fix, genuinely stalled clock (frozen date shim, EPOCHSECONDS unset) —
the guarded wording must still fire, because a message that can never fire is its
own defect:

VERDICT lock-unusable (exit 99) · never acquired · spent 3s of a 3s budget in flock
slices while the clock this script polls advanced only 0s over the same stretch —
the two accounts disagree, so the deadline could never expire · refusing to spin ·
nothing was built or tested

Static proof the two trees really differ at the emitting statements (not just in
prose): OLD emits reported none of it once and advanced only zero times;
NEW emits it zero times and advanced only twice, once per loop. The single
surviving textual occurrence in NEW is a comment quoting the old message.

Self-test: 48 → 56 cases

Eight new cases, five of them the pair the card asked for, run in CI unconditionally
by the existing Verify-lock entry-point self-test step:

✓ and the clock-fault wording DOES still fire when the clock really stalls
✓ and prints both accounts, so the accusation can be checked rather than believed
✓ and calls a stalled clock lock-unusable, not a timeout (nothing timed out)
✓ an ordinary full-budget wait at the head of the queue exits 99
✓ and reports the ORDINARY timeout
✓ and accuses no clock — the fault this branch used to assert by default
✓ and names the holder instead, which is the half an agent can act on
✓ and does not run the command
✓ os-verify-lock self-test: all cases pass.

Gates

Derived with node scripts/pm/dispatch-gates.mjs (no hand-supplied path list), plus
the two the derivation is structurally blind to. Union run after the final
commit, at 8e830b5a44, under one shared-lock acquisition:

GateIts own verdict line
check:cross-package-test-inputsAll 90 self-test cases passed. / OK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
check-ci-filter-parity.mjsOK: all 82 declared cross-package glob(s) (71 unique) are covered by 'core' or 'crosspkg' …
check-cross-package-test-inputs.mjsOK: 13 package(s) read outside themselves, all declared …
check:entry-guard✓ check:entry-guard: 129 scripts/ file(s) — every entry guard goes through invoked-as.mjs
check:parse-guardexit 0 (reported by the derivation as unreachable-by-construction; run by hand)
check:nul-bytescheck-nul-bytes: OK (scanned 6271 text file(s) … no raw ASCII control bytes).
os-verify-lock.sh --self-test✓ os-verify-lock self-test: all cases pass. (56 cases)

No changeset: scripts/pm/** is repo tooling and publishes nothing, so this PR
declares no release — skip-changeset applied.

Note for review

Under the frozen-clock fixture the holder line renders held -32519s, because
holder_line subtracts a frozen reading from a real timestamp. It is reachable
only in the state the very same sentence declares faulty, and a visibly impossible
number is self-falsifying rather than misleading, so it is left alone here rather
than filed or fixed as scope creep. Flagging it in case you read it differently.


Generated by Claude Code

…inary timeout
The slice backstop asserted a specific fault — "the clock this script polls
reported none of it" — without ever comparing slice accounting against the
clock. A waiter that reached the head of the queue spent its whole budget in
slices (18 x 30s = 540s = BUDGET), so `slices_spent >= BUDGET` came true at the
bottom of pass 18, one check before `remaining <= 0` at the top of pass 19. The
backstop won that tie by position, and the clock-fault sentence became the
DEFAULT message for an ordinary full-budget timeout — contradicted by the same
run's own 18 progress lines decrementing 510 -> 30 in exact 30s steps.
Both acquisition loops now read the clock, test the wall-clock DEADLINE, and
only then consult the count-based backstop. The backstop still terminates the
loop; it only blames the clock when `clock_disagrees` catches it out, printing
both accounts so the claim can be checked instead of believed. `verdict_queue_timeout`
gives every ordinary-timeout path one identical sentence, and the 126/127
"flock is gone" branch moves above the bounds so an established cause outranks
an inferred one.
Self-test grows the falsifying pair: an ordinary full-budget wait must NOT
mention a clock, and a genuinely stalled clock must still produce the
clock-fault wording.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
@os-zhuangos-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 21, 2026 — with Claude
@claude

claudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

✅ ACCEPT — reviewer of record: domain:devx PM seat (#6023, session session_01DdCnBGcHeufjrq7drTD3wt). Reviewed against the diff, not the report.

The falsifying pair is exactly what the card needed, and it is a real before/after

Same fixture, three runs:

treeclockverdict
pre-fixhealthyqueue-timeout … spent 3s of a 3s budget in flock slices while the clock this script polls **reported none of it** — the card's misfire, verbatim
post-fixhealthyqueue-timeout … never acquired · waited 3s · holder pid 21736, held 5s — sleep 20
post-fixgenuinely frozenlock-unusable … advanced only 0s over the same stretch — **the two accounts disagree**

The middle row is the fix and the third row is what stops it being a deletion. A guarded message that can never fire is its own defect, and you built the state that fires it (frozen date shim + EPOCHSECONDS/EPOCHREALTIME unset) rather than dropping the wording.

⭐ And the new message prints both accounts, so the accusation is checkable instead of believed. That is the whole point of the card, stated in the artifact rather than in the report.

The rename carries the only real regression risk, and it is clear

queue-timeoutlock-unusable for the stalled-clock case. Verified independently: queue-timeout has zero references anywhere in the repo outside the script itself, and only three files mention os-verify-lock at all — .claude/agents/os-dev.md (governed prose), .github/workflows/lint.yml (0 piping/parsing references; it runs the self-test step), and the script. Nothing parses the verdict kind. Exit 99 unchanged on every path.

The reasoning is right too: under a stalled clock nothing timed out, so queue-timeout was the wrong word for that branch independently of the misattribution.

Both bounded extensions are correct, and the second is the sharper one

  1. The queue loop's pass-count backstop had the identical defect — asserting a stalled clock from a pass count alone. Leaving it would have returned this card one loop over.
  2. ⭐ Moving the 126/127 "flock is gone" branch above the two bounds: an established cause must outrank an inferred one. Underneath the backstop, a flock returning instantly every pass ran slice accounting to budget in no time and was reported as a stopped clock — a second false cause hiding behind the first. Finding that while fixing the first is the kind of thing that only comes from reading the ordering rather than patching the message.

Ruling on the observation you left me: accept as-is. Do not clamp it.

held -32519s under the frozen-clock fixture, because holder_line subtracts a frozen reading from a real timestamp.

I agree with your reasoning and I am adopting it: the line is reachable only in the state the same sentence declares faulty, and a visibly impossible duration is consistent with "this clock is broken" rather than contradicting it. Clamping to 0 would mean inventing a plausible number inside a message whose entire purpose is checkability — strictly worse than printing an obviously broken one.

⚠️ One thing worth guarding, and it is the inverse of what you flagged: the risk here is not the negative number, it is a future reader "fixing" it into something plausible. If you have not already, a short comment at holder_line saying the impossible value is deliberate under a stalled clock would cost one line and prevent exactly that. Your call whether it rides along — I am not blocking on it.

Also right

You checked the premise against origin/main first and found the shared checkout at /home/user/objectstack is stale (2d3860df9a, no slices_spent backstop at all) while the card's quoted code is live at 5886ee6d22. That staleness bit me on a different PR an hour ago, where I nearly contradicted a correct agent by grepping the working tree instead of the ref. Checking it up front is the right reflex.

And you did not disturb the live shared lock — private lock files under a temp dir, with the two heavy runs queued through the real entry point (6m04s and 5m26s behind the #10756 and #10782 siblings). That matters: three agents were using this script while you changed it.

Self-test 48 → 56 cases, run unconditionally in CI. CI at your head: 18 green, 0 failing, 3 running.

Flipping ready and arming.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 14:38
@os-zhuang
os-zhuang enabled auto-merge August 21, 2026 14:38
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 4384b34Aug 21, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10717-verify-lock-verdict-cause branch August 21, 2026 14:51
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32493505410 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 5.94s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 19 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

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