fix(#838): bound broker reply deadlines + fail-close empty-read; de-flake acceptance harness #839

2026-07-18T07:07:07Z

jason.woltje commented

2026-07-18 07:07:07 +00:00

Summary

split broker read, lock-queue, and send budgets so completed transactions receive a fresh reply flush window
return explicit fail-closed BROKER_BUSY before mutation when lock contention exceeds its bound
deduplicate acceptance socket reads and reject typed empty, truncated, malformed, or trailing-data transport failures without retry-to-green
prove real Claude and Pi gate adapters deny empty/truncated replies with GATE_UNAVAILABLE

Verification

73 Vitest files / 1,394 tests green
Python deadline, runtime adapter, launch guard, and shell gates green
per-file helper coverage: 99.35% statements/lines, 86.66% branches, 100% functions
typecheck, lint, and format gates green
terra CODE APPROVE and Opus SECREV APPROVED at reviewed head

closes #838

## Summary - split broker read, lock-queue, and send budgets so completed transactions receive a fresh reply flush window - return explicit fail-closed `BROKER_BUSY` before mutation when lock contention exceeds its bound - deduplicate acceptance socket reads and reject typed empty, truncated, malformed, or trailing-data transport failures without retry-to-green - prove real Claude and Pi gate adapters deny empty/truncated replies with `GATE_UNAVAILABLE` ## Verification - 73 Vitest files / 1,394 tests green - Python deadline, runtime adapter, launch guard, and shell gates green - per-file helper coverage: 99.35% statements/lines, 86.66% branches, 100% functions - typecheck, lint, and format gates green - terra CODE APPROVE and Opus SECREV APPROVED at reviewed head closes #838

jason.woltje added 7 commits 2026-07-18 07:07:07 +00:00

test(#838 ): define broker reply framing behavior d0d3aa163a

test(#838 ): require typed broker transport failures 6b2e4adf62

test(#838 ): reject retry masking and dropped replies b3728fdeaa

fix(#838 ): bound broker reply deadlines be7ecbd63c

test(#838 ): reject chunked garbage and redact tokens 6f0f8eecdc

fix(#838 ): validate complete broker reply frames 6ae214a4a2

test(#838 ): bound adapter failure proof

ci/woodpecker/pr/ci Pipeline was successful

Details

b5dd5963cf

jason.woltje commented

2026-07-18 07:10:05 +00:00

REVIEW-OF-RECORD — #838 / PR #839 (durable persist; pr-review gitea-comment path BROKEN #812 → posted via Gitea API + read-back)

Reviewed head (pinned): b5dd5963cfc6b57e1f4c0970ad2bc31bf16e738a — must equal pushed == CI'd == merged SHA. Any new commit invalidates this RoR (Gate-16 re-review).

Gate-16 3-principal independence: author = gpt-sol ≠ CODE reviewer = pi-terra ≠ SECREV reviewer = claude-Opus. Single Gitea account (jason.woltje) → attested by LANE, not account identity.

1) CODE review — verdict: APPROVE (reviewer: pi-terra, lane ms-rev-828, exact-head worktree @ `b5dd5963`)

Full suite green; coverage 100% lines / 89.36% branches.
Bounded deadline-repair correctness verified; deadline/reply failure paths + RIDER-1 integrity paths (read-correctly-or-fail-deterministically; no false-green swallow) verified.
Shared broker-test-client.ts dedup of both acceptance helpers verified.

2) SECURITY review (MANDATORY Opus SECREV) — verdict: APPROVED (reviewer: claude-Opus, lane ms-secrev-828, exact-head worktree @ b5dd5963; independent construct+run, NOT banked from builder/Codex self-claim)

(A) Empty/truncated read FAIL-CLOSES — PROVEN by construct+run: empty / truncated / partial / non-dict / BROKER_BUSY all → exit 2 DENY on both Claude and Pi adapters via the same mutator-gate.py binary (allow-control → exit 0; launch read-path identical → return 1 no-launch). Empty→ALLOW would have been HIGH; proven empty→DENY.
(B) Per-direction dropped-reply divergence — ALL FAIL-SAFE: grant-dropped = orphaned inert (client got nothing); revoke-dropped = broker UNVERIFIED enforced at every authorize_tool; promote-dropped = client can only UNDER-claim (authority == broker intent, never more), retry → INVALID_LEASE_TRANSITION + token consumed = no double-apply, bounded TTL ≤ 300s + observer/gen-bump revoke; gen-bump-dropped = retry idempotent, stale → STALE_GENERATION deny.
Bounded-repair adds NO new ALLOW: BROKER_BUSY = ok:false→deny; emitted BEFORE broker.handle ⇒ no partial-commit; fresh send budget removes drop-after-commit. Daemon state machine (promote/authenticate/authorize/revoke) UNCHANGED vs WI-2 (#829); only handle_connection + constants touched.
TRIPWIRE: NOT tripped. promote-lease-lost-ACK residual = FAIL-SAFE two-generals observability-gap (NOT authority-divergence, NOT ALLOW-risk, NOT retry-double-apply).

3) Residual routing (Mos ratified) — ROUTE (i)

promote-lease-lost-ACK residual = NAMED-DISCLOSED-BOUNDED-RESIDUAL owned by WI-3 (#830) D2-v5. WI-3 does not merge until a sha-pinned SPEC amendment discloses it (window + fail-safe direction + bound + backstop-assert AC that the D2-v5 "≥1 observer fires OR expiry" fail-closure catches the promoted-lease-lost-ACK case) AND the WI-3 Opus SECREV covers that amendment. #838 stays bounded (deadline-repair + empty-read fail-close + 2 helper dedups).

Merge basis: repo enable_status_check:False → pr/ci-green-on-exact-head b5dd5963 is the coordinator (Mos) merge basis, independently re-verified by Mos at the merge instant (6-check). This RoR does NOT advance the merge — merge authority is the coordinator's under Gate 13, gated on pr/ci #1918 terminal-green.

## REVIEW-OF-RECORD — #838 / PR #839 (durable persist; pr-review gitea-comment path BROKEN #812 → posted via Gitea API + read-back) **Reviewed head (pinned): `b5dd5963cfc6b57e1f4c0970ad2bc31bf16e738a`** — must equal pushed == CI'd == merged SHA. Any new commit invalidates this RoR (Gate-16 re-review). **Gate-16 3-principal independence:** author = **gpt-sol** ≠ CODE reviewer = **pi-terra** ≠ SECREV reviewer = **claude-Opus**. Single Gitea account (jason.woltje) → attested by LANE, not account identity. --- ### 1) CODE review — verdict: **APPROVE** (reviewer: pi-terra, lane ms-rev-828, exact-head worktree @ b5dd5963) - Full suite green; coverage 100% lines / 89.36% branches. - Bounded deadline-repair correctness verified; deadline/reply failure paths + RIDER-1 integrity paths (read-correctly-or-fail-deterministically; no false-green swallow) verified. - Shared `broker-test-client.ts` dedup of both acceptance helpers verified. ### 2) SECURITY review (MANDATORY Opus SECREV) — verdict: **APPROVED** (reviewer: claude-Opus, lane ms-secrev-828, exact-head worktree @ b5dd5963; independent construct+run, NOT banked from builder/Codex self-claim) - **(A) Empty/truncated read FAIL-CLOSES — PROVEN by construct+run:** empty / truncated / partial / non-dict / BROKER_BUSY all → exit 2 DENY on **both** Claude and Pi adapters via the same `mutator-gate.py` binary (allow-control → exit 0; launch read-path identical → return 1 no-launch). Empty→ALLOW would have been HIGH; proven empty→DENY. - **(B) Per-direction dropped-reply divergence — ALL FAIL-SAFE:** grant-dropped = orphaned inert (client got nothing); revoke-dropped = broker UNVERIFIED enforced at every authorize_tool; promote-dropped = client can only UNDER-claim (authority == broker intent, never more), retry → INVALID_LEASE_TRANSITION + token consumed = no double-apply, bounded TTL ≤ 300s + observer/gen-bump revoke; gen-bump-dropped = retry idempotent, stale → STALE_GENERATION deny. - **Bounded-repair adds NO new ALLOW:** BROKER_BUSY = ok:false→deny; emitted BEFORE broker.handle ⇒ no partial-commit; fresh send budget removes drop-after-commit. Daemon state machine (promote/authenticate/authorize/revoke) UNCHANGED vs WI-2 (#829); only `handle_connection` + constants touched. - **TRIPWIRE: NOT tripped.** promote-lease-lost-ACK residual = FAIL-SAFE two-generals observability-gap (NOT authority-divergence, NOT ALLOW-risk, NOT retry-double-apply). ### 3) Residual routing (Mos ratified) — **ROUTE (i)** promote-lease-lost-ACK residual = NAMED-DISCLOSED-BOUNDED-RESIDUAL owned by **WI-3 (#830) D2-v5**. WI-3 does not merge until a sha-pinned SPEC amendment discloses it (window + fail-safe direction + bound + backstop-assert AC that the D2-v5 "≥1 observer fires OR expiry" fail-closure catches the promoted-lease-lost-ACK case) AND the WI-3 Opus SECREV covers that amendment. #838 stays bounded (deadline-repair + empty-read fail-close + 2 helper dedups). --- **Merge basis:** repo `enable_status_check:False` → pr/ci-green-on-exact-head `b5dd5963` is the coordinator (Mos) merge basis, independently re-verified by Mos at the merge instant (6-check). This RoR does NOT advance the merge — merge authority is the coordinator's under Gate 13, gated on pr/ci #1918 terminal-green.

jason.woltje merged commit 8dfcf1903e into main

2026-07-18 07:15:51 +00:00

jason.woltje deleted branch fix/838-broker-acceptance-flake

2026-07-18 07:15:52 +00:00

jason.woltje referenced this issue from a commit

2026-07-18 07:15:52 +00:00

fix(#838): bound broker reply deadlines + fail-close empty-read; de-flake acceptance harness (#839)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#839