feat(FN-867): add task-scoped diff base via baseCommitSha
- Introduce baseCommitSha on sessions for task-scoped file diffs independent of shared worktrees - Add /api/sessions/:id/files and /api/sessions/:id/file-diffs routes using baseCommitSha - Update TerminalModal to consume task-scoped file list and diff endpoints - Add regression tests for shared worktree scenario (multiple tasks in same worktree) - Expand route tests for file diffs and session files with edge cases - Clean up stale TerminalModal test helpers and simplify diff display styles - Update README and route documentation for task-scoped diffing
This commit is contained in:
@@ -51,7 +51,7 @@ AI-guided interactive planning for creating well-specified tasks from high-level
|
||||
- **Layered Model Dropdowns**: Shared model combobox menus render in a top-level portal attached to `document.body`, so they stay above board columns and scrollable modal content instead of being clipped behind surrounding dashboard surfaces.
|
||||
- **Bulk Model Editing**: Update AI model configuration for multiple tasks at once in the list view. Select tasks via checkboxes (archived tasks excluded), then use the "Bulk Edit Models" toolbar to apply executor and/or validator model changes to all selected tasks. Selection persists in localStorage across page reloads.
|
||||
- **Task Details**: View full task specifications, agent logs, and attachments. The Agent Log tab expands to fill the full modal body height above the action bar, providing maximum vertical space for watching live agent output. The tab header shows the effective executor and validator model names resolved from task-level overrides or project/global settings fallbacks, matching the same resolution order the engine uses at runtime. A **Markdown/Plain toggle** in the header bar switches between formatted markdown rendering (default) and literal plain-text display — useful for debugging raw agent output, checking escaped markdown syntax, or inspecting exactly what the agent emitted without formatting. The toggle applies to `text` and `thinking` entries only; tool entries always render as plain text. React-markdown handles sanitization in markdown mode (no raw HTML is executed); plain-text mode uses React's built-in text escaping for safe literal output. The refinement modal positions the "Create Refinement Task" button adjacent to the feedback textarea alongside the character count, creating a tight input group that connects the submit action directly to the text being edited. The **Changes** tab for done tasks loads the diff from the recorded merge commit (`mergeDetails.commitSha`) via `fetchCommitDiff` rather than requiring a live worktree — changes remain visible even after the worktree is cleaned up. The tab shows commit metadata (short SHA, merge commit message, merged timestamp) alongside the file-level diff with addition/deletion totals. In-progress and in-review tasks continue to use the worktree-based diff path.
|
||||
- **Changed Files Viewer**: Click a task card's "files changed" button to open a dedicated diff viewer showing only files changed in that task worktree, with per-file statuses and sidebar navigation. On mobile (≤768px), the viewer switches to a single-pane flow: the file list and diff are shown one at a time with a back button for navigation between them. The viewer always opens to the file list on mobile, and only switches to the diff view when the user taps a specific file. Pressing Escape on the diff view returns to the file list first; pressing Escape again closes the modal. Loading, error, and empty states use theme-aware styling (including light mode). Diff syntax highlighting (additions, deletions, hunks) adapts to the active theme for correct contrast. The board card file count and the changed-files viewer always agree — both use the same merge-base diff strategy, so the card never advertises files that the viewer cannot inspect
|
||||
- **Changed Files Viewer**: Click a task card's "files changed" button to open a dedicated diff viewer showing only files changed in that task worktree, with per-file statuses and sidebar navigation. On mobile (≤768px), the viewer switches to a single-pane flow: the file list and diff are shown one at a time with a back button for navigation between them. The viewer always opens to the file list on mobile, and only switches to the diff view when the user taps a specific file. Pressing Escape on the diff view returns to the file list first; pressing Escape again closes the modal. Loading, error, and empty states use theme-aware styling (including light mode). Diff syntax highlighting (additions, deletions, hunks) adapts to the active theme for correct contrast. The board card file count and the changed-files viewer always agree — both use a shared diff-base resolution strategy. When the task has a `baseCommitSha` (captured at worktree creation time) that is still a valid ancestor of the current HEAD, the diff is scoped to only files introduced by that specific task. If `baseCommitSha` is stale or unavailable, the system falls back to a branch merge-base, then to `HEAD~1`. This ensures accurate file counts in shared or recycled worktree scenarios where a broader merge-base would include files from previous tasks.
|
||||
- **GitHub Import**: Import issues directly from GitHub repositories
|
||||
- **PR Management**: Create, monitor, and merge pull requests for in-review tasks
|
||||
- **Deep Links**: Dashboard task links using `?task=FN-123` (or `?project=proj_456&task=FN-123` for cross-project) open the task detail modal as a one-time launch. Dismissing the modal removes the `task` parameter from the URL so that refreshing the page does not reopen it. Other query parameters (e.g., `?project=...`) are preserved. Task detail modals opened normally from the board, list, or activity log are not affected.
|
||||
|
||||
@@ -134,16 +134,16 @@ describe("GET /api/tasks/:id/file-diffs", () => {
|
||||
|
||||
it("uses merge-base to resolve diff base and returns per-file diffs", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ baseBranch: "main" }));
|
||||
store.addTask(createTask({ baseBranch: "main", baseCommitSha: "taskbase456" }));
|
||||
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
const cmd = String(command);
|
||||
// Merge-base resolution (same strategy as session-files)
|
||||
if (cmd === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
|
||||
return "mergebase456\n" as any;
|
||||
// Task-scoped baseCommitSha validation
|
||||
if (cmd === "git merge-base --is-ancestor taskbase456 HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
// Committed changes against merge-base
|
||||
if (cmd === "git diff --name-status mergebase456..HEAD") {
|
||||
// Committed changes against baseCommitSha
|
||||
if (cmd === "git diff --name-status taskbase456..HEAD") {
|
||||
return "M\tsrc/updated.ts\nA\tsrc/added.ts\n" as any;
|
||||
}
|
||||
// Working tree changes
|
||||
@@ -151,10 +151,10 @@ describe("GET /api/tasks/:id/file-diffs", () => {
|
||||
return "" as any;
|
||||
}
|
||||
// Per-file diffs
|
||||
if (cmd === 'git diff mergebase456..HEAD -- "src/updated.ts"') {
|
||||
if (cmd === 'git diff taskbase456..HEAD -- "src/updated.ts"') {
|
||||
return "diff --git a/src/updated.ts b/src/updated.ts\n--- a/src/updated.ts\n+++ b/src/updated.ts\n+hello\n" as any;
|
||||
}
|
||||
if (cmd === 'git diff mergebase456..HEAD -- "src/added.ts"') {
|
||||
if (cmd === 'git diff taskbase456..HEAD -- "src/added.ts"') {
|
||||
return "diff --git a/src/added.ts b/src/added.ts\nnew file mode 100644\n+++ b/src/added.ts\n+added\n" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${cmd}`);
|
||||
@@ -178,20 +178,20 @@ describe("GET /api/tasks/:id/file-diffs", () => {
|
||||
|
||||
it("supports rename metadata with merge-base strategy", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ baseBranch: "main" }));
|
||||
store.addTask(createTask({ baseBranch: "main", baseCommitSha: "taskbase456" }));
|
||||
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
const cmd = String(command);
|
||||
if (cmd === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
|
||||
return "mergebase456\n" as any;
|
||||
if (cmd === "git merge-base --is-ancestor taskbase456 HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
if (cmd === "git diff --name-status mergebase456..HEAD") {
|
||||
if (cmd === "git diff --name-status taskbase456..HEAD") {
|
||||
return "R100\tsrc/old-name.ts\tsrc/new-name.ts\n" as any;
|
||||
}
|
||||
if (cmd === "git diff --name-status") {
|
||||
return "" as any;
|
||||
}
|
||||
if (cmd === 'git diff mergebase456..HEAD -- "src/new-name.ts"') {
|
||||
if (cmd === 'git diff taskbase456..HEAD -- "src/new-name.ts"') {
|
||||
return "diff --git a/src/old-name.ts b/src/new-name.ts\nsimilarity index 100%\nrename from src/old-name.ts\nrename to src/new-name.ts\n" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${cmd}`);
|
||||
@@ -257,24 +257,24 @@ describe("GET /api/tasks/:id/file-diffs", () => {
|
||||
|
||||
it("includes working-tree changes alongside committed changes", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ baseBranch: "main" }));
|
||||
store.addTask(createTask({ baseBranch: "main", baseCommitSha: "taskbase456" }));
|
||||
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
const cmd = String(command);
|
||||
if (cmd === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
|
||||
return "mergebase456\n" as any;
|
||||
if (cmd === "git merge-base --is-ancestor taskbase456 HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
if (cmd === "git diff --name-status mergebase456..HEAD") {
|
||||
if (cmd === "git diff --name-status taskbase456..HEAD") {
|
||||
return "M\tsrc/committed.ts\n" as any;
|
||||
}
|
||||
// Working tree has a different file
|
||||
if (cmd === "git diff --name-status") {
|
||||
return "A\tsrc/uncommitted.ts\n" as any;
|
||||
}
|
||||
if (cmd === 'git diff mergebase456..HEAD -- "src/committed.ts"') {
|
||||
if (cmd === 'git diff taskbase456..HEAD -- "src/committed.ts"') {
|
||||
return "diff --git a/src/committed.ts b/src/committed.ts\n+committed\n" as any;
|
||||
}
|
||||
if (cmd === 'git diff mergebase456..HEAD -- "src/uncommitted.ts"') {
|
||||
if (cmd === 'git diff taskbase456..HEAD -- "src/uncommitted.ts"') {
|
||||
return "diff --git a/src/uncommitted.ts b/src/uncommitted.ts\n+uncommitted\n" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${cmd}`);
|
||||
@@ -292,21 +292,21 @@ describe("GET /api/tasks/:id/file-diffs", () => {
|
||||
|
||||
it("deduplicates files that appear in both committed and working-tree diffs", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ baseBranch: "main" }));
|
||||
store.addTask(createTask({ baseBranch: "main", baseCommitSha: "taskbase456" }));
|
||||
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
const cmd = String(command);
|
||||
if (cmd === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
|
||||
return "mergebase456\n" as any;
|
||||
if (cmd === "git merge-base --is-ancestor taskbase456 HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
// Same file in both committed and working-tree
|
||||
if (cmd === "git diff --name-status mergebase456..HEAD") {
|
||||
if (cmd === "git diff --name-status taskbase456..HEAD") {
|
||||
return "M\tsrc/shared.ts\n" as any;
|
||||
}
|
||||
if (cmd === "git diff --name-status") {
|
||||
return "M\tsrc/shared.ts\n" as any;
|
||||
}
|
||||
if (cmd === 'git diff mergebase456..HEAD -- "src/shared.ts"') {
|
||||
if (cmd === 'git diff taskbase456..HEAD -- "src/shared.ts"') {
|
||||
return "diff --git a/src/shared.ts b/src/shared.ts\n+shared\n" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${cmd}`);
|
||||
@@ -346,22 +346,22 @@ describe("GET /api/tasks/:id/file-diffs", () => {
|
||||
|
||||
it("uses the 10-second cache before recomputing", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ baseBranch: "main" }));
|
||||
store.addTask(createTask({ baseBranch: "main", baseCommitSha: "taskbase456" }));
|
||||
|
||||
let callCount = 0;
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
callCount++;
|
||||
const cmd = String(command);
|
||||
if (cmd === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
|
||||
return "mergebase456\n" as any;
|
||||
if (cmd === "git merge-base --is-ancestor taskbase456 HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
if (cmd === "git diff --name-status mergebase456..HEAD") {
|
||||
if (cmd === "git diff --name-status taskbase456..HEAD") {
|
||||
return "M\tsrc/cached.ts\n" as any;
|
||||
}
|
||||
if (cmd === "git diff --name-status") {
|
||||
return "" as any;
|
||||
}
|
||||
if (cmd === 'git diff mergebase456..HEAD -- "src/cached.ts"') {
|
||||
if (cmd === 'git diff taskbase456..HEAD -- "src/cached.ts"') {
|
||||
return "diff --git a/src/cached.ts b/src/cached.ts\n+cached\n" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${cmd}`);
|
||||
@@ -464,4 +464,124 @@ describe("GET /api/tasks/:id/file-diffs", () => {
|
||||
// Both endpoints must report the same set of files
|
||||
expect(diffPaths.sort()).toEqual(sessionFiles.sort());
|
||||
});
|
||||
|
||||
// ── Regression: shared/recycled worktree produces broader file sets ─────────────────
|
||||
|
||||
it("task-scoped baseCommitSha narrows file-diffs to this task's work", async () => {
|
||||
const store = new MockStore();
|
||||
// Scenario: previous task left commits A, B, C. Current task started after and added D, E.
|
||||
// baseCommitSha = commit C (the commit where the current task started).
|
||||
// merge-base would return commit A (oldest common ancestor), which would be broader.
|
||||
// With task-scoped diffing using baseCommitSha=C, we should only see D, E.
|
||||
store.addTask(createTask({
|
||||
id: "FN-REGDIFF",
|
||||
baseCommitSha: "commitC",
|
||||
worktree: "/tmp/worktree",
|
||||
}));
|
||||
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
const cmd = String(command);
|
||||
// baseCommitSha is valid — ancestor check passes
|
||||
if (cmd === "git merge-base --is-ancestor commitC HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
// Task-scoped diff shows only D, E
|
||||
if (cmd === "git diff --name-status commitC..HEAD") {
|
||||
return "M\tsrc/d.ts\nA\tsrc/e.ts\n" as any;
|
||||
}
|
||||
// No working tree changes
|
||||
if (cmd === "git diff --name-status") {
|
||||
return "" as any;
|
||||
}
|
||||
// Per-file diffs
|
||||
if (cmd === 'git diff commitC..HEAD -- "src/d.ts"') {
|
||||
return "diff d" as any;
|
||||
}
|
||||
if (cmd === 'git diff commitC..HEAD -- "src/e.ts"') {
|
||||
return "diff e" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${cmd}`);
|
||||
});
|
||||
|
||||
const response = await requestFileDiffs(store, "FN-REGDIFF");
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
// Task-scoped: should only show files D and E, not A, B, C
|
||||
const paths = response.body.map((f: any) => f.path);
|
||||
expect(paths.sort()).toEqual(["src/d.ts", "src/e.ts"]);
|
||||
});
|
||||
|
||||
it("agrees with session-files under task-scoped base resolution", async () => {
|
||||
const store = new MockStore();
|
||||
// Both routes should produce the same file list when baseCommitSha is valid
|
||||
store.addTask(createTask({
|
||||
baseBranch: "main",
|
||||
id: "KB-AGREE-SCOPED",
|
||||
baseCommitSha: "scopedbase789",
|
||||
worktree: "/tmp/kb-agree-scoped",
|
||||
}));
|
||||
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
const cmd = String(command);
|
||||
// Task-scoped ancestor check
|
||||
if (cmd === "git merge-base --is-ancestor scopedbase789 HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
// session-files uses --name-only
|
||||
if (cmd === "git diff --name-only scopedbase789..HEAD") {
|
||||
return "src/x.ts\nsrc/y.ts\n" as any;
|
||||
}
|
||||
if (cmd === "git diff --name-only") {
|
||||
return "src/z.ts\n" as any;
|
||||
}
|
||||
// file-diffs uses --name-status
|
||||
if (cmd === "git diff --name-status scopedbase789..HEAD") {
|
||||
return "M\tsrc/x.ts\nA\tsrc/y.ts\n" as any;
|
||||
}
|
||||
if (cmd === "git diff --name-status") {
|
||||
return "M\tsrc/z.ts\n" as any;
|
||||
}
|
||||
// Per-file diffs for file-diffs
|
||||
if (cmd.includes('git diff scopedbase789..HEAD -- "src/x.ts"')) {
|
||||
return "diff x" as any;
|
||||
}
|
||||
if (cmd.includes('git diff scopedbase789..HEAD -- "src/y.ts"')) {
|
||||
return "diff y" as any;
|
||||
}
|
||||
if (cmd.includes('git diff scopedbase789..HEAD -- "src/z.ts"')) {
|
||||
return "diff z" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${cmd}`);
|
||||
});
|
||||
|
||||
// Request session-files (card count)
|
||||
const sessionHandler = await import("../routes.js").then(({ createApiRoutes }) => {
|
||||
const router = createApiRoutes(store as any);
|
||||
const layer = (router as any).stack.find(
|
||||
(candidate: any) =>
|
||||
candidate.route?.path === "/tasks/:id/session-files" &&
|
||||
candidate.route?.methods?.get,
|
||||
);
|
||||
return layer.route.stack[layer.route.stack.length - 1].handle as (req: any, res: any) => Promise<void>;
|
||||
});
|
||||
|
||||
const sessionReq = { params: { id: "KB-AGREE-SCOPED" } };
|
||||
const sessionRes = createMockResponse();
|
||||
await sessionHandler(sessionReq, sessionRes);
|
||||
|
||||
expect(sessionRes.statusCode).toBe(200);
|
||||
const sessionFiles: string[] = sessionRes.body as string[];
|
||||
expect(sessionFiles).toEqual(["src/x.ts", "src/y.ts", "src/z.ts"]);
|
||||
|
||||
// Request file-diffs (modal viewer)
|
||||
const diffsHandler = await getFileDiffsHandler(store);
|
||||
const diffsRes = await requestFileDiffsWithHandler(diffsHandler, "KB-AGREE-SCOPED");
|
||||
|
||||
expect(diffsRes.status).toBe(200);
|
||||
const diffFiles = diffsRes.body as Array<{ path: string }>;
|
||||
const diffPaths = diffFiles.map((f) => f.path);
|
||||
|
||||
// Both endpoints must report the same set of files under task-scoped base
|
||||
expect(diffPaths.sort()).toEqual(sessionFiles.sort());
|
||||
});
|
||||
});
|
||||
|
||||
@@ -135,10 +135,10 @@ describe("GET /api/tasks/:id/session-files", () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ id: "FN-675-base", baseCommitSha: "abc123" }));
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
if (String(command) === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
|
||||
return "mergebase123\n" as any;
|
||||
if (String(command) === "git merge-base --is-ancestor abc123 HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
if (String(command) === "git diff --name-only mergebase123..HEAD") {
|
||||
if (String(command) === "git diff --name-only abc123..HEAD") {
|
||||
return "src/a.ts\n" as any;
|
||||
}
|
||||
if (String(command) === "git diff --name-only") {
|
||||
@@ -151,14 +151,15 @@ describe("GET /api/tasks/:id/session-files", () => {
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual(["src/a.ts", "src/b.ts"]);
|
||||
// Should use task-scoped baseCommitSha (not merge-base)
|
||||
expect(mockExecSync).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
"git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main",
|
||||
"git merge-base --is-ancestor abc123 HEAD",
|
||||
expect.objectContaining({ cwd: "/tmp/fn-675" }),
|
||||
);
|
||||
expect(mockExecSync).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
"git diff --name-only mergebase123..HEAD",
|
||||
"git diff --name-only abc123..HEAD",
|
||||
expect.objectContaining({ cwd: "/tmp/fn-675" }),
|
||||
);
|
||||
expect(mockExecSync).toHaveBeenNthCalledWith(
|
||||
@@ -168,10 +169,14 @@ describe("GET /api/tasks/:id/session-files", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("ignores stale baseCommitSha values and uses current branch merge-base", async () => {
|
||||
it("ignores stale baseCommitSha values and falls back to merge-base", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ id: "FN-675-stale-base", baseCommitSha: "stale123" }));
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
// baseCommitSha is stale — is-ancestor fails
|
||||
if (String(command) === "git merge-base --is-ancestor stale123 HEAD") {
|
||||
throw new Error("not an ancestor");
|
||||
}
|
||||
if (String(command) === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
|
||||
return "mergebase123\n" as any;
|
||||
}
|
||||
@@ -188,19 +193,20 @@ describe("GET /api/tasks/:id/session-files", () => {
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual(["packages/engine/src/executor.ts"]);
|
||||
// Should try baseCommitSha first, then fall back to merge-base
|
||||
expect(mockExecSync).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
"git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main",
|
||||
"git merge-base --is-ancestor stale123 HEAD",
|
||||
expect.objectContaining({ cwd: "/tmp/fn-675" }),
|
||||
);
|
||||
expect(mockExecSync).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
"git diff --name-only mergebase123..HEAD",
|
||||
"git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main",
|
||||
expect.objectContaining({ cwd: "/tmp/fn-675" }),
|
||||
);
|
||||
expect(mockExecSync).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
"git diff --name-only",
|
||||
"git diff --name-only mergebase123..HEAD",
|
||||
expect.objectContaining({ cwd: "/tmp/fn-675" }),
|
||||
);
|
||||
});
|
||||
@@ -307,10 +313,10 @@ describe("GET /api/tasks/:id/session-files", () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ id: "FN-675-cache", baseCommitSha: "cachebase" }));
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
if (String(command) === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
|
||||
return "mergebase123\n" as any;
|
||||
if (String(command) === "git merge-base --is-ancestor cachebase HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
if (String(command) === "git diff --name-only mergebase123..HEAD") {
|
||||
if (String(command) === "git diff --name-only cachebase..HEAD") {
|
||||
return "cached/file.ts\n" as any;
|
||||
}
|
||||
if (String(command) === "git diff --name-only") {
|
||||
@@ -333,4 +339,42 @@ describe("GET /api/tasks/:id/session-files", () => {
|
||||
expect(third.body).toEqual(["cached/file.ts"]);
|
||||
expect(mockExecSync).toHaveBeenCalledTimes(6);
|
||||
});
|
||||
|
||||
// ── Regression: shared/recycled worktree produces broader file sets ─────────────────
|
||||
|
||||
it("task-scoped baseCommitSha narrows changed-files to this task's work", async () => {
|
||||
const store = new MockStore();
|
||||
// Scenario: previous task left commits A, B, C. Current task started after and added D, E.
|
||||
// baseCommitSha = commit C (the commit where the current task started)
|
||||
// merge-base would return commit A (oldest common ancestor), which would be broader.
|
||||
// With task-scoped diffing using baseCommitSha=C, we should only see D, E.
|
||||
store.addTask(createTask({
|
||||
id: "FN-REGRESSION",
|
||||
baseCommitSha: "commitC",
|
||||
worktree: "/tmp/worktree",
|
||||
}));
|
||||
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
const cmd = String(command);
|
||||
// baseCommitSha is valid — ancestor check passes
|
||||
if (cmd === "git merge-base --is-ancestor commitC HEAD") {
|
||||
return "" as any;
|
||||
}
|
||||
// Task-scoped diff shows only D, E
|
||||
if (cmd === "git diff --name-only commitC..HEAD") {
|
||||
return "src/d.ts\nsrc/e.ts\n" as any;
|
||||
}
|
||||
// No working tree changes
|
||||
if (cmd === "git diff --name-only") {
|
||||
return "" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${cmd}`);
|
||||
});
|
||||
|
||||
const response = await requestSessionFiles(store, "FN-REGRESSION");
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
// Task-scoped: should only show files D and E, not the A, B, C
|
||||
expect(response.body).toEqual(["src/d.ts", "src/e.ts"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2059,6 +2059,59 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Resolve the diff base ref for a task's worktree.
|
||||
*
|
||||
* Strategy (in priority order):
|
||||
* 1. **Task-scoped** — When the task has a `baseCommitSha` that is still
|
||||
* a valid ancestor of the current HEAD in the worktree, use it. This
|
||||
* keeps the changed-files list scoped to files introduced by *this*
|
||||
* specific task, even in shared or recycled worktree scenarios.
|
||||
* 2. **Branch merge-base** — Fall back to the merge-base between HEAD and
|
||||
* `origin/{baseBranch}` (or bare `{baseBranch}`).
|
||||
* 3. **HEAD~1** — Last resort when neither baseCommitSha nor merge-base
|
||||
* can be resolved.
|
||||
*/
|
||||
function resolveDiffBase(task: { baseCommitSha?: string; baseBranch?: string }, cwd: string): string | undefined {
|
||||
// 1. Try task-scoped baseCommitSha
|
||||
if (task.baseCommitSha) {
|
||||
try {
|
||||
// Validate that the stored SHA is still an ancestor of HEAD.
|
||||
// If the branch was rebased or the SHA is otherwise unreachable,
|
||||
// this will exit non-zero and we fall through.
|
||||
nodeChildProcess.execSync(
|
||||
`git merge-base --is-ancestor ${task.baseCommitSha} HEAD`,
|
||||
{ cwd, encoding: "utf-8", timeout: 5000, stdio: "pipe" },
|
||||
);
|
||||
return task.baseCommitSha;
|
||||
} catch {
|
||||
// baseCommitSha is stale or invalid — fall through to merge-base
|
||||
}
|
||||
}
|
||||
|
||||
// 2. Branch merge-base
|
||||
const baseBranch = task.baseBranch ?? "main";
|
||||
try {
|
||||
return nodeChildProcess.execSync(
|
||||
`git merge-base HEAD origin/${baseBranch} 2>/dev/null || git merge-base HEAD ${baseBranch}`,
|
||||
{ cwd, encoding: "utf-8", timeout: 5000 },
|
||||
).trim() || undefined;
|
||||
} catch {
|
||||
// merge-base unavailable — fall through to HEAD~1
|
||||
}
|
||||
|
||||
// 3. HEAD~1 fallback
|
||||
try {
|
||||
return nodeChildProcess.execSync("git rev-parse HEAD~1", {
|
||||
cwd,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim() || undefined;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
router.get("/tasks/:id/session-files", async (req, res) => {
|
||||
try {
|
||||
const scopedStore = await getScopedStore(req);
|
||||
@@ -2078,29 +2131,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
|
||||
try {
|
||||
const fileSet = new Set<string>();
|
||||
const baseBranch = task.baseBranch ?? "main";
|
||||
let baseRef: string | undefined;
|
||||
|
||||
try {
|
||||
baseRef = nodeChildProcess.execSync(
|
||||
`git merge-base HEAD origin/${baseBranch} 2>/dev/null || git merge-base HEAD ${baseBranch}`,
|
||||
{
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
},
|
||||
).trim();
|
||||
} catch {
|
||||
try {
|
||||
baseRef = nodeChildProcess.execSync("git rev-parse HEAD~1", {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
} catch {
|
||||
baseRef = undefined;
|
||||
}
|
||||
}
|
||||
const baseRef = resolveDiffBase(task, task.worktree);
|
||||
|
||||
if (baseRef) {
|
||||
const committedOutput = nodeChildProcess.execSync(`git diff --name-only ${baseRef}..HEAD`, {
|
||||
@@ -7075,8 +7106,9 @@ Output ONLY the prompt text (no markdown, no explanations).`;
|
||||
/**
|
||||
* GET /api/tasks/:id/file-diffs
|
||||
* Fetch changed files with individual git diffs for a task worktree.
|
||||
* Uses the same merge-base resolution strategy as the session-files route
|
||||
* so the board card count and the changed-files viewer always agree.
|
||||
* Uses the shared resolveDiffBase() helper so the board card count and the
|
||||
* changed-files viewer always agree. Prefers task.baseCommitSha when valid,
|
||||
* falling back to branch merge-base / HEAD~1.
|
||||
* Returns: Array<{ path, status, diff, oldPath? }>
|
||||
*/
|
||||
router.get("/tasks/:id/file-diffs", async (req, res) => {
|
||||
@@ -7094,29 +7126,12 @@ Output ONLY the prompt text (no markdown, no explanations).`;
|
||||
return;
|
||||
}
|
||||
|
||||
const baseBranch = task.baseBranch ?? "main";
|
||||
const cwd = task.worktree;
|
||||
|
||||
// Resolve a diff base using the same merge-base strategy as session-files
|
||||
// so both endpoints always agree on which files have changed.
|
||||
let diffBase: string | undefined;
|
||||
|
||||
try {
|
||||
diffBase = nodeChildProcess.execSync(
|
||||
`git merge-base HEAD origin/${baseBranch} 2>/dev/null || git merge-base HEAD ${baseBranch}`,
|
||||
{ cwd, encoding: "utf-8", timeout: 5000 },
|
||||
).trim();
|
||||
} catch {
|
||||
try {
|
||||
diffBase = nodeChildProcess.execSync("git rev-parse HEAD~1", {
|
||||
cwd,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
} catch {
|
||||
diffBase = undefined;
|
||||
}
|
||||
}
|
||||
// Resolve a diff base using the shared strategy so both endpoints
|
||||
// always agree on which files have changed. Prefer task-scoped
|
||||
// baseCommitSha when it is still valid for the current HEAD.
|
||||
const diffBase = resolveDiffBase(task, cwd);
|
||||
|
||||
// Collect file statuses from both committed changes (against diffBase)
|
||||
// and working-tree changes, deduplicating by path to match session-files.
|
||||
|
||||
Reference in New Issue
Block a user