fix: use ACP auth errors without access updates (#1179)

Authentication failures now use the ACP authRequired error only. The adapter does not send a second access failure or login text as a chat message. Provider verification and cloud credential errors use the service category because signing in cannot resolve them. Checks: npm run check; npm run build; npm run test:run (1468 passed, 28 skipped).
2026-10-27 04:30:59 +00:00 · 2026-09-25 15:11:37 +02:00
parent 5dbb453c63
commit e6681d2a57
5 changed files with 49 additions and 183 deletions
--- 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 } : {}),
--- 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<Query, "accountInfo">;
  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<RequestError | undefined> {
  const account = await readGuardAccount(options);
  if (!account) {
@@ -259,14 +255,9 @@ async function evaluateGuard(options: GuardOptions): Promise<RequestError | unde
    );
  }
  if (billsClaudeSubscription(account)) {
-    await publishRefusal(options.sessionFailures(), {
-      details: CLAUDE_SUBSCRIPTION_NOT_SUPPORTED_MESSAGE,
-      reason: CLAUDE_SUBSCRIPTION_NOT_SUPPORTED_REASON,
-    });
    return claudeSubscriptionNotSupportedError();
  }
  if (!holdsNonSubscriptionCredential(account)) {
-    await publishRefusal(options.sessionFailures(), { details: CLAUDE_LOGIN_REQUIRED_MESSAGE });
    return claudeLoginRequiredError();
  }
  return undefined;
@@ -293,19 +284,3 @@ async function readGuardAccount(options: GuardOptions): Promise<AccountInfo | un
    return undefined;
  }
 }
-
-/** The refusal state does not change between turns: publish once, and skip a
- *  republish while the previous failure with this reason is still active. */
-async function publishRefusal(
-  sessionFailures: SessionFailureController,
-  failure: { details: string; reason?: string },
-): Promise<void> {
-  if (sessionFailures.hasActiveSessionError("auth_required", failure.reason)) {
-    return;
-  }
-  await sessionFailures.publish("auth_required", {
-    sessionScoped: true,
-    details: failure.details,
-    ...(failure.reason ? { reason: failure.reason } : {}),
-  });
-}
--- 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.",
  },
--- 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 = "<synthetic>";
    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 () => {
--- 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.