Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions src-tauri/src/acp/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7386,6 +7386,13 @@ fn stop_reason_to_str(reason: StopReason) -> &'static str {
/// same `SessionLoadFailed` banner (Reload / New conversation) instead of a raw
/// protocol error.
///
/// A third case is archived rather than lost: `codex archive <id>` parks a
/// rollout, and a later `session/load` answers -32603 with a body naming both
/// the session and the command that brings it back. That one is a *recoverable*
/// state, so it earns its own code — the banner can name the fix — but it takes
/// the same banner rather than the silent `session/new` fallback, which would
/// orphan a history the user is one command away from restoring.
///
/// Returns `None` for failures that must keep the existing behavior:
/// "Method not found" (agent lacks resume → silent `session/new` fallback),
/// "Authentication required" (silent stop), and any other error (emit
Expand All @@ -7397,6 +7404,14 @@ fn classify_session_load_failure(
if matches!(code, sacp::schema::ErrorCode::ResourceNotFound) {
return Some("resource_not_found");
}
// codex-acp on an archived rollout: the -32603 body reads
// "session <id> is archived. Run `codex unarchive <id>` …". Matched on the
// wire message for the same reason as the family below — the code is a
// generic Internal error. Checked BEFORE that family so the more specific
// (and recoverable) verdict wins if a body ever carries both signals.
if message.contains("is archived") {
return Some("session_archived");
}
// Upstream signals for an unrecoverable session (claude-agent-acp 0.58.1):
// - "process exited" → "Claude Code process exited with code 1",
// "The Claude Agent process exited unexpectedly…"
Expand Down Expand Up @@ -13480,6 +13495,46 @@ mod tests {
);
}

#[test]
fn classify_load_failure_names_an_archived_session() {
// The reported case: `codex archive <id>`, then reopen the conversation.
// codex-acp answers session/load with a generic -32603 whose data names
// the session and the command that restores it.
let archived = "Internal error: {\n \"details\": \"session \
019bf0c4-4d1a-7c3e-9f21-6a0e5b8d2c47 is archived. Run `codex \
unarchive 019bf0c4-4d1a-7c3e-9f21-6a0e5b8d2c47` to restore it.\"\n}";
assert_eq!(
classify_session_load_failure(sacp::schema::ErrorCode::InternalError, archived),
Some("session_archived"),
);

// Archived is the more specific verdict: a body carrying both signals
// must not degrade into the generic "unavailable" family, which offers
// the user no way back.
assert_eq!(
classify_session_load_failure(
sacp::schema::ErrorCode::InternalError,
"Session not found: session abc is archived.",
),
Some("session_archived"),
);

// Codex reads history back out of its own rollout store, so an archived
// session must stop with the banner — silently opening a new session
// would orphan history that one command restores.
assert!(!recovers_load_failure_locally(
AgentType::Codex,
Some("session_archived")
));
// A custom agent's history is codeg's own transcript, so it keeps the
// silent local recovery it has for the other classified failures.
let custom = AgentType::custom("glm-acp-agent").expect("valid id");
assert!(recovers_load_failure_locally(
custom,
Some("session_archived")
));
}

