fix(dashboard): show only files actually changed by the task
Two fixes for the in-review/in-progress "files changed" count on task cards and the Changes tab: - Drop untracked files from /tasks/:id/diff and /tasks/:id/file-diffs. At review time these are almost always build artifacts/cache/logs not in .gitignore, not real task changes, and they inflated the count. - Add display-only `enableDisplayRecovery` option to resolveDiffBase. When a worktree was rebased onto origin/main after baseCommitSha was recorded and baseBranch was not set, the prior code fell through to HEAD~1 — undercounting to just the last commit's files (e.g. FN-2957 showed 2 in review, 6 after merge). Recovery now tries merge-base(HEAD, main) / origin/main before HEAD~1. Routes opt into recovery; the merger's mirrored copy (resolveTaskDiffBaseRef) is intentionally untouched so merge-time scope warnings still evaluate the strict task base. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -240,9 +240,13 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
}
|
||||
const cwd = resolvedWorktree;
|
||||
|
||||
const diffBase = await resolveDiffBase(task, cwd);
|
||||
const diffBase = await resolveDiffBase(task, cwd, "HEAD", undefined, { enableDisplayRecovery: true });
|
||||
const diffRange = diffBase ? `${diffBase}..HEAD` : "HEAD";
|
||||
|
||||
// Only count files actually changed by the task: committed (base..HEAD)
|
||||
// + staged + unstaged. Untracked files are intentionally excluded — at
|
||||
// review time they're almost always build artifacts/cache/logs that
|
||||
// weren't in .gitignore, not real task changes.
|
||||
const fileMap = new Map<string, string>();
|
||||
|
||||
if (diffBase) {
|
||||
@@ -283,15 +287,6 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
// working tree diff failed
|
||||
}
|
||||
|
||||
try {
|
||||
const untrackedOutput = (await runGitCommand(["ls-files", "--others", "--exclude-standard"], cwd, 10000)).trim();
|
||||
for (const line of untrackedOutput.split("\n").filter(Boolean)) {
|
||||
fileMap.set(line, "U");
|
||||
}
|
||||
} catch {
|
||||
// untracked listing failed
|
||||
}
|
||||
|
||||
const files: Array<{
|
||||
path: string;
|
||||
status: "added" | "modified" | "deleted";
|
||||
@@ -304,17 +299,13 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
if (!filePath) continue;
|
||||
|
||||
let status: "added" | "modified" | "deleted";
|
||||
if (statusCode.startsWith("A") || statusCode === "U") status = "added";
|
||||
if (statusCode.startsWith("A")) status = "added";
|
||||
else if (statusCode.startsWith("D")) status = "deleted";
|
||||
else status = "modified";
|
||||
|
||||
let patch = "";
|
||||
try {
|
||||
if (statusCode === "U") {
|
||||
patch = await runGitCommand(["diff", "--no-index", "/dev/null", filePath], cwd, 10000).catch(() => "");
|
||||
} else {
|
||||
patch = await runGitCommand(["diff", diffRange, "--", filePath], cwd, 10000);
|
||||
}
|
||||
patch = await runGitCommand(["diff", diffRange, "--", filePath], cwd, 10000);
|
||||
} catch {
|
||||
// ignore individual file errors
|
||||
}
|
||||
@@ -423,8 +414,12 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
}
|
||||
|
||||
const cwd = worktree;
|
||||
const diffBase = await resolveDiffBase(task, cwd);
|
||||
const fileMap = new Map<string, { statusCode: string; oldPath?: string; isUntracked?: boolean }>();
|
||||
const diffBase = await resolveDiffBase(task, cwd, "HEAD", undefined, { enableDisplayRecovery: true });
|
||||
|
||||
// Only files actually changed by the task: committed + staged + unstaged.
|
||||
// Untracked files (build artifacts, cache, logs) are intentionally
|
||||
// excluded so the count matches "ACTUAL files changed by the task".
|
||||
const fileMap = new Map<string, { statusCode: string; oldPath?: string }>();
|
||||
|
||||
if (diffBase) {
|
||||
try {
|
||||
@@ -479,24 +474,13 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
// ignore unstaged diff failures
|
||||
}
|
||||
|
||||
try {
|
||||
const untrackedOutput = (await runGitCommand(["ls-files", "--others", "--exclude-standard"], cwd, 5000)).trim();
|
||||
for (const line of untrackedOutput.split("\n").filter(Boolean)) {
|
||||
if (line && !fileMap.has(line)) {
|
||||
fileMap.set(line, { statusCode: "U", isUntracked: true });
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// ignore untracked listing failures
|
||||
}
|
||||
|
||||
const diffRange = diffBase ? `${diffBase}..HEAD` : "HEAD";
|
||||
const files = [];
|
||||
|
||||
for (const [filePath, { statusCode, oldPath, isUntracked }] of fileMap.entries()) {
|
||||
for (const [filePath, { statusCode, oldPath }] of fileMap.entries()) {
|
||||
let status: "added" | "modified" | "deleted" | "renamed" = "modified";
|
||||
|
||||
if (statusCode.startsWith("A") || statusCode === "U") {
|
||||
if (statusCode.startsWith("A")) {
|
||||
status = "added";
|
||||
} else if (statusCode.startsWith("D")) {
|
||||
status = "deleted";
|
||||
@@ -506,16 +490,12 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
|
||||
let diff = "";
|
||||
try {
|
||||
if (isUntracked) {
|
||||
diff = await runGitCommand(["diff", "--no-index", "/dev/null", filePath], cwd, 5000).catch(() => "");
|
||||
} else {
|
||||
diff = await runGitCommand(["diff", diffRange, "--", filePath], cwd, 5000);
|
||||
}
|
||||
diff = await runGitCommand(["diff", diffRange, "--", filePath], cwd, 5000);
|
||||
} catch {
|
||||
diff = "";
|
||||
}
|
||||
|
||||
if (!diff && !isUntracked) {
|
||||
if (!diff) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
@@ -34,19 +34,41 @@ export interface ResolveDiffBaseTaskInput {
|
||||
baseBranch?: string;
|
||||
}
|
||||
|
||||
export interface ResolveDiffBaseOptions {
|
||||
/**
|
||||
* Display-only recovery: when the normal resolution would fall through to
|
||||
* `headRef~1` (because `baseBranch` is missing AND `baseCommitSha` is no
|
||||
* longer an ancestor of HEAD — e.g., the worktree was rebased onto
|
||||
* `origin/main` after `baseCommitSha` was recorded), attempt one final
|
||||
* `merge-base(headRef, "main")` (then `origin/main`) before giving up to
|
||||
* `headRef~1`.
|
||||
*
|
||||
* This is for the dashboard "files changed" UI only. The merger never opts
|
||||
* in — its scope checks must stay tied to the recorded task base, not a
|
||||
* widened display range.
|
||||
*
|
||||
* Default: false.
|
||||
*/
|
||||
enableDisplayRecovery?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the diff base ref for a task worktree.
|
||||
*
|
||||
* IMPORTANT: `packages/engine/src/merger.ts` mirrors this exact ordering for
|
||||
* merge-time scope warnings. Keep both implementations in sync so dashboard
|
||||
* changed-files views and merger scope enforcement evaluate the same range.
|
||||
* The `enableDisplayRecovery` option is *display-only* and intentionally not
|
||||
* mirrored in the merger.
|
||||
*
|
||||
* Strategy (in priority order):
|
||||
* 1. **Branch merge-base** — Prefer the live merge-base between `headRef` and
|
||||
* local `{baseBranch}` (fallback: `origin/{baseBranch}`).
|
||||
* 2. **Task-scoped baseCommitSha** — If merge-base is unavailable or equals
|
||||
* `headRef`, use `baseCommitSha` when still an ancestor of `headRef`.
|
||||
* 3. **headRef~1** — Last-resort fallback.
|
||||
* 3. **Display recovery (opt-in)** — `merge-base(headRef, "main")` /
|
||||
* `origin/main` when steps 1 and 2 yielded nothing.
|
||||
* 4. **headRef~1** — Last-resort fallback.
|
||||
*
|
||||
* Note: callers must validate the worktree still belongs to the task (e.g.
|
||||
* compare `git rev-parse --abbrev-ref HEAD` to `task.branch`) before invoking
|
||||
@@ -59,6 +81,7 @@ export async function resolveDiffBase(
|
||||
cwd: string,
|
||||
headRef = "HEAD",
|
||||
runGit: (args: string[], cwd?: string, timeout?: number) => Promise<string> = runGitCommand,
|
||||
options: ResolveDiffBaseOptions = {},
|
||||
): Promise<string | undefined> {
|
||||
// When baseBranch was nulled (e.g., upstream dep merged and its branch was
|
||||
// deleted) but a task-scoped baseCommitSha is still recorded, skip the
|
||||
@@ -100,6 +123,25 @@ export async function resolveDiffBase(
|
||||
}
|
||||
}
|
||||
|
||||
// Display-only recovery before the HEAD~1 fallback. Only kicks in when the
|
||||
// caller explicitly opted in AND the original resolution skipped the
|
||||
// merge-base step (no baseBranch was recorded). This catches the case where
|
||||
// a worktree got rebased onto origin/main after baseCommitSha was
|
||||
// recorded, leaving the SHA as a non-ancestor of HEAD.
|
||||
if (options.enableDisplayRecovery && !task.baseBranch?.trim()) {
|
||||
try {
|
||||
const out = (await runGit(["merge-base", headRef, "main"], cwd, 5000)).trim();
|
||||
if (out) return out;
|
||||
} catch {
|
||||
try {
|
||||
const out = (await runGit(["merge-base", headRef, "origin/main"], cwd, 5000)).trim();
|
||||
if (out) return out;
|
||||
} catch {
|
||||
// no recovery possible — fall through to HEAD~1
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
return (await runGit(["rev-parse", `${headRef}~1`], cwd, 5000)).trim() || undefined;
|
||||
} catch {
|
||||
|
||||
Reference in New Issue
Block a user