Repository navigation
feat(native-chat): resume an Agent Session History row into a new structured chat - #19176
Conversation
…uctured chat A Claude or Codex row in Agent Session History gains "Resume in New Chat": it opens a new structured native-chat tab that continues that provider conversation, with the prior turns already in the journal. Until now those rows could only be resumed into a PTY terminal; the structured branch could reveal a chat Orca already owned but could not adopt one it had never held. Almost all of the machinery existed. Both lanes already resume from the record's provider handle chain, the journal already has a transcript importer, and the handle chain already models `adopted` as an origin. The gap was that a create always minted an empty chain, so the adapters started a fresh conversation. This seeds that chain. The client names only the conversation. `agentSession.create` is reachable by paired mobile clients, so the transcript path and the account home are derived by the executing host and validated against the account homes it recognises — a client-supplied path would choose which file the host imports and which credential directory the provider child launches against. Failure refuses rather than degrades. A transcript that cannot be found refuses before anything is created; one that fails or decodes empty *after* the provider has resumed fails the attach, tearing the child down and publishing no tab, because an empty journal beside a context-carrying agent claims a continuity the provider never gave. Codex can resume into any workspace since it is handed the rollout path; Claude resolves transcripts under a project key derived from the launch cwd, so it is offered only for the workspace the conversation was recorded in.
…ches untouched Three corrections from review of the first commit. The adoption's account-home candidates now include the extra Codex homes session discovery already scans. A row this host listed could otherwise refuse to resume, which reads as the feature being broken rather than as a scope. Ordinary launches call `createStructuredAgentSessionLaunchIntent` with two arguments again. Passing the resume source unconditionally appended a trailing `undefined` that four existing call-site assertions had to absorb; the churn was the caller's fault, not the tests'. The transactional adoption guard's comment claimed the self-exemption is what lets a committed create replay. It is not: replay is settled earlier by the operation ledger, and an adoption always arrives with a null expected fence, so a request naming an existing session id is refused a few lines below either way. The exemption is part of what "another record" means, and the comment now says that instead.
Follow-up commit: three review correctionsAdopted-home discovery was narrower than session discovery. The candidate account homes now include the extra Codex homes Ordinary launches call the intent builder with two arguments again. Passing the resume source unconditionally appended a trailing A comment claimed a mechanism that isn't operating. The transactional adoption guard's self-exemption does not enable replay — replay is settled earlier by the operation ledger, and an adoption always arrives with a null expected fence, so a request naming an existing session id is refused a few lines below either way. The exemption is part of what "another record" means; the comment now says that rather than asserting a false rationale. The guard itself is unchanged and still load-bearing (ablation turns 3 reservation tests red). Gates re-run after the change: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds structured session resumption from existing Claude and Codex conversations. It carries resume identity through launch intents, fingerprints, validation, and launch state. The host resolves transcript locations, checks conversation ownership, seeds provider handle chains, and imports transcripts into the new journal. The AI Vault adds eligibility checks, resume actions, workspace handling, error messages, and UI controls. Merge Risk: 🔵 Low · up to Claude sessions recorded in a directory whose name ends or begins with whitespace can be resumed from a different workspace, causing the resumed chat to miss its source conversation. Preserve exact POSIX path components before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the user-visible behavior, scope, reliability work, visual proof, and validation results. However, it omits the required Linked Issue section and Resolution Add the required
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: fc157234-4620-4f68-b324-a827b929bc5a
📒 Files selected for processing (32)
src/main/native-chat/agent-session-wire/structured-agent-session-adopted-import.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.tssrc/main/native-chat/agent-session-wire/structured-agent-session-attach.tssrc/main/native-chat/structured-agent-session-history-adoption.test.tssrc/main/native-chat/structured-agent-session-history-adoption.tssrc/main/runtime/agent-session-reservation-admission.test.tssrc/main/runtime/agent-session-reservation-admission.tssrc/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.tssrc/main/runtime/rpc/methods/structured-agent-session-schemas.tssrc/main/runtime/rpc/methods/structured-agent-session.tssrc/renderer/src/components/right-sidebar/AiVaultPanel.tsxsrc/renderer/src/components/right-sidebar/AiVaultSessionActionMenuItems.tsxsrc/renderer/src/components/right-sidebar/AiVaultSessionDetails.tsxsrc/renderer/src/components/right-sidebar/AiVaultSessionRow.tsxsrc/renderer/src/components/right-sidebar/AiVaultSessionVirtualList.tsxsrc/renderer/src/components/right-sidebar/SessionRowTrailingActions.tsxsrc/renderer/src/components/right-sidebar/ai-vault-session-launch-actions.tssrc/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat.test.tssrc/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/agent-launch-routing.tssrc/renderer/src/lib/launch-agent-structured-chat-guard.test.tssrc/renderer/src/lib/launch-structured-agent-session.tssrc/renderer/src/lib/structured-agent-session-launch-callers.tssrc/renderer/src/lib/structured-agent-session-launch-resume-identity.test.tssrc/renderer/src/lib/structured-agent-session-launch.test.tssrc/renderer/src/lib/structured-agent-session-launch.tssrc/shared/agent-session-provider-handle.test.tssrc/shared/protocol-version.tssrc/shared/structured-agent-session-create.test.tssrc/shared/structured-agent-session-create.tssrc/shared/structured-agent-session-mutation.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.
| normalizeRuntimePathForComparison(cwd.trim()) === | ||
| normalizeRuntimePathForComparison(workspacePath.trim()) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not trim path names before comparison.
normalizeRuntimePathForComparison preserves leading and trailing whitespace. Trimming both values can mark /repo/orca as eligible for /repo/orca, even though they are different POSIX directories. The structured Claude launch resolves the selected worktree path as cwd, so Claude can search the wrong transcript project. Pass the raw paths and change the whitespace test to expect false.
Proposed fix
- normalizeRuntimePathForComparison(cwd.trim()) ===
- normalizeRuntimePathForComparison(workspacePath.trim())
+ normalizeRuntimePathForComparison(cwd) ===
+ normalizeRuntimePathForComparison(workspacePath)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| normalizeRuntimePathForComparison(cwd.trim()) === | |
| normalizeRuntimePathForComparison(workspacePath.trim()) | |
| normalizeRuntimePathForComparison(cwd) === | |
| normalizeRuntimePathForComparison(workspacePath) |
There was a problem hiding this comment.
Important
The specifically-authored "conflict" / "transcript missing" toasts never render: the refusal code rides on error.code, but the matcher inspects error.message. See the inline comment — the failure surfacing also double-toasts and misclassifies a conflict as an unknown outcome.
Reviewed changes
- Wire & capability —
agentSession.creategains an optionalresumeFrom: { providerSessionId }, gated by the newagent-session.structured.resume-history.v1capability so older strict-schema hosts are negotiated against rather than probed. - Host adoption resolution — the executing host derives the account home and transcript path from the session id alone (never a client-supplied path), refusing a conversation it can't find or that another chat already holds.
- Handle-chain seeding — an adopting create seeds an
adoptedhead link so the adapter resumes instead of starting fresh; one-writer-per-conversation is enforced both pre-commit and inside the store transaction. - Journal import — the adopted transcript is imported post-acquisition and before publication, failing the attach (tearing the child down, publishing no tab) if it can't be read or decodes empty.
- UI entry point — "Resume in New Chat" on history rows with per-agent eligibility (Claude bound to its recorded workspace, Codex anywhere), and launch coalescing keyed on
resumeFromso a resume is never folded into a pending blank launch.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
| * generic "could not prepare" the terminal resume reports. */ | ||
| function notifyAiVaultSessionResumeInChatFailure(error: unknown): void { | ||
| const message = error instanceof Error ? error.message : String(error) | ||
| if (message.includes('agent_session_conflict')) { |
There was a problem hiding this comment.
These two specific toasts can never fire over the real wire path. The host maps a conflict to a refusal with a generic message — precommitRefusal returns { code: 'agent_session_conflict', message: 'Orca cannot open a structured agent chat for this workspace.' } (see structured-agent-session-precommit-refusal.ts). launchStructuredAgentSession then re-throws that as StructuredAgentSessionCreateUnknownOutcomeError(message, code), so the code is on error.code and error.message is the generic sentence — message.includes('agent_session_conflict') is always false. Match on the code (e.g. error instanceof StructuredAgentSessionCreateUnknownOutcomeError && error.code === ...) or have the host surface the code in the message, instead of message.includes(...).
Technical details
# Resume refusal toasts are dead code; conflict is also misclassified
## Affected sites
- src/renderer/src/components/right-sidebar/ai-vault-session-launch-actions.ts:248 — matches on `error.message` only.
- src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.ts:46 — coded refusal keeps the generic `'Orca cannot open a structured agent chat for this workspace.'` message.
- src/renderer/src/lib/launch-structured-agent-session.ts:219 — non-definitive code rethrown as `StructuredAgentSessionCreateUnknownOutcomeError(message, code)`.
## Required outcome
- A refused resume shows the authored toast (`resumeInChatConflict` / `resumeInChatTranscriptMissing`), not a generic one.
## Observed consequences to fix alongside
1. `agent_session_conflict` / `agent_session_identity_required` are not in the definitive-refusal allowlist, so a conflict is treated as an unknown outcome: `launchAndReconcile` retries the create, refreshes tabs, and reads history before giving up, then sets `visibilityUnknown = true`. The launch state is left `unknown` (never cleaned up, since `handleResumeInNewChat` never calls `releaseCallerAfterUnknownOutcome`), which can linger in `getStructuredAgentLaunchStatus` for that (worktree, agent).
2. `trackLaunchFailureToast` in `structured-agent-session-launch.ts` also fires on the same rejection, so the user sees the generic `'Could not open {{agent}} chat'` toast *and* `notifyAiVaultSessionResumeInChatFailure`'s generic `'Could not resume this session in a new chat.'` toast — two generic toasts, no actionable one.
## Open questions for the human
- Should `agent_session_conflict` / `agent_session_identity_required` count as definitive create refusals (nothing was created, so a terminal fallback is not offered and no retry is attempted)? That would also drop the redundant retry/reconcile.There was a problem hiding this comment.
ℹ️ One edge case worth a look — the discovery widening is correct for local homes, but it skips the running-distro filter the scanner applies. Minor suggestion inline.
Reviewed changes
- Widen adopted-home discovery —
structuredAdoptionAccountHomeCandidatesnow appendsconfiguredAdditionalCodexHomePaths()(a new export fromcached-session-list.ts) so a Codex row listed from an extra home can also be resumed from it, not just listed. - Keep ordinary launches untouched —
createStructuredAgentSessionLaunchIntentis now only called with a third argument whenresumeFromis present, reverting theundefined-arg test churn inlaunch-agent-structured-chat-guard.test.tsandstructured-agent-session-launch.test.ts. - Docstring tightening — the
assertAdoptedConversationUnownedcomment now states the self-exemption is part of the guard's definition, not a replay mechanism.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
| ...managedHomes, | ||
| // The same extra homes session discovery scans. Without these a row that this host listed | ||
| // could refuse to resume, which reads as the feature being broken rather than as a scope. | ||
| ...configuredAdditionalCodexHomePaths(), |
There was a problem hiding this comment.
The scanner filters these same homes through filterPathsToRunningWslDistrosAsync before scanning (cached-session-list.ts:91), but here they're appended raw. A stopped-WSL home in the set makes resolveSessionFilePath throw an unavailable refusal (gated WSL readdir), which aborts resolveStructuredAgentSessionAdoption's loop before it reaches getOrcaManagedCodexHomePath() / getSystemCodexHomePath() — a row whose conversation lives in one of those later local homes would refuse with "couldn't look" instead of resuming.
Technical details
# Additional Codex homes skip the running-WSL filter the scanner applies
## Affected sites
- src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.ts:172 — raw `configuredAdditionalCodexHomePaths()` appended to the candidate list.
- src/main/ai-vault/cached-session-list.ts:91 — the scanner filters the same set through `filterPathsToRunningWslDistrosAsync` first.
- src/main/native-chat/session-file-resolver.ts:308 — `findCodexRolloutInDirs` throws `unavailable` when a WSL root's readdir is gated/refused; each candidate is a separate `resolveSessionFilePath` call, so the throw propagates out of `resolveStructuredAgentSessionAdoption` rather than continuing to later candidates.
## Required outcome
- Resolving a row whose conversation lives under a local home must not be aborted by a stopped-WSL additional home; either filter the additional homes the same way (running-distro filter) or order them after the always-local `getOrcaManagedCodexHomePath()` / `getSystemCodexHomePath()`.
## Suggested approach
- Apply `filterPathsToRunningWslDistrosAsync` to `configuredAdditionalCodexHomePaths()` before appending (the resolver already runs in an async context), matching the scanner's view.|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
ℹ️ No new issues in this merge reconciliation. Not an approval only because the must-address feedback from the first review remains open — the
resumeInChat*toasts matcherror.messagewhere the refusal code actually rides onerror.code. That code lives inai-vault-session-launch-actions.ts, which this commit does not touch.
Reviewed changes
- Delegate launch routing to the shared module —
resolveAgentLaunchRouteandstructuredAgentLaunchSupportednow go throughprefersStructuredNativeChatByDefault/resolveStructuredNativeChatSupportfromstructured-native-chat-launch-route.ts, replacing the inline feasibility predicate. - Align resume eligibility with the fresh-chat route —
structuredAgentLaunchSupportednow inherits theexecutionHostId !== 'local'blocker, so "Resume in New Chat" is no longer offered for a remote target workspace where a structured chat could not run anyway. TheresolveAgentLaunchRoutegate swap is behavior-preserving, sinceinitialViewMode === 'chat'already implies theprefersStructuredNativeChatByDefaultpreconditions.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the create/retry path is now handled correctly. Not an approval because the must-address feedback from the initial review is still open: the
resumeInChat*toasts matcherror.messagewhere the refusal code rides onerror.code(ai-vault-session-launch-actions.ts). This commit makes that path actually reachable, since forwardingresumeFrommeans an adoption conflict or an unresolvable conversation now really does reach the client.
Reviewed changes
- Skip adopted-transcript import on replay —
importAdoptedTranscriptmoves to its own module and returns early whenjournal.cursor().sequence > 1, so a retried committed create keeps its journal-only messages and never re-reads a since-deleted transcript. - Close the provisional journal on import failure —
performAttachwraps the import intry/catchand hands the journal toagentSessionJournalCloseRetries.closeOrRetainbefore rethrowing, closing a handleonAttachFailedcannot reach. - Forward
resumeFrominto create resolution —prepareStructuredAgentSessionCreateForWorktreeand theagentSession.createhandler now passresumeFromtoresolveStructuredAgentSessionCreateIntent, so an adopting create seeds the handle chain instead of minting a blank session.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat.ts (1)
55-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve exact POSIX path components in Claude workspace matching
AiVaultPanelcan enable Claude resume whenaiVaultSessionCwdMatchesWorkspacetrims both paths, so distinct directories such as/repoand/repocompare equal. The structured launch uses the selected worktree, causing Claude to launch from/repowhile looking for a transcript recorded under/repo, which can yield no conversation. Remove.trim()from these comparison arguments;normalizeRuntimePathForComparisonalready applies the required platform normalization while preserving POSIX path components.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: e74fbb52-4404-40d9-b7f8-12e2d1349d4f
📒 Files selected for processing (11)
src/main/native-chat/agent-session-wire/structured-agent-session-adopted-import.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adopted-import.tssrc/main/native-chat/agent-session-wire/structured-agent-session-attach.tssrc/main/native-chat/structured-agent-session-history-adoption.test.tssrc/main/native-chat/structured-agent-session-history-adoption.tssrc/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.tssrc/main/runtime/rpc/methods/structured-agent-session-adoption-replay.test.tssrc/main/runtime/rpc/methods/structured-agent-session-create.tssrc/main/runtime/rpc/methods/structured-agent-session.test.tssrc/main/runtime/rpc/methods/structured-agent-session.tssrc/main/runtime/structured-agent-session-create-adoption.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/runtime/rpc/methods/structured-agent-session.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the replay-from-durable-identity path is sound and well-tested. Not an approval because the must-address feedback from the initial review is still open: the
resumeInChat*toasts matcherror.messagewhere the refusal code rides onerror.code(ai-vault-session-launch-actions.ts), and this delta does not touch that file.
Reviewed changes
- Replay committed adoption from durable identity — a retried committed adopting create now reconstructs its attach intent from the durable record and operation ledger (
findCommittedStructuredAgentSessionAdoptionReplay/resolveCommittedStructuredAgentSessionAdoptionIntent) instead of re-reading a deleted transcript or re-resolving a drifted account selection. - Extract adoption resolution into a type-checked module — the account-home/transcript-path decision moves out of the
@ts-nocheckruntime class intostructured-agent-session-create-adoption.ts, keeping the client-supplied-path hazard behind type-checked code. - Make the adopted transcript path optional —
adopt.transcriptPathis omitted on a durable replay;importAdoptedTranscriptrefuses a fresh journal that needs an import but got no path (agent_session_identity_required) rather than silently importing nothing. - Thread caller identity into create resolution —
agentSession.createforwardscallerKeyso replay is scoped to the exact committed caller+operation namespace, covered by a new RPC-level replay test plus unit coverage for the replay finder.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the pre-acquisition validation and the renderer extraction are both clean, with one minor suggestion inline. Not an approval because the must-address feedback from the initial review is still open:
notifyAiVaultSessionResumeInChatFailurematcheserror.messagewhere the refusal code actually rides onerror.code(ai-vault-session-launch-actions.ts), and this delta does not touch that matcher.
Reviewed changes
- Validate history before claiming —
prepareAdoptedTranscriptnow runs beforereserveOwner/acquisition, refusing an unreadable, oversized, empty, or invalid source withagent_session_identity_requiredbefore a new record claims the provider conversation.importAdoptedTranscriptconsumes the pre-prepared items, so the transcript is read once (a source deleted after prepare still imports, pinned by a test) instead of re-read post-acquisition. - Extract renderer resume domains —
resolveAiVaultSessionLaunchTarget/aiVaultResumeUnsupportedMessage,resolveAiVaultSessionResumeInChatForWorkspace, andAiVaultVirtualRoware pulled into their own modules (ai-vault-session-launch-target.ts,ai-vault-session-resume-in-chat-workspace.ts,AiVaultVirtualRow.tsx) with no behavior change.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
| return { | ||
| ok: false, | ||
| refusal: { | ||
| code: 'agent_session_identity_required', |
There was a problem hiding this comment.
Every pre-acquisition failure — unreadable, oversized (the prepared.error "exceeds import bound" path), invalid JSON, and empty — collapses to the single wire code agent_session_identity_required; only error.message survives. That code means "no resolvable source", so a client matching on code would report an oversized but present transcript as "transcript not found". Consider a distinct refusal code for unreadable/oversized sources rather than folding them into the identity-missing case.
Final-SHA Electron QA —
|
| # | Layer | Scenario | Result |
|---|---|---|---|
| T2 | host RPC | Committed create, then identical envelope replayed with the source file moved away | ✅ replayed: true, same sessionId, same fence, chain not re-minted, accountHome preserved, record count unchanged, exactly one record holds the conversation |
| T3 | host RPC | Second adoption, new session id, same conversation | ✅ refused agent_session_conflict — and refused before source resolution (source was absent, yet the code is conflict, not identity) |
| T4 | host RPC | Bogus providerSessionId |
✅ refused agent_session_identity_required |
| T5 | host RPC | Oversized disposable transcript (19.38 MB vs the 16 MB bound) | ✅ refused pre-acquire: Legacy transcript exceeds the 16777216-byte import bound |
No record was created by any refusal (count held at 6 throughout). UI menu-hiding is not offered as evidence for any of these — each is a real host refusal.
Unit tests: 12 files / 179 tests passed (vitest run --config config/vitest.config.ts).
One defect found — refusal codes never reach the user 🐞
The two tailored messages this PR adds (resumeInChatConflict, resumeInChatTranscriptMissing) can never display. A user hitting a diagnosable condition gets the generic fallback instead.
Reproduced end-to-end through the real UI: with a history row already listed, its Codex rollout removed (the ordinary "user deleted the transcript after the list was cached" case), clicking Resume in New Chat shows:
Could not resume this session in a new chat.
Could not open Codex chat — Orca could not open a structured Codex chat. See the logs for details.
Expected: "This conversation's transcript could not be found, so it cannot be resumed."
Two compounding causes, both confirmed live:
-
agent_session_conflictandagent_session_identity_requiredare absent fromDEFINITIVE_REFUSAL_CODES(src/shared/agent-session-definitive-refusal.ts, untouched by this PR, currently onlystructured_agent_session_unsupported). So the client raisesStructuredAgentSessionCreateUnknownOutcomeErrorand routes intoretrySameIntentrather than surfacing a refusal. Console from the live run:
[native-chat] structured launch failed StructuredAgentSessionCreateUnknownOutcomeError: Orca cannot open a structured agent chat for this workspace. … at async retrySameIntent
Both codes are refused before anything is created (T3/T4/T5 all left the record count unchanged), so by this file's own definition — "proves the host neither created a session nor will on a retry" — they qualify as definitive. -
Even on the refusal branch,
notifyAiVaultSessionResumeInChatFailurematcheserror.message.includes('agent_session_conflict' | 'agent_session_identity_required'), but the host's message never contains the code — it is either the generic "Orca cannot open a structured agent chat for this workspace." (T3, T4) or a specific unrelated string (T5's byte-bound message). The code travels onerror.code, whichStructuredAgentSessionCreateRefusalErroralready carries.
Suggested minimal fix: add both codes to the definitive set, and match on the error's code rather than its message. Safety is unaffected — the host refuses correctly in every case; this is a diagnosability defect. The green suite does not catch it because no test exercises the renderer's refusal→toast mapping against a real host refusal payload.
Gaps — explicitly not verified
- Account-selection change not exercised. It was dropped from the final priority list, and I would not touch real
CODEX_HOME/CLAUDE_CONFIG_DIRcredentials; a disposable-account-home variant was not set up. Unverified, not passing. - Dev build only (blind to packaged-build regressions); macOS only; no SSH/WSL execution — those refusals remain code-verified.
- No crash injection; the retry was exercised via a repeated committed envelope, which is the intended replay path.
agent_session_ownership_unknown(lease present but not admitting a writer) not reached.- Single run per scenario.
Full report: /tmp/orca-pr19176-qa/final-report.md.
Toast finding resolved — verified at
|
|
Review complete at Fixes pushed during review:
Performance and precedent review found no remaining issue: the fix extends existing admission/import/replay paths, retains the 16 MiB source bound, and introduces no polling, subprocess fan-out, or retained transcript payload. No unrelated product expansion was added. Validation:
Grok was unavailable because its weekly quota was exhausted; orchestrated Claude Opus performed Electron QA instead. Live QA used an isolated macOS dev app and disposable fixtures; no live Windows/Linux/SSH/WSL/mobile or packaged-app smoke test is claimed. Account-selection drift is covered by deterministic runtime tests, and the conflict toast by the shared classifier rather than a live UI race. Generic disk-full/crash recovery remains outside this focused fix. The branch is conflict-free and pushed, the working tree is clean, and supervised workers are settled. PR left open; not merged. |
There was a problem hiding this comment.
Important
The matcher fix is correct and finally makes the authored toasts reachable, but it leaves the two consequences from the initial review live — and the double toast is now visible for the first time.
Reviewed changes
- Match resume refusals by wire code —
notifyAiVaultSessionResumeInChatFailurenow classifies failures viahasRuntimeRpcErrorCodeinstead oferror.message.includes(...), soagent_session_conflict/agent_session_identity_requiredactually reach their authored toasts. - Widen the transcript-missing copy —
resumeInChatTranscriptMissingnow reads "history could not be loaded", covering the oversized / corrupt / unreadable sources that also collapse toagent_session_identity_required.
⚠️ Conflict and identity refusals still aren't definitive
The two refusal codes still prove nothing was created, yet they are absent from the definitive-refusal allowlist, so a refused resume retries the create and then leaves the launch state lingering as unknown.
Technical details
# Make conflict/identity refusals definitive so a refused resume stops retrying and lingering
## Affected sites
- src/shared/agent-session-definitive-refusal.ts:18 — `DEFINITIVE_REFUSAL_CODES` still holds only `structured_agent_session_unsupported`.
- src/renderer/src/lib/launch-structured-agent-session.ts:219 — a `!result.ok` refusal with `agent_session_conflict` / `agent_session_identity_required` throws `StructuredAgentSessionCreateUnknownOutcomeError` (non-definitive).
- src/renderer/src/lib/structured-agent-session-launch-recovery.ts:112 — `launchAndReconcile` then retries (`retrySameIntent`), re-running the create and re-reading history before giving up.
- src/renderer/src/lib/structured-agent-session-launch.ts:160 — the state is left `outcome: 'unknown'` with `visibilityUnknown = true` and never cleaned up (`handleResumeInNewChat` never calls `releaseCallerAfterUnknownOutcome`), so `getStructuredAgentLaunchStatus` reports `unknown` for that (worktree, agent) pair indefinitely.
## Required outcome
- A `agent_session_conflict` / `agent_session_identity_required` refusal flows as a definitive refusal: no create retry, no lingering `unknown` launch state.
## Suggested approach
- Add `agent_session_conflict` and `agent_session_identity_required` to `DEFINITIVE_REFUSAL_CODES`. Both mean the host refused before creating anything; a retry cannot change that outcome.DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
| * cannot find under any account home it recognises. Both are actionable, and neither is the | ||
| * generic "could not prepare" the terminal resume reports. */ | ||
| function notifyAiVaultSessionResumeInChatFailure(error: unknown): void { | ||
| if (hasRuntimeRpcErrorCode(error, 'agent_session_conflict')) { |
There was a problem hiding this comment.
The specific toast now renders, but trackLaunchFailureToast in structured-agent-session-launch.ts still fires the generic "Could not open {{agent}} chat" toast on the same rejection, so a conflict or missing-transcript refusal now stacks two toasts and the second is misleading.
Technical details
# A refused resume shows two toasts
## Affected sites
- src/renderer/src/components/right-sidebar/ai-vault-session-launch-actions.ts:245 — the specific toast now fires.
- src/renderer/src/lib/structured-agent-session-launch.ts:177 — `trackLaunchFailureToast` fires the generic toast on any non-`StructuredAgentSessionCreateRefusalError` rejection (and on a refusal whose fallback was never claimed).
## Required outcome
- A refused resume shows exactly one, actionable toast.
## Suggested approach
- Have the resume-in-chat path signal that it handles its own failure (e.g. claim a no-op fallback via `claimDefinitiveRefusalFallback`, or gate `trackLaunchFailureToast` on no caller having registered a notifier), so the generic toast is suppressed when the specific one fired.…tured gate (#19246) Semantic conflict between two green PRs. #19176 added this replay test while `agentSession.*` still admitted a `runtime` client on its negotiated capability alone; #18700 then made `experimentalStructuredNativeChat` one rule for every caller. Neither branch saw the other, and main runs no post-merge test gate, so `agentSession.create` started refusing at the envelope level and the test's `ok: true` expectation broke. #18700's rule is the intended behaviour and `create` starts work, so it belongs behind the gate. The fixture is what is stale: it builds a real `OrcaRuntimeService` whose client settings are unset. Enable the setting the way #18700 already did for the sibling pre-commit fixture. The assertions about durable-identity replay are untouched and now actually run.
…uctured chat (stablyai#19176) * feat(native-chat): resume an Agent Session History row into a new structured chat A Claude or Codex row in Agent Session History gains "Resume in New Chat": it opens a new structured native-chat tab that continues that provider conversation, with the prior turns already in the journal. Until now those rows could only be resumed into a PTY terminal; the structured branch could reveal a chat Orca already owned but could not adopt one it had never held. Almost all of the machinery existed. Both lanes already resume from the record's provider handle chain, the journal already has a transcript importer, and the handle chain already models `adopted` as an origin. The gap was that a create always minted an empty chain, so the adapters started a fresh conversation. This seeds that chain. The client names only the conversation. `agentSession.create` is reachable by paired mobile clients, so the transcript path and the account home are derived by the executing host and validated against the account homes it recognises — a client-supplied path would choose which file the host imports and which credential directory the provider child launches against. Failure refuses rather than degrades. A transcript that cannot be found refuses before anything is created; one that fails or decodes empty *after* the provider has resumed fails the attach, tearing the child down and publishing no tab, because an empty journal beside a context-carrying agent claims a continuity the provider never gave. Codex can resume into any workspace since it is handed the rollout path; Claude resolves transcripts under a project key derived from the launch cwd, so it is offered only for the workspace the conversation was recorded in. * fix(native-chat): widen adopted-home discovery and keep ordinary launches untouched Three corrections from review of the first commit. The adoption's account-home candidates now include the extra Codex homes session discovery already scans. A row this host listed could otherwise refuse to resume, which reads as the feature being broken rather than as a scope. Ordinary launches call `createStructuredAgentSessionLaunchIntent` with two arguments again. Passing the resume source unconditionally appended a trailing `undefined` that four existing call-site assertions had to absorb; the churn was the caller's fault, not the tests'. The transactional adoption guard's comment claimed the self-exemption is what lets a committed create replay. It is not: replay is settled earlier by the operation ledger, and an adoption always arrives with a null expected fence, so a request naming an existing session id is refused a few lines below either way. The exemption is part of what "another record" means, and the comment now says that instead. * fix: preserve history adoption through create and retries * fix: replay committed history adoption from durable identity * fix: validate history before claiming adopted sessions * fix: extract AI vault resume domains * fix: recognize typed history resume refusals --------- Co-authored-by: Merge Sim <sim@local>
…tured gate (stablyai#19246) Semantic conflict between two green PRs. stablyai#19176 added this replay test while `agentSession.*` still admitted a `runtime` client on its negotiated capability alone; stablyai#18700 then made `experimentalStructuredNativeChat` one rule for every caller. Neither branch saw the other, and main runs no post-merge test gate, so `agentSession.create` started refusing at the envelope level and the test's `ok: true` expectation broke. stablyai#18700's rule is the intended behaviour and `create` starts work, so it belongs behind the gate. The fixture is what is stale: it builds a real `OrcaRuntimeService` whose client settings are unset. Enable the setting the way stablyai#18700 already did for the sibling pre-commit fixture. The assertions about durable-identity replay are untouched and now actually run.













What users will see
Users can continue an existing Claude or Codex conversation from Agent Session History in Orca’s chat interface, including conversations originally started in the CLI.
For example, a user who started debugging with Codex in a terminal can find that conversation in history, open it in chat, read the earlier exchange, and ask a follow-up without explaining the problem again. The explicit Resume in New Chat action opens chat even when the user’s default for new agent tabs is terminal mode.
Availability and behavior
Implementation and reliability
The existing create/import flow now resolves the selected conversation and account on the execution host, validates history before acquiring ownership, and imports it into the new chat. Committed retries preserve the same conversation and imported messages, including when the original history file disappears or the selected account changes. Existing single-writer checks prevent duplicate adoption; history decoding remains bounded to 16 MiB and runs once.
Ownership tracking covers Orca-managed sessions; a CLI resume started manually outside Orca remains outside that tracking.
Visual proof
Live Electron dev instance, isolated profile, structured native chat on. Both conversations were created by the CLI (
codex exec/claude -p) with a planted token, then resumed from the history row. Continuity is proven by token recall, not by the model's self-report.1. The action on a history row
2. Codex — resumed, prior turns present, recalls
CODEX-PROOF-77313. Claude — resumed, prior turns present, recalls
CLAUDE-PROOF-4482Both rows went from 2 to 4 messages and became structured-owned afterwards — the resume continued the same conversation rather than forking it, and the one-way ownership above is observed, not just reasoned.
Validation
d5d70af577, including eight test shards, static analysis, typecheck, wire compatibility, and both packaging jobs.Live UI validation used an isolated macOS dev app. No live Windows/Linux/SSH/WSL/mobile or packaged-app smoke test is claimed.