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

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

Merged
jason.woltje merged 7 commits from fix/838-broker-acceptance-flake into main 2026-07-18 07:15:51 +00:00
Conversation 1 Commits 7 Files Changed 10 +595 -51
jason.woltje commented 2026-07-18 07:07:07 +00:00
Owner
Copy Link

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
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Details
b5dd5963cf
jason.woltje commented 2026-07-18 07:10:05 +00:00
Author
Owner
Copy Link

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
Sign in to join this conversation.
Reviewers
No Reviewers
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
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#839
Delete Branch "fix/838-broker-acceptance-flake"

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?