mosaicstack/stack
1
0
Fork 0
Code Issues 85 Pull Requests Actions Packages Projects Releases 15 Wiki Activity

fix(lease-broker): recovery_runtime_unittest wait_ready() connect-probe (co-equal CI flake, cherry-pick #898) #900

Merged
Mos merged 1 commits from fix/recovery-runtime-wait-ready-race into main 2026-07-25 22:26:38 +00:00
Conversation 2 Commits 1 Files Changed 1 +6 -1
jason.woltje commented 2026-07-25 22:13:53 +00:00
Owner
Copy Link

Root cause

wait_ready() in recovery_runtime_unittest.py polled socket_path.exists() to
decide the daemon was ready to accept connections. exists() only proves the
path was bind()'d — it does not prove the daemon has called listen() yet.
Under CI load the gap between bind() and listen() widens enough that the
first client connect() lands in that window and gets ECONNREFUSED, flaking
the test.

This is the identical TOCTOU race that PR #898 fixed in
recovery_b1_adversarial_unittest.py, in the same test:framework-shell /
pnpm test chain, in this test-locked (#828/#869) fail-closed area.

Same proven form as #898

Cherry-picked the exact wait_ready() transformation from #898: replace the
existence check with a real socket.connect()-and-close probe (context
manager, per-probe settimeout(0.25), retrying on
ConnectionRefusedError/FileNotFoundError until success or the deadline).

Adapted to this file's own existing structure rather than overwritten
wholesale:

  • its own 5.0s deadline is unchanged
  • its own daemon-exited-early check (process.poll() / RuntimeError) is unchanged
  • its own exact TimeoutError("daemon did not create private broker socket") message is unchanged

Zero assertion / daemon / gate-timeout changes

  • Zero assertion changes anywhere in the file (verified: git diff origin/main -- packages/mosaic/src/lease-broker/recovery_runtime_unittest.py | grep -E '^[-+].*assert' is empty).
  • Zero daemon/source changes — only wait_ready() in this one test file was touched.
  • Zero gate-timeout relaxation.
  • recovery_b1_adversarial_unittest.py (already fixed by #898) and the p5/p6 probes (separate trailing PR) are untouched.

Verification

  • git diff origin/main --stat → exactly 1 file changed (6 insertions, 1 deletion).
  • Ran python3 -m unittest recovery_runtime_unittest -v 8x in a row from the file's directory: all green (3/3 tests each run), no ECONNREFUSED.

Closes #899

## Root cause `wait_ready()` in `recovery_runtime_unittest.py` polled `socket_path.exists()` to decide the daemon was ready to accept connections. `exists()` only proves the path was `bind()`'d — it does not prove the daemon has called `listen()` yet. Under CI load the gap between `bind()` and `listen()` widens enough that the first client `connect()` lands in that window and gets `ECONNREFUSED`, flaking the test. This is the identical TOCTOU race that PR #898 fixed in `recovery_b1_adversarial_unittest.py`, in the same `test:framework-shell` / `pnpm test` chain, in this test-locked (#828/#869) fail-closed area. ## Same proven form as #898 Cherry-picked the exact `wait_ready()` transformation from #898: replace the existence check with a real `socket.connect()`-and-close probe (context manager, per-probe `settimeout(0.25)`, retrying on `ConnectionRefusedError`/`FileNotFoundError` until success or the deadline). Adapted to this file's own existing structure rather than overwritten wholesale: - its own 5.0s deadline is unchanged - its own daemon-exited-early check (`process.poll()` / `RuntimeError`) is unchanged - its own exact `TimeoutError("daemon did not create private broker socket")` message is unchanged ## Zero assertion / daemon / gate-timeout changes - Zero assertion changes anywhere in the file (verified: `git diff origin/main -- packages/mosaic/src/lease-broker/recovery_runtime_unittest.py | grep -E '^[-+].*assert'` is empty). - Zero daemon/source changes — only `wait_ready()` in this one test file was touched. - Zero gate-timeout relaxation. - `recovery_b1_adversarial_unittest.py` (already fixed by #898) and the p5/p6 probes (separate trailing PR) are untouched. ## Verification - `git diff origin/main --stat` → exactly 1 file changed (6 insertions, 1 deletion). - Ran `python3 -m unittest recovery_runtime_unittest -v` 8x in a row from the file's directory: all green (3/3 tests each run), no ECONNREFUSED. Closes #899
jason.woltje added 1 commit 2026-07-25 22:13:54 +00:00
fix(lease-broker): recovery_runtime_unittest wait_ready() connect-probe (co-equal CI flake, cherry-pick #898)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Details
fc9feb4d08 
wait_ready() polled socket_path.exists() to decide the daemon was ready,
but exists() only proves the path was bind()'d — not that the daemon has
called listen() yet. Under CI load the gap between bind() and listen()
widens enough that the first client connect() can land in the window and
get ECONNREFUSED, flaking the suite.

PR #898 fixed the identical TOCTOU race in
recovery_b1_adversarial_unittest.py by replacing the existence check with
a real socket.connect()-and-close probe (contextmanager, per-probe
settimeout, retrying ConnectionRefusedError/FileNotFoundError). This
applies the same proven form here, adapted to this file's own wait_ready:
its existing 5.0s deadline, its daemon-exited-early check, and its exact
"daemon did not create private broker socket" TimeoutError message are
all preserved byte-for-byte.

Zero assertion changes, zero daemon/source changes, zero gate-timeout
relaxation. Only wait_ready() in this file was touched; recovery_b1 and
the p5/p6 probes are untouched (separate trailing PR).

Closes #899
jason.woltje commented 2026-07-25 22:17:27 +00:00
Author
Owner
Copy Link

Record of Review — PR #900 (recovery_runtime_unittest wait_ready() connect-probe; Closes #899)

VERDICT: APPROVE (GO conditional on CI terminal-green). Reviewer: independent subagent aa839a9e (ms-lead-reviewer), fresh clone — ≠ builder (a0a052a4, mosaic-coder). REVIEWED-HEAD: fc9feb4d0870d004c0745c369016e32420e21d5a.

  1. ★ MANDATORY zero-assertion-change gate — PASS. git diff origin/main | grep -E "^[-+].*assert" = EMPTY. Only wait_ready readiness changed (socket_path.exists → real connect probe); this file own 5.0s deadline, daemon-exited-early check, and exact TimeoutError("daemon did not create private broker socket") message byte-identical.
  2. Scope — PASS. 1 file, +6/-1; recovery_b1 + p5/p6 untouched.
  3. Correctness — PASS. Structurally identical to #898 (context-mgr, 0.25s probe timeout, same exception set) adapted to this file; readiness=post-listen.
  4. 10x green — PASS. Ran suite 10x, all green, zero ECONNREFUSED.
  5. Gates/linkage — PASS. firewall 0; body Closes #899; commit-author mosaic-coder ≠ reviewer.
    CI: pipeline 2022 (recovery_runtime IS in CI test:framework-shell chain, so green confirms the fix in CI). GO for id-11 + merge at FULL-40 fc9feb4d0870d004c0745c369016e32420e21d5a on terminal-green. Non-executor.
# Record of Review — PR #900 (recovery_runtime_unittest wait_ready() connect-probe; Closes #899) **VERDICT: APPROVE** (GO conditional on CI terminal-green). **Reviewer:** independent subagent `aa839a9e` (`ms-lead-reviewer`), fresh clone — ≠ builder (`a0a052a4`, `mosaic-coder`). **REVIEWED-HEAD:** `fc9feb4d0870d004c0745c369016e32420e21d5a`. 1. **★ MANDATORY zero-assertion-change gate — PASS.** `git diff origin/main | grep -E "^[-+].*assert"` = EMPTY. Only wait_ready readiness changed (socket_path.exists → real connect probe); this file own 5.0s deadline, daemon-exited-early check, and exact `TimeoutError("daemon did not create private broker socket")` message byte-identical. 2. **Scope — PASS.** 1 file, +6/-1; recovery_b1 + p5/p6 untouched. 3. **Correctness — PASS.** Structurally identical to #898 (context-mgr, 0.25s probe timeout, same exception set) adapted to this file; readiness=post-listen. 4. **10x green — PASS.** Ran suite 10x, all green, zero ECONNREFUSED. 5. **Gates/linkage — PASS.** firewall 0; body `Closes #899`; commit-author mosaic-coder ≠ reviewer. CI: pipeline 2022 (recovery_runtime IS in CI test:framework-shell chain, so green confirms the fix in CI). **GO for id-11 + merge at FULL-40 `fc9feb4d0870d004c0745c369016e32420e21d5a` on terminal-green. Non-executor.**
Mos approved these changes 2026-07-25 22:26:37 +00:00
Mos left a comment
First-time contributor
Copy Link

GO — Gate-16 id-11 stamp, pinned to exact FULL head fc9feb4d08.
Basis: verbatim RoR (comment 18817) — independent review APPROVE, MANDATORY zero-assertion-change gate PASSED (assert-diff empty; only the wait_ready readiness probe changed, deadline and error message byte-identical; structurally identical to the proven #898 form); CI terminal-green (2022) — and because recovery_runtime IS in the CI chain, this green is in-situ proof the fix works under the very harness it repairs. Closes #899. Vector 2 of 2: with #898 + #900 on main, the test:framework-shell chain's known TOCTOU flake class is closed. Named executor: Mos (id-11), squash pinned.

GO — Gate-16 id-11 stamp, pinned to exact FULL head fc9feb4d0870d004c0745c369016e32420e21d5a. Basis: verbatim RoR (comment 18817) — independent review APPROVE, MANDATORY zero-assertion-change gate PASSED (assert-diff empty; only the wait_ready readiness probe changed, deadline and error message byte-identical; structurally identical to the proven #898 form); CI terminal-green (2022) — and because recovery_runtime IS in the CI chain, this green is in-situ proof the fix works under the very harness it repairs. Closes #899. Vector 2 of 2: with #898 + #900 on main, the test:framework-shell chain's known TOCTOU flake class is closed. Named executor: Mos (id-11), squash pinned.
Mos merged commit 48a0c86093 into main 2026-07-25 22:26:38 +00:00
Mos deleted branch fix/recovery-runtime-wait-ready-race 2026-07-25 22:26:39 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Mos
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#900
Delete Branch "fix/recovery-runtime-wait-ready-race"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?