Skip to content

fix(sessions): make failed-run history recovery idempotent - #1311

Draft
Genius229 wants to merge 1 commit into
usestrix:mainfrom
Genius229:fix/idempotent-session-recovery
Draft

Genius229 wants to merge 1 commit into
usestrix:mainfrom
Genius229:fix/idempotent-session-recovery

Conversation

@Genius229

Copy link
Copy Markdown

Summary

Closes #1310.

Make failed-run recovery idempotent without discarding messages persisted after the pre-run snapshot.

stream.to_input_list() already contains the SDK's input history. Prepending the snapshot duplicates previous messages and tool exchanges each time recovery runs. The existing test hid this because its stream stub returned only new messages.

Before After Why
Repeated recovery of two existing records produces 4, 8, then 16 records. A full replay is reconciled once; repeated recovery without new events leaves two records. Existing history is already part of the SDK replay.
A read/clear/add rewrite can overwrite concurrent messages or expose a partially rewritten SQLite session. SQLite read/compare/delete/insert runs in one BEGIN IMMEDIATE transaction, with rollback on failure. SDK writers do not acquire Strix's asyncio write lock.

Changes

  • Reconcile the full replay against the pre-run prefix and current session.
  • Preserve unambiguous late user messages by occurrence rather than globally deduplicating equal content.
  • Leave the current session unchanged if compaction or a divergent generated branch makes reconciliation ambiguous.
  • Make SQLite rewrites atomic; retain the existing locked fallback for non-SQLite session implementations.
  • Correct the existing salvage fixture to return a full input history.
  • Add regressions for repeated recovery, partial SDK persistence, late messages, divergent/later turns, rollback, transaction visibility, and real SDK failure/continuation using httpx.MockTransport.

Scope

Only four source/test files are included. This PR does not add a model-input validator, change model/provider configuration, bump versions, migrate existing scan databases, or re-execute tools. All test data is synthetic; no scan artifacts are included.

The missing-tool-output provider error motivated the investigation, but this change targets the independently reproduced recovery defect. It does not claim to explain every cause of that error.

Verification

  • Current upstream base: 84f4108195fb516d48745aa912ba8862c7360ebb.
  • With the new tests and unmodified production code: 13 failed.
  • With this patch: 78 passed for tests/test_tool_history_recovery.py, tests/test_execution.py, and tests/test_session_fd.py.
  • The focused suite passes with both openai-agents==0.19.0 / openai==2.53.0 and openai-agents==0.19.4 / openai==2.54.0, on Python 3.13.3.
  • Ruff check/format, Mypy, Bandit with project configuration, and git diff --check pass.
  • Pyright reports 864 diagnostics on both the pristine base and this branch; comparison by file, rule and normalized message finds no new diagnostics.
  • Full suite: 1762 passed, 3 failed, with no errors or skips. The same three failures reproduce on pristine HEAD in the same environment:
    • tests/test_install_script.py::test_installer_downloads_and_runs_linux_arm64_release
    • tests/test_install_script.py::test_installer_rejects_unsupported_architecture
    • tests/test_pricing.py::test_resolves_common_bare_model_names

This PR is a draft because the repository-wide checks are not fully green in this environment. No failures have been hidden with new skips or xfails. The recovery regression suite is green on both SDK versions; the failures above and the existing Pyright diagnostics are unchanged from the base.

Reproducible focused command:

uv run pytest -q tests/test_tool_history_recovery.py tests/test_execution.py tests/test_session_fd.py

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.

[BUG] Failed-run recovery duplicates existing session history

1 participant