Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
{
Expand Down
2 changes: 1 addition & 1 deletion .claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion .plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion gemini-extension.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
2 changes: 1 addition & 1 deletion plugins/claude/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
4 changes: 3 additions & 1 deletion src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
129 changes: 127 additions & 2 deletions src/commands/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -477,7 +478,7 @@ describe("loginFlow", () => {
{ port: 8080 },
{
authService,
authStorage: createMockAuthStorage(),
authStorage,
browserService: createMockBrowserService(),
mcpUrl,
},
Expand All @@ -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<never>(() => {}),
close,
}),
),
});

const timeout = setTimeout;
globalThis.setTimeout = ((callback: () => void) => {
if (typeof callback === "function") callback();
return 0 as unknown as ReturnType<typeof setTimeout>;
}) 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 () => {
Expand Down
56 changes: 53 additions & 3 deletions src/commands/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -92,6 +94,7 @@ export async function loginFlow(
output: LoginOutput = stdoutLoginOutput,
): Promise<LoginFlowResult> {
const { authService, authStorage, browserService, mcpUrl } = deps;
const existing = await authStorage.loadTokens(mcpUrl);

// Validate port if provided
if (
Expand All @@ -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();
Expand All @@ -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;

Expand All @@ -153,6 +157,7 @@ export async function loginFlow(
redirectUri,
registeredAt: new Date().toISOString(),
};
shouldClearClientOnFailedAttempt = !hadStoredClient;
}
port = options.port;
} else {
Expand All @@ -175,6 +180,7 @@ export async function loginFlow(
redirectUri,
registeredAt: new Date().toISOString(),
};
shouldClearClientOnFailedAttempt = !hadStoredClient;
}

// Step 3: Generate PKCE parameters
Expand Down Expand Up @@ -222,7 +228,7 @@ export async function loginFlow(
let timeoutId: ReturnType<typeof setTimeout> | undefined;
const timeoutPromise = new Promise<never>((_, reject) => {
timeoutId = setTimeout(
() => reject(new Error("Authentication timed out")),
() => reject(new Error(AUTH_TIMEOUT_MESSAGE)),
TIMEOUT_MS,
);
});
Expand All @@ -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.",
Expand Down Expand Up @@ -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")) {
Expand All @@ -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.
Expand Down
14 changes: 8 additions & 6 deletions src/shared/root-cli-pre-action.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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);
};
}
Expand Down
Loading