Skip to content

fix(session-db): advance flush cursor per committed message - #21025

Closed
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/session-db-flush-cursor-12563
Closed

fix(session-db): advance flush cursor per committed message#21025
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/session-db-flush-cursor-12563

Conversation

@LeonSGP43

Copy link
Copy Markdown
Contributor

Summary

Fixes the remaining duplicate-write failure mode from #12563: when _flush_messages_to_session_db() writes several messages and one append_message() fails mid-loop, any earlier rows that already committed must not be retried on the next flush.

The current full-success path already avoids duplicate writes, and merged lazy-session creation work covers the missing-row side of the issue. This PR keeps the change narrow to the partial-failure cursor gap: advance _last_flushed_db_idx immediately after each successful append, while preserving the final success-path cursor assignment.

Why now

Under SQLite lock contention, append_message() can fail after one or more earlier messages have already committed. Previously the cursor stayed at the old value until the whole loop completed, so the next persist path retried committed rows and inflated message_count / transcript rows.

Test coverage

  • Added test_flush_advances_cursor_after_each_successful_message, which simulates database is locked on the third append and verifies the retry resumes at the third message without duplicating the first two.

Verification run locally:

scripts/run_tests.sh tests/run_agent/test_860_dedup.py
scripts/run_tests.sh tests/run_agent/test_860_dedup.py tests/run_agent/test_compression_persistence.py tests/run_agent/test_413_compression.py tests/run_agent/test_compress_focus_plugin_fallback.py
python -m py_compile run_agent.py
git diff --check

Results:

  • 10 passed, 4 warnings
  • 31 passed, 4 warnings
  • py_compile passed
  • git diff --check passed

Fixes #12563

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists labels May 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to closed PR #19407 (same fix — advance flush cursor per-message). That PR was closed because the author believed the change was already in main, but inspection shows main still only advances the cursor after the full loop completes. This PR is a valid re-implementation of the same fix.

@LeonSGP43

Copy link
Copy Markdown
Contributor Author

Packaging follow-up as of 2026-05-07.

  • This PR stays narrowly on #12563: the partial-failure cursor gap in _flush_messages_to_session_db(). It does not reopen lazy-session creation or broader session-db behavior.
  • The overlap note on closed PR fix(session-db): advance flush cursor per-message to prevent duplicate writes on partial failure #19407 is acknowledged. This branch is still useful because main does not yet advance _last_flushed_db_idx after each successful append; the added regression test here covers that exact retry boundary.
  • Lint (ruff + ty) is red for workflow reasons, not a new lint failure in this diff: the job reached its PR-comment step and then failed with 403 Resource not accessible by integration while trying to post the summary. That same summary reported 0 new Ruff issues, and the ty diagnostics in this workflow are surfaced as warnings.
  • The red GitHub Tests / test run (25476014531) failed in a broad repo-wide set of ACP/gateway/Slack/tooling cases rather than the targeted session-db flush tests added here. I have not reproduced a diff-attributed regression from this PR's changed-area coverage.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression analysis. This is already implemented on current main by a later, stronger persistence redesign.

  • Automated hermes-sweeper review verified run_agent.py:1848 skips messages carrying the durable persistence marker, and run_agent.py:1905 stamps that marker immediately after each successful append_message() call. A later append failure therefore cannot cause already committed rows to be retried.
  • Commit e4c6d1b22bd33e3a180099d0132e16c0ef775b67 (fix(agent): persist messages by intrinsic marker to stop id() reuse data loss) introduced this mechanism and superseded the positional flush-cursor approach in this PR.
  • The fix shipped in v2026.7.1.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] create_session 静默失败 + 消息重复写入 — 会话从 state.db 消失

3 participants