Conversation
…naged-home copy When one Codex conversation exists in two homes and the copies have diverged, list and resume the copy with the latest activity instead of always preferring Orca's managed home. Identical copies (hardlinks, same-size copies) keep the managed-home preference. Fixes #22478
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — the fix is sound and well-covered. Two observations: one cross-subsystem consequence of the symmetric comparison, and one residual limitation.
Reviewed changes
- Post-parse winner (
codexSessionAliasBeats) compares content activity (updatedAt) beforecodexSessionRootRank, so the copy with the later last record wins; equal/absent activity falls through to the existing rank → sort-time → path order. Identical copies and hardlinks tie, preserving the root preference. - Pre-parse collapse (
dedupeCodexRolloutCopyAliases) now keys onsessionId+sizeBytes, so same-size copies still collapse to the preferred root but diverged copies survive to the parser;session-scanner-candidates.tssuppliesfile.sizeBytes. Callers without the optional accessor reproduce the old behavior, and the remote scanner inherits the fix through the shared post-parse comparator. - Tests cover diverged-copy retention in both orders, same-size collapse, the newer-activity winner, and the equal-activity root preference; the
session-scan-cutofffixture was retargeted to a row with no activity time so that test still exercises the rank recount.
Verified locally: the 3 dedup/cutoff/collection suites (36 tests), the full src/main/ai-vault-search suite (573), pnpm tc:node, and pnpm run check:code-quality:changed all pass. The search index folds forked copies newest-first by content hash, so it stays consistent with the new list winner.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| // Why content time before root rank: a diverged copy's last record is later; | ||
| // identical copies tie here and keep the root preference (#22478). | ||
| const candidateActivity = candidate.updatedAt ? Date.parse(candidate.updatedAt) : Number.NaN | ||
| const bestActivity = best.updatedAt ? Date.parse(best.updatedAt) : Number.NaN | ||
| if ( | ||
| !Number.isNaN(candidateActivity) && | ||
| !Number.isNaN(bestActivity) && | ||
| candidateActivity !== bestActivity | ||
| ) { | ||
| return candidateActivity > bestActivity | ||
| } |
There was a problem hiding this comment.
This comparison is symmetric, so a newer managed-home copy now outranks the system real-home row, not just the custom-home case the PR targets. On AI Vault resume the managed winner then reaches prepareLegacySharedCodexSessionResume, which materializes the rollout into ~/.codex; when a diverged file already occupies that target, assertMatchingExistingTarget throws Retry resume. Please confirm this managed↔real-home direction is acceptable.
Technical details
# Managed-home winner routes into the legacy migration guard
## Affected sites
- `src/main/ai-vault/codex-session-root-dedup.ts:249-257` — activity now outranks `codexSessionRootRank` in both directions, so the managed home can win over the real-home row.
- `src/main/codex/codex-legacy-session-resume.ts:45-53` — materialization fires when the winner's `codexHome` equals the managed home and the host is on the real-home lane.
- `src/main/codex/codex-legacy-session-resume.ts:192-203` — `assertMatchingExistingTarget` throws when the target exists and is not the same inode/content.
- `src/main/runtime/rpc/methods/ai-vault.ts:112` — the AI Vault RPC surfaces that throw to the renderer; the launch path (`src/main/startup/codex-session-resume-launch.ts:66-81`) catches it and falls back to the origin home.
## Required outcome
- Decide whether a diverged managed-vs-real-home pair should resume the newer managed copy in place (`useRealCodexHome: false`) rather than fail the migration, or treat the guarded failure as the intended outcome.
## Open questions for the human
- Is a diverged managed/real-home pair (as opposed to the custom-home pair in the PR description) expected to occur? If so, is a resume error preferable to the previous silently-stale resume?| const copyKey = (candidate: T): string | null => { | ||
| const id = idByCandidate.get(candidate) | ||
| if (!id) { | ||
| // Why size: a copy resumed in another home grows there while the original | ||
| // stays frozen; root rank alone would hide the newer turns (#22478). | ||
| return id ? `${id}\0${accessors.getSizeBytes?.(candidate) ?? ''}` : null | ||
| } |
There was a problem hiding this comment.
Byte-size equality is only a proxy for divergence, so two copies whose content differs but happens to match in size are still collapsed to the ranked (possibly stale) copy before the parser can compare activity. Append-only growth makes this unlikely, but it is a residual hole in "show the newer copy" — worth calling out as an accepted limitation rather than an oversight.
Technical details
# Size-equality divergence proxy
## Affected sites
- `src/main/ai-vault/codex-session-root-dedup.ts:207-212` — `copyKey` collapses by `id + size`; matching sizes produce one key and the rank loser is dropped pre-parse.
## Required outcome
- No change required if byte-size growth is accepted as the divergence signal; document the residual case or key the collapse on something the parser can confirm.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughCodex rollout deduplication now uses session ID and file size to distinguish copies. Copies with different sizes remain available, while same-size copies compete for selection. When selecting an alias, later parsed Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🟡 Moderate · up to This change makes Agent Session History prefer the newer copy of a Codex conversation. When one copy has no activity time, however, the displayed copy can depend on scan order, so users may still see a stale transcript. Copies with equal file sizes are also treated as identical without comparing their content. Resolve or explicitly accept these selection issues before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 92c65629-1188-4826-809d-20ee67b24711
📒 Files selected for processing (4)
src/main/ai-vault/codex-session-root-dedup.test.tssrc/main/ai-vault/codex-session-root-dedup.tssrc/main/ai-vault/session-scan-cutoff.test.tssrc/main/ai-vault/session-scanner-candidates.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| if (!id) { | ||
| // Why size: a copy resumed in another home grows there while the original | ||
| // stays frozen; root rank alone would hide the newer turns (#22478). | ||
| return id ? `${id}\0${accessors.getSizeBytes?.(candidate) ?? ''}` : null |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not discard same-size transcripts before comparing their content.
Two homes can contain different records for the same session ID and still have equal file sizes. In that case, bestByCopy drops one transcript before parsing, even if the dropped transcript has later activity. The equal-size assertion in src/main/ai-vault/codex-session-root-dedup.test.ts preserves this behavior. Keep both candidates for post-parse selection unless content equality is established.
| if ( | ||
| !Number.isNaN(candidateActivity) && | ||
| !Number.isNaN(bestActivity) && | ||
| candidateActivity !== bestActivity | ||
| ) { | ||
| return candidateActivity > bestActivity |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make alias selection independent of input order.
This comparison is non-transitive when one copy lacks updatedAt. Consider an older ~/.codex copy, a managed copy with no activity time, and a newer custom-home copy. In the order custom, managed, ~/.codex, root rank selects managed over custom and then ~/.codex over managed. In another order, the newer custom copy wins. Select a winner per alias group using a consistent rule for missing activity times, and cover this three-copy case in multiple orders.
This branch has not been deployed
1 participant
ELI5
Sometimes the same Codex conversation is saved in two folders. Orca shows each conversation once, so it has to pick one of the two files. It always picked the one in Orca's own folder, even when the other file was newer and had more messages. Now, if the two files are different, Orca picks the one with the latest messages.
What Changed
Before: a user copied a Codex conversation out of Orca's account folder into their own
CODEX_HOMEand kept chatting there. Agent Session History still showed the old copy (2 messages instead of 6). Resume continued from that old copy, which silently dropped the later messages.After: Orca shows the copy with the latest activity and resumes it.
Mechanism (
codex-session-root-dedup.ts):~/.codex, then Orca's managed homes, then other homes). If either copy has no recorded activity time, the folder preference decides, as before.Search needs no change of its own. Its index reads the same file list, and it already folds forked copies into one result, newest first (
session-search-hit-ranking.ts).Why
The folder preference exists so Resume runs in a home whose login Orca keeps fresh. That only matters when the copies are identical, which is the normal case: Orca's multi-account setup links one file into two folders. Once the copies differ, the managed copy is simply out of date. Showing it hides the newer turns, and resuming it forks the conversation from an old point.
Alternatives considered:
Linked Issue
Fixes #22478 (STA-8287)
Visual Proof
N/A. This changes which file backs an existing history row; there's no new UI. The regression tests cover it.
Testing
Added regressions for a stale managed copy plus a newer custom-home copy, in both input orders, at both the before-reading and after-reading stages. Also added coverage that identical copies keep the managed-home preference.
Confirmed both new tests fail on the old code.
pnpm test src/main/ai-vault src/main/ai-vault-search(1512 pass),pnpm tc:node, andpnpm run check:code-quality:changedall pass.Updated one existing cutoff test: it relied on the folder preference beating a newer copy. It now uses a row with no activity time, where the folder preference still decides, so it checks the same recount behavior.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)