FN-014: scope session diffs to task-owned files
Scope rebase session diffs to files proven to belong to the task. - Attribute rebase-range files to task commits before falling back to execution snapshots. - Return empty diffs when ownership cannot be proven instead of exposing unrelated files. - Add parity and route coverage plus a patch changeset. Files changed: .changeset/fn-014-task-diff-attribution.md | 7 ++ .../TaskCard-TaskChangesTab-parity.test.tsx | 42 ++++++++++ .../dashboard/src/__tests__/routes-github.test.ts | 90 +++++++++++++++++++++ .../src/routes/register-session-diff-routes.ts | 66 ++++++++++++---- 4 files changed, 188 insertions(+), 17 deletions(-) Fusion-Task-Id: FN-014 Fusion-Task-Lineage: e5aebee9-4190-4f56-b386-9448b2ab7ffd Co-authored-by: Fusion <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-014-task-diff-attribution.md
Normal file
7
.changeset/fn-014-task-diff-attribution.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Keep Files Changed scoped to task-owned files after rebases.
|
||||
category: fix
|
||||
dev: Rebase-backed dashboard diffs now prefer attributed commits or execution-scoped files and omit unproven remote changes.
|
||||
@@ -129,4 +129,46 @@ describe("TaskCard/TaskChangesTab files-changed parity", () => {
|
||||
});
|
||||
expect(container.textContent).toContain("3 files changed");
|
||||
});
|
||||
|
||||
it("keeps the card and Changes tab on the filtered rebase result", async () => {
|
||||
const stats = { filesChanged: 1, additions: 2, deletions: 0 };
|
||||
useTaskDiffStatsMock.mockReturnValue({ stats, loading: false });
|
||||
fetchTaskDiffMock.mockResolvedValue({
|
||||
files: [{ path: "task.ts", status: "modified", additions: 2, deletions: 0, patch: "@@ task" }],
|
||||
stats,
|
||||
});
|
||||
|
||||
const task = makeTask({ id: "FN-014", column: "done", mergeDetails: { commitSha: "rebased-tip" } });
|
||||
const { container } = render(
|
||||
<>
|
||||
<TaskCard task={task} onOpenDetail={() => {}} addToast={() => {}} />
|
||||
<TaskChangesTab taskId={task.id} column="done" mergeDetails={task.mergeDetails} />
|
||||
</>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector(".card-session-files")?.textContent).toMatch(/1 file changed/);
|
||||
expect(screen.getByText("Files Changed (1)")).toBeTruthy();
|
||||
});
|
||||
expect(container.textContent).toContain("task.ts");
|
||||
expect(container.textContent).not.toContain("foreign.ts");
|
||||
expect(container.querySelectorAll(".card-session-files")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("does not render a card files button for an empty scoped result", async () => {
|
||||
const stats = { filesChanged: 0, additions: 0, deletions: 0 };
|
||||
useTaskDiffStatsMock.mockReturnValue({ stats, loading: false });
|
||||
fetchTaskDiffMock.mockResolvedValue({ files: [], stats });
|
||||
|
||||
const task = makeTask({ id: "FN-014", column: "done", mergeDetails: { commitSha: "rebased-tip" } });
|
||||
const { container } = render(
|
||||
<>
|
||||
<TaskCard task={task} onOpenDetail={() => {}} addToast={() => {}} />
|
||||
<TaskChangesTab taskId={task.id} column="done" mergeDetails={task.mergeDetails} />
|
||||
</>,
|
||||
);
|
||||
|
||||
await waitFor(() => expect(screen.getByText("Files Changed (0)")).toBeTruthy());
|
||||
expect(container.querySelector(".card-session-files")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3269,6 +3269,96 @@ describe("GET /tasks/:id/diff", () => {
|
||||
expect(Array.isArray(res.body.files)).toBe(true);
|
||||
expect(res.body.stats).toHaveProperty("filesChanged");
|
||||
});
|
||||
|
||||
it("scopes both done diff endpoints to an attributed commit after a remote rebase", async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "kb-dashboard-rebase-attribution-"));
|
||||
try {
|
||||
execFileSync("git", ["init", "--initial-branch=main", root], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "config", "user.email", "kb-tests@example.com"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "config", "user.name", "KB Tests"], { stdio: "pipe" });
|
||||
writeFileSync(join(root, "base.ts"), "export const base = true;\n");
|
||||
execFileSync("git", ["-C", root, "add", "base.ts"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "commit", "-m", "base"], { stdio: "pipe" });
|
||||
const rebaseBaseSha = execFileSync("git", ["-C", root, "rev-parse", "HEAD"], { encoding: "utf-8", stdio: "pipe" }).trim();
|
||||
|
||||
writeFileSync(join(root, "foreign.ts"), "export const remote = true;\n");
|
||||
execFileSync("git", ["-C", root, "add", "foreign.ts"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "commit", "-m", "remote work"], { stdio: "pipe" });
|
||||
writeFileSync(join(root, "task.ts"), "export const task = true;\n");
|
||||
execFileSync("git", ["-C", root, "add", "task.ts"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "commit", "-m", "fix(FN-014): task execution change"], { stdio: "pipe" });
|
||||
const commitSha = execFileSync("git", ["-C", root, "rev-parse", "HEAD"], { encoding: "utf-8", stdio: "pipe" }).trim();
|
||||
|
||||
const localStore = createMockStore({ getRootDir: vi.fn().mockReturnValue(root) });
|
||||
(localStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-014",
|
||||
column: "done",
|
||||
modifiedFiles: ["task.ts"],
|
||||
mergeDetails: { commitSha, rebaseBaseSha, filesChanged: 2 },
|
||||
});
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(localStore));
|
||||
|
||||
const diffResponse = await GET(app, "/api/tasks/FN-014/diff");
|
||||
expect(diffResponse.status).toBe(200);
|
||||
expect(diffResponse.body.files.map((file: { path: string }) => file.path)).toEqual(["task.ts"]);
|
||||
expect(diffResponse.body.files[0].patch).toContain("task = true");
|
||||
expect(diffResponse.body.files.map((file: { path: string }) => file.path)).not.toContain("foreign.ts");
|
||||
expect(diffResponse.body.stats.filesChanged).toBe(1);
|
||||
|
||||
const fileDiffsResponse = await GET(app, "/api/tasks/FN-014/file-diffs");
|
||||
expect(fileDiffsResponse.status).toBe(200);
|
||||
expect(fileDiffsResponse.body.map((file: { path: string }) => file.path)).toEqual(["task.ts"]);
|
||||
expect(fileDiffsResponse.body[0].diff).toContain("task = true");
|
||||
expect(fileDiffsResponse.body.map((file: { path: string }) => file.path)).not.toContain("foreign.ts");
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("returns no files for a foreign-only rebase when execution evidence is empty", async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "kb-dashboard-rebase-unproven-"));
|
||||
try {
|
||||
execFileSync("git", ["init", "--initial-branch=main", root], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "config", "user.email", "kb-tests@example.com"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "config", "user.name", "KB Tests"], { stdio: "pipe" });
|
||||
writeFileSync(join(root, "base.ts"), "export const base = true;\n");
|
||||
execFileSync("git", ["-C", root, "add", "base.ts"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "commit", "-m", "base"], { stdio: "pipe" });
|
||||
const rebaseBaseSha = execFileSync("git", ["-C", root, "rev-parse", "HEAD"], { encoding: "utf-8", stdio: "pipe" }).trim();
|
||||
writeFileSync(join(root, "foreign.ts"), "export const remote = true;\n");
|
||||
execFileSync("git", ["-C", root, "add", "foreign.ts"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "commit", "-m", "remote work"], { stdio: "pipe" });
|
||||
writeFileSync(join(root, "unproven.ts"), "export const task = true;\n");
|
||||
execFileSync("git", ["-C", root, "add", "unproven.ts"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "commit", "-m", "local task work"], { stdio: "pipe" });
|
||||
const commitSha = execFileSync("git", ["-C", root, "rev-parse", "HEAD"], { encoding: "utf-8", stdio: "pipe" }).trim();
|
||||
|
||||
const localStore = createMockStore({ getRootDir: vi.fn().mockReturnValue(root) });
|
||||
(localStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-014",
|
||||
column: "done",
|
||||
modifiedFiles: [],
|
||||
mergeDetails: { commitSha, rebaseBaseSha, filesChanged: 2 },
|
||||
});
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(localStore));
|
||||
|
||||
const diffResponse = await GET(app, "/api/tasks/FN-014/diff");
|
||||
expect(diffResponse.status).toBe(200);
|
||||
expect(diffResponse.body).toEqual({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } });
|
||||
|
||||
const fileDiffsResponse = await GET(app, "/api/tasks/FN-014/file-diffs");
|
||||
expect(fileDiffsResponse.status).toBe(200);
|
||||
expect(fileDiffsResponse.body).toEqual([]);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves missing done-task commitSha from run-audit commit events", async () => {
|
||||
|
||||
@@ -249,6 +249,8 @@ async function isReachableFromHead(sha: string, rootDir: string): Promise<boolea
|
||||
type DoneTaskAggregationTask = {
|
||||
id: string;
|
||||
lineageId?: string | null;
|
||||
/** Executor-captured paths used only when landed-file capture cannot prove ownership. */
|
||||
modifiedFiles?: string[];
|
||||
mergeDetails?: {
|
||||
commitSha?: string;
|
||||
rebaseBaseSha?: string;
|
||||
@@ -258,6 +260,7 @@ type DoneTaskAggregationTask = {
|
||||
landedFiles?: string[];
|
||||
landedFilesAttributionRestricted?: boolean;
|
||||
noOpVerifiedShortCircuit?: boolean;
|
||||
landedFilesCaptureFallback?: "attribution-failed";
|
||||
} | null;
|
||||
};
|
||||
|
||||
@@ -818,30 +821,39 @@ async function restrictRebaseRangeFiles(
|
||||
return rebaseRangeFiles.filter((file) => landedSet.has(file.path));
|
||||
}
|
||||
|
||||
if (Array.isArray(landed) && landed.length > 0) {
|
||||
return rebaseRangeFiles.filter((file) => landedSet.has(file.path));
|
||||
}
|
||||
|
||||
let attribution: Awaited<ReturnType<typeof filterFilesToOwnTaskCommits>> | undefined;
|
||||
try {
|
||||
const attribution = await filterFilesToOwnTaskCommits({
|
||||
attribution = await filterFilesToOwnTaskCommits({
|
||||
worktreePath: deps.rootDir,
|
||||
baseRef: deps.rebaseBaseShaForAggregation,
|
||||
taskId: task.id,
|
||||
runGit: deps.runGit,
|
||||
});
|
||||
if (attribution.files.length === 0) {
|
||||
// Read-only done-task diff display should still surface the rebase range
|
||||
// when commit attribution cannot prove ownership from subjects/trailers.
|
||||
return rebaseRangeFiles;
|
||||
}
|
||||
const ownSet = new Set(attribution.files);
|
||||
return rebaseRangeFiles.filter((file) => ownSet.has(file.path));
|
||||
} catch (err) {
|
||||
severityAuditLog.warn(
|
||||
`[diff] FN-5154 attribution failed for ${task.id}: ${(err as Error).message}; falling back to unrestricted range`,
|
||||
`[diff] FN-5154 attribution failed for ${task.id}: ${(err as Error).message}`,
|
||||
);
|
||||
return rebaseRangeFiles;
|
||||
}
|
||||
|
||||
if (attribution?.files.length) {
|
||||
const ownSet = new Set(attribution.files);
|
||||
return rebaseRangeFiles.filter((file) => ownSet.has(file.path));
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:TaskDiffAttribution 2026-08-18-18:44:
|
||||
A rebase range describes repository history, not task ownership. Prefer commit attribution,
|
||||
then the executor's persisted snapshot only when landed-file capture is absent or explicitly
|
||||
failed; never widen to foreign range files when neither source proves ownership.
|
||||
*/
|
||||
const captureFailed = task.mergeDetails?.landedFilesCaptureFallback === "attribution-failed";
|
||||
const mergeCaptureAbsent = !Array.isArray(landed);
|
||||
if (captureFailed || mergeCaptureAbsent) {
|
||||
const executionFiles = new Set(task.modifiedFiles ?? []);
|
||||
return rebaseRangeFiles.filter((file) => executionFiles.has(file.path));
|
||||
}
|
||||
|
||||
return [];
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1114,8 +1126,15 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
}
|
||||
|
||||
const doneFiles = await collectDoneRangeFiles(diffSpec.range, rootDir).catch(() => []);
|
||||
if (doneFiles.length > 0) {
|
||||
const files = doneFiles.map((file) => ({
|
||||
const scopedDoneFiles = diffSpec.mode === "rebase-range"
|
||||
? await restrictRebaseRangeFiles(task, doneFiles, {
|
||||
rootDir,
|
||||
rebaseBaseShaForAggregation: diffSpec.base,
|
||||
runGit: (args: string[]) => runGitCommand(args, rootDir, 10000),
|
||||
})
|
||||
: doneFiles;
|
||||
if (scopedDoneFiles.length > 0) {
|
||||
const files = scopedDoneFiles.map((file) => ({
|
||||
...file,
|
||||
status: file.status === "renamed" ? "modified" : file.status,
|
||||
}));
|
||||
@@ -1130,6 +1149,12 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
return;
|
||||
}
|
||||
|
||||
// A failed or foreign-only rebase range has no task-owned shortstat to report.
|
||||
if (diffSpec.mode === "rebase-range") {
|
||||
res.json({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } });
|
||||
return;
|
||||
}
|
||||
|
||||
const shortstat = await runGitCommand(["show", "--shortstat", "--format=", resolvedMergeSha], rootDir, 10000)
|
||||
.then((output) => parseGitShortstat(output))
|
||||
.catch(() => ({ filesChanged: 0, additions: 0, deletions: 0 }));
|
||||
@@ -1310,7 +1335,14 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
|
||||
try {
|
||||
const doneFiles = await collectDoneRangeFiles(diffSpec.range, rootDir);
|
||||
res.json(doneFiles.map((file) => ({ path: file.path, status: file.status, diff: file.patch })));
|
||||
const scopedDoneFiles = diffSpec.mode === "rebase-range"
|
||||
? await restrictRebaseRangeFiles(task, doneFiles, {
|
||||
rootDir,
|
||||
rebaseBaseShaForAggregation: diffSpec.base,
|
||||
runGit: (args: string[]) => runGitCommand(args, rootDir, 10000),
|
||||
})
|
||||
: doneFiles;
|
||||
res.json(scopedDoneFiles.map((file) => ({ path: file.path, status: file.status, diff: file.patch })));
|
||||
} catch {
|
||||
res.json([]);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user