feat(acp): surface bridge auth failure in the UI with fallback / fix-auth (R17)

When the bridged `claude` can't authenticate (detached daemon / no keychain), the turn returns "Not logged in" instead of a real answer. Rather than silently relay that, detect it and let the user choose. - Driver: detect a "Not logged in"-only turn and write a cross-process signal (fusion-acp-bridge-auth.json); a real response clears it (acp-driver test). - Dashboard status: GET /providers/claude-cli/status reports acp.authFailed + authReason from the signal. - UI: the Claude CLI provider card shows an auth-failure banner with "Use claude -p" (sets experimentalFeatures.claudeCliAcp=false) and "I fixed auth — re-test", plus a fix hint (run `claude` to log in). - Enable resolution now recomputes each call with an operator force-override (FUSION_CLAUDE_ACP_FORCE), so the "Use -p" fallback takes effect on the next turn — no restart. claude-acp-enable tests updated. pi-claude-cli + engine tests green; dashboard typecheck clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 12:32:00 -07:00
parent 2e0bcd75c4
commit daa37d08c5
7 changed files with 167 additions and 16 deletions
--- a/packages/dashboard/app/api/legacy.ts
+++ b/packages/dashboard/app/api/legacy.ts
@@ -1674,6 +1674,18 @@ export interface ClaudeCliStatus {
    reason?: string;
  } | null;
  ready: boolean;
+  /** Route A ACP transport state (Claude CLI via the claude-code-cli-acp bridge). */
+  acp?: {
+    /** experimentalFeatures.claudeCliAcp (default ON). */
+    enabled: boolean;
+    /** The acp-runtime plugin published a bundled bridge path. */
+    bridgeAvailable: boolean;
+    /** Claude CLI is actually routing through the bridge (enabled + flag + bridge). */
+    active: boolean;
+    /** The bridged `claude` returned "Not logged in" — needs fallback or re-auth (R17). */
+    authFailed: boolean;
+    authReason?: string;
+  };
 }

 export interface DroidCliStatus {
--- a/packages/dashboard/app/components/ClaudeCliProviderCard.tsx
+++ b/packages/dashboard/app/components/ClaudeCliProviderCard.tsx
@@ -4,6 +4,8 @@ import { Loader2 } from "lucide-react";
 import {
  fetchClaudeCliStatus,
  setClaudeCliEnabled,
+  fetchGlobalSettings,
+  updateGlobalSettings,
  type ClaudeCliStatus,
 } from "../api";
 import { ProviderIcon } from "./ProviderIcon";
@@ -122,6 +124,29 @@ export function ClaudeCliProviderCard({
    [onToggled, refresh],
  );

+  // R17 fallback: the bridge can't authenticate Claude. Turn the ACP transport
+  // off (experimentalFeatures.claudeCliAcp=false) so Claude CLI uses `claude -p`.
+  const handleFallbackToDashP = useCallback(async () => {
+    setBusy("disabling");
+    setLastAction(null);
+    try {
+      const gs = await fetchGlobalSettings();
+      await updateGlobalSettings({
+        experimentalFeatures: { ...(gs.experimentalFeatures ?? {}), claudeCliAcp: false },
+      });
+      if (mountedRef.current) {
+        setLastAction({ kind: "disabled", restartRequired: false });
+      }
+      await refresh();
+    } catch (err) {
+      if (mountedRef.current) {
+        setLastAction({ kind: "error", message: err instanceof Error ? err.message : String(err) });
+      }
+    } finally {
+      if (mountedRef.current) setBusy(null);
+    }
+  }, [refresh]);
+
  const binaryAvailable = status?.binary.available ?? false;
  const currentlyEnabled = status?.enabled ?? authenticated;

@@ -217,6 +242,39 @@ export function ClaudeCliProviderCard({
        </strong>
        {description}
        <ClaudeCliStatusLine status={status} authenticated={authenticated} />
+        {status?.acp?.authFailed && (
+          <div
+            className="onboarding-provider-card__alert"
+            role="alert"
+            data-testid="claude-cli-acp-auth-banner"
+          >
+            <strong>
+              {t("setup.claudeCli.acpAuthFailedTitle", "Claude CLI bridge can't authenticate")}
+            </strong>
+            <p>
+              {status.acp.authReason ??
+                t(
+                  "setup.claudeCli.acpAuthFailed",
+                  "The ACP bridge reached a Claude session that isn't logged in. Fall back to `claude -p`, or fix authentication and re-test.",
+                )}
+            </p>
+            <div className="onboarding-provider-card__actions">
+              <button type="button" onClick={handleFallbackToDashP} disabled={busy !== null}>
+                {busy === "disabling" && <Loader2 className="spin" size={14} />}
+                {t("setup.claudeCli.useDashP", "Use claude -p")}
+              </button>
+              <button type="button" onClick={handleTest} disabled={busy !== null}>
+                {t("setup.claudeCli.recheckAuth", "I fixed auth — re-test")}
+              </button>
+            </div>
+            <p className="onboarding-provider-card__hint">
+              {t(
+                "setup.claudeCli.fixAuthHint",
+                "To fix: run `claude` in a terminal and complete login, then re-test.",
+              )}
+            </p>
+          </div>
+        )}
      </div>
      <div className="onboarding-provider-card__actions">{actions}</div>
      {lastAction && <ClaudeCliActionToast action={lastAction} />}
--- a/packages/dashboard/src/routes/register-auth-routes.ts
+++ b/packages/dashboard/src/routes/register-auth-routes.ts
@@ -1,4 +1,7 @@
 import type { Request } from "express";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { existsSync, readFileSync } from "node:fs";
 import { isGhAvailable, isGhAuthenticated } from "@fusion/core";
 import { probeClaudeCli } from "../claude-cli-probe.js";
 import { probeDroidCli } from "../droid-cli-probe.js";
@@ -600,6 +603,23 @@ export const registerAuthRoutes: ApiRouteRegistrar = (ctx) => {
      const acpBridgeAvailable =
        typeof process.env.FUSION_CLAUDE_ACP_BRIDGE === "string" &&
        process.env.FUSION_CLAUDE_ACP_BRIDGE.length > 0;
+      // R17: the driver writes this signal when a turn comes back "Not logged in"
+      // (the bridged `claude` can't authenticate). Surface it so the UI can offer
+      // fall-back-to-`-p` or fix-auth. Path matches ACP_BRIDGE_AUTH_SIGNAL_PATH.
+      let acpAuthFailed = false;
+      let acpAuthReason: string | undefined;
+      try {
+        const signalPath = join(tmpdir(), "fusion-acp-bridge-auth.json");
+        if (existsSync(signalPath)) {
+          const sig = JSON.parse(readFileSync(signalPath, "utf8")) as { authFailed?: boolean; reason?: string };
+          if (sig?.authFailed) {
+            acpAuthFailed = true;
+            acpAuthReason = typeof sig.reason === "string" ? sig.reason : undefined;
+          }
+        }
+      } catch {
+        // best-effort; absence of the signal means no known auth failure
+      }

      res.json({
        binary,
@@ -609,6 +629,8 @@ export const registerAuthRoutes: ApiRouteRegistrar = (ctx) => {
          enabled: acpEnabled,
          bridgeAvailable: acpBridgeAvailable,
          active: enabled && acpEnabled && acpBridgeAvailable,
+          authFailed: acpAuthFailed,
+          authReason: acpAuthReason,
        },
        // Convenience field: the provider card considers everything "ready"
        // when the binary is available, the user has enabled the toggle,
--- a/packages/engine/src/tests/claude-acp-enable.test.ts
+++ b/packages/engine/src/tests/claude-acp-enable.test.ts
@@ -16,22 +16,22 @@ describe("claudeAcpExperimentalEnabled — default ON", () => {
 });

 describe("applyClaudeAcpEnable — translates the flag to FUSION_CLAUDE_ACP", () => {
-  it("sets FUSION_CLAUDE_ACP=1 when enabled (default ON) and env unset", () => {
+  it("sets FUSION_CLAUDE_ACP=1 when enabled (default ON)", () => {
    const env: NodeJS.ProcessEnv = {};
    expect(applyClaudeAcpEnable({}, env)).toBe(true);
    expect(env.FUSION_CLAUDE_ACP).toBe("1");
  });
-  it("does NOT set the env when the flag is explicitly false", () => {
-    const env: NodeJS.ProcessEnv = {};
+  it("sets FUSION_CLAUDE_ACP=0 when the flag is explicitly false (recomputed each call)", () => {
+    const env: NodeJS.ProcessEnv = { FUSION_CLAUDE_ACP: "1" }; // stale prior value
    expect(applyClaudeAcpEnable({ experimentalFeatures: { claudeCliAcp: false } }, env)).toBe(false);
-    expect(env.FUSION_CLAUDE_ACP).toBeUndefined();
+    expect(env.FUSION_CLAUDE_ACP).toBe("0"); // flip takes effect — no latch on our own write
  });
-  it("honors an explicit env override (operator/test wins over the flag)", () => {
-    const off: NodeJS.ProcessEnv = { FUSION_CLAUDE_ACP: "0" };
-    expect(applyClaudeAcpEnable({}, off)).toBe(false); // flag default-on, but env says off
+  it("honors the operator force-override FUSION_CLAUDE_ACP_FORCE over the flag", () => {
+    const off: NodeJS.ProcessEnv = { FUSION_CLAUDE_ACP_FORCE: "0" };
+    expect(applyClaudeAcpEnable({}, off)).toBe(false); // flag default-on, but forced off
    expect(off.FUSION_CLAUDE_ACP).toBe("0");

-    const on: NodeJS.ProcessEnv = { FUSION_CLAUDE_ACP: "1" };
+    const on: NodeJS.ProcessEnv = { FUSION_CLAUDE_ACP_FORCE: "1" };
    expect(applyClaudeAcpEnable({ experimentalFeatures: { claudeCliAcp: false } }, on)).toBe(true);
    expect(on.FUSION_CLAUDE_ACP).toBe("1");
  });
--- a/packages/engine/src/claude-acp-enable.ts
+++ b/packages/engine/src/claude-acp-enable.ts
@@ -8,8 +8,10 @@
 *      `FUSION_CLAUDE_ACP_BRIDGE` on load — KTD10; absent → fail-closed to `-p`).
 *
 * The user-facing switch is `experimentalFeatures.claudeCliAcp`: ON unless the
- * user explicitly sets it to `false`. An explicit `FUSION_CLAUDE_ACP` env value
- * always wins (operator / test override) — see {@link applyClaudeAcpEnable}.
+ * user explicitly sets it to `false`. An operator force-override
+ * (`FUSION_CLAUDE_ACP_FORCE=0|1`) always wins. The decision is recomputed every
+ * call (each `createFnAgent`) so flipping the flag — e.g. the UI "use `claude -p`"
+ * fallback after an auth failure — takes effect on the next turn, no restart.
 */

 /** True unless `experimentalFeatures.claudeCliAcp === false` (default ON). */
@@ -29,8 +31,11 @@ export function applyClaudeAcpEnable(
  globalSettings: Record<string, unknown> | undefined,
  env: NodeJS.ProcessEnv = process.env,
 ): boolean {
-  if (typeof env.FUSION_CLAUDE_ACP === "string") return env.FUSION_CLAUDE_ACP === "1";
-  const enabled = claudeAcpExperimentalEnabled(globalSettings);
-  if (enabled) env.FUSION_CLAUDE_ACP = "1";
+  // Operator force-override (set in the launch environment), re-read every call
+  // so our own writes to FUSION_CLAUDE_ACP can't latch the decision.
+  const force = env.FUSION_CLAUDE_ACP_FORCE;
+  const enabled =
+    force === "1" ? true : force === "0" ? false : claudeAcpExperimentalEnabled(globalSettings);
+  env.FUSION_CLAUDE_ACP = enabled ? "1" : "0";
  return enabled;
 }
--- a/packages/pi-claude-cli/src/tests/acp-driver.test.ts
+++ b/packages/pi-claude-cli/src/tests/acp-driver.test.ts
@@ -6,7 +6,9 @@ import { PassThrough } from "node:stream";
 let scriptedUpdates: Array<Record<string, unknown>> = [];

 // Driver validates the bridge path with existsSync — make the fake path "exist".
-vi.mock("node:fs", () => ({ existsSync: () => true }));
+// writeFileSync/unlinkSync back the R17 auth-failure signal (spied).
+const fsSpies = vi.hoisted(() => ({ writeFileSync: vi.fn(), unlinkSync: vi.fn() }));
+vi.mock("node:fs", () => ({ existsSync: () => true, writeFileSync: fsSpies.writeFileSync, unlinkSync: fsSpies.unlinkSync }));

 vi.mock("node:child_process", () => ({
  spawn: vi.fn(() => {
@@ -119,6 +121,19 @@ describe("streamViaAcp — ACP→pi translation (U11)", () => {
    expect(done!.reason).toBe("toolUse");
  });

+  it("records the R17 auth-failure signal when the bridge turn is only 'Not logged in'", async () => {
+    fsSpies.writeFileSync.mockClear();
+    scriptedUpdates = [
+      { sessionUpdate: "agent_message_chunk", content: { type: "text", text: "Not logged in · Please run /login" } },
+    ];
+    const stream = streamViaAcp(MODEL, CTX, OPTS) as unknown as { _events: Array<Record<string, unknown>> };
+    await flush();
+    // signal file written with authFailed:true
+    const wrote = fsSpies.writeFileSync.mock.calls.find((c) => String(c[1]).includes("authFailed"));
+    expect(wrote).toBeTruthy();
+    expect(String(wrote![1])).toContain("\"authFailed\":true");
+  });
+
  it("ends with done even when the turn produces no content", async () => {
    scriptedUpdates = [];
    const stream = streamViaAcp(MODEL, CTX, OPTS) as unknown as { _events: Array<Record<string, unknown>> };
--- a/packages/pi-claude-cli/src/acp-driver.ts
+++ b/packages/pi-claude-cli/src/acp-driver.ts
@@ -35,8 +35,9 @@

 import { spawn, type ChildProcess } from "node:child_process";
 import { Readable, Writable } from "node:stream";
-import { isAbsolute } from "node:path";
-import { existsSync } from "node:fs";
+import { isAbsolute, join } from "node:path";
+import { existsSync, writeFileSync, unlinkSync } from "node:fs";
+import { tmpdir } from "node:os";
 import {
  ClientSideConnection,
  ndJsonStream,
@@ -77,6 +78,35 @@ const MAX_CHUNK_CHARS = 64 * 1024;
 const MAX_TURN_CHARS = 5_000_000;
 const MAX_ID_CHARS = 256;

+/**
+ * Cross-process signal for the dashboard: when the bridged `claude` can't
+ * authenticate (R17 — e.g. a detached daemon with no keychain), the turn comes
+ * back as "Not logged in · Please run /login" instead of a real answer. We
+ * record that here so `GET /providers/claude-cli/status` can surface it and the
+ * UI can prompt the user to fall back to `-p` or fix auth. A real response
+ * clears it. Best-effort; the path is recomputed identically dashboard-side.
+ */
+export const ACP_BRIDGE_AUTH_SIGNAL_PATH = join(tmpdir(), "fusion-acp-bridge-auth.json");
+const NOT_LOGGED_IN_RE = /not logged in|please run \/login/i;
+let lastAuthFailed: boolean | undefined;
+
+function recordBridgeAuthState(failed: boolean, reason?: string): void {
+  if (lastAuthFailed === failed) return; // only write on transition
+  lastAuthFailed = failed;
+  try {
+    if (failed) {
+      writeFileSync(
+        ACP_BRIDGE_AUTH_SIGNAL_PATH,
+        JSON.stringify({ authFailed: true, at: new Date().toISOString(), reason: reason ?? "Claude in the ACP bridge is not logged in" }),
+      );
+    } else {
+      unlinkSync(ACP_BRIDGE_AUTH_SIGNAL_PATH);
+    }
+  } catch {
+    /* best-effort signal — never let it affect the turn */
+  }
+}
+
 /**
 * Bridge subprocess env allow-list. The bridged `claude` needs HOME (for
 * `~/.claude` auth/keychain, R17) and PATH; terminal vars improve rendering.
@@ -173,6 +203,15 @@ export function streamViaAcp(
      // Downgrade a tool_use turn that surfaced zero pi tool calls → stop, so pi
      // doesn't try to dispatch non-existent tools (mirrors provider.ts:366-375).
      const toolCount = (bridge.getOutput().content ?? []).filter((c) => (c as { type?: string }).type === "toolCall").length;
+      // R17: a turn that is ONLY "Not logged in" (no tools, no real text) means
+      // the bridged `claude` can't authenticate — signal it for the UI. A real
+      // response (tools or non-trivial text) clears the signal.
+      const fullText = (bridge.getOutput().content ?? [])
+        .filter((c) => (c as { type?: string }).type === "text")
+        .map((c) => (c as { text?: string }).text ?? "")
+        .join("");
+      if (toolCount === 0 && NOT_LOGGED_IN_RE.test(fullText)) recordBridgeAuthState(true);
+      else if (toolCount > 0 || fullText.trim().length > 0) recordBridgeAuthState(false);
      const effective: "stop" | "tool_use" = reason === "tool_use" && toolCount > 0 ? "tool_use" : "stop";
      bridge.handleEvent({ type: "message_delta", delta: { stop_reason: effective === "tool_use" ? "tool_use" : "end_turn" } } as ClaudeApiEvent);
      stream.push({ type: "done", reason: effective === "tool_use" ? "toolUse" : "stop", message: bridge.getOutput() });