fix(ci): session concurrency test flakes during child handshake#99212
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 2, 2026, 3:25 PM ET / 19:25 UTC. Summary PR surface: Tests +2. Total +2 across 1 file. Reproducibility: no. deterministic high-confidence reproduction was run in this read-only review. The source race is clear because current main waits for marker existence and immediately parses a file that the child writes directly, and the PR body reports the same JSON parse failure across unrelated CI attempts. Review metrics: none identified. Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land this exact narrow test-helper fix after required CI and maintainer review; no product/session runtime changes should be added for this flake. Do we have a high-confidence way to reproduce the issue? No deterministic high-confidence reproduction was run in this read-only review. The source race is clear because current main waits for marker existence and immediately parses a file that the child writes directly, and the PR body reports the same JSON parse failure across unrelated CI attempts. Is this the best way to solve the issue? Yes. Publishing the same marker path with a same-directory temp write and rename is the narrowest maintainable fix because it preserves the existing synchronization signal while preventing partial JSON reads. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a51b06fd4d37. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Tests +2. Total +2 across 1 file. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
Land-ready at
No runtime behavior, docs, or changelog change. |
|
Merged via squash.
|
(cherry picked from commit a004e18)
* test(sessions): publish concurrency markers atomically (#99212) (cherry picked from commit a004e18) * chore(release): refresh 2026.6.33 candidate record --------- Co-authored-by: Peter Steinberger <[email protected]>
1 participant
What Problem This Solves
Resolves a CI problem where the reply-session initialization concurrency test could read its child-process JSON readiness marker while that file was only partially written. The same
Unexpected end of JSON inputfailure reproduced across unrelated PRs #95973 and #94385.Why This Change Was Made
The child now writes each JSON marker to a same-directory temporary path and atomically renames it into place. File existence therefore remains the synchronization signal, but readers can only observe a complete JSON document; product session behavior is unchanged.
User Impact
No product behavior change. Maintainers get a deterministic concurrency test instead of unrelated PRs failing on a marker-file race.
Evidence
src/config/sessions/session-accessor.reply-init-concurrency.test.tsfailed withUnexpected end of JSON inputin fix(telegram): explain disabled plugin approval failures #95973 CI attempts 1-3 and fix(feishu): preserve button command values in fallback text and add Feishu comment guidance with callback privacy #94385 CI attempt 1.git diff --checkpassed.