Repository navigation
fix(tools): identify a wake source by process identity, not pid alone (#437) - #847
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Pullfrog stalled — likely cause: The agent stopped emitting events for 125s and was killed by the activity-timeout watchdog. 100 events were processed before the failure. Recent agent stderr
|
|
Pullfrog stalled — likely cause: The agent stopped emitting events for 120s and was killed by the activity-timeout watchdog. 100 events were processed before the failure. Recent agent stderr
|
There was a problem hiding this comment.
Important
The wake-source decision still has a PID-reuse race, and the new OS interfaces need durable evidence before this can merge.
Reviewed changes I reviewed the full six-file PR delta because no prior Pullfrog review exists; the synchronization merged redesign/main without changing that delta.
- Persisted OS start identities. Wake-source records now bind launcher PIDs to Windows start-time ticks or Linux boot-and-start-time values.
- Filtered stale registrations. The reader now rejects mismatched, legacy, malformed, inaccessible, and proven-dead wake-source records.
- Exercised wake-source lifecycle paths. The tool and hook harnesses now cover registration, identity mismatch, legacy records, dead-record sweeping, and launcher cleanup.
- Merged the current base. The synchronized merge commit brought in
redesign/mainwithout modifying the PR's six changed files.
⚠️ External interface evidence is not durable
The implementation parses new PowerShell and Linux process-metadata outputs, but the PR body contains neither the real invocations nor their complete observed shapes. The source comment that both were read cannot preserve or re-derive that evidence for review.
Technical details
# Preserve evidence for the process-identity interfaces
## Affected sites
- tools/lib/run-state.mjs:123-136 — reads PowerShell process output and Linux `/proc` fields using newly introduced formats
- PR #847 body — contains no command, complete observed output, installed-source reference, or prior-PR citation for those formats
## Required outcome
- Add independently reproducible evidence for every newly parsed external value to the PR body, including exact redacted commands and complete outputs with literal timestamp/tick formats preserved, or cite the prior PR that established each shape.GPT Sol | 𝕏
There was a problem hiding this comment.
Important
The final PID-reuse race is fixed, but the Linux interface evidence is still incomplete and must be preserved before merge.
Reviewed changes I reviewed the substantive commit added after the prior Pullfrog review at fed1fdb6:
- Rechecked final process identity. Passed each complete wake registration through
isWakeSourceAliveso the sleep decision compares the stored identity with a fresh OS observation. - Added deterministic final-probe regressions. Exercised the actual isolated Stop adapter with a matching reader observation followed by either a replaced or unreadable identity.
- Expanded external-interface and validation evidence. Recorded the complete Windows
spawnSyncresponse, Linux reproduction command, regression behavior, and current harness results in the PR body.
⚠️ Linux process response is not preserved
The PR body now gives the exact WSL command, but it summarizes the two scalar responses instead of pasting the complete observed /proc/self/stat and boot_id stdout. Without that response, the field-22 layout remains an assertion rather than durable evidence of the interface the parser consumes.
Technical details
# Preserve the complete Linux process-identity response
## Affected sites
- PR #847 body, `External interface evidence` — gives a prose description of the Linux response but no complete observed output
## Required outcome
- Paste the complete two-line observed stdout with values replaced while preserving every `/proc/self/stat` field in order and the literal boot-ID format.
- Keep the existing credential-free reproduction command beside that response.GPT Sol | 𝕏
Independent verification of both OS start-identity interfacesPullfrog asked that the Linux interface evidence be complete and preserved before merge. The Linux, through WSLCommand, run twice against separate short-lived processes: Run A, first 90 characters: Run B, first 90 characters: Applying exactly the parsing in
Why this confirms the index rather than merely being consistent with it. The two runs are The slice-after- Boot id, same host: 36 characters, matching the WindowsA bare decimal digit string, matching the One behaviour worth recording, because the implementation depends on it: the same command against a
What this does and does not establish
|
|
@pullfrog review |
There was a problem hiding this comment.
Important
The independent follow-up confirms the parsed Linux columns, but the required interface evidence is still incomplete and is not preserved in the PR body.
Reviewed changes I reviewed the full current six-file delta at bbc12545, the synchronized base commit, and the independent OS-probe evidence posted since the prior Pullfrog review.
- Bound wake registrations to OS identities. Wake records now persist Windows start-time ticks or Linux boot-and-start-time values and reject mismatched, legacy, malformed, inaccessible, or proven-dead records.
- Revalidated the final Stop decision. The rule now passes each complete registration through
isWakeSourceAlive, closing the PID-reuse race between reader validation and the decision to sleep. - Covered adapter and lifecycle paths. The hook regressions exercise changed and unreadable final observations, while the tool cases observe registration identity and cleanup around a real launcher.
- Rechecked external evidence. The Windows response shape is preserved in the PR body, and the follow-up comment independently samples the Linux fields the implementation reads.
⚠️ Linux response remains incomplete and outside the PR body
The follow-up comment labels both /proc/self/stat samples as only their “first 90 characters,” so the remainder of this positional response is still omitted; the PR body itself still contains only a prose summary. Because production indexes field 22 of this external interface, an excerpt cannot establish the complete response shape or preserve the required evidence with the implementation.
Technical details
# Preserve the complete Linux process response in the PR body
## Affected sites
- `tools/lib/run-state.mjs:132-136` — parses process state and start time from positional `/proc/<pid>/stat` fields and combines them with `boot_id`
- PR #847 body, `External interface evidence` — gives the reproduction command but no observed Linux stdout
- PR comment `#issuecomment-5562770172` — independently verifies relevant columns but explicitly truncates each stat line after 90 characters
## Required outcome
- Add an untruncated observed `/proc/self/stat` line and the complete boot-ID line to the PR body, replacing values as needed while preserving every positional field and each literal format.
- Keep the credential-free reproduction command beside that complete response.GPT Sol | 𝕏
|
@pullfrog review |
There was a problem hiding this comment.
Important
Linux dead-process states can still satisfy the new identity predicate, and the complete Linux interface response remains unpreserved.
Reviewed changes I reviewed the current six-file delta at bbc12545, the resolution of the prior final-probe race, all wake-source callers, the regression harness, and the latest PR-body evidence.
- Persisted process identities. Wake registrations now bind launcher PIDs to Windows start ticks or Linux boot-and-start-time values and filter malformed, inaccessible, legacy, and mismatched records.
- Revalidated the Stop decision.
checkSleepStopnow passes complete registrations throughisWakeSourceAlive, closing the prior PID-only decision path. - Expanded lifecycle coverage. The hook and tool harnesses exercise identity registration, changed and unreadable final observations, dead-record sweeping, and launcher cleanup.
- Preserved follow-up evidence. The PR body now includes the independently rerun Windows and Linux probe notes, including the positional Linux samples currently available.
⚠️ Linux process response is still incomplete
The independent follow-up is now preserved in the PR body, but both /proc/self/stat samples explicitly stop after their first 90 characters. Because production parses this positional external interface, omitting the remaining fields still leaves the complete response shape unverified.
Technical details
# Preserve the complete Linux process response
## Affected sites
- `tools/lib/run-state.mjs:132-136` — parses state and start time by position from `/proc/<pid>/stat`
- PR #847 body, `Independent verification of both OS start-identity interfaces` — labels both observed stat lines as only their first 90 characters
## Required outcome
- Put an untruncated observed `/proc/self/stat` line and the complete boot-ID line in the PR body, preserving every positional field and literal format.
- Keep the credential-free reproduction command beside the complete response.GPT Sol | 𝕏
|
|
@pullfrog review |
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>




Closes thomasluizon/orbit-tickets#437
Ticket: orbit-tickets #437
Wake registrations persist the OS process start identity. The final sleep decision now compares that same identity with a fresh OS observation, so a pid recycled after reader validation cannot allow an unattended run to stop. An unreadable final identity blocks sleep while work remains.
Writes still fail soft, legacy registrations still provide no wake evidence, and the Stop exit contract, run record schema, and readiness ledger are unchanged.
Review round 3
Closed P1 thread
PRRT_kwDOR5Siws6fwdyain commit568de8eda81a457c708484a24c41b5ca2807a9b0. The Linux identity probe rejects the complete documented dead-state set,Z,X, and historicalx, using a named set and a comment explaining the historical spelling. A dead observation now supplies no identity at either probe.The regression
wake identity: Linux dead states block the real Stop adapter at either proberuns six scenarios through the actual isolated Stop adapter: each ofX,x, andZat the reader, and each after a live reader observation. Only the OS stat boundary and platform selection are replaced. The pid existence probe, registration reader, identity predicate, and Stop decision remain real. It checks the observed states, probe count, exit 2, and missing-wake explanation.I saw this test red before the fix. With production code unchanged, the four
Xandxscenarios exited 0, including observations[X, X]and[x, x]; the two existingZbehaviors were controls and already blocked.The requested three observations were also completed:
Xandxnow blocked.Round 3 validation
node --check.node .claude/hooks/test-hooks.mjs: all 369 assertions passed.node tools/test-tools.mjs: completed with 1,401 passing checks and two failures, the same CPU-progress and log-progress launcher timing cases recorded in round 2. Both expected the hard-ceiling outcome. The full suite remains failed; this round changes only the Linux dead-state set and its regression.Round 3 Linux interface evidence
The proc_pid_stat(5) state table lists
Zas zombie,Xas dead, andxas dead on Linux 2.6.33 through 3.13. The complete documented character set isR S D Z T t W X x K P I;Whas two historical meanings. No other listed value denotes a dead process. The kill(2) contract confirms that signal 0 performs existence and permission checks.Re-derived the stat layout and boot-id format with these real commands on this host:
Both exited 0. Stat stdout was one record with all 52 fields, followed by a newline. This is the complete positional shape with values replaced and decimal integer types preserved; stat has no JSON keys:
State was
R; it is field 3, or index 0 after the closing command parenthesis. Starttime is field 22, or index 19 in that same slice. Boot-id stdout was a lowercase hexadecimal UUID plus newline, with this redacted literal format:The regression retains the captured stat record and mutates only its pid and state; its boot-id fixture was captured by the second command. The documented dead-state values are injected deliberately so the case is deterministic on Windows as well as Linux.
Review round 2
Closed P1 thread
PRRT_kwDOR5Siws6fvY3nin commit41c6c95df00d37972625be314a2f1d3808fcf903: the final liveness decision was pid-only.checkSleepStopnow passes the complete registration toisWakeSourceAlive, shared with the reader. The adapter and every rule caller use the new predicate contract.The regression cases run the actual isolated Stop adapter with a preloaded OS probe seam. The first observation uses the real registered identity. Only the second observation changes its start identity or throws an unreadable-identity error. Each case requires two observations, a matching reader identity, a changed final observation, exit 2, and the missing-wake explanation.
wake identity: replaced identity after reader validation blocks the real Stop adapter: I saw this test red before the fix. The unfixed adapter exited 0 after one matching observation; the fixed adapter exits 2 after two observations with different identities.wake identity: unreadable identity after reader validation blocks the real Stop adapter: I saw this test red before the fix. The unfixed adapter exited 0 after one matching observation; the fixed adapter exits 2 after the second observation becomes unreadable.The three observations were: the existing hook suite passed with the defect present, the new regression assertions failed against unchanged production code, and the hook suite including both regressions passed after the fix.
Validation from round 2
node --checkpassed for all four changed JavaScript modules.node .claude/hooks/test-hooks.mjspassed, including both regressions and the existing identity, legacy, cleanup, and Stop-contract cases.node tools/test-tools.mjscompleted with 1,376 passing assertions and two failures: the existing CPU-progress and log-progress launcher timing cases expected the hard-ceiling outcome.fed1fdb6run-state module. This points to timing sensitivity, but the full suite result remains recorded as failed. The isolated comparison was staged outside the repository and changed no test or production code.node tools/arch-map.mjsregenerated both artifacts with no diff.External interface evidence
The Windows start-time expression was executed on this machine. It returned a decimal string; its .NET type was
System.String. The final predicate reuses this existing OS probe without introducing another OS field.Reproduce the complete observed Node response with this command, which uses the probe process's own pid and contains no credentials:
Complete observed key set, with values replaced and types retained:
The probe accepts only status
0and trimmed stdout matching^\d+$; any other status or malformed output supplies no identity. Identity comparison requires exact string equality. A thrown probe also supplies no identity. The test modifies the real returnedstdout, preserving the remaining response keys, rather than inventing a response object.The Linux source was also read through
wsl -d docker-desktop -- sh -c 'cat /proc/self/stat; cat /proc/sys/kernel/random/boot_id'. Both interfaces return scalar text, with no object keys. The stat response contains the pid, parenthesized command, state, and numeric fields; field 22 is the decimal start tick count. The boot id has UUID format. The production probe and Linux test seam keep the existing field positions; the seam changes only field 22. The state check rejectsZ; other states retain the existing handling.Assumptions
Independent verification of both OS start-identity interfaces
Pullfrog asked that the Linux interface evidence be complete and preserved before merge. The
orchestrator re-ran both probes itself rather than relying on the implementing worker's report, and
this is the raw output.
Linux, through WSL
Command, run twice against separate short-lived processes:
Run A, first 90 characters:
Run B, first 90 characters:
Applying exactly the parsing in
tools/lib/run-state.mjs,stat.slice(stat.lastIndexOf(") ") + 2).trim().split(/\s+/):/procfieldfields[0]RRfields[17]11fields[18]00fields[19]1362794fields[20]17367041736704fields[21]210190Why this confirms the index rather than merely being consistent with it. The two runs are
different processes started at different times.
starttimeis the only column that moves as a starttime should (136 then 2794), while
vsizeis byte-identical at 1736704 across both, which is what aconstant per-binary field looks like for the same
cat, andnum_threadsstays 1. A wrong indexwould have to land on a field that happens to vary the same way and leave
vsizewhere it is; itdoes not.
/^\d+$/.test(fields[19])is true in both runs.The slice-after-
") "approach is also the right one for this file, because the second field is theparenthesized command and can itself contain spaces and parentheses. Splitting the whole line on
whitespace would shift every index for such a process.
Boot id, same host:
36 characters, matching the
/^[0-9a-f-]{36}$/guard in the implementation.Windows
A bare decimal digit string, matching the
/^\d+$/guard.One behaviour worth recording, because the implementation depends on it: the same command against a
pid that does not exist does not return an empty string, it throws. Observed verbatim, on a
Portuguese-locale host:
Get-Process -ErrorAction Stopfails,.StartTimeis then called on null, and PowerShell raises. Sothe probe's
result.status === 0test is what rejects a dead pid, and the regex is a second guardrather than the first. The error text is localized, so nothing may ever parse it.
What this does and does not establish
format against real output from both platforms.
starttimewithboot_idis there to cover, and which no single-session run can show.