Repository navigation
Responder lock: a live owner is evicted when pstart carries stray whitespace - #138
Conversation
…espace
session-bootstrap.sh reads pstart verbatim out of the lock file while
proc_lstart() trims the ps output it compares against:
proc_lstart: ps -p "$1" -o lstart= | sed 's/^ *//;s/ *$//'
read_lock: sed -n 's/^pstart=//p' "$LOCK_FILE" | head -1
So a lock whose pstart keeps ps's trailing padding compares unequal in
owner_alive(), the LIVE owner reads as dead, and the next session removes
the lock and claims it.
responder-lock.mjs already trims both sides (readResponderLock does
m[1].trim(), procStart does .trim()), so the two halves disagreed about the
same file: the MCP path refused the post because it saw a valid lock held by
another live session, while the bootstrap kept handing that lock to whoever
started next. Observed today as a session being evicted 16 s and then 12 s
after claiming, which looked like two sessions fighting and was actually one
malformed pstart.
Trims LOCK_PSTART the same way proc_lstart trims, so the bash and JS halves
agree on what a live owner looks like.
Test fails before the change ("a live owner must not be evicted over
whitespace") and passes after. Full suite 815/815.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3237402c29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const owner = startSleeper(); | ||
| const realStart = execFileSync('ps', ['-p', String(owner), '-o', 'lstart='], { encoding: 'utf8' }) | ||
| .replace(/^ +/, '').replace(/\n$/, ''); // leading trimmed, TRAILING KEPT | ||
| assert.match(realStart, / $/, 'this fixture needs ps to emit trailing padding'); |
There was a problem hiding this comment.
On Linux with procps-ng, ps -o lstart= ends immediately before the newline, so this assertion fails before exercising the fix. I reproduced this with procps-ng 4.0.4, and the repository's Node.js CI runs npm test on ubuntu-latest, meaning every matrix job can fail here. Construct the padded fixture explicitly from the trimmed start time instead of depending on platform-specific ps formatting.
Useful? React with 👍 / 👎.
The fixture built the padded pstart from ps output and asserted ps had emitted trailing spaces. macOS pads lstart and Linux does not, so CI on ubuntu failed the fixture guard itself: 'this fixture needs ps to emit trailing padding'. The guard was right to refuse - it will not pass while testing nothing - but it was guarding the wrong premise. The defect is that read_lock takes pstart verbatim from the FILE, so what must be tolerated is padding in the file, whichever tool wrote it. Padding both ends explicitly tests that on every platform. Still fails without the trim and passes with it; full suite 815/815. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
The bug
session-bootstrap.shreadspstartverbatim from the lock file, but compares it against trimmedpsoutput:ps -o lstart=emits trailing padding. So a lock file whosepstartkeeps that padding compares unequal inowner_alive(), the live owner reads as dead, and the next session deletes the lock and claims it.Why it matters: the two halves disagree
responder-lock.mjsalready trims both sides —readResponderLockdoesm[1].trim(),procStartdoes.trim(). So for the same lock file:pstartresponder-lock.mjs(enforcement)session-bootstrap.sh(claiming)That is a split brain. The symptom is a session that cannot post and cannot keep the lock:
room_postrefuses it while the bootstrap hands the lock to whoever starts next.Seen in practice today — a session was evicted 16 s after claiming, then 12 s on a retry. It looked like two sessions fighting over an unarbitrable lock. It was one malformed
pstart.The fix
Trim
LOCK_PSTARTexactly asproc_lstarttrims, so both halves agree on what a live owner looks like. One line plus the comment explaining why.Verification
The test asserts a live owner with padded
pstartis not evicted, and asserts the fixture is real (assert.match(realStart, / $/)) so it cannot silently stop testing the thing ifpsformatting changes.🤖 Generated with Claude Code