From 30619616c499112c2885b5ad2cae33a9e2775932 Mon Sep 17 00:00:00 2001
From: stuffbucket <231133237+stuffbucket@users.noreply.github.com>
Date: Thu, 11 Jun 2026 13:04:49 -0700
Subject: [PATCH] =?UTF-8?q?fix(auth):=20gh-reuse=20=E2=80=94=20validate=20?=
=?UTF-8?q?before=20reboot=20+=20refresh=20discovery=20on=20focus?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Completes the gh-reuse feature to a clean state. Two gaps:
1. Errors were handled safely but imprecisely: a stale/no-subscription gh
account got written + rebooted into, then surfaced a generic 'came back
unauthenticated' ~20s later. Now POST /gh/use PRE-FLIGHTS the token against
Copilot (/copilot_internal/user) BEFORE writing/rebooting and returns a
specific 422 — 'token expired or revoked' (401) vs 'no Copilot subscription'
(403/404) vs a network message — which the UI shows immediately, no reboot.
The shell now extracts {error:{message}} so the specific reason renders.
Also closes the getModels timeout gap (it was missing from the #110 set;
cacheModels is on the boot critical path).
2. gh discovery was only refreshed on entering the Account section — adding an
account in gh while settings was already open didn't show up. Now refreshes
on window focus / visibilitychange (the natural 'returned from the terminal'
moment) + a manual Refresh button. No polling (gh auth status hits the
keyring).
+6 pre-flight unit tests (status→message mapping, DI'd usage fn — no
mock.module). Full suite green (824); valid-account import still 200 (no
happy-path regression); shell tsc clean.
---
shell/index.html | 13 ++++++++-
shell/src/main.ts | 37 ++++++++++++++++++++++++-
shell/src/styles.css | 8 ++++++
src/routes/settings/gh.ts | 40 ++++++++++++++++++++++++++-
src/services/copilot/get-models.ts | 4 +++
tests/gh-preflight.test.ts | 44 ++++++++++++++++++++++++++++++
6 files changed, 143 insertions(+), 3 deletions(-)
create mode 100644 tests/gh-preflight.test.ts
diff --git a/shell/index.html b/shell/index.html
index 24b6108a..5e42f087 100644
--- a/shell/index.html
+++ b/shell/index.html
@@ -119,7 +119,18 @@
-
Reuse a GitHub CLI account
+
Sign in with an account you’re already logged into
with gh on this Mac — no code
diff --git a/shell/src/main.ts b/shell/src/main.ts
index 69a42cf6..263f4ef9 100644
--- a/shell/src/main.ts
+++ b/shell/src/main.ts
@@ -638,6 +638,21 @@ function hideGhError(): void {
el.textContent = "";
}
+/** apiCall surfaces the raw response body in `error`. Our settings endpoints
+ * reply `{ error: { message } }`; pull that out so the UI shows the specific
+ * reason (e.g. the /gh/use pre-flight message) rather than raw JSON. */
+function apiErrorMessage(raw: string): string {
+ try {
+ const parsed = JSON.parse(raw) as { error?: { message?: string } };
+ if (typeof parsed.error?.message === "string" && parsed.error.message) {
+ return parsed.error.message;
+ }
+ } catch {
+ // not JSON — use the raw text
+ }
+ return raw;
+}
+
/**
* Populate the "reuse a GitHub CLI account" list from GET /gh/status. The
* wrapper stays hidden unless gh is installed AND has ≥1 account — a user
@@ -732,8 +747,11 @@ async function useGhAccount(button: HTMLElement): Promise {
for (const b of signInButtons) b.disabled = false;
buttonEl.textContent = originalLabel;
row?.removeAttribute("aria-busy");
+ // The pre-flight (POST /gh/use) returns a specific reason — stale token,
+ // no Copilot subscription, etc. — caught BEFORE any reboot. Show it.
showGhError(
- "Couldn't sign in with that account. Try the code-based sign-in above.",
+ apiErrorMessage(result.error) ||
+ "Couldn't sign in with that account. Try the code-based sign-in above.",
);
buttonEl.focus();
return;
@@ -995,6 +1013,9 @@ function wireAccount(): void {
case "gh-use":
void useGhAccount(button);
break;
+ case "gh-refresh":
+ void loadGhAccounts();
+ break;
default:
break;
}
@@ -1030,3 +1051,17 @@ window.addEventListener("hashchange", () => {
stopAuthPolling();
}
});
+
+// Refresh on returning to the window. The common flow is: open the Account
+// section here, switch to a terminal, `gh auth login` (or out) of an account,
+// then switch back — re-loading auth status on focus re-runs gh discovery so
+// the new account appears without leaving + re-entering the section. We do NOT
+// poll (gh auth status touches the OS keyring); focus/visibility is the right
+// "moment of attention" to refresh.
+function refreshAccountOnAttention(): void {
+ if (readHashSection() === "account") void loadAuthStatus();
+}
+window.addEventListener("focus", refreshAccountOnAttention);
+document.addEventListener("visibilitychange", () => {
+ if (document.visibilityState === "visible") refreshAccountOnAttention();
+});
diff --git a/shell/src/styles.css b/shell/src/styles.css
index 633fef4b..c8cd5511 100644
--- a/shell/src/styles.css
+++ b/shell/src/styles.css
@@ -252,6 +252,14 @@ a:hover {
display: none;
}
+/* Title + a quiet "Refresh" affordance on one row. */
+.gh-reuse__header {
+ display: flex;
+ align-items: baseline;
+ justify-content: space-between;
+ gap: var(--space-3);
+}
+
/* Centered "or" rule between the device-code button and the gh list. */
.gh-reuse__divider {
display: flex;
diff --git a/src/routes/settings/gh.ts b/src/routes/settings/gh.ts
index 5401ed0a..40eb8fdf 100644
--- a/src/routes/settings/gh.ts
+++ b/src/routes/settings/gh.ts
@@ -8,9 +8,39 @@
import { Hono } from "hono"
-import { forwardError } from "~/lib/error"
+import { forwardError, HTTPError } from "~/lib/error"
import { makeRecord, writeDefaultRecord } from "~/lib/github-token-store"
import { detectGhCli, getGhAccountToken } from "~/services/gh-cli"
+import { getCopilotUsage } from "~/services/github/get-copilot-usage"
+
+/**
+ * Verify a gh account's token actually works for Copilot BEFORE we adopt it.
+ * Mirrors what boot does (the live identity check that was failing silently):
+ * a stale/revoked token → GitHub rejects it; an account with no Copilot →
+ * /copilot_internal/user 403/404. Returns a specific, user-facing message so
+ * the UI can say WHY synchronously, instead of writing the token, rebooting,
+ * and surfacing a generic "came back unauthenticated" 20s later. Returns null
+ * when the token is usable.
+ */
+export async function preflightCopilotError(
+ token: string,
+ login: string,
+ usage: (token: string) => Promise = getCopilotUsage,
+): Promise {
+ try {
+ await usage(token)
+ return null
+ } catch (error) {
+ const status = error instanceof HTTPError ? error.response.status : 0
+ if (status === 401) {
+ return `GitHub rejected ${login}'s token — it may be expired or revoked. Run \`gh auth login\` and try again, or sign in with a code.`
+ }
+ if (status === 403 || status === 404) {
+ return `${login} doesn't have access to GitHub Copilot. Pick another account, or sign in with a code.`
+ }
+ return `Couldn't verify ${login} with GitHub${status ? ` (HTTP ${status})` : ""}. Check your connection and try again.`
+ }
+}
export const ghRoutes = new Hono()
@@ -70,6 +100,14 @@ ghRoutes.post("/use", async (c) => {
)
}
+ // Pre-flight: confirm the token works for Copilot BEFORE writing it +
+ // rebooting, so a stale/no-subscription account fails fast with a specific
+ // reason rather than a generic post-reboot "came back unauthenticated".
+ const preflightError = await preflightCopilotError(token, login)
+ if (preflightError) {
+ return c.json({ error: { message: preflightError } }, 422)
+ }
+
await writeDefaultRecord(makeRecord(token))
return c.json({ ok: true, login, host })
} catch (error) {
diff --git a/src/services/copilot/get-models.ts b/src/services/copilot/get-models.ts
index 29c769ed..1092ff97 100644
--- a/src/services/copilot/get-models.ts
+++ b/src/services/copilot/get-models.ts
@@ -2,12 +2,16 @@ import consola from "consola"
import { copilotBaseUrl, copilotModelsHeaders } from "~/lib/api-config"
import { HTTPError } from "~/lib/error"
+import { GITHUB_API_TIMEOUT_MS } from "~/lib/http-timeouts"
import { state } from "~/lib/state"
export const getModels = async () => {
consola.info(`Fetching models from ${copilotBaseUrl(state)}/models`)
const response = await fetch(`${copilotBaseUrl(state)}/models`, {
headers: copilotModelsHeaders(state),
+ // Bounded like the other auth/discovery fetches — cacheModels runs on the
+ // cold-boot critical path, so an unbounded hang here would stall boot.
+ signal: AbortSignal.timeout(GITHUB_API_TIMEOUT_MS),
})
if (!response.ok) {
diff --git a/tests/gh-preflight.test.ts b/tests/gh-preflight.test.ts
new file mode 100644
index 00000000..0d294e7f
--- /dev/null
+++ b/tests/gh-preflight.test.ts
@@ -0,0 +1,44 @@
+import { describe, expect, test } from "bun:test"
+
+import { HTTPError } from "~/lib/error"
+import { preflightCopilotError } from "~/routes/settings/gh"
+
+const ok = () => Promise.resolve({ copilot_plan: "enterprise" })
+const throwsHttp = (status: number) => () =>
+ Promise.reject(new HTTPError("x", new Response(null, { status })))
+
+describe("preflightCopilotError", () => {
+ test("returns null when the token works for Copilot", async () => {
+ expect(await preflightCopilotError("gho_x", "alice", ok)).toBeNull()
+ })
+
+ test("401 → expired/revoked, names the account", async () => {
+ const msg = await preflightCopilotError("gho_x", "alice", throwsHttp(401))
+ expect(msg).toContain("expired or revoked")
+ expect(msg).toContain("alice")
+ })
+
+ test("403 / 404 → no Copilot subscription", async () => {
+ for (const status of [403, 404]) {
+ const msg = await preflightCopilotError(
+ "gho_x",
+ "bob",
+ throwsHttp(status),
+ )
+ expect(msg).toContain("doesn't have access to GitHub Copilot")
+ }
+ })
+
+ test("other status → generic verify message including the status", async () => {
+ const msg = await preflightCopilotError("gho_x", "carol", throwsHttp(503))
+ expect(msg).toContain("Couldn't verify")
+ expect(msg).toContain("503")
+ })
+
+ test("non-HTTP error (network) → generic verify message", async () => {
+ const msg = await preflightCopilotError("gho_x", "carol", () =>
+ Promise.reject(new Error("network down")),
+ )
+ expect(msg).toContain("Couldn't verify")
+ })
+})