From 95ad56fbc0dad6051720324320f766427e62be36 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:29:25 +0000 Subject: [PATCH 1/2] fix(self-driving): stop a 401 stranding the GitHub gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Self-driving GitHub gate polled /integrations/ with whatever access token the earlier integration phase left behind. Only the agent bootstrap refreshes the token, and it runs after this screen, so a token that aged out in between was rejected on every tick. fetchGithubConnected threw a raw AxiosError, the poll caught it and pinned githubConnected to false, and the gate is not skippable — so the user's only exit was to decline, which ends the run. - fetchGithubConnected (and its Slack twin, now one shared helper) throws a typed ApiError, so the caller can tell a 401 from "not connected". - The poll refreshes the token before the first tick, forces one refresh when the server rejects it, and raises the session-expired auth screen when the login stays rejected. - refreshAccessTokenIfNeeded moves to a leaf module both the agent bootstrap and the gate can import, and gains a force option plus a boolean result. Generated-By: PostHog Desktop Task-Id: fcb56dbc-e622-42f3-8659-0bbab14bb514 --- .../session-token.test.ts} | 32 ++- src/lib/agent/runner/shared/authenticate.ts | 59 +----- src/lib/agent/runner/shared/bootstrap.ts | 3 +- src/lib/api.ts | 74 +++---- src/lib/session-token.ts | 84 ++++++++ .../__tests__/useGithubConnection.test.ts | 125 ++++++++++- src/ui/tui/hooks/useGithubConnection.ts | 199 ++++++++++++------ src/ui/tui/screens/AuthErrorScreen.tsx | 7 +- 8 files changed, 416 insertions(+), 167 deletions(-) rename src/lib/{agent/runner/shared/__tests__/refresh-access-token-if-needed.test.ts => __tests__/session-token.test.ts} (83%) create mode 100644 src/lib/session-token.ts diff --git a/src/lib/agent/runner/shared/__tests__/refresh-access-token-if-needed.test.ts b/src/lib/__tests__/session-token.test.ts similarity index 83% rename from src/lib/agent/runner/shared/__tests__/refresh-access-token-if-needed.test.ts rename to src/lib/__tests__/session-token.test.ts index 5138af7c..edc7004a 100644 --- a/src/lib/agent/runner/shared/__tests__/refresh-access-token-if-needed.test.ts +++ b/src/lib/__tests__/session-token.test.ts @@ -1,4 +1,4 @@ -import { refreshAccessTokenIfNeeded } from '../authenticate'; +import { refreshAccessTokenIfNeeded } from '@lib/session-token'; import { refreshAccessToken } from '@utils/oauth'; import { OAuthError } from '@utils/oauth-errors'; import { isGrantRevoked, resetAuthSessionState } from '@lib/auth-session-state'; @@ -120,7 +120,7 @@ describe('refreshAccessTokenIfNeeded', () => { mockedRefresh.mockRejectedValueOnce(new Error('network down')); const session = sessionWith(aging()); - await expect(refreshAccessTokenIfNeeded(session)).resolves.toBeUndefined(); + await expect(refreshAccessTokenIfNeeded(session)).resolves.toBe(false); expect(session.credentials!.accessToken).toBe('pha_old'); expect(setAccessToken).not.toHaveBeenCalled(); }); @@ -140,4 +140,32 @@ describe('refreshAccessTokenIfNeeded', () => { expect(isGrantRevoked()).toBe(false); }); + + // The Self-driving GitHub gate calls this after a 401: the token was + // rejected, so its stated expiry proves nothing. + it('refreshes a token that still looks fresh when forced', async () => { + mockedRefresh.mockResolvedValueOnce({ + access_token: 'pha_new', + expires_in: 3600, + token_type: 'Bearer', + scope: 'project:read', + }); + const session = sessionWith( + aging({ expiresAt: Date.now() + 6 * 60 * 60 * 1000 }), + ); + + await expect( + refreshAccessTokenIfNeeded(session, { force: true }), + ).resolves.toBe(true); + expect(session.credentials!.accessToken).toBe('pha_new'); + }); + + it('still needs a refresh token when forced', async () => { + await expect( + refreshAccessTokenIfNeeded(sessionWith({ accessToken: 'pha_ci_key' }), { + force: true, + }), + ).resolves.toBe(false); + expect(mockedRefresh).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/agent/runner/shared/authenticate.ts b/src/lib/agent/runner/shared/authenticate.ts index 8644a328..3c649eb9 100644 --- a/src/lib/agent/runner/shared/authenticate.ts +++ b/src/lib/agent/runner/shared/authenticate.ts @@ -10,12 +10,9 @@ * back rather than fetching again. */ -import type { Credentials, WizardSession } from '@lib/wizard-session'; +import type { WizardSession } from '@lib/wizard-session'; import type { ProgramId } from '@lib/programs/program-registry'; import { getOrAskForProjectData } from '@utils/setup-utils'; -import { refreshAccessToken } from '@utils/oauth'; -import { OAuthError } from '@utils/oauth-errors'; -import { markGrantRevoked } from '@lib/auth-session-state'; import { analytics, groupsFromUser } from '@utils/analytics'; import { getUI } from '@ui'; import { logToFile } from '@utils/debug'; @@ -74,57 +71,3 @@ export async function authenticate( if (user) analytics.identifyUser(user); analytics.setGroups(groupsFromUser(user, host.apiHost)); } - -// Below this remaining lifetime a run risks outliving its token; just-minted and 7-day tokens skip. -// Only a second agent run in one invocation can be this old — see self-driving's chained phases. -const REFRESH_WHEN_REMAINING_MS = 50 * 60 * 1000; - -/** - * Grants the token endpoint refuses permanently. A dead grant means the login - * is gone, not that the network blipped, so only these mark the session. - */ -const DEAD_GRANT_CODES = new Set(['invalid_grant', 'invalid_client']); - -// Best-effort pre-run refresh: no refresh token or a failed grant keeps the existing token. -export async function refreshAccessTokenIfNeeded( - session: WizardSession, -): Promise { - const credentials = session.credentials; - if (!credentials?.refreshToken) return; - - // No expiry means we cannot tell how much life is left, so leave it alone — - // refreshing every run would spend a rotation for nothing. - if (credentials.expiresAt === undefined) return; - if (credentials.expiresAt - Date.now() >= REFRESH_WHEN_REMAINING_MS) return; - - try { - const token = await refreshAccessToken( - credentials.refreshToken, - session.baseUrl, - credentials.oauthClientId, - ); - // Replaced, not mutated: readers hold this object, and a new one keeps the - // store and the (possibly shallow-copied) session explicitly in step. - const refreshed: Credentials = { - ...credentials, - accessToken: token.access_token, - // Rotation: keep the returned refresh token or the old one stops working. - refreshToken: token.refresh_token ?? credentials.refreshToken, - expiresAt: Date.now() + token.expires_in * 1000, - }; - session.credentials = refreshed; - getUI().setAccessToken(refreshed); - } catch (error) { - // A dead grant is recorded but not thrown: the current token may still have - // minutes of life, and failing here would break runs that would have worked. - // If a 401 does follow, the auth-error screen can finally name the cause. - if (error instanceof OAuthError && DEAD_GRANT_CODES.has(error.code)) { - markGrantRevoked(); - analytics.wizardCapture('auth session expired', { reason: error.code }); - } - logToFile( - '[oauth] pre-run token refresh failed, continuing with the existing token:', - error instanceof Error ? error.message : error, - ); - } -} diff --git a/src/lib/agent/runner/shared/bootstrap.ts b/src/lib/agent/runner/shared/bootstrap.ts index a9e751e9..429e3c2d 100644 --- a/src/lib/agent/runner/shared/bootstrap.ts +++ b/src/lib/agent/runner/shared/bootstrap.ts @@ -10,7 +10,8 @@ import type { WizardSession } from '@lib/wizard-session'; import { analytics } from '@utils/analytics'; import { getUI } from '@ui'; -import { authenticate, refreshAccessTokenIfNeeded } from './authenticate'; +import { authenticate } from './authenticate'; +import { refreshAccessTokenIfNeeded } from '@lib/session-token'; import { maybeStampAiSdkDetected } from '@lib/programs/posthog-integration/detect'; import { createTriageLLMProvider } from '@lib/agent/triage-provider'; import { gatewayAuth } from '@lib/gateway-session'; diff --git a/src/lib/api.ts b/src/lib/api.ts index 02bf1c04..ecf1da64 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -235,58 +235,54 @@ const IntegrationsResponseSchema = z.object({ }); /** - * Check whether the project already has a Slack integration connected. - * Requires the `integration:read` scope. Throws on failure — callers - * (including the SlackConnectScreen poll) decide how to degrade and - * are responsible for capturing the error exactly once. + * Check whether the project already has an integration of `kind` connected. + * Requires the `integration:read` scope. Throws an `ApiError` on failure — + * the connect-screen polls decide how to degrade and are responsible for + * capturing the error exactly once. The status matters to them: the + * Self-driving GitHub gate reads a 401 as an expired login, not as an + * integration that is missing. */ -export async function fetchSlackConnected( +async function fetchIntegrationConnected( + kind: string, accessToken: string, projectId: number, baseUrl: string, signal?: AbortSignal, ): Promise { - const response = await axios.get( - `${baseUrl}/api/projects/${projectId}/integrations/`, - { - headers: { - Authorization: `Bearer ${accessToken}`, - 'User-Agent': WIZARD_USER_AGENT, + try { + const response = await axios.get( + `${baseUrl}/api/projects/${projectId}/integrations/`, + { + headers: { + Authorization: `Bearer ${accessToken}`, + 'User-Agent': WIZARD_USER_AGENT, + }, + signal, }, - signal, - }, - ); - const parsed = IntegrationsResponseSchema.safeParse(response.data); - if (!parsed.success) return false; - return parsed.data.results.some((i) => i.kind === 'slack'); + ); + const parsed = IntegrationsResponseSchema.safeParse(response.data); + if (!parsed.success) return false; + return parsed.data.results.some((i) => i.kind === kind); + } catch (error) { + throw handleApiError(error, `check the ${kind} connection`); + } } -/** - * Check whether the project already has a GitHub App integration connected. - * Requires the `integration:read` scope. Throws on failure — callers (the - * SelfDrivingGitHubScreen poll) decide how to degrade and are responsible for - * capturing the error exactly once. - */ -export async function fetchGithubConnected( +export const fetchSlackConnected = ( accessToken: string, projectId: number, baseUrl: string, signal?: AbortSignal, -): Promise { - const response = await axios.get( - `${baseUrl}/api/projects/${projectId}/integrations/`, - { - headers: { - Authorization: `Bearer ${accessToken}`, - 'User-Agent': WIZARD_USER_AGENT, - }, - signal, - }, - ); - const parsed = IntegrationsResponseSchema.safeParse(response.data); - if (!parsed.success) return false; - return parsed.data.results.some((i) => i.kind === 'github'); -} +): Promise => + fetchIntegrationConnected('slack', accessToken, projectId, baseUrl, signal); + +export const fetchGithubConnected = ( + accessToken: string, + projectId: number, + baseUrl: string, + signal?: AbortSignal, +): Promise => + fetchIntegrationConnected('github', accessToken, projectId, baseUrl, signal); export function handleApiError(error: unknown, operation: string): ApiError { if (axios.isAxiosError(error)) { diff --git a/src/lib/session-token.ts b/src/lib/session-token.ts new file mode 100644 index 00000000..81b8a8a0 --- /dev/null +++ b/src/lib/session-token.ts @@ -0,0 +1,84 @@ +/** + * The run's access token, kept fresh. + * + * A leaf module on purpose. Two callers far apart need it: the agent bootstrap + * refreshes right before it mints a gateway token, and the Self-driving GitHub + * gate refreshes before it polls the PostHog API — a screen that runs *before* + * the bootstrap and would otherwise poll with whatever token the earlier + * integration phase left behind. + */ + +import type { Credentials, WizardSession } from '@lib/wizard-session'; +import { refreshAccessToken } from '@utils/oauth'; +import { OAuthError } from '@utils/oauth-errors'; +import { markGrantRevoked } from '@lib/auth-session-state'; +import { analytics } from '@utils/analytics'; +import { getUI } from '@ui'; +import { logToFile } from '@utils/debug'; + +// Below this remaining lifetime a run risks outliving its token; just-minted and 7-day tokens skip. +// Only a second agent run in one invocation can be this old — see self-driving's chained phases. +const REFRESH_WHEN_REMAINING_MS = 50 * 60 * 1000; + +/** + * Grants the token endpoint refuses permanently. A dead grant means the login + * is gone, not that the network blipped, so only these mark the session. + */ +const DEAD_GRANT_CODES = new Set(['invalid_grant', 'invalid_client']); + +/** + * Best-effort refresh: no refresh token or a failed grant keeps the existing + * token. Returns true only when the token was replaced. + * + * `force` skips the remaining-lifetime check. Pass it when the server already + * rejected the token, because then the expiry says nothing useful — a token can + * be revoked long before it runs out. + */ +export async function refreshAccessTokenIfNeeded( + session: WizardSession, + options: { force?: boolean } = {}, +): Promise { + const credentials = session.credentials; + if (!credentials?.refreshToken) return false; + + if (!options.force) { + // No expiry means we cannot tell how much life is left, so leave it alone — + // refreshing every run would spend a rotation for nothing. + if (credentials.expiresAt === undefined) return false; + if (credentials.expiresAt - Date.now() >= REFRESH_WHEN_REMAINING_MS) + return false; + } + + try { + const token = await refreshAccessToken( + credentials.refreshToken, + session.baseUrl, + credentials.oauthClientId, + ); + // Replaced, not mutated: readers hold this object, and a new one keeps the + // store and the (possibly shallow-copied) session explicitly in step. + const refreshed: Credentials = { + ...credentials, + accessToken: token.access_token, + // Rotation: keep the returned refresh token or the old one stops working. + refreshToken: token.refresh_token ?? credentials.refreshToken, + expiresAt: Date.now() + token.expires_in * 1000, + }; + session.credentials = refreshed; + getUI().setAccessToken(refreshed); + return true; + } catch (error) { + // A dead grant is recorded but not thrown: the current token may still have + // minutes of life, and failing here would break runs that would have worked. + // If a 401 does follow, the auth-error screen can finally name the cause. + if (error instanceof OAuthError && DEAD_GRANT_CODES.has(error.code)) { + markGrantRevoked(); + analytics.wizardCapture('auth session expired', { reason: error.code }); + } + logToFile( + '[oauth] token refresh failed, continuing with the existing token:', + error instanceof Error ? error.message : error, + ); + return false; + } +} diff --git a/src/ui/tui/hooks/__tests__/useGithubConnection.test.ts b/src/ui/tui/hooks/__tests__/useGithubConnection.test.ts index 96ba780e..f7128936 100644 --- a/src/ui/tui/hooks/__tests__/useGithubConnection.test.ts +++ b/src/ui/tui/hooks/__tests__/useGithubConnection.test.ts @@ -1,14 +1,69 @@ -import { fetchLoginUrl } from '@ui/tui/hooks/useGithubConnection'; -import { requestDeepLink } from '@utils/provisioning'; +import { + fetchLoginUrl, + pollGithubConnection, +} from '@ui/tui/hooks/useGithubConnection'; +import { ApiError, fetchGithubConnected } from '@lib/api'; +import { refreshAccessTokenIfNeeded } from '@lib/session-token'; +import type { WizardStore } from '@ui/tui/store'; import type { WizardSession, Credentials } from '@lib/wizard-session'; import type { HostResolution } from '@lib/host-resolution'; +import { requestDeepLink } from '@utils/provisioning'; +vi.mock('@lib/api', async () => { + const actual = await vi.importActual('@lib/api'); + return { ApiError: actual.ApiError, fetchGithubConnected: vi.fn() }; +}); +vi.mock('@lib/session-token', () => ({ + refreshAccessTokenIfNeeded: vi.fn().mockResolvedValue(false), +})); +vi.mock('@utils/debug', () => ({ getLogFilePath: () => '/tmp/wizard.log' })); +vi.mock('@utils/analytics', () => ({ + analytics: { wizardCapture: vi.fn(), captureException: vi.fn() }, +})); vi.mock('@utils/provisioning', () => ({ requestDeepLink: vi.fn() })); +const mockedFetch = fetchGithubConnected as Mock; +const mockedRefresh = refreshAccessTokenIfNeeded as Mock; const mockedDeepLink = requestDeepLink as Mock; const host = { appHost: 'https://us.posthog.com' } as HostResolution; +/** Minimal store double — the poll only reads credentials and calls two setters. */ +function storeDouble() { + const session = { + credentials: { + accessToken: 'pha_old', + projectId: 1, + host: { apiHost: 'https://us.posthog.com' }, + }, + githubConnected: null as boolean | null, + }; + return { + session, + setGithubConnected: vi.fn((v: boolean) => { + session.githubConnected = v; + }), + showAuthError: vi.fn(), + } as unknown as WizardStore & { showAuthError: Mock }; +} + +/** Cancels the poll once it has made `ticks` calls, so the loop terminates. */ +function abortAfter(controller: AbortController, ticks: number): void { + let seen = 0; + mockedFetch.mockImplementation(() => { + if (++seen >= ticks) controller.abort(); + return Promise.reject(new ApiError('Authentication failed', 401)); + }); +} + +const run = (store: WizardStore, controller = new AbortController()) => + pollGithubConnection( + store, + { refreshAttempted: false, sawAuthFailure: false, errorReported: false }, + controller.signal, + 0, + ); + function sessionWith(over: Partial): WizardSession { return { signup: false, @@ -17,6 +72,72 @@ function sessionWith(over: Partial): WizardSession { } as WizardSession; } +describe('pollGithubConnection', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockedRefresh.mockResolvedValue(false); + }); + + it('refreshes the token before the first check, since nothing else does before this screen', async () => { + const store = storeDouble(); + mockedFetch.mockResolvedValueOnce(true); + + await run(store); + + expect(mockedRefresh).toHaveBeenCalledWith(store.session); + expect(store.setGithubConnected).toHaveBeenCalledWith(true); + }); + + it('forces one refresh when the server rejects the token, and retries with the new one', async () => { + const store = storeDouble(); + mockedRefresh + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true) + .mockResolvedValue(false); + mockedFetch + .mockRejectedValueOnce(new ApiError('Authentication failed', 401)) + .mockResolvedValueOnce(true); + + await run(store); + + expect(mockedRefresh).toHaveBeenLastCalledWith(store.session, { + force: true, + }); + expect(store.setGithubConnected).toHaveBeenCalledWith(true); + expect(store.showAuthError).not.toHaveBeenCalled(); + }); + + it('names the expired login instead of polling a dead token forever', async () => { + const store = storeDouble(); + const controller = new AbortController(); + abortAfter(controller, 10); + + await run(store, controller); + + expect(store.showAuthError).toHaveBeenCalledWith( + expect.objectContaining({ sessionExpired: true }), + ); + // Gave up on the second rejection rather than running to the abort. + expect(mockedFetch).toHaveBeenCalledTimes(2); + }); + + it('keeps polling through a non-auth failure, which a retry can still resolve', async () => { + const store = storeDouble(); + const controller = new AbortController(); + let seen = 0; + mockedFetch.mockImplementation(() => { + if (++seen >= 3) controller.abort(); + return Promise.reject(new ApiError('Failed to reach PostHog', 503)); + }); + + await run(store, controller); + + expect(store.showAuthError).not.toHaveBeenCalled(); + expect(store.setGithubConnected).toHaveBeenCalledWith(false); + expect(mockedFetch).toHaveBeenCalledTimes(3); + }); +}); + describe('fetchLoginUrl', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/src/ui/tui/hooks/useGithubConnection.ts b/src/ui/tui/hooks/useGithubConnection.ts index 0c68aa42..518aea1e 100644 --- a/src/ui/tui/hooks/useGithubConnection.ts +++ b/src/ui/tui/hooks/useGithubConnection.ts @@ -5,15 +5,23 @@ * Installing the App is a manual browser step, so polling is what flips the * gate once the user comes back. The first tick also resolves the session's * unknown (`null`) state. + * + * A rejected token is the one failure the poll must not absorb. The gate is not + * skippable, so a 401 answered with "not connected" reads to the user as an App + * that never installs, and the only way out is to decline — which ends the run. + * So the token is refreshed before the first tick and once more when the server + * rejects it, and a login that stays rejected raises the auth-error screen. */ -import { useEffect } from 'react'; +import { useEffect, useRef } from 'react'; import type { WizardStore } from '@ui/tui/store'; import type { WizardSession } from '@lib/wizard-session'; -import { fetchGithubConnected } from '@lib/api'; +import { ApiError, fetchGithubConnected } from '@lib/api'; +import { refreshAccessTokenIfNeeded } from '@lib/session-token'; import { requestDeepLink } from '@utils/provisioning'; import { analytics } from '@utils/analytics'; +import { getLogFilePath } from '@utils/debug'; const POLL_INTERVAL_MS = 3000; @@ -29,72 +37,139 @@ export async function fetchLoginUrl( return deepLink ?? `${session.credentials.host.appHost}/login`; } -export function useGithubConnection(store: WizardStore): void { - const credentials = store.session.credentials; - const connected = store.session.githubConnected === true; - - useEffect(() => { - if (!credentials || connected) return; +/** + * Carried across polls because the screen can remount while the same login + * stays dead. Kept in one object, or a stale token would buy an unlimited + * number of refreshes and never reach the give-up path. + */ +export interface GithubPollState { + refreshAttempted: boolean; + sawAuthFailure: boolean; + errorReported: boolean; +} - const controller = new AbortController(); - let stopped = false; - let timer: ReturnType | undefined; - let errorReported = false; +const isAuthFailure = (error: unknown): boolean => + error instanceof ApiError && error.statusCode === 401; - /** A check that came back "not connected" — including a failed one. */ - const settleUnknown = (): void => { - if (store.session.githubConnected === null) { - store.setGithubConnected(false); - } +const sleep = (ms: number, signal: AbortSignal): Promise => + new Promise((resolve) => { + const done = (): void => { + clearTimeout(timer); + signal.removeEventListener('abort', done); + resolve(); }; + const timer = setTimeout(done, ms); + signal.addEventListener('abort', done, { once: true }); + }); - const wait = (): Promise => - new Promise((resolve) => { - timer = setTimeout(resolve, POLL_INTERVAL_MS); - }); +export async function pollGithubConnection( + store: WizardStore, + state: GithubPollState, + signal: AbortSignal, + pollIntervalMs: number, +): Promise { + /** A check that came back "not connected" — including a failed one. */ + const settleUnknown = (): void => { + if (store.session.githubConnected === null) { + store.setGithubConnected(false); + } + }; - void (async () => { - while (!stopped) { - try { - const isConnected = await fetchGithubConnected( - credentials.accessToken, - credentials.projectId, - credentials.host.apiHost, - controller.signal, - ); - if (stopped) return; - if (isConnected) { - // Only a false→true flip means the user installed during this - // screen; true on the first check means they arrived connected. - if (store.session.githubConnected === false) { - analytics.wizardCapture('github connect completed'); - } - store.setGithubConnected(true); - return; - } - settleUnknown(); - } catch (err) { - if (stopped) return; - // Report once, then keep polling. Unlike Slack's nudge, this gate - // can't degrade to a skip — the run cannot proceed until it - // resolves — so a transient API blip must not strand the user. - if (!errorReported) { - errorReported = true; - analytics.captureException( - err instanceof Error ? err : new Error(String(err)), - { step: 'github_connected_check' }, - ); - } - settleUnknown(); + // Nothing refreshes the token before this screen — the agent bootstrap does, + // and it runs after the gate — so a token that aged out during the earlier + // integration phase would be rejected on every tick from here on. + if (await refreshAccessTokenIfNeeded(store.session)) { + state.refreshAttempted = true; + } + + while (!signal.aborted) { + // Read the credentials per tick: a refresh replaces them mid-poll. + const credentials = store.session.credentials; + if (!credentials) return; + + try { + const isConnected = await fetchGithubConnected( + credentials.accessToken, + credentials.projectId, + credentials.host.apiHost, + signal, + ); + if (signal.aborted) return; + if (isConnected) { + // Only a false→true flip means the user installed during this screen; + // true on the first check means they arrived connected. + if (store.session.githubConnected === false) { + analytics.wizardCapture('github connect completed'); } - await wait(); + store.setGithubConnected(true); + return; + } + settleUnknown(); + } catch (err) { + if (signal.aborted) return; + // Report once, then keep polling. Unlike Slack's nudge, this gate can't + // degrade to a skip — the run cannot proceed until it resolves — so a + // transient API blip must not strand the user. + if (!state.errorReported) { + state.errorReported = true; + analytics.captureException( + err instanceof Error ? err : new Error(String(err)), + { step: 'github_connected_check' }, + ); } - })(); + if (isAuthFailure(err)) { + // One forced refresh: the server rejected the token, so its stated + // expiry says nothing and the usual lifetime check would skip it. + if (!state.refreshAttempted) { + state.refreshAttempted = true; + const swapped = await refreshAccessTokenIfNeeded(store.session, { + force: true, + }); + if (swapped) continue; + } + if (state.sawAuthFailure) { + // A second rejection after a fresh token: the login itself is gone. + // Name it, rather than leaving a gate the user can only answer by + // ending their run. + analytics.wizardCapture('github connect auth failed'); + store.showAuthError({ + hasSettingsConflict: false, + sessionExpired: true, + logFilePath: getLogFilePath(), + }); + return; + } + state.sawAuthFailure = true; + } + settleUnknown(); + } + await sleep(pollIntervalMs, signal); + } +} - return () => { - stopped = true; - if (timer) clearTimeout(timer); - controller.abort(); - }; - }, [credentials, connected, store]); +export function useGithubConnection(store: WizardStore): void { + // Presence, not identity: a refresh replaces `credentials`, and the poll + // already re-reads them per tick, so restarting on the swap only duplicates + // the in-flight request. + const hasCredentials = store.session.credentials !== null; + const connected = store.session.githubConnected === true; + const state = useRef({ + refreshAttempted: false, + sawAuthFailure: false, + errorReported: false, + }); + + useEffect(() => { + if (!hasCredentials || connected) return; + + const controller = new AbortController(); + void pollGithubConnection( + store, + state.current, + controller.signal, + POLL_INTERVAL_MS, + ); + + return () => controller.abort(); + }, [hasCredentials, connected, store]); } diff --git a/src/ui/tui/screens/AuthErrorScreen.tsx b/src/ui/tui/screens/AuthErrorScreen.tsx index f07382d8..a3d80271 100644 --- a/src/ui/tui/screens/AuthErrorScreen.tsx +++ b/src/ui/tui/screens/AuthErrorScreen.tsx @@ -1,5 +1,6 @@ /** - * AuthErrorScreen — Shown when the PostHog LLM Gateway returns a 401. + * AuthErrorScreen — Shown when the PostHog LLM Gateway, or the PostHog API a + * gate polls, returns a 401. * * Distinct causes, most specific first: * 0. The OAuth grant is gone — a pre-run refresh already got `invalid_grant` @@ -47,8 +48,8 @@ export const AuthErrorScreen = ({ store }: AuthErrorScreenProps) => { <> - Your PostHog login expired while the wizard was running, so the - LLM Gateway rejected it (401). Nothing on this machine is + Your PostHog login expired while the wizard was running, so + PostHog rejected it (401). Nothing on this machine is misconfigured — the session simply ran out. From 846a9754ae4b5bfcfc56087f8f003d1c9ef2dbcb Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:29:27 +0000 Subject: [PATCH 2/2] fix(self-driving): bound the GitHub integration poll with a timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `/integrations/` poll passed no request timeout, and axios defaults to none. One stalled socket would block the Self-driving GitHub gate — which cannot be skipped — the same way a rejected token did before this PR, leaving decline (which ends the run) as the only exit. Add a 10s timeout. A timeout raises ECONNABORTED with no response, so `handleApiError` yields an `ApiError` with an undefined `statusCode`; the poll reads that as a non-auth blip, settles the unknown state, and retries on the next tick rather than hanging forever. Generated-By: PostHog Desktop Task-Id: ebd30b1e-799d-48b6-9af3-2b52bf4287e2 --- src/lib/api.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/lib/api.ts b/src/lib/api.ts index ecf1da64..4c3876e3 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -258,6 +258,12 @@ async function fetchIntegrationConnected( 'User-Agent': WIZARD_USER_AGENT, }, signal, + // Bound the request so one stalled socket can't wedge the poll. The + // Self-driving GitHub gate can't be skipped, so a hang here would + // strand the user the same way a rejected token does. A timeout raises + // ECONNABORTED with no response, which the poll reads as a non-auth + // blip and retries. + timeout: 10_000, }, ); const parsed = IntegrationsResponseSchema.safeParse(response.data);