fix(core): resolve deferred session candidate races - #65
Merged
Conversation
A simultaneous-dial collision resolved with session_collision_kept_new tore down the existing session even while it carried a direct-call engagement, cancelling the callee's pending accept prompt and stranding the caller's outgoing call (system-test failure: accept_call rejected with "unknown accept_call request_id"). Defer the collision winner through the pending-candidate registry while the current session is engaged with the peer, and promote it once the engagement releases (or the predecessor finishes). An inbound winner with a pending incoming prompt is still honored immediately so a restarted endpoint replaces its stale predecessor. A session-stopped Goodbye during outgoing negotiation now preserves the pending outgoing ownership when a deferred candidate will re-drive the call instead of terminalizing it. Also fix SessionStatusProbe::park_connecting in the test harness: watch::Sender::send drops the value when no receiver exists, so the park was a silent no-op; send_replace stores unconditionally.
…waps The previous approach deferred the collision winner while the current session carried a call engagement, but that broke tie-break convergence: both sides could keep their own dialers, stranding each side's Hello on the peer's parked session (system-test regressions in test_call_simultaneous_dial and others). Instead, let the tie-break swap proceed and preserve the call through it: - When an outbound collision winner tears down a session with a pending accept prompt, the prompt's accept handle is parked (AcceptPromptGuard) instead of cancelled; the re-driven incoming negotiation on the replacement session adopts the same handle, so the user's prompt survives the swap without a cancel/re-prompt flicker. Inbound winners (identity restart) keep the existing cancel-and-retain semantics. - A session-stopped Goodbye during outgoing negotiation now waits briefly for a replacement session or deferred candidate before being treated as terminal; when one materializes it owns the call's resolution and the pending slot is preserved. - The hello-ack wait's transport-error arm no longer releases the pending slot for critical errors: the session-error path owns terminalization, which emits CallEnded for a genuinely lost call and skips it when a replacement installed. This also fixes a lost CallEnded when the transport error raced ahead of the peer's goodbye. Adds outbound_collision_transfers_accept_prompt_and_completes_call covering prompt survival, adoption, and call completion with no re-prompt.
…g early wait_for_stable_session_pair returned on the first poll where both ids were stable, then asserted require_a_id_change — racing an in-flight collision replacement, which routinely lands more than one 100ms poll interval after the trigger (deferred-candidate promotion waits for the predecessor session to finish). A required-but-not-yet-landed id change is not a stable end state: keep polling until it lands, bounded by the existing 30s deadline. Eliminates load-dependent flakes in deferred_collision_before_prompt_preserves_original_call, start_call_waits_for_trusted_session_attempt_and_cancellation_leaves_no_call, and aborted_deferred_collision_terminalizes_original_call_once.
Concurrent room end_call + immediate rejoin crosses the previous room generation's teardown Goodbye with the fresh join's Hello. The outgoing room negotiation treated the first Goodbye as a current-room leave and ended silently while the peer completed the leg, wedging the mesh asymmetrically (the intermittent 60s hang in concurrent_room_end_immediately_rejoins_without_direct_negotiation). Outgoing room negotiations now hold goodbyes in a 500ms grace window and complete on the peer's affirmative response. A parked accept prompt whose caller never re-drives (hangup, crash) leaked: the prompt stayed open and its retained pending generation held the call slot forever. Parked transfers now expire after the caller's offer window (HELLO_TIMEOUT), cancelling the prompt and releasing the exact generation. Tests: deterministic recreation of the stale-goodbye sequence via a parked room admission (fails pre-fix), reset/expiry coverage for parked accept prompts, transfer-registry unit tests, and a log-capture harness. Also fixes a race in active_room_retains_same_identity_candidate_until_predecessor_finishes, which gated promotion on the connection-level Connected status instead of the session-map id.
Use the target-conditional sleep/sleep_until imports (tokio on native, wasmtimer on wasm) for the room-goodbye grace deadline and the parked accept transfer reaper; tokio::time::sleep_until's Instant type does not match wasmtimer's on wasm.
…ompt restart flows Adds an expect_absent step (non-consuming, scans the actor event log) and injects identity_key_b64 so YAML scenarios can use restart_actor. - session_simultaneous_dial_then_call: start_call during the session glare; the callee's accept prompt must survive collision resolution so accept_call never fails with 'unknown accept_call request_id', and no second prompt is raised (the original system-test failure geometry) - caller_cancel_during_glare_then_room: caller hangs up mid-glare-prompt; prompt cancellation must be visible only after the pending slot is released, so an immediate room join succeeds - call_prompt_survives_session_restart: caller restarts mid-prompt; exactly one cancel, then a fresh prompt completes the call
System testing (seeded sweeps) exposed two gaps in collision-churn call flows: - An incoming direct-call negotiation never re-checked session currency after the accept prompt resolved, so an accepted call could complete its HelloAck on a session the manager had already replaced, stranding the caller until its HelloAck timeout. The still-current check that already covered rooms now covers direct calls, aborting the stale negotiation so the caller ends promptly and recovery works. - Adopted (transferred) accept prompts had no expiry: when the caller died after adoption onto a session that stayed up, the prompt waited forever. An incoming offer now expires with the caller's own negotiation timeout (HELLO_TIMEOUT), cancelling the prompt and releasing the pending slot. Deterministic recreations in call_lifecycle (both fail pre-fix), plus hardening of three pre-existing load-sensitive tests: drop the racy first-poll in start_call_waits_for_trusted_session_attempt (the unreachable peer can fail all retries within milliseconds), gate candidate-promotion asserts on the session-map id instead of the connection-level Connected status, and allow a legitimate sandwiched RoomLeave (peer's teardown goodbye beating the local end_call to the controller) in the rejoin sequence while keeping the post-rejoin window strict.
The still-current abort sent an explicit session-stopped goodbye; under sustained collision churn the caller's grace path treats each such goodbye as superseded-by-replacement and silently re-drives, so the next stale abort fed the next goodbye and the caller never terminalized (system-test call_hello_ack_timeout livelock). The stale session's connection teardown already informs the caller through its critical-error path, so the abort now only releases the pending generation.
Session churn can cancel a raised prompt before the scenario's accept lands; the YAML DSL cannot express retry, so the restart coverage moves to a Python test that accepts the first prompt still open. The original prompt's cancellation after caller restart remains strictly asserted.
…cripts on failure
The outgoing negotiation's hello-timeout, peer-terminal (EndedWith), and setup_call error branches emitted CallEnded to the frontend before releasing the pending slot, so an observer reacting to CallEnded with an immediate join_room could hit CallAlreadyActive (system-test call_timeout_then_room_join). All three now release before notifying. Also widens the hello_ack_timeout scenario window (collision re-drives extend the caller's offer legitimately) and drops an incorrect cancel-then-idle ordering assertion in the parked-prompt reset coverage (the reset drain cancels parked transfers before the slot barrier by design).
The session-churn environment on wan makes prompt/accept races dominate the behavior under test; the race logic itself is covered deterministically by the core integration suite. Matches the existing precedent of running timing-sensitive scenarios on the clean profile only.
…ation A call Hello arriving on a deferred same-identity candidate connection has no reader: the candidate parks until the predecessor session finishes, so the caller waits out its full HelloAck timeout even though the callee holds an active call with the same identity. This is the mechanism behind the system-test failure in session_simultaneous_dial_then_call (caller 'did not respond' while the callee Connected). Marked ignore until the parked candidate learns to answer.
Bug-track docs for the four fixes landed in this branch's CI iteration: the stale room-teardown goodbye grace, accept-prompt expiry/leak fixes, the no-goodbye stale-negotiation abort (livelock), and release-before-notify terminal ordering; plus a conventions doc on asserting authoritative state instead of observable proxies. Seeds CONCEPTS.md with the session/call/room collision vocabulary.
…cabulary The rename commit overwrote the existing identity-switching and session establishment entries; this restores them alongside the new collision, call, and room terms.
…y-races # Conflicts: # docs/CONCEPTS.md
…vel core logs pytest-rerunfailures was already a dependency; every collected test now carries flaky(reruns=2, only_rerun=[...]) limited to environment startup signatures (relay readiness timeout, pkarr publish timeout), so hostile network profiles losing a setup handshake retry instead of failing the run, while real assertion failures still fail immediately. The system-test workflow now runs the CLI with telepathy_core=debug, so failure artifacts capture negotiation/collision detail that info-level logs omit.
…cle logs Adds .agents/skills/system-test-artifact-analysis: artifact layout, tracing/stdout event vocabulary, analysis flows, and pitfalls (monitor re-emission vs real churn, ghost sessions, relayed death-detection latency, scenario DSL ordering). Adds two info-level diagnostics so future artifacts pinpoint negotiation stalls: outgoing_negotiation_waiting_hello_ack (after the Hello write) and session_candidate_resolution_wait (before the deferred-candidate resolution loop).
…y-races # Conflicts: # .github/workflows/system-tests.yml
Add ignored real-path regressions for issues #91 and #92. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pending outgoing calls now survive viable replacement-session collisions and terminate exactly once when every deferred replacement aborts. Accept-prompt cancellation is observable only after pending call-slot ownership is released.
Root cause
Deferred terminal intent was attached to one candidate attempt instead of the predecessor call lifecycle. A successor could replace an aborted candidate while predecessor cleanup still held the stale resolution, causing either a missing
CallEndedcallback or premature termination of a viable replacement. Prompt cancellation also notified observers before releasingPendingIncomingownership.Solution
Tests
cargo fmt --manifest-path rust/Cargo.toml --all -- --check- passed after rebase.cargo nextest run --manifest-path rust/Cargo.toml -p telepathy_core --test core_integration_test --features integration-testing call_lifecycle::- 23 passed, 0 failed.cargo nextest run --manifest-path rust/Cargo.toml -p telepathy_core terminal_resolution_survives_sequential_aborted_candidates stale_aborted_resolution_waits_for_viable_successor promoted_candidate_does_not_inherit_abort_outcome- 3 passed, 0 failed.Platforms and limitations
Rust checks ran locally on Linux. System tests require manual WSL execution and were not run locally for this PR.