From c97ea1e72a180289689e97092ead1f0b9acddb0b Mon Sep 17 00:00:00 2001 From: Griffen Fargo <3642037+gfargo@users.noreply.github.com> Date: Thu, 14 May 2026 08:53:29 -0400 Subject: [PATCH] feat(cli): global --repo flag (every subcommand inherits) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #948 which added --repo to coco ui + coco log only. Contributor asked to lift it so commit / review / recap / changelog all inherit too — particularly for scripts and editor / shell wrappers that want to drive coco non-interactively against arbitrary repos. ## What this PR adds - **\`--repo \` (alias \`--cwd\`) declared globally** at the yargs root in src/index.ts (\`y.option('repo', { ..., global: true })\`). Every subcommand inherits it automatically; \`coco --help\` and per-subcommand help both surface it. - **\`repo?: string\` lifted to \`BaseArgvOptions\`** so the TypeScript layer matches — every command's argv type sees the field through normal inheritance. - **Shared \`applyRepoFlag(argv)\` helper** at src/commands/utils/applyRepoFlag.ts. Encapsulates the "resolve path → chdir → getRepo(repoPath)" dance so every handler does it identically. Returns the bound SimpleGit instance for direct use. ## Handlers updated to use the shared helper - commit/handler.ts - recap/handler.ts - changelog/handler.ts - review/handler.ts - ui/handler.ts (replaces #948's inline implementation) - log/handler.ts (replaces #948's inline implementation) The inline / per-command --repo declarations from #948 are removed in favor of the single global declaration. Behavior unchanged; just deduplicated. ## Why chdir + bound git instance + global flag together A repo target isn't a single value — it touches three independent subsystems: 1. **simple-git's baseDir** — controls where git operations run 2. **process.cwd()** — what every \`findUp\` config-discovery walks from 3. **Path resolution** — relative paths in argv (e.g. \`--branch\`) interpret against cwd Setting only one of these would create "coco operates on repo A but reads config from repo B" surprises. \`applyRepoFlag\` does all three in one shot so the rest of each handler can ignore the question. ## Compatibility Users on the \`cd && coco ...\` path see zero change — \`argv.repo === undefined\` makes \`applyRepoFlag\` skip the chdir and call \`getRepo()\` with no baseDir, preserving original behavior exactly. The \`--cwd\` alias is preserved (added in #948 for users who think "change directory" before "target this repo"). ## Tests +4 unit tests for \`applyRepoFlag\`: - returns getRepo() when --repo is omitted - chdir's + returns getRepo(absolutePath) when --repo is set - resolves relative paths to absolute before chdir - treats empty string the same as omitted (defensive) 1681 tests pass, lint + tsc clean. ## Verified end-to-end \`coco log --repo /tmp/scenario --limit 5\` from inside the coco repo shows the scenario's commits. \`coco --help\` lists the global flag under each subcommand's usage block. --- src/commands/changelog/handler.ts | 4 +- src/commands/commit/handler.ts | 4 +- src/commands/log/config.ts | 15 +----- src/commands/log/handler.ts | 17 ++---- src/commands/recap/handler.ts | 4 +- src/commands/review/handler.ts | 4 +- src/commands/types.ts | 15 ++++++ src/commands/ui/config.ts | 17 +----- src/commands/ui/handler.ts | 29 +++------- src/commands/utils/applyRepoFlag.test.ts | 68 ++++++++++++++++++++++++ src/commands/utils/applyRepoFlag.ts | 40 ++++++++++++++ src/index.ts | 13 +++++ 12 files changed, 160 insertions(+), 70 deletions(-) create mode 100644 src/commands/utils/applyRepoFlag.test.ts create mode 100644 src/commands/utils/applyRepoFlag.ts diff --git a/src/commands/changelog/handler.ts b/src/commands/changelog/handler.ts index 6d9f0052..99002642 100644 --- a/src/commands/changelog/handler.ts +++ b/src/commands/changelog/handler.ts @@ -17,8 +17,8 @@ import { getCommitLogCurrentBranch } from '../../lib/simple-git/getCommitLogCurr import { getCommitLogRangeDetails, CommitDetails } from '../../lib/simple-git/getCommitLogRangeDetails' import { getCurrentBranchName } from '../../lib/simple-git/getCurrentBranchName' import { getChangesByCommit } from '../../lib/simple-git/getChangesByCommit' -import { getRepo } from '../../lib/simple-git/getRepo' import { CommandHandler, FileChange } from '../../lib/types' +import { applyRepoFlag } from '../utils/applyRepoFlag' import { generateAndReviewLoop } from '../../lib/ui/generateAndReviewLoop' import { handleResult } from '../../lib/ui/handleResult' import { LOGO, isInteractive } from '../../lib/ui/helpers' @@ -63,8 +63,8 @@ async function processInWaves( } export const handler: CommandHandler = async (argv, logger) => { + const git = applyRepoFlag(argv) const config = loadConfig(argv) - const git = getRepo() const key = getApiKeyForModel(config) const { provider } = getModelAndProviderFromConfig(config) const changelogService = resolveDynamicService(config, 'changelog') diff --git a/src/commands/commit/handler.ts b/src/commands/commit/handler.ts index 432969e3..0ebd794a 100644 --- a/src/commands/commit/handler.ts +++ b/src/commands/commit/handler.ts @@ -16,8 +16,8 @@ import { extractTicketIdFromBranchName } from '../../lib/simple-git/extractTicke import { getChanges } from '../../lib/simple-git/getChanges' import { getCurrentBranchName } from '../../lib/simple-git/getCurrentBranchName' import { getPreviousCommits } from '../../lib/simple-git/getPreviousCommits' -import { getRepo } from '../../lib/simple-git/getRepo' import { CommandHandler, FileChange } from '../../lib/types' +import { applyRepoFlag } from '../utils/applyRepoFlag' import { generateAndReviewLoop } from '../../lib/ui/generateAndReviewLoop' import { handleResult } from '../../lib/ui/handleResult' import { LOGO, SEPERATOR, isInteractive } from '../../lib/ui/helpers' @@ -37,7 +37,7 @@ import { COMMIT_PROMPT, CONVENTIONAL_COMMIT_PROMPT } from './prompt' import { handleCommitSplit, isCommitSplitCommand } from './split' export const handler: CommandHandler = async (argv, logger) => { - const git = getRepo() + const git = applyRepoFlag(argv) const config = loadConfig(argv) const key = getApiKeyForModel(config) const { provider } = getModelAndProviderFromConfig(config) diff --git a/src/commands/log/config.ts b/src/commands/log/config.ts index 88bf68bc..4d5af703 100644 --- a/src/commands/log/config.ts +++ b/src/commands/log/config.ts @@ -15,17 +15,11 @@ export interface LogOptions extends BaseCommandOptions { merges?: boolean noMerges?: boolean path?: string | string[] - /** - * Repository directory to operate against. When set, log reads - * commit data from this path instead of `process.cwd()`. Mirrors - * the `--repo` flag on `coco ui` so scripts / shell wrappers / - * test scenarios can target arbitrary repos without `cd`-ing. - * `--cwd` is an alias. - */ - repo?: string since?: string until?: string view?: LogView + // `repo` (alias `cwd`) is inherited from BaseCommandOptions — declared + // globally at the yargs root so every subcommand sees it. } export type LogArgv = Arguments @@ -81,11 +75,6 @@ export const options = { description: 'Filter commits by changed path', type: 'array', }, - repo: { - description: 'Target a specific repository directory instead of the current working directory.', - type: 'string', - alias: 'cwd', - }, since: { description: 'Show commits more recent than a date', type: 'string', diff --git a/src/commands/log/handler.ts b/src/commands/log/handler.ts index 751e4c88..1432a4c3 100644 --- a/src/commands/log/handler.ts +++ b/src/commands/log/handler.ts @@ -1,26 +1,19 @@ -import * as path from 'node:path' import { CommandHandler } from '../../lib/types' import { Config } from '../../lib/config/types' import { loadConfig } from '../../lib/config/utils/loadConfig' -import { getRepo } from '../../lib/simple-git/getRepo' import { handleResult } from '../../lib/ui/handleResult' import { getCommitDetail, getLogRows } from './data' import { startCocoUiFromLogArgv } from '../ui/handler' +import { applyRepoFlag } from '../utils/applyRepoFlag' import { formatCommitDetail, formatLogJson, formatLogTable } from './render' import { LogArgv } from './config' export const handler: CommandHandler = async (argv) => { - // `--repo ` (alias `--cwd`) lets users target an arbitrary - // repository without `cd`-ing first. Mirrors the same flag on - // `coco ui`. chdir up-front so config + git both resolve against - // the same canonical path. Resolve to absolute to avoid the - // confusion of relative paths interacting with the chdir. - if (argv.repo) { - process.chdir(path.resolve(argv.repo)) - } - + // `--repo ` (alias `--cwd`) — apply the global flag via the + // shared helper. After this returns, `process.cwd()` and the git + // instance are both bound to the targeted repo. + const git = applyRepoFlag(argv) const config = loadConfig(argv) - const git = getRepo(argv.repo ? path.resolve(argv.repo) : undefined) const format = argv.format === 'json' ? 'json' : 'table' if (argv.commit) { diff --git a/src/commands/recap/handler.ts b/src/commands/recap/handler.ts index 725f4dfa..000465ff 100644 --- a/src/commands/recap/handler.ts +++ b/src/commands/recap/handler.ts @@ -12,7 +12,7 @@ import { getPrompt } from '../../lib/langchain/utils/getPrompt' import { getChanges } from '../../lib/simple-git/getChanges' import { getChangesByTimestamp } from '../../lib/simple-git/getChangesByTimestamp' import { getChangesSinceLastTag } from '../../lib/simple-git/getChangesSinceLastTag' -import { getRepo } from '../../lib/simple-git/getRepo' +import { applyRepoFlag } from '../utils/applyRepoFlag' import { getCurrentBranchName } from '../../lib/simple-git/getCurrentBranchName' import { getDiffForBranch } from '../../lib/simple-git/getDiffForBranch' import { CommandHandler } from '../../lib/types' @@ -29,7 +29,7 @@ import { fileChangeParser } from '../../lib/parsers/default' import { createFileChangeParserOptions } from '../../lib/parsers/default/utils/createFileChangeParserOptions' export const handler: CommandHandler = async (argv, logger) => { - const git = getRepo() + const git = applyRepoFlag(argv) const config = loadConfig(argv) const key = getApiKeyForModel(config) const { provider } = getModelAndProviderFromConfig(config) diff --git a/src/commands/review/handler.ts b/src/commands/review/handler.ts index 8010be18..bd88fa7b 100644 --- a/src/commands/review/handler.ts +++ b/src/commands/review/handler.ts @@ -14,9 +14,9 @@ import { fileChangeParser } from '../../lib/parsers/default/index' import { createFileChangeParserOptions } from '../../lib/parsers/default/utils/createFileChangeParserOptions' import { getChanges } from '../../lib/simple-git/getChanges' import { getDiffForBranch } from '../../lib/simple-git/getDiffForBranch' -import { getRepo } from '../../lib/simple-git/getRepo' import { getCurrentBranchName } from '../../lib/simple-git/getCurrentBranchName' import { CommandHandler } from '../../lib/types' +import { applyRepoFlag } from '../utils/applyRepoFlag' import { generateAndReviewLoop } from '../../lib/ui/generateAndReviewLoop' import { isInteractive, LOGO, severityColor } from '../../lib/ui/helpers' import { TaskList } from '../../lib/ui/TaskList' @@ -35,7 +35,7 @@ const ReviewFeedbackResponseSchema = z.preprocess( ) export const handler: CommandHandler = async (argv, logger) => { - const git = getRepo() + const git = applyRepoFlag(argv) const config = loadConfig(argv) const key = getApiKeyForModel(config) const { provider } = getModelAndProviderFromConfig(config) diff --git a/src/commands/types.ts b/src/commands/types.ts index 26d7ba80..88c8a0af 100644 --- a/src/commands/types.ts +++ b/src/commands/types.ts @@ -5,6 +5,21 @@ export interface BaseArgvOptions { verbose: boolean version: boolean help: boolean + /** + * Repository directory to operate against. When set, the command + * chdir's to this path before loading config / opening a git + * instance, so every downstream read (config lookup, simple-git + * baseDir, commitlint discovery, etc.) sees the same root. + * + * `--cwd` is an alias. + * + * Inherited by every coco subcommand so scripts / editor wrappers + * / scenario tests can target arbitrary repos without `cd`-ing + * first. Defaults to `process.cwd()` when omitted (unchanged + * behavior for users who launch via the regular `cd && coco ...` + * path). + */ + repo?: string } export interface BaseCommandOptions extends BaseArgvOptions {} diff --git a/src/commands/ui/config.ts b/src/commands/ui/config.ts index 85fbecf4..39147201 100644 --- a/src/commands/ui/config.ts +++ b/src/commands/ui/config.ts @@ -10,18 +10,10 @@ export interface UiOptions extends BaseCommandOptions { branch?: string limit?: number path?: string | string[] - /** - * Repository directory to operate against. When set, the workstation - * binds its git instance + cwd reads to this path instead of - * `process.cwd()`. Lets users / scripts / scenarios launch coco - * against arbitrary repos without a leading `cd`. - * - * `--cwd` is exposed as an alias since users reaching for this - * flag often think "change directory" before "target this repo." - */ - repo?: string theme?: LogInkThemePreset view?: UiView + // `repo` (alias `cwd`) is inherited from BaseCommandOptions — declared + // globally at the yargs root so every subcommand sees it. } export type UiArgv = Arguments @@ -53,11 +45,6 @@ export const options = { description: 'Filter history by changed path', type: 'array', }, - repo: { - description: 'Target a specific repository directory instead of the current working directory.', - type: 'string', - alias: 'cwd', - }, theme: { description: 'TUI theme preset', choices: ['default', 'monochrome', 'catppuccin', 'gruvbox'], diff --git a/src/commands/ui/handler.ts b/src/commands/ui/handler.ts index a529169f..6a593ec9 100644 --- a/src/commands/ui/handler.ts +++ b/src/commands/ui/handler.ts @@ -1,4 +1,3 @@ -import * as path from 'node:path' import { Arguments } from 'yargs' import { SimpleGit } from 'simple-git' import { CommandHandler } from '../../lib/types' @@ -8,6 +7,7 @@ import { getRepo } from '../../lib/simple-git/getRepo' import { LogArgv, LogOptions } from '../log/config' import { GitLogRow, getLogRows } from '../log/data' import { startInkInteractiveLog } from '../log/inkRuntime' +import { applyRepoFlag } from '../utils/applyRepoFlag' import { readCachedCommits, writeCachedCommits } from '../../workstation/chrome/overviewCache' import { LogInkThemeConfig } from '../../workstation/chrome/theme' import { UiArgv } from './config' @@ -100,29 +100,14 @@ export async function startCocoUiFromLogArgv( } export async function startCocoUi(argv: UiArgv): Promise { - // `--repo ` (alias `--cwd`) lets users target an arbitrary - // repository without `cd`-ing first. Resolve to absolute up-front - // so: - // 1. process.chdir gets a stable path (relative paths against - // the original cwd would surprise the user if any later code - // reads cwd after the chdir). - // 2. The disk cache key (rooted at repoPath) is canonical. - // 3. simple-git's baseDir is unambiguous. - // When the flag is omitted, fall back to process.cwd() — original - // behavior, no surprise for users who launch via `cd && coco ui`. - const repoPath = argv.repo ? path.resolve(argv.repo) : process.cwd() - - // chdir BEFORE loadConfig so .coco.config.json lookup walks up - // from the targeted repo, not from wherever the user ran coco - // from. Many config-resolution paths (lib/utils/findUp, etc.) - // read process.cwd() — keeping them honest means doing the chdir - // up-front rather than passing a path everywhere. - if (argv.repo) { - process.chdir(repoPath) - } + // `--repo ` (alias `--cwd`) — apply the global flag via the + // shared helper. After this returns, `process.cwd()` and the git + // instance are both bound to the targeted repo, so loadConfig + // walks the right tree and downstream reads stay consistent. + const git = applyRepoFlag(argv) + const repoPath = process.cwd() const config = loadConfig(argv) - const git = getRepo(repoPath) const logArgv = createLogArgvFromUiArgv(argv) // Same three-stage boot as startCocoUiFromLogArgv — mount with diff --git a/src/commands/utils/applyRepoFlag.test.ts b/src/commands/utils/applyRepoFlag.test.ts new file mode 100644 index 00000000..bdc5d4f2 --- /dev/null +++ b/src/commands/utils/applyRepoFlag.test.ts @@ -0,0 +1,68 @@ +import { applyRepoFlag } from './applyRepoFlag' + +jest.mock('../../lib/simple-git/getRepo', () => ({ + getRepo: jest.fn(), +})) + +import { getRepo } from '../../lib/simple-git/getRepo' + +const mockedGetRepo = getRepo as jest.MockedFunction + +describe('applyRepoFlag', () => { + let originalCwd: string + let chdirSpy: jest.SpyInstance + + beforeEach(() => { + originalCwd = process.cwd() + chdirSpy = jest.spyOn(process, 'chdir').mockImplementation(() => undefined) + mockedGetRepo.mockReset() + mockedGetRepo.mockReturnValue({} as ReturnType) + }) + + afterEach(() => { + chdirSpy.mockRestore() + // Reset cwd in case the real chdir slipped through. + if (process.cwd() !== originalCwd) { + try { process.chdir(originalCwd) } catch { /* noop */ } + } + }) + + it('returns getRepo() (no baseDir) when --repo is omitted', () => { + applyRepoFlag({ repo: undefined }) + + expect(mockedGetRepo).toHaveBeenCalledWith() + expect(chdirSpy).not.toHaveBeenCalled() + }) + + it('returns getRepo(absolutePath) and chdirs when --repo is set', () => { + applyRepoFlag({ repo: '/tmp/some-repo' }) + + expect(chdirSpy).toHaveBeenCalledTimes(1) + expect(chdirSpy).toHaveBeenCalledWith('/tmp/some-repo') + expect(mockedGetRepo).toHaveBeenCalledWith('/tmp/some-repo') + }) + + it('resolves a relative --repo path to absolute before chdir', () => { + // Whatever cwd we're running in, a './fixture' relative path + // should be resolved against it. We assert only that the chdir + // target is absolute — testing the resolved value end-to-end + // ties this to the runner's cwd, which is brittle. + applyRepoFlag({ repo: './fixture' }) + + expect(chdirSpy).toHaveBeenCalledTimes(1) + const target = chdirSpy.mock.calls[0][0] as string + expect(target.startsWith('/')).toBe(true) + expect(target.endsWith('/fixture')).toBe(true) + expect(mockedGetRepo).toHaveBeenCalledWith(target) + }) + + it('does not chdir when --repo is an empty string', () => { + // Empty string is treated the same as omitted — defensive guard + // since yargs can produce `''` for flags that were set without + // a value in some edge cases. + applyRepoFlag({ repo: '' }) + + expect(mockedGetRepo).toHaveBeenCalledWith() + expect(chdirSpy).not.toHaveBeenCalled() + }) +}) diff --git a/src/commands/utils/applyRepoFlag.ts b/src/commands/utils/applyRepoFlag.ts new file mode 100644 index 00000000..512efee2 --- /dev/null +++ b/src/commands/utils/applyRepoFlag.ts @@ -0,0 +1,40 @@ +import * as path from 'node:path' +import { getRepo } from '../../lib/simple-git/getRepo' +import { BaseArgvOptions } from '../types' + +/** + * Apply the global `--repo ` (alias `--cwd`) flag for any + * command handler. Returns the bound simple-git instance. + * + * Behavior: + * - When `argv.repo` is set, resolves the path to absolute, + * `process.chdir`s up-front, and returns `getRepo(repoPath)`. + * - When omitted, returns `getRepo()` (defaults to cwd) — original + * behavior, no surprise for users on the `cd && coco ...` path. + * + * Why chdir up-front: + * Many config / discovery paths (loadConfig's findUp for + * `.coco.config.json`, commitlint config detection, etc.) read + * `process.cwd()` directly. If we only changed simple-git's + * baseDir without chdir-ing, those would resolve against the + * original cwd — leading to "coco is reading this repo but + * loading config from somewhere else" surprises. + * + * Returns the SimpleGit instance so callers can use it directly: + * + * ```ts + * export const handler: CommandHandler = async (argv) => { + * const git = applyRepoFlag(argv) + * const config = loadConfig(argv) + * // ... rest of handler uses git + config + * } + * ``` + */ +export function applyRepoFlag(argv: Pick): ReturnType { + if (!argv.repo) { + return getRepo() + } + const repoPath = path.resolve(argv.repo) + process.chdir(repoPath) + return getRepo(repoPath) +} diff --git a/src/index.ts b/src/index.ts index 16acb0bd..09c47f7b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,6 +26,19 @@ const y = yargs() y.scriptName('coco') +// Global `--repo ` (alias `--cwd`) — every subcommand inherits +// it. The shared `applyRepoFlag` helper handlers call up-front +// chdir's to this directory + binds the simple-git instance so every +// downstream read (config lookup, simple-git baseDir, commitlint +// discovery) sees the same root. Lets users / scripts / editor +// integrations target arbitrary repos without `cd`-ing first. +y.option('repo', { + type: 'string', + alias: 'cwd', + description: 'Target a specific repository directory instead of the current working directory.', + global: true, +}) + y.command( [commit.command, '$0'], commit.desc,