Skip to content

fix(session-history): keep a legacy Codex conversation's branch current after Codex updates it - #26563

Open
brennanb2025 wants to merge 2 commits into
mainfrom
brennanb2025/codex-history-same-id-meta
Open

brennanb2025 wants to merge 2 commits into
mainfrom
brennanb2025/codex-history-same-id-meta

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​86 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​86
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​72 $\color{#cf222e}{\Huge{\mathbf{−}}}$​43 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​29

ELI5

Orca's Agent Session History lists your past Codex conversations and remembers which git branch each one was on. When Codex moves a conversation to a new branch, it writes a small "here is this conversation's info again" record into the conversation's file. A recent change (#26336) made Orca read only the first such record, so it stopped noticing those updates and kept showing the branch the conversation started on. This PR makes Orca accept a later record when it belongs to the same conversation, and only for the branch.

What Changed

The problem. #26336 ("Resume in New CLI") fixed a real bug: when Codex forks an older-format conversation, the fork's file starts with the fork's own info record and then contains a copy of the parent's, which has the parent's conversation id. Orca used to let that copied record overwrite the fork's row, so the fork looked like the parent. #26336 fixed that by reading only the first info record. But Codex also appends a later info record with the same conversation id whenever it updates the conversation's git info (for example, when the Codex terminal app notices a reply created a branch). Orca now ignored those too, so the history row kept a stale branch.

What you see.

  • Before: a Codex conversation (older history format) that moved to a new branch still shows, and is searchable by, its original branch in Agent Session History. Search always matches on the branch. The row shows the branch as its place label only when it can't place the row any other way: no matching worktree, no folder, and no worktree path in the title. Codex rows almost always have a folder, so search is where most people would notice.
  • After: the row picks up the new branch, and loses the branch if Codex clears it. A fork still shows its own info and never its parent's.

The mechanism. All handling of Codex's info records (session_meta) now lives in one function, consumeCodexSessionMeta in session-scanner-codex-session-meta.ts:

  • The first record is the conversation's own. It sets the id, the history format, whether the conversation is the user's own (vs. one Codex spawned for itself), the title, the folder and the branch, as before.
  • A later record whose id matches the conversation's id updates the branch only. If it carries a git object, the branch is set to exactly what it says, so an empty object clears it. If it has no git at all (Codex's memory-setting update writes it that way), the branch stays.
  • A later record with any other id (a fork's copy of its parent's), or with no id, changes nothing.

The function moved out of session-scanner-codex-parser.ts because that file was already at its 300-line lint limit.

Scope. Display metadata only. No change to how transcripts are written, which session owns a conversation, forks, resume, or UI.

Why

Codex's own metadata reader uses the same rule: it applies later records with the conversation's own id and skips ones with another id. The writer copies the conversation's first record, changes only its git info (and memory setting), refuses to write if the ids differ, and appends it. Matching on id rather than position fixes both directions. Reading only the first record broke updates, and reading every record (the behavior before #26336) broke forks.

Not refreshed on purpose:

The alternative of keeping the first-only check and adding a branch exception beside it was rejected: it would leave two checks that decide which record speaks for the conversation. Here one rule decides it.

Linked Issue

N/A (maintainer follow-up to #26336, found in its post-merge audit)

Visual Proof

N/A. This is a parser-only fix with no UI change: the history row renders branch exactly as before, and only the value the scanner produces changes. The parser tests below show the before/after values. Two of them fail on main (the branch stays main / fork-branch) and pass with this change.

Testing

  • New tests in session-scanner-codex-parser.test.ts:

    • A conversation with a later same-id record moves the branch, while the id, folder (from a turn record), user-derived title, history format and "user's own" classification stay as the first record set them.
    • A later record with no git keeps the branch; one with an empty git object clears it.
    • A fork: the parent's copied record, placed both before and after the fork's own update, never changes the fork's branch, folder or id; a record with no id is ignored.
  • Targeted vitest: the 8 Codex scanner test files (78 tests) pass. tc:node and orca-ci-checks (lint, code quality, localization) pass locally at the pushed commit.

  • Codex writer behavior was read first-hand from Codex's source (the metadata update path, the reader that copies the first record, the git info serialization, and Codex's own reader). It was not observed with a live Codex run.

  • Not run: the app. The scanner is platform-neutral; local, SSH and paired-host scans all go through this parser.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Review

