From c091c33c59098542f3d2a76810af5ff13cf23880 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Thu, 30 Apr 2026 08:44:31 +0800 Subject: [PATCH 1/7] fix: split git staged diff helpers --- packages/opencode/src/git/index.ts | 149 +++++++++++++++++++++++++ packages/opencode/test/git/git.test.ts | 104 +++++++++++++++++ 2 files changed, 253 insertions(+) diff --git a/packages/opencode/src/git/index.ts b/packages/opencode/src/git/index.ts index 4c1294230..677085765 100644 --- a/packages/opencode/src/git/index.ts +++ b/packages/opencode/src/git/index.ts @@ -66,9 +66,17 @@ export namespace Git { readonly hasHead: (cwd: string) => Effect.Effect readonly mergeBase: (cwd: string, base: string, head?: string) => Effect.Effect readonly show: (cwd: string, ref: string, file: string, prefix?: string) => Effect.Effect + readonly showIndex: (cwd: string, file: string, prefix?: string) => Effect.Effect readonly status: (cwd: string) => Effect.Effect + readonly statusUnstaged: (cwd: string) => Effect.Effect readonly diff: (cwd: string, ref: string) => Effect.Effect + readonly diffUnstaged: (cwd: string) => Effect.Effect + readonly diffStaged: (cwd: string) => Effect.Effect + readonly diffHead: (cwd: string, ref: string) => Effect.Effect readonly stats: (cwd: string, ref: string) => Effect.Effect + readonly statsUnstaged: (cwd: string) => Effect.Effect + readonly statsStaged: (cwd: string) => Effect.Effect + readonly statsHead: (cwd: string, ref: string) => Effect.Effect } const kind = (code: string): Kind => { @@ -193,6 +201,14 @@ export namespace Git { return result.text() }) + const showIndex = Effect.fn("Git.showIndex")(function* (cwd: string, file: string, prefix = "") { + const target = prefix ? `${prefix}${file}` : file + const result = yield* run(["show", `:${target}`], { cwd }) + if (result.exitCode !== 0) return "" + if (result.stdout.includes(0)) return "" + return result.text() + }) + const status = Effect.fn("Git.status")(function* (cwd: string) { return nuls( yield* text(["status", "--porcelain=v1", "--untracked-files=all", "--no-renames", "-z", "--", "."], { @@ -206,6 +222,22 @@ export namespace Git { }) }) + const statusUnstaged = Effect.fn("Git.statusUnstaged")(function* (cwd: string) { + return nuls( + yield* text(["status", "--porcelain=v1", "--untracked-files=all", "--no-renames", "-z", "--", "."], { + cwd, + }), + ).flatMap((item) => { + const file = item.slice(3) + if (!file) return [] + const index = item[0] ?? " " + const worktree = item[1] ?? " " + if (index !== "?" && worktree === " ") return [] + const code = item.slice(0, 2) + return [{ file, code, status: kind(code) } satisfies Item] + }) + }) + const diff = Effect.fn("Git.diff")(function* (cwd: string, ref: string) { const list = nuls( yield* text(["diff", "--no-ext-diff", "--no-renames", "--name-status", "-z", ref, "--", "."], { cwd }), @@ -218,6 +250,46 @@ export namespace Git { }) }) + const diffUnstaged = Effect.fn("Git.diffUnstaged")(function* (cwd: string) { + const list = nuls( + yield* text(["diff", "--no-ext-diff", "--no-renames", "--name-status", "-z", "--", "."], { cwd }), + ) + return list.flatMap((code, idx) => { + if (idx % 2 !== 0) return [] + const file = list[idx + 1] + if (!code || !file) return [] + return [{ file, code, status: kind(code) } satisfies Item] + }) + }) + + const diffStaged = Effect.fn("Git.diffStaged")(function* (cwd: string) { + const list = nuls( + yield* text(["diff", "--no-ext-diff", "--no-renames", "--cached", "--name-status", "-z", "--", "."], { + cwd, + }), + ) + return list.flatMap((code, idx) => { + if (idx % 2 !== 0) return [] + const file = list[idx + 1] + if (!code || !file) return [] + return [{ file, code, status: kind(code) } satisfies Item] + }) + }) + + const diffHead = Effect.fn("Git.diffHead")(function* (cwd: string, ref: string) { + const list = nuls( + yield* text(["diff", "--no-ext-diff", "--no-renames", "--name-status", "-z", ref, "HEAD", "--", "."], { + cwd, + }), + ) + return list.flatMap((code, idx) => { + if (idx % 2 !== 0) return [] + const file = list[idx + 1] + if (!code || !file) return [] + return [{ file, code, status: kind(code) } satisfies Item] + }) + }) + const stats = Effect.fn("Git.stats")(function* (cwd: string, ref: string) { return nuls( yield* text(["diff", "--no-ext-diff", "--no-renames", "--numstat", "-z", ref, "--", "."], { cwd }), @@ -241,6 +313,75 @@ export namespace Git { }) }) + const statsUnstaged = Effect.fn("Git.statsUnstaged")(function* (cwd: string) { + return nuls( + yield* text(["diff", "--no-ext-diff", "--no-renames", "--numstat", "-z", "--", "."], { cwd }), + ).flatMap((item) => { + const a = item.indexOf("\t") + const b = item.indexOf("\t", a + 1) + if (a === -1 || b === -1) return [] + const file = item.slice(b + 1) + if (!file) return [] + const adds = item.slice(0, a) + const dels = item.slice(a + 1, b) + const additions = adds === "-" ? 0 : Number.parseInt(adds || "0", 10) + const deletions = dels === "-" ? 0 : Number.parseInt(dels || "0", 10) + return [ + { + file, + additions: Number.isFinite(additions) ? additions : 0, + deletions: Number.isFinite(deletions) ? deletions : 0, + } satisfies Stat, + ] + }) + }) + + const statsStaged = Effect.fn("Git.statsStaged")(function* (cwd: string) { + return nuls( + yield* text(["diff", "--no-ext-diff", "--no-renames", "--cached", "--numstat", "-z", "--", "."], { cwd }), + ).flatMap((item) => { + const a = item.indexOf("\t") + const b = item.indexOf("\t", a + 1) + if (a === -1 || b === -1) return [] + const file = item.slice(b + 1) + if (!file) return [] + const adds = item.slice(0, a) + const dels = item.slice(a + 1, b) + const additions = adds === "-" ? 0 : Number.parseInt(adds || "0", 10) + const deletions = dels === "-" ? 0 : Number.parseInt(dels || "0", 10) + return [ + { + file, + additions: Number.isFinite(additions) ? additions : 0, + deletions: Number.isFinite(deletions) ? deletions : 0, + } satisfies Stat, + ] + }) + }) + + const statsHead = Effect.fn("Git.statsHead")(function* (cwd: string, ref: string) { + return nuls( + yield* text(["diff", "--no-ext-diff", "--no-renames", "--numstat", "-z", ref, "HEAD", "--", "."], { cwd }), + ).flatMap((item) => { + const a = item.indexOf("\t") + const b = item.indexOf("\t", a + 1) + if (a === -1 || b === -1) return [] + const file = item.slice(b + 1) + if (!file) return [] + const adds = item.slice(0, a) + const dels = item.slice(a + 1, b) + const additions = adds === "-" ? 0 : Number.parseInt(adds || "0", 10) + const deletions = dels === "-" ? 0 : Number.parseInt(dels || "0", 10) + return [ + { + file, + additions: Number.isFinite(additions) ? additions : 0, + deletions: Number.isFinite(deletions) ? deletions : 0, + } satisfies Stat, + ] + }) + }) + return Service.of({ run, branch, @@ -249,9 +390,17 @@ export namespace Git { hasHead, mergeBase, show, + showIndex, status, + statusUnstaged, diff, + diffUnstaged, + diffStaged, + diffHead, stats, + statsUnstaged, + statsStaged, + statsHead, }) }), ) diff --git a/packages/opencode/test/git/git.test.ts b/packages/opencode/test/git/git.test.ts index a897a38e6..6b6b7881b 100644 --- a/packages/opencode/test/git/git.test.ts +++ b/packages/opencode/test/git/git.test.ts @@ -114,6 +114,110 @@ describe("Git", () => { }) }) + test("statusUnstaged() excludes staged-only changes and includes untracked files", async () => { + await using tmp = await tmpdir({ git: true }) + await fs.writeFile(path.join(tmp.path, "tracked.txt"), "base\n", "utf-8") + await $`git add tracked.txt`.cwd(tmp.path).quiet() + await $`git commit --no-gpg-sign -m "base"`.cwd(tmp.path).quiet() + + await fs.writeFile(path.join(tmp.path, "staged.txt"), "staged\n", "utf-8") + await $`git add staged.txt`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "unstaged.txt"), "unstaged\n", "utf-8") + await fs.writeFile(path.join(tmp.path, "tracked.txt"), "base\nunstaged\n", "utf-8") + + await withGit(async (rt) => { + const status = await rt.runPromise(Git.Service.use((git) => git.statusUnstaged(tmp.path))) + expect(status).toEqual( + expect.arrayContaining([ + expect.objectContaining({ file: "tracked.txt", status: "modified" }), + expect.objectContaining({ file: "unstaged.txt", status: "added" }), + ]), + ) + expect(status).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "staged.txt" })])) + }) + }) + + test("showIndex() reads staged content instead of working tree content", async () => { + await using tmp = await tmpdir({ git: true }) + await fs.writeFile(path.join(tmp.path, "file.txt"), "base\n", "utf-8") + await $`git add file.txt`.cwd(tmp.path).quiet() + await $`git commit --no-gpg-sign -m "base"`.cwd(tmp.path).quiet() + + await fs.writeFile(path.join(tmp.path, "file.txt"), "staged\n", "utf-8") + await $`git add file.txt`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "file.txt"), "working\n", "utf-8") + + await withGit(async (rt) => { + const text = await rt.runPromise(Git.Service.use((git) => git.showIndex(tmp.path, "file.txt"))) + expect(text).toBe("staged\n") + }) + }) + + test("diffUnstaged(), statsUnstaged(), diffStaged(), and statsStaged() split index from working tree", async () => { + await using tmp = await tmpdir({ git: true }) + await fs.writeFile(path.join(tmp.path, "tracked.txt"), "base\n", "utf-8") + await $`git add tracked.txt`.cwd(tmp.path).quiet() + await $`git commit --no-gpg-sign -m "base"`.cwd(tmp.path).quiet() + + await fs.writeFile(path.join(tmp.path, "staged.txt"), "staged\n", "utf-8") + await $`git add staged.txt`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "unstaged.txt"), "unstaged\n", "utf-8") + await fs.writeFile(path.join(tmp.path, "tracked.txt"), "base\nstaged\n", "utf-8") + await $`git add tracked.txt`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "tracked.txt"), "base\nstaged\nworking\n", "utf-8") + + await withGit(async (rt) => { + const [unstagedDiff, unstagedStats, stagedDiff, stagedStats] = await Promise.all([ + rt.runPromise(Git.Service.use((git) => git.diffUnstaged(tmp.path))), + rt.runPromise(Git.Service.use((git) => git.statsUnstaged(tmp.path))), + rt.runPromise(Git.Service.use((git) => git.diffStaged(tmp.path))), + rt.runPromise(Git.Service.use((git) => git.statsStaged(tmp.path))), + ]) + + expect(unstagedDiff).toEqual( + expect.arrayContaining([expect.objectContaining({ file: "tracked.txt", status: "modified" })]), + ) + expect(unstagedDiff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "staged.txt" })])) + expect(unstagedStats).toEqual( + expect.arrayContaining([expect.objectContaining({ file: "tracked.txt", additions: 1, deletions: 0 })]), + ) + + expect(stagedDiff).toEqual( + expect.arrayContaining([ + expect.objectContaining({ file: "staged.txt", status: "added" }), + expect.objectContaining({ file: "tracked.txt", status: "modified" }), + ]), + ) + expect(stagedDiff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "unstaged.txt" })])) + expect(stagedStats).toEqual( + expect.arrayContaining([expect.objectContaining({ file: "staged.txt", additions: 1, deletions: 0 })]), + ) + }) + }) + + test("diffHead() and statsHead() compare a ref to HEAD without working tree changes", async () => { + await using tmp = await tmpdir({ git: true }) + await $`git branch -M main`.cwd(tmp.path).quiet() + await $`git checkout -b feature/test`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "branch.txt"), "branch\n", "utf-8") + await $`git add branch.txt`.cwd(tmp.path).quiet() + await $`git commit --no-gpg-sign -m "branch file"`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "unstaged.txt"), "unstaged\n", "utf-8") + + await withGit(async (rt) => { + const [diff, stats] = await Promise.all([ + rt.runPromise(Git.Service.use((git) => git.diffHead(tmp.path, "main"))), + rt.runPromise(Git.Service.use((git) => git.statsHead(tmp.path, "main"))), + ]) + + expect(diff).toEqual(expect.arrayContaining([expect.objectContaining({ file: "branch.txt", status: "added" })])) + expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "unstaged.txt" })])) + expect(stats).toEqual( + expect.arrayContaining([expect.objectContaining({ file: "branch.txt", additions: 1, deletions: 0 })]), + ) + }) + }) + test("show() returns empty text for binary blobs", async () => { await using tmp = await tmpdir({ git: true }) await fs.writeFile(path.join(tmp.path, "bin.dat"), new Uint8Array([0, 1, 2, 3])) From eb542bce710be6e0bb48ab3c43b6e68a21358afa Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Thu, 30 Apr 2026 08:51:04 +0800 Subject: [PATCH 2/7] fix: expose staged review diff mode --- packages/opencode/src/project/vcs.ts | 109 +++++++++++------- .../opencode/src/server/instance/index.ts | 2 +- packages/opencode/test/project/vcs.test.ts | 75 ++++++++++-- packages/sdk/js/src/v2/gen/sdk.gen.ts | 4 +- packages/sdk/js/src/v2/gen/types.gen.ts | 2 +- packages/sdk/openapi.json | 5 +- 6 files changed, 140 insertions(+), 57 deletions(-) diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index 6832fa78f..5b58efb7a 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -40,24 +40,19 @@ export namespace Vcs { return [...out.values()] } - const files = Effect.fnUntraced(function* ( - fs: AppFileSystem.Interface, - git: Git.Interface, - cwd: string, - ref: string | undefined, - list: Git.Item[], - map: Map, - ) { - const base = ref ? yield* git.prefix(cwd) : "" + const staged = Effect.fnUntraced(function* (fs: AppFileSystem.Interface, git: Git.Interface, cwd: string) { + const [list, stats] = yield* Effect.all([git.diffStaged(cwd), git.statsStaged(cwd)], { concurrency: 2 }) + const statMap = nums(stats) + const base = yield* git.prefix(cwd) const patch = (file: string, before: string, after: string) => formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER })) const next = yield* Effect.forEach( list, (item) => Effect.gen(function* () { - const before = item.status === "added" || !ref ? "" : yield* git.show(cwd, ref, item.file, base) - const after = item.status === "deleted" ? "" : yield* work(fs, cwd, item.file) - const stat = map.get(item.file) + const before = item.status === "added" ? "" : yield* git.show(cwd, "HEAD", item.file, base) + const after = item.status === "deleted" ? "" : yield* git.showIndex(cwd, item.file, base) + const stat = statMap.get(item.file) return { file: item.file, patch: patch(item.file, before, after), @@ -71,40 +66,75 @@ export namespace Vcs { return next.toSorted((a, b) => a.file.localeCompare(b.file)) }) - const track = Effect.fnUntraced(function* ( + const unstaged = Effect.fnUntraced(function* ( fs: AppFileSystem.Interface, git: Git.Interface, cwd: string, - ref: string | undefined, ) { - if (!ref) return yield* files(fs, git, cwd, ref, yield* git.status(cwd), new Map()) - const [list, stats] = yield* Effect.all([git.status(cwd), git.stats(cwd, ref)], { concurrency: 2 }) - return yield* files(fs, git, cwd, ref, list, nums(stats)) + const [tracked, extra, stats] = yield* Effect.all( + [git.diffUnstaged(cwd), git.statusUnstaged(cwd), git.statsUnstaged(cwd)], + { concurrency: 3 }, + ) + const list = merge( + tracked, + extra.filter((item) => item.code === "??"), + ) + const statMap = nums(stats) + const base = yield* git.prefix(cwd) + const patch = (file: string, before: string, after: string) => + formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER })) + const next = yield* Effect.forEach( + list, + (item) => + Effect.gen(function* () { + const before = item.code === "??" ? "" : yield* git.showIndex(cwd, item.file, base) + const after = item.status === "deleted" ? "" : yield* work(fs, cwd, item.file) + const stat = statMap.get(item.file) + return { + file: item.file, + patch: patch(item.file, before, after), + additions: stat?.additions ?? (item.status === "added" ? count(after) : 0), + deletions: stat?.deletions ?? (item.status === "deleted" ? count(before) : 0), + status: item.status, + } satisfies FileDiff + }), + { concurrency: 8 }, + ) + return next.toSorted((a, b) => a.file.localeCompare(b.file)) }) - const compare = Effect.fnUntraced(function* ( + const branchHead = Effect.fnUntraced(function* ( fs: AppFileSystem.Interface, git: Git.Interface, cwd: string, ref: string, ) { - const [list, stats, extra] = yield* Effect.all([git.diff(cwd, ref), git.stats(cwd, ref), git.status(cwd)], { - concurrency: 3, - }) - return yield* files( - fs, - git, - cwd, - ref, - merge( - list, - extra.filter((item) => item.code === "??"), - ), - nums(stats), + const [list, stats] = yield* Effect.all([git.diffHead(cwd, ref), git.statsHead(cwd, ref)], { concurrency: 2 }) + const statMap = nums(stats) + const base = yield* git.prefix(cwd) + const patch = (file: string, before: string, after: string) => + formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER })) + const next = yield* Effect.forEach( + list, + (item) => + Effect.gen(function* () { + const before = item.status === "added" ? "" : yield* git.show(cwd, ref, item.file, base) + const after = item.status === "deleted" ? "" : yield* git.show(cwd, "HEAD", item.file, base) + const stat = statMap.get(item.file) + return { + file: item.file, + patch: patch(item.file, before, after), + additions: stat?.additions ?? (item.status === "added" ? count(after) : 0), + deletions: stat?.deletions ?? (item.status === "deleted" ? count(before) : 0), + status: item.status, + } satisfies FileDiff + }), + { concurrency: 8 }, ) + return next.toSorted((a, b) => a.file.localeCompare(b.file)) }) - export const Mode = z.enum(["git", "branch"]) + export const Mode = z.enum(["unstaged", "staged", "branch"]) export type Mode = z.infer export const Event = { @@ -207,20 +237,19 @@ export namespace Vcs { diff: Effect.fn("Vcs.diff")(function* (mode: Mode) { const value = yield* InstanceState.get(state) if (Instance.project.vcs !== "git") return [] - if (mode === "git") { - return yield* track( - fs, - git, - Instance.directory, - (yield* git.hasHead(Instance.directory)) ? "HEAD" : undefined, - ) + if (mode === "unstaged") { + return yield* unstaged(fs, git, Instance.directory) + } + + if (mode === "staged") { + return yield* staged(fs, git, Instance.directory) } if (!value.root) return [] if (value.current && value.current === value.root.name) return [] const ref = yield* git.mergeBase(Instance.directory, value.root.ref) if (!ref) return [] - return yield* compare(fs, git, Instance.directory, ref) + return yield* branchHead(fs, git, Instance.directory, ref) }), }) }), diff --git a/packages/opencode/src/server/instance/index.ts b/packages/opencode/src/server/instance/index.ts index e8580d6b6..13375ab41 100644 --- a/packages/opencode/src/server/instance/index.ts +++ b/packages/opencode/src/server/instance/index.ts @@ -128,7 +128,7 @@ export const InstanceRoutes = (upgrade: UpgradeWebSocket): Hono => "/vcs/diff", describeRoute({ summary: "Get VCS diff", - description: "Retrieve the current git diff for the working tree or against the default branch.", + description: "Retrieve the current unstaged, staged, or default-branch git diff.", operationId: "vcs.diff", responses: { 200: { diff --git a/packages/opencode/test/project/vcs.test.ts b/packages/opencode/test/project/vcs.test.ts index 1610902af..78ae49136 100644 --- a/packages/opencode/test/project/vcs.test.ts +++ b/packages/opencode/test/project/vcs.test.ts @@ -169,32 +169,34 @@ describe("Vcs diff", () => { }) }) - test("diff('git') returns uncommitted changes", async () => { + test("diff('unstaged') returns unstaged and untracked changes only", async () => { await using tmp = await tmpdir({ git: true }) - await fs.writeFile(path.join(tmp.path, "file.txt"), "original\n", "utf-8") - await $`git add .`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "tracked.txt"), "original\n", "utf-8") + await $`git add tracked.txt`.cwd(tmp.path).quiet() await $`git commit --no-gpg-sign -m "add file"`.cwd(tmp.path).quiet() - await fs.writeFile(path.join(tmp.path, "file.txt"), "changed\n", "utf-8") + await fs.writeFile(path.join(tmp.path, "tracked.txt"), "changed\n", "utf-8") + await fs.writeFile(path.join(tmp.path, "staged.txt"), "staged\n", "utf-8") + await $`git add staged.txt`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "untracked.txt"), "untracked\n", "utf-8") await withVcsOnly(tmp.path, async () => { - const diff = await Vcs.diff("git") + const diff = await Vcs.diff("unstaged") expect(diff).toEqual( expect.arrayContaining([ - expect.objectContaining({ - file: "file.txt", - status: "modified", - }), + expect.objectContaining({ file: "tracked.txt", status: "modified" }), + expect.objectContaining({ file: "untracked.txt", status: "added" }), ]), ) + expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "staged.txt" })])) }) }) - test("diff('git') handles special filenames", async () => { + test("diff('unstaged') handles special filenames", async () => { await using tmp = await tmpdir({ git: true }) await fs.writeFile(path.join(tmp.path, weird), "hello\n", "utf-8") await withVcsOnly(tmp.path, async () => { - const diff = await Vcs.diff("git") + const diff = await Vcs.diff("unstaged") expect(diff).toEqual( expect.arrayContaining([ expect.objectContaining({ @@ -206,6 +208,57 @@ describe("Vcs diff", () => { }) }) + test("diff('staged') returns staged changes only", async () => { + await using tmp = await tmpdir({ git: true }) + await fs.writeFile(path.join(tmp.path, "tracked.txt"), "original\n", "utf-8") + await $`git add tracked.txt`.cwd(tmp.path).quiet() + await $`git commit --no-gpg-sign -m "add file"`.cwd(tmp.path).quiet() + + await fs.writeFile(path.join(tmp.path, "staged.txt"), "staged\n", "utf-8") + await $`git add staged.txt`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "unstaged.txt"), "unstaged\n", "utf-8") + + await withVcsOnly(tmp.path, async () => { + const diff = await Vcs.diff("staged") + expect(diff).toEqual(expect.arrayContaining([expect.objectContaining({ file: "staged.txt", status: "added" })])) + expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "unstaged.txt" })])) + }) + }) + + test("diff('staged') returns staged files before the first commit", async () => { + await using tmp = await tmpdir() + await $`git init`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "first.txt"), "first\n", "utf-8") + await $`git add first.txt`.cwd(tmp.path).quiet() + + await withVcsOnly(tmp.path, async () => { + const diff = await Vcs.diff("staged") + expect(diff).toEqual(expect.arrayContaining([expect.objectContaining({ file: "first.txt", status: "added" })])) + }) + }) + + test("diff('branch') returns committed branch changes without staged, unstaged, or untracked files", async () => { + await using tmp = await tmpdir({ git: true }) + await $`git branch -M main`.cwd(tmp.path).quiet() + await $`git checkout -b feature/test`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "branch.txt"), "branch\n", "utf-8") + await $`git add branch.txt`.cwd(tmp.path).quiet() + await $`git commit --no-gpg-sign -m "branch file"`.cwd(tmp.path).quiet() + + await fs.writeFile(path.join(tmp.path, "staged.txt"), "staged\n", "utf-8") + await $`git add staged.txt`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, "unstaged.txt"), "unstaged\n", "utf-8") + await fs.writeFile(path.join(tmp.path, "untracked.txt"), "untracked\n", "utf-8") + + await withVcsOnly(tmp.path, async () => { + const diff = await Vcs.diff("branch") + expect(diff).toEqual(expect.arrayContaining([expect.objectContaining({ file: "branch.txt", status: "added" })])) + expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "staged.txt" })])) + expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "unstaged.txt" })])) + expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "untracked.txt" })])) + }) + }) + test("diff('branch') returns changes against default branch", async () => { await using tmp = await tmpdir({ git: true }) await $`git branch -M main`.cwd(tmp.path).quiet() diff --git a/packages/sdk/js/src/v2/gen/sdk.gen.ts b/packages/sdk/js/src/v2/gen/sdk.gen.ts index 2b6ce2fc1..af6a8e5b0 100644 --- a/packages/sdk/js/src/v2/gen/sdk.gen.ts +++ b/packages/sdk/js/src/v2/gen/sdk.gen.ts @@ -3609,13 +3609,13 @@ export class Vcs extends HeyApiClient { /** * Get VCS diff * - * Retrieve the current git diff for the working tree or against the default branch. + * Retrieve the current unstaged, staged, or default-branch git diff. */ public diff( parameters: { directory?: string workspace?: string - mode: "git" | "branch" + mode: "unstaged" | "staged" | "branch" }, options?: Options, ) { diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index 34f95951f..3f49ca625 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -4897,7 +4897,7 @@ export type VcsDiffData = { query: { directory?: string workspace?: string - mode: "git" | "branch" + mode: "unstaged" | "staged" | "branch" } url: "/vcs/diff" } diff --git a/packages/sdk/openapi.json b/packages/sdk/openapi.json index 7bc270413..510b494f2 100644 --- a/packages/sdk/openapi.json +++ b/packages/sdk/openapi.json @@ -6507,7 +6507,8 @@ "schema": { "type": "string", "enum": [ - "git", + "unstaged", + "staged", "branch" ] }, @@ -6515,7 +6516,7 @@ } ], "summary": "Get VCS diff", - "description": "Retrieve the current git diff for the working tree or against the default branch.", + "description": "Retrieve the current unstaged, staged, or default-branch git diff.", "responses": { "200": { "description": "VCS diff", From 10773a121ee7144da4125f647bc721788220987c Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Thu, 30 Apr 2026 08:52:09 +0800 Subject: [PATCH 3/7] fix: define review change modes --- .../pages/session/review-change-mode.test.ts | 71 +++++++++++++++++++ .../src/pages/session/review-change-mode.ts | 34 +++++++++ 2 files changed, 105 insertions(+) create mode 100644 packages/app/src/pages/session/review-change-mode.test.ts create mode 100644 packages/app/src/pages/session/review-change-mode.ts diff --git a/packages/app/src/pages/session/review-change-mode.test.ts b/packages/app/src/pages/session/review-change-mode.test.ts new file mode 100644 index 000000000..47d4b5bcc --- /dev/null +++ b/packages/app/src/pages/session/review-change-mode.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, test } from "bun:test" +import { + coerceReviewChangeMode, + DEFAULT_REVIEW_CHANGE_MODE, + isVcsReviewMode, + nextReviewModeForSessionChange, + reviewChangeOptions, + reviewDiffsForMode, + reviewModeLabelKey, +} from "./review-change-mode" + +describe("review change mode", () => { + test("defaults to last turn", () => { + expect(DEFAULT_REVIEW_CHANGE_MODE).toBe("turn") + }) + + test("keeps all review modes selectable for git projects", () => { + expect(reviewChangeOptions({ isGit: true })).toEqual(["unstaged", "staged", "branch", "turn"]) + }) + + test("keeps branch selectable even when the branch diff is empty", () => { + expect(reviewChangeOptions({ isGit: true })).toContain("branch") + }) + + test("limits non-git projects to last turn", () => { + expect(reviewChangeOptions({ isGit: false })).toEqual(["turn"]) + }) + + test("falls back to last turn when the selected mode is unavailable", () => { + expect(coerceReviewChangeMode("branch", ["turn"])).toBe("turn") + }) + + test("identifies VCS-backed review modes", () => { + expect(isVcsReviewMode("unstaged")).toBe(true) + expect(isVcsReviewMode("staged")).toBe(true) + expect(isVcsReviewMode("branch")).toBe(true) + expect(isVcsReviewMode("turn")).toBe(false) + }) + + test("maps modes to translation keys", () => { + expect(reviewModeLabelKey("unstaged")).toBe("ui.sessionReview.title.unstaged") + expect(reviewModeLabelKey("staged")).toBe("ui.sessionReview.title.staged") + expect(reviewModeLabelKey("branch")).toBe("ui.sessionReview.title.branch") + expect(reviewModeLabelKey("turn")).toBe("ui.sessionReview.title.lastTurn") + }) + + test("resets session changes to last turn", () => { + expect(nextReviewModeForSessionChange()).toBe("turn") + }) + + test("uses turn diffs without falling back to VCS diffs", () => { + const turn = ["turn diff"] + const vcs = { + unstaged: ["unstaged diff"], + staged: ["staged diff"], + branch: ["branch diff"], + } + + expect(reviewDiffsForMode("turn", { turn, vcs })).toEqual(turn) + }) + + test("keeps an empty last turn empty when VCS diffs exist", () => { + const vcs = { + unstaged: ["unstaged diff"], + staged: ["staged diff"], + branch: ["branch diff"], + } + + expect(reviewDiffsForMode("turn", { turn: [], vcs })).toEqual([]) + }) +}) diff --git a/packages/app/src/pages/session/review-change-mode.ts b/packages/app/src/pages/session/review-change-mode.ts new file mode 100644 index 000000000..5ce9ae349 --- /dev/null +++ b/packages/app/src/pages/session/review-change-mode.ts @@ -0,0 +1,34 @@ +export type ReviewChangeMode = "unstaged" | "staged" | "branch" | "turn" +export type VcsReviewMode = Exclude + +export const DEFAULT_REVIEW_CHANGE_MODE: ReviewChangeMode = "turn" + +export const isVcsReviewMode = (mode: ReviewChangeMode): mode is VcsReviewMode => + mode === "unstaged" || mode === "staged" || mode === "branch" + +export const reviewChangeOptions = (input: { isGit: boolean }): ReviewChangeMode[] => { + if (!input.isGit) return [DEFAULT_REVIEW_CHANGE_MODE] + return ["unstaged", "staged", "branch", DEFAULT_REVIEW_CHANGE_MODE] +} + +export const coerceReviewChangeMode = ( + mode: ReviewChangeMode, + options: readonly ReviewChangeMode[], +): ReviewChangeMode => (options.includes(mode) ? mode : DEFAULT_REVIEW_CHANGE_MODE) + +export const reviewModeLabelKey = (mode: ReviewChangeMode) => { + if (mode === "unstaged") return "ui.sessionReview.title.unstaged" + if (mode === "staged") return "ui.sessionReview.title.staged" + if (mode === "branch") return "ui.sessionReview.title.branch" + return "ui.sessionReview.title.lastTurn" +} + +export const nextReviewModeForSessionChange = () => DEFAULT_REVIEW_CHANGE_MODE + +export const reviewDiffsForMode = ( + mode: ReviewChangeMode, + input: { turn: readonly T[]; vcs: Record }, +): readonly T[] => { + if (isVcsReviewMode(mode)) return input.vcs[mode] + return input.turn +} From a31312a0d771852218ca10ab48d7cdb9451386ab Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Thu, 30 Apr 2026 08:56:52 +0800 Subject: [PATCH 4/7] fix: default review to last turn --- packages/app/src/i18n/en.ts | 6 + packages/app/src/i18n/parity.test.ts | 7 ++ packages/app/src/i18n/zh.ts | 7 ++ packages/app/src/pages/session.tsx | 157 ++++++++------------------- 4 files changed, 65 insertions(+), 112 deletions(-) diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index b24897f9c..3fe68ea01 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -595,7 +595,13 @@ export const dict = { "session.review.noSnapshot": "Snapshot tracking is disabled in config, so session changes are unavailable", "session.review.noChanges": "No changes", "session.review.noUncommittedChanges": "No uncommitted changes yet", + "session.review.noUnstagedChanges": "No unstaged changes yet", + "session.review.noStagedChanges": "No staged changes yet", "session.review.noBranchChanges": "No branch changes yet", + "ui.sessionReview.title.unstaged": "Unstaged", + "ui.sessionReview.title.staged": "Staged", + "ui.sessionReview.title.branch": "Branch", + "ui.sessionReview.title.lastTurn": "Last Turn", "session.files.selectToOpen": "Select a file to open", "session.files.all": "All files", diff --git a/packages/app/src/i18n/parity.test.ts b/packages/app/src/i18n/parity.test.ts index 9b15490b5..519b1d76c 100644 --- a/packages/app/src/i18n/parity.test.ts +++ b/packages/app/src/i18n/parity.test.ts @@ -18,6 +18,13 @@ const keys = [ "session.panel.utility", "session.panel.files", "session.panel.changes", + "session.review.noUnstagedChanges", + "session.review.noStagedChanges", + "session.review.noBranchChanges", + "ui.sessionReview.title.unstaged", + "ui.sessionReview.title.staged", + "ui.sessionReview.title.branch", + "ui.sessionReview.title.lastTurn", ] as const describe("i18n parity", () => { diff --git a/packages/app/src/i18n/zh.ts b/packages/app/src/i18n/zh.ts index 9edcf834e..491744590 100644 --- a/packages/app/src/i18n/zh.ts +++ b/packages/app/src/i18n/zh.ts @@ -559,6 +559,13 @@ export const dict = { "session.review.noVcs": "未检测到 Git 版本控制系统，无法显示更改", "session.review.noSnapshot": "配置中已禁用快照跟踪，因此会话更改不可用", "session.review.noChanges": "无更改", + "session.review.noUnstagedChanges": "暂无未暂存变更", + "session.review.noStagedChanges": "暂无已暂存变更", + "session.review.noBranchChanges": "暂无分支变更", + "ui.sessionReview.title.unstaged": "未暂存变更", + "ui.sessionReview.title.staged": "已暂存变更", + "ui.sessionReview.title.branch": "分支变更", + "ui.sessionReview.title.lastTurn": "上轮变更", "session.files.selectToOpen": "选择要打开的文件", "session.files.all": "所有文件", "session.files.empty": "无文件", diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx index d747bc77e..af06314ea 100644 --- a/packages/app/src/pages/session.tsx +++ b/packages/app/src/pages/session.tsx @@ -1,4 +1,4 @@ -import type { Project, UserMessage, VcsFileDiff } from "@opencode-ai/sdk/v2" +import type { UserMessage, VcsFileDiff } from "@opencode-ai/sdk/v2" import { useDialog } from "@opencode-ai/ui/context/dialog" import { useMutation } from "@tanstack/solid-query" import { @@ -26,7 +26,6 @@ import { Select } from "@opencode-ai/ui/select" import { Tabs } from "@opencode-ai/ui/tabs" import { createAutoScroll } from "@opencode-ai/ui/hooks" import { previewSelectedLines } from "@opencode-ai/ui/pierre/selection-bridge" -import { Button } from "@opencode-ai/ui/button" import { showToast } from "@opencode-ai/ui/toast" import { checksum } from "@opencode-ai/util/encode" import { useLocation, useSearchParams } from "@solidjs/router" @@ -54,6 +53,17 @@ import { } from "@/pages/session/helpers" import { MessageTimeline } from "@/pages/session/message-timeline" import { SessionReviewTab, type SessionReviewTabProps } from "@/pages/session/review-tab" +import { + coerceReviewChangeMode, + DEFAULT_REVIEW_CHANGE_MODE, + isVcsReviewMode, + nextReviewModeForSessionChange, + reviewChangeOptions, + reviewDiffsForMode, + reviewModeLabelKey, + type ReviewChangeMode, + type VcsReviewMode, +} from "@/pages/session/review-change-mode" import { useSessionLayout } from "@/pages/session/session-layout" import { emptyMessages, @@ -80,9 +90,6 @@ type FollowupItem = FollowupDraft & { id: string } type FollowupEdit = Pick const emptyFollowups: FollowupItem[] = [] -type ChangeMode = "git" | "branch" | "turn" -type VcsMode = "git" | "branch" - type SessionHistoryWindowInput = { sessionID: () => string | undefined messagesReady: () => boolean @@ -614,27 +621,23 @@ export default function Page() { const [store, setStore] = createStore({ messageId: undefined as string | undefined, mobileTab: "session" as "session" | "changes", - changes: "git" as ChangeMode, + changes: DEFAULT_REVIEW_CHANGE_MODE as ReviewChangeMode, newSessionWorktree: "main", deferRender: false, }) const [vcs, setVcs] = createStore<{ - diff: { - git: VcsFileDiff[] - branch: VcsFileDiff[] - } - ready: { - git: boolean - branch: boolean - } + diff: Record + ready: Record }>({ diff: { - git: [] as VcsFileDiff[], + unstaged: [] as VcsFileDiff[], + staged: [] as VcsFileDiff[], branch: [] as VcsFileDiff[], }, ready: { - git: false, + unstaged: false, + staged: false, branch: false, }, }) @@ -671,18 +674,18 @@ export default function Page() { let todoTimer: number | undefined let diffFrame: number | undefined let diffTimer: number | undefined - const vcsTask = new Map>() - const vcsRun = new Map() + const vcsTask = new Map>() + const vcsRun = new Map() - const bumpVcs = (mode: VcsMode) => { + const bumpVcs = (mode: VcsReviewMode) => { const next = (vcsRun.get(mode) ?? 0) + 1 vcsRun.set(mode, next) return next } - const resetVcs = (mode?: VcsMode) => { - const list = mode ? [mode] : (["git", "branch"] as const) - list.forEach((item) => { + const resetVcs = (mode?: VcsReviewMode) => { + const modes = mode ? [mode] : (["unstaged", "staged", "branch"] as const) + modes.forEach((item) => { bumpVcs(item) vcsTask.delete(item) setVcs("diff", item, []) @@ -690,7 +693,7 @@ export default function Page() { }) } - const loadVcs = (mode: VcsMode, force = false) => { + const loadVcs = (mode: VcsReviewMode, force = false) => { if (sync.project?.vcs !== "git") return Promise.resolve() if (!force && vcs.ready[mode]) return Promise.resolve() @@ -761,34 +764,24 @@ export default function Page() { }), ) }) - const nogit = createMemo(() => !!sync.project && sync.project.vcs !== "git") - const changesOptions = createMemo(() => { - const list: ChangeMode[] = [] - if (sync.project?.vcs === "git") list.push("git") - if ( - sync.project?.vcs === "git" && - sync.data.vcs?.branch && - sync.data.vcs?.default_branch && - sync.data.vcs.branch !== sync.data.vcs.default_branch - ) { - list.push("branch") - } - list.push("turn") - return list - }) - const vcsMode = createMemo(() => { - if (store.changes === "git" || store.changes === "branch") return store.changes + const changesOptions = createMemo(() => + reviewChangeOptions({ isGit: sync.project?.vcs === "git" }), + ) + const vcsMode = createMemo(() => { + if (isVcsReviewMode(store.changes)) return store.changes }) const reviewDiffs = createMemo(() => { - if (store.changes === "git") return list(vcs.diff.git) - if (store.changes === "branch") return list(vcs.diff.branch) - return turnDiffs() + return list( + reviewDiffsForMode(store.changes, { + turn: turnDiffs(), + vcs: vcs.diff, + }), + ) }) const reviewCount = createMemo(() => reviewDiffs().length) const hasReview = createMemo(() => reviewCount() > 0) const reviewReady = createMemo(() => { - if (store.changes === "git") return vcs.ready.git - if (store.changes === "branch") return vcs.ready.branch + if (isVcsReviewMode(store.changes)) return vcs.ready[store.changes] return true }) @@ -856,45 +849,6 @@ export default function Page() { scrollToMessage(msgs[targetIndex], "auto") } - function upsert(next: Project) { - const list = globalSync.data.project - sync.set("project", next.id) - const idx = list.findIndex((item) => item.id === next.id) - if (idx >= 0) { - globalSync.set( - "project", - list.map((item, i) => (i === idx ? { ...item, ...next } : item)), - ) - return - } - const at = list.findIndex((item) => item.id > next.id) - if (at >= 0) { - globalSync.set("project", [...list.slice(0, at), next, ...list.slice(at)]) - return - } - globalSync.set("project", [...list, next]) - } - - const gitMutation = useMutation(() => ({ - mutationFn: () => sdk.client.project.initGit(), - onSuccess: (x) => { - if (!x.data) return - upsert(x.data) - }, - onError: (err) => { - showToast({ - variant: "error", - title: language.t("common.requestFailed"), - description: formatServerError(err, language.t), - }) - }, - })) - - function initGit() { - if (gitMutation.isPending) return - gitMutation.mutate() - } - let inputRef!: HTMLDivElement let promptDock: HTMLDivElement | undefined let dockHeight = 0 @@ -1003,7 +957,7 @@ export default function Page() { sessionKey, () => { setStore("messageId", undefined) - setStore("changes", "git") + setStore("changes", nextReviewModeForSessionChange()) setUi("pendingMessage", undefined) }, { defer: true }, @@ -1207,9 +1161,8 @@ export default function Page() { createEffect(() => { const list = changesOptions() - if (list.includes(store.changes)) return - const next = list[0] - if (!next) return + const next = coerceReviewChangeMode(store.changes, list) + if (next === store.changes) return setStore("changes", next) }) @@ -1287,11 +1240,7 @@ export default function Page() { return null } - const label = (option: ChangeMode) => { - if (option === "git") return language.t("ui.sessionReview.title.git") - if (option === "branch") return language.t("ui.sessionReview.title.branch") - return language.t("ui.sessionReview.title.lastTurn") - } + const label = (option: ReviewChangeMode) => language.t(reviewModeLabelKey(option)) return (