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

test(broker): harden acceptance socket-read against empty/truncated reply (flaky main #1917) #838

New Issue
Closed
opened 2026-07-18 06:19:37 +00:00 by jason.woltje · 0 comments
jason.woltje commented 2026-07-18 06:19:37 +00:00
Owner
Copy Link

Broker acceptance harness reddens main under load — root-caused to broker-side deadline exhaustion (flaky), not a test-only read race.

UPDATE (2026-07-18): RIDER-2 resolves to branch (b), PRODUCT-SIDE. Earlier parser-only / client-retry framing is SUPERSEDED. Do NOT implement retry-to-green — it masks the broker deadline bug. Required repair = deterministic typed transport failure (no uncaught, no tolerate/retry) + bounded broker deadline semantics + real Claude/Pi fail-closed proof, with Opus SECREV mandatory.

Symptom

push/ci pipeline 1917 on main (abd2791f, WI-2 squash) test step failed with 3 uncaught exceptions, all SyntaxError: Unexpected end of JSON input thrown in Socket.<anonymous> handlers:

  • packages/mosaic/src/lease-broker/lease-broker.acceptance.spec.ts:69
  • packages/mosaic/src/mutator-gate/mutator-gate.acceptance.spec.ts:54

The identical tree passed 100% on pr/ci 1915. 1917 runs test concurrently with build+publish (heavy contention). Load-sensitive, not a guard-logic regression (1382/1385 pass; mutator-class regression matrix green).

Root cause (broker-side, confirmed)

framework/tools/lease-broker/daemon.py handle_connection() uses a single connection deadline deadline = time.monotonic() + CONNECTION_DEADLINE_SECONDS spanning all of: read_frame -> broker_lock/broker.handle (state mutation + fsync) -> sendall. The send block:

remaining = deadline - time.monotonic()
if remaining <= 0:
    return   # closes the connection WITHOUT sending any reply

Under contention the deadline can exhaust after handle() mutated+fsync'd state but before the reply is sent, so the broker closes with no reply. The client then reads empty/truncated, and both specs duplicate a request() helper that does socket.once('end', () => resolve(JSON.parse(response))) — JSON.parse('') throws synchronously inside the 'end' listener, after the Promise executor returned => uncaught exception, failing the file.

Required repair

1 — Deterministic typed transport failure (test harness). Both request() helpers must surface a typed transport failure (empty / truncated / early-close) — reject-with-context (response bytes + length), no uncaught exception, and no silent tolerate/retry that turns red into green. A test that hits the empty-reply path must fail loud and deterministic, never pass by retrying until the broker happens to reply. Dedupe to one shared helper if clean.

2 — Bounded broker deadline semantics (product-side). A completed broker.handle() must always get its reply flushed. Do not let one shared deadline drop a reply after authority state was already mutated+fsync'd. Introduce separate/extended budgets for read vs handle vs send so a processed request is never answered with a silent close. State the security reasoning.

3 — Real adapter fail-closed proof (Opus SECREV mandatory). PROVE (construct + run, not assume) that the real Claude/Pi lease adapter read-path FAIL-CLOSES on an empty/truncated reply: parse-error -> GATE_UNAVAILABLE -> deny. If empty could ever be mis-parsed/defaulted -> ALLOW, that is HIGH (this is exactly WI-2's lifecycle/auth surface). Also assess the dropped-reply TOCTOU: handle() mutates+fsyncs authority (generation bump / revoke) but the client never learns the outcome when the reply is dropped -> divergent broker/client authority state.

Integrity (load-bearing)

  • Red-first: add a test that FORCES the empty/truncated / deadline-exhaustion path and asserts the DETERMINISTIC typed-failure outcome, proving red-before-green. No false-green swallow (catch -> resolve({ok:false}) is assertion-laundering — rejected).
  • Full suite green; then a fresh main push/ci must go genuine deterministic-green (verify, don't assume).
  • Never edit tests to pass / never force-merge red / never --no-verify.

Context

  • De-flakes the shared acceptance harness -> also on WI-3 (#830) critical path (same helper). HIGH priority.
  • Milestone: Compaction-Refresh (M188). Blocks WI-2 (#829) COMPLETION-clean (merged but main-red per Constitution Gate 5) and WI-3 (#830) MERGE.
**Broker acceptance harness reddens `main` under load — root-caused to broker-side deadline exhaustion (flaky), not a test-only read race.** > **UPDATE (2026-07-18): RIDER-2 resolves to branch (b), PRODUCT-SIDE.** Earlier parser-only / client-retry framing is SUPERSEDED. Do **NOT** implement retry-to-green — it masks the broker deadline bug. Required repair = deterministic typed transport failure (no uncaught, no tolerate/retry) **+ bounded broker deadline semantics** + real Claude/Pi fail-closed proof, with **Opus SECREV mandatory**. ## Symptom push/ci pipeline 1917 on `main` (`abd2791f`, WI-2 squash) `test` step failed with **3 uncaught exceptions**, all `SyntaxError: Unexpected end of JSON input` thrown in `Socket.<anonymous>` handlers: - `packages/mosaic/src/lease-broker/lease-broker.acceptance.spec.ts:69` - `packages/mosaic/src/mutator-gate/mutator-gate.acceptance.spec.ts:54` The **identical tree** passed 100% on pr/ci 1915. 1917 runs `test` concurrently with `build`+`publish` (heavy contention). Load-sensitive, not a guard-logic regression (1382/1385 pass; mutator-class regression matrix green). ## Root cause (broker-side, confirmed) `framework/tools/lease-broker/daemon.py` `handle_connection()` uses a **single** connection deadline `deadline = time.monotonic() + CONNECTION_DEADLINE_SECONDS` spanning **all** of: `read_frame` -> `broker_lock`/`broker.handle` (state mutation + fsync) -> `sendall`. The send block: ```python remaining = deadline - time.monotonic() if remaining <= 0: return # closes the connection WITHOUT sending any reply ``` Under contention the deadline can exhaust **after** `handle()` mutated+fsync'd state but **before** the reply is sent, so the broker closes with no reply. The client then reads empty/truncated, and both specs duplicate a `request()` helper that does `socket.once('end', () => resolve(JSON.parse(response)))` — `JSON.parse('')` throws synchronously inside the `'end'` listener, after the Promise executor returned => **uncaught exception**, failing the file. ## Required repair **1 — Deterministic typed transport failure (test harness).** Both `request()` helpers must surface a **typed** transport failure (empty / truncated / early-close) — reject-with-context (`response` bytes + length), **no uncaught exception**, and **no silent tolerate/retry that turns red into green**. A test that hits the empty-reply path must fail **loud and deterministic**, never pass by retrying until the broker happens to reply. Dedupe to one shared helper if clean. **2 — Bounded broker deadline semantics (product-side).** A completed `broker.handle()` must always get its reply **flushed**. Do not let one shared deadline drop a reply after authority state was already mutated+fsync'd. Introduce separate/extended budgets for read vs handle vs send so a processed request is never answered with a silent close. State the security reasoning. **3 — Real adapter fail-closed proof (Opus SECREV mandatory).** PROVE (construct + run, not assume) that the real Claude/Pi lease adapter read-path FAIL-CLOSES on an empty/truncated reply: parse-error -> `GATE_UNAVAILABLE` -> **deny**. If empty could ever be mis-parsed/defaulted -> **ALLOW**, that is **HIGH** (this is exactly WI-2's lifecycle/auth surface). Also assess the dropped-reply **TOCTOU**: `handle()` mutates+fsyncs authority (generation bump / revoke) but the client never learns the outcome when the reply is dropped -> divergent broker/client authority state. ## Integrity (load-bearing) - Red-first: add a test that FORCES the empty/truncated / deadline-exhaustion path and asserts the DETERMINISTIC typed-failure outcome, proving red-before-green. No false-green swallow (`catch -> resolve({ok:false})` is assertion-laundering — rejected). - Full suite green; then a fresh `main` push/ci must go **genuine deterministic-green** (verify, don't assume). - Never edit tests to pass / never force-merge red / never `--no-verify`. ## Context - De-flakes the shared acceptance harness -> also on WI-3 (#830) critical path (same helper). HIGH priority. - Milestone: Compaction-Refresh (M188). Blocks WI-2 (#829) COMPLETION-clean (merged but main-red per Constitution Gate 5) and WI-3 (#830) MERGE.
jason.woltje added this to the Compaction-Refresh Mechanism (M1: Claude+Pi) milestone 2026-07-18 06:19:37 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No Label
Milestone
No items
No Milestone Compaction-Refresh Mechanism (M1: Claude+Pi)
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#838
Delete Branch "%!s()"

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?