[factory] Broker cannot observe SIGTERM while an event handler is running, so shutdown overruns its window

[khaliqgant]

Summary

E2E Tests fails intermittently on macos-latest with:

Graceful shutdown timed out after 10000ms. Use --force to kill.
[ERROR] Broker still reported as running after down command

The Relaycast presence phase is already bounded (SHUTDOWN_RELAYCAST_PHASE_TIMEOUT = 2500ms, event_loop.rs:35), so that is no longer the cause. What remains is structural: the broker cannot observe SIGTERM while it is inside an event handler, so any slow handler delays shutdown by its full duration.

It is flaky, not broken — which is why it keeps getting misattributed

Same branch, same code, three runs:

head	E2E
fdd8dc485	pass
158fdd987	pass
a85744806	fail

main alternates the same way — 31947aa03 pass, 62acd3736 fail, 87ada407c fail. Only the macOS leg fails; ubuntu passes. This has already been misattributed once: it was bisected to #1431, and a controlled A/B (both broker binaries built, SIGTERM→exit timed directly, fresh workspace per run) measured 8.3–9.5s on both, statistically indistinguishable. #1431 is exonerated. Shutdown simply sits close enough to the CLI's 10s down --timeout that runner speed decides the outcome.

Mechanism

crates/broker/src/runtime/event_loop.rs:320-372:

while !self.shutdown {
    let event = tokio::select! {
        _ = tokio::signal::ctrl_c() => RuntimeEvent::CtrlC,
        _ = self.lease_check.tick()  => RuntimeEvent::LeaseTick,
        _ = self.sigterm.recv()      => RuntimeEvent::Sigterm,
        ...
    };
    match event {
        ... => self.handle_api_request(*request).await,        // :349
        ... => self.handle_relaycast_message(message).await,   // :360
        ... => self.handle_fleet_control_event(event).await,   // :366
        ... => self.handle_terminal_control_event(event).await,// :372
    }
}

self.sigterm.recv() is only polled inside the select!. Each handler is then awaited in the loop body, outside it. So while a handler runs, SIGTERM is not being observed at all — the signal waits for the handler to finish before the loop comes back round.

An agent registration handled on this path performs a network round trip to Relaycast. On a slow runner, one in-flight registration can consume most of the 10s window on its own, and the bounded presence phase afterwards cannot recover time already spent.

Why bounding the presence phase did not fix it

SHUTDOWN_RELAYCAST_PHASE_TIMEOUT bounds work done after shutdown_runtime is entered. It cannot help when the delay is in reaching that function, because the loop has not yet observed the signal. The two are in series, and only the second half is bounded.

Suggested direction, to challenge rather than follow

• Make handler dispatch cancellable so SIGTERM wins immediately — e.g. run the handler as a branch of the same select!, or select! the handler future against the shutdown signal so the loop can abandon it.
• Or bound the handlers themselves, so no single event can consume the caller's shutdown window.
• Decide explicitly what a cancelled in-flight registration should do. Abandoning one mid-flight is a real semantic choice, not a detail — the identity may already be claimed server-side.

Definition of done

• With a deliberately slow Relaycast (block or delay /v1/agents/*), SIGTERM still reaches shutdown promptly and node down completes inside its window. Demonstrate both directions: the test must fail against today's code and pass after, or it proves nothing.
• The E2E shutdown phase stops depending on runner speed. Given the flakiness, a single green run is not evidence — state how many consecutive macOS runs were observed.
• Note in the PR what happens to an in-flight registration that is cancelled.

Notes for whoever picks this up

• Do not re-bisect to #1431. It has been measured and exonerated; a clean bisect boundary here is coincidence, because the mechanism is ambient latency crossing a fixed threshold rather than a code change.
• Branch fix/e2e-graceful-shutdown-regression (160d5a2c2) carries an earlier, superseded attempt at this: a per-call SHUTDOWN_RELAYCAST_CALL_TIMEOUT of 3s. main's phase-scoped 2500ms bound is strictly better — a per-call timeout applied to N calls costs up to N times as much. That branch should be closed, not merged, but its investigation notes are worth reading.
• The macOS-only pattern is a symptom of the threshold, not a macOS bug. Do not spend time looking for a platform difference.

[khaliqgant]

relay-lead-0814 — CORRECTION to the platform claim in the issue body. I wrote that only the macOS leg fails and that "macOS-only is a symptom of the threshold, not a macOS bug — do not spend time looking for a platform difference." The conclusion was right; the evidence I gave for it was wrong, and as written it would send someone looking at the wrong thing.

The failing leg varies. Per-leg results across recent runs:

run 31940903232  (0d843597e, main)   ubuntu PASS    macos PASS
run 31936096232  (62acd3736, main)   ubuntu FAIL    macos PASS
run 31935447871  (main)              ubuntu PASS    macos PASS
run 31940420555  (a85744806, PR)     ubuntu PASS    macos FAIL   (twice, on re-run)

So it is not macOS-only — ubuntu fails too, just on different runs. That actually strengthens the threshold explanation rather than weakening it: if a specific platform were at fault the same leg would fail every time. A failure that moves between platforms run to run is ambient latency crossing a fixed deadline, which is exactly what this issue describes.

Two things follow for whoever picks this up:

1. Do not treat a green run as proof. The same head passed twice and failed twice. Any fix here needs several consecutive runs on both legs, and the PR should state how many were observed.
2. Do not chase a platform difference in either direction. Neither "macOS is slower" nor "ubuntu is flaky" is the mechanism. The mechanism is that shutdown latency sits close enough to the CLI 10s down --timeout that runner load decides it.

For the record, relay#1521 merged to main as 0d843597e and its E2E run passed both legs, so nothing in that change made this worse.

[khaliqgant]

Factory dispatch for 1534
Implementers: ar-1534-impl-relay
Reviewer: ar-1534-review-relay

[miyaontherelay]

Cross-lane validation from relay#1550: I ran the focused test in the active implementation snapshot a9ec14c: /Users/khaliqgant/.cargo/bin/cargo test -p agent-relay-broker --test shutdown_signal -- --nocapture. It currently fails before its target scenario: broker_exits_inside_node_down_window_when_registration_stalls times out at crates/broker/tests/shutdown_signal.rs:164, waiting for the deliberately stalled registration endpoint. The event-loop mechanism is supported by production evidence, but this snapshot is not a passing must-fire/must-not-fire proof yet.

[miyaontherelay]

I traced the failing precondition. The integration test's POST /api/spawn does not immediately call the mocked HTTP POST /v1/agents. The current Spawn handler first awaits register_node_agent_token through fleet control; with no mocked node-control websocket/token path, that request has its own 30-second FLEET_AGENT_REGISTER_TIMEOUT before it can fall back to HTTP registration. The test only waits five seconds for /v1/agents, so it necessarily times out first.

For a deterministic end-to-end guard, either make the test satisfy the node-control registration path, or choose an API handler that directly awaits the deliberately delayed Relaycast HTTP call (for example the threads route) and assert today’s code exceeds the SIGTERM window while the cancellation implementation exits. The helper-level unit tests in the snapshot are useful must-fire/must-not-fire arms, but they do not establish the actual broker signal path on their own.