diff --git a/docs/implementation/signup-flow.md b/docs/implementation/signup-flow.md new file mode 100644 index 00000000..470db979 --- /dev/null +++ b/docs/implementation/signup-flow.md @@ -0,0 +1,159 @@ +# Streamlined Signup Flow + +## Purpose + +This document captures the implementation checklist for a smoother first-run +authentication experience in the CLI. The goal is to let an unauthenticated +user invoke an auth-required command, complete browser sign-in or sign-up, and +then continue the original command without re-running it manually. + +## Background + +The underlying OAuth pieces already exist: + +- `src/commands/login.ts` orchestrates the PKCE + browser flow. +- `src/services/auth-service.ts` serves the callback success page and exchanges + the auth code for tokens. +- `src/container.ts` resolves token state at startup. +- `src/shared/require-auth.ts` currently blocks individual command actions when + no valid token is present. + +Today, auth-required commands fail fast and tell the user to run `githits +login`. That works, but it adds friction to first-run adoption and makes the +recommended one-shot entry point less compelling. + +## Target Flow + +1. User runs an auth-required interactive command such as + `npx -y githits@latest init`. +2. CLI detects that no valid token is available. +3. CLI opens the browser to the sign-in / sign-up page. +4. User completes the GitHub-backed auth flow. +5. Browser shows the authenticated success page. +6. CLI resumes and executes the originally requested command. + +## Scope Boundaries + +The feature should not be phrased as "every command except help". Some commands +are informational, recovery-oriented, or host-driven, and auto-login would be +surprising or actively harmful. + +### Commands that should stay exempt + +| Command surface | Why exempt | +|---|---| +| `help`, `--help`, `-h` | Never trigger side effects for help output. | +| `login` | Already the explicit auth entry point. | +| `logout` | Must work even when auth is broken or absent. | +| `auth status` | Informational; should explain missing auth, not launch browser. | +| `mcp` / `mcp start` | Agent hosts and stdio launches should not unexpectedly open a browser. | + +### Phase 1 candidates + +These are human-invoked commands where auto-login is a clear UX improvement: + +- `init` +- `init --skip-login` remains an explicit opt-out +- `example` +- `languages` +- `feedback` +- `search` / `search-status` +- `code files` / `code read` / `code grep` +- `docs list` / `docs read` +- `pkg info` / `pkg vulns` / `pkg deps` / `pkg changelog` + +### Package/source registration note + +Package/source commands (`search`, `search-status`, `code`, `docs`, and `pkg`) +register through the normal package-service URL configuration path. The signup +bootstrap can therefore run for their interactive invocations before the command +action reaches the final `requireAuth()` check. + +## Decision Criteria + +Use these rules when implementing the feature: + +- Keep `requireAuth()` as the final action-level invariant even after adding + auto-login at the CLI boundary. +- Do not corrupt machine-readable output. Commands using `--json` or piping + must not mix login progress logs into stdout. +- Avoid launching the browser from non-interactive or host-driven contexts. +- Reuse `loginFlow()` rather than duplicating OAuth orchestration. +- Keep startup registration behavior explicit for package/source command groups: + registration follows package-service URL configuration. + +## Implementation Checklist + +### Phase 1: interactive auto-login bootstrap + +- [x] Add a shared auth-bootstrap helper that checks current auth state and, + when appropriate, runs `loginFlow()` before dispatching the command. +- [x] Put the bootstrap decision at the CLI boundary in `src/cli.ts` rather + than duplicating it in every command handler. +- [x] Add an explicit command policy helper that identifies exempt commands and + auto-login-eligible commands. +- [x] Gate bootstrap on interactivity: no browser launch for non-TTY execution. +- [x] Keep automatic login progress off stdout when the invoked command + requests `--json`. +- [x] Keep the current `requireAuth()` checks in command actions as a final + invariant. +- [x] Wire the new bootstrap flow for `init` and interactive user-facing + commands that require authentication. + +### Phase 1.1: login output hygiene + +- [x] Refactor `src/commands/login.ts` so login progress can be reported via a + small output interface instead of always writing directly to stdout. +- [x] Ensure automatic login can either write to stderr or stay quiet when the + original command expects pipe-friendly stdout. +- [x] Preserve the current human-friendly standalone `githits login` output. + +### Phase 1.2: docs and product guidance + +- [x] Document the signup bootstrap behavior and command scope in this + implementation note. + +### Phase 1.3: tests + +- [x] Add CLI-level tests for exempt commands never triggering auto-login. +- [x] Add CLI-level tests for eligible commands triggering login when no valid + token is available. +- [x] Add tests for login failure preserving a clear error path. +- [x] Add tests proving `--json` commands do not receive login chatter on + stdout. +- [ ] Keep existing command-level `AuthRequiredError` tests to verify the final + invariant still holds. + +### Phase 2: package/source command registration + +- [x] Decide whether `search`, `code`, `docs`, and `pkg` should trigger browser + login on first use. Decision: yes for interactive CLI use, while keeping + action-level auth checks as the final invariant. +- [x] Keep startup registration in `src/commands/search.ts`, + `src/commands/code/index.ts`, and `src/commands/pkg/index.ts` aligned + with the package-service URL configuration path. +- [x] Re-check help output and discoverability with the package/source command + registration policy in place. +- [x] Add tests covering URL-configured registration behavior. + +## Open Decisions + +- Should `mcp start` remain strictly manual-auth, or should there be a separate + device-code or non-browser bootstrap for host-driven setups? + +## Key Reference Files + +| File | Why it matters | +|---|---| +| `src/cli.ts` | Best hook point for a single auto-login bootstrap policy. | +| `src/commands/login.ts` | Reusable login flow and current standalone login output. | +| `src/shared/require-auth.ts` | Final invariant for command actions after bootstrap. | +| `src/container.ts` | Startup token resolution and auth-state snapshot. | +| `src/commands/init/init.ts` | Advertised onboarding command; `--skip-login` remains the explicit opt-out. | +| `src/commands/search.ts` | Top-level package/source registration and auth-required action. | +| `src/commands/code/index.ts` | `code` group package-service URL registration. | +| `src/commands/docs/index.ts` | `docs` group package-service URL registration. | +| `src/commands/pkg/index.ts` | `pkg` group package-service URL registration. | +| `src/commands/example.ts` | Auth-required command action still protected by `requireAuth()`. | +| `src/commands/languages.ts` | Auth-required command action still protected by `requireAuth()`. | +| `src/commands/feedback.ts` | Auth-required command action still protected by `requireAuth()`. | \ No newline at end of file diff --git a/src/cli.test.ts b/src/cli.test.ts index 4e31ba58..5444552f 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -1,5 +1,20 @@ -import { afterEach, describe, expect, it } from "bun:test"; +import { afterEach, describe, expect, it, mock, spyOn } from "bun:test"; import { Command } from "commander"; +import { + registerCodeCommandGroup, + registerExampleCommand, + registerFeedbackCommand, + registerLanguagesCommand, + registerPkgCommandGroup, + registerUnifiedSearchCommands, +} from "./commands/index.js"; +import type { LoginDependencies } from "./commands/login.js"; +import { + createMockAuthService, + createMockAuthStorage, + createMockBrowserService, +} from "./services/test-helpers.js"; +import { createRootCliPreAction } from "./shared/root-cli-pre-action.js"; describe("--no-color flag", () => { const origNoColor = process.env.NO_COLOR; @@ -56,3 +71,331 @@ describe("--no-color flag", () => { expect(captured).toBeUndefined(); }); }); + +function createLoginDeps( + overrides: Partial = {}, +): LoginDependencies & { hasValidToken: boolean } { + return { + authService: createMockAuthService(), + authStorage: createMockAuthStorage(), + browserService: createMockBrowserService(), + mcpUrl: "https://mcp.githits.com", + hasValidToken: false, + ...overrides, + }; +} + +function createProgramWithRootPreAction( + dependencies: Parameters[0], +): Command { + const program = new Command(); + program + .name("githits") + .option("--no-color", "Disable colored output") + .hook( + "preAction", + createRootCliPreAction({ + stdinIsTTY: true, + stdoutIsTTY: true, + ...dependencies, + }), + ); + return program; +} + +async function createProgramForHelpSurface(options: { + codeNavigationUrl?: string; +}): Promise { + const program = new Command(); + program.name("githits"); + + registerExampleCommand(program); + registerLanguagesCommand(program); + registerFeedbackCommand(program); + await registerUnifiedSearchCommands(program, options); + await registerCodeCommandGroup(program, options); + await registerPkgCommandGroup(program, options); + + return program; +} + +describe("root CLI preAction", () => { + it("does not trigger auto-login for exempt commands", async () => { + const createContainer = mock(() => Promise.resolve(createLoginDeps())); + const loginFlow = mock(() => + Promise.resolve({ + status: "success" as const, + message: "Logged in successfully.", + }), + ); + const program = createProgramWithRootPreAction({ + createContainer, + loginFlow, + }); + + let ran = false; + program.command("logout").action(() => { + ran = true; + }); + + await program.parseAsync(["node", "githits", "logout"]); + + expect(ran).toBe(true); + expect(createContainer).not.toHaveBeenCalled(); + expect(loginFlow).not.toHaveBeenCalled(); + }); + + it("triggers auto-login for eligible commands before the action runs", async () => { + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const container = createLoginDeps({ hasValidToken: false }); + const createContainer = mock(() => Promise.resolve(container)); + const loginFlow = mock(() => + Promise.resolve({ + status: "success" as const, + message: "Logged in successfully.", + }), + ); + const program = createProgramWithRootPreAction({ + createContainer, + loginFlow, + }); + + let ran = false; + program.command("example").action(() => { + ran = true; + }); + + await program.parseAsync(["node", "githits", "example"]); + + expect(ran).toBe(true); + expect(createContainer).toHaveBeenCalledTimes(1); + expect(loginFlow).toHaveBeenCalledWith({}, container); + expect(errorSpy.mock.calls.map((call) => call[0])).toEqual([ + "Authentication complete. Running example search...", + ]); + errorSpy.mockRestore(); + }); + + it("prints the languages continuation message after successful auto-login", async () => { + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const container = createLoginDeps({ hasValidToken: false }); + const createContainer = mock(() => Promise.resolve(container)); + const loginFlow = mock(() => + Promise.resolve({ + status: "success" as const, + message: "Logged in successfully.", + }), + ); + const program = createProgramWithRootPreAction({ + createContainer, + loginFlow, + }); + + let ran = false; + program.command("languages").action(() => { + ran = true; + }); + + await program.parseAsync(["node", "githits", "languages"]); + + expect(ran).toBe(true); + expect(errorSpy.mock.calls.map((call) => call[0])).toEqual([ + "Authentication complete. Loading supported languages...", + ]); + errorSpy.mockRestore(); + }); + + it("prints the feedback continuation message after successful auto-login", async () => { + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const container = createLoginDeps({ hasValidToken: false }); + const createContainer = mock(() => Promise.resolve(container)); + const loginFlow = mock(() => + Promise.resolve({ + status: "success" as const, + message: "Logged in successfully.", + }), + ); + const program = createProgramWithRootPreAction({ + createContainer, + loginFlow, + }); + + let ran = false; + program.command("feedback").action(() => { + ran = true; + }); + + await program.parseAsync(["node", "githits", "feedback"]); + + expect(ran).toBe(true); + expect(errorSpy.mock.calls.map((call) => call[0])).toEqual([ + "Authentication complete. Submitting feedback...", + ]); + errorSpy.mockRestore(); + }); + + it("triggers auto-login for init before setup runs", async () => { + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const container = createLoginDeps({ hasValidToken: false }); + const createContainer = mock(() => Promise.resolve(container)); + const loginFlow = mock(() => + Promise.resolve({ + status: "success" as const, + message: "Logged in successfully.", + }), + ); + const program = createProgramWithRootPreAction({ + createContainer, + loginFlow, + }); + + let ran = false; + program.command("init").action(() => { + ran = true; + }); + + await program.parseAsync(["node", "githits", "init"]); + + expect(ran).toBe(true); + expect(createContainer).toHaveBeenCalledTimes(1); + expect(loginFlow).toHaveBeenCalledWith({}, container); + expect(errorSpy.mock.calls.map((call) => call[0])).toEqual([ + "Authentication complete. Continuing setup...", + ]); + errorSpy.mockRestore(); + }); + + it("triggers auto-login for nested package/source commands", async () => { + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const container = createLoginDeps({ hasValidToken: false }); + const createContainer = mock(() => Promise.resolve(container)); + const loginFlow = mock(() => + Promise.resolve({ + status: "success" as const, + message: "Logged in successfully.", + }), + ); + const program = createProgramWithRootPreAction({ + createContainer, + loginFlow, + }); + + let ran = false; + const pkgCommand = program.command("pkg"); + pkgCommand.command("info").action(() => { + ran = true; + }); + + await program.parseAsync(["node", "githits", "pkg", "info"]); + + expect(ran).toBe(true); + expect(createContainer).toHaveBeenCalledTimes(1); + expect(loginFlow).toHaveBeenCalledWith({}, container); + expect(errorSpy.mock.calls.map((call) => call[0])).toEqual([ + "Authentication complete. Running command...", + ]); + errorSpy.mockRestore(); + }); + + it("preserves a clear failure path when auto-login fails", async () => { + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const createContainer = mock(() => Promise.resolve(createLoginDeps())); + const loginFlow = mock(() => + Promise.resolve({ + status: "failed" as const, + message: "Authentication timed out.", + }), + ); + const exit = mock(() => { + throw new Error("process.exit"); + }); + const program = createProgramWithRootPreAction({ + createContainer, + loginFlow, + exit, + }); + + let ran = false; + program.command("example").action(() => { + ran = true; + }); + + await expect( + program.parseAsync(["node", "githits", "example"]), + ).rejects.toThrow("process.exit"); + + expect(ran).toBe(false); + expect(errorSpy.mock.calls.map((call) => call[0])).toEqual([ + "Authentication timed out.\n", + "Run `githits login` to try again.", + ]); + errorSpy.mockRestore(); + }); + + it("keeps stdout clean for interactive --json auto-login flows", async () => { + const logSpy = spyOn(console, "log").mockImplementation(() => {}); + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const createContainer = mock(() => Promise.resolve(createLoginDeps())); + const loginFlow = mock(() => { + console.error("Opening browser..."); + console.error("Waiting for authentication...\n"); + return Promise.resolve({ + status: "success" as const, + message: "Logged in successfully.", + }); + }); + const program = createProgramWithRootPreAction({ + createContainer, + loginFlow, + }); + + program + .command("example") + .option("--json", "Output JSON") + .action((options: { json?: boolean }) => { + console.log(JSON.stringify({ ok: options.json === true })); + }); + + await program.parseAsync(["node", "githits", "example", "--json"]); + + expect(logSpy.mock.calls.map((call) => call[0])).toEqual([ + JSON.stringify({ ok: true }), + ]); + expect(errorSpy.mock.calls.map((call) => call[0])).toEqual([ + "Opening browser...", + "Waiting for authentication...\n", + "Authentication complete. Running example search...", + ]); + logSpy.mockRestore(); + errorSpy.mockRestore(); + }); +}); + +describe("CLI help surface", () => { + it("keeps package/source commands out of root help when URL is disabled", async () => { + const program = await createProgramForHelpSurface({ + codeNavigationUrl: "", + }); + + const help = program.helpInformation(); + + expect(help).toMatch(/^\s{2}example\b/m); + expect(help).toMatch(/^\s{2}languages\b/m); + expect(help).toMatch(/^\s{2}feedback\b/m); + expect(help).not.toMatch(/^\s{2}search\b/m); + expect(help).not.toMatch(/^\s{2}code\b/m); + expect(help).not.toMatch(/^\s{2}pkg\b/m); + }); + + it("shows package/source commands in root help when URL is configured", async () => { + const program = await createProgramForHelpSurface({ + codeNavigationUrl: "https://pkgseer.dev", + }); + + const help = program.helpInformation(); + + expect(help).toMatch(/^\s{2}search\b/m); + expect(help).toMatch(/^\s{2}code\b/m); + expect(help).toMatch(/^\s{2}pkg\b/m); + }); +}); diff --git a/src/cli.ts b/src/cli.ts index 07c60f83..3cffc139 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -22,11 +22,14 @@ import { registerPkgCommandGroup, registerUnifiedSearchCommands, } from "./commands/index.js"; +import { loginFlow, stderrLoginOutput } from "./commands/login.js"; +import { createContainer } from "./container.js"; import { FileSystemServiceImpl, NpmRegistryUpdateCheckService, } from "./services/index.js"; import { + createRootCliPreAction, endTelemetrySpan, flushTelemetry, isTelemetryEnabled, @@ -74,21 +77,24 @@ if (isTelemetryEnabled()) { }); } +const rootCliPreAction = createRootCliPreAction({ + createContainer, + loginFlow: (options, deps) => loginFlow(options, deps, stderrLoginOutput), +}); + program .name("githits") .description("Code examples from global open source for your AI assistant") .version(version) .option("--no-color", "Disable colored output") - .hook("preAction", (thisCommand, actionCommand) => { - if (thisCommand.opts().color === false) { - process.env.NO_COLOR = "1"; - } - + .hook("preAction", async (thisCommand, actionCommand) => { const command = actionCommand ?? thisCommand; commandSpans.set( command, startTelemetrySpan(getTelemetryCommandName(command)), ); + + await rootCliPreAction(thisCommand, actionCommand); }) .hook("postAction", (_thisCommand, actionCommand) => { endTelemetrySpan(commandSpans.get(actionCommand)); diff --git a/src/commands/login.test.ts b/src/commands/login.test.ts index 6d5c63da..11f24236 100644 --- a/src/commands/login.test.ts +++ b/src/commands/login.test.ts @@ -5,7 +5,7 @@ import { createMockBrowserService, createValidTokenData, } from "../services/test-helpers.js"; -import { loginAction, loginFlow } from "./login.js"; +import { loginAction, loginFlow, silentLoginOutput } from "./login.js"; describe("loginAction", () => { const mcpUrl = "https://mcp.githits.com"; @@ -411,6 +411,32 @@ describe("loginFlow", () => { consoleSpy.mockRestore(); }); + it("routes progress through the provided reporter instead of stdout", async () => { + const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); + const writes: string[] = []; + + const result = await loginFlow( + { port: 8080 }, + { + authService: createMockAuthService(), + authStorage: createMockAuthStorage(), + browserService: createMockBrowserService(), + mcpUrl, + }, + { + write: (message: string) => { + writes.push(message); + }, + }, + ); + + expect(result.status).toBe("success"); + expect(writes).toContain("Discovering OAuth endpoints..."); + expect(writes).toContain("Opening browser..."); + expect(consoleSpy).not.toHaveBeenCalled(); + consoleSpy.mockRestore(); + }); + it("returns already_authenticated when valid tokens exist", async () => { const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); @@ -436,6 +462,24 @@ describe("loginFlow", () => { consoleSpy.mockRestore(); }); + it("stays quiet when the silent reporter is used", async () => { + const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); + + await loginFlow( + { port: 8080 }, + { + authService: createMockAuthService(), + authStorage: createMockAuthStorage(), + browserService: createMockBrowserService(), + mcpUrl, + }, + silentLoginOutput, + ); + + expect(consoleSpy).not.toHaveBeenCalled(); + consoleSpy.mockRestore(); + }); + it("returns failed on invalid port", async () => { const result = await loginFlow( { port: -1 }, diff --git a/src/commands/login.ts b/src/commands/login.ts index 693afc8e..41d9a8b7 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -18,6 +18,26 @@ export interface LoginFlowResult { message: string; } +export interface LoginOutput { + write(message: string): void; +} + +export const stdoutLoginOutput: LoginOutput = { + write: (message: string) => { + console.log(message); + }, +}; + +export const stderrLoginOutput: LoginOutput = { + write: (message: string) => { + console.error(message); + }, +}; + +export const silentLoginOutput: LoginOutput = { + write: () => {}, +}; + const TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes function randomPort(): number { @@ -69,6 +89,7 @@ async function preflightAuthPersistence( export async function loginFlow( options: LoginOptions, deps: LoginDependencies, + output: LoginOutput = stdoutLoginOutput, ): Promise { const { authService, authStorage, browserService, mcpUrl } = deps; @@ -91,9 +112,9 @@ export async function loginFlow( if (!isExpired) { return { status: "already_authenticated", message: "Already logged in." }; } - console.log("Token expired. Starting new login...\n"); + output.write("Token expired. Starting new login...\n"); } else if (existing && options.force) { - console.log("Re-authenticating (--force flag)...\n"); + output.write("Re-authenticating (--force flag)...\n"); } // If tokens were cleared (expired+refreshFailed or never existed) but a stale @@ -106,7 +127,7 @@ export async function loginFlow( if (persistenceError) return persistenceError; // Step 1: Discover OAuth endpoints - console.log("Discovering OAuth endpoints..."); + output.write("Discovering OAuth endpoints..."); const metadata = await authService.discoverEndpoints(mcpUrl); // Step 2: Load or register client via DCR @@ -121,7 +142,7 @@ export async function loginFlow( redirectUri = `http://127.0.0.1:${options.port}/callback`; if (redirectUri !== client.redirectUri) { // Port changed - need to re-register - console.log("Registering CLI client with new port..."); + output.write("Registering CLI client with new port..."); const registration = await authService.registerClient({ registrationEndpoint: metadata.registrationEndpoint, redirectUri, @@ -143,7 +164,7 @@ export async function loginFlow( } else { port = options.port ?? randomPort(); redirectUri = `http://127.0.0.1:${port}/callback`; - console.log("Registering CLI client..."); + output.write("Registering CLI client..."); const registration = await authService.registerClient({ registrationEndpoint: metadata.registrationEndpoint, redirectUri, @@ -173,14 +194,14 @@ export async function loginFlow( // Step 6: Open browser or show URL if (options.browser === false) { - console.log("Open this URL in your browser:\n"); - console.log(` ${authUrl}\n`); + output.write("Open this URL in your browser:\n"); + output.write(` ${authUrl}\n`); } else { - console.log("Opening browser..."); + output.write("Opening browser..."); await browserService.open(authUrl); } - console.log("Waiting for authentication...\n"); + output.write("Waiting for authentication...\n"); // Wait for callback with timeout let timeoutId: ReturnType | undefined; @@ -266,7 +287,7 @@ export async function loginAction( options: LoginOptions, deps: LoginDependencies, ): Promise { - const result = await loginFlow(options, deps); + const result = await loginFlow(options, deps, stdoutLoginOutput); if (result.status === "already_authenticated") { console.log("Already logged in.\n"); diff --git a/src/shared/auto-login.test.ts b/src/shared/auto-login.test.ts new file mode 100644 index 00000000..8b1e87d9 --- /dev/null +++ b/src/shared/auto-login.test.ts @@ -0,0 +1,250 @@ +import { describe, expect, it, mock } from "bun:test"; +import type { LoginDependencies } from "../commands/login.js"; +import { + createMockAuthService, + createMockAuthStorage, + createMockBrowserService, +} from "../services/test-helpers.js"; +import { + type CommandLike, + getCommandPath, + isAutoLoginEligibleCommand, + maybeAutoLoginBeforeCommand, +} from "./auto-login.js"; + +function createCommand( + path: string[], + options: Record = {}, +): CommandLike { + let current: CommandLike = { + name: () => "githits", + opts: () => ({}), + parent: null, + }; + + for (let index = 0; index < path.length; index += 1) { + const segment = path[index]!; + const isLeaf = index === path.length - 1; + const parent = current; + current = { + name: () => segment, + opts: () => (isLeaf ? options : {}), + parent, + }; + } + + return current; +} + +function createLoginDeps( + overrides: Partial = {}, +): LoginDependencies & { hasValidToken: boolean } { + return { + authService: createMockAuthService(), + authStorage: createMockAuthStorage(), + browserService: createMockBrowserService(), + mcpUrl: "https://mcp.githits.com", + hasValidToken: false, + ...overrides, + }; +} + +describe("getCommandPath", () => { + it("returns the leaf command path without the root program", () => { + expect(getCommandPath(createCommand(["auth", "status"]))).toEqual([ + "auth", + "status", + ]); + }); +}); + +describe("isAutoLoginEligibleCommand", () => { + it("allows interactive example invocations", () => { + expect( + isAutoLoginEligibleCommand(createCommand(["example"]), { + stdinIsTTY: true, + stdoutIsTTY: true, + }), + ).toBe(true); + }); + + it("allows interactive init invocations for first-run onboarding", () => { + expect( + isAutoLoginEligibleCommand(createCommand(["init"]), { + stdinIsTTY: true, + stdoutIsTTY: true, + }), + ).toBe(true); + }); + + it("honors init --skip-login", () => { + expect( + isAutoLoginEligibleCommand(createCommand(["init"], { skipLogin: true }), { + stdinIsTTY: true, + stdoutIsTTY: true, + }), + ).toBe(false); + }); + + it("allows interactive package/source command invocations", () => { + for (const path of [ + ["search"], + ["search-status"], + ["code", "files"], + ["code", "read"], + ["code", "grep"], + ["docs", "list"], + ["docs", "read"], + ["pkg", "info"], + ["pkg", "vulns"], + ["pkg", "deps"], + ["pkg", "changelog"], + ]) { + expect( + isAutoLoginEligibleCommand(createCommand(path), { + stdinIsTTY: true, + stdoutIsTTY: true, + }), + ).toBe(true); + } + }); + + it("allows interactive --json invocations once login output is redirected", () => { + expect( + isAutoLoginEligibleCommand(createCommand(["example"], { json: true }), { + stdinIsTTY: true, + stdoutIsTTY: true, + }), + ).toBe(true); + }); + + it("skips eligible commands when stdout is not interactive", () => { + expect( + isAutoLoginEligibleCommand(createCommand(["languages"]), { + stdinIsTTY: true, + stdoutIsTTY: false, + }), + ).toBe(false); + }); + + it("skips exempt commands", () => { + for (const path of [ + ["login"], + ["logout"], + ["auth", "status"], + ["mcp"], + ["mcp", "start"], + ]) { + expect( + isAutoLoginEligibleCommand(createCommand(path), { + stdinIsTTY: true, + stdoutIsTTY: true, + }), + ).toBe(false); + } + }); +}); + +describe("maybeAutoLoginBeforeCommand", () => { + it("skips bootstrap for ineligible commands", async () => { + const createContainer = mock(() => Promise.resolve(createLoginDeps())); + const login = mock(() => + Promise.resolve({ + status: "success" as const, + message: "Logged in successfully.", + }), + ); + + const result = await maybeAutoLoginBeforeCommand( + createCommand(["logout"]), + { + createContainer, + loginFlow: login, + stdinIsTTY: true, + stdoutIsTTY: true, + }, + ); + + expect(result).toEqual({ status: "skipped" }); + expect(createContainer).not.toHaveBeenCalled(); + expect(login).not.toHaveBeenCalled(); + }); + + it("skips login when the container already has a valid token", async () => { + const createContainer = mock(() => + Promise.resolve(createLoginDeps({ hasValidToken: true })), + ); + const login = mock(() => + Promise.resolve({ + status: "success" as const, + message: "Logged in successfully.", + }), + ); + + const result = await maybeAutoLoginBeforeCommand( + createCommand(["example"]), + { + createContainer, + loginFlow: login, + stdinIsTTY: true, + stdoutIsTTY: true, + }, + ); + + expect(result).toEqual({ status: "already-authenticated" }); + expect(createContainer).toHaveBeenCalledTimes(1); + expect(login).not.toHaveBeenCalled(); + }); + + it("runs login for eligible unauthenticated commands", async () => { + const container = createLoginDeps({ hasValidToken: false }); + const createContainer = mock(() => Promise.resolve(container)); + const login = mock(() => + Promise.resolve({ + status: "success" as const, + message: "Logged in successfully. Token expires in 1 hour.", + }), + ); + + const result = await maybeAutoLoginBeforeCommand( + createCommand(["feedback"]), + { + createContainer, + loginFlow: login, + stdinIsTTY: true, + stdoutIsTTY: true, + }, + ); + + expect(result).toEqual({ + status: "authenticated", + message: "Logged in successfully. Token expires in 1 hour.", + }); + expect(login).toHaveBeenCalledWith({}, container); + }); + + it("returns the login failure without continuing", async () => { + const createContainer = mock(() => Promise.resolve(createLoginDeps())); + const login = mock(() => + Promise.resolve({ + status: "failed" as const, + message: "Authentication timed out.", + }), + ); + + const result = await maybeAutoLoginBeforeCommand( + createCommand(["languages"]), + { + createContainer, + loginFlow: login, + stdinIsTTY: true, + stdoutIsTTY: true, + }, + ); + + expect(result).toEqual({ + status: "failed", + message: "Authentication timed out.", + }); + }); +}); diff --git a/src/shared/auto-login.ts b/src/shared/auto-login.ts new file mode 100644 index 00000000..5ee7119b --- /dev/null +++ b/src/shared/auto-login.ts @@ -0,0 +1,118 @@ +import type { + LoginDependencies, + LoginFlowResult, + LoginOptions, +} from "../commands/login.js"; + +const AUTO_LOGIN_ELIGIBLE_COMMANDS = new Set([ + "init", + "example", + "languages", + "feedback", + "search", + "search-status", + "code files", + "code read", + "code grep", + "docs list", + "docs read", + "pkg info", + "pkg vulns", + "pkg deps", + "pkg changelog", +]); + +export interface CommandLike { + name(): string; + opts(): Record; + parent?: CommandLike | null; +} + +export interface AutoLoginBootstrapDependencies { + createContainer: () => Promise< + LoginDependencies & { hasValidToken: boolean } + >; + loginFlow: ( + options: LoginOptions, + deps: LoginDependencies, + ) => Promise; + stdinIsTTY?: boolean; + stdoutIsTTY?: boolean; +} + +export interface AutoLoginBootstrapResult { + status: "skipped" | "already-authenticated" | "authenticated" | "failed"; + message?: string; +} + +interface AutoLoginRuntime { + stdinIsTTY: boolean; + stdoutIsTTY: boolean; +} + +export function getCommandPath(command: CommandLike): string[] { + const names: string[] = []; + let current: CommandLike | null | undefined = command; + + while (current) { + const name = current.name(); + if (name && name !== "githits") { + names.unshift(name); + } + current = current.parent ?? null; + } + + return names; +} + +export function isAutoLoginEligibleCommand( + command: CommandLike, + runtime: AutoLoginRuntime = { + stdinIsTTY: Boolean(process.stdin.isTTY), + stdoutIsTTY: Boolean(process.stdout.isTTY), + }, +): boolean { + const commandPath = getCommandPath(command).join(" "); + if (commandPath === "init" && command.opts().skipLogin === true) { + return false; + } + + if (!AUTO_LOGIN_ELIGIBLE_COMMANDS.has(commandPath)) { + return false; + } + + if (!runtime.stdinIsTTY || !runtime.stdoutIsTTY) { + return false; + } + + return true; +} + +export async function maybeAutoLoginBeforeCommand( + command: CommandLike, + deps: AutoLoginBootstrapDependencies, +): Promise { + if ( + !isAutoLoginEligibleCommand(command, { + stdinIsTTY: deps.stdinIsTTY ?? Boolean(process.stdin.isTTY), + stdoutIsTTY: deps.stdoutIsTTY ?? Boolean(process.stdout.isTTY), + }) + ) { + return { status: "skipped" }; + } + + const container = await deps.createContainer(); + if (container.hasValidToken) { + return { status: "already-authenticated" }; + } + + const result = await deps.loginFlow({}, container); + switch (result.status) { + case "success": + return { status: "authenticated", message: result.message }; + case "already_authenticated": + return { status: "already-authenticated", message: result.message }; + case "failed": + return { status: "failed", message: result.message }; + } +} diff --git a/src/shared/index.ts b/src/shared/index.ts index 78b38115..045785f3 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -169,6 +169,10 @@ export { } from "./read-package-doc-response.js"; export { renderReadPackageDocText } from "./read-package-doc-text.js"; export { AuthRequiredError, requireAuth } from "./require-auth.js"; +export { + createRootCliPreAction, + type RootCliPreActionDependencies, +} from "./root-cli-pre-action.js"; export { endTelemetrySpan, flushTelemetry, diff --git a/src/shared/root-cli-pre-action.ts b/src/shared/root-cli-pre-action.ts new file mode 100644 index 00000000..c9a90983 --- /dev/null +++ b/src/shared/root-cli-pre-action.ts @@ -0,0 +1,78 @@ +import type { Command } from "commander"; +import type { + LoginDependencies, + LoginFlowResult, + LoginOptions, +} from "../commands/login.js"; +import { getCommandPath, maybeAutoLoginBeforeCommand } from "./auto-login.js"; + +export interface RootCliPreActionDependencies { + createContainer: () => Promise< + LoginDependencies & { hasValidToken: boolean } + >; + loginFlow: ( + options: LoginOptions, + deps: LoginDependencies, + ) => Promise; + stdinIsTTY?: boolean; + stdoutIsTTY?: boolean; + exit?: (code: number) => void; +} + +export function createRootCliPreAction( + deps: RootCliPreActionDependencies, +): (thisCommand: Command, actionCommand?: Command) => Promise { + return async (thisCommand: Command, actionCommand?: Command) => { + if (thisCommand.opts().color === false) { + process.env.NO_COLOR = "1"; + } + + const command = actionCommand ?? thisCommand; + const authResult = await maybeAutoLoginBeforeCommand(command, { + ...deps, + stdinIsTTY: deps.stdinIsTTY, + stdoutIsTTY: deps.stdoutIsTTY, + }); + if (authResult.status === "authenticated") { + const continuationMessage = getPostLoginContinuationMessage(command); + if (continuationMessage) { + console.error(continuationMessage); + } + } + + if (authResult.status !== "failed") { + return; + } + + console.error(`${authResult.message}\n`); + console.error("Run `githits login` to try again."); + (deps.exit ?? process.exit)(1); + }; +} + +function getPostLoginContinuationMessage(command: Command): string | undefined { + switch (getCommandPath(command).join(" ")) { + case "init": + return "Authentication complete. Continuing setup..."; + case "example": + return "Authentication complete. Running example search..."; + case "languages": + return "Authentication complete. Loading supported languages..."; + case "feedback": + return "Authentication complete. Submitting feedback..."; + case "search": + case "search-status": + case "code files": + case "code read": + case "code grep": + case "docs list": + case "docs read": + case "pkg info": + case "pkg vulns": + case "pkg deps": + case "pkg changelog": + return "Authentication complete. Running command..."; + default: + return undefined; + } +}