Skip to content

Make the partition contract able to fail, and stop pinning a pid race - #171

Open
silverstein wants to merge 1 commit into
masterfrom
claude/partition-and-latch-honesty
Open

silverstein wants to merge 1 commit into
masterfrom
claude/partition-and-latch-honesty

Conversation

@silverstein

Copy link
Copy Markdown
Member

Two test defects. No product change.

1. The partition contract could not fail

test-runner-contract.test.js compared each project's exclude list to the same arrays vite.config.mjs imports to build it. It constrained the config and never the registries.

Measured against the old test:

mutation old new
rename a serial-native entry → its suite runs in no project 18 passed 2 failed
double-list test-runner-contract → it runs in two projects 36 passed 4 failed
drop docling-macos-supervisor from serial-native 18 passed 1 failed

The middle row is the one to look at: the old contract ran itself twice and reported green. I reproduced that independently — with the double-claim applied, the new test fails five ways and visibly executes itself twice while doing it.

Not all four projects were broken. serial-resource and source-identity have the same self-comparison shape but are safe — one has a literal oracle beside it, the other is derived from the import graph. serial-native had no oracle anywhere, which was the live hole, and the disjointness check only ran from serial-resource's side, so source-identity ∩ serial-native was never compared.

The replacement walks the repository the way Vitest globs it, resolves each project's real claim from the built config, and requires every non-native suite claimed exactly once, native suites zero times, no project empty, and no include entry naming a file that does not exist. It mutates its own inputs in-test and asserts both a double-include and a renamed entry are reported, so the resolver itself cannot go vacuous. Adds the missing serial-native oracle and full pairwise disjointness. Patterns that are neither a Vitest default nor a literal path are refused rather than silently resolving to empty.

2. A test pinned one of two documented outcomes

embedded-native-canvas-latch.test.js wrote a marker owned by deadPid() and asserted "latched". But pdfjs-subprocess.js:1105 documents in its own comment that a reused pid makes the guard answer "concurrent" instead — deliberately, failing toward retryable. The test pinned one of two correct outcomes and the OS chose which.

My suggested fix — hold the pid unreusable — does not work, and the reason is worth recording. The only POSIX way to reserve a pid is an unreaped zombie, and kill(pid, 0) succeeds on a zombie, so the product would read it as alive and answer "concurrent" — precisely the outcome to avoid.

Instead deadPid() probes before handing the number out, and assertions re-run with a fresh pid only when the check failed and the owner is now alive — exactly when "concurrent" is correct. A genuine latch break fails on the first pass, because its owner stays dead. The two force cases keep a bare deadPid() since force short-circuits before the latch is read.

Detection is unchanged: three product mutations against the guard produce identical red before and after.

Reported, not hidden

One mutation stays uncaught by either version — processIsAlive misreading another user's live process as dead. It needs a process owned by a different uid, which a test cannot reliably create without root. Pre-existing.

Full gate: 2484 passed, the only failures being the three source-identity suites on the dirty-tree guard. Both changed suites green standalone (49 tests). server/ untouched, mirrors verified identical, no oracle regeneration needed.

🤖 Generated with Claude Code

Two test defects, no product change.

The partition contract compared each project's exclude list to the same
arrays vite.config.mjs imports to build it. It constrained the config and
never the registries, so a suite could run in no project at all, or in
two, and the contract stayed green. Measured on the old test: renaming a
serial-native entry so its suite ran nowhere passed 18; also listing
test-runner-contract in serial-native so it ran twice passed 36 - it
literally executed itself twice and reported success.

serial-resource and source-identity turned out to be safe despite the
same shape, because one has a literal oracle beside it and the other is
derived from the import graph. serial-native had no oracle anywhere,
which was the live hole, and the disjointness check only ran from
serial-resource's side so source-identity against serial-native was never
compared.

Replaced with a test that walks the repository the way Vitest globs it,
resolves each project's real claim from the built config, and requires
every non-native suite to be claimed exactly once, native suites zero
times, no project empty, and no include entry naming a file that does not
exist. It mutates its own inputs in-test and asserts both a
double-include and a renamed entry are reported, so the resolver cannot
go vacuous. Added the missing serial-native oracle and full pairwise
disjointness. Patterns that are neither a Vitest default nor a literal
path are refused rather than silently resolving to nothing.

The canvas latch test wrote a marker owned by deadPid() and asserted
"latched". pdfjs-subprocess.js documents in its own comment that a reused
pid makes the guard answer "concurrent" instead, deliberately, failing
toward retryable. The test pinned one of two correct outcomes and the OS
picked which.

Holding the pid unreusable does not work and the reason is worth
recording: the only POSIX way to reserve one is an unreaped zombie, and
kill(pid, 0) succeeds on a zombie, so the product would read it as alive
and answer "concurrent" - precisely the outcome to avoid. Instead
deadPid() now probes before handing the number out, and the assertions
re-run with a fresh pid only when the check failed and the owner is now
alive, which is exactly when "concurrent" is correct. A real latch break
fails on the first pass because its owner stays dead. The two force cases
keep a bare deadPid() because force short-circuits before the latch is
read.

Both fixes cost nothing in detection: three product mutations against the
latch guard produce identical red before and after. One mutation stays
uncaught by either version - misreading another user's live process as
dead - because it needs a process owned by a different uid. Pre-existing,
recorded rather than hidden.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant