Skip to content

fix(codex): persist app-server turns to session DB, exactly once (salvage #49225) - #56343

Merged
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/49225-codex-persist
Jul 1, 2026
Merged

kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/49225-codex-persist

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Contributor

Summary

Codex app-server gateway turns now reach the session DB, so session_search (FTS) and conversation-distill can see real codex conversations. Previously the codex runtime early-returned past conversation_loop's per-step flushes, so its projected assistant/tool messages were persisted nowhere (state.db held only session_meta rows).

Salvage of #49225 by @lubosxyz, cherry-picked with authorship preserved, plus a maintainer follow-up correcting the persistence approach and adding tests.

Changes

  • agent/codex_runtime.py (contributor commit + follow-up): run_codex_app_server_turn flushes its own projected assistant/tool messages via _flush_messages_to_session_db() after splicing them, and returns agent_persisted=True.
  • gateway/run.py (contributor commit): reads agent_persisted from the runtime result at both persistence sites (default preserves standard-runtime behaviour).
  • tests/agent/test_codex_app_server_persist.py (follow-up): regression coverage.

Follow-up correction (why the approach changed)

The contributor's original diff returned agent_persisted=False and had the gateway write the codex turn. During review I found — and confirmed by E2E — that this reintroduces the exact #860/#42039 duplicate-write bug: in gateway mode the AIAgent is built with a session_db, so the inbound user turn is already flushed at turn start (turn_context._persist_session). Having the gateway then re-write the new-message slice via append_to_transcriptappend_message (a raw INSERT with no dedup) duplicates that user turn.

E2E proof (before fix): turn-start flush + gateway write → USER_TURN row count = 2.

Corrected single-writer approach: the codex runtime flushes its own projected messages (the marker-based _flush_messages_to_session_db dedups the already-persisted user turn) and reports agent_persisted=True so the gateway skips its write. Lubos's diagnosis and gateway wiring are preserved; the correction lands as a follow-up commit with Co-authored-by.

Validation

lubosxyz and others added 2 commits July 1, 2026 16:47
…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.
Follow-up correcting the salvaged fix's persistence approach to avoid a
duplicate user-message write (verified via E2E — the NousResearch#860/NousResearch#42039 bug class
the original diff aimed to avoid).

Root cause: in gateway mode the AIAgent is built WITH a session_db, so the
inbound user turn is already flushed at turn start (turn_context.
_persist_session). The original fix returned agent_persisted=False, making the
gateway re-write the whole new-message slice via append_to_transcript ->
append_message (a raw INSERT with no dedup), duplicating the already-flushed
user turn.

Corrected approach (single writer): run_codex_app_server_turn now flushes its
OWN projected assistant/tool messages via _flush_messages_to_session_db (which
dedups the already-persisted user turn through _DB_PERSISTED_MARKER) and
returns agent_persisted=True so the gateway skips its write. Net result:
session_search/distill see the full codex conversation, each message persisted
exactly once.

Adds regression coverage asserting exactly-once persistence on a real
SessionDB, agent_persisted=True, FTS visibility, and standard-runtime skip-db
behaviour preserved.

Co-authored-by: Lubos Buracinsky <lubos@komfi.health>
@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 comp/gateway Gateway runner, session dispatch, delivery codex sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P2 Medium — degraded but workaround exists labels Jul 1, 2026
@kshitijk4poor
kshitijk4poor merged commit dc1ea00 into NousResearch:main Jul 1, 2026
31 checks passed
@kshitijk4poor
kshitijk4poor deleted the salvage/49225-codex-persist branch August 5, 2026 07:10
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 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