Skip to content

agent: every MCP Events start path waits for the session's gates; restore window covers runtime-only daemons - #156

Merged
jaredLunde merged 1 commit into
mainfrom
jared/events-start-races
Oct 7, 2026
Merged

jaredLunde merged 1 commit into
mainfrom
jared/events-start-races

Conversation

@jaredLunde

Copy link
Copy Markdown
Contributor

Fixes the four audit findings on #152. Each one has a test that fails without its fix, checked by reverting the fix.

# Finding Fix Test and its failure without the fix
1 (MED) Restored runtime subscriptions started from the restore task spawned in McpEventsHub::attach, before the session's elicitation and sampling gates existed. That covers both boot restore and the restart after a panic. A question on their first poll could still be declined as "no client". One signal for every start path: Hub::questions_ready, a latch set by McpEventsHub::start (formerly start_configured). serve calls it once the gates are installed. Every subscription task (configured, restored, resubscribing) awaits it before its first request. mcp_events_nested::a_restored_runtime_subscription_asks_only_once_the_session_takes_questions. Run 1 makes a runtime subscription. Run 2 restores it with the server asking on its first poll while BEYOND_AI_AGENT_TEST_SLOW_GATE_INSTALL_MS holds the gates off. Without the wait, the question is declined and the test times out.
2 (LOW) The 503-while-restoring window was armed only when configured subscriptions existed. A daemon with only runtime subscriptions still answered 410 to early retries. The window now tracks the set of sessions the daemon starts to restore callbacks: the events session plus every runtime-subscription session, listed before any of them starts. Each session's hub calls restored(session_id) after reserving its tokens, and the window closes when the set is empty. It is still bounded at 10 min. mcp_events_receiver::with_only_runtime_subscriptions_an_early_retry_is_told_to_retry_not_to_stop gets 410 without the fix.
3 Dropping restored() survived every test, so a daemon stuck at 503 for the full 10 minutes would go unnoticed. — Both restart tests now assert that a token nothing holds gets 410 immediately after restore completes. With restored() removed, both fail (503).
4 The audit said the serve_ws.rs call-site seams would fail #151's release_seams checker. The seams were fine; the checker had a hole. simulated_delay("…SLOW_SESSION_END_MS"/"…SLOW_ATTACH_MS") sit inside #[cfg(debug_assertions)] functions. A release build of this branch contains none of the BEYOND_AI_AGENT_TEST_ names (grep on target/release/beyond-ai-agent: 0; control string: 1). The hole: any attribute within three lines above counted as gating, so #[cfg(debug_assertions)] let x = 1; "gated" a call site written after it. The checker now requires the attribute directly above the start of the statement or arm that contains the name, walking back over continuation lines. The restore seam also moved into a gated function of its own. release_seams::the_seam_check_catches_a_name_at_a_call_site gains the gated-neighbour case, which the old heuristic passes and the new one catches, and a multi-line gated statement that must still pass.

Verification

  • clippy (-D warnings, all targets), cargo fmt and dprint check are clean.
  • lib: 1594 passed.
  • All 10 mcp_events_* targets pass, plus release_seams, serve_service_session_end and serve_session_lifecycle.
  • ARCHITECTURE (MCP Events webhook section) describes the window and the start gating.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

… restore window covers runtime-only daemons

Audit of #152:

- A runtime subscription restored when its session starts (boot restore,
  and the restart after a panic) was started from the restore task spawned
  in `McpEventsHub::attach`, before the session's elicitation and sampling
  gates existed, so a question on its first poll could still be declined as
  "no client". One signal now gates them all: `Hub::questions_ready`, set by
  `McpEventsHub::start` once the gates are installed, and awaited by every
  subscription task (configured, restored, resubscribing) before its first
  request. Test: a restored runtime subscription asks on its first poll
  while the slow-gate seam holds the gates off; the question reaches the
  client. Fails (declined) without the wait.
- The 503-while-restoring window was armed only when configured
  subscriptions existed, so a daemon whose only subscriptions are runtime
  ones still answered 410 to early retries. The window now tracks every
  session the daemon starts to restore callbacks (the events session and
  each runtime-subscription session) and closes when the last has reserved
  its tokens (still bounded at 10 min). Test: a runtime-only daemon answers
  an early retry 503, not 410.
- Once restore completes, a token nothing holds is 410 at once, asserted in
  both restart tests, so a daemon stuck at 503 (restored() dropped) fails
  them.
- release_seams: the serve_ws seams named by the audit already sit in
  `#[cfg(debug_assertions)]` functions (a release build carries none of
  the names: checked with grep on the release binary). The checker's hole
  was elsewhere: any attribute within three lines above counted as gating,
  so `#[cfg(debug_assertions)] let x = 1;` gated the call site after it.
  It now requires the attribute directly above the start of the statement
  or arm the name is in; pinned with that case. The restore seam moved into
  a gated function of its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
@jaredLunde
jaredLunde merged commit 68a2608 into main Oct 7, 2026
21 checks passed
@jaredLunde
jaredLunde deleted the jared/events-start-races branch October 7, 2026 13:13
jaredLunde added a commit that referenced this pull request Oct 7, 2026
…ore; release binaries scanned for seams

Audit of #156:

- A listed restoring session could hold the 503 window for its full 10 min
  if it failed to start, ended with an error, got no events hub (its MCP
  catalog now empty), or panicked on every start. `mcp_events::restored`
  is now called on each of those terminal paths: a failed boot pin (events
  session or runtime session), a session body that ends with an error, a
  session that starts with no hub, and every panic (a restart reserves its
  own tokens again). Tests: a listed session with no events hub, and one
  whose transcript no longer opens, each close the window, so a dead token
  is 410 within moments; both stay 503 without the fix.
- No test seam ships, enforced on the artifact: `build (release)` now runs
  `mise check:release-seams`, which scans every release binary for the
  `BEYOND_AI_AGENT_TEST_` prefix and fails on any hit (a grep, seconds; no
  extra build). The source checker stays as the fast early warning. The
  scanner is pinned by a test: it fails on this crate's debug build (which
  carries the seams), on a `concat!`-assembled name, and passes a clean
  file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant