fix(codex): preserve transcript cursors across commit and compaction - #4191
Open
t0saki wants to merge 2 commits into
Open
fix(codex): preserve transcript cursors across commit and compaction#4191t0saki wants to merge 2 commits into
t0saki wants to merge 2 commits into
Conversation
…urns after resume or transcript compaction Fixes volcengine#4058 Ref: volcengine#4058
Preserving the transcript cursor after a commit stops the replay, but it also means nothing deletes state files any more: clearState() lost its last caller, so every codex session — including ones that never captured a turn — leaves a file behind, and listStates() reads all of them on every SessionStart. The sweep now retires cursor-only states in the same pass: a real cursor is kept for resume until OPENVIKING_CODEX_COMMITTED_TTL_MS (default 30 days, past the life of the codex rollout it indexes), and a state that never captured anything goes on the idle schedule, which is what the old sweep did with it. Releasing ovSessionId also wrote lastUpdatedAt, making a committed session look freshly active; saveState() takes touch:false so the field keeps meaning "last transcript activity" for both the active window and retention. Requiring a live ovSessionId to count as recently-active made the heuristic miss sessions PreCompact had just committed, which can still be running: the count is back on activity alone, and only a state with a live session is committed. Also name the shrink predicate: role === "user" covers tool results too (normalizeCaptureRole maps them onto the user role), so findLastHumanTurnIndex requires a text part, and the no-human-turn fallback to a full replay is now visible in the log instead of silent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Builds on @7487's fix in #4113 (cherry-picked as the first commit, authorship preserved) and closes the gaps that fix opened.
The original bug: the Codex plugin tracks capture progress with a numeric
capturedTurnCount, and two paths threw it away.commitAndClear()insession-start-commit.mjsdeleted the whole state file after a successful commit, so resuming the same codexsession_id— whichDESIGN.mddocuments as keeping the same id and the same transcript — recreated the cursor at0and re-posted the entire history to the same deterministiccx-<codex-session-id>OV session. Andauto-capture.mjsreset the cursor to0whenever the transcript shrank, so a/compactrewrite re-uploaded the compacted summary plus the retained tail. This contradicted the plugin's own model:PreCompactalready releasesovSessionIdwhile keeping the cursor, andDESIGN.md("Commit-then-resume", "State file schema") definesovSessionId: nullas "committed, awaiting next Stop". The SessionStart path was the one place that didn't follow it.#4113 fixes both paths, and the second commit here addresses what that leaves behind:
Nothing deletes state files any more. With both sweep call sites requiring a live
ovSessionId,clearState()lost its last caller and theclear_no_ovbranch became unreachable. Sincemain()writes a state file for every newsession_idat SessionStart, even sessions that never capture a turn now leak one file forever, andlistStates()reads every file on every SessionStart. The sweep now retires cursor-only states in the same pass: a real cursor is kept for resume untilOPENVIKING_CODEX_COMMITTED_TTL_MS(default 30 days, past the life of the codex rollout it indexes), and a state that never captured anything goes on the idle schedule — which is exactly what the old sweep did with it.Releasing the session id bumped
lastUpdatedAt. A committed session looked freshly active to the next SessionStart, and the field no longer meant "last transcript activity" — which both the active window and the new retention rule depend on.saveState()now takestouch: falsefor writes that aren't transcript activity.Cursor-only sessions stopped counting as concurrent. Requiring a live
ovSessionIdto be "recently active" hides a sessionPreCompactjust committed, which may still be running — so the1 activebranch could seal a sibling session mid-flight where the old code correctly took the≥2branch. Concurrency is counted on activity again; only a state with a live session is actually committed.The shrink predicate needed a name.
role === "user"is not "a human turn" here:normalizeCaptureRole()maps tool results onto the user role too, and the inline check relied on their parts happening to be typedtoolrather thantext. It is nowfindLastHumanTurnIndex()incapture-utils.mjswith that coupling documented and unit-tested, and the-1case (no human turn survived the rewrite, so we replay the whole transcript) is logged instead of silently folded intoMath.max(0, ...).Human Involvement
Related Issue
Fixes #4058
Supersedes #4113
Type of Change
Changes Made
OPENVIKING_CODEX_COMMITTED_TTL_MS(default 30 days) for real cursors, the idle TTL for states that never captured anything.saveState(state, { touch: false })so releasingovSessionIddoesn't rewritelastUpdatedAt.ovSessionId.findLastHumanTurnIndex()and log the full-transcript fallback when no human turn survives compaction.DESIGN.md(new §6 "Cursor retention", heuristic and shrink trade-off notes),README.md,VERIFICATION.md(§6d-2), and the zh/en Codex integration docs with the new env var; bump the plugin to 0.7.6.Testing
New tests: cursor-only retention (expired cursor and never-captured state retired, in-window cursor kept, no commit issued), deferral to idle TTL when a cursor-only sibling is still active, and
findLastHumanTurnIndex()against tool results mapped onto the user role plus the empty case. Both new hook tests were confirmed RED against the #4113 commit alone.Full CI memory-plugin suite on Node 24.13.0: 335 tests, 334 passed, 1 skipped, 0 failed.
node --checkon every changed script,bash -non the six installer scripts CI parses, andstage-memory-plugin-marketplace.shstaging build all pass.Checklist
Additional Notes
The shrink path trades duplication for a bounded drop: turns older than the latest human turn are assumed captured, so if earlier
Stophooks failed to reach OpenViking and compaction happened before they were retried, those turns are dropped rather than replayed. That trade-off is now written down inDESIGN.mdunder "Post-compact transcript shrink".