diff --git a/src/lib/session-title.test.ts b/src/lib/session-title.test.ts index 57ef5192..c28fcf87 100644 --- a/src/lib/session-title.test.ts +++ b/src/lib/session-title.test.ts @@ -88,3 +88,32 @@ describe("mayReplaceTitle", () => { expect(mayReplaceTitle("Matty comp", "Fix the table layout")).toBe(false); }); }); + +describe("sanitizeGeneratedTitle — auth-expiry guard", () => { + it("rejects the exact string that branded a session in v0.6.0", () => { + // Passes every other filter: 1 line, 72 chars, 11 words - which is how it + // became the permanent title "Failed to authenticate: OAuth session ...". + expect( + sanitizeGeneratedTitle( + "Failed to authenticate: OAuth session expired and could not be refreshed", + ), + ).toBeNull(); + }); + + it("rejects other vendor auth-failure phrasings", () => { + expect(sanitizeGeneratedTitle("Session expired")).toBeNull(); + expect(sanitizeGeneratedTitle("invalid_grant")).toBeNull(); + expect(sanitizeGeneratedTitle("not_authenticated")).toBeNull(); + }); + + it("keeps titles that merely mention auth", () => { + // The strict matcher exists for exactly these: real work about auth must + // still get a real name. + expect(sanitizeGeneratedTitle("Fixing the unauthorized API error")).toBe( + "Fixing the unauthorized API error", + ); + expect(sanitizeGeneratedTitle("Add OAuth login to the desktop app")).toBe( + "Add OAuth login to the desktop app", + ); + }); +}); diff --git a/src/lib/session-title.ts b/src/lib/session-title.ts index bee9c361..45a3de9c 100644 --- a/src/lib/session-title.ts +++ b/src/lib/session-title.ts @@ -8,6 +8,8 @@ * generated title takes over once the first exchange is done. */ +import { isAuthExpiryText } from "$lib/utils/auth-errors"; + /** Tabs truncate around here; longer titles only add ellipsis. */ export const MAX_TITLE_LENGTH = 60; @@ -51,6 +53,14 @@ export function cleanPromptTitle(prompt: string): string { export function sanitizeGeneratedTitle(raw: string): string | null { let text = String(raw ?? "").trim(); if (!text) return null; + // The title comes from a throwaway CLI run. When that run fails on expired + // auth it returns the failure text, which passes every check below - one + // line, 72 chars, 11 words - so the session gets permanently named + // "Failed to authenticate: OAuth session expired and could not". A transient + // auth blip must not brand a session forever; drop it and keep the + // prompt-derived title. Strict matcher: a title that merely mentions auth + // (say "Fixing the unauthorized API error") is a legitimate name. + if (isAuthExpiryText(text)) return null; // Unwrap a fenced block before counting lines — the fence is the model's formatting, not // part of the answer, and its newlines would otherwise look like an essay. const fenced = text.match(/^```[a-z]*\s*\n?([\s\S]*?)\n?\s*```$/i); diff --git a/src/lib/utils/auth-errors.ts b/src/lib/utils/auth-errors.ts index 74c12bc8..6c571c97 100644 --- a/src/lib/utils/auth-errors.ts +++ b/src/lib/utils/auth-errors.ts @@ -27,3 +27,31 @@ export function isAuthExpiryError(text: string | null | undefined): boolean { const t = String(text).toLowerCase(); return AUTH_EXPIRY_MARKERS.some((m) => t.includes(m)); } + +/** Phrasings the vendor CLIs actually emit when their own OAuth failed. Unlike + * AUTH_EXPIRY_MARKERS this must not match text that merely *discusses* auth: + * bare "authenticate" / "unauthorized" are deliberately absent, because a + * legitimate chat about a 401 would otherwise be mistaken for a failure. + * + * Still not airtight, and it does not need to be: "session expired" would also + * reject a title like "Session expired handling in Redis". The cost of a false + * positive here is bounded - the session keeps its prompt-derived name - so the + * list is tuned to never MISS a real failure, and to miss as few good titles as + * it reasonably can. */ +const AUTH_EXPIRY_STRICT_MARKERS = [ + "oauth session expired", + "could not be refreshed", + "failed to authenticate", + "session expired", + "not_authenticated", + "invalid_grant", +]; + +/** Strict variant of {@link isAuthExpiryError}, for places where the text may be + * ordinary content rather than a raw error — e.g. a model-generated session + * title, where a false positive silently discards a good title. */ +export function isAuthExpiryText(text: string | null | undefined): boolean { + if (!text) return false; + const t = String(text).toLowerCase(); + return AUTH_EXPIRY_STRICT_MARKERS.some((m) => t.includes(m)); +} diff --git a/src/routes/+page.svelte b/src/routes/+page.svelte index bea33db3..7f0d90b7 100644 --- a/src/routes/+page.svelte +++ b/src/routes/+page.svelte @@ -365,6 +365,37 @@ // and couldn't refresh, we show a friendly "Session expired → Reconnect" state // instead of the raw error, and stash the failed prompt so the user can retry it. let authExpired = $state<{ prompt: string } | null>(null); + /** Reconnect was clicked and the login flow is out in the browser; the next + * probe decides whether to confirm success or re-raise the expired card. + * Mirrored into sessionStorage (see RECONNECT_INTENT_KEY) because the login + * flow can swap the view and remount this component, which would drop it. */ + let authReconnecting = $state(false); + /** Survives the remount that the login stage can cause. Timestamped so a flag + * left behind by an abandoned login cannot confirm a later, unrelated launch. */ + const RECONNECT_INTENT_KEY = "brains.authReconnectIntent"; + const RECONNECT_INTENT_MAX_AGE_MS = 10 * 60_000; + /** Brief "you're signed back in" confirmation — otherwise a successful login + * is indistinguishable from one that silently failed. */ + let authReconnected = $state(false); + let authReconnectedTimer: ReturnType | null = null; + /** Tracked so the reconnect poll series can be cancelled on destroy, and so a + * second Reconnect click replaces the series instead of stacking one. */ + let authPollTimers: ReturnType[] = []; + /** Consecutive signed-out probe verdicts. The backends report an unreachable or + * slow CLI as a confident "signed out", so one verdict is not enough to raise a + * card that blocks sending. Reset by any signed-in answer. */ + let signedOutStreak = 0; + /** Set when the user overrides a refused send. The probe's verdict is a guess; + * this is how someone whose CLI is merely slow gets out of the way of it. */ + let authBlockOverridden = $state(false); + /** How often to re-check the vendor CLI's sign-in while the window is visible. + * The probe spawns a process (~700ms), so this stays well above the polling + * used for cheap in-process state. */ + const AUTH_PROBE_MS = 120_000; + /** Delay before confirming a first signed-out verdict. Long enough that a + * single slow spawn still cannot raise the card on its own (the probe itself + * is ~700ms), short enough that the card is up before anyone finishes typing. */ + const AUTH_CONFIRM_MS = 3_000; // model selection (per new chat; a run's model is fixed once it starts). // VENDOR-AWARE: lists/defaults/persistence live in $lib/model-options (tested // there) — each vendor has its own list + its own persisted selection so a @@ -7560,6 +7591,22 @@ const dispatchContext = [...allContext]; const dispatchCurrentItem = currentItem; const dispatchExtraContext = [...extraContext]; + // Known-signed-out (from the background probe): refuse to spend the message on a + // turn that can only fail. Without this the prompt is consumed, the vendor answers + // with its own auth failure, and that failure renders as the assistant's reply - + // which is how a long prompt became a dead-end error bubble. This must run BEFORE + // any turn state is mutated: past this point the message is appended to the + // transcript, so bailing later would strand a user bubble with no reply. + if (authExpired && !hidden && !authBlockOverridden) { + authExpired = { prompt: t }; // stash it so the card offers it back + if (t && !input.trim()) input = t; // the composer was already cleared - restore it + // The verdict above is cached from the last probe, so a user who signed in + // by another route (a terminal `claude auth login`) would stay blocked until + // a focus event or the 120s tick. Re-probe now so the block self-heals in a + // tick instead of stranding someone who is actually signed in. + void probeAuth(); + return; + } turnStopping = false; hideTurn = hidden; // A new user turn clears completed subagent rows (their result was there to @@ -7763,13 +7810,211 @@ } } + /** Ask the vendor CLI whether it is still signed in, and surface the recovery + * card the moment it is not — before the user writes anything. + * + * Why proactive rather than a pre-send check: `claude auth status` spawns a + * process and costs ~700ms, so gating every send on it would tax every + * message to catch a rare state. Probing on focus / visibility / an idle + * interval keeps sends instant, and turns "typed a long prompt, lost it to a + * dead-end error" into "saw you were signed out before typing". + * + * NOT fail-soft by exception, whatever an earlier version of this comment + * claimed. Neither backend rejects: `check_codex_auth` returns + * `Ok(logged_in: false)` for codex-not-installed, an exec error, AND a 12s + * timeout; `check_auth_status` has no fallible call at all and maps a Claude + * CLI timeout or non-zero exit to `has_oauth: false`. So an infrastructure + * failure arrives here indistinguishable from a real sign-out, and the `catch` + * below only ever sees an IPC-layer fault. Three things keep a wrong verdict + * from becoming a lockout: + * + * 1. Where the backend hands us evidence of "couldn't tell" rather than + * "signed out" (codex `installed: false`, or a `status_text` naming a + * timeout / exec error), treat it as INDETERMINATE and change nothing. + * 2. Otherwise require two consecutive signed-out verdicts before raising the + * card, so a single slow spawn cannot block anyone. + * 3. The card's refusal is overridable — see "Send anyway", which lasts only + * until the next signed-in verdict re-arms the guard — because the + * Anthropic backend exposes no indeterminate signal, so a machine where + * the CLI is *persistently* slow (an AV scanning `node.exe` on every + * spawn) would fail (1) and (2) alike and otherwise be stuck for good. + * + * VENDOR-AWARE, and it has to be: check_auth_status is Anthropic-specific + * (it shells out to the `claude` binary), so asking it about a Codex user + * answers "signed out" no matter how healthy their ChatGPT login is. Because + * a raised card also blocks sending, that wrong answer locked Codex users out + * of the app entirely - signing in via `loginCodex()` succeeded and the next + * probe still asked Claude. Branch the same way SetupWizard does. */ + async function isVendorSignedOut(): Promise { + if (agentFor(model) === "codex") { + const codex = await api.checkCodexAuth(); + // The evidence the Anthropic path lacks: a false `logged_in` that only + // means the check never got an answer. + if (!codex.installed) return null; + const why = (codex.status_text ?? "").toLowerCase(); + if (why.includes("timed out") || why.includes("exec error")) return null; + return !codex.logged_in; + } + const status = await api.checkAuthStatus(); + return !status.has_oauth && !status.has_api_key; + } + + async function probeAuth() { + if (!isApp || sending) return; + try { + const verdict = await isVendorSignedOut(); + if (verdict === null) return; // couldn't tell - leave the UI exactly as it is + const signedOut = verdict; + if (signedOut) { + // Two strikes. One slow CLI spawn must not raise a card that blocks + // sending; a genuinely signed-out user just sees it a probe later, and + // probes already run on mount/focus/visibility plus the idle tick. + signedOutStreak += 1; + if (signedOutStreak < 2 && !authExpired) { + // Confirm this promptly instead of waiting for the next probe. At + // startup only one probe runs, and the next comes from a focus / + // visibility event or the AUTH_PROBE_MS tick - so on a genuinely + // signed-out machine the two-strike rule would leave no card for up to + // two minutes, and anyone who launched the app and typed straight away + // would spend a message into exactly the dead end this card exists to + // prevent. A short confirm keeps both properties: the card is up before + // the user can type, and it still takes two verdicts to raise it, so a + // single slow spawn never blocks anyone. + authPollTimers.push(setTimeout(() => void probeAuth(), AUTH_CONFIRM_MS)); + return; + } + // Keep whatever prompt a previous failure stashed; this probe adds no + // prompt of its own (nothing was consumed to discover this). + if (!authExpired) authExpired = { prompt: "" }; + authReconnecting = false; + } else { + signedOutStreak = 0; + // Re-arm the send guard. The override exists to get past ONE bad verdict, + // so it has to expire with that verdict: left latched, a single click - + // most likely from someone whose CLI was merely slow - would disable the + // guard for the rest of the session, and the next genuine expiry would + // spend a message and land as a dead-end error bubble, since that failure + // arrives as assistant content rather than through handleTurnError. + authBlockOverridden = false; + // Signed in. Clear the card unconditionally: gating this on "no stashed + // prompt" left a stale "session expired" claim on screen after a + // successful reconnect. The prompt is not lost - it was handed back to + // the composer when the send was refused. + if (authExpired) authExpired = null; + // Confirm it, but only to someone who was actually waiting: an + // unprompted "Reconnected" note on every launch would be noise. + if (authReconnecting || hasPendingReconnectIntent()) { + authReconnecting = false; + clearReconnectIntent(); + showAuthReconnected(); + } + } + } catch { + /* probe failure is not proof of signed-out; leave the UI alone */ + } + } + /** Reconnect after an auth-expiry failure: re-run the current vendor's existing * login flow. On success the connect/login stages re-enter the app; the user's * failed prompt was already restored to the composer so they can resend. */ + /** Was a Reconnect click still pending when this component (re)mounted? + * Read-only by design — callers pair it with clearReconnectIntent() once they + * have acted on it. Stale intents (older than the max age) are dropped here. */ + function hasPendingReconnectIntent(): boolean { + try { + const raw = sessionStorage.getItem(RECONNECT_INTENT_KEY); + if (!raw) return false; + const at = Number(raw); + if (!Number.isFinite(at) || Date.now() - at > RECONNECT_INTENT_MAX_AGE_MS) { + clearReconnectIntent(); + return false; + } + return true; + } catch { + return false; // storage unavailable — fall back to in-memory state only + } + } + + function markReconnectIntent() { + try { + sessionStorage.setItem(RECONNECT_INTENT_KEY, String(Date.now())); + } catch { + /* in-memory authReconnecting still covers the no-remount case */ + } + } + + function clearReconnectIntent() { + try { + sessionStorage.removeItem(RECONNECT_INTENT_KEY); + } catch { + /* ignore */ + } + } + + /** Show the "you're back" confirmation briefly, then let it fade. */ + function showAuthReconnected() { + authReconnected = true; + if (authReconnectedTimer) clearTimeout(authReconnectedTimer); + authReconnectedTimer = setTimeout(() => { + authReconnected = false; + authReconnectedTimer = null; + }, 8000); + } + + /** Stop refusing sends, for someone who believes the probe is wrong about them. + * The verdict is a guess built on a backend that cannot distinguish "signed + * out" from "the CLI did not answer in 12s", so there has to be a way past it + * that is not "reinstall the app". The card stays up - the claim may well be + * right - but it no longer blocks. Two things end the override, because it must + * not outlive the verdict it was overriding: a real turn failure re-blocks + * through handleTurnError (evidence rather than inference), and the next + * signed-in probe verdict re-arms the guard. */ + function sendAnywayDespiteAuth() { + authBlockOverridden = true; + void probeAuth(); // and re-check, in case the truth has moved on + } + function reconnectAuth() { - authExpired = null; chatError = null; + authBlockOverridden = false; // a fresh reconnect re-arms the guard + // Do NOT tear the card down here. Clearing it on click made a completed + // login look exactly like one that silently failed: the card vanished and + // nothing ever said the app was signed in again. Hold a "reconnecting" + // state and let the probe report the outcome - loginVendor() is + // fire-and-forget, but returning from the browser fires window focus, which + // runs the probe within a tick. + authReconnecting = true; + markReconnectIntent(); // survives the remount the login stage can cause loginVendor(); + pollAuthWhileReconnecting(); + } + + /** Watch for the outcome of an in-flight reconnect. + * + * The login flow finishes in-process and fires no window focus event, so + * waiting for the idle tick would leave "Waiting for sign-in..." on screen for + * up to two minutes after the user was already signed in. Poll instead, + * backing off, stopping as soon as the state resolves. + * + * Must be callable on mount, not just from the click: signing in can remount + * this component, which kills the timers scheduled by the click. That left the + * persisted intent with nothing watching it, so the confirmation only appeared + * once some unrelated focus/visibility event happened to run a probe - which is + * exactly the "only after switching tabs" behaviour observed in testing. */ + function pollAuthWhileReconnecting() { + clearAuthPollTimers(); // a second Reconnect click must not stack a second series + for (const delay of [1500, 4000, 8000, 15000, 30000, 45000]) { + authPollTimers.push( + setTimeout(() => { + if (authReconnecting) void probeAuth(); + }, delay), + ); + } + } + + function clearAuthPollTimers() { + for (const timer of authPollTimers) clearTimeout(timer); + authPollTimers = []; } function relTime(ts?: string): string { @@ -9584,10 +9829,26 @@ // Artifact creation can happen in another session, browser, or device. The // server does not currently emit a workspace-wide catalog event, so refresh // on return to the app and poll lightly while this window remains visible. - const refreshCatalogOnFocus = () => void refreshResourceCatalog(true); + const refreshCatalogOnFocus = () => { + void refreshResourceCatalog(true); + void probeAuth(); // catch a session that died while the app was in the background + }; const refreshCatalogOnVisibility = () => { - if (document.visibilityState === "visible") void refreshResourceCatalog(true); + if (document.visibilityState === "visible") { + void refreshResourceCatalog(true); + void probeAuth(); + } }; + const authProbeTimer = setInterval(() => { + if (document.visibilityState === "visible") void probeAuth(); + }, AUTH_PROBE_MS); + // A reconnect that was in flight when this component remounted (signing in can + // swap the view) has to be picked back up here, or nothing watches it finish. + if (hasPendingReconnectIntent()) { + authReconnecting = true; + pollAuthWhileReconnecting(); + } + void probeAuth(); // and at startup, so a dead session is visible before typing const resourceCatalogTimer = setInterval(() => { if (document.visibilityState === "visible") void refreshResourceCatalog(); }, RESOURCE_CATALOG_REFRESH_MS); @@ -9603,6 +9864,9 @@ window.removeEventListener("focus", refreshCatalogOnFocus); document.removeEventListener("visibilitychange", refreshCatalogOnVisibility); clearInterval(resourceCatalogTimer); + clearInterval(authProbeTimer); + if (authReconnectedTimer) clearTimeout(authReconnectedTimer); + clearAuthPollTimers(); dashResizeObserver?.disconnect(); dashResizeObserver = undefined; }); @@ -11740,10 +12004,30 @@ >Your message was kept — reconnect and send again.{/if} + {#if !authBlockOverridden} + + {/if} {authReconnecting ? "Waiting for sign-in…" : "Reconnect"} + + {:else if authReconnected} +
+ Reconnected to {currentProvider === "openai" ? "ChatGPT" : "Claude"}. + Your message is back in the composer — send it again.
{:else if chatError}