diff --git a/README.md b/README.md index 571e226..5603478 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,7 @@ This organizes your local changes into reviewable chapters and opens a browser U | Flag | Description | |------|-------------| | `--base ` | Base ref to diff against (default: auto-detect main/master) | +| `--compare ` | Compare ref to diff against `--base` | | `--ref ` | Diff scope: `work` (staged + unstaged + untracked), `staged`, or `unstaged` (default: auto-detect) | Examples: @@ -62,6 +63,11 @@ Examples: # Diff against a specific branch /stage-chapters --base feature-a + +# Compare two branches +/stage-chapters main feature +/stage-chapters main..feature +/stage-chapters --base main --compare feature ``` Stage CLI diff --git a/packages/cli/src/__tests__/git.test.ts b/packages/cli/src/__tests__/git.test.ts index 5c341b9..f6d537f 100644 --- a/packages/cli/src/__tests__/git.test.ts +++ b/packages/cli/src/__tests__/git.test.ts @@ -1,5 +1,65 @@ -import { describe, expect, it } from "vitest"; -import { parseRepoName } from "../git.js"; +import { execFileSync } from "node:child_process"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { parseRepoName, resolveScope } from "../git.js"; +import { SCOPE_KIND, WORKING_TREE_REF } from "../schema.js"; + +let tmpDir: string; +let originalCwd: string; + +beforeEach(async () => { + originalCwd = process.cwd(); + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-cli-git-")); +}); + +afterEach(async () => { + process.chdir(originalCwd); + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +function git(...args: string[]): string { + return execFileSync("git", args, { + cwd: tmpDir, + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + env: { ...process.env, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null" }, + }); +} + +async function writeFile(filePath: string, contents: string): Promise { + await fs.writeFile(path.join(tmpDir, filePath), contents); +} + +async function initDivergedRepo(): Promise<{ + commonSha: string; + mainSha: string; + featureSha: string; +}> { + git("init", "--initial-branch=main"); + git("config", "user.email", "test@example.com"); + git("config", "user.name", "Test"); + git("config", "commit.gpgsign", "false"); + + await writeFile("file.txt", "common\n"); + git("add", "file.txt"); + git("commit", "-m", "common"); + const commonSha = git("rev-parse", "HEAD").trim(); + + git("checkout", "-b", "feature"); + await writeFile("file.txt", "common\nfeature\n"); + git("commit", "-am", "feature change"); + const featureSha = git("rev-parse", "HEAD").trim(); + + git("checkout", "main"); + await writeFile("file.txt", "common\nmain\n"); + git("commit", "-am", "main change"); + const mainSha = git("rev-parse", "HEAD").trim(); + + process.chdir(tmpDir); + return { commonSha, mainSha, featureSha }; +} describe("parseRepoName", () => { const FALLBACK_ROOT = "/Users/dev/conductor/workspaces/stage-cli/monterrey-v3"; @@ -37,3 +97,83 @@ describe("parseRepoName", () => { expect(parseRepoName(".git", FALLBACK_ROOT)).toBe("monterrey-v3"); }); }); + +describe("resolveScope", () => { + it("compares two positional refs through their merge base", async () => { + const { commonSha, mainSha, featureSha } = await initDivergedRepo(); + + const result = resolveScope({ refs: ["main", "feature"] }); + + expect(mainSha).not.toBe(featureSha); + expect(result.scope.kind).toBe(SCOPE_KIND.COMMITTED); + expect(result.scope.baseSha).toBe(commonSha); + expect(result.scope.mergeBaseSha).toBe(commonSha); + expect(result.scope.headSha).toBe(featureSha); + expect(result.rawDiff).toContain("+feature"); + expect(result.rawDiff).not.toContain("+main"); + }); + + it("compares range refs through their merge base", async () => { + const { commonSha, featureSha } = await initDivergedRepo(); + + const result = resolveScope({ refs: ["main..feature"] }); + + expect(result.scope.kind).toBe(SCOPE_KIND.COMMITTED); + expect(result.scope.baseSha).toBe(commonSha); + expect(result.scope.headSha).toBe(featureSha); + expect(result.rawDiff).toContain("+feature"); + }); + + it("defaults a missing left range ref to HEAD", async () => { + const { commonSha, featureSha } = await initDivergedRepo(); + + const result = resolveScope({ refs: ["..feature"] }); + + expect(result.scope.kind).toBe(SCOPE_KIND.COMMITTED); + expect(result.scope.baseSha).toBe(commonSha); + expect(result.scope.headSha).toBe(featureSha); + expect(result.rawDiff).toContain("+feature"); + }); + + it("defaults a missing right range ref to HEAD", async () => { + const { commonSha, mainSha } = await initDivergedRepo(); + + const result = resolveScope({ refs: ["feature.."] }); + + expect(result.scope.kind).toBe(SCOPE_KIND.COMMITTED); + expect(result.scope.baseSha).toBe(commonSha); + expect(result.scope.headSha).toBe(mainSha); + expect(result.rawDiff).toContain("+main"); + }); + + it("compares --base and --compare through their merge base", async () => { + const { commonSha, featureSha } = await initDivergedRepo(); + + const result = resolveScope({ base: "main", compare: "feature" }); + + expect(result.scope.kind).toBe(SCOPE_KIND.COMMITTED); + expect(result.scope.baseSha).toBe(commonSha); + expect(result.scope.headSha).toBe(featureSha); + }); + + it("prefers a valid branch over a positional working-tree keyword", async () => { + const { mainSha } = await initDivergedRepo(); + git("checkout", "-b", "staged", "main"); + await writeFile("file.txt", "common\nbranch named staged\n"); + git("commit", "-am", "branch named staged"); + git("checkout", "main"); + await writeFile("file.txt", "common\nmain\nstaged index change\n"); + git("add", "file.txt"); + await writeFile("file.txt", "common\nmain\nstaged index change\nunstaged change\n"); + + const result = resolveScope({ refs: ["staged"] }); + + expect(result.scope.kind).toBe(SCOPE_KIND.WORKING_TREE); + if (result.scope.kind !== SCOPE_KIND.WORKING_TREE) { + throw new Error("Expected working-tree scope"); + } + expect(result.scope.ref).toBe(WORKING_TREE_REF.WORK); + expect(result.scope.headSha).toBe(mainSha); + expect(result.rawDiff).toContain("+unstaged change"); + }); +}); diff --git a/packages/cli/src/git.ts b/packages/cli/src/git.ts index 17add88..3b0f7a4 100644 --- a/packages/cli/src/git.ts +++ b/packages/cli/src/git.ts @@ -183,8 +183,8 @@ export function getUntrackedDiff(files: string[]): string { return patches.join("\n"); } -export function getCommitMessages(mergeBase: string): string { - return execFileSync("git", ["log", "--oneline", `${mergeBase}..HEAD`], { +export function getCommitMessages(mergeBase: string, head: string): string { + return execFileSync("git", ["log", "--oneline", `${mergeBase}..${head}`], { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], }).trim(); @@ -204,6 +204,23 @@ export interface ResolvedScope { rawDiff: string; } +export interface ResolveScopeOptions { + base?: string; + compare?: string; + refs?: string[]; + workingTreeRef?: WorkingTreeRef; +} + +const RANGE_SEPARATOR = { + TWO_DOT: "..", + THREE_DOT: "...", +} as const; + +interface RefRange { + left: string; + right: string; +} + function workingTreeDiffArgs(ref: WorkingTreeRef, mergeBaseSha: string): string[] { switch (ref) { case WORKING_TREE_REF.UNSTAGED: @@ -233,12 +250,87 @@ function buildWorkingTreeDiff(ref: WorkingTreeRef, mergeBaseSha: string): string return rawDiff; } -export function resolveScope(baseOverride?: string, ref?: WorkingTreeRef): ResolvedScope { - const base = baseOverride ?? detectBaseRef(); +function resolveRefToSha(ref: string): string { + return execFileSync("git", ["rev-parse", "--verify", ref], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); +} + +function canResolveRef(ref: string): boolean { + try { + resolveRefToSha(ref); + return true; + } catch { + return false; + } +} + +function resolveMergeBaseBetween(left: string, right: string): string { + return execFileSync("git", ["merge-base", left, right], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); +} + +function parseRefRange(ref: string): RefRange | null { + const threeDotIndex = ref.indexOf(RANGE_SEPARATOR.THREE_DOT); + if (threeDotIndex !== -1) { + return { + left: ref.slice(0, threeDotIndex), + right: ref.slice(threeDotIndex + RANGE_SEPARATOR.THREE_DOT.length), + }; + } + + const twoDotIndex = ref.indexOf(RANGE_SEPARATOR.TWO_DOT); + if (twoDotIndex !== -1) { + return { + left: ref.slice(0, twoDotIndex), + right: ref.slice(twoDotIndex + RANGE_SEPARATOR.TWO_DOT.length), + }; + } + + return null; +} + +function resolveCommittedComparison(left: string, right: string): ResolvedScope { + const effectiveLeft = left || "HEAD"; + const effectiveRight = right || "HEAD"; + + const mergeBaseSha = resolveMergeBaseBetween(effectiveLeft, effectiveRight); + const headSha = resolveRefToSha(effectiveRight); + + return { + scope: { + kind: SCOPE_KIND.COMMITTED, + baseSha: mergeBaseSha, + headSha, + mergeBaseSha, + }, + mergeBaseSha, + rawDiff: getRawDiff([`${mergeBaseSha}..${headSha}`]), + }; +} + +function parseWorkingTreeRefArg(ref: string): WorkingTreeRef | null { + switch (ref) { + case ".": + case WORKING_TREE_REF.WORK: + return WORKING_TREE_REF.WORK; + case WORKING_TREE_REF.STAGED: + return WORKING_TREE_REF.STAGED; + case WORKING_TREE_REF.UNSTAGED: + return WORKING_TREE_REF.UNSTAGED; + default: + return null; + } +} + +function resolveSingleRefScope(base: string, workingTreeRef?: WorkingTreeRef): ResolvedScope { const mergeBaseSha = resolveMergeBase(base); const headSha = resolveHead(); - const effectiveRef = ref ?? (hasUncommittedChanges() ? WORKING_TREE_REF.WORK : null); + const effectiveRef = workingTreeRef ?? (hasUncommittedChanges() ? WORKING_TREE_REF.WORK : null); if (effectiveRef) { return { @@ -265,3 +357,57 @@ export function resolveScope(baseOverride?: string, ref?: WorkingTreeRef): Resol rawDiff: getRawDiff([`${mergeBaseSha}..${headSha}`]), }; } + +export function resolveScope(options: ResolveScopeOptions = {}): ResolvedScope { + const refs = options.refs === undefined ? [] : options.refs; + if (refs.length > 2) { + throw new Error("Expected at most two git ref arguments."); + } + if (refs.length > 0 && (options.base !== undefined || options.compare !== undefined)) { + throw new Error("Cannot use --base/--compare with positional git ref arguments."); + } + if (refs.length > 0 && options.workingTreeRef !== undefined) { + throw new Error("Cannot use --ref with positional git ref arguments."); + } + if (options.compare !== undefined && options.workingTreeRef !== undefined) { + throw new Error("Cannot use --compare with --ref."); + } + + if (options.compare !== undefined) { + if (options.base === undefined) { + throw new Error("--compare requires --base."); + } + return resolveCommittedComparison(options.base, options.compare); + } + + if (refs.length === 2) { + const left = refs[0]; + const right = refs[1]; + if (left === undefined || right === undefined) { + throw new Error("Expected both base and compare refs."); + } + return resolveCommittedComparison(left, right); + } + + if (refs.length === 1) { + const ref = refs[0]; + if (ref === undefined) { + throw new Error("Expected a git ref argument."); + } + + const range = parseRefRange(ref); + if (range) return resolveCommittedComparison(range.left, range.right); + + if (!canResolveRef(ref)) { + const workingTreeRef = parseWorkingTreeRefArg(ref); + if (workingTreeRef) { + return resolveSingleRefScope(detectBaseRef(), workingTreeRef); + } + } + + return resolveSingleRefScope(ref); + } + + const base = options.base === undefined ? detectBaseRef() : options.base; + return resolveSingleRefScope(base, options.workingTreeRef); +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 2f61207..3518dba 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -21,14 +21,30 @@ const refOption = new Option( "Diff scope: work (staged + unstaged + untracked), staged, or unstaged (default: auto-detect)", ).choices(Object.values(WORKING_TREE_REF)); +interface DiffCommandOptions { + base?: string; + compare?: string; + ref?: string; +} + +function parseWorkingTreeRef(workingTreeRef?: string) { + return workingTreeRef !== undefined ? z.enum(WORKING_TREE_REF).parse(workingTreeRef) : undefined; +} + +function readWorkingTreeRef(options: DiffCommandOptions) { + return parseWorkingTreeRef(options.ref); +} + program .command("prep") .description("Parse the current branch diff and prepare input for chapter generation") + .argument("[refs...]", "Git refs to diff, for example: main, main feature, or main..feature") .option("--base ", "Base ref to diff against (default: auto-detect main/master)") + .option("--compare ", "Compare ref to diff against --base") .addOption(refOption) - .action((opts: { base?: string; ref?: string }) => { - const ref = opts.ref !== undefined ? z.enum(WORKING_TREE_REF).parse(opts.ref) : undefined; - const filePath = runPrep(opts.base, ref); + .action((refs: string[], opts: DiffCommandOptions) => { + const workingTreeRef = readWorkingTreeRef(opts); + const filePath = runPrep(opts.base, workingTreeRef, refs, opts.compare); process.stdout.write(filePath); }); @@ -36,11 +52,13 @@ program .command("show") .description("Load a chapters.json file and open it in a local browser") .argument("", "Path to a chapters.json file") + .argument("[refs...]", "Git refs to diff, for example: main, main feature, or main..feature") .option("--base ", "Base ref to diff against (default: auto-detect main/master)") + .option("--compare ", "Compare ref to diff against --base") .addOption(refOption) - .action(async (jsonPath: string, opts: { base?: string; ref?: string }) => { - const ref = opts.ref !== undefined ? z.enum(WORKING_TREE_REF).parse(opts.ref) : undefined; - await show(jsonPath, opts.base, ref); + .action(async (jsonPath: string, refs: string[], opts: DiffCommandOptions) => { + const workingTreeRef = readWorkingTreeRef(opts); + await show(jsonPath, opts.base, workingTreeRef, refs, opts.compare); }); program.parseAsync(process.argv).catch((err) => { diff --git a/packages/cli/src/prep.ts b/packages/cli/src/prep.ts index 4476cb6..9ad6968 100644 --- a/packages/cli/src/prep.ts +++ b/packages/cli/src/prep.ts @@ -5,7 +5,7 @@ import type { Hunk, PullRequestFile } from "@stagereview/types/parsed-diff"; import { parseGitDiff } from "./diff-parser.js"; import { filterFilesForLlm } from "./filter-files.js"; import { formatHunkDiffWithLineNumbers } from "./format-diff.js"; -import { getCommitMessages, resolveScope } from "./git.js"; +import { getCommitMessages, type ResolveScopeOptions, resolveScope } from "./git.js"; import type { WorkingTreeRef } from "./schema.js"; function formatHunkForPrompt(file: PullRequestFile, hunk: Hunk): string { @@ -14,8 +14,19 @@ function formatHunkForPrompt(file: PullRequestFile, hunk: Hunk): string { ${formatHunkDiffWithLineNumbers(hunk)}`; } -export function runPrep(base?: string, ref?: WorkingTreeRef): string { - const { rawDiff, mergeBaseSha } = resolveScope(base, ref); +export function runPrep( + base?: string, + workingTreeRef?: WorkingTreeRef, + refs?: string[], + compare?: string, +): string { + const options: ResolveScopeOptions = { + base, + compare, + refs, + workingTreeRef, + }; + const { scope, rawDiff, mergeBaseSha } = resolveScope(options); const allFiles = parseGitDiff(rawDiff); const { files } = filterFilesForLlm(allFiles); @@ -24,7 +35,7 @@ export function runPrep(base?: string, ref?: WorkingTreeRef): string { .flatMap((file) => file.hunks.map((hunk) => formatHunkForPrompt(file, hunk))) .join("\n\n"); - const commitMessages = getCommitMessages(mergeBaseSha); + const commitMessages = getCommitMessages(mergeBaseSha, scope.headSha); const sections = ["=== COMMIT MESSAGES ===", commitMessages, "", "=== HUNKS ===", formattedHunks]; diff --git a/packages/cli/src/show.ts b/packages/cli/src/show.ts index 6a5fe8d..a81f1e6 100644 --- a/packages/cli/src/show.ts +++ b/packages/cli/src/show.ts @@ -5,7 +5,7 @@ import { buildOtherChangesChapter } from "./build-other-changes.js"; import { closeDb, getDb } from "./db/client.js"; import { parseGitDiff } from "./diff-parser.js"; import { filterFilesForLlm } from "./filter-files.js"; -import { readRepoContext, resolveScope } from "./git.js"; +import { type ResolveScopeOptions, readRepoContext, resolveScope } from "./git.js"; import { diffRoutes } from "./routes/diff.js"; import { runRoutes } from "./routes/runs.js"; import { viewStateRoutes } from "./routes/view-state.js"; @@ -21,9 +21,15 @@ import { } from "./schema.js"; import { LOOPBACK_HOST, startServer } from "./server.js"; -export async function show(jsonPath: string, base?: string, ref?: WorkingTreeRef): Promise { +export async function show( + jsonPath: string, + base?: string, + workingTreeRef?: WorkingTreeRef, + refs?: string[], + compare?: string, +): Promise { const db = getDb(); - const chaptersFile = loadChaptersFile(jsonPath, base, ref); + const chaptersFile = loadChaptersFile(jsonPath, base, workingTreeRef, refs, compare); const { runId } = insertChaptersFile(db, chaptersFile, readRepoContext()); const handle = await startServer({ @@ -47,7 +53,13 @@ export async function show(jsonPath: string, base?: string, ref?: WorkingTreeRef closeDb(); } -function loadChaptersFile(jsonPath: string, base?: string, ref?: WorkingTreeRef): ChaptersFile { +function loadChaptersFile( + jsonPath: string, + base?: string, + workingTreeRef?: WorkingTreeRef, + refs?: string[], + compare?: string, +): ChaptersFile { const absolute = path.resolve(jsonPath); const raw = readFileSync(absolute, "utf8"); const parsed = JSON.parse(raw) as unknown; @@ -56,7 +68,9 @@ function loadChaptersFile(jsonPath: string, base?: string, ref?: WorkingTreeRef) if (fullResult.success) return fullResult.data; const agentResult = AgentOutputSchema.safeParse(parsed); - if (agentResult.success) return assembleChaptersFile(agentResult.data, base, ref); + if (agentResult.success) { + return assembleChaptersFile(agentResult.data, base, workingTreeRef, refs, compare); + } throw fullResult.error; } @@ -64,9 +78,17 @@ function loadChaptersFile(jsonPath: string, base?: string, ref?: WorkingTreeRef) function assembleChaptersFile( agentOutput: AgentOutput, base?: string, - ref?: WorkingTreeRef, + workingTreeRef?: WorkingTreeRef, + refs?: string[], + compare?: string, ): ChaptersFile { - const { scope, rawDiff } = resolveScope(base, ref); + const options: ResolveScopeOptions = { + base, + compare, + refs, + workingTreeRef, + }; + const { scope, rawDiff } = resolveScope(options); const allFiles = parseGitDiff(rawDiff); const { files: filteredFiles, excludedByPath } = filterFilesForLlm(allFiles); diff --git a/skills/stage-chapters/SKILL.md b/skills/stage-chapters/SKILL.md index 67b1b3d..594838d 100644 --- a/skills/stage-chapters/SKILL.md +++ b/skills/stage-chapters/SKILL.md @@ -38,21 +38,41 @@ PREP_FILE=$(stagereview prep) `stagereview prep` auto-detects the base ref (main/master), computes the merge-base, generates the diff, filters out lockfiles/binaries, and formats hunks with line numbers for analysis. By default it auto-detects the diff scope: if uncommitted changes are present the diff includes staged, unstaged, and untracked files; otherwise it uses the committed branch diff. It writes a plain-text file and prints only the file path to stdout. +`prep` and `show` also accept positional git refs: + +```bash +PREP_FILE=$(stagereview prep main) +PREP_FILE=$(stagereview prep main feature) +PREP_FILE=$(stagereview prep main..feature) +PREP_FILE=$(stagereview prep main...feature) +``` + +Use the same positional refs for `show`: + +```bash +stagereview show "$AGENT_OUTPUT" main..feature +``` + Both `prep` and `show` accept these optional flags: - **`--base `** — base ref to diff against (default: auto-detect main/master). +- **`--compare `** — compare ref to diff against `--base`. - **`--ref `** — diff scope. One of: - `work` — staged + unstaged + untracked changes (full working tree vs merge-base). - `staged` — only staged changes (index vs HEAD). - `unstaged` — only unstaged changes (working tree vs index). - Omitted — auto-detect (equivalent to `work` when uncommitted changes exist, committed branch diff otherwise). -When either flag is specified, pass it to **both** `prep` and `show`: +When flags or positional refs are specified, pass the same scope to **both** `prep` and `show`: ```bash PREP_FILE=$(stagereview prep --base feature-a --ref staged) # ... later ... stagereview show --base feature-a --ref staged "$AGENT_OUTPUT" + +PREP_FILE=$(stagereview prep --base main --compare feature) +# ... later ... +stagereview show --base main --compare feature "$AGENT_OUTPUT" ``` If `prep` exits non-zero, relay its stderr to the user and stop.