feat(FN-4522): complete Step 2 — resolve done-task merge sha fallbacks
Fusion-Task-Id: FN-4522 Fusion-Task-Lineage: fbaedf11-6bc6-4c77-91b0-6b1ef606d81f
This commit is contained in:
@@ -1979,6 +1979,108 @@ describe("GET /tasks/:id/diff", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves missing done-task commitSha from run-audit commit events", async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "kb-dashboard-done-audit-"));
|
||||
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, "tracked.ts"), "export const value = 1;\n");
|
||||
execFileSync("git", ["-C", root, "add", "tracked.ts"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "commit", "-m", "initial"], { stdio: "pipe" });
|
||||
writeFileSync(join(root, "tracked.ts"), "export const value = 2;\n");
|
||||
execFileSync("git", ["-C", root, "commit", "-am", "update"], { stdio: "pipe" });
|
||||
const mergeSha = execFileSync("git", ["-C", root, "rev-parse", "HEAD"], { encoding: "utf-8", stdio: "pipe" }).trim();
|
||||
|
||||
const localStore = createMockStore({
|
||||
getRootDir: vi.fn().mockReturnValue(root),
|
||||
getRunAuditEvents: vi.fn().mockReturnValue([{ mutationType: "commit:create", target: mergeSha }]),
|
||||
getTaskCommitAssociationsByLineageId: vi.fn().mockResolvedValue([]),
|
||||
});
|
||||
(localStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-001",
|
||||
column: "done",
|
||||
mergeDetails: { filesChanged: 1, insertions: 1, deletions: 1 },
|
||||
});
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(localStore));
|
||||
|
||||
const res = await GET(app, "/api/tasks/FN-001/diff");
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.stats.filesChanged).toBe(1);
|
||||
expect(res.body.stats.additions).toBeGreaterThan(0);
|
||||
expect(res.body.stats.deletions).toBeGreaterThan(0);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("resolves missing done-task commitSha from lineage associations when audit has no sha", async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "kb-dashboard-done-lineage-"));
|
||||
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, "tracked.ts"), "export const value = 1;\n");
|
||||
execFileSync("git", ["-C", root, "add", "tracked.ts"], { stdio: "pipe" });
|
||||
execFileSync("git", ["-C", root, "commit", "-m", "initial"], { stdio: "pipe" });
|
||||
writeFileSync(join(root, "tracked.ts"), "export const value = 3;\n");
|
||||
execFileSync("git", ["-C", root, "commit", "-am", "lineage change"], { stdio: "pipe" });
|
||||
const lineageSha = execFileSync("git", ["-C", root, "rev-parse", "HEAD"], { encoding: "utf-8", stdio: "pipe" }).trim();
|
||||
|
||||
const localStore = createMockStore({
|
||||
getRootDir: vi.fn().mockReturnValue(root),
|
||||
getRunAuditEvents: vi.fn().mockReturnValue([]),
|
||||
getTaskCommitAssociationsByLineageId: vi.fn().mockResolvedValue([{ commitSha: lineageSha, authoredAt: new Date().toISOString() }]),
|
||||
});
|
||||
(localStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-001",
|
||||
lineageId: "lineage-1",
|
||||
column: "done",
|
||||
mergeDetails: { filesChanged: 1, insertions: 1, deletions: 1 },
|
||||
});
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(localStore));
|
||||
|
||||
const res = await GET(app, "/api/tasks/FN-001/diff");
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.stats.filesChanged).toBe(1);
|
||||
expect(res.body.stats.additions).toBeGreaterThan(0);
|
||||
expect(res.body.stats.deletions).toBeGreaterThan(0);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps mergeDetails summary fallback when no done-task commit sha can be resolved", async () => {
|
||||
const localStore = createMockStore({
|
||||
getRunAuditEvents: vi.fn().mockReturnValue([{ mutationType: "commit:create", target: "HEAD" }]),
|
||||
getTaskCommitAssociationsByLineageId: vi.fn().mockResolvedValue([]),
|
||||
});
|
||||
(localStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-001",
|
||||
lineageId: "lineage-1",
|
||||
column: "done",
|
||||
mergeDetails: { filesChanged: 3, insertions: 10, deletions: 2 },
|
||||
});
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(localStore));
|
||||
|
||||
const res = await GET(app, "/api/tasks/FN-001/diff");
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.files).toEqual([]);
|
||||
expect(res.body.stats).toEqual({ filesChanged: 3, additions: 10, deletions: 2 });
|
||||
});
|
||||
|
||||
it("uses destination path for rename entries in active-task name-status parsing", async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "kb-dashboard-rename-"));
|
||||
try {
|
||||
|
||||
@@ -228,13 +228,20 @@ async function isReachableFromHead(sha: string, rootDir: string): Promise<boolea
|
||||
}
|
||||
|
||||
type DoneTaskAggregationTask = {
|
||||
id: string;
|
||||
lineageId?: string | null;
|
||||
mergeDetails?: { commitSha?: string } | null;
|
||||
mergeDetails?: { commitSha?: string; filesChanged?: number; insertions?: number; deletions?: number } | null;
|
||||
};
|
||||
|
||||
type DoneTaskAggregationStore = {
|
||||
getRootDir: () => string;
|
||||
getTaskCommitAssociationsByLineageId: (lineageId: string) => Promise<Array<{ commitSha: string; authoredAt?: string | null }>>;
|
||||
getRunAuditEvents?: (filter: {
|
||||
taskId?: string;
|
||||
domain?: string;
|
||||
mutationType?: string;
|
||||
limit?: number;
|
||||
}) => Array<{ target?: unknown; metadata?: unknown; payload?: unknown; newValue?: unknown }> | Promise<Array<{ target?: unknown; metadata?: unknown; payload?: unknown; newValue?: unknown }>>;
|
||||
};
|
||||
|
||||
async function resolveCommitDiffSpec(sha: string, rootDir: string): Promise<
|
||||
@@ -305,6 +312,64 @@ async function collectDoneRangeFiles(range: string, rootDir: string): Promise<Ag
|
||||
return files;
|
||||
}
|
||||
|
||||
function extractCommitShaCandidate(event: { target?: unknown; metadata?: unknown; payload?: unknown; newValue?: unknown }): string | undefined {
|
||||
if (typeof event.target === "string" && event.target.trim()) {
|
||||
return event.target.trim();
|
||||
}
|
||||
|
||||
const candidateObjects = [event.metadata, event.payload, event.newValue].filter((value): value is Record<string, unknown> => !!value && typeof value === "object");
|
||||
for (const candidate of candidateObjects) {
|
||||
const commitSha = candidate.commitSha;
|
||||
if (typeof commitSha === "string" && commitSha.trim()) {
|
||||
return commitSha.trim();
|
||||
}
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
async function resolveAuditCommitSha(taskId: string, scopedStore: DoneTaskAggregationStore): Promise<string | undefined> {
|
||||
if (!scopedStore.getRunAuditEvents) return undefined;
|
||||
|
||||
const rootDir = scopedStore.getRootDir();
|
||||
const mutationTypes = ["git:commit", "commit:create", "commit:amend"];
|
||||
for (const mutationType of mutationTypes) {
|
||||
const events = await Promise.resolve(scopedStore.getRunAuditEvents({ taskId, domain: "git", mutationType, limit: 5 }));
|
||||
for (const event of events) {
|
||||
const candidate = extractCommitShaCandidate(event);
|
||||
if (!candidate) continue;
|
||||
try {
|
||||
const resolved = (await runGitCommand(["rev-parse", "--verify", "--quiet", candidate], rootDir, 5000)).trim();
|
||||
if (resolved && (await isReachableFromHead(resolved, rootDir))) {
|
||||
return resolved;
|
||||
}
|
||||
} catch {
|
||||
// ignore invalid or unreachable candidates
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
async function resolveDoneTaskMergeSha(task: DoneTaskAggregationTask, scopedStore: DoneTaskAggregationStore): Promise<string | undefined> {
|
||||
const existing = task.mergeDetails?.commitSha?.trim();
|
||||
if (existing) return existing;
|
||||
|
||||
const auditSha = await resolveAuditCommitSha(task.id, scopedStore);
|
||||
if (auditSha) return auditSha;
|
||||
|
||||
if (!task.lineageId) return undefined;
|
||||
const associations = await scopedStore.getTaskCommitAssociationsByLineageId(task.lineageId);
|
||||
for (const association of associations) {
|
||||
if (association.commitSha && (await isReachableFromHead(association.commitSha, scopedStore.getRootDir()))) {
|
||||
return association.commitSha;
|
||||
}
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
async function collectDoneTaskFiles(task: DoneTaskAggregationTask, scopedStore: DoneTaskAggregationStore): Promise<{
|
||||
files: AggregatedDoneTaskFile[];
|
||||
stats: { filesChanged: number; additions: number; deletions: number };
|
||||
@@ -530,10 +595,21 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
return;
|
||||
}
|
||||
|
||||
if (task.column === "done" && task.mergeDetails?.commitSha) {
|
||||
const aggregated = await collectDoneTaskFiles(task, scopedStore);
|
||||
if (task.column === "done") {
|
||||
const resolvedMergeSha = await resolveDoneTaskMergeSha(task, scopedStore);
|
||||
const doneTaskForDiff = resolvedMergeSha
|
||||
? {
|
||||
...task,
|
||||
mergeDetails: {
|
||||
...task.mergeDetails,
|
||||
commitSha: resolvedMergeSha,
|
||||
},
|
||||
}
|
||||
: task;
|
||||
|
||||
const aggregated = await collectDoneTaskFiles(doneTaskForDiff, scopedStore);
|
||||
const expectedFilesChanged = task.mergeDetails?.filesChanged ?? 0;
|
||||
const aggregationLooksComplete = expectedFilesChanged <= 0 || aggregated.files.length >= expectedFilesChanged;
|
||||
const aggregationLooksComplete = expectedFilesChanged <= 0 || aggregated.stats.filesChanged === expectedFilesChanged;
|
||||
|
||||
if (aggregated.usedAggregation && aggregated.files.length > 0 && aggregationLooksComplete) {
|
||||
res.json({
|
||||
@@ -546,8 +622,32 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
return;
|
||||
}
|
||||
|
||||
if (aggregated.usedAggregation && aggregated.files.length > 0) {
|
||||
res.json({
|
||||
files: aggregated.files.map((file) => ({
|
||||
...file,
|
||||
status: file.status === "renamed" ? "modified" : file.status,
|
||||
})),
|
||||
stats: aggregated.stats,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (!resolvedMergeSha) {
|
||||
const md = task.mergeDetails;
|
||||
res.json({
|
||||
files: [],
|
||||
stats: {
|
||||
filesChanged: md?.filesChanged ?? 0,
|
||||
additions: md?.insertions ?? 0,
|
||||
deletions: md?.deletions ?? 0,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const rootDir = scopedStore.getRootDir();
|
||||
const sha = task.mergeDetails.commitSha;
|
||||
const sha = resolvedMergeSha;
|
||||
|
||||
let diffSpec: Awaited<ReturnType<typeof resolveCommitDiffSpec>>;
|
||||
try {
|
||||
@@ -590,18 +690,6 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
return;
|
||||
}
|
||||
|
||||
if (task.column === "done") {
|
||||
const md = task.mergeDetails;
|
||||
res.json({
|
||||
files: [],
|
||||
stats: {
|
||||
filesChanged: md?.filesChanged ?? 0,
|
||||
additions: md?.insertions ?? 0,
|
||||
deletions: md?.deletions ?? 0,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const worktree = typeof req.query.worktree === "string" ? req.query.worktree : undefined;
|
||||
const resolvedWorktree = worktree || task.worktree;
|
||||
@@ -728,18 +816,39 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
return;
|
||||
}
|
||||
|
||||
if (task.column === "done" && task.mergeDetails?.commitSha) {
|
||||
const aggregated = await collectDoneTaskFiles(task, scopedStore);
|
||||
if (task.column === "done") {
|
||||
const resolvedMergeSha = await resolveDoneTaskMergeSha(task, scopedStore);
|
||||
const doneTaskForDiff = resolvedMergeSha
|
||||
? {
|
||||
...task,
|
||||
mergeDetails: {
|
||||
...task.mergeDetails,
|
||||
commitSha: resolvedMergeSha,
|
||||
},
|
||||
}
|
||||
: task;
|
||||
|
||||
const aggregated = await collectDoneTaskFiles(doneTaskForDiff, scopedStore);
|
||||
const expectedFilesChanged = task.mergeDetails?.filesChanged ?? 0;
|
||||
const aggregationLooksComplete = expectedFilesChanged <= 0 || aggregated.files.length >= expectedFilesChanged;
|
||||
const aggregationLooksComplete = expectedFilesChanged <= 0 || aggregated.stats.filesChanged === expectedFilesChanged;
|
||||
|
||||
if (aggregated.usedAggregation && aggregated.files.length > 0 && aggregationLooksComplete) {
|
||||
res.json(aggregated.files.map((file) => ({ path: file.path, status: file.status, diff: file.patch })));
|
||||
return;
|
||||
}
|
||||
|
||||
if (aggregated.usedAggregation && aggregated.files.length > 0) {
|
||||
res.json(aggregated.files.map((file) => ({ path: file.path, status: file.status, diff: file.patch })));
|
||||
return;
|
||||
}
|
||||
|
||||
if (!resolvedMergeSha) {
|
||||
res.json([]);
|
||||
return;
|
||||
}
|
||||
|
||||
const rootDir = scopedStore.getRootDir();
|
||||
const sha = task.mergeDetails.commitSha;
|
||||
const sha = resolvedMergeSha;
|
||||
|
||||
let diffSpec: Awaited<ReturnType<typeof resolveCommitDiffSpec>>;
|
||||
|
||||
@@ -759,10 +868,6 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
return;
|
||||
}
|
||||
|
||||
if (task.column === "done") {
|
||||
res.json([]);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!task.worktree) {
|
||||
const fallbackFiles = await tryBranchRefFallbackFileDiffs(task, scopedStore.getRootDir());
|
||||
@@ -820,13 +925,9 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
try {
|
||||
const committedOutput = (await runGitCommand(["diff", "--name-status", `${diffBase}..HEAD`], cwd, 5000)).trim();
|
||||
for (const line of committedOutput.split("\n").filter(Boolean)) {
|
||||
const parts = line.split("\t");
|
||||
const statusCode = parts[0] ?? "M";
|
||||
if (statusCode.startsWith("R")) {
|
||||
fileMap.set(parts[2] ?? parts[1] ?? "", { statusCode, oldPath: parts[1] });
|
||||
} else {
|
||||
fileMap.set(parts[1] ?? "", { statusCode });
|
||||
}
|
||||
const parsed = parseNameStatusLine(line);
|
||||
if (!parsed) continue;
|
||||
fileMap.set(parsed.path, { statusCode: parsed.statusCode, oldPath: parsed.oldPath });
|
||||
}
|
||||
} catch {
|
||||
// continue with working-tree-only changes
|
||||
@@ -836,16 +937,9 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
try {
|
||||
const stagedOutput = (await runGitCommand(["diff", "--cached", "--name-status"], cwd, 5000)).trim();
|
||||
for (const line of stagedOutput.split("\n").filter(Boolean)) {
|
||||
const parts = line.split("\t");
|
||||
const statusCode = parts[0] ?? "M";
|
||||
const filePath = parts[1] ?? "";
|
||||
if (filePath && !fileMap.has(filePath)) {
|
||||
if (statusCode.startsWith("R")) {
|
||||
fileMap.set(filePath, { statusCode, oldPath: parts[2] });
|
||||
} else {
|
||||
fileMap.set(filePath, { statusCode });
|
||||
}
|
||||
}
|
||||
const parsed = parseNameStatusLine(line);
|
||||
if (!parsed || fileMap.has(parsed.path)) continue;
|
||||
fileMap.set(parsed.path, { statusCode: parsed.statusCode, oldPath: parsed.oldPath });
|
||||
}
|
||||
} catch {
|
||||
// ignore staged diff failures
|
||||
@@ -854,16 +948,9 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
try {
|
||||
const workingTreeOutput = (await runGitCommand(["diff", "--name-status"], cwd, 5000)).trim();
|
||||
for (const line of workingTreeOutput.split("\n").filter(Boolean)) {
|
||||
const parts = line.split("\t");
|
||||
const statusCode = parts[0] ?? "M";
|
||||
const filePath = parts[1] ?? "";
|
||||
if (filePath && !fileMap.has(filePath)) {
|
||||
if (statusCode.startsWith("R")) {
|
||||
fileMap.set(filePath, { statusCode, oldPath: parts[2] });
|
||||
} else {
|
||||
fileMap.set(filePath, { statusCode });
|
||||
}
|
||||
}
|
||||
const parsed = parseNameStatusLine(line);
|
||||
if (!parsed || fileMap.has(parsed.path)) continue;
|
||||
fileMap.set(parsed.path, { statusCode: parsed.statusCode, oldPath: parsed.oldPath });
|
||||
}
|
||||
} catch {
|
||||
// ignore unstaged diff failures
|
||||
|
||||
Reference in New Issue
Block a user