#[test]
fn classify_load_failure_keeps_existing_behavior_for_recoverable_errors() {
// "Method not found" (agent lacks resume) and "Authentication required"
Expand Down
14 changes: 8 additions & 6 deletions src-tauri/src/acp/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,15 +367,17 @@ pub enum AcpEvent {
/// text chunks), so severity-`warning` records take over the retry-banner
/// role on those connections.
SessionFailure { record: SessionFailureRecord },
/// `session/load` failed in a non-recoverable way (e.g. the agent has no
/// record of this `session_id`). Emitted instead of silently falling back
/// to `session/new`, so the frontend can surface the failure with reload
/// / new-conversation actions.
/// `session/load` failed in a way codeg cannot paper over — the agent has
/// no record of this `session_id`, the session/process died, or it is
/// archived. Emitted instead of silently falling back to `session/new`, so
/// the frontend can surface the failure with reload / new-conversation
/// actions.
SessionLoadFailed {
session_id: String,
message: String,
/// Stable machine-readable identifier — currently
/// `"resource_not_found"` for JSON-RPC -32002.
/// Stable machine-readable identifier: `"resource_not_found"` for
/// JSON-RPC -32002, or `"session_unavailable"` / `"session_archived"`
/// matched on the wire message. See `classify_session_load_failure`.
code: String,
},
/// Available slash commands updated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,22 @@ describe("ConversationDetailPanel session-load failure surface", () => {
expect(banner).toContain("hasPersistedConversation && acpLoadError")
expect(banner).toContain("handleReloadDetail")
expect(banner).toContain("handleOpenNewSession")
// A failure with a runnable fix (archived session → `codex unarchive
// <id>`) offers it as a copy action. The message itself renders in a
// one-line ellipsized strip, so a 36-char session id inside the prose is
// exactly what gets truncated away — the button is what makes the
// command reachable at all, and it must not show when there is no
// command to copy.
expect(banner).toContain("{recoveryCommand && (")
expect(banner).toContain("handleCopyRecoveryCommand")
// Every action is shrink-0 and the message is the only elastic child, so
// a third action has to be able to wrap. Without `flex-wrap` plus a floor
// under the message, the row silently pushes "New conversation" outside
// the banner at narrow widths (measured 34-172px past the edge at
// 320-384px) — i.e. adding a recovery action would break the two that
// were already there.
expect(banner).toContain("flex w-full flex-wrap items-center")
expect(banner).toContain("min-w-40 flex-1 overflow-hidden")
// The shell renders the banner inside the composer dock, constrained to
// the same message-column width as the input it replaces.
const dockIdx = conversationShellSource.indexOf("{composerBanner && (")
Expand Down
60 changes: 57 additions & 3 deletions src/components/conversations/conversation-detail-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react"
import {
AlertCircle,
Check,
Copy,
Download,
FileCode,
FileImage,
Expand All @@ -28,7 +30,7 @@ import { useTabActions, useTabStore } from "@/contexts/tab-context"
import { groupOfTab, isReparentUnmount } from "@/stores/tab-store"
import { computeRects, leafIds } from "@/lib/tab-group-layout"
import { useTaskContext } from "@/contexts/task-context"
import { cn, randomUUID } from "@/lib/utils"
import { cn, copyTextToClipboard, randomUUID } from "@/lib/utils"
import { buildAskPrompt, buildQuotedMarkdown } from "@/lib/message-quote"
import {
ASK_SELECTION_PARKED_EVENT,
Expand Down Expand Up @@ -1690,24 +1692,76 @@ const ConversationTabView = memo(function ConversationTabView({
closeTab(tabId)
}, [closeTab, folder, openNewConversationTab, tabId, workingDirForConnection])

// Some load failures come with a shell command that undoes them (today:
// `codex unarchive <id>`). The banner names it in prose, but prose here is
// one ellipsized line — so offer the exact string as a copy action too,
// rather than asking the user to retype a session id they may not even be
// able to see. Read off the connection, which is where the connections
// layer parks it beside the localized message.
const recoveryCommand = conn.loadErrorCommand
const [commandCopied, setCommandCopied] = useState(false)
const copiedResetRef = useRef<ReturnType<typeof setTimeout> | null>(null)
useEffect(
() => () => {
if (copiedResetRef.current) clearTimeout(copiedResetRef.current)
},
[]
)
const handleCopyRecoveryCommand = useCallback(async () => {
if (!recoveryCommand) return
const ok = await copyTextToClipboard(recoveryCommand)
if (!ok) return
setCommandCopied(true)
if (copiedResetRef.current) clearTimeout(copiedResetRef.current)
copiedResetRef.current = setTimeout(() => setCommandCopied(false), 1500)
}, [recoveryCommand])

// A session/load failure no longer hijacks the whole message area (the
// transcript stays readable — see message-list-view's blockingLoadError);
// instead the failure lands here, as a banner docked where the composer
// sits, carrying the same Reload / New session recovery actions. Persisted
// conversations only: drafts never session/load.
const acpLoadErrorBanner =
hasPersistedConversation && acpLoadError ? (
// `flex-wrap` + a message floor, because the actions are all `shrink-0`
// and the row has no other give: without them a third action pushes the
// message to zero width and shoves "New conversation" outside the
// banner (measured: 34-172px past the edge at 320-384px, worse in
// French/German). Wrapping costs a second row only when the panel is
// actually too narrow — at >=700px this lays out exactly as before.
<div
role="alert"
className="flex w-full items-center gap-2 rounded-lg border border-destructive/30 bg-destructive/5 px-3 py-2 text-xs text-destructive"
className="flex w-full flex-wrap items-center gap-2 rounded-lg border border-destructive/30 bg-destructive/5 px-3 py-2 text-xs text-destructive"
>
<AlertCircle aria-hidden="true" className="h-4 w-4 shrink-0" />
<span
className="min-w-0 flex-1 overflow-hidden text-ellipsis whitespace-nowrap"
className="min-w-40 flex-1 overflow-hidden text-ellipsis whitespace-nowrap"
title={acpLoadError}
>
{acpLoadError}
</span>
{/* Deliberately outside `canShowDetailErrorActions`: that gate exists
because Reload refetches the DB detail and New session opens a tab
in the folder, so both need a conversation id and a folder. Copying
a string needs neither — withholding the one recovery the user can
still act on would be the wrong call. */}
{recoveryCommand && (
<button
type="button"
onClick={handleCopyRecoveryCommand}
title={recoveryCommand}
className="flex shrink-0 items-center gap-1 rounded border border-destructive/40 px-2 py-0.5 font-medium transition-colors hover:bg-destructive/10"
>
{commandCopied ? (
<Check aria-hidden="true" className="h-3 w-3" />
) : (
<Copy aria-hidden="true" className="h-3 w-3" />
)}
{commandCopied
? tMessageList("errorActionCommandCopied")
: tMessageList("errorActionCopyCommand")}
</button>
)}
{canShowDetailErrorActions && (
<>
<button
Expand Down
1 change: 1 addition & 0 deletions src/components/message/sub-agent-session-dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ function makeConnState(overrides: Partial<ConnectionState>): ConnectionState {
sessionFailures: [],
error: null,
loadError: null,
loadErrorCommand: null,
lastAppliedSeq: 0,
isDelegationChild: true,
parentToolUseId: "pt-1",
Expand Down
Loading
Loading