diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index d92065af..c72cc28e 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -6,7 +6,7 @@ }, "metadata": { "description": "GitHits plugins for Claude Code - code examples from global open source", - "version": "0.4.0" + "version": "0.4.1" }, "plugins": [ { diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index c982151c..454af6e9 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.4.0", + "version": "0.4.1", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/.plugin/plugin.json b/.plugin/plugin.json index c982151c..454af6e9 100644 --- a/.plugin/plugin.json +++ b/.plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.4.0", + "version": "0.4.1", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/gemini-extension.json b/gemini-extension.json index 38ff3f65..945b27a0 100644 --- a/gemini-extension.json +++ b/gemini-extension.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.4.0", + "version": "0.4.1", "description": "Code examples from global open source for developers and AI assistants.", "mcpServers": { "githits": { diff --git a/package.json b/package.json index 39562f07..8bf15175 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "githits", "description": "CLI companion for GitHits - code examples from global open source for developers and AI assistants", - "version": "0.4.0", + "version": "0.4.1", "type": "module", "files": [ "dist", diff --git a/plugins/claude/.claude-plugin/plugin.json b/plugins/claude/.claude-plugin/plugin.json index c982151c..454af6e9 100644 --- a/plugins/claude/.claude-plugin/plugin.json +++ b/plugins/claude/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.4.0", + "version": "0.4.1", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/src/cli.test.ts b/src/cli.test.ts index bae79960..7b7df88c 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -325,7 +325,9 @@ describe("root CLI preAction", () => { expect(ran).toBe(false); expect(errorSpy.mock.calls.map((call) => call[0])).toEqual([ "Authentication timed out.\n", - "Run `githits login` to try again.", + "Run the same command again to open a fresh sign-in link.", + "If the browser did not open, run `githits login --no-browser` and follow the printed link.", + "If sign-in keeps failing after a retry, run `githits logout` and then run your command again.", ]); errorSpy.mockRestore(); }); diff --git a/src/commands/login.test.ts b/src/commands/login.test.ts index 4b8aaca1..84d53d79 100644 --- a/src/commands/login.test.ts +++ b/src/commands/login.test.ts @@ -462,8 +462,9 @@ describe("loginFlow", () => { expect(writes).toContain(" http://example.com/auth\n"); }); - it("closes callback server when authentication wait fails", async () => { + it("closes callback server and clears fresh client when authentication wait fails", async () => { const close = mock(() => Promise.resolve()); + const authStorage = createMockAuthStorage(); const authService = createMockAuthService({ startCallbackServer: mock(() => Promise.resolve({ @@ -477,7 +478,7 @@ describe("loginFlow", () => { { port: 8080 }, { authService, - authStorage: createMockAuthStorage(), + authStorage, browserService: createMockBrowserService(), mcpUrl, }, @@ -487,6 +488,130 @@ describe("loginFlow", () => { expect(result.status).toBe("failed"); expect(result.message).toBe("callback failed."); expect(close).toHaveBeenCalledTimes(1); + expect(authStorage.clearClient).toHaveBeenCalledWith(mcpUrl); + }); + + it("returns actionable timeout message and clears fresh client", async () => { + const close = mock(() => Promise.resolve()); + const authStorage = createMockAuthStorage(); + const authService = createMockAuthService({ + startCallbackServer: mock(() => + Promise.resolve({ + result: new Promise(() => {}), + close, + }), + ), + }); + + const timeout = setTimeout; + globalThis.setTimeout = ((callback: () => void) => { + if (typeof callback === "function") callback(); + return 0 as unknown as ReturnType; + }) as typeof setTimeout; + + try { + const result = await loginFlow( + { port: 8080 }, + { + authService, + authStorage, + browserService: createMockBrowserService(), + mcpUrl, + }, + silentLoginOutput, + ); + + expect(result.status).toBe("failed"); + expect(result.message).toContain( + "Authentication timed out after 5 minutes", + ); + expect(result.message).toContain("Run the same command again"); + expect(close).toHaveBeenCalledTimes(1); + expect(authStorage.clearClient).toHaveBeenCalledWith(mcpUrl); + } finally { + globalThis.setTimeout = timeout; + } + }); + + it("does not clear reused client when forced login times out", async () => { + const existingToken = createValidTokenData({ + expiresAt: new Date(Date.now() + 3600_000).toISOString(), + }); + const close = mock(() => Promise.resolve()); + const authStorage = createMockAuthStorage({ + loadTokens: mock(() => Promise.resolve(existingToken)), + loadClient: mock(() => + Promise.resolve({ + clientId: "existing-client", + clientSecret: "existing-secret", + redirectUri: "http://127.0.0.1:8080/callback", + registeredAt: "2025-01-01T00:00:00Z", + }), + ), + }); + const authService = createMockAuthService({ + startCallbackServer: mock(() => + Promise.resolve({ + result: Promise.reject(new Error("callback failed")), + close, + }), + ), + }); + + const result = await loginFlow( + { force: true }, + { + authService, + authStorage, + browserService: createMockBrowserService(), + mcpUrl, + }, + silentLoginOutput, + ); + + expect(result.status).toBe("failed"); + expect(authStorage.clearClient).not.toHaveBeenCalledWith(mcpUrl); + }); + + it("does not clear stored client when changed-port login fails before saving", async () => { + const existingToken = createValidTokenData({ + expiresAt: new Date(Date.now() + 3600_000).toISOString(), + }); + const close = mock(() => Promise.resolve()); + const authStorage = createMockAuthStorage({ + loadTokens: mock(() => Promise.resolve(existingToken)), + loadClient: mock(() => + Promise.resolve({ + clientId: "existing-client", + clientSecret: "existing-secret", + redirectUri: "http://127.0.0.1:8080/callback", + registeredAt: "2025-01-01T00:00:00Z", + }), + ), + }); + const authService = createMockAuthService({ + startCallbackServer: mock(() => + Promise.resolve({ + result: Promise.reject(new Error("callback failed")), + close, + }), + ), + }); + + const result = await loginFlow( + { force: true, port: 9090 }, + { + authService, + authStorage, + browserService: createMockBrowserService(), + mcpUrl, + }, + silentLoginOutput, + ); + + expect(result.status).toBe("failed"); + expect(authService.registerClient).toHaveBeenCalled(); + expect(authStorage.clearClient).not.toHaveBeenCalledWith(mcpUrl); }); it("does not open browser when callback server cannot start", async () => { diff --git a/src/commands/login.ts b/src/commands/login.ts index bac2c986..73526a17 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -39,6 +39,8 @@ export const silentLoginOutput: LoginOutput = { }; const TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes +const AUTH_TIMEOUT_MESSAGE = + "Authentication timed out after 5 minutes. The browser link has expired, so it will not work anymore. Run the same command again to try signing in again."; function randomPort(): number { return Math.floor(Math.random() * 2000) + 8000; // 8000-9999 @@ -92,6 +94,7 @@ export async function loginFlow( output: LoginOutput = stdoutLoginOutput, ): Promise { const { authService, authStorage, browserService, mcpUrl } = deps; + const existing = await authStorage.loadTokens(mcpUrl); // Validate port if provided if ( @@ -105,7 +108,6 @@ export async function loginFlow( } // Check if already logged in - const existing = await authStorage.loadTokens(mcpUrl); if (existing && !options.force) { const isExpired = existing.expiresAt && new Date(existing.expiresAt) < new Date(); @@ -132,6 +134,8 @@ export async function loginFlow( // Step 2: Load or register client via DCR let client = await authStorage.loadClient(mcpUrl); + const hadStoredClient = client !== null; + let shouldClearClientOnFailedAttempt = false; let port: number; let redirectUri: string; @@ -153,6 +157,7 @@ export async function loginFlow( redirectUri, registeredAt: new Date().toISOString(), }; + shouldClearClientOnFailedAttempt = !hadStoredClient; } port = options.port; } else { @@ -175,6 +180,7 @@ export async function loginFlow( redirectUri, registeredAt: new Date().toISOString(), }; + shouldClearClientOnFailedAttempt = !hadStoredClient; } // Step 3: Generate PKCE parameters @@ -222,7 +228,7 @@ export async function loginFlow( let timeoutId: ReturnType | undefined; const timeoutPromise = new Promise((_, reject) => { timeoutId = setTimeout( - () => reject(new Error("Authentication timed out")), + () => reject(new Error(AUTH_TIMEOUT_MESSAGE)), TIMEOUT_MS, ); }); @@ -234,15 +240,21 @@ export async function loginFlow( } catch (error) { if (timeoutId) clearTimeout(timeoutId); await callbackServer.close().catch(() => {}); + if (shouldClearClientOnFailedAttempt) { + await authStorage.clearClient(mcpUrl).catch(() => {}); + } const msg = error instanceof Error ? error.message : "Authentication failed"; - return { status: "failed", message: `${msg}.` }; + return { status: "failed", message: ensureTerminalPeriod(msg) }; } // Step 7: Handle callback outcome if (callback.type !== "success") { // Let the callback server finish sending the error page to the browser await new Promise((r) => setTimeout(r, 2000)); + if (shouldClearClientOnFailedAttempt) { + await authStorage.clearClient(mcpUrl).catch(() => {}); + } return { status: "failed", message: callback.message ?? "Authentication callback failed.", @@ -327,6 +339,16 @@ export async function loginAction( function printLoginRecoveryHint(message: string): void { console.log("Recovery steps:"); + if (message.includes("Authentication timed out")) { + console.log(" Run the same command again to open a fresh sign-in link."); + console.log( + " githits login --no-browser # if the browser did not open or you are on SSH", + ); + console.log( + " githits logout && githits login # if sign-in keeps failing after a retry", + ); + return; + } console.log(" githits auth status"); console.log(" githits login --force"); if (message.includes("Cannot persist OAuth credentials")) { @@ -340,6 +362,34 @@ function printLoginRecoveryHint(message: string): void { } } +export function printAutoLoginRecoveryHint(message: string): void { + if (message.includes("Authentication timed out")) { + console.error("Run the same command again to open a fresh sign-in link."); + console.error( + "If the browser did not open, run `githits login --no-browser` and follow the printed link.", + ); + console.error( + "If sign-in keeps failing after a retry, run `githits logout` and then run your command again.", + ); + return; + } + + console.error("Run the same command again to try signing in again."); + console.error( + "Run `githits auth status` to check whether you are signed in.", + ); + if (message.includes("Cannot persist OAuth credentials")) { + console.error( + "If your system keychain is locked or unavailable, unlock it and try again.", + ); + console.error("For CI/automation, set GITHITS_API_TOKEN."); + } +} + +function ensureTerminalPeriod(message: string): string { + return /[.!?]$/.test(message) ? message : `${message}.`; +} + const LOGIN_DESCRIPTION = `Authenticate with your GitHits account via browser. Opens your browser to complete authentication securely using OAuth. diff --git a/src/shared/root-cli-pre-action.ts b/src/shared/root-cli-pre-action.ts index 07ab2a4b..f3ed517e 100644 --- a/src/shared/root-cli-pre-action.ts +++ b/src/shared/root-cli-pre-action.ts @@ -1,8 +1,9 @@ import type { Command } from "commander"; -import type { - LoginDependencies, - LoginFlowResult, - LoginOptions, +import { + type LoginDependencies, + type LoginFlowResult, + type LoginOptions, + printAutoLoginRecoveryHint, } from "../commands/login.js"; import { getCommandPath, maybeAutoLoginBeforeCommand } from "./auto-login.js"; @@ -44,8 +45,9 @@ export function createRootCliPreAction( return; } - console.error(`${authResult.message}\n`); - console.error("Run `githits login` to try again."); + const failureMessage = authResult.message ?? "Authentication failed."; + console.error(`${failureMessage}\n`); + printAutoLoginRecoveryHint(failureMessage); (deps.exit ?? process.exit)(1); }; }