Reviewed by a fresh reviewer and an architecture challenger, who first sketched a fix from the problem statement alone. Both independently found one more case: a branch Codex clears was kept, which was also true before #26336. It is fixed in the second commit. Deferred nits: no test for a first record without an id (the id then comes from the file name, which Codex sets to the conversation id); no scanner-level append-and-rescan test. Also, a history row saved by a build that includes #26336 keeps its stale branch until the conversation file changes again. #26336 is not in any release yet.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH / remote / paired host: same parser for every source, so same fix. No wire change.
  • Mobile: unchanged.
  • Cross-platform: no path or platform logic touched.
  • Performance: one string compare per later info record; records are rare (one per metadata update).

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Author: @BrennanKB5

…er its metadata updates

Since #26336 the Codex history scanner reads only a rollout's first session
record, so a fork's copy of its parent's record cannot take over the fork's
row. Codex also re-appends a thread's own first session record, same id, when
it updates the thread's git metadata on an older (non-paginated) history, and
the scanner ignored those too: the row kept the branch the thread started on.

A later record with the thread's own id now moves the branch; identity, history
mode, non-user classification, title and folder stay with the first record,
and records with another id stay ignored. The session_meta handling moves into
its own module so the parser stays under its line limit.
… clears it

Clearing a thread's branch re-appends its session record with an empty git
object, which the branch refresh treated like a missing one and so kept the old
branch. A record with a git object now sets the branch as written; only a
record with no git (a memory_mode update) keeps it. The fork test also puts the
parent's record after the fork's own update, so a refresh that took any later
id would fail it.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review summary (head 63cd74d)

Problem. #26336 made the Codex history scanner read only a conversation's first info record (session_meta), so a fork's copy of its parent's record could no longer take over the fork's row. Codex also re-appends a conversation's own record, with the same id, when it updates the conversation's git info on the older history format: for example, when the Codex terminal app sees that a reply created a branch. Those updates were now ignored, so Agent Session History kept the branch the conversation started on.

What changes for you. A Codex conversation in the older format shows its current branch in Session History search, and as its place label when nothing else can place it. If Codex clears the branch, the row clears it too. Forks still show only their own info. Nothing else changes: no transcript writes, no change to which session owns a conversation, no change to fork or resume, no UI.

Fixed.

Readiness checklist: pass, no P0/P1.

  • SSH, remote and paired-host scans use the same parser. There is no wire change and no platform logic.
  • No casts, retries or growth were added. Each later record costs one string compare.
  • No dependency or lockfile change.
  • No persisted schema change. No mobile-facing surface was touched beyond a new value in an existing field.

Deferred (P2 / nits).

Verified.

  • Tests: two new parser tests failed on main and pass here. The 8 Codex scanner test files (78 tests) pass.
  • Checks: tc:node and orca-ci-checks (22/22) pass locally at the pushed head.
  • Codex's writer and reader behavior was read first-hand in its source: the metadata update path, the reader that copies the first record, how git info is written, and Codex's own replay rule.
  • A fresh reviewer and an architecture challenger, who first sketched a fix from the problem alone, both agree with the design and confirmed the second commit.

Not verified.

  • No live Codex run.
  • No app run: this is a parser-only change, and the unit tests above are the evidence.
  • No Windows, WSL, SSH or paired-host run.
  • CI is still pending at the time of writing.

Author: @BrennanKB5

@coderabbitai

coderabbitai Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f5e4115f-2bdd-4dc8-9117-e0617d24cadf
📥 Commits

Reviewing files that changed from the base of the PR and between 1e76c87 and 63cd74d.

📒 Files selected for processing (3)
  • src/main/ai-vault/session-scanner-codex-parser.test.ts
  • src/main/ai-vault/session-scanner-codex-parser.ts
  • src/main/ai-vault/session-scanner-codex-session-meta.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

Codex session parsing now delegates session_meta records to consumeCodexSessionMeta. The shared consumer handles initial session metadata and later branch updates for matching session IDs. New tests cover branch updates and clearing for resumed sessions, and branch updates for forked sessions while ignoring parent-session and ID-less metadata.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 63cd7

The inspected rescan paths support branch updates, and no actionable merge-blocking issue remains after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: keeping a legacy Codex conversation’s branch current after Codex updates it.
Description check ✅ Passed The description covers the required sections, explains the change and rationale, and reports testing and platform considerations. It marks visual proof as not applicable and identifies the linked issu…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant