Skip to content

fix(codex): persist app-server turns to session DB (fixes starved recall) - #49225

Closed
lubosxyz wants to merge 1 commit into
NousResearch:mainfrom
lubosxyz:fix/codex-persist-turns
Closed

lubosxyz wants to merge 1 commit into
NousResearch:mainfrom
lubosxyz:fix/codex-persist-turns

Conversation

@lubosxyz

Copy link
Copy Markdown
Contributor

Problem

Gateway sessions using the codex_app_server runtime have no conversation memory: session_search (full-text search over state.db) and conversation-distill return empty results for all turns driven by the Codex app-server path. The agent effectively forgets everything that happened in a conversation.

Root cause

agent/codex_runtime.pyrun_codex_app_server_turn is an early-return path that bypasses conversation_loop.

The standard runtime drives every turn through conversation_loop, which calls _flush_messages_to_session_db() to persist user + assistant + tool messages to state.db. The codex app-server path returns early (before conversation_loop is entered) and never calls _flush_messages_to_session_db().

gateway/run.py — persistence block assumes the agent self-persisted.

# gateway/run.py ~line 9732
agent_persisted = self._session_db is not None   # always True
# → skip_db=True on every append_to_transcript call

The comment says "the agent already persisted these messages". That assumption is true for the standard runtime, but false for the codex app-server path. With skip_db=True, the gateway also skips writing to state.db. End result: codex turns are written to neither location.

state.db accumulates only session_meta rows — no user messages, no assistant responses, no tool calls. session_search FTS returns nothing; conversation-distill has nothing to distill.

gateway/run.pyagent_persisted flag is silently dropped in the result rebuild.

Even if run_codex_app_server_turn returned agent_persisted=False, the rebuilt result dict at the return {…} of _run_agent did not include a passthrough for that key. agent_result.get("agent_persisted", …) would always see the default.

Fix

Three backward-compatible changes:

1. agent/codex_runtime.pyrun_codex_app_server_turn success return now includes:

"agent_persisted": False,

This signals to the gateway that the codex path did NOT self-persist its turn.

2. gateway/run.py — persistence block now reads the flag instead of hard-coding True:

agent_persisted = agent_result.get("agent_persisted", self._session_db is not None)

For the standard runtime (which does not set the key), the default self._session_db is not None preserves the existing skip-db behaviour — no duplicate-write regression (#860 / #42039). For the codex app-server runtime, the flag is False, so the gateway writes the new turn's messages to state.db and the FTS index.

3. gateway/run.py — result rebuild now passes the flag through:

"agent_persisted": (result_holder[0].get("agent_persisted", True) if result_holder[0] else True),

Without this, the flag returned by run_codex_app_server_turn was discarded when _run_agent rebuilt the result dict from result_holder[0], so edit 2 would never see False.

Repro

  1. Configure a gateway profile with api_mode = "codex_app_server" (the Codex CLI runtime).
  2. Send several messages through the gateway.
  3. Call session_search with a keyword from one of those messages.
  4. Without this fix: empty results. With this fix: the turn messages are returned.

Alternatively, check state.db directly:

import sqlite3, pathlib
db = sqlite3.connect(pathlib.Path("~/.hermes/profiles/<profile>/state.db").expanduser())
rows = db.execute("SELECT role, content FROM messages ORDER BY id DESC LIMIT 10").fetchall()
print(rows)
# Before fix: only session_meta rows
# After fix:  user + assistant + tool rows present

Test plan

  • Unit: mock run_codex_app_server_turn to return {"agent_persisted": False, …}; assert that _run_agent's return dict contains "agent_persisted": False.
  • Unit: mock agent_result with agent_persisted=False and a non-None self._session_db; assert agent_persisted local variable is False (gateway writes to DB).
  • Unit: mock agent_result without agent_persisted key (standard runtime); assert agent_persisted defaults to self._session_db is not None (existing behaviour preserved).
  • Integration: run two turns through a codex app-server gateway session; assert state.db contains role=user and role=assistant rows for both turns.
  • Regression: run two turns through the standard runtime; assert each message appears exactly once in state.db (no duplicate-write regression from bug: SQLite session transcript accumulates duplicate messages (3-4x token inflation) #860 / Bug: User messages stored twice in state.db when agent and gateway both write to SQLite #42039).

🤖 Generated with Claude Code

…all)

The codex_app_server runtime path (run_codex_app_server_turn in
agent/codex_runtime.py) is an early-return that bypasses
conversation_loop and never calls _flush_messages_to_session_db().

Meanwhile, gateway/run.py sets:

  agent_persisted = self._session_db is not None   # always True

and passes skip_db=agent_persisted to every append_to_transcript call,
assuming the agent self-persisted (correct for the standard runtime,
wrong for codex). The result: codex turn messages are persisted nowhere.
state.db accumulates only session_meta rows; session_search (full-text
search over state.db) and conversation-distill are blind to real gateway
conversations, causing 'the agent has no memory of what we discussed'.

Fix (three-part, all backward-compatible):

1. agent/codex_runtime.py — run_codex_app_server_turn success return
   now includes 'agent_persisted': False, signalling that the codex path
   did NOT self-persist its turn.

2. gateway/run.py — the agent_persisted assignment now reads:

     agent_result.get('agent_persisted', self._session_db is not None)

   For the standard runtime (which does not set the key) the default
   (self._session_db is not None) preserves the existing skip-db
   behaviour so no duplicate-write regression (NousResearch#860 / NousResearch#42039) occurs.
   For the codex runtime the flag is False, so the gateway writes the
   new turn's messages to state.db and FTS index.

3. gateway/run.py — the rebuilt result dict (run_agent return, which
   becomes agent_result upstream) now includes agent_persisted passed
   through from result_holder[0], with a safe True default.  Without
   this passthrough the flag set in step 1 was discarded when the result
   was reconstructed, causing agent_result.get('agent_persisted', ...)
   to always see the default True and never write codex turns.
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery codex labels Jun 19, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Related: #38210 (root issue — codex sessions persist 0 messages), #38254 (open competitor — persists in agent/codex_runtime.py via _persist_session), #38264 (closed predecessor), #27637 and #41905 (broader codex app-server persistence/continuity work).

This PR takes a distinct mechanism from #38254: instead of persisting inside the runtime, it propagates an agent_persisted=False flag from run_codex_app_server_turn and has the gateway (gateway/run.py) perform the state.db write — preserving the standard-runtime skip-db default to avoid the duplicate-write regression (#860 / #42039). Same goal, different code site, so not a duplicate. Verified on main: gateway/run.py still hard-codes agent_persisted = self._session_db is not None, so the bug is still live.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jun 21, 2026
kshitijk4poor added a commit that referenced this pull request Jul 1, 2026
Maps the two plain-email contributors whose PRs are being salvaged so
contributor_audit.py passes:
- info@djimit.nl -> djimit (PR #48034)
- lubos@komfi.health -> lubosxyz (PR #49225)

The other two PRs in the batch (#50405 sasquatch9818, #48764 srojk34)
use users.noreply.github.com emails, which check-attribution auto-skips.
@kshitijk4poor

Copy link
Copy Markdown
Contributor

Merged via #56343 (commit 5558382) with your authorship preserved via rebase. Thanks @lubosxyz — your diagnosis and gateway wiring landed. During review I found the original agent_persisted=False approach reintroduced the #860/#42039 duplicate user-message write (the gateway agent already flushes the user turn at turn start), verified via E2E, so a follow-up commit (dc1ea00) switched to a single-writer design: the codex runtime flushes its own projected messages and returns agent_persisted=True. You are credited as co-author.

waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
Maps the two plain-email contributors whose PRs are being salvaged so
contributor_audit.py passes:
- info@djimit.nl -> djimit (PR NousResearch#48034)
- lubos@komfi.health -> lubosxyz (PR NousResearch#49225)

The other two PRs in the batch (NousResearch#50405 sasquatch9818, NousResearch#48764 srojk34)
use users.noreply.github.com emails, which check-attribution auto-skips.
Jasper6439 pushed a commit to Jasper6439/hermes-agent that referenced this pull request Jul 5, 2026
Maps the two plain-email contributors whose PRs are being salvaged so
contributor_audit.py passes:
- info@djimit.nl -> djimit (PR NousResearch#48034)
- lubos@komfi.health -> lubosxyz (PR NousResearch#49225)

The other two PRs in the batch (NousResearch#50405 sasquatch9818, NousResearch#48764 srojk34)
use users.noreply.github.com emails, which check-attribution auto-skips.
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
Maps the two plain-email contributors whose PRs are being salvaged so
contributor_audit.py passes:
- info@djimit.nl -> djimit (PR NousResearch#48034)
- lubos@komfi.health -> lubosxyz (PR NousResearch#49225)

The other two PRs in the batch (NousResearch#50405 sasquatch9818, NousResearch#48764 srojk34)
use users.noreply.github.com emails, which check-attribution auto-skips.
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
Maps the two plain-email contributors whose PRs are being salvaged so
contributor_audit.py passes:
- info@djimit.nl -> djimit (PR NousResearch#48034)
- lubos@komfi.health -> lubosxyz (PR NousResearch#49225)

The other two PRs in the batch (NousResearch#50405 sasquatch9818, NousResearch#48764 srojk34)
use users.noreply.github.com emails, which check-attribution auto-skips.
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
Maps the two plain-email contributors whose PRs are being salvaged so
contributor_audit.py passes:
- info@djimit.nl -> djimit (PR NousResearch#48034)
- lubos@komfi.health -> lubosxyz (PR NousResearch#49225)

The other two PRs in the batch (NousResearch#50405 sasquatch9818, NousResearch#48764 srojk34)
use users.noreply.github.com emails, which check-attribution auto-skips.
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
Maps the two plain-email contributors whose PRs are being salvaged so
contributor_audit.py passes:
- info@djimit.nl -> djimit (PR NousResearch#48034)
- lubos@komfi.health -> lubosxyz (PR NousResearch#49225)

The other two PRs in the batch (NousResearch#50405 sasquatch9818, NousResearch#48764 srojk34)
use users.noreply.github.com emails, which check-attribution auto-skips.
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
Maps the two plain-email contributors whose PRs are being salvaged so
contributor_audit.py passes:
- info@djimit.nl -> djimit (PR NousResearch#48034)
- lubos@komfi.health -> lubosxyz (PR NousResearch#49225)

The other two PRs in the batch (NousResearch#50405 sasquatch9818, NousResearch#48764 srojk34)
use users.noreply.github.com emails, which check-attribution auto-skips.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

4 participants