Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions apps/mobile/src/features/review/ReviewSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
const actions: MenuAction[] = [
sectionAction(sectionMenu.workingTree, "Working tree"),
sectionAction(sectionMenu.branchChanges, "Branch changes"),
sectionAction(sectionMenu.sinceFork, "Since fork"),
sectionAction(sectionMenu.latestTurn, "Latest turn"),
];

Expand Down Expand Up @@ -731,6 +732,17 @@ export function ReviewSheet(props: ReviewSheetProps) {
>
<NativeHeaderToolbar.Label>Branch changes</NativeHeaderToolbar.Label>
</NativeHeaderToolbar.MenuAction>
<NativeHeaderToolbar.MenuAction
disabled={sectionMenu.sinceFork === null}
isOn={selectedSection?.id === sectionMenu.sinceFork?.id}
onPress={() => {
if (sectionMenu.sinceFork) {
selectSection(sectionMenu.sinceFork.id);
}
}}
>
<NativeHeaderToolbar.Label>Since fork</NativeHeaderToolbar.Label>
</NativeHeaderToolbar.MenuAction>
<NativeHeaderToolbar.MenuAction
disabled={sectionMenu.latestTurn === null}
isOn={selectedSection?.id === sectionMenu.latestTurn?.id}
Expand Down
17 changes: 11 additions & 6 deletions apps/mobile/src/features/review/review-section-menu.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,24 @@ describe("buildReviewSectionMenu", () => {
const turn27 = section("turn:27", "turn");
const workingTree = section("git:working-tree", "working-tree");
const branchChanges = section("git:branch-range", "branch-range");
const sinceFork = section("git:since-fork", "since-fork");

expect(buildReviewSectionMenu([turn28, turn27, workingTree, branchChanges])).toEqual({
workingTree,
branchChanges,
latestTurn: turn28,
turns: [turn28, turn27],
});
expect(buildReviewSectionMenu([turn28, turn27, workingTree, branchChanges, sinceFork])).toEqual(
{
workingTree,
branchChanges,
sinceFork,
latestTurn: turn28,
turns: [turn28, turn27],
},
);
});

it("keeps unavailable scopes empty while data loads", () => {
expect(buildReviewSectionMenu([])).toEqual({
workingTree: null,
branchChanges: null,
sinceFork: null,
latestTurn: null,
turns: [],
});
Expand Down
2 changes: 2 additions & 0 deletions apps/mobile/src/features/review/review-section-menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { ReviewSectionItem } from "./reviewModel";
export interface ReviewSectionMenu {
readonly workingTree: ReviewSectionItem | null;
readonly branchChanges: ReviewSectionItem | null;
readonly sinceFork: ReviewSectionItem | null;
readonly latestTurn: ReviewSectionItem | null;
readonly turns: ReadonlyArray<ReviewSectionItem>;
}
Expand All @@ -15,6 +16,7 @@ export function buildReviewSectionMenu(
return {
workingTree: sections.find((section) => section.kind === "working-tree") ?? null,
branchChanges: sections.find((section) => section.kind === "branch-range") ?? null,
sinceFork: sections.find((section) => section.kind === "since-fork") ?? null,
latestTurn: turns[0] ?? null,
turns,
};
Expand Down
6 changes: 4 additions & 2 deletions apps/mobile/src/features/review/reviewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as Arr from "effect/Array";
import { pipe } from "effect/Function";
import * as Order from "effect/Order";

export type ReviewSectionKind = "turn" | "working-tree" | "branch-range";
export type ReviewSectionKind = "turn" | "working-tree" | "branch-range" | "since-fork";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High review/reviewModel.ts:8

A git:since-fork section added by buildReviewSectionItems is unreachable in the mobile UI. buildReviewSectionMenu only extracts working-tree, branch-range, and turn sections, and ReviewSheet only renders actions for those entries, so a since-fork section appears in reviewSections but the user can never select it from the section menu. Consider adding a sinceFork entry to the menu output (and a matching action in ReviewSheet) so the scope is accessible.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/review/reviewModel.ts around line 8:

A `git:since-fork` section added by `buildReviewSectionItems` is unreachable in the mobile UI. `buildReviewSectionMenu` only extracts `working-tree`, `branch-range`, and turn sections, and `ReviewSheet` only renders actions for those entries, so a `since-fork` section appears in `reviewSections` but the user can never select it from the section menu. Consider adding a `sinceFork` entry to the menu output (and a matching action in `ReviewSheet`) so the scope is accessible.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since-fork unreachable on mobile

Medium Severity

The new since-fork review section is available from the server, but mobile's review section menus don't expose this option. Users are unable to select or view the since-fork review scope.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4861e56. Configure here.


const DIRTY_WORKTREE_SECTION_ID = "git:working-tree";
const DIRTY_WORKTREE_TITLE = "Dirty worktree";
Expand Down Expand Up @@ -160,7 +160,9 @@ function gitSubtitle(section: ReviewDiffPreviewSource): string | null {
return DIRTY_WORKTREE_SUBTITLE;
}
if (section.baseRef) {
return `${section.baseRef} ... ${section.headRef ?? "HEAD"}`;
return section.kind === "since-fork"
? `${section.baseRef} ... worktree`
: `${section.baseRef} ... ${section.headRef ?? "HEAD"}`;
}
return "Base branch unavailable";
}
Expand Down
43 changes: 43 additions & 0 deletions apps/server/src/vcs/GitVcsDriverCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,49 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
);
}),
);

it.effect("covers committed and uncommitted work in the since-fork preview", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const { initialBranch } = yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* git(cwd, ["checkout", "-b", "feature/since-fork"]);
yield* writeTextFile(cwd, "committed.txt", "committed\n");
yield* git(cwd, ["add", "committed.txt"]);
yield* git(cwd, ["commit", "-m", "add committed file"]);
yield* writeTextFile(cwd, "README.md", "# dirty tracked edit\n");
yield* writeTextFile(cwd, "untracked.txt", "untracked\n");

const preview = yield* driver.getReviewDiffPreview({ cwd, baseRef: initialBranch });
const sinceFork = preview.sources.find((source) => source.kind === "since-fork");
const branchRange = preview.sources.find((source) => source.kind === "branch-range");

assert.isDefined(sinceFork);
assert.include(sinceFork.diff, "committed.txt");
assert.include(sinceFork.diff, "README.md");
assert.include(sinceFork.diff, "untracked.txt");
assert.strictEqual(sinceFork.baseRef, initialBranch);

// The committed-only view stays narrow so both scopes remain distinguishable.
assert.include(branchRange?.diff ?? "", "committed.txt");
assert.notInclude(branchRange?.diff ?? "", "untracked.txt");
}),
);

it.effect("keeps the since-fork preview empty when no base branch resolves", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* writeTextFile(cwd, "README.md", "# dirty\n");

const preview = yield* driver.getReviewDiffPreview({ cwd, baseRef: "does-not-exist" });
const sinceFork = preview.sources.find((source) => source.kind === "since-fork");

assert.isDefined(sinceFork);
assert.strictEqual(sinceFork.diff, "");
}),
);
});

describe("repository status", () => {
Expand Down
83 changes: 64 additions & 19 deletions apps/server/src/vcs/GitVcsDriverCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2136,6 +2136,14 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
};
});

const EMPTY_GIT_RESULT = {
exitCode: 0,
stdout: "",
stderr: "",
stdoutTruncated: false,
stderrTruncated: false,
};

const getReviewDiffPreview = Effect.fn("getReviewDiffPreview")(function* (
input: ReviewDiffPreviewInput,
) {
Expand Down Expand Up @@ -2175,15 +2183,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
appendTruncationMarker: true,
},
).pipe(
Effect.orElseSucceed(() => ({
exitCode: 0,
stdout: "",
stderr: "",
stdoutTruncated: false,
stderrTruncated: false,
})),
);
).pipe(Effect.orElseSucceed(() => EMPTY_GIT_RESULT));
const dirtyUntracked = yield* readUntrackedReviewDiffs(input.cwd).pipe(
Effect.orElseSucceed(() => ({ diff: "", truncated: false })),
);
Expand All @@ -2210,17 +2210,51 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
appendTruncationMarker: true,
},
).pipe(
Effect.orElseSucceed(() => ({
exitCode: 0,
stdout: "",
stderr: "",
stdoutTruncated: false,
stderrTruncated: false,
})),
)
).pipe(Effect.orElseSucceed(() => EMPTY_GIT_RESULT))
: null;
const baseDiff = baseResult?.stdout ?? "";

// `git diff A...B` has no working-tree form, so resolve the fork point explicitly and diff it
// against the worktree to cover committed and uncommitted work in a single patch.
const mergeBaseResult =
baseRef && branch
? yield* executeGit(
"GitVcsDriver.getReviewDiffPreview.mergeBase",
input.cwd,
["merge-base", baseRef, "HEAD"],
{ allowNonZeroExit: true },
).pipe(Effect.orElseSucceed(() => ({ ...EMPTY_GIT_RESULT, exitCode: 1 })))
: null;
const mergeBaseSha =
mergeBaseResult?.exitCode === 0 && mergeBaseResult.stdout.trim().length > 0
? mergeBaseResult.stdout.trim()
: null;
const sinceForkResult = mergeBaseSha
? yield* executeGit(
"GitVcsDriver.getReviewDiffPreview.sinceFork",
input.cwd,
[
"diff",
"--patch",
"--no-color",
"--no-ext-diff",
"--no-textconv",
"--minimal",
...(input.ignoreWhitespace ? ["--ignore-all-space"] : []),
mergeBaseSha,
"--",
],
{
maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
appendTruncationMarker: true,
},
).pipe(Effect.orElseSucceed(() => EMPTY_GIT_RESULT))
: null;
const sinceForkDiff = sinceForkResult
? [sinceForkResult.stdout.trimEnd(), dirtyUntracked.diff.trimEnd()]
.filter((diff) => diff.length > 0)
.join("\n")
: "";
const hashDiff = (diff: string) =>
crypto.digest("SHA-256", new TextEncoder().encode(diff)).pipe(
Effect.map(Encoding.encodeHex),
Expand All @@ -2235,9 +2269,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
}),
),
);
const [dirtyDiffHash, baseDiffHash] = yield* Effect.all([
const [dirtyDiffHash, baseDiffHash, sinceForkDiffHash] = yield* Effect.all([
hashDiff(dirtyDiff),
hashDiff(baseDiff),
hashDiff(sinceForkDiff),
]);

const sources: ReviewDiffPreviewSource[] = [
Expand All @@ -2261,6 +2296,16 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
diffHash: baseDiffHash,
truncated: baseResult?.stdoutTruncated ?? false,
},
{
id: "since-fork",
kind: "since-fork",
title: baseRef ? `All changes since ${baseRef}` : "All changes since base branch",
baseRef,
headRef: branch ?? "HEAD",
diff: sinceForkDiff,
diffHash: sinceForkDiffHash,
truncated: (sinceForkResult?.stdoutTruncated ?? false) || dirtyUntracked.truncated,
},
];

return {
Expand Down
Loading
Loading