fix(history): resume cross-project sessions & find Codex sessions by payload.id#1058
fix(history): resume cross-project sessions & find Codex sessions by payload.id#1058pedramamini wants to merge 1 commit into
Conversation
…y payload.id (#251) Repro A — "Open session … as new tab" (and the detail-modal Resume button) read the stored session using the active session's projectRoot, so resuming a history entry that belongs to a different local project read the wrong path and threw an AgentSessions read error. Thread the history entry's projectPath through HistoryEntryItem / HistoryDetailModal → RightPanel → useRightPanelProps → handleResumeSession and use it for both the agentSessions.read and getSessionOrigins lookups (falling back to the active project root). Repro B — Codex resume opened an empty tab because findSessionFile only matched the rollout filename UUID or the legacy top-level metadata.id, never the new-format session_meta payload.id. Match payload.id as well (locally and over SSH), consistent with how the rest of the storage extracts session IDs. Closes #251
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR extends session resumption to support cross-project scenarios by threading project path through the history flow and updating storage matching to recognize both legacy and newer session-meta payload IDs, resolving failures when opening history entries from different projects. ChangesCross-project session resumption
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryFixes two independent History → "Open session as new tab" bugs: cross-project sessions were read from the wrong storage path (Repro A), and Codex sessions with a thread id that differed from the rollout filename UUID were never found (Repro B).
Confidence Score: 4/5Both bugs are fixed correctly and the changed paths are well tested; the only open gap is the agentId derivation in handleResumeSession, which would silently fail for cross-project sessions resumed from a unified history view containing entries from a different agent type — a scenario not yet reachable in the current UI. The cross-project path fix and the Codex payload.id fix are straightforward, targeted, and backed by new tests covering the happy path and the fallback. The one concern is that agentId is still read from the active session's tool type rather than the history entry, meaning a future unified-history view resuming a cross-tool entry would silently read from the wrong storage. This is pre-existing behaviour not worsened by the PR, but worth tracking before the unified view ships. src/renderer/hooks/agent/useAgentSessionManagement.ts — the agentId derivation from activeSession.toolType is the only remaining gap; src/renderer/hooks/props/useRightPanelProps.ts — the two identical handler lambdas are minor but worth cleaning up. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant HEI as HistoryEntryItem
participant HP as HistoryPanel
participant RP as RightPanel
participant URPP as useRightPanelProps
participant UASM as useAgentSessionManagement
participant API as agentSessions.read
U->>HEI: click session pill
HEI->>HP: onOpenSessionAsTab(sessionId, entry.projectPath)
HP->>RP: onOpenSessionAsTab(sessionId, projectPath)
RP->>URPP: onOpenSessionAsTab(sessionId, projectPath)
URPP->>UASM: handleResumeSession(sessionId, ···, projectPath)
Note over UASM: readProjectPath = projectPath ?? activeSession.projectRoot
UASM->>API: read(agentId, readProjectPath, sessionId, opts)
API-->>UASM: messages
UASM-->>U: new AI tab opened
Reviews (1): Last reviewed commit: "fix(history): resume cross-project sessi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Use projectRoot (not cwd) for consistent session storage access | ||
| // Use the resolved project path so cross-project history entries | ||
| // read from their own storage location instead of the active one. | ||
| const agentId = activeSession.toolType || 'claude-code'; |
There was a problem hiding this comment.
agentId always mirrors the active session's tool type
HistoryEntry carries no toolType field, so the agent type used for agentSessions.read is always derived from activeSession.toolType. For the scenarios this PR targets (same-tool cross-project) that is fine. However, HistoryEntryItem already accepts a showAgentName prop described as "used in unified history view," implying a view where entries from different agent types may coexist. If a Codex entry from a different project is resumed inside a Claude Code agent's panel, agentId will be 'claude-code' and the read will silently fail to find the Codex session. Adding toolType?: ToolType to HistoryEntry and threading it here would close this gap when the unified view lands.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // History entries can belong to a different local project than the active | ||
| // session, so forward the entry's project path to handleResumeSession (its | ||
| // trailing parameter) and let it read the stored session from there. | ||
| onResumeSession: (agentSessionId: string, projectPath?: string) => | ||
| deps.handleResumeSession( | ||
| agentSessionId, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| projectPath | ||
| ), | ||
| onOpenSessionAsTab: (agentSessionId: string, projectPath?: string) => | ||
| deps.handleResumeSession( | ||
| agentSessionId, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| projectPath | ||
| ), |
There was a problem hiding this comment.
onResumeSession/onOpenSessionAsTab lambdas
Both inline functions have an identical body — they differ only in the key name. A single shared adapter function assigned to both keys would make a future signature change a one-line edit instead of two, and eliminates the risk of the two implementations drifting.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Closes #251
Problem
Two distinct bugs in History → "Open session … as new tab":
Repro A (local Claude Code): Resuming a history entry that belongs to a different local project than the active session failed with
[AgentSessions] read errorand no tab opened.handleResumeSessionalways read the stored session fromactiveSession.projectRoot, so a cross-project entry pointed at the wrong storage path.Repro B (local Codex): Resuming a Codex history entry opened an empty tab.
findSessionFile()only matched the rollout filename UUID or the legacy top-levelmetadata.id, but new-format Codex sessions record their id undersession_meta→payload.id. When the session's thread id differed from the filename UUID, the file was never found and the read returned no messages.Fix
Repro A — Thread the history entry's
projectPathfrom the click site down to the reader:HistoryEntryItem/HistoryDetailModal→HistoryPanel→RightPanel→useRightPanelProps→handleResumeSession. The resolved path (projectPath ?? activeSession.projectRoot) is now used for bothagentSessions.readand the ClaudegetSessionOriginslookup. Both resume affordances (the session pill and the detail-modal Resume button) are covered.Repro B —
findSessionFile()(andfindSessionFileRemote()for SSH) now also matchmetadata.payload?.id, consistent with how the rest ofcodex-session-storage.tsalready extracts session IDs (metadata?.payload?.id || metadata?.id).Notes
{}in the error log is already addressed on currentmain:withIpcErrorLoggingruns the error throughserializeError, which extractsname/message/stackforErrorinstances. The bare{}was from the 0.14.5 reported build.HistoryEntrydoes not currently persist ansshRemoteId, so threading that is out of scope here and would need a data-model change.Tests
useAgentSessionManagement: reads a cross-project session from the providedprojectPath; falls back to the active project root when none is provided.useRightPanelProps:onOpenSessionAsTabforwards the project path tohandleResumeSession's trailing parameter.codex-session-storage: finds a session bysession_metapayload.idwhen the filename UUID differs.onOpenSessionAsTabcall-signature assertions inHistoryEntryItem/HistoryPanel(unit + integration).All affected unit + integration tests pass locally; ESLint and Prettier clean on changed files.
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features