From d1034d62b28e19c19240085566a39a92c5f9b466 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 2 Oct 2026 22:52:42 -0700 Subject: [PATCH] fix(web): diff panel opens on all branch changes, not just uncommitted (#15005) Co-authored-by: Claude Opus 5.5 (1M context) --- .../src/features/review/ReviewSheet.tsx | 4 +- .../src/features/review/reviewModel.test.ts | 35 +- .../mobile/src/features/review/reviewModel.ts | 24 +- .../features/threads/git/GitOverviewSheet.tsx | 2 +- apps/server/src/git/GitManager.ts | 7 +- apps/server/src/vcs/GitVcsDriver.ts | 3 + apps/server/src/vcs/GitVcsDriverCore.test.ts | 185 +++++++++- apps/server/src/vcs/GitVcsDriverCore.ts | 331 ++++++++++++------ .../src/vcs/VcsStatusBroadcaster.test.ts | 100 +++++- apps/server/src/vcs/VcsStatusBroadcaster.ts | 20 +- apps/web/src/components/ChatView.tsx | 8 +- apps/web/src/components/DiffPanel.tsx | 20 +- apps/web/src/components/GitActionsControl.tsx | 10 +- .../components/settings/SettingsPanels.tsx | 2 +- apps/web/src/diffPanelStore.test.ts | 21 +- apps/web/src/diffPanelStore.ts | 3 +- packages/contracts/src/git.ts | 11 + packages/shared/src/git.ts | 1 + 18 files changed, 611 insertions(+), 176 deletions(-) diff --git a/apps/mobile/src/features/review/ReviewSheet.tsx b/apps/mobile/src/features/review/ReviewSheet.tsx index 08888dd469d23aaa2d7fa5faf82ed822a2b80c9f..20c87b496ab3c4f64a344428a2262766c235f471 100644 --- a/apps/mobile/src/features/review/ReviewSheet.tsx +++ b/apps/mobile/src/features/review/ReviewSheet.tsx @@ -126,8 +126,8 @@ function ReviewHeader( id: "sections", inline: true, items: [ - sectionAction(props.sectionMenu.workingTree, "Working tree"), - sectionAction(props.sectionMenu.branchChanges, "Branch changes"), + sectionAction(props.sectionMenu.branchChanges, "Changes"), + sectionAction(props.sectionMenu.workingTree, "Uncommitted"), sectionAction(props.sectionMenu.latestTurn, "Latest turn"), ], }, diff --git a/apps/mobile/src/features/review/reviewModel.test.ts b/apps/mobile/src/features/review/reviewModel.test.ts index 2af0fb4ba063fbbc541279d7f1d3560846115bcd..9c2cc9b567c9b7bd43f95c3cdea7ffc921d38c01 100644 --- a/apps/mobile/src/features/review/reviewModel.test.ts +++ b/apps/mobile/src/features/review/reviewModel.test.ts @@ -62,7 +62,7 @@ describe("buildReviewSectionItems", () => { { id: "working-tree", kind: "working-tree", - title: "Dirty worktree", + title: "Uncommitted", baseRef: "HEAD", headRef: null, diff: "diff --git a/a.ts b/a.ts", @@ -72,7 +72,7 @@ describe("buildReviewSectionItems", () => { { id: "branch-range", kind: "branch-range", - title: "Against main", + title: "Changes vs main", baseRef: "main", headRef: "feature", diff: "diff --git a/a.ts b/a.ts", @@ -105,10 +105,28 @@ describe("buildReviewSectionItems", () => { isLoading: false, diff: expect.stringContaining("loaded.ts"), }); - expect(getDefaultReviewSectionId(items)).toBe("turn:2"); + expect(getDefaultReviewSectionId(items)).toBe("git:branch-range"); }); - it("shows dirty worktree while git preview is loading", () => { + it("falls back to the first turn without git sections", () => { + const items = buildReviewSectionItems({ + checkpoints: [ + makeCheckpoint({ + runId: RunId.make("run-1"), + checkpointTurnCount: 1, + completedAt: "2026-04-01T00:00:00.000Z", + }), + ], + gitSections: [], + turnDiffById: {}, + loadingTurnIds: {}, + loadingGitSections: false, + }); + + expect(getDefaultReviewSectionId(items)).toBe("turn:1"); + }); + + it("holds the Changes place while git preview is loading", () => { const items = buildReviewSectionItems({ checkpoints: [], gitSections: [], @@ -119,15 +137,14 @@ describe("buildReviewSectionItems", () => { expect(items).toEqual([ expect.objectContaining({ - id: "git:working-tree", - kind: "working-tree", - title: "Dirty worktree", - subtitle: "Tracked, staged, and untracked worktree changes", + id: "git:branch-range", + kind: "branch-range", + title: "Changes", diff: null, isLoading: true, }), ]); - expect(getDefaultReviewSectionId(items)).toBe("git:working-tree"); + expect(getDefaultReviewSectionId(items)).toBe("git:branch-range"); }); }); diff --git a/apps/mobile/src/features/review/reviewModel.ts b/apps/mobile/src/features/review/reviewModel.ts index aa935bd287104f097fc7b2de14c0185214fa17ef..86ded9a6264e8a86123f77278385cb969699fbd2 100644 --- a/apps/mobile/src/features/review/reviewModel.ts +++ b/apps/mobile/src/features/review/reviewModel.ts @@ -9,9 +9,9 @@ import * as Order from "effect/Order"; export type ReviewSectionKind = "turn" | "working-tree" | "branch-range"; -const DIRTY_WORKTREE_SECTION_ID = "git:working-tree"; -const DIRTY_WORKTREE_TITLE = "Dirty worktree"; -const DIRTY_WORKTREE_SUBTITLE = "Tracked, staged, and untracked worktree changes"; +const CHANGES_SECTION_ID = "git:branch-range"; +const CHANGES_TITLE = "Changes"; +const UNCOMMITTED_SUBTITLE = "Staged, unstaged, and untracked files"; export interface ReviewSectionItem { readonly id: string; @@ -124,7 +124,7 @@ const readyCheckpointOrder = Order.make( function gitSubtitle(section: ReviewDiffPreviewSource): string | null { if (section.kind === "working-tree") { - return DIRTY_WORKTREE_SUBTITLE; + return UNCOMMITTED_SUBTITLE; } if (section.baseRef) { return `${section.baseRef} ... ${section.headRef ?? "HEAD"}`; @@ -443,15 +443,16 @@ export function buildReviewSectionItems(input: { truncated: section.truncated, isLoading: false, })); - const hasDirtyWorktreeItem = gitItems.some((item) => item.id === DIRTY_WORKTREE_SECTION_ID); + // Changes is the default section, so it holds the place while git sources load. + const hasChangesItem = gitItems.some((item) => item.id === CHANGES_SECTION_ID); const visibleGitItems = - input.loadingGitSections && !hasDirtyWorktreeItem + input.loadingGitSections && !hasChangesItem ? [ { - id: DIRTY_WORKTREE_SECTION_ID, - kind: "working-tree", - title: DIRTY_WORKTREE_TITLE, - subtitle: DIRTY_WORKTREE_SUBTITLE, + id: CHANGES_SECTION_ID, + kind: "branch-range", + title: CHANGES_TITLE, + subtitle: null, diff: null, isLoading: true, } satisfies ReviewSectionItem, @@ -462,10 +463,11 @@ export function buildReviewSectionItems(input: { return [...turnItems, ...visibleGitItems]; } +/** Prefers Changes, then the first section (a turn when the project is not a git repo). */ export function getDefaultReviewSectionId( sections: ReadonlyArray, ): string | null { - return sections[0]?.id ?? null; + return (sections.find((section) => section.id === CHANGES_SECTION_ID) ?? sections[0])?.id ?? null; } export function buildReviewParsedDiff( diff --git a/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx b/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx index 831ef1145cec2747285d57490777a30f0506020c..17b1b3ea377c74f509fe30a7b14b14dd605c14e2 100644 --- a/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx +++ b/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx @@ -282,7 +282,7 @@ export function GitOverviewSheet(props: GitOverviewSheetProps) { { const params = { environmentId, threadId }; diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index d2a244e40f05de15b34791cbfb12c6996912943c..2b88cd79f8e683355c23a8281717627f82228faf 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -1000,7 +1000,7 @@ export const make = Effect.gen(function* () { const canonicalizeExistingPath = (value: string) => fileSystem.realPath(value).pipe(Effect.orElseSucceed(() => value)); const normalizeStatusCacheKey = canonicalizeExistingPath; - const nonRepositoryStatusDetails = { + const nonRepositoryStatusDetails: GitVcsDriver.GitStatusDetails = { isRepo: false, hasOriginRemote: false, isDefaultBranch: false, @@ -1012,10 +1012,10 @@ export const make = Effect.gen(function* () { aheadCount: 0, behindCount: 0, aheadOfDefaultCount: 0, - } satisfies GitVcsDriver.GitStatusDetails; + }; const readLocalStatus = Effect.fn("readLocalStatus")(function* (cwd: string) { const details = yield* gitCore - .statusDetailsLocal(cwd, { includeDivergence: false }) + .statusDetailsLocal(cwd, { includeDivergence: false, includeBranchChanges: true }) .pipe( Effect.catchIf(isNotGitRepositoryError, () => Effect.succeed(nonRepositoryStatusDetails)), ); @@ -1031,6 +1031,7 @@ export const make = Effect.gen(function* () { refName: details.branch, hasWorkingTreeChanges: details.hasWorkingTreeChanges, workingTree: details.workingTree, + ...(details.branchChanges ? { branchChanges: details.branchChanges } : {}), } satisfies VcsStatusLocalResult; }); const localStatusResultCache = yield* Cache.makeWith(readLocalStatus, { diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index d7e511afa8c908f60e40809a4932e8b4d163d994..99220a8bf78f63e56c104f73346e6653db9d6b4c 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -76,6 +76,7 @@ export interface GitStatusDetails { upstreamRef: string | null; hasWorkingTreeChanges: boolean; workingTree: VcsStatusResult["workingTree"]; + branchChanges?: VcsStatusResult["branchChanges"]; hasUpstream: boolean; aheadCount: number; behindCount: number; @@ -85,6 +86,8 @@ export interface GitStatusDetails { export interface GitLocalStatusOptions { /** Skip revision walks and return zero divergence counts for local-only consumers. */ readonly includeDivergence?: boolean; + /** Also read the diff panel's Changes totals. Failures leave them out. */ + readonly includeBranchChanges?: boolean; } export interface GitRemoteStatusDetails { diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 32dc3441dd6a52ef58e33ca1baf55d67fd078dc5..fec77d110dbb44bd1464e18789a31a60468121de 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -26,6 +26,7 @@ import { GitCommandError, ReviewDiffPreviewInput, type ReviewDiffFileContentsInput, + type ReviewDiffPreviewResult, type WorktreeSubmodules, } from "@t3tools/contracts"; import * as ServerConfig from "../config.ts"; @@ -1128,6 +1129,7 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { Effect.gen(function* () { const cwd = yield* makeTmpDir(); const { initialBranch } = yield* initRepoWithCommit(cwd); + const mergeBase = yield* git(cwd, ["rev-parse", "HEAD"]); yield* writeTextFile(cwd, "untracked.txt", "untracked content\n"); const paths = Array.from({ length: 5000 }, (_, index) => `${"a".repeat(220)}-${index}.txt`); const stats = paths.map((path) => `1\t0\t${path}\0`).join(""); @@ -1138,10 +1140,7 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { const delegate = yield* ChildProcessSpawner.ChildProcessSpawner; const spawner = ChildProcessSpawner.make((command) => { if (ChildProcess.isStandardCommand(command)) { - if ( - command.args.includes("--numstat") && - command.args.includes(`${initialBranch}...HEAD`) - ) { + if (command.args.includes("--numstat") && command.args.includes(mergeBase)) { return Effect.succeed(makeSuccessfulHandle(stats)); } if (command.args.includes("ls-files") && command.args.includes("--others")) { @@ -1566,6 +1565,7 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { assert.deepStrictEqual(branch.files, [ { path: "a-large.txt", previousPath: null, additions: 4000, deletions: 0 }, + { path: "untracked.txt", previousPath: null, additions: 4000, deletions: 0 }, { path: "z-last.txt", previousPath: null, additions: 1, deletions: 0 }, ]); assert.deepStrictEqual(dirty.files, [ @@ -1740,7 +1740,7 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { }), ); - it.effect("loads merge-base and head contents for branch diff expansion", () => + it.effect("loads merge-base and disk contents for Changes expansion", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); const { initialBranch } = yield* initRepoWithCommit(cwd); @@ -1749,6 +1749,7 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { yield* writeTextFile(cwd, "README.md", "# branch change\nunchanged context\n"); yield* git(cwd, ["add", "README.md"]); yield* git(cwd, ["commit", "-m", "change readme"]); + yield* writeTextFile(cwd, "README.md", "# dirty change\nunchanged context\n"); const contents = yield* driver.getReviewDiffFileContents( makeReviewDiffFileContentsInput(cwd, { @@ -1759,12 +1760,184 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { ); assert.strictEqual(contents.oldContents, "# test\n"); - assert.strictEqual(contents.newContents, "# branch change\nunchanged context\n"); + assert.strictEqual(contents.newContents, "# dirty change\nunchanged context\n"); + }), + ); + + it.effect("Changes combines commits, uncommitted edits, and untracked files", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + const sourceFiles = ( + preview: ReviewDiffPreviewResult, + kind: "working-tree" | "branch-range", + ) => preview.sources.find((source) => source.kind === kind)!.files; + yield* git(cwd, ["checkout", "-b", "feature/combined"]); + yield* writeTextFile(cwd, "README.md", "# test\ncommitted\n"); + yield* git(cwd, ["commit", "-am", "commit edit"]); + + // A fully committed branch has no uncommitted work, but Changes still shows it. + const clean = yield* driver.getReviewDiffPreview({ cwd, baseRef: initialBranch }); + assert.deepStrictEqual(sourceFiles(clean, "working-tree"), []); + assert.deepStrictEqual(sourceFiles(clean, "branch-range"), [ + { path: "README.md", previousPath: null, additions: 1, deletions: 0 }, + ]); + + yield* writeTextFile(cwd, "README.md", "# test\ncommitted\ndirty\n"); + yield* writeTextFile(cwd, "new.txt", "new\n"); + const dirty = yield* driver.getReviewDiffPreview({ cwd, baseRef: initialBranch }); + const changes = dirty.sources.find((source) => source.kind === "branch-range")!; + assert.deepStrictEqual(changes.files, [ + { path: "README.md", previousPath: null, additions: 2, deletions: 0 }, + { path: "new.txt", previousPath: null, additions: 1, deletions: 0 }, + ]); + assert.strictEqual(changes.diff.match(/^diff --git a\/README\.md /gm)?.length, 1); + assert.include(changes.diff, "+dirty"); + assert.deepStrictEqual(sourceFiles(dirty, "working-tree"), [ + { path: "README.md", previousPath: null, additions: 1, deletions: 0 }, + { path: "new.txt", previousPath: null, additions: 1, deletions: 0 }, + ]); + }), + ); + + it.effect("Changes on the default branch compares with its remote copy", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const remote = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* git(cwd, ["branch", "-M", "main"]); + yield* writeTextFile(cwd, "unpushed.txt", "unpushed\n"); + yield* git(cwd, ["add", "unpushed.txt"]); + yield* git(cwd, ["commit", "-m", "unpushed"]); + + // Without a remote there is no base, so Changes equals Uncommitted. + const local = yield* driver.getReviewDiffPreview({ cwd }); + const localChanges = local.sources.find((source) => source.kind === "branch-range")!; + assert.isNull(localChanges.baseRef); + assert.deepStrictEqual(localChanges.files, []); + + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "origin", "HEAD~1:refs/heads/main"]); + yield* git(cwd, ["fetch", "origin"]); + const preview = yield* driver.getReviewDiffPreview({ cwd }); + const changes = preview.sources.find((source) => source.kind === "branch-range")!; + assert.strictEqual(changes.baseRef, "origin/main"); + assert.deepStrictEqual(changes.files, [ + { path: "unpushed.txt", previousPath: null, additions: 1, deletions: 0 }, + ]); + }), + ); + + it.effect("Changes accepts an explicit base on a detached HEAD and rejects a bad one", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* git(cwd, ["checkout", "--detach"]); + yield* writeTextFile(cwd, "detached.txt", "detached\n"); + yield* git(cwd, ["add", "detached.txt"]); + yield* git(cwd, ["commit", "-m", "detached work"]); + + const explicit = yield* driver.getReviewDiffPreview({ cwd, baseRef: initialBranch }); + assert.deepStrictEqual( + explicit.sources.find((source) => source.kind === "branch-range")!.files, + [{ path: "detached.txt", previousPath: null, additions: 1, deletions: 0 }], + ); + const implicit = yield* driver.getReviewDiffPreview({ cwd }); + assert.deepStrictEqual( + implicit.sources.find((source) => source.kind === "branch-range")!.files, + [], + ); + const error = yield* driver + .getReviewDiffPreview({ cwd, baseRef: "missing-base" }) + .pipe(Effect.flip); + assert.strictEqual(error.operation, "GitVcsDriver.resolveReviewMergeBase"); + }), + ); + + it.effect("Changes compares with the empty tree before the first commit", () => + Effect.gen(function* () { + const upstream = yield* makeTmpDir(); + yield* initRepoWithCommit(upstream); + yield* git(upstream, ["branch", "-M", "main"]); + const cwd = yield* makeTmpDir(); + const driver = yield* GitVcsDriver.GitVcsDriver; + // An unborn main whose remote copy exists must not fail on merge-base. + yield* git(cwd, ["init", "-b", "main"]); + yield* git(cwd, ["remote", "add", "origin", upstream]); + yield* git(cwd, ["fetch", "origin"]); + yield* writeTextFile(cwd, "new.txt", "one\ntwo\n"); + + const preview = yield* driver.getReviewDiffPreview({ cwd }); + const changes = preview.sources.find((source) => source.kind === "branch-range")!; + assert.isNull(changes.baseRef); + assert.deepStrictEqual(changes.files, [ + { path: "new.txt", previousPath: null, additions: 2, deletions: 0 }, + ]); + const status = yield* driver.statusDetailsLocal(cwd, { includeBranchChanges: true }); + assert.deepStrictEqual(status.branchChanges, { + baseRef: null, + insertions: 2, + deletions: 0, + }); + }), + ); + + it.effect("Changes does not show newer base commits as deletions", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* git(cwd, ["checkout", "-b", "feature/rebased"]); + yield* writeTextFile(cwd, "feature.txt", "feature\n"); + yield* git(cwd, ["add", "feature.txt"]); + yield* git(cwd, ["commit", "-m", "feature work"]); + yield* git(cwd, ["checkout", initialBranch]); + yield* writeTextFile(cwd, "upstream.txt", "upstream\n"); + yield* git(cwd, ["add", "upstream.txt"]); + yield* git(cwd, ["commit", "-m", "base moves"]); + yield* git(cwd, ["checkout", "feature/rebased"]); + + for (const rebase of [false, true]) { + if (rebase) yield* git(cwd, ["rebase", initialBranch]); + const preview = yield* driver.getReviewDiffPreview({ cwd, baseRef: initialBranch }); + assert.deepStrictEqual( + preview.sources.find((source) => source.kind === "branch-range")!.files, + [{ path: "feature.txt", previousPath: null, additions: 1, deletions: 0 }], + ); + } }), ); }); describe("repository status", () => { + it.effect("reads Changes totals with untracked files when requested", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + const pathService = yield* Path.Path; + yield* git(cwd, ["branch", "-M", "main"]); + yield* git(cwd, ["checkout", "-b", "feature/totals"]); + yield* writeTextFile(cwd, "README.md", "# test\ncommitted\n"); + yield* git(cwd, ["commit", "-am", "commit edit"]); + yield* writeTextFile(cwd, "nested/untracked.txt", "one\ntwo\n"); + + const status = yield* driver.statusDetailsLocal(pathService.join(cwd, "nested"), { + includeBranchChanges: true, + }); + assert.deepStrictEqual(status.branchChanges, { + baseRef: "main", + insertions: 3, + deletions: 0, + }); + assert.isUndefined((yield* driver.statusDetailsLocal(cwd)).branchChanges); + }), + ); + it.effect("reports non-repository directories without failing", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 61c84424219bad9e4426ce4bd156e23b278e3a4f..971d0e7a9a95869111f0f28c2a741931e5324825 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -62,6 +62,16 @@ const REVIEW_DIFF_FILE_MAX_OUTPUT_BYTES = 1024 * 1024; // prefixes. A repository or global diff.noprefix or diff.mnemonicPrefix would // otherwise leak into the patch and leave every parsed file unnamed. export const PATCH_RENDER_PREFIX_ARGS = ["--src-prefix=a/", "--dst-prefix=b/"] as const; +// Shared by review previews and status totals, so the Changes row matches the Changes view. +const REVIEW_DIFF_ARGS = [ + "diff", + "--find-renames", + "--no-color", + "--no-ext-diff", + "--no-textconv", + "--minimal", + ...PATCH_RENDER_PREFIX_ARGS, +]; const STATUS_UPSTREAM_REFRESH_INTERVAL = Duration.seconds(15); const STATUS_UPSTREAM_REFRESH_TIMEOUT = Duration.seconds(5); @@ -1518,9 +1528,11 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return remoteName; }); + // `allowRemoteOfCurrent` lets the review diff compare the default branch with its remote copy. const resolveBaseBranchForNoUpstream = Effect.fn("resolveBaseBranchForNoUpstream")(function* ( cwd: string, refName: string, + options?: { readonly allowRemoteOfCurrent?: boolean }, ) { const configuredBaseBranch = yield* runGitStdout( "GitVcsDriver.resolveBaseBranchForNoUpstream.config", @@ -1552,7 +1564,21 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* : remotePrefix && candidate.startsWith(remotePrefix) ? candidate.slice(remotePrefix.length) : candidate; - if (normalizedCandidate.length === 0 || normalizedCandidate === refName) { + if (normalizedCandidate.length === 0) { + continue; + } + if (normalizedCandidate === refName) { + if ( + options?.allowRemoteOfCurrent && + primaryRemoteName && + (yield* remoteBranchExists({ + cwd, + remoteName: primaryRemoteName, + refName: normalizedCandidate, + })) + ) { + return `${primaryRemoteName}/${normalizedCandidate}`; + } continue; } @@ -1923,6 +1949,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } files.sort((a, b) => a.path.localeCompare(b.path)); + const branchChanges = options?.includeBranchChanges + ? yield* readBranchChangeTotals(repositoryPaths?.worktreeRoot ?? cwd, refName).pipe( + Effect.orElseSucceed(() => undefined), + ) + : undefined; + return { isRepo: true, hasOriginRemote: hasPrimaryRemote, @@ -1935,6 +1967,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* insertions, deletions, }, + ...(branchChanges ? { branchChanges } : {}), hasUpstream: upstreamRef !== null, aheadCount, behindCount, @@ -2413,6 +2446,145 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return env; }); + // Before the first commit, review diffs compare with the empty tree instead of HEAD. + const readEmptyTreeHash = Effect.fn("readEmptyTreeHash")(function* (cwd: string) { + const stdout = yield* runGitStdout("GitVcsDriver.review.emptyTree", cwd, [ + "hash-object", + "-t", + "tree", + (yield* HostProcessPlatform) === "win32" ? "NUL" : "/dev/null", + ]); + return stdout.trim(); + }); + + // Lists untracked files and adds them to a temporary index, so a diff against any commit + // shows them as new. Returns null when the list is too big to read. Needs a Scope. + const prepareUntrackedReviewIndex = Effect.fn("prepareUntrackedReviewIndex")(function* ( + cwd: string, + pathArgs: ReadonlyArray, + onlyPath?: string, + ) { + const untracked = yield* executeGit( + "GitVcsDriver.review.listUntracked", + cwd, + ["ls-files", "--others", "--exclude-standard", "-z", "--", ...pathArgs], + { maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES }, + ).pipe( + Effect.catchIf( + (error) => error.outputLength === undefined, + () => Effect.succeed(null), + ), + ); + if (untracked === null) return null; + const paths = splitNullSeparatedGitStdoutPaths(untracked).filter( + (candidate) => onlyPath === undefined || candidate === onlyPath, + ); + if (paths.length === 0) return { env: undefined }; + const env = yield* prepareReviewIndex(cwd, paths).pipe( + Effect.catchTags({ + PlatformError: (cause) => + Effect.fail( + new GitCommandError({ + operation: "GitVcsDriver.prepareReviewIndex", + cwd, + command: "git diff", + detail: "Could not prepare the review index.", + cause, + }), + ), + }), + ); + return { env }; + }); + + // The diff panel's Changes view compares the working tree with merge-base(base, HEAD). + // With no usable base it compares with HEAD, so Changes equals Uncommitted. + const resolveReviewMergeBase = Effect.fn("resolveReviewMergeBase")(function* ( + cwd: string, + branch: string | null, + explicitBaseRef?: string, + ) { + const baseRef = + explicitBaseRef ?? + (branch + ? yield* resolveBaseBranchForNoUpstream(cwd, branch, { allowRemoteOfCurrent: true }).pipe( + Effect.orElseSucceed(() => null), + ) + : null); + if (baseRef === null) return { baseRef, mergeBase: "HEAD" }; + const args = ["merge-base", baseRef, "HEAD"]; + const result = yield* executeGit("GitVcsDriver.resolveReviewMergeBase", cwd, args, { + allowNonZeroExit: true, + }); + const mergeBase = result.stdout.trim(); + if (result.exitCode !== 0 || mergeBase.length === 0) { + // Before the first commit there is nothing to compare with, so Changes equals Uncommitted. + const head = yield* executeGit( + "GitVcsDriver.resolveReviewMergeBase.head", + cwd, + ["rev-parse", "--verify", "--quiet", "HEAD"], + { allowNonZeroExit: true }, + ); + if (explicitBaseRef === undefined && head.exitCode !== 0) { + return { baseRef: null, mergeBase: "HEAD" }; + } + return yield* new GitCommandError({ + ...gitCommandContext({ operation: "GitVcsDriver.resolveReviewMergeBase", cwd, args }), + detail: `Could not find a common commit between '${baseRef}' and HEAD.`, + exitCode: result.exitCode, + }); + } + return { baseRef, mergeBase }; + }); + + // Totals for the thread panel's Changes row. Same base and untracked files as the Changes view. + const readBranchChangeTotals = Effect.fn("readBranchChangeTotals")(function* ( + cwd: string, + branch: string | null, + ) { + const { baseRef, mergeBase } = yield* resolveReviewMergeBase(cwd, branch); + const untracked = yield* prepareUntrackedReviewIndex(cwd, []); + if (untracked === null) { + return yield* new GitCommandError({ + operation: "GitVcsDriver.readBranchChangeTotals", + command: "git ls-files", + cwd, + detail: "Too many untracked files to count.", + }); + } + const readNumstat = (ref: string) => + executeGit( + "GitVcsDriver.readBranchChangeTotals", + cwd, + [...REVIEW_DIFF_ARGS, "--numstat", "-z", ref, "--"], + { + allowNonZeroExit: true, + maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES, + env: untracked.env, + }, + ); + let result = yield* readNumstat(mergeBase); + if (result.exitCode !== 0 && mergeBase === "HEAD" && isUnbornHeadStderr(result.stderr)) { + result = yield* readNumstat(yield* readEmptyTreeHash(cwd)); + } + if (result.exitCode !== 0) { + return yield* new GitCommandError({ + operation: "GitVcsDriver.readBranchChangeTotals", + command: "git diff --numstat", + cwd, + detail: "Could not read Changes totals.", + exitCode: result.exitCode, + }); + } + let insertions = 0; + let deletions = 0; + for (const file of parseReviewNumstat(result.stdout)) { + insertions += file.additions; + deletions += file.deletions; + } + return { baseRef, insertions, deletions }; + }, Effect.scoped); + const getReviewDiffPreview = Effect.fn("getReviewDiffPreview")(function* ( input: ReviewDiffPreviewInput, ) { @@ -2439,21 +2611,15 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } const cwd = repository.worktreeRoot; - const branch = repository.currentBranch; - const baseRef = - input.baseRef ?? - (branch - ? yield* resolveBaseBranchForNoUpstream(cwd, branch).pipe(Effect.orElseSucceed(() => null)) - : null); + // A per-file request only reads its own source. + const dirtyRef = input.file?.sourceKind === "branch-range" ? null : "HEAD"; + const review = + input.file?.sourceKind === "working-tree" + ? { baseRef: input.baseRef ?? null, mergeBase: null } + : yield* resolveReviewMergeBase(cwd, repository.currentBranch, input.baseRef); const diffArgs = [ - "diff", - "--find-renames", - "--no-color", - "--no-ext-diff", - "--no-textconv", - "--minimal", - ...PATCH_RENDER_PREFIX_ARGS, + ...REVIEW_DIFF_ARGS, ...(input.ignoreWhitespace ? ["--ignore-all-space"] : []), ]; const readStats = Effect.fn("GitVcsDriver.getReviewDiffPreview.stat")(function* ( @@ -2469,12 +2635,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ); if (result.exitCode === 0) return { ref, files: parseReviewNumstat(result.stdout) }; if (ref === "HEAD" && isUnbornHeadStderr(result.stderr)) { - const emptyTree = (yield* runGitStdout("GitVcsDriver.getReviewDiffPreview.emptyTree", cwd, [ - "hash-object", - "-t", - "tree", - (yield* HostProcessPlatform) === "win32" ? "NUL" : "/dev/null", - ])).trim(); + const emptyTree = yield* readEmptyTreeHash(cwd); const stdout = yield* runGitStdoutWithOptions( "GitVcsDriver.getReviewDiffPreview.unbornStat", cwd, @@ -2491,6 +2652,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* exitCode: result.exitCode, }); }); + // One commit argument diffs that commit against the working tree. const readTrackedDiff = Effect.fn("GitVcsDriver.getReviewDiffPreview.tracked")(function* ( ref: string | null, env?: NodeJS.ProcessEnv, @@ -2506,54 +2668,27 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ); return { ...patch, files: stat.files }; }); - const readDirty = Effect.gen(function* () { - if (input.file?.sourceKind === "branch-range") return yield* readTrackedDiff(null); - const untracked = yield* executeGit( - "GitVcsDriver.review.listUntracked", - cwd, - ["ls-files", "--others", "--exclude-standard", "-z", "--", ...pathArgs], - { maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES }, - ).pipe( - Effect.catchIf( - (error) => error.outputLength === undefined, - () => Effect.succeed(null), - ), - ); - if (untracked === null) { - const tracked = yield* readTrackedDiff("HEAD"); - return { ...tracked, files: undefined, stdoutTruncated: true }; - } - const paths = splitNullSeparatedGitStdoutPaths(untracked).filter( - (candidate) => !input.file || candidate === input.file.path, - ); - if (paths.length === 0) return yield* readTrackedDiff("HEAD"); - const env = yield* prepareReviewIndex(cwd, paths).pipe( - Effect.catchTags({ - PlatformError: (cause) => - Effect.fail( - new GitCommandError({ - operation: "GitVcsDriver.prepareReviewIndex", - cwd, - command: "git diff", - detail: "Could not prepare the review index.", - cause, - }), - ), - }), - ); - return yield* readTrackedDiff("HEAD", env); + const [dirtyTrackedResult, baseResult] = yield* Effect.gen(function* () { + const untracked = yield* prepareUntrackedReviewIndex(cwd, pathArgs, input.file?.path); + // With no base both sources diff HEAD, so read it once. + const [dirty, base] = + review.mergeBase === dirtyRef + ? yield* readTrackedDiff(dirtyRef, untracked?.env).pipe( + Effect.map((result) => [result, result] as const), + ) + : yield* Effect.all( + [ + readTrackedDiff(dirtyRef, untracked?.env), + readTrackedDiff(review.mergeBase, untracked?.env), + ], + { concurrency: 2 }, + ); + if (untracked !== null) return [dirty, base] as const; + // Too many untracked files to list: show tracked changes and mark totals incomplete. + const incomplete = (ref: string | null, result: typeof dirty) => + ref === null ? result : { ...result, files: undefined, stdoutTruncated: true }; + return [incomplete(dirtyRef, dirty), incomplete(review.mergeBase, base)] as const; }).pipe(Effect.scoped); - const [dirtyTrackedResult, baseResult] = yield* Effect.all( - [ - readDirty, - readTrackedDiff( - baseRef && branch && input.file?.sourceKind !== "working-tree" - ? `${baseRef}...HEAD` - : null, - ), - ], - { concurrency: 2 }, - ); const dirtyFiles = dirtyTrackedResult.files; const baseFiles = baseResult.files; const dirtyDiff = dirtyTrackedResult.stdout; @@ -2574,14 +2709,14 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ); const [dirtyDiffHash, baseDiffHash] = yield* Effect.all([ hashDiff(dirtyDiff, dirtyFiles ?? []), - hashDiff(baseDiff, baseFiles), + hashDiff(baseDiff, baseFiles ?? []), ]); const sources: ReviewDiffPreviewSource[] = [ { id: "working-tree", kind: "working-tree", - title: "Dirty worktree", + title: "Uncommitted", baseRef: "HEAD", headRef: null, diff: dirtyDiff, @@ -2592,11 +2727,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* { id: "branch-range", kind: "branch-range", - title: baseRef ? `Against ${baseRef}` : "Against base branch", - baseRef, - headRef: branch ?? "HEAD", + title: review.baseRef ? `Changes vs ${review.baseRef}` : "Changes", + baseRef: review.baseRef, + // For display only. The new side is the working tree. + headRef: repository.currentBranch ?? "HEAD", diff: baseDiff, - files: baseFiles, + ...(baseFiles === undefined ? {} : { files: baseFiles }), diffHash: baseDiffHash, truncated: baseResult.stdoutTruncated, }, @@ -2712,54 +2848,31 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return new TextDecoder("utf-8").decode(bytes); }); + // Both views compare a commit with the working tree: HEAD for Uncommitted, + // merge-base(base, HEAD) for Changes. The new side always comes from disk. const getReviewDiffFileContents = Effect.fn("getReviewDiffFileContents")(function* ( input: ReviewDiffFileContentsInput, ) { - if (input.sourceKind === "working-tree") { - const repositoryRoot = yield* runGitStdout( - "GitVcsDriver.getReviewDiffFileContents.repositoryRoot", - input.cwd, - ["rev-parse", "--show-toplevel"], - ).pipe(Effect.map((value) => value.trim())); - if (repositoryRoot.length === 0) { - return yield* reviewDiffFileError(input, "Could not resolve the Git repository root."); - } - const [oldContents, newContents] = yield* Effect.all( - [ - input.changeType === "new" - ? Effect.succeed("") - : readReviewFileAtRevision(input, input.baseRef ?? "HEAD", input.oldPath), - input.changeType === "deleted" - ? Effect.succeed("") - : readWorkingTreeReviewFile(input, repositoryRoot), - ], - { concurrency: 2 }, - ); - return { oldContents, newContents }; - } - - if (!input.baseRef || !input.headRef) { - return yield* reviewDiffFileError( - input, - "Branch diff file expansion requires both base and head refs.", - ); - } - const mergeBase = yield* runGitStdout( - "GitVcsDriver.getReviewDiffFileContents.mergeBase", + const repositoryRoot = yield* runGitStdout( + "GitVcsDriver.getReviewDiffFileContents.repositoryRoot", input.cwd, - ["merge-base", input.baseRef, input.headRef], + ["rev-parse", "--show-toplevel"], ).pipe(Effect.map((value) => value.trim())); - if (mergeBase.length === 0) { - return yield* reviewDiffFileError(input, "Could not resolve the branch comparison base."); + if (repositoryRoot.length === 0) { + return yield* reviewDiffFileError(input, "Could not resolve the Git repository root."); } + const oldRevision = + input.sourceKind === "working-tree" + ? (input.baseRef ?? "HEAD") + : (yield* resolveReviewMergeBase(input.cwd, null, input.baseRef ?? undefined)).mergeBase; const [oldContents, newContents] = yield* Effect.all( [ input.changeType === "new" ? Effect.succeed("") - : readReviewFileAtRevision(input, mergeBase, input.oldPath), + : readReviewFileAtRevision(input, oldRevision, input.oldPath), input.changeType === "deleted" ? Effect.succeed("") - : readReviewFileAtRevision(input, input.headRef, input.newPath), + : readWorkingTreeReviewFile(input, repositoryRoot), ], { concurrency: 2 }, ); diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts index ac21a61ddf7894d8a4b146bc2ba5f195540f19d0..24a7657d0e274ab34df5c6b605dd21f4d9318d0b 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts @@ -78,6 +78,8 @@ function makeTestLayer(state: { remoteInvalidationCalls: number; remoteStatusRefreshUpstreamValues?: Array; backgroundWorkEnabled?: boolean; + /** Runs before each remote status read, e.g. to hold a fetch open. */ + beforeRemoteStatus?: Effect.Effect; }) { return VcsStatusBroadcaster.layer.pipe( Layer.provideMerge(NodeServices.layer), @@ -90,11 +92,15 @@ function makeTestLayer(state: { return state.currentLocalStatus; }), remoteStatus: (_input, options) => - Effect.sync(() => { - state.remoteStatusCalls += 1; - state.remoteStatusRefreshUpstreamValues?.push(options?.refreshUpstream); - return state.currentRemoteStatus; - }), + Effect.suspend(() => state.beforeRemoteStatus ?? Effect.void).pipe( + Effect.andThen( + Effect.sync(() => { + state.remoteStatusCalls += 1; + state.remoteStatusRefreshUpstreamValues?.push(options?.refreshUpstream); + return state.currentRemoteStatus; + }), + ), + ), invalidateLocalStatus: () => Effect.sync(() => { state.localInvalidationCalls += 1; @@ -847,6 +853,90 @@ describe("VcsStatusBroadcaster", () => { }).pipe(Effect.provide(Layer.merge(makeTestLayer(state), TestClock.layer()))); }); + // A push from a terminal moves ahead; a PR merged on the host moves ahead-of-default. + it.effect.each([ + ["a push", { ...baseRemoteStatus, aheadCount: 1, aheadOfDefaultCount: 0 }], + ["a merged pull request", { ...baseRemoteStatus, aheadOfDefaultCount: 2 }], + ] as const)("re-reads local status when a fetch reflects %s", ([, initialRemote]) => { + const state = { + currentLocalStatus: baseLocalStatus, + currentRemoteStatus: initialRemote as VcsStatusRemoteResult | null, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + }; + + return Effect.gen(function* () { + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + yield* broadcaster.getStatus({ cwd: "/repo" }); + const scope = yield* Scope.make(); + const snapshotDeferred = yield* Deferred.make(); + const localUpdatedDeferred = yield* Deferred.make(); + yield* Stream.runForEach( + broadcaster.streamStatus( + { cwd: "/repo" }, + { automaticRemoteRefreshInterval: Effect.succeed(Duration.minutes(1)) }, + ), + (event) => + event._tag === "snapshot" + ? Deferred.succeed(snapshotDeferred, event).pipe(Effect.ignore) + : event._tag === "localUpdated" + ? Deferred.succeed(localUpdatedDeferred, event).pipe(Effect.ignore) + : Effect.void, + ).pipe(Effect.forkIn(scope)); + yield* Deferred.await(snapshotDeferred); + + // The next fetch moves the base, so the Changes totals shrink. + const pushedLocal: VcsStatusLocalResult = { + ...baseLocalStatus, + branchChanges: { baseRef: "origin/main", insertions: 0, deletions: 0 }, + }; + state.currentLocalStatus = pushedLocal; + state.currentRemoteStatus = { ...baseRemoteStatus, aheadOfDefaultCount: 0 }; + yield* TestClock.adjust(Duration.minutes(1)); + + assert.deepStrictEqual(yield* Deferred.await(localUpdatedDeferred), { + _tag: "localUpdated", + local: pushedLocal, + } satisfies VcsStatusStreamEvent); + yield* Scope.close(scope, Exit.void); + }).pipe(Effect.provide(Layer.merge(makeTestLayer(state), TestClock.layer()))); + }); + + it.effect("an explicit refresh reads local totals after the fetch", () => { + const state: Parameters[0] = { + currentLocalStatus: baseLocalStatus, + currentRemoteStatus: baseRemoteStatus, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + }; + + return Effect.gen(function* () { + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + const fetchStarted = yield* Deferred.make(); + const finishFetch = yield* Deferred.make(); + state.beforeRemoteStatus = Deferred.succeed(fetchStarted, undefined).pipe( + Effect.andThen(Deferred.await(finishFetch)), + ); + const refresh = yield* broadcaster.refreshStatus("/repo").pipe(Effect.forkScoped); + yield* Deferred.await(fetchStarted); + + // The fetch moves the base while it runs, so totals read before it ends would be stale. + const fetchedLocal: VcsStatusLocalResult = { + ...baseLocalStatus, + branchChanges: { baseRef: "origin/main", insertions: 0, deletions: 0 }, + }; + state.currentLocalStatus = fetchedLocal; + yield* Deferred.succeed(finishFetch, undefined); + + const status = yield* Fiber.join(refresh); + assert.deepStrictEqual(status.branchChanges, fetchedLocal.branchChanges); + }).pipe(Effect.provide(makeTestLayer(state))); + }); + it("backs off remote refresh failures exponentially and honors larger configured intervals", () => { assert.equal( Duration.toMillis(VcsStatusBroadcaster.remoteRefreshFailureDelay(1, Duration.seconds(1))), diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.ts b/apps/server/src/vcs/VcsStatusBroadcaster.ts index 40f0e1bc8685c447f2d5a1d7c0ffdb4ff1a64531..118e2bd29807dce1dc43808a2fc3a25176171401 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.ts @@ -462,9 +462,22 @@ export const make = Effect.gen(function* () { if (options?.refreshUpstream !== false) { yield* workflow.invalidateRemoteStatus(cwd); } + const previousRemote = (yield* getCachedStatus(cwd))?.remote?.value; const remote = yield* workflow.remoteStatus({ cwd }, options); const pulled = yield* maybeAutoPull(cwd, remote, options?.policyCwds ?? [cwd]); if (pulled !== null) return pulled.remote; + // Local status holds the Changes totals, which compare against remote refs. A fetch can + // move them with no local trigger (a push from a terminal, a PR merged on the host), so + // re-read local status on the first fetch and whenever divergence moves. + if ( + remote && + (!previousRemote || + previousRemote.aheadCount !== remote.aheadCount || + previousRemote.behindCount !== remote.behindCount || + previousRemote.aheadOfDefaultCount !== remote.aheadOfDefaultCount) + ) { + yield* refreshLocalStatusCore(cwd); + } return yield* updateCachedRemoteStatus(cwd, remote, { publish: true }); }), ); @@ -480,10 +493,9 @@ export const make = Effect.gen(function* () { cwd, Effect.gen(function* () { yield* workflow.invalidateStatus(cwd); - const [local, remote] = yield* Effect.all( - [workflow.localStatus({ cwd }), workflow.remoteStatus({ cwd })], - { concurrency: "unbounded" }, - ); + // Local after remote: the fetch can move the base that the Changes totals compare with. + const remote = yield* workflow.remoteStatus({ cwd }); + const local = yield* workflow.localStatus({ cwd }); const pulled = yield* maybeAutoPull(cwd, remote, [rawCwd]); if (pulled !== null) return mergeGitStatusParts(pulled.local, pulled.remote); return yield* updateCachedStatus(cwd, local, remote, { publish: true }); diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index d819312a4685a111cec12e49170467369723511a..233878f5de8e9b5750d839308d1f5ed3b24ad158 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -2222,10 +2222,10 @@ export default function ChatView(props: ChatViewProps) { useLayoutEffect(() => { const explicitThreadRef = explicitDiffOpenRef.current; explicitDiffOpenRef.current = null; - // Generic openings always show the checkout, including tab fallbacks and thread changes. + // Generic openings always show Changes, including tab fallbacks and thread changes. // A timeline click instead opens the specific turn/file the user requested. if (diffOpen && activeThreadRef && explicitThreadRef !== activeThreadRef) { - useDiffPanelStore.getState().selectGitScope(activeThreadRef, "unstaged"); + useDiffPanelStore.getState().selectGitScope(activeThreadRef, "branch"); } }, [activeThreadRef, diffOpen]); const rightPanelState = useRightPanelStore((state) => @@ -5023,7 +5023,7 @@ export default function ChatView(props: ChatViewProps) { ); const addDiffSurface = useCallback(() => { if (!activeThreadRef || !isServerThread || !isGitRepo) return; - useDiffPanelStore.getState().selectGitScope(activeThreadRef, "unstaged"); + useDiffPanelStore.getState().selectGitScope(activeThreadRef, "branch"); useRightPanelStore.getState().open(activeThreadRef, "diff"); onDiffPanelOpen?.(); }, [activeThreadRef, isGitRepo, isServerThread, onDiffPanelOpen]); @@ -5378,7 +5378,7 @@ export default function ChatView(props: ChatViewProps) { if (!panels.openProactive(activeThreadRef, { id: "diff", kind: "diff" }, userActionRevision)) { return; } - useDiffPanelStore.getState().selectGitScope(activeThreadRef, "unstaged"); + useDiffPanelStore.getState().selectGitScope(activeThreadRef, "branch"); onDiffPanelOpen?.(); }, [ turnDiffSummaries, diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 7746910f31f1047356f40d10b7336134fb90001a..32f5bfabcbafa00941473cb76100c57df397970f 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -233,8 +233,8 @@ export default function DiffPanel({ const selectedScopeLabel = selectedRunId === null ? selectedGitScope === "unstaged" - ? "Working tree" - : "Branch changes" + ? "Uncommitted" + : "Changes" : selectedTurn?.runId === latestTurn?.runId ? "Latest turn" : `Turn ${selectedCheckpointTurnCount ?? "?"}`; @@ -246,8 +246,8 @@ export default function DiffPanel({ const reviewSectionTitle = selectedTurn ? `Turn ${selectedCheckpointTurnCount ?? "?"}` : selectedGitScope === "unstaged" - ? "Working tree" - : "Branch changes"; + ? "Uncommitted" + : "Changes"; const selectedCheckpointRange = useMemo( () => typeof selectedCheckpointTurnCount === "number" @@ -676,11 +676,11 @@ export default function DiffPanel({ - - Working tree - - Branch changes + Changes + + + Uncommitted Latest turn @@ -1004,8 +1004,8 @@ export default function DiffPanel({ selectedTurn ? "Loading checkpoint diff..." : selectedGitScope === "unstaged" - ? "Loading working tree diff..." - : "Loading branch diff..." + ? "Loading uncommitted changes..." + : "Loading changes..." } /> ) : ( diff --git a/apps/web/src/components/GitActionsControl.tsx b/apps/web/src/components/GitActionsControl.tsx index b1bab01c38c842f502332d065fead1a39523ff0f..9d9d82c6273f46f7da85efd8cd4b7d748c421adf 100644 --- a/apps/web/src/components/GitActionsControl.tsx +++ b/apps/web/src/components/GitActionsControl.tsx @@ -1192,6 +1192,8 @@ export default function GitActionsControl({ const isRepo = gitStatus?.isRepo ?? true; const hasPrimaryRemote = gitStatus?.hasPrimaryRemote ?? false; const gitStatusForActions = gitStatus; + // Matches the diff panel's Changes view. Older servers only report uncommitted totals. + const changesTotals = gitStatusForActions?.branchChanges ?? gitStatusForActions?.workingTree; const allFiles = gitStatusForActions?.workingTree.files ?? []; const selectedFiles = allFiles.filter((f) => !excludedFiles.has(f.path)); @@ -1936,12 +1938,8 @@ export default function GitActionsControl({ Changes - - +{gitStatusForActions?.workingTree.insertions ?? 0} - - - -{gitStatusForActions?.workingTree.deletions ?? 0} - + +{changesTotals?.insertions ?? 0} + -{changesTotals?.deletions ?? 0} ) : null} diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index 2d3442bdbceeabb0228229f7b487724b1bdc1413..8c5a3a935c5a09495ef2bd4282429625debea929 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -2673,7 +2673,7 @@ export function GeneralSettingsPanel() { { }), ); - it("defaults each thread to working tree changes without requiring git status", () => { + it("defaults each thread to Changes without requiring git status", () => { expect( selectThreadDiffPanelSelection(useDiffPanelStore.getState().byThreadKey, THREAD_REF), - ).toEqual({ kind: "unstaged" }); + ).toEqual({ kind: "branch", baseRef: null }); }); - it("defaults to working tree changes before a thread is selected", () => { + it("defaults to Changes before a thread is selected", () => { expect(selectThreadDiffPanelSelection(useDiffPanelStore.getState().byThreadKey, null)).toEqual({ - kind: "unstaged", + kind: "branch", + baseRef: null, }); }); + it("keeps a custom base when a generic open selects Changes again", () => { + const store = useDiffPanelStore.getState(); + store.selectBranchBaseRef(THREAD_REF, "origin/release"); + store.selectGitScope(THREAD_REF, "branch"); + store.selectTurn(THREAD_REF, RunId.make("turn-1")); + store.selectGitScope(THREAD_REF, "branch"); + + expect( + selectThreadDiffPanelSelection(useDiffPanelStore.getState().byThreadKey, THREAD_REF), + ).toEqual({ kind: "branch", baseRef: "origin/release" }); + }); + it("preserves an explicit branch selection", () => { useDiffPanelStore.getState().selectGitScope(THREAD_REF, "branch"); diff --git a/apps/web/src/diffPanelStore.ts b/apps/web/src/diffPanelStore.ts index 1111e68fb8198448595c0c797a9efb7f710d3c2e..534eee5932e8c22581e5050b7391c53a4f999f3f 100644 --- a/apps/web/src/diffPanelStore.ts +++ b/apps/web/src/diffPanelStore.ts @@ -10,7 +10,8 @@ export type DiffPanelSelection = | { kind: "unstaged" } | { kind: "turn"; turnId: RunId; filePath: string | null; revealRequestId: number }; -const DEFAULT_SELECTION: DiffPanelSelection = { kind: "unstaged" }; +// "branch" is the Changes view: everything this checkout changed since its base. +const DEFAULT_SELECTION: DiffPanelSelection = { kind: "branch", baseRef: null }; interface DiffPanelStoreState { byThreadKey: Record; diff --git a/packages/contracts/src/git.ts b/packages/contracts/src/git.ts index 2efd79f5baa25aead1849dc58365ca16d297ba4b..0698529d10c0807adb556e0c4bc248e759e98646 100644 --- a/packages/contracts/src/git.ts +++ b/packages/contracts/src/git.ts @@ -235,6 +235,17 @@ const VcsStatusLocalShape = { insertions: NonNegativeInt, deletions: NonNegativeInt, }), + /** + * Totals for the diff panel's Changes view: merge-base with the base branch to the + * working tree, untracked files included. Absent on older servers. + */ + branchChanges: Schema.optional( + Schema.Struct({ + baseRef: Schema.NullOr(TrimmedNonEmptyStringSchema), + insertions: NonNegativeInt, + deletions: NonNegativeInt, + }), + ), }; const VcsStatusRemoteShape = { diff --git a/packages/shared/src/git.ts b/packages/shared/src/git.ts index e866ac05812e1c90a222304fb708ec498f6656e0..d5866d32e5d10812739178c14a2eb5e0ebc07971 100644 --- a/packages/shared/src/git.ts +++ b/packages/shared/src/git.ts @@ -373,6 +373,7 @@ function toLocalStatusPart(status: VcsStatusResult): VcsStatusLocalResult { refName: status.refName, hasWorkingTreeChanges: status.hasWorkingTreeChanges, workingTree: status.workingTree, + ...(status.branchChanges ? { branchChanges: status.branchChanges } : {}), }; } -- 2.54.0