Skip to content

fix(gateway): make in-flight session recovery crash-safe - #67078

Open
StellarisW wants to merge 9 commits into
NousResearch:mainfrom
StellarisW:codex/gateway-session-recovery
Open

fix(gateway): make in-flight session recovery crash-safe#67078
StellarisW wants to merge 9 commits into
NousResearch:mainfrom
StellarisW:codex/gateway-session-recovery

Conversation

@StellarisW

@StellarisW StellarisW commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Critical availability issue: Under high-concurrency gateway workloads, a forced restart can strand otherwise recoverable sessions or leave already-generated replies undelivered. One restart can therefore stall many active conversations at once and require manual user intervention.

What does this PR do?

Makes in-flight gateway recovery crash-safe and deterministic across process death, drain timeouts, and delivery ambiguity.

Root causes

The previous recovery path had several independent gaps:

  1. in-flight work was inferred from a short routing-activity window instead of a durable per-run identity;
  2. the triggering platform message ID was persisted too late to prove which durable user request belonged to the interrupted run;
  3. startup recovery could replay sessions before process/delegation consumers and pending outbound replies were reconciled;
  4. the recovery branch accidentally stopped invoking the delivery-ledger sweep and retained a call to a removed startup method;
  5. a controlled drain-timeout interruption looked identical to unknown assistant delivery, so safe sessions paused instead of continuing; and
  6. rejected recovery inbox claims could remain at the FIFO head and block later user turns.

Implementation

  • Adds an atomic sessions/.active_runs.json journal with session_key, random run_id, trigger message ID, phase, recovery attempts, and controlled-interruption metadata. It stores no user message content.
  • Persists the raw inbound platform message ID during the agent's first turn-start SQLite flush, before model or tool execution.
  • Classifies recovery conservatively: durable requests and unfinished read-only work can resume; missing trigger identity, uncertain external side effects, or exhausted retry budgets pause for user input.
  • Marks all turns interrupted by a drain timeout in one atomic journal write. This avoids one fsync per active session under high concurrency.
  • Starts recovered process/delegation consumers first, redelivers durable outbound obligations second, and only then schedules session replay.
  • Adds a nullable run_id to delivery_obligations through a backward-compatible additive migration. A confirmed redelivery CAS-clears only the matching active run, so an old reply cannot clear a newer task.
  • Keeps failed redeliveries recoverable and recovery-paused. Resume state is cleared only after the platform confirms a successful send.
  • Removes the stale _finish_startup_restore() call that could abort startup after recovery consumers were initialized.
  • Preserves per-session preclaims and bounded recovery concurrency, and releases rejected durable-inbox claims so later explicit instructions are not permanently queued.
Recovered state Behavior
Controlled gateway interruption, durable trigger, no uncertain side effect Auto-resume
Pending final reply with matching run_id Redeliver first; clear recovery state only after confirmed delivery
Ambiguous mid-send reply Redeliver with a visible recovered-reply marker
Active recovered background process Wait for its watcher; do not relaunch duplicate work
Dangling or interrupted side-effecting tool Pause and notify; never replay blindly
Trigger request missing or identity unavailable Ask the user to resend
Three interrupted recovery boots Pause and notify

Safety boundaries

  • No exactly-once guarantee is claimed for arbitrary external systems.
  • Uncertain side-effecting tools remain human-gated.
  • Legacy delivery rows without run_id are still deliverable, but cannot clear a currently journaled run without exact identity.
  • Failed ledger or journal maintenance remains best-effort and cannot block the primary platform send path.

Type of Change

  • Bug fix
  • Tests
  • Feature
  • Security fix
  • Documentation-only change

Validation

Validated after rebasing linearly onto main@c4f5a45d5d9903998fb318ac6f3c5e6623e60445:

  • Recovery, delivery-ledger, drain-timeout, producer, and Slack ignored-channel matrix: 169 passed, 0 failed.
  • Broader startup/shutdown/reconnect matrix: 109 passed, 0 failed; one pre-existing timing-sensitive startup-race test passed on automatic retry and then passed 5/5 in an isolated rerun.
  • Full gateway config test file after correcting its stale short-key fixture to meet the current 16-character API-server security gate: 149 passed, 0 failed.
  • ruff check on all touched recovery files and tests: passed.
  • Python bytecode compilation and git diff --check: passed.
  • Active-run fault injection still verifies that a SIGKILL leaves exactly one durable recoverable run.
  • All nine PR commits are signed and the history is linear with no merge commits.

Compatibility and rollback

  • The delivery-ledger change is an additive nullable SQLite column migration; existing rows remain valid.
  • Older versions ignore the active-run sidecar, which contains no message text.
  • Corrupt or unsupported sidecar data fails safe without preventing gateway startup.
  • Rollback is a normal revert of this PR; no config key or public command schema is introduced.

Checklist

  • Conventional, signed commits
  • Regression and fault-injection coverage
  • High-concurrency shutdown path avoids per-session journal fsync
  • Uncertain side effects remain fail-closed
  • Existing PR refreshed onto current upstream main
  • Full repository suite locally; GitHub CI is running on this head

Infographic

Crash-safe gateway session recovery flow and safety boundaries

Final head

65fac02e2

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages needs-decision Awaiting maintainer decision before any implementation labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #8143 and #23314. This PR uses an active-run journal with phase-aware delivery and side-effect recovery decisions, rather than their checkpoint-suspension or blank-replay mechanisms; maintainer selection is needed.

@StellarisW
StellarisW marked this pull request as ready for review July 18, 2026 19:58
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused recovery work. The reported premise is verified on current origin/main: gateway/run.py:7311-7320 skips crash inference when .clean_shutdown exists, gateway/session.py:2286-2319 retains the 120-second activity heuristic, and gateway/run.py:7337-7344 / 9723-9729 impose the global inbound restore barrier. The PR addresses those boundaries with exact active-run state and per-session recovery handling.

GitHub reports the branch CLEAN/MERGEABLE, and its base e45d12642d5d0753e492d51be17cccf687aa8b06 is an ancestor of current origin/main; salvage should be mechanical. The diff includes lifecycle, recovery classification, restart, and session-boundary tests. No verified blocking issue was found in this review.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit area/sessions Session lifecycle, resume, persistence, history labels Jul 19, 2026
@StellarisW
StellarisW force-pushed the codex/gateway-session-recovery branch 3 times, most recently from 5afd52a to a192c9c Compare July 23, 2026 19:18
@StellarisW
StellarisW force-pushed the codex/gateway-session-recovery branch from a192c9c to 16dc0f8 Compare July 23, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants