diff --git a/packages/coding-agent/src/extensibility/custom-commands/bundled/review/index.ts b/packages/coding-agent/src/extensibility/custom-commands/bundled/review/index.ts index 33392738a..d66734a18 100644 --- a/packages/coding-agent/src/extensibility/custom-commands/bundled/review/index.ts +++ b/packages/coding-agent/src/extensibility/custom-commands/bundled/review/index.ts @@ -449,7 +449,7 @@ async function getGitStatus(api: CustomCommandAPI): Promise { } async function getUncommittedReviewDiff(api: CustomCommandAPI): Promise { - if (await jj.isRepository(api.cwd)) { + if (await jj.repo.is(api.cwd)) { return { diffText: await jj.diff(api.cwd), diffInstruction: JJ_UNCOMMITTED_DIFF_INSTRUCTION, diff --git a/packages/coding-agent/src/utils/jj.ts b/packages/coding-agent/src/utils/jj.ts index d1f52ab91..ee5478326 100644 --- a/packages/coding-agent/src/utils/jj.ts +++ b/packages/coding-agent/src/utils/jj.ts @@ -1,7 +1,12 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; +import { $which } from "@oh-my-pi/pi-utils"; import { LRUCache } from "lru-cache/raw"; +// ════════════════════════════════════════════════════════════════════════════ +// Types +// ════════════════════════════════════════════════════════════════════════════ + /** Result from a completed `jj` subprocess invocation. */ export interface JjCommandResult { /** Process exit code reported by `jj`. */ @@ -12,10 +17,20 @@ export interface JjCommandResult { stderr: string; } -/** Options for `jj diff --git` invocations. */ +/** Resolved Jujutsu workspace metadata. */ +export interface JjRepository { + /** Root directory containing the `.jj` workspace metadata. */ + repoRoot: string; + /** Path to the workspace store directory used to verify a real JJ checkout. */ + storeDir: string; +} + +/** Options for `jj diff` invocations. */ export interface DiffOptions { /** Optional file paths to restrict the diff with `-- `. */ readonly files?: readonly string[]; + /** Return only changed file names instead of Git-format diff text. */ + readonly nameOnly?: boolean; /** Optional abort signal passed to the spawned `jj` process. */ readonly signal?: AbortSignal; } @@ -24,6 +39,10 @@ interface CommandOptions { readonly signal?: AbortSignal; } +// ════════════════════════════════════════════════════════════════════════════ +// Error +// ════════════════════════════════════════════════════════════════════════════ + /** Error thrown when a checked `jj` command exits non-zero. */ export class JjCommandError extends Error { /** Arguments passed after the common `jj --no-pager --color=never` prefix. */ @@ -40,6 +59,17 @@ export class JjCommandError extends Error { } } + +// ════════════════════════════════════════════════════════════════════════════ +// Internal: Core execution +// ════════════════════════════════════════════════════════════════════════════ + +function ensureAvailable(): void { + if (!$which("jj")) { + throw new Error("jj is not installed."); + } +} + function formatCommandFailure( args: readonly string[], result: Pick, @@ -79,6 +109,7 @@ async function runChecked( args: readonly string[], options: CommandOptions = {}, ): Promise { + ensureAvailable(); const result = await jj(cwd, args, options); if (result.exitCode !== 0) { throw new JjCommandError(args, result); @@ -86,14 +117,34 @@ async function runChecked( return result; } +async function runText(cwd: string, args: readonly string[], options: CommandOptions = {}): Promise { + return (await runChecked(cwd, args, options)).stdout; +} + +function splitLines(text: string): string[] { + return text + .split("\n") + .map(line => line.trim()) + .filter(Boolean); +} + function buildDiffArgs(options: DiffOptions): string[] { - const args = ["diff", "--git"]; + const args = ["diff"]; + args.push(options.nameOnly ? "--name-only" : "--git"); if (options.files?.length) args.push("--", ...options.files); return args; } +// ════════════════════════════════════════════════════════════════════════════ +// Internal: Repository resolution +// ════════════════════════════════════════════════════════════════════════════ + +interface WorkspaceRootCacheEntry { + readonly root?: string; +} + const WORKSPACE_ROOT_CACHE_MAX_ENTRIES = 256; -const workspaceRootCache = new LRUCache({ max: WORKSPACE_ROOT_CACHE_MAX_ENTRIES }); +const workspaceRootCache = new LRUCache({ max: WORKSPACE_ROOT_CACHE_MAX_ENTRIES }); async function hasJjWorkspaceMetadata(dir: string): Promise { try { @@ -110,35 +161,70 @@ function parentOf(dir: string): string | undefined { async function findWorkspaceRoot(cwd: string): Promise { const key = path.resolve(cwd); - if (workspaceRootCache.has(key)) return workspaceRootCache.get(key) ?? undefined; + if (workspaceRootCache.has(key)) return workspaceRootCache.get(key)?.root; for (let dir: string | undefined = key; dir; dir = parentOf(dir)) { if (await hasJjWorkspaceMetadata(dir)) { - workspaceRootCache.set(key, dir); + workspaceRootCache.set(key, { root: dir }); return dir; } } - workspaceRootCache.set(key, null); + workspaceRootCache.set(key, {}); return undefined; } -/** Clear cached workspace roots. Intended for tests that mutate JJ metadata under an existing path. */ -export function clearWorkspaceRootCache(): void { - workspaceRootCache.clear(); +function repositoryFromRoot(root: string): JjRepository { + return { + repoRoot: root, + storeDir: path.join(root, ".jj", "repo", "store"), + }; } -/** Resolve the current Jujutsu workspace root, or `undefined` when `cwd` is not in a JJ repository. */ -export async function workspaceRoot(cwd: string): Promise { - return findWorkspaceRoot(cwd); -} - -/** Return whether `cwd` is inside a Jujutsu repository. */ -export async function isRepository(cwd: string): Promise { - return (await workspaceRoot(cwd)) !== undefined; -} +// ════════════════════════════════════════════════════════════════════════════ +// API: diff +// ════════════════════════════════════════════════════════════════════════════ /** Run `jj diff --git` for the current workspace commit and return the raw Git-format diff text. */ -export async function diff(cwd: string, options: DiffOptions = {}): Promise { - return (await runChecked(cwd, buildDiffArgs(options), { signal: options.signal })).stdout; -} +export const diff = Object.assign( + async function diff(cwd: string, options: DiffOptions = {}): Promise { + return runText(cwd, buildDiffArgs(options), { signal: options.signal }); + }, + { + /** List changed file paths. */ + async changedFiles( + cwd: string, + options: Pick = {}, + ): Promise { + return splitLines(await diff(cwd, { ...options, nameOnly: true })); + }, + }, +); + +// ════════════════════════════════════════════════════════════════════════════ +// API: repo +// ════════════════════════════════════════════════════════════════════════════ + +export const repo = { + /** Clear cached workspace roots. Intended for tests that mutate JJ metadata under an existing path. */ + clearRootCache(): void { + workspaceRootCache.clear(); + }, + + /** Resolve the current Jujutsu workspace root, or `null` when `cwd` is not in a JJ repository. */ + async root(cwd: string): Promise { + return (await findWorkspaceRoot(cwd)) ?? null; + }, + + /** Full Jujutsu workspace metadata. */ + async resolve(cwd: string): Promise { + const root = await repo.root(cwd); + return root ? repositoryFromRoot(root) : null; + }, + + /** Check whether `cwd` is inside a Jujutsu repository. */ + async is(cwd: string): Promise { + return (await repo.root(cwd)) !== null; + }, +}; + diff --git a/packages/coding-agent/test/extensibility/custom-commands/review.test.ts b/packages/coding-agent/test/extensibility/custom-commands/review.test.ts index 7b485b024..076412a38 100644 --- a/packages/coding-agent/test/extensibility/custom-commands/review.test.ts +++ b/packages/coding-agent/test/extensibility/custom-commands/review.test.ts @@ -120,7 +120,7 @@ describe("ReviewCommand", () => { it("uses JJ diff for uncommitted review prompts", async () => { const dir = await createTempDir(); - const jjRepoSpy = spyOn(jj, "isRepository").mockResolvedValue(true); + const jjRepoSpy = spyOn(jj.repo, "is").mockResolvedValue(true); const jjDiffSpy = spyOn(jj, "diff").mockResolvedValue(SAMPLE_JJ_DIFF); const gitStatusSpy = spyOn(git, "status").mockResolvedValue(" M src/workspace.ts\n"); const gitDiffSpy = spyOn(git, "diff").mockResolvedValue(""); @@ -150,7 +150,7 @@ describe("ReviewCommand", () => { it("includes reviewer task orchestration for single-agent diff reviews", async () => { const dir = await createTempDir(); - const jjRepoSpy = spyOn(jj, "isRepository").mockResolvedValue(false); + const jjRepoSpy = spyOn(jj.repo, "is").mockResolvedValue(false); const gitStatusSpy = spyOn(git, "status").mockResolvedValue(" M review-target.ts\n"); const gitDiffSpy = spyOn(git, "diff").mockResolvedValue(`diff --git a/review-target.ts b/review-target.ts --- a/review-target.ts @@ -181,7 +181,7 @@ describe("ReviewCommand", () => { it("includes JJ diff context for custom review prompts", async () => { const dir = await createTempDir(); - const jjRepoSpy = spyOn(jj, "isRepository").mockResolvedValue(true); + const jjRepoSpy = spyOn(jj.repo, "is").mockResolvedValue(true); const jjDiffSpy = spyOn(jj, "diff").mockResolvedValue(SAMPLE_JJ_DIFF); const gitStatusSpy = spyOn(git, "status").mockResolvedValue(""); const gitDiffSpy = spyOn(git, "diff").mockResolvedValue(""); diff --git a/packages/coding-agent/test/utils/jj.test.ts b/packages/coding-agent/test/utils/jj.test.ts index d6f8916ae..9870eae97 100644 --- a/packages/coding-agent/test/utils/jj.test.ts +++ b/packages/coding-agent/test/utils/jj.test.ts @@ -2,13 +2,13 @@ import { afterEach, describe, expect, it } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; -import { clearWorkspaceRootCache, isRepository, workspaceRoot } from "../../src/utils/jj"; +import * as jj from "../../src/utils/jj"; describe("jj workspace detection", () => { let tmpDir: string | undefined; afterEach(async () => { - clearWorkspaceRootCache(); + jj.repo.clearRootCache(); if (tmpDir) { await fs.rm(tmpDir, { recursive: true, force: true }); tmpDir = undefined; @@ -26,8 +26,8 @@ describe("jj workspace detection", () => { await fs.mkdir(path.join(dir, ".jj", "repo", "store"), { recursive: true }); await fs.mkdir(nested, { recursive: true }); - expect(await workspaceRoot(nested)).toBe(dir); - expect(await isRepository(nested)).toBe(true); + expect(await jj.repo.root(nested)).toBe(dir); + expect(await jj.repo.is(nested)).toBe(true); }); it("caches each requested cwd to its resolved workspace root", async () => { @@ -36,18 +36,18 @@ describe("jj workspace detection", () => { await fs.mkdir(path.join(dir, ".jj", "repo", "store"), { recursive: true }); await fs.mkdir(nested, { recursive: true }); - expect(await workspaceRoot(nested)).toBe(dir); + expect(await jj.repo.root(nested)).toBe(dir); await fs.rm(path.join(dir, ".jj"), { recursive: true, force: true }); - expect(await workspaceRoot(nested)).toBe(dir); - expect(await workspaceRoot(path.join(dir, "src"))).toBeUndefined(); + expect(await jj.repo.root(nested)).toBe(dir); + expect(await jj.repo.root(path.join(dir, "src"))).toBeNull(); }); it("does not treat a bare .jj directory as a workspace", async () => { const dir = await createTempDir(); await fs.mkdir(path.join(dir, ".jj"), { recursive: true }); - expect(await workspaceRoot(dir)).toBeUndefined(); - expect(await isRepository(dir)).toBe(false); + expect(await jj.repo.root(dir)).toBeNull(); + expect(await jj.repo.is(dir)).toBe(false); }); });