From c090c43a2d1256c599e140d243ce6047c8513133 Mon Sep 17 00:00:00 2001 From: BIackFIame <77388790+BIackFIame@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:16:29 +0000 Subject: [PATCH 1/6] fix(github): cancel device sign-in and report denied, expired and failed outcomes A started GitHub device sign-in could not be cancelled, retried poll failures until the code expired, and ended silently when GitHub answered access_denied or expired_token. A sign-in that could not start showed the raw IPC error ("Error invoking remote method 'github-auth:start': ...") at the top of the Plugins page, in English in the Russian UI. - GithubAuthService.cancelDeviceFlow() is public and exposed through the existing github-auth IPC (main-renderer only) and preload. - status() reports deviceFlowState: idle, pending, denied, expired, failed or cancelled. - access_denied, expired_token, unknown provider errors and non-transient HTTP errors stop polling with that outcome; OAuth error bodies on HTTP 400 are read as well. - Network errors, timeouts, HTTP 408/429/5xx and unreadable bodies back off and retry, but five failures in a row end the flow as failed. - slow_down keeps adding five seconds to every later poll. - The host clock ends the flow as expired even if GitHub keeps answering authorization_pending. - Settings shows a Cancel button next to the pending code and a localized status line (en, ru) for the outcome, using existing classes: denied, expired and failed use the Settings error style, cancelled is a plain status line. A start or cancel failure shows a localized line ("not configured" for a build without an OAuth client); the details go to the log. The new sign-in lines live in PluginSettingsSection (the lazily loaded Settings chunk), not in the startup string table; renderer-bundle-splitting checks they stay out of the startup bundle. --- src/main/ipc/registerIpc.ts | 4 + src/main/services/GithubAuthService.ts | 132 ++++++++--- src/preload/index.ts | 1 + .../plugins/PluginSettingsSection.tsx | 103 ++++++-- src/shared/contracts.ts | 6 + tests/github-auth-integration.test.mjs | 18 ++ tests/github-auth.test.mjs | 221 +++++++++++++++++- tests/github-signin-errors-ui.test.mjs | 72 ++++++ tests/renderer-bundle-splitting.test.mjs | 43 +++- 9 files changed, 548 insertions(+), 52 deletions(-) create mode 100644 tests/github-signin-errors-ui.test.mjs diff --git a/src/main/ipc/registerIpc.ts b/src/main/ipc/registerIpc.ts index 5be3a8ee6..0923313d3 100644 --- a/src/main/ipc/registerIpc.ts +++ b/src/main/ipc/registerIpc.ts @@ -929,6 +929,10 @@ export function registerIpc(ipcMain: IpcRegistrar, { assertMainRenderer(event, getMainWindow); return githubAuth.signOut(); }); + ipcMain.handle(IPC.githubAuthCancel, (event) => { + assertMainRenderer(event, getMainWindow); + githubAuth.cancelDeviceFlow(); + }); ipcMain.handle(IPC.githubAuthOpenUrl, (event, value: unknown) => { assertMainRenderer(event, getMainWindow); if (typeof value !== "string") throw new Error("URL is required."); diff --git a/src/main/services/GithubAuthService.ts b/src/main/services/GithubAuthService.ts index defbdb6b7..3d416d1b2 100644 --- a/src/main/services/GithubAuthService.ts +++ b/src/main/services/GithubAuthService.ts @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"; import * as electron from "electron"; import { readFile, writeFile, mkdir, rename, rm } from "node:fs/promises"; import { join, dirname } from "node:path"; +import type { GithubDeviceFlowState } from "../../shared/contracts"; /** GitHub OAuth Device Flow for the plugin showcase. */ @@ -10,6 +11,7 @@ export interface GithubAuthStatus { authorized: boolean; login: string | null; tokenExpiresAt: number | null; + deviceFlowState: GithubDeviceFlowState; } interface StoredTokens { @@ -60,6 +62,7 @@ export class GithubAuthService { private readonly pollTimeoutMs: number; private generation = 0; private deviceFlow: { generation: number; controller: AbortController } | null = null; + private deviceFlowState: GithubDeviceFlowState = "idle"; private storeWrite = Promise.resolve(); constructor(userDataPath: string, clientId?: string, options: GithubAuthServiceOptions = {}) { @@ -126,13 +129,20 @@ export class GithubAuthService { async status(): Promise { if (!this.tokens) { - return { configured: this.clientConfigured, authorized: false, login: null, tokenExpiresAt: null }; + return { + configured: this.clientConfigured, + authorized: false, + login: null, + tokenExpiresAt: null, + deviceFlowState: this.deviceFlowState + }; } return { configured: this.clientConfigured, authorized: true, login: this.tokens.login, - tokenExpiresAt: this.tokens.expiresAt + tokenExpiresAt: this.tokens.expiresAt, + deviceFlowState: this.deviceFlowState }; } @@ -146,10 +156,11 @@ export class GithubAuthService { throw new Error("GitHub OAuth is not configured (missing client id)."); } - this.cancelDeviceFlow(); + this.abortDeviceFlow(); const generation = ++this.generation; const controller = new AbortController(); this.deviceFlow = { generation, controller }; + this.deviceFlowState = "pending"; try { const body = new URLSearchParams({ client_id: this.clientId }); @@ -176,7 +187,9 @@ export class GithubAuthService { void this.pollDeviceCode(deviceCode, interval, flowLifetimeMs, generation, controller.signal) .catch((error) => { - if (!isAbortError(error)) console.warn("CanvasTTY GitHub OAuth device poll failed.", error); + if (controller.signal.aborted || isAbortError(error)) return; + this.finishDeviceFlow(generation, "failed"); + console.warn("CanvasTTY GitHub OAuth device poll failed.", error); }) .finally(() => { if (this.deviceFlow?.generation === generation) this.deviceFlow = null; @@ -184,14 +197,26 @@ export class GithubAuthService { return { userCode, verificationUri, interval, expiresAt: this.now() + flowLifetimeMs }; } catch (error) { - if (this.deviceFlow?.generation === generation) this.deviceFlow = null; + if (this.deviceFlow?.generation === generation) { + this.deviceFlow = null; + // The caller receives this error directly, so no separate outcome is kept. + this.deviceFlowState = "idle"; + } throw error; } } + /** Stops a pending device sign-in. A completed sign-in or an existing session is left untouched. */ + cancelDeviceFlow(): void { + if (!this.deviceFlow || this.deviceFlowState !== "pending") return; + this.abortDeviceFlow(); + this.deviceFlowState = "cancelled"; + } + async signOut(): Promise { this.generation += 1; - this.cancelDeviceFlow(); + this.abortDeviceFlow(); + this.deviceFlowState = "idle"; this.tokens = null; this.refreshPromise = null; await this.enqueueStoreWrite(async () => { @@ -199,9 +224,17 @@ export class GithubAuthService { }); } - private cancelDeviceFlow(): void { - this.deviceFlow?.controller.abort(); + private abortDeviceFlow(): void { + const flow = this.deviceFlow; + // Clear the flow before aborting so a fetcher that ignores its AbortSignal + // cannot apply a late response: every step re-checks isCurrentFlow(). this.deviceFlow = null; + flow?.controller.abort(); + } + + private finishDeviceFlow(generation: number, state: GithubDeviceFlowState): void { + if (this.deviceFlow?.generation !== generation || this.deviceFlowState !== "pending") return; + this.deviceFlowState = state; } private isCurrentFlow(generation: number, signal: AbortSignal): boolean { @@ -217,51 +250,82 @@ export class GithubAuthService { ): Promise { const started = this.now(); let interval = initialInterval; + let transientFailures = 0; + // A dropped connection, the request timeout, HTTP 429/5xx or an unreadable + // body is not the end of the flow: the person may still approve the code. + // Back off (RFC 8628 section 3.5) and poll again, but give up after a few + // failures in a row instead of polling a broken endpoint until the code expires. + const retryOrFail = (): boolean => { + transientFailures += 1; + if (transientFailures >= MAX_CONSECUTIVE_POLL_FAILURES) { + this.finishDeviceFlow(generation, "failed"); + return false; + } + interval = backedOff(interval); + return true; + }; while (this.now() - started < lifetimeMs) { await this.wait(interval * 1000, signal); if (!this.isCurrentFlow(generation, signal)) return; + if (this.now() - started >= lifetimeMs) break; const body = new URLSearchParams({ client_id: this.clientId, device_code: deviceCode, grant_type: "urn:ietf:params:oauth:grant-type:device_code" }); - // A dropped connection, the 15 s request timeout or a GitHub 5xx is not - // the end of the flow: the person may still approve the code. Back off - // (RFC 8628 section 3.5) and poll again until the code expires. - let payload: unknown; + let response: Response; + let payload: unknown = null; try { - const response = await this.request("https://github.com/login/oauth/access_token", { + response = await this.request("https://github.com/login/oauth/access_token", { method: "POST", headers: oauthHeaders(), body: body.toString() }, signal); - if (!response.ok) { - interval = backedOff(interval); - continue; - } - payload = await response.json(); + // OAuth errors may arrive with HTTP 400 (RFC 6749 section 5.2), so read the body either way. + payload = await response.json().catch(() => null); } catch (error) { if (signal.aborted) throw error; - interval = backedOff(interval); - continue; + if (!this.isCurrentFlow(generation, signal)) return; + if (retryOrFail()) continue; + return; } - if (!isRecord(payload)) { - interval = backedOff(interval); - continue; + if (!this.isCurrentFlow(generation, signal)) return; + const providerError = isRecord(payload) && typeof payload.error === "string" ? payload.error : null; + if (providerError === null && (!response.ok || !isRecord(payload))) { + if (isTransientStatus(response.status) || response.ok) { + if (retryOrFail()) continue; + return; + } + this.finishDeviceFlow(generation, "failed"); + return; } - if (payload.error === "authorization_pending" || payload.error === "slow_down") { - if (payload.error === "slow_down") { + if (providerError === "authorization_pending" || providerError === "slow_down") { + transientFailures = 0; + if (providerError === "slow_down") { // +5 s for this and later polls; GitHub may name a longer interval. - const requested = typeof payload.interval === "number" && Number.isFinite(payload.interval) ? payload.interval : 0; + const requested = isRecord(payload) && typeof payload.interval === "number" && Number.isFinite(payload.interval) + ? payload.interval + : 0; interval = Math.max(interval + 5, Math.min(requested, MAX_POLL_INTERVAL_SECONDS)); } continue; } - if (payload.error === "access_denied" || payload.error === "expired_token") return; - if (typeof payload.access_token === "string") { + if (providerError === "access_denied") { + this.finishDeviceFlow(generation, "denied"); + return; + } + if (providerError === "expired_token") { + this.finishDeviceFlow(generation, "expired"); + return; + } + if (providerError === null && isRecord(payload) && typeof payload.access_token === "string") { const login = await this.fetchLogin(payload.access_token, signal); - if (!login || !this.isCurrentFlow(generation, signal)) return; + if (!this.isCurrentFlow(generation, signal)) return; + if (!login) { + this.finishDeviceFlow(generation, "failed"); + return; + } const expiresIn = typeof payload.expires_in === "number" && payload.expires_in > 0 ? payload.expires_in : null; @@ -272,11 +336,15 @@ export class GithubAuthService { login }; this.tokens = next; + this.deviceFlowState = "idle"; await this.persist(next, generation); return; } + // Any other provider error (device_flow_disabled, incorrect_client_credentials, ...) is final. + this.finishDeviceFlow(generation, "failed"); return; } + this.finishDeviceFlow(generation, "expired"); } private async refreshAccessToken(expected: StoredTokens, generation: number): Promise { @@ -433,6 +501,12 @@ function isMissingFile(error: unknown): boolean { } const MAX_POLL_INTERVAL_SECONDS = 60; +/** Consecutive network, timeout, HTTP 429/5xx or unreadable answers before the sign-in is reported as failed. */ +const MAX_CONSECUTIVE_POLL_FAILURES = 5; + +function isTransientStatus(status: number): boolean { + return status === 408 || status === 429 || status >= 500; +} function backedOff(interval: number): number { return Math.min(Math.max(interval * 2, interval + 1), MAX_POLL_INTERVAL_SECONDS); diff --git a/src/preload/index.ts b/src/preload/index.ts index 8b048ffce..55cd354df 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -260,6 +260,7 @@ const api: CanvasTTYApi = { status: () => ipcRenderer.invoke(IPC.githubAuthStatus), start: () => ipcRenderer.invoke(IPC.githubAuthStart), signOut: () => ipcRenderer.invoke(IPC.githubAuthSignOut), + cancel: () => ipcRenderer.invoke(IPC.githubAuthCancel), openUrl: (url: string) => ipcRenderer.invoke(IPC.githubAuthOpenUrl, url) }, browser: { diff --git a/src/renderer/src/features/plugins/PluginSettingsSection.tsx b/src/renderer/src/features/plugins/PluginSettingsSection.tsx index df1a80f31..cf9ee0554 100644 --- a/src/renderer/src/features/plugins/PluginSettingsSection.tsx +++ b/src/renderer/src/features/plugins/PluginSettingsSection.tsx @@ -2,6 +2,7 @@ import { useEffect, useLayoutEffect, useRef, useState } from "react"; import type { AppSettings, GithubAuthStatus, + GithubDeviceFlowState, GithubDeviceFlowStart, GithubPluginSearchResult, InstalledPlugin, @@ -181,6 +182,7 @@ export function PluginSettingsSection({ const [githubCode, setGithubCode] = useState(null); const [githubBusy, setGithubBusy] = useState(false); const [codeCopied, setCodeCopied] = useState(false); + const githubFlowNotice = githubFlowNoticeText(locale, githubStatus?.deviceFlowState); const [showcasePreviews, setShowcasePreviews] = useState>({}); const [showcaseManifests, setShowcaseManifests] = useState>({}); const [selectedShowcase, setSelectedShowcase] = useState(null); @@ -197,28 +199,27 @@ export function PluginSettingsSection({ // The main process owns the device flow; this only reads its status, so it pauses while Settings is // closed and picks up (sign-in finished or code expired) as soon as Settings opens again. const stop = pollWhileOpen(open, githubCode.interval * 1000, async () => { - if (Date.now() >= githubCode.expiresAt) { - if (!cancelled) { - setGithubCode(null); - setError(t(locale, "githubAuthExpired")); - } - return STOP_POLLING; - } // A transient status read failure is retried on the next tick. const status = await window.canvasTTY.githubAuth.status(); if (cancelled) return STOP_POLLING; - setGithubStatus(status); - if (status.authorized) { + if (status.authorized || isGithubFlowOutcome(status.deviceFlowState)) { + setGithubStatus(status); + setGithubCode(null); + return STOP_POLLING; + } + if (Date.now() >= githubCode.expiresAt) { + setGithubStatus({ ...status, deviceFlowState: "expired" }); setGithubCode(null); return STOP_POLLING; } + setGithubStatus(status); return undefined; }, { immediate: false }); return () => { cancelled = true; stop(); }; - }, [githubCode, locale, open]); + }, [githubCode, open]); const inspect = async (): Promise => { if (busy || sourceUrl.trim().length === 0) return; @@ -365,10 +366,22 @@ export function PluginSettingsSection({ if (githubBusy) return; setGithubBusy(true); setError(null); + let flow: GithubDeviceFlowStart; + try { + flow = await window.canvasTTY.githubAuth.start(); + } catch (reason) { + // The IPC error is main-process text ("Error invoking remote method …", HTTP codes): it goes to the log, and the + // GitHub section shows its own localized line (not configured, or sign-in failed). + console.warn("GitHub sign-in could not start.", reason); + const status = await window.canvasTTY.githubAuth.status().catch(() => null); + setGithubStatus(status && !status.configured ? status + : { configured: true, authorized: false, login: null, tokenExpiresAt: null, ...status, deviceFlowState: "failed" }); + setGithubBusy(false); + return; + } try { - const flow = await window.canvasTTY.githubAuth.start(); setGithubCode(flow); - setGithubStatus({ configured: true, authorized: false, login: null, tokenExpiresAt: null }); + setGithubStatus({ configured: true, authorized: false, login: null, tokenExpiresAt: null, deviceFlowState: "pending" }); await openGithubAuthorization(flow, target); } catch (reason) { setError(errorMessage(reason, t(locale, "githubAuthNotConfigured"))); @@ -387,7 +400,8 @@ export function PluginSettingsSection({ configured: current?.configured ?? false, authorized: false, login: null, - tokenExpiresAt: null + tokenExpiresAt: null, + deviceFlowState: "idle" })); setGithubCode(null); } finally { @@ -395,6 +409,24 @@ export function PluginSettingsSection({ } }; + const runGithubCancel = async (): Promise => { + if (githubBusy) return; + setGithubBusy(true); + setError(null); + try { + await window.canvasTTY.githubAuth.cancel(); + // The poll may have finished first; the status says which outcome won. + setGithubStatus(await window.canvasTTY.githubAuth.status()); + setGithubCode(null); + setCodeCopied(false); + } catch (reason) { + console.warn("GitHub sign-in could not be cancelled.", reason); + setError(githubSigninText(locale, "cancelFailed")); + } finally { + setGithubBusy(false); + } + }; + const selectShowcaseTile = (fullName: string): void => { if (busy) return; setSelectedShowcase(fullName); @@ -833,6 +865,12 @@ export function PluginSettingsSection({ )} + {!githubAuthorized && !githubCode && githubFlowNotice && ( + // A cancelled sign-in is the person's own choice; a denied, expired or failed one is an error. + githubStatus?.deviceFlowState === "cancelled" + ?

{githubFlowNotice}

+ :

{githubFlowNotice}

+ )} {!githubAuthorized && !githubCode &&

{t(locale, "githubAuthOptional")}

} {githubStatus?.authorized ? (
@@ -882,6 +920,9 @@ export function PluginSettingsSection({ {t(locale, "githubAuthOpenExternal")}
+ ) : githubStatus && !githubConfigured ? (

{t(locale, "githubAuthNotConfigured")}

@@ -1156,3 +1197,39 @@ function errorMessage(error: unknown, fallback: string): string { .replace(/[\u0000-\u001f\u007f]/g, " ") .slice(0, 320); } + +function isGithubFlowOutcome(state: GithubDeviceFlowState | undefined): boolean { + return state === "denied" || state === "expired" || state === "failed" || state === "cancelled"; +} + +// The sign-in outcome lines live here, in the lazily loaded Settings chunk, not in the startup string table. +const GITHUB_SIGNIN_TEXT = { + ru: { + denied: "Запрос на вход в GitHub отклонён. Можно попробовать снова.", + failed: "Не удалось завершить вход в GitHub. Можно попробовать снова.", + cancelled: "Вход в GitHub отменён.", + cancel: "Отменить вход", + cancelFailed: "Не удалось отменить вход в GitHub. Подробности записаны в журнал." + }, + en: { + denied: "The GitHub sign-in request was denied. You can try again.", + failed: "GitHub sign-in could not be completed. You can try again.", + cancelled: "GitHub sign-in was cancelled.", + cancel: "Cancel sign-in", + cancelFailed: "GitHub sign-in could not be cancelled. Details are in the log." + } +} satisfies Record>; + +function githubSigninText(locale: LocaleId, key: keyof typeof GITHUB_SIGNIN_TEXT.en): string { + return GITHUB_SIGNIN_TEXT[locale === "ru" ? "ru" : "en"][key]; +} + +function githubFlowNoticeText(locale: LocaleId, state: GithubDeviceFlowState | undefined): string | null { + switch (state) { + case "denied": return githubSigninText(locale, "denied"); + case "expired": return t(locale, "githubAuthExpired"); + case "failed": return githubSigninText(locale, "failed"); + case "cancelled": return githubSigninText(locale, "cancelled"); + default: return null; + } +} diff --git a/src/shared/contracts.ts b/src/shared/contracts.ts index 6827fde93..e1845a525 100644 --- a/src/shared/contracts.ts +++ b/src/shared/contracts.ts @@ -1170,8 +1170,12 @@ export interface GithubAuthStatus { authorized: boolean; login: string | null; tokenExpiresAt: number | null; + /** Outcome of the most recent device sign-in; "idle" when none is pending or to report. */ + deviceFlowState: GithubDeviceFlowState; } +export type GithubDeviceFlowState = "idle" | "pending" | "denied" | "expired" | "failed" | "cancelled"; + export interface GithubDeviceFlowStart { userCode: string; verificationUri: string; @@ -2091,6 +2095,7 @@ export interface CanvasTTYApi { githubAuth: { status(): Promise; start(): Promise; + cancel(): Promise; signOut(): Promise; openUrl(url: string): Promise; }; @@ -2292,6 +2297,7 @@ export const IPC = { githubAuthStatus: "github-auth:status", githubAuthStart: "github-auth:start", githubAuthSignOut: "github-auth:sign-out", + githubAuthCancel: "github-auth:cancel", githubAuthOpenUrl: "github-auth:open-url", terminalList: "terminal:list", terminalOpenFile: "terminal:open-file", diff --git a/tests/github-auth-integration.test.mjs b/tests/github-auth-integration.test.mjs index 7035b7eea..078a00a1e 100644 --- a/tests/github-auth-integration.test.mjs +++ b/tests/github-auth-integration.test.mjs @@ -9,6 +9,8 @@ const settingsPath = new URL( "../src/renderer/src/features/plugins/PluginSettingsSection.tsx", import.meta.url ); +const ipcPath = new URL("../src/main/ipc/registerIpc.ts", import.meta.url); +const preloadPath = new URL("../src/preload/index.ts", import.meta.url); test("GitHub sign-in supports optional build input and both browser routes", async () => { const [config, main, releaseWorkflow, settings] = await Promise.all([ @@ -31,3 +33,19 @@ test("GitHub sign-in supports optional build input and both browser routes", asy assert.match(settings, /else await window\.canvasTTY\.githubAuth\.openUrl\(url\)/); assert.match(settings, /const url = flow\.verificationUri/); }); + +test("a pending GitHub sign-in can be cancelled through the trusted renderer only", async () => { + const [ipc, preload, settings] = await Promise.all([ + readFile(ipcPath, "utf8"), + readFile(preloadPath, "utf8"), + readFile(settingsPath, "utf8") + ]); + const start = ipc.indexOf("ipcMain.handle(IPC.githubAuthCancel,"); + assert.notEqual(start, -1); + const handler = ipc.slice(start, ipc.indexOf("ipcMain.", start + 1)); + assert.match(handler, /assertMainRenderer\(event, getMainWindow\)/); + assert.match(handler, /githubAuth\.cancelDeviceFlow\(\)/); + assert.match(preload, /cancel: \(\) => ipcRenderer\.invoke\(IPC\.githubAuthCancel\)/); + assert.match(settings, /window\.canvasTTY\.githubAuth\.cancel\(\)/); + assert.match(settings, /githubSigninText\(locale, "cancel"\)/); +}); diff --git a/tests/github-auth.test.mjs b/tests/github-auth.test.mjs index 793039552..e5dae66b1 100644 --- a/tests/github-auth.test.mjs +++ b/tests/github-auth.test.mjs @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { mkdtemp, readFile, readdir, rm } from "node:fs/promises"; +import { mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import test from "node:test"; import { GithubAuthService } from "../src/main/services/GithubAuthService.ts"; @@ -63,7 +63,8 @@ test("GitHub device flow persists an encrypted session atomically without reques configured: true, authorized: true, login: "howdeploy", - tokenExpiresAt: null + tokenExpiresAt: null, + deviceFlowState: "idle" }); assert.equal(await restored.getToken(), "access"); await service.signOut(); @@ -157,7 +158,8 @@ test("signOut cancels an in-flight device flow and prevents a late token from be configured: true, authorized: false, login: null, - tokenExpiresAt: null + tokenExpiresAt: null, + deviceFlowState: "idle" }); await assert.rejects(() => readFile(`${userData}/github-oauth.json`, "utf8"), { code: "ENOENT" }); } finally { @@ -211,7 +213,8 @@ test("reports an unconfigured OAuth client before starting device flow", async ( configured: false, authorized: false, login: null, - tokenExpiresAt: null + tokenExpiresAt: null, + deviceFlowState: "idle" }); await assert.rejects( () => service.startDeviceFlow(), @@ -271,3 +274,213 @@ test("device polling survives network errors and timeouts, backs off, and honour await rm(userData, { recursive: true, force: true }); } }); + +const deviceCodeAnswer = () => Response.json({ + device_code: "test-device-code", + user_code: "TEST-CODE", + verification_uri: "https://github.com/login/device", + expires_in: 900, + interval: 1 +}); + +function serviceWith(userData, pollAnswer, options = {}) { + const requests = { polls: 0, profile: 0 }; + const fetcher = async (url, init = {}) => { + if (String(url).endsWith("/login/device/code")) return deviceCodeAnswer(); + if (String(url).endsWith("/login/oauth/access_token")) { + requests.polls += 1; + return pollAnswer(requests.polls, init); + } + if (String(url) === "https://api.github.com/user") { + requests.profile += 1; + return Response.json({ login: "fixture-user" }); + } + return new Response("missing", { status: 404 }); + }; + const service = new GithubAuthService(userData, "client-id", { + fetcher, + safeStorage, + delay: async () => undefined, + ...options + }); + return { service, requests }; +} + +test("access_denied, expired_token and unknown provider errors stop polling with a reported outcome", async () => { + for (const [answer, expected] of [ + [() => Response.json({ error: "access_denied" }), "denied"], + [() => Response.json({ error: "expired_token" }), "expired"], + [() => Response.json({ error: "access_denied" }, { status: 400 }), "denied"], + [() => Response.json({ error: "device_flow_disabled" }), "failed"], + [() => Response.json({ error: "incorrect_client_credentials" }, { status: 400 }), "failed"], + [() => new Response("forbidden", { status: 403 }), "failed"] + ]) { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-${expected}-`); + try { + const { service, requests } = serviceWith(userData, answer); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).deviceFlowState !== "pending"); + const status = await service.status(); + assert.equal(status.authorized, false); + assert.equal(status.deviceFlowState, expected); + // Give a runaway poll loop a chance to show itself. + for (let i = 0; i < 5; i += 1) await new Promise((resolve) => setImmediate(resolve)); + assert.equal(requests.polls, 1, `${expected}: polling stopped after the terminal answer`); + assert.equal(requests.profile, 0); + await assert.rejects(() => readFile(`${userData}/github-oauth.json`, "utf8"), { code: "ENOENT" }); + } finally { + await rm(userData, { recursive: true, force: true }); + } + } +}); + +test("cancelDeviceFlow stops polling, reports cancelled and keeps an existing session", async () => { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-cancel-flow-`); + const existing = { accessToken: "existing-fx", refreshToken: null, expiresAt: null, login: "existing-user" }; + const storedFile = JSON.stringify({ data: Buffer.from(JSON.stringify(existing)).toString("base64") }); + await writeFile(`${userData}/github-oauth.json`, storedFile); + const pollSignals = []; + const releases = []; + try { + const { service, requests } = serviceWith( + userData, + // The fetcher ignores its AbortSignal on purpose: a late answer must still be discarded. + () => new Promise((resolve) => { releases.push(resolve); }), + { + delay: (_duration, signal) => { + pollSignals.push(signal); + return signal.aborted ? Promise.reject(new DOMException("cancelled", "AbortError")) : Promise.resolve(); + } + } + ); + await service.load(); + await service.startDeviceFlow(); + await waitFor(() => releases.length === 1); + assert.equal((await service.status()).deviceFlowState, "pending"); + + service.cancelDeviceFlow(); + assert.equal(pollSignals[0].aborted, true); + assert.equal((await service.status()).deviceFlowState, "cancelled"); + + releases[0](Response.json({ access_token: "late-fx", token_type: "bearer", scope: "" })); + for (let i = 0; i < 5; i += 1) await new Promise((resolve) => setImmediate(resolve)); + + const status = await service.status(); + assert.equal(status.deviceFlowState, "cancelled"); + assert.equal(status.login, "existing-user"); + assert.equal(requests.polls, 1, "no poll after cancel"); + assert.equal(requests.profile, 0, "the late token was not used"); + assert.equal(await service.getToken(), "existing-fx"); + assert.equal(await readFile(`${userData}/github-oauth.json`, "utf8"), storedFile); + + // Cancelling again, or with nothing pending, changes nothing. + service.cancelDeviceFlow(); + assert.equal((await service.status()).deviceFlowState, "cancelled"); + } finally { + await rm(userData, { recursive: true, force: true }); + } +}); + +test("cancelDeviceFlow after a completed sign-in keeps it", async () => { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-cancel-done-`); + try { + const { service } = serviceWith(userData, () => Response.json({ access_token: "done-fx", token_type: "bearer", scope: "" })); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).authorized); + const completed = await service.status(); + assert.equal(completed.deviceFlowState, "idle"); + service.cancelDeviceFlow(); + assert.deepEqual(await service.status(), completed); + assert.equal(await service.getToken(), "done-fx"); + await service.signOut(); + } finally { + await rm(userData, { recursive: true, force: true }); + } +}); + +test("slow_down adds five seconds to this and every later poll", async () => { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-slow-down-`); + const waits = []; + const answers = [ + { error: "authorization_pending" }, + { error: "slow_down" }, + { error: "authorization_pending" }, + { error: "slow_down" }, + { error: "authorization_pending" }, + { access_token: "slow-fx", token_type: "bearer", scope: "" } + ]; + try { + const { service } = serviceWith(userData, () => Response.json(answers.shift()), { + delay: async (ms) => { waits.push(ms); } + }); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).authorized); + assert.deepEqual(waits, [1_000, 1_000, 6_000, 6_000, 11_000, 11_000]); + await service.signOut(); + } finally { + await rm(userData, { recursive: true, force: true }); + } +}); + +test("transient poll errors retry with back-off, reset on a valid answer, and give up after a bounded run", async () => { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-bounded-`); + let clock = 1_000_000; + const waits = []; + const transient = [ + () => { throw new TypeError("fetch failed"); }, + () => new Response("unavailable", { status: 503 }), + () => new Response("too many", { status: 429 }), + () => new Response("not json", { status: 200 }), + () => new Response("bad gateway", { status: 502 }) + ]; + // Four failures, one pending answer (resets the count), then five failures in a row. + const answers = [...transient.slice(0, 4), () => Response.json({ error: "authorization_pending" }), ...transient]; + const warn = console.warn; + console.warn = () => undefined; + try { + const { service, requests } = serviceWith(userData, () => answers.shift()(), { + now: () => clock, + delay: async (ms) => { waits.push(ms); clock += ms; } + }); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).deviceFlowState !== "pending"); + assert.equal((await service.status()).deviceFlowState, "failed"); + assert.equal(requests.polls, 10); + assert.equal(answers.length, 0); + assert.deepEqual(waits, [1_000, 2_000, 4_000, 8_000, 16_000, 16_000, 32_000, 60_000, 60_000, 60_000]); + } finally { + console.warn = warn; + await rm(userData, { recursive: true, force: true }); + } +}); + +test("the device code expires on the host clock even while GitHub keeps answering pending", async () => { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-elapsed-`); + let clock = 1_000_000; + try { + const { service, requests } = serviceWith(userData, () => Response.json({ error: "authorization_pending" }), { + now: () => clock, + pollTimeoutMs: 2_500, + delay: async (ms) => { clock += ms; } + }); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).deviceFlowState !== "pending"); + assert.equal((await service.status()).deviceFlowState, "expired"); + assert.equal(requests.polls, 2); + } finally { + await rm(userData, { recursive: true, force: true }); + } +}); + +test("signOut clears a reported sign-in outcome", async () => { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-signout-outcome-`); + try { + const { service } = serviceWith(userData, () => Response.json({ error: "access_denied" })); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).deviceFlowState === "denied"); + await service.signOut(); + assert.equal((await service.status()).deviceFlowState, "idle"); + } finally { + await rm(userData, { recursive: true, force: true }); + } +}); diff --git a/tests/github-signin-errors-ui.test.mjs b/tests/github-signin-errors-ui.test.mjs new file mode 100644 index 000000000..db7e6edd9 --- /dev/null +++ b/tests/github-signin-errors-ui.test.mjs @@ -0,0 +1,72 @@ +// The GitHub sign-in buttons show localized outcomes; main-process error text (IPC prefix, HTTP codes) only goes to the log. +import assert from "node:assert/strict"; +import test from "node:test"; +import { findAll, importWithFakeReact, tick } from "./helpers/fake-react.mjs"; + +const section = await importWithFakeReact("src/renderer/src/features/plugins/PluginSettingsSection.tsx", "PluginSettingsSection"); + +function harness(t, locale, githubAuth) { + const oldWindow = globalThis.window; + const warnings = []; + t.mock.method(console, "warn", (...args) => { warnings.push(args); }); + globalThis.window = { setTimeout, clearTimeout, canvasTTY: { githubAuth, appVersion: async () => "1.8.1" } }; + t.after(() => { section.__unmount(); globalThis.window = oldWindow; }); + section.__reset(); + const none = async () => ({}); + const props = { settings: { locale }, plugins: [], open: false, onPreviewPlugin: none, onInstallPlugin: none, onSearchPlugins: async () => [], + onShowcasePlugins: async () => [], onFetchPluginIcons: none, onPreviewManifests: none, onOpenBrowser: none, onCheckPluginUpdates: none, + onUpdatePlugin: none, onSetPluginModules: none, onSetPluginEnabled: none, onUninstallPlugin: none, onOpenPluginContribution: none }; + let tree; + const render = () => { section.__flush(); tree = section.__render(section.PluginSettingsSection, props); return tree; }; + const settle = async () => { for (let i = 0; i < 6; i++) { render(); await tick(); } render(); }; + const button = (label) => findAll(tree, (node) => node.type === "button" && node.props?.children === label)[0]; + return { render, settle, button, warnings, tree: () => tree }; +} + +const ipcFailure = new Error("Error invoking remote method 'github-auth:start': Error: GitHub device flow failed with HTTP 404."); + +test("a sign-in that cannot start shows the localized failure in the GitHub block and logs the details (ru)", async (t) => { + const status = { configured: true, authorized: false, login: null, tokenExpiresAt: null, deviceFlowState: "idle" }; + const ui = harness(t, "ru", { status: async () => status, start: async () => { throw ipcFailure; } }); + await ui.settle(); + const signIn = ui.button("Войти в системном браузере"); + assert.ok(signIn, "the sign-in button"); + signIn.props.onClick(); + await ui.settle(); + const json = JSON.stringify(ui.tree()); + assert.match(json, /Не удалось завершить вход в GitHub/u); + assert.doesNotMatch(json, /Error invoking|github-auth:start|HTTP 404/u); + assert.ok(ui.warnings.some(([, reason]) => reason === ipcFailure), "the details are logged"); +}); + +test("a build without an OAuth client shows its own line, not the raw error", async (t) => { + let configured = true; + const ui = harness(t, "en", { + status: async () => ({ configured, authorized: false, login: null, tokenExpiresAt: null, deviceFlowState: "idle" }), + start: async () => { configured = false; throw new Error("Error invoking remote method 'github-auth:start': Error: GitHub OAuth is not configured (missing client id)."); } + }); + await ui.settle(); + ui.button("Sign in with the system browser").props.onClick(); + await ui.settle(); + const json = JSON.stringify(ui.tree()); + assert.match(json, /this build has no OAuth Client ID/u); + assert.doesNotMatch(json, /Error invoking|missing client id/u); +}); + +test("a failed, denied or expired sign-in reads as an error; a cancelled one as a plain status line", async (t) => { + const line = (tree, text) => findAll(tree, (node) => node.type === "p" && node.props?.children === text)[0]; + for (const [state, text, error] of [ + ["failed", "Не удалось завершить вход в GitHub. Можно попробовать снова.", true], + ["denied", null, true], + ["expired", null, true], + ["cancelled", null, false] + ]) { + const ui = harness(t, "ru", { status: async () => ({ configured: true, authorized: false, login: null, tokenExpiresAt: null, deviceFlowState: state }) }); + await ui.settle(); + const notice = text ? line(ui.tree(), text) + : findAll(ui.tree(), (node) => node.type === "p" && /plugin-github-unavailable|plugin-settings__error/u.test(node.props?.className ?? ""))[0]; + assert.ok(notice, state); + assert.equal(notice.props.className, error ? "plugin-settings__error" : "plugin-github-unavailable", state); + assert.equal(notice.props.role, error ? "alert" : "status", state); + } +}); diff --git a/tests/renderer-bundle-splitting.test.mjs b/tests/renderer-bundle-splitting.test.mjs index c30dc7f40..1b8b89ce5 100644 --- a/tests/renderer-bundle-splitting.test.mjs +++ b/tests/renderer-bundle-splitting.test.mjs @@ -12,10 +12,17 @@ import { build } from "esbuild"; // must shrink accordingly. const appEntry = fileURLToPath(new URL("../src/renderer/src/App.tsx", import.meta.url)); -async function bundleApp() { - // Only the dependency graph and byte counts are inspected; esbuild can produce - // both in memory without writing and deleting an entire renderer bundle. - const { metafile } = await build({ +let bundled; + +function bundleApp() { + bundled ??= bundleAppOnce(); + return bundled; +} + +async function bundleAppOnce() { + // Only the dependency graph, byte counts and chunk text are inspected; esbuild can produce + // them in memory without writing and deleting an entire renderer bundle. + const { metafile, outputFiles } = await build({ entryPoints: [appEntry], bundle: true, splitting: true, @@ -33,7 +40,7 @@ async function bundleApp() { logLevel: "silent", loader: { ".svg": "text", ".css": "empty", ".png": "empty", ".webp": "empty", ".ttf": "empty", ".woff2": "empty", ".ico": "empty" } }); - return metafile; + return { metafile, outputFiles }; } function outputFor(metafile, suffix) { @@ -57,7 +64,7 @@ function staticallyReachableBytes(metafile, entry) { } test("Settings and the launch/link dialogs are split out of the app's static import graph", async () => { - const metafile = await bundleApp(); + const { metafile } = await bundleApp(); const appPath = outputFor(metafile, "App.js"); assert.ok(appPath, "expected an App.js bundle output"); @@ -92,3 +99,27 @@ test("Settings and the launch/link dialogs are split out of the app's static imp `code statically reachable from App.js should stay bounded now that heavy UI loads on demand, got ${eagerBytes} bytes` ); }); + +// Text that only Settings shows is kept next to it (not in the startup string table), so it loads with the Settings +// chunk. Each entry: a module that must not be statically reachable from App.js, and an English string from it that +// must not appear in any chunk App.js loads at startup. +const SETTINGS_ONLY_TEXT = [ + ["src/renderer/src/features/plugins/PluginSettingsSection.tsx", "GitHub sign-in could not be cancelled"] +]; + +test("text that only Settings shows stays out of the startup bundle", async () => { + const { metafile, outputFiles } = await bundleApp(); + const { paths: eagerPaths } = staticallyReachableBytes(metafile, outputFor(metafile, "App.js")); + const textOf = (path) => outputFiles.find((file) => file.path.replaceAll("\\", "/").endsWith(`/${path}`))?.text ?? ""; + const eagerText = [...eagerPaths].map(textOf).join("\n"); + assert.ok(eagerText.length > 0, "the startup chunks must be readable"); + for (const [module, text] of SETTINGS_ONLY_TEXT) { + const holders = Object.entries(metafile.outputs).filter(([, output]) => Object.keys(output.inputs).some((input) => input.endsWith(module))); + assert.ok(holders.length > 0, `${module} is bundled`); + for (const [path] of holders) { + assert.ok(!eagerPaths.has(path), `${module} must only load with Settings`); + assert.ok(textOf(path).includes(text), `"${text}" must be bundled with ${module}`); + } + assert.ok(!eagerText.includes(text), `"${text}" must not be in the startup bundle`); + } +}); From b795d42e09ea2e88eabf0fffe2b3e815a622ee70 Mon Sep 17 00:00:00 2001 From: BIackFIame <77388790+BIackFIame@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:16:29 +0000 Subject: [PATCH 2/6] fix(github): report a timed-out profile request as a failed sign-in When GitHub returned a token but the /user request hit the request timeout, the timeout's AbortError was taken for a cancellation: the poll stopped silently, the flow was cleared and the status stayed "pending", so Cancel did nothing. Only this flow's own abort now counts as a cancellation; a timed-out profile request ends the sign-in as failed. --- src/main/services/GithubAuthService.ts | 7 +++++-- tests/github-auth.test.mjs | 28 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/main/services/GithubAuthService.ts b/src/main/services/GithubAuthService.ts index 3d416d1b2..168791b71 100644 --- a/src/main/services/GithubAuthService.ts +++ b/src/main/services/GithubAuthService.ts @@ -187,7 +187,9 @@ export class GithubAuthService { void this.pollDeviceCode(deviceCode, interval, flowLifetimeMs, generation, controller.signal) .catch((error) => { - if (controller.signal.aborted || isAbortError(error)) return; + // Only this flow's own abort (Cancel, sign-out, a new sign-in) is a cancellation; a request + // that hit its timeout also throws an AbortError, and that is a failure the person must see. + if (controller.signal.aborted) return; this.finishDeviceFlow(generation, "failed"); console.warn("CanvasTTY GitHub OAuth device poll failed.", error); }) @@ -394,7 +396,8 @@ export class GithubAuthService { const payload: unknown = await response.json(); return isRecord(payload) && typeof payload.login === "string" ? payload.login : null; } catch (error) { - if (isAbortError(error)) throw error; + // Rethrow only a cancelled flow; a timed-out or failed profile request is a failed sign-in. + if (signal.aborted && isAbortError(error)) throw error; return null; } } diff --git a/tests/github-auth.test.mjs b/tests/github-auth.test.mjs index e5dae66b1..e22b232cc 100644 --- a/tests/github-auth.test.mjs +++ b/tests/github-auth.test.mjs @@ -484,3 +484,31 @@ test("signOut clears a reported sign-in outcome", async () => { await rm(userData, { recursive: true, force: true }); } }); + +test("a profile request that times out after the token arrived reports a failed sign-in, not a stuck pending one", async () => { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-profile-timeout-`); + const fetcher = async (url, init = {}) => { + if (String(url).endsWith("/login/device/code")) return deviceCodeAnswer(); + if (String(url).endsWith("/login/oauth/access_token")) return Response.json({ access_token: "access", token_type: "bearer", scope: "" }); + // The profile request hangs until the service's own request timeout aborts it. + return new Promise((_resolve, reject) => { + init.signal.addEventListener("abort", () => reject(new DOMException("The operation was aborted.", "AbortError")), { once: true }); + }); + }; + const warn = console.warn; + console.warn = () => undefined; + try { + const service = new GithubAuthService(userData, "client-id", { + fetcher, safeStorage, requestTimeoutMs: 10, delay: async () => undefined + }); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).deviceFlowState !== "pending"); + const status = await service.status(); + assert.equal(status.deviceFlowState, "failed"); + assert.equal(status.authorized, false); + await assert.rejects(() => readFile(`${userData}/github-oauth.json`, "utf8"), { code: "ENOENT" }); + } finally { + console.warn = warn; + await rm(userData, { recursive: true, force: true }); + } +}); From 7fb908c53e24588e153a55c53a7b431303f03b89 Mon Sep 17 00:00:00 2001 From: BIackFIame <77388790+BIackFIame@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:16:29 +0000 Subject: [PATCH 3/6] fix(github): retry HTTP 429/5xx token answers that carry an OAuth error body A transient 429/5xx from the token endpoint with a JSON body such as {"error":"temporarily_unavailable"} skipped the transient-status check and ended the sign-in as an unknown provider error after one answer. These statuses are now retried with backoff whatever the body says; slow_down keeps its own handling. --- src/main/services/GithubAuthService.ts | 8 +++++++- tests/github-auth.test.mjs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/main/services/GithubAuthService.ts b/src/main/services/GithubAuthService.ts index 168791b71..66055a245 100644 --- a/src/main/services/GithubAuthService.ts +++ b/src/main/services/GithubAuthService.ts @@ -294,8 +294,14 @@ export class GithubAuthService { } if (!this.isCurrentFlow(generation, signal)) return; const providerError = isRecord(payload) && typeof payload.error === "string" ? payload.error : null; + // HTTP 429/5xx is transient whatever its body says (e.g. {"error":"temporarily_unavailable"}); + // slow_down keeps its own handling below, which also raises the interval. + if (!response.ok && isTransientStatus(response.status) && providerError !== "slow_down") { + if (retryOrFail()) continue; + return; + } if (providerError === null && (!response.ok || !isRecord(payload))) { - if (isTransientStatus(response.status) || response.ok) { + if (response.ok) { if (retryOrFail()) continue; return; } diff --git a/tests/github-auth.test.mjs b/tests/github-auth.test.mjs index e22b232cc..9f53255de 100644 --- a/tests/github-auth.test.mjs +++ b/tests/github-auth.test.mjs @@ -512,3 +512,22 @@ test("a profile request that times out after the token arrived reports a failed await rm(userData, { recursive: true, force: true }); } }); + +test("HTTP 429 and 5xx answers with an OAuth error body are retried like other transient answers", async () => { + for (const status of [429, 503]) { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-transient-json-`); + try { + const { service, requests } = serviceWith(userData, (poll) => (poll === 1 + ? Response.json({ error: "temporarily_unavailable" }, { status }) + : Response.json({ access_token: "access", token_type: "bearer", scope: "" }))); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).deviceFlowState !== "pending" || (await service.status()).authorized); + const result = await service.status(); + assert.equal(result.authorized, true, `HTTP ${status}: the sign-in went on after the transient answer`); + assert.equal(requests.polls, 2); + await service.signOut(); + } finally { + await rm(userData, { recursive: true, force: true }); + } + } +}); From ed4477c3e52d9ea70291a2306f352ac148fa8b0e Mon Sep 17 00:00:00 2001 From: BIackFIame <77388790+BIackFIame@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:42:55 +0000 Subject: [PATCH 4/6] fix(github): bound OAuth response bodies by the request timeout, cancel and code lifetime A token answer whose headers arrived but whose body never finished (for example a 429/5xx from a proxy) was read after request() had already cleared its timeout and dropped the flow's abort listener, so the poll stayed pending past every deadline. request() now reads the body inside the same window: the request timeout, the flow's cancel and, while polling, the device code's remaining lifetime all end the read. --- src/main/services/GithubAuthService.ts | 64 ++++++++++++++++++-------- tests/github-auth.test.mjs | 58 +++++++++++++++++++++++ 2 files changed, 103 insertions(+), 19 deletions(-) diff --git a/src/main/services/GithubAuthService.ts b/src/main/services/GithubAuthService.ts index 66055a245..75c6abae1 100644 --- a/src/main/services/GithubAuthService.ts +++ b/src/main/services/GithubAuthService.ts @@ -164,13 +164,14 @@ export class GithubAuthService { try { const body = new URLSearchParams({ client_id: this.clientId }); - const response = await this.request("https://github.com/login/device/code", { + const payload = await this.request("https://github.com/login/device/code", { method: "POST", headers: oauthHeaders(), body: body.toString() - }, controller.signal); - if (!response.ok) throw new Error(`GitHub device flow failed with HTTP ${response.status}.`); - const payload: unknown = await response.json(); + }, controller.signal, async (response): Promise => { + if (!response.ok) throw new Error(`GitHub device flow failed with HTTP ${response.status}.`); + return response.json(); + }); if (!isRecord(payload) || typeof payload.device_code !== "string" || typeof payload.user_code !== "string") { throw new Error("GitHub device flow returned an invalid response."); } @@ -279,13 +280,17 @@ export class GithubAuthService { let response: Response; let payload: unknown = null; try { - response = await this.request("https://github.com/login/oauth/access_token", { + // OAuth errors may arrive with HTTP 400 (RFC 6749 section 5.2), so read the body either way. + // The body read stays under the request timeout, the flow's abort and the code's remaining + // lifetime: a 429/5xx whose body never finishes must not keep the flow pending. + ({ response, payload } = await this.request("https://github.com/login/oauth/access_token", { method: "POST", headers: oauthHeaders(), body: body.toString() - }, signal); - // OAuth errors may arrive with HTTP 400 (RFC 6749 section 5.2), so read the body either way. - payload = await response.json().catch(() => null); + }, signal, async (answer) => ({ + response: answer, + payload: await answer.json().catch(() => null) as unknown + }), Math.max(1, lifetimeMs - (this.now() - started)))); } catch (error) { if (signal.aborted) throw error; if (!this.isCurrentFlow(generation, signal)) return; @@ -363,13 +368,11 @@ export class GithubAuthService { refresh_token: expected.refreshToken }); try { - const response = await this.request("https://github.com/login/oauth/access_token", { + const payload = await this.request("https://github.com/login/oauth/access_token", { method: "POST", headers: oauthHeaders(), body: body.toString() - }); - if (!response.ok) return null; - const payload: unknown = await response.json(); + }, undefined, async (response): Promise => (response.ok ? response.json() : null)); if (!isRecord(payload) || typeof payload.access_token !== "string") return null; if (this.generation !== generation || this.tokens !== expected) return null; const accessToken = payload.access_token; @@ -391,15 +394,13 @@ export class GithubAuthService { private async fetchLogin(accessToken: string, signal: AbortSignal): Promise { try { - const response = await this.request("https://api.github.com/user", { + const payload = await this.request("https://api.github.com/user", { headers: { authorization: `Bearer ${accessToken}`, accept: "application/vnd.github+json", "user-agent": "CanvasTTY plugin showcase" } - }, signal); - if (!response.ok) return null; - const payload: unknown = await response.json(); + }, signal, async (response): Promise => (response.ok ? response.json() : null)); return isRecord(payload) && typeof payload.login === "string" ? payload.login : null; } catch (error) { // Rethrow only a cancelled flow; a timed-out or failed profile request is a failed sign-in. @@ -408,15 +409,27 @@ export class GithubAuthService { } } - private async request(input: string, init: RequestInit, externalSignal?: AbortSignal): Promise { + /** + * Sends one request and reads its answer with `read`. The timeout (at most `timeoutMs`, never more + * than the configured request timeout) and `externalSignal` cover the whole exchange, body included: + * a response whose headers arrived but whose body never finishes is abandoned like a hung request. + */ + private async request( + input: string, + init: RequestInit, + externalSignal: AbortSignal | undefined, + read: (response: Response) => Promise, + timeoutMs = this.requestTimeoutMs + ): Promise { const controller = new AbortController(); const abort = (): void => controller.abort(); if (externalSignal?.aborted) controller.abort(); else externalSignal?.addEventListener("abort", abort, { once: true }); - const timer = setTimeout(() => controller.abort(), this.requestTimeoutMs); + const timer = setTimeout(() => controller.abort(), Math.min(timeoutMs, this.requestTimeoutMs)); timer.unref(); try { - return await this.fetcher(input, { ...init, signal: controller.signal }); + const exchange = (async (): Promise => read(await this.fetcher(input, { ...init, signal: controller.signal })))(); + return await untilAborted(exchange, controller.signal); } finally { clearTimeout(timer); externalSignal?.removeEventListener("abort", abort); @@ -493,6 +506,19 @@ function abortableDelay(durationMs: number, signal: AbortSignal): Promise }); } +/** Settles with `work`, or rejects with an AbortError as soon as `signal` aborts, whichever comes first. */ +function untilAborted(work: Promise, signal: AbortSignal): Promise { + if (signal.aborted) { + work.catch(() => undefined); + return Promise.reject(abortedError()); + } + return new Promise((resolve, reject) => { + const abort = (): void => reject(abortedError()); + signal.addEventListener("abort", abort, { once: true }); + work.then(resolve, reject).finally(() => signal.removeEventListener("abort", abort)); + }); +} + function abortedError(): DOMException { return new DOMException("GitHub OAuth operation was cancelled.", "AbortError"); } diff --git a/tests/github-auth.test.mjs b/tests/github-auth.test.mjs index 9f53255de..491346dfe 100644 --- a/tests/github-auth.test.mjs +++ b/tests/github-auth.test.mjs @@ -531,3 +531,61 @@ test("HTTP 429 and 5xx answers with an OAuth error body are retried like other t } } }); + +/** A response whose headers arrived but whose body never finishes; it ignores any AbortSignal. */ +function unfinishedBody(status) { + return new Response(new ReadableStream({ + start(controller) { controller.enqueue(new TextEncoder().encode('{"error":"temporarily_')); } + }), { status, headers: { "content-type": "application/json" } }); +} + +test("a 429/5xx answer whose body never finishes is cut off by the request timeout and polled again", async () => { + for (const status of [429, 503]) { + const userData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-unfinished-body-`); + try { + const { service, requests } = serviceWith(userData, (poll) => (poll === 1 + ? unfinishedBody(status) + : Response.json({ access_token: "access", token_type: "bearer", scope: "" })), { requestTimeoutMs: 20 }); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).authorized, 2_000); + assert.equal(requests.polls, 2, `HTTP ${status}: the hung body was abandoned and the next poll ran`); + await service.signOut(); + } finally { + await rm(userData, { recursive: true, force: true }); + } + } +}); + +test("a 503 answer whose body never finishes cannot outlive the device code or a cancel", async () => { + const expiredData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-unfinished-expiry-`); + try { + // The request timeout alone would wait an hour; the code's own lifetime must end the read. + const { service, requests } = serviceWith(expiredData, () => unfinishedBody(503), { + requestTimeoutMs: 3_600_000, + pollTimeoutMs: 50 + }); + await service.startDeviceFlow(); + await waitFor(async () => (await service.status()).deviceFlowState !== "pending", 2_000); + assert.equal((await service.status()).deviceFlowState, "expired"); + // A real timer may fire a moment before the host clock passes the deadline, allowing one more short poll. + assert.ok(requests.polls <= 2, `polls: ${requests.polls}`); + } finally { + await rm(expiredData, { recursive: true, force: true }); + } + + const cancelData = await mkdtemp(`${tmpdir()}/canvastty-github-auth-unfinished-cancel-`); + try { + const signals = []; + const { service } = serviceWith(cancelData, (_poll, init) => { + signals.push(init.signal); + return unfinishedBody(503); + }, { requestTimeoutMs: 3_600_000 }); + await service.startDeviceFlow(); + await waitFor(() => signals.length === 1); + service.cancelDeviceFlow(); + assert.equal(signals[0].aborted, true, "the cancel reached the request that is still reading its body"); + assert.equal((await service.status()).deviceFlowState, "cancelled"); + } finally { + await rm(cancelData, { recursive: true, force: true }); + } +}); From 43463861d75d26bee5c6ef53dc82b1ab3956fab8 Mon Sep 17 00:00:00 2001 From: BIackFIame <77388790+BIackFIame@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:45:03 +0000 Subject: [PATCH 5/6] fix(github): keep Cancel for a pending sign-in after Settings remounts Switching Settings sections remounts the plugin section, which dropped the displayed device code while the host flow kept running, and Cancel lived only next to that code. The section now reads a pending flow from the host status: without the code it shows that the sign-in waits for approval, offers Cancel, and keeps reading the status until the flow ends. --- .../plugins/PluginSettingsSection.tsx | 24 ++++++++++++++--- tests/github-signin-errors-ui.test.mjs | 27 ++++++++++++++++--- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/renderer/src/features/plugins/PluginSettingsSection.tsx b/src/renderer/src/features/plugins/PluginSettingsSection.tsx index cf9ee0554..87751358b 100644 --- a/src/renderer/src/features/plugins/PluginSettingsSection.tsx +++ b/src/renderer/src/features/plugins/PluginSettingsSection.tsx @@ -183,6 +183,9 @@ export function PluginSettingsSection({ const [githubBusy, setGithubBusy] = useState(false); const [codeCopied, setCodeCopied] = useState(false); const githubFlowNotice = githubFlowNoticeText(locale, githubStatus?.deviceFlowState); + // The host's flow can outlive this section (switching Settings sections remounts it and drops the code), + // so a pending flow is read from the host status, not from the locally kept code. + const githubFlowPending = !githubAuthorized && githubStatus?.deviceFlowState === "pending"; const [showcasePreviews, setShowcasePreviews] = useState>({}); const [showcaseManifests, setShowcaseManifests] = useState>({}); const [selectedShowcase, setSelectedShowcase] = useState(null); @@ -194,11 +197,11 @@ export function PluginSettingsSection({ const [updatingId, setUpdatingId] = useState(null); useEffect(() => { - if (!githubCode) return; + if (!githubCode && !githubFlowPending) return; let cancelled = false; // The main process owns the device flow; this only reads its status, so it pauses while Settings is // closed and picks up (sign-in finished or code expired) as soon as Settings opens again. - const stop = pollWhileOpen(open, githubCode.interval * 1000, async () => { + const stop = pollWhileOpen(open, (githubCode?.interval ?? GITHUB_PENDING_POLL_SECONDS) * 1000, async () => { // A transient status read failure is retried on the next tick. const status = await window.canvasTTY.githubAuth.status(); if (cancelled) return STOP_POLLING; @@ -207,7 +210,7 @@ export function PluginSettingsSection({ setGithubCode(null); return STOP_POLLING; } - if (Date.now() >= githubCode.expiresAt) { + if (githubCode && Date.now() >= githubCode.expiresAt) { setGithubStatus({ ...status, deviceFlowState: "expired" }); setGithubCode(null); return STOP_POLLING; @@ -219,7 +222,7 @@ export function PluginSettingsSection({ cancelled = true; stop(); }; - }, [githubCode, open]); + }, [githubCode, githubFlowPending, open]); const inspect = async (): Promise => { if (busy || sourceUrl.trim().length === 0) return; @@ -924,6 +927,14 @@ export function PluginSettingsSection({ {githubSigninText(locale, "cancel")} + ) : githubFlowPending ? ( + // The code itself was shown before the section remounted; the host flow still runs and can be stopped here. +
+

{githubSigninText(locale, "pending")}

+ +
) : githubStatus && !githubConfigured ? (

{t(locale, "githubAuthNotConfigured")}

) : null} @@ -1198,6 +1209,9 @@ function errorMessage(error: unknown, fallback: string): string { .slice(0, 320); } +/** Status read interval for a host flow whose code this section no longer has (after a remount). */ +const GITHUB_PENDING_POLL_SECONDS = 5; + function isGithubFlowOutcome(state: GithubDeviceFlowState | undefined): boolean { return state === "denied" || state === "expired" || state === "failed" || state === "cancelled"; } @@ -1208,6 +1222,7 @@ const GITHUB_SIGNIN_TEXT = { denied: "Запрос на вход в GitHub отклонён. Можно попробовать снова.", failed: "Не удалось завершить вход в GitHub. Можно попробовать снова.", cancelled: "Вход в GitHub отменён.", + pending: "Вход в GitHub ждёт подтверждения в браузере. Можно отменить его или начать заново.", cancel: "Отменить вход", cancelFailed: "Не удалось отменить вход в GitHub. Подробности записаны в журнал." }, @@ -1215,6 +1230,7 @@ const GITHUB_SIGNIN_TEXT = { denied: "The GitHub sign-in request was denied. You can try again.", failed: "GitHub sign-in could not be completed. You can try again.", cancelled: "GitHub sign-in was cancelled.", + pending: "A GitHub sign-in is waiting for approval in the browser. You can cancel it or start again.", cancel: "Cancel sign-in", cancelFailed: "GitHub sign-in could not be cancelled. Details are in the log." } diff --git a/tests/github-signin-errors-ui.test.mjs b/tests/github-signin-errors-ui.test.mjs index db7e6edd9..f82bd4929 100644 --- a/tests/github-signin-errors-ui.test.mjs +++ b/tests/github-signin-errors-ui.test.mjs @@ -5,15 +5,15 @@ import { findAll, importWithFakeReact, tick } from "./helpers/fake-react.mjs"; const section = await importWithFakeReact("src/renderer/src/features/plugins/PluginSettingsSection.tsx", "PluginSettingsSection"); -function harness(t, locale, githubAuth) { +function harness(t, locale, githubAuth, { open = false } = {}) { const oldWindow = globalThis.window; const warnings = []; t.mock.method(console, "warn", (...args) => { warnings.push(args); }); - globalThis.window = { setTimeout, clearTimeout, canvasTTY: { githubAuth, appVersion: async () => "1.8.1" } }; + globalThis.window = { setTimeout, clearTimeout, canvasTTY: { githubAuth, appVersion: async () => "1.8.1", clipboard: { writeText: () => undefined } } }; t.after(() => { section.__unmount(); globalThis.window = oldWindow; }); section.__reset(); const none = async () => ({}); - const props = { settings: { locale }, plugins: [], open: false, onPreviewPlugin: none, onInstallPlugin: none, onSearchPlugins: async () => [], + const props = { settings: { locale }, plugins: [], open, onPreviewPlugin: none, onInstallPlugin: none, onSearchPlugins: async () => [], onShowcasePlugins: async () => [], onFetchPluginIcons: none, onPreviewManifests: none, onOpenBrowser: none, onCheckPluginUpdates: none, onUpdatePlugin: none, onSetPluginModules: none, onSetPluginEnabled: none, onUninstallPlugin: none, onOpenPluginContribution: none }; let tree; @@ -70,3 +70,24 @@ test("a failed, denied or expired sign-in reads as an error; a cancelled one as assert.equal(notice.props.role, error ? "alert" : "status", state); } }); + +const pause = (ms) => new Promise((resolve) => { setTimeout(resolve, ms); }); + +test("a sign-in still pending in the host after the section remounts keeps its Cancel action", async (t) => { + // Switching Settings sections remounts this section: the displayed code is gone, the host flow is not. + let state = "pending"; + let cancels = 0; + const ui = harness(t, "en", { + status: async () => ({ configured: true, authorized: false, login: null, tokenExpiresAt: null, deviceFlowState: state }), + cancel: async () => { cancels += 1; state = "cancelled"; } + }, { open: true }); + await ui.settle(); + assert.match(JSON.stringify(ui.tree()), /waiting for approval in the browser/u); + const cancel = ui.button("Cancel sign-in"); + assert.ok(cancel, "Cancel is offered for the host's pending flow"); + cancel.props.onClick(); + await ui.settle(); + assert.equal(cancels, 1); + assert.equal(ui.button("Cancel sign-in"), undefined); + assert.match(JSON.stringify(ui.tree()), /GitHub sign-in was cancelled\./u); +}); From 0cc803e1043e5845be4d0393803449947cb208b8 Mon Sep 17 00:00:00 2001 From: BIackFIame <77388790+BIackFIame@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:45:04 +0000 Subject: [PATCH 6/6] fix(github): expire a displayed sign-in code even when the status read fails The Settings poll checked the code's deadline only after a successful host status read, so while reads failed an expired code stayed on screen with no notice. A failed read now still applies the deadline: past it, the code is cleared and the expired notice is shown; before it, the read is retried as before. --- .../plugins/PluginSettingsSection.tsx | 21 ++++++++++++++-- tests/github-signin-errors-ui.test.mjs | 24 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/features/plugins/PluginSettingsSection.tsx b/src/renderer/src/features/plugins/PluginSettingsSection.tsx index 87751358b..7731e1e11 100644 --- a/src/renderer/src/features/plugins/PluginSettingsSection.tsx +++ b/src/renderer/src/features/plugins/PluginSettingsSection.tsx @@ -199,11 +199,28 @@ export function PluginSettingsSection({ useEffect(() => { if (!githubCode && !githubFlowPending) return; let cancelled = false; + const showExpired = (): void => { + setGithubStatus((current) => ({ + configured: true, authorized: false, login: null, tokenExpiresAt: null, ...current, deviceFlowState: "expired" + })); + setGithubCode(null); + }; // The main process owns the device flow; this only reads its status, so it pauses while Settings is // closed and picks up (sign-in finished or code expired) as soon as Settings opens again. const stop = pollWhileOpen(open, (githubCode?.interval ?? GITHUB_PENDING_POLL_SECONDS) * 1000, async () => { - // A transient status read failure is retried on the next tick. - const status = await window.canvasTTY.githubAuth.status(); + let status: GithubAuthStatus; + try { + status = await window.canvasTTY.githubAuth.status(); + } catch (reason) { + if (cancelled) return STOP_POLLING; + // The displayed code's deadline does not depend on reading the host status. + if (githubCode && Date.now() >= githubCode.expiresAt) { + showExpired(); + return STOP_POLLING; + } + // A transient status read failure is retried on the next tick. + throw reason; + } if (cancelled) return STOP_POLLING; if (status.authorized || isGithubFlowOutcome(status.deviceFlowState)) { setGithubStatus(status); diff --git a/tests/github-signin-errors-ui.test.mjs b/tests/github-signin-errors-ui.test.mjs index f82bd4929..7806756d4 100644 --- a/tests/github-signin-errors-ui.test.mjs +++ b/tests/github-signin-errors-ui.test.mjs @@ -91,3 +91,27 @@ test("a sign-in still pending in the host after the section remounts keeps its C assert.equal(ui.button("Cancel sign-in"), undefined); assert.match(JSON.stringify(ui.tree()), /GitHub sign-in was cancelled\./u); }); + +test("a displayed code past its deadline shows the expired notice even when the status read fails", async (t) => { + let reads = 0; + const ui = harness(t, "en", { + status: async () => { + reads += 1; + if (reads > 1) throw new Error("status unavailable"); + return { configured: true, authorized: false, login: null, tokenExpiresAt: null, deviceFlowState: "idle" }; + }, + start: async () => ({ userCode: "TEST-CODE", verificationUri: "https://github.com/login/device", interval: 0.001, expiresAt: Date.now() - 1 }), + openUrl: async () => undefined + }, { open: true }); + await ui.settle(); + ui.button("Sign in with the system browser").props.onClick(); + await ui.settle(); + for (let i = 0; i < 20 && !/code expired/u.test(JSON.stringify(ui.tree())); i += 1) { + await pause(5); + await ui.settle(); + } + const json = JSON.stringify(ui.tree()); + assert.ok(reads > 1, "the poll tried to read the status"); + assert.match(json, /The GitHub authorization code expired\. Start sign-in again\./u); + assert.doesNotMatch(json, /TEST-CODE/u, "the expired code is no longer shown"); +});