diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 59fd70e5..aa7276a8 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -184,8 +184,6 @@ import { } from "./session-failure-extension.js"; import { billsClaudeSubscription, - CLAUDE_SUBSCRIPTION_NOT_SUPPORTED_MESSAGE, - CLAUDE_SUBSCRIPTION_NOT_SUPPORTED_REASON, claudeLoginRequiredError, type ClaudeSubscriptionGuardState, claudeSubscriptionNotSupportedError, @@ -3122,15 +3120,6 @@ export class ClaudeAcpAgent { // The guard reads the account anyway; reuse that read to keep the // reported identity current, refusal or not. onAccount: (account) => this.publishSessionAccountIdentity(account), - sessionFailures: () => - new SessionFailureController({ - sessionId, - state: session.sessionFailureState, - capabilities: this.clientCapabilities, - isCurrent: () => this.sessions[sessionId] === session, - sendUpdate: (notification) => this.client.sessionUpdate(notification), - logger: this.logger, - }), }); } @@ -4000,12 +3989,7 @@ export class ClaudeAcpAgent { * signal that starts the client's own auth flow (AIR parks the refused * prompt, shows its method chooser, and resumes the prompt after sign-in), * so replacing it with a successful `end_turn` would disable that flow. - * Every client keeps the rejection; capable clients additionally receive - * the signed-out *state* as one session-scoped failure whose `login` - * action remains the way back in after the client's auth prompt is - * dismissed. Its title stays the policy's client-neutral fallback — the - * CLI's own text ("… Please run /login") is TUI advice, meaningless in an - * ACP client, so it travels as expandable details instead. */ + * Every client keeps the rejection. The client owns the login UI. */ const failActiveWithSessionFailure = async ( kind: ClaudeFailureKind, error: unknown, @@ -4015,22 +3999,10 @@ export class ClaudeAcpAgent { await compaction.interrupt(); } if (kind === "auth_required") { - // One sign-out arrives twice — the synthetic login assistant message - // and the turn's error-shaped result repeat the same text — and the - // signed-out state does not change in between: publish once, and skip - // republishing while the previous sign-out is still active. A retry - // warning of the same kind is not a sign-out and must not suppress it. - if (!sessionFailures.hasActiveSessionError(kind)) { - await publishSessionFailure(kind, { turnScoped: false, details: title }); - } - // Preserve legacy codes: only explicit `/login` signals trigger ACP's auth flow. - // The first delivery rejects the turn, so the second one finds it settled. - // That is the normal course here, not the lost-failure case `failActive` - // logs an error for. - if (session.activeTurn && !session.activeTurn.settled) failActive(error); - // The row is published and the turn is rejected. Under - // `--hide-claude-auth` the account cached at `initialize` is now - // wrong, so end the query and let the next turn recreate it. + // The auth error starts the client's login flow. Do not send an access + // failure as a second notification or render login text in the chat. + if (session.activeTurn && !session.activeTurn.settled) + failActive(RequestError.authRequired()); this.markSessionForSignOutRespawn(params.sessionId, session); return; } @@ -5567,9 +5539,7 @@ export class ClaudeAcpAgent { // it reports a login the query process itself runs, and a // client can sign the user in out of band (AIR runs // `claude /login` in a terminal, in a separate process). - // Without this a stale signed-out record would outlive the - // sign-out and suppress the next one, which the - // `auth_required` dedupe below reads. + // Clear any older signed-out record after a real answer. failure.recoveryPolicy === "auth_status" || (failure.severity === "warning" && failure.turnId === activeTurnId), ); @@ -8959,9 +8929,7 @@ export class ClaudeAcpAgent { } } - /** Tell a capable client that a session died during a provider switch. The - * session is already removed, so the failure is published on the state it - * left behind, with the refusal reason when the error carries one. */ + /** Tell a capable client when a non-auth error ends a session during a provider switch. */ private reportSessionLostOnProviderUpdate( sessionId: string, session: Session, @@ -8971,6 +8939,7 @@ export class ClaudeAcpAgent { `Session ${sessionId}: could not be recreated for the provider update: ${error}`, ); const isAuthRequired = error instanceof RequestError && error.code === AUTH_REQUIRED_CODE; + if (isAuthRequired) return; const reason = error instanceof RequestError && typeof error.data === "object" && @@ -8979,12 +8948,7 @@ export class ClaudeAcpAgent { typeof error.data.reason === "string" ? error.data.reason : undefined; - const details = - reason === CLAUDE_SUBSCRIPTION_NOT_SUPPORTED_REASON - ? CLAUDE_SUBSCRIPTION_NOT_SUPPORTED_MESSAGE - : error instanceof Error - ? error.message - : String(error); + const details = error instanceof Error ? error.message : String(error); const controller = new SessionFailureController({ sessionId, state: session.sessionFailureState, @@ -8996,7 +8960,7 @@ export class ClaudeAcpAgent { logger: this.logger, }); void controller - .publish(isAuthRequired ? "auth_required" : "internal_error", { + .publish("internal_error", { sessionScoped: true, details, ...(reason ? { reason } : {}), diff --git a/src/hide-claude-auth.ts b/src/hide-claude-auth.ts index 59624802..2fd51583 100644 --- a/src/hide-claude-auth.ts +++ b/src/hide-claude-auth.ts @@ -33,7 +33,6 @@ import { RequestError } from "@agentclientprotocol/sdk"; import type { AccountInfo, Query } from "@anthropic-ai/claude-agent-sdk"; -import type { SessionFailureController } from "./session-failure-extension.js"; export function shouldHideClaudeAuth(): boolean { return process.argv.includes("--hide-claude-auth"); @@ -144,7 +143,7 @@ export function claudeLoginRequiredError(): RequestError { /** * The state the guard keeps for one session. It lives on the session so that a - * concurrent pair of turns shares one account read and one failure row, and so + * concurrent pair of turns shares one account read, and so * that each degraded-guard warning is logged once. */ export type ClaudeSubscriptionGuardState = { @@ -190,8 +189,6 @@ type GuardOptions = { sessionId: string; query: Pick; guardState: ClaudeSubscriptionGuardState; - /** Built lazily: most turns pass and never need a controller. */ - sessionFailures: () => SessionFailureController; logger: GuardLogger; /** Display hook: called with the account the guard just read, before the * guard decides on it. It exists so a refused turn still reports which @@ -219,8 +216,7 @@ type GuardOptions = { * mid-session. The refusal mirrors the signed-out path exactly (see * `failActiveWithSessionFailure` for `auth_required` in `acp-agent.ts`): every * client gets the `authRequired` JSON-RPC rejection that starts its own auth - * flow, and a capable client additionally gets one session-scoped access - * failure whose `login` action stays the way back in. + * flow. The guard sends no separate access message. * * Fails open when no account can be read at all: session creation already * refused every account without a usable credential, and an unavailable probe @@ -241,8 +237,8 @@ export async function refuseClaudeSubscriptionTurn(options: GuardOptions): Promi } } -/** Read the account, publish at most one failure row, and return the error the - * turn must be rejected with. Never throws: an unreadable account fails open. */ +/** Read the account and return the error for the refused turn. + * Never throws: an unreadable account fails open. */ async function evaluateGuard(options: GuardOptions): Promise { const account = await readGuardAccount(options); if (!account) { @@ -259,14 +255,9 @@ async function evaluateGuard(options: GuardOptions): Promise { - if (sessionFailures.hasActiveSessionError("auth_required", failure.reason)) { - return; - } - await sessionFailures.publish("auth_required", { - sessionScoped: true, - details: failure.details, - ...(failure.reason ? { reason: failure.reason } : {}), - }); -} diff --git a/src/session-failure-extension.ts b/src/session-failure-extension.ts index 3f4609cf..ff9164c5 100644 --- a/src/session-failure-extension.ts +++ b/src/session-failure-extension.ts @@ -99,7 +99,7 @@ const AIR_FAILURE_POLICY: Record< // credentials must be refreshed outside Claude. No `login` action, and not // sticky like `auth_required` (nothing clears at `auth_status`). access_denied: { - category: "access", + category: "service", actions: ["retry"], fallbackTitle: "Claude could not access the account or provider credentials.", }, diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index 27afad29..5db39faa 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -7110,8 +7110,8 @@ describe("stop reason propagation", () => { it.each([ ["billing_error", "limit"], ["account_on_hold", "limit"], - ["verification_required", "access"], - ["cloud_credential_error", "access"], + ["verification_required", "service"], + ["cloud_credential_error", "service"], ["rate_limit", "limit"], ["overloaded", "service"], ["invalid_request", "request"], @@ -7293,7 +7293,7 @@ describe("stop reason propagation", () => { const sessionFailuresFromUpdates = (updates: SessionNotification[]) => updates.map((u) => (u.update as any)?._meta?.jetbrains?.air?.sessionFailure).filter(Boolean); - it("rejects authentication_failed and publishes a session-scoped access failure", async () => { + it("rejects authentication_failed through ACP without an access update", async () => { const updates: SessionNotification[] = []; const agent = new ClaudeAcpAgent( { @@ -7314,26 +7314,13 @@ describe("stop reason propagation", () => { }), ]); - // The auth flow hangs off the JSON-RPC rejection (the client parks the - // prompt and runs its own sign-in), so the turn is rejected even for - // capable clients; the signed-out state travels separately as one - // session-scoped failure with a client-neutral title. + // The JSON-RPC rejection starts the client's sign-in flow. await expect( agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }), - ).rejects.toThrow(); + ).rejects.toMatchObject({ code: -32000 }); await agent.sessions["test-session"]?.consumer; const failures = sessionFailuresFromUpdates(updates); - expect(failures).toHaveLength(1); - expect(failures[0]).toEqual( - expect.objectContaining({ - category: "access", - severity: "error", - title: "Sign in to continue using Claude.", - details: "Claude request failed.", - actions: ["login"], - }), - ); - expect(failures[0].id).toContain("session-error"); + expect(failures).toEqual([]); }); it("recovers auth_required internally after a successful auth_status", async () => { @@ -7368,18 +7355,8 @@ describe("stop reason propagation", () => { agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }), ).rejects.toThrow(); await agent.sessions["test-session"]?.consumer; - const failures = sessionFailuresFromUpdates(updates); - expect(failures).toHaveLength(1); - expect(failures[0]).toEqual( - expect.objectContaining({ - category: "access", - revision: 1, - details: "Authentication required.", - }), - ); - expect(agent.sessions["test-session"].sessionFailureState.active.has(failures[0].id)).toBe( - false, - ); + expect(sessionFailuresFromUpdates(updates)).toEqual([]); + expect(agent.sessions["test-session"].sessionFailureState.active.size).toBe(0); expect(JSON.stringify(updates)).not.toContain("private login token"); }); @@ -7416,12 +7393,8 @@ describe("stop reason propagation", () => { agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }), ).rejects.toThrow(); await agent.sessions["test-session"]?.consumer; - const failures = sessionFailuresFromUpdates(updates); - expect(failures).toHaveLength(1); - expect(failures[0]).toEqual(expect.objectContaining({ category: "access", severity: "error" })); - expect(agent.sessions["test-session"].sessionFailureState.active.has(failures[0].id)).toBe( - true, - ); + expect(sessionFailuresFromUpdates(updates)).toEqual([]); + expect(agent.sessions["test-session"].sessionFailureState.active.size).toBe(0); expect(JSON.stringify(updates)).not.toContain("private login token"); expect(JSON.stringify(updates)).not.toContain("private login error"); }); @@ -7445,8 +7418,7 @@ describe("stop reason propagation", () => { (agent as any).clientCapabilities = airSessionFailureCapabilities; // The CLI reports one signed-out turn twice: the synthetic login assistant // message rejects the turn, then the result repeats the same text. One - // session-scoped failure covers both; its title is the client-neutral - // fallback while the CLI's TUI advice travels as details. + // The ACP auth error covers both. The CLI's TUI advice stays local. const syntheticLogin = createAssistantError("authentication_failed"); syntheticLogin.message.model = ""; syntheticLogin.message.content = [ @@ -7466,19 +7438,8 @@ describe("stop reason propagation", () => { agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }), ).rejects.toThrow(); await agent.sessions["test-session"]?.consumer; - const failures = sessionFailuresFromUpdates(updates); - expect(failures).toHaveLength(1); - expect(failures[0]).toEqual( - expect.objectContaining({ - category: "access", - severity: "error", - title: "Sign in to continue using Claude.", - details: "Not logged in · Please run /login", - }), - ); - expect([...agent.sessions["test-session"].sessionFailureState.active.keys()]).toEqual([ - failures[0].id, - ]); + expect(sessionFailuresFromUpdates(updates)).toEqual([]); + expect(agent.sessions["test-session"].sessionFailureState.active.size).toBe(0); // The first delivery already rejected the turn, so the second one has no // turn to fail. That is expected here and must not be logged as a fault. expect(logged.join("\n")).not.toContain("cannot fail active turn"); @@ -7513,7 +7474,7 @@ describe("stop reason propagation", () => { expect(sessionFailuresFromUpdates(updates)).toEqual([]); }); - it("republishes the signed-out state after a real model answer cleared the last one", async () => { + it("rejects a later sign-out after a real model answer", async () => { const updates: SessionNotification[] = []; const agent = new ClaudeAcpAgent( { @@ -7579,13 +7540,8 @@ describe("stop reason propagation", () => { agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "three" }] }), ).rejects.toThrow(); - // The next sign-out reaches the client as its own row, not as silence. - const failures = sessionFailuresFromUpdates(updates); - expect(failures).toHaveLength(2); - expect(failures[1].id).not.toEqual(failures[0].id); - expect(failures[1]).toEqual( - expect.objectContaining({ category: "access", severity: "error", actions: ["login"] }), - ); + // The next sign-out also uses the ACP auth error. + expect(sessionFailuresFromUpdates(updates)).toEqual([]); }); it("publishes no retry warning for an authentication_failed api_retry", async () => { @@ -7600,8 +7556,7 @@ describe("stop reason propagation", () => { ); (agent as any).clientCapabilities = airSessionFailureCapabilities; // The 401 retry is the CLI's credential re-check. The sign-out that - // follows is the signal, so the retry publishes nothing and the only - // failure is the session-scoped error with the `login` action. + // follows is the signal. Neither event publishes an access update. injectSession(agent, [ { type: "system", @@ -7627,17 +7582,7 @@ describe("stop reason propagation", () => { agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }), ).rejects.toThrow(); await agent.sessions["test-session"]?.consumer; - const failures = sessionFailuresFromUpdates(updates); - expect(failures).toHaveLength(1); - expect(failures[0]).toEqual( - expect.objectContaining({ - category: "access", - severity: "error", - title: "Sign in to continue using Claude.", - actions: ["login"], - }), - ); - expect(failures[0].id).toContain("session-error"); + expect(sessionFailuresFromUpdates(updates)).toEqual([]); }); it("keeps the historical failure record unchanged after recovery", async () => { diff --git a/src/tests/authorization.test.ts b/src/tests/authorization.test.ts index 39ab52b0..d74ea986 100644 --- a/src/tests/authorization.test.ts +++ b/src/tests/authorization.test.ts @@ -783,7 +783,7 @@ describe("authorization", () => { ]); }); - it("publishes one session-scoped access failure for capable clients", async () => { + it("does not publish an access failure for capable clients", async () => { const [agent, updates] = createRecordingAgent(true); hideClaudeAuth(); mockAccount(SIGNED_IN_WITH_KEY, { @@ -794,20 +794,8 @@ describe("authorization", () => { await expect(agent.prompt(promptParams(sessionId))).rejects.toMatchObject(refusal); await expect(agent.prompt(promptParams(sessionId))).rejects.toMatchObject(refusal); - const failures = failuresIn(updates); - expect(failures).toHaveLength(1); - expect(failures[0]).toEqual( - expect.objectContaining({ - category: "access", - severity: "error", - title: "Sign in to continue using Claude.", - details: MESSAGE, - reason: REASON, - actions: ["login"], - }), - ); - expect(failures[0].turnId).toBeUndefined(); - expect([...agent.sessions[sessionId].sessionFailureState.active.values()]).toHaveLength(1); + expect(failuresIn(updates)).toEqual([]); + expect([...agent.sessions[sessionId].sessionFailureState.active.values()]).toEqual([]); }); it("gives a client without the capability the reason on the error only", async () => { @@ -820,7 +808,7 @@ describe("authorization", () => { expect(failuresIn(updates)).toEqual([]); }); - it("publishes one failure row for two concurrent prompts", async () => { + it("does not publish a failure row for two concurrent prompts", async () => { const [agent, updates] = createRecordingAgent(true); hideClaudeAuth(); mockAccount(SIGNED_IN_WITH_KEY, { accountInfo: async () => ({ subscriptionType: "pro" }) }); @@ -835,7 +823,7 @@ describe("authorization", () => { for (const outcome of outcomes) { expect((outcome as PromiseRejectedResult).reason).toMatchObject(refusal); } - expect(failuresIn(updates)).toHaveLength(1); + expect(failuresIn(updates)).toEqual([]); }); it("survives a cancel that races the refused prompt", async () => { @@ -1052,9 +1040,7 @@ describe("authorization", () => { expect(mockQuery.mock.calls[1][0].options).toMatchObject({ resume: sessionId }); // The refusal keeps the husk, so the client can sign in and retry here. expect(agent.sessions[sessionId]).toBeDefined(); - expect(failuresIn(updates).filter((failure) => failure.kind !== "advisory")).toHaveLength( - 1, - ); + expect(failuresIn(updates).filter((failure) => failure.kind !== "advisory")).toEqual([]); }); it("proceeds when the CLI now holds a key, on a clean failure state", async () => { @@ -1474,7 +1460,7 @@ describe("authorization", () => { }); }); - describe("reason-aware failure dedupe", () => { + describe("auth refusal and existing failure state", () => { /** A controller on the session's own state, publishing through the same * client, so its rows land in the same `updates` array the agent uses. */ function controllerFor(agent: ClaudeAcpAgent, sessionId: string) { @@ -1493,7 +1479,7 @@ describe("authorization", () => { prompt: [{ type: "text" as const, text: "hello" }], }); - it("publishes the subscription row while a plain sign-out is active", async () => { + it("does not add a subscription row while a plain sign-out is active", async () => { const [agent, updates] = createRecordingAgent(true); hideClaudeAuth(); mockAccount(SIGNED_IN_WITH_KEY, { accountInfo: async () => ({ subscriptionType: "pro" }) }); @@ -1503,12 +1489,11 @@ describe("authorization", () => { await expect(agent.prompt(promptParams(sessionId))).rejects.toMatchObject(refusal); const failures = failuresIn(updates); - expect(failures).toHaveLength(2); + expect(failures).toHaveLength(1); expect(failures[0].reason).toBeUndefined(); - expect(failures[1].reason).toBe(REASON); }); - it("publishes a plain sign-out while the subscription row is active", async () => { + it("does not add a subscription row before a later sign-out", async () => { const [agent, updates] = createRecordingAgent(true); hideClaudeAuth(); mockAccount(SIGNED_IN_WITH_KEY, { accountInfo: async () => ({ subscriptionType: "pro" }) }); @@ -1517,16 +1502,14 @@ describe("authorization", () => { await expect(agent.prompt(promptParams(sessionId))).rejects.toMatchObject(refusal); const controller = controllerFor(agent, sessionId); - // This is the check `failActiveWithSessionFailure` makes before it - // publishes a sign-out. The subscription row must not answer it. + // The guard does not change the session failure state. expect(controller.hasActiveSessionError("auth_required")).toBe(false); - expect(controller.hasActiveSessionError("auth_required", REASON)).toBe(true); + expect(controller.hasActiveSessionError("auth_required", REASON)).toBe(false); await controller.publish("auth_required", { sessionScoped: true }); const failures = failuresIn(updates); - expect(failures).toHaveLength(2); - expect(failures[0].reason).toBe(REASON); - expect(failures[1].reason).toBeUndefined(); + expect(failures).toHaveLength(1); + expect(failures[0].reason).toBeUndefined(); }); }); @@ -1565,12 +1548,11 @@ describe("authorization", () => { await expect(agent.unstable_disableProvider({ providerId: "main" })).resolves.toEqual({}); - // The loop ran to the end: both sessions are gone and both reported. + // The loop ran to the end. Both sessions are gone. expect(agent.sessions[first.sessionId]).toBeUndefined(); expect(agent.sessions[second.sessionId]).toBeUndefined(); const failures = failuresIn(updates).filter((failure) => failure.reason === REASON); - expect(failures).toHaveLength(2); - expect(failures[0]).toMatchObject({ category: "access", details: MESSAGE }); + expect(failures).toEqual([]); // `providerUpdate` is not poisoned: the next call reaches the guard // and reports the guard's own reason.