Skip to content

fix(state): self-heal missing session parent row on transcript writes - #87451

Closed
shali10 wants to merge 1 commit into
NousResearch:mainfrom
shali10:pr-a-fk
Closed

shali10 wants to merge 1 commit into
NousResearch:mainfrom
shali10:pr-a-fk

Conversation

@shali10

@shali10 shali10 commented Aug 16, 2026

Copy link
Copy Markdown

What

append_message / append_messages_batch insert into messages with a FOREIGN KEY to sessions. A missing parent row — deleted by cleanup, or never created after a transient create_session failure — raises FOREIGN KEY constraint failed and aborts the whole turn, surfacing as a misleading "session storage could not be written" warning.

Change

Ensure the parent row exists (INSERT OR IGNORE INTO sessions) before writing messages in both write paths, so a missing row can never destroy a turn.

Context

Observed on a long-running gateway where a session row vanished (cleanup race) and every subsequent transcript write failed until manual repair.

@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference, author can ignore or act on any point.

fix(state): self-heal missing session parent row on transcript writes

  • hermes_state.py (~9149-9162, ~9272-9280): the FK self-heal materializes a synthetic sessions row (source='unknown', fresh started_at, no end_reason, no messages) whenever the parent is missing. That phantom row will surface in session listings/stats and in any retention/cleanup keyed on source or started_at, and can keep alive exactly the kind of row a cleanup pass deleted. Suggest checking cursor.rowcount and logging a warning when the heal actually inserts, and stamping the row distinctly (e.g. source='self-healed') so tooling can filter it out.
  • The previous behavior aborted the turn loudly on FK failure; now the root cause (row deleted by cleanup, or a transient create_session failure) is silently absorbed. A WARNING-level log at the heal site with the session_id would make future "where did my session go" debugging much cheaper.
  • The exact same SQL string is duplicated in both _do blocks — consider extracting a small _ensure_session_row(conn, session_id) helper so the two heal paths cannot drift.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 16, 2026
append_message / append_messages_batch insert into messages with a
FOREIGN KEY to sessions; a missing parent row (deleted by cleanup or
never created after a transient create_session failure) raises FOREIGN
KEY constraint failed and aborts the whole turn, surfacing as a
misleading 'session storage could not be written' warning.

Ensure the parent row exists (INSERT OR IGNORE) before writing messages
so a missing row can never destroy a turn.
JoaoMarcos44 added a commit to JoaoMarcos44/hermes-agent that referenced this pull request Sep 26, 2026
Treat _session_db_created as a creation cache rather than permanent proof
that the row still exists. A transcript FK reject can now rebuild the
session through AIAgent's canonical metadata path and retry the batch once.

If lazy row creation itself fails, stop before the guaranteed-failing
message append. Keep raw SessionDB append semantics unchanged.

Refs NousResearch#123583
Related: NousResearch#87451
Related: NousResearch#44266
JoaoMarcos44 added a commit to JoaoMarcos44/hermes-agent that referenced this pull request Sep 26, 2026
Pin the scope boundary for NousResearch#123583: SessionDB itself still rejects a
message whose parent session does not exist. Recovery belongs to AIAgent,
where the full session identity is available.

This distinguishes the fix from the broader state-layer behavior proposed
in NousResearch#87451 and from silently dropping the write as in NousResearch#44266.
@shali10

shali10 commented Sep 26, 2026

Copy link
Copy Markdown
Author

Closing as superseded.

The maintainer triage on #123583 (teknium1, 2026-09-26) reviewed this PR directly: right instinct, wrong layer. The store cannot know the row's identity — source, model, gateway routing key, profile, parent — so INSERT OR IGNORE INTO sessions mints a source='unknown' phantom that later bookkeeping cannot tell from a real session. The agent already holds every one of those fields via _ensure_db_session, so the class fix belongs there.

Since then:

Nothing from this branch is carried into either. Thanks to @teknium1 for the review.

@shali10 shali10 closed this Sep 26, 2026
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/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists 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