diff --git a/src/acp-agent.ts b/src/acp-agent.ts index f107af74..1c8e8bb7 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -656,6 +656,39 @@ export function isLocalCommandMetadata(content: unknown): boolean { return stripLocalCommandMetadata(content) === null; } +/** + * True for the synthetic assistant message the CLI injects into the transcript + * when a turn fails authentication (e.g. "Not logged in · Please run /login", + * "Session expired. Please run /login to sign in again."). The `/login` + * instruction is Claude Code TUI-specific and meaningless to ACP clients + * (issue #863). The live prompt loop suppresses the text and fails the turn + * with `authRequired` so the client can run its own auth flow; replay must + * skip it too — both for parity with what the client saw live and because the + * message stays in the transcript forever, so it would resurface on every + * session/load even after the user has logged back in. + * + * Takes the API message (`message.message`), which replay only knows as + * `unknown`. The persisted record's structured `error: "authentication_failed"` + * marker is stripped by `getSessionMessages`, so the synthetic model + text is + * all both paths have to match on. + */ +export function isSyntheticLoginMessage(apiMessage: unknown): boolean { + if (!apiMessage || typeof apiMessage !== "object") { + return false; + } + const { model, content } = apiMessage as { model?: unknown; content?: unknown }; + if (model !== "" || !Array.isArray(content) || content.length !== 1) { + return false; + } + const block = content[0] as { type?: unknown; text?: unknown } | null; + return ( + !!block && + block.type === "text" && + typeof block.text === "string" && + block.text.includes("Please run /login") + ); +} + const PERMISSION_MODE_ALIASES: Record = { auto: "auto", default: "default", @@ -2570,14 +2603,7 @@ export class ClaudeAcpAgent { break; } - if ( - message.type === "assistant" && - message.message.model === "" && - Array.isArray(message.message.content) && - message.message.content.length === 1 && - message.message.content[0].type === "text" && - message.message.content[0].text.includes("Please run /login") - ) { + if (message.type === "assistant" && isSyntheticLoginMessage(message.message)) { failActive(RequestError.authRequired()); break; } @@ -3192,6 +3218,13 @@ export class ClaudeAcpAgent { replaySession.messageIdToUuid.set(replayMessageId, message.uuid); } + // The live prompt loop converts the synthetic "Please run /login" + // assistant message into an authRequired error instead of showing its + // TUI-specific text; skip it on replay too (issue #863). + if (message.type === "assistant" && isSyntheticLoginMessage(message.message)) { + continue; + } + // @ts-expect-error - untyped in SDK but we handle all of these let content: unknown = message.message.content; // @ts-expect-error - untyped in SDK but we handle all of these diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index f6de2292..45bff93f 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -30,6 +30,7 @@ import { toAcpNotifications, promptToClaude, isLocalCommandMetadata, + isSyntheticLoginMessage, stripLocalCommandMetadata, ClaudeAcpAgent, claudeCliPath, @@ -59,6 +60,10 @@ vi.mock("@anthropic-ai/claude-agent-sdk", async (importOriginal) => { ...actual, deleteSession: vi.fn(), getSessionInfo: vi.fn(), + // Delegates to the real implementation so integration tests that read + // actual transcripts keep working; unit tests override per-call with + // `mockResolvedValueOnce`. + getSessionMessages: vi.fn(actual.getSessionMessages), }; }); import type { @@ -1470,6 +1475,94 @@ describe("isLocalCommandMetadata", () => { }); }); +describe("synthetic login message (issue #863)", () => { + // The exact shape the CLI persists (and streams) when a turn fails auth: + // model "", a single text block, structured `error` stripped by + // getSessionMessages. + const syntheticLoginApiMessage = { + id: "0a1dfa6b-1c2f-4aa2-8bff-ae1690acd6e1", + model: "", + role: "assistant", + type: "message", + stop_reason: "stop_sequence", + content: [{ type: "text", text: "Not logged in · Please run /login" }], + }; + + it("isSyntheticLoginMessage matches the CLI auth-error message", () => { + expect(isSyntheticLoginMessage(syntheticLoginApiMessage)).toBe(true); + expect( + isSyntheticLoginMessage({ + ...syntheticLoginApiMessage, + content: [{ type: "text", text: "Session expired. Please run /login to sign in again." }], + }), + ).toBe(true); + }); + + it("isSyntheticLoginMessage does not match real assistant messages", () => { + // Real model output that merely mentions the phrase. + expect( + isSyntheticLoginMessage({ + ...syntheticLoginApiMessage, + model: "claude-sonnet-5", + }), + ).toBe(false); + // Other synthetic error texts (e.g. API errors) still replay as-is. + expect( + isSyntheticLoginMessage({ + ...syntheticLoginApiMessage, + content: [{ type: "text", text: "API Error: 500 Internal Server Error" }], + }), + ).toBe(false); + expect(isSyntheticLoginMessage(undefined)).toBe(false); + expect(isSyntheticLoginMessage("Not logged in · Please run /login")).toBe(false); + }); + + it("loadSession replay skips the synthetic login message but keeps the rest", async () => { + const updates: SessionNotification[] = []; + const client = { + sessionUpdate: async (u: SessionNotification) => { + updates.push(u); + }, + } as unknown as AcpClient; + const agent = new ClaudeAcpAgent(client, { log: () => {}, error: () => {} }); + + vi.mocked(getSessionMessages).mockResolvedValueOnce([ + { + type: "user", + uuid: "u1", + session_id: "s1", + parent_tool_use_id: null, + parent_agent_id: null, + message: { role: "user", content: [{ type: "text", text: "hi, say one word" }] }, + }, + { + type: "assistant", + uuid: "a1", + session_id: "s1", + parent_tool_use_id: null, + parent_agent_id: null, + message: syntheticLoginApiMessage, + }, + ] as Awaited>); + + await ( + agent as unknown as { replaySessionHistory(sessionId: string): Promise } + ).replaySessionHistory("s1"); + + // The user's prompt still replays… + expect( + updates.some( + (u) => + u.update.sessionUpdate === "user_message_chunk" && + u.update.content.type === "text" && + u.update.content.text.includes("hi, say one word"), + ), + ).toBe(true); + // …but the TUI-specific "/login" instruction never reaches the client. + expect(JSON.stringify(updates)).not.toContain("/login"); + }); +}); + describe("escape markdown", () => { it("should escape markdown characters", () => { let text = "Hello *world*!";