feat(KB-651): add changed files diff viewer to dashboard
- Add backend API endpoint /api/tasks/:id/diff for file diffs - Add useChangedFiles hook for fetching and managing diff state - Create ChangedFilesModal component with file list and diff viewer - Integrate modal with TaskCard via files changed badge click - Add CSS styling for changed files layout and diff display - Add keyboard navigation (Escape to close) - Include unit tests for ChangedFilesModal component
This commit is contained in:
gsxdsm
@@ -7,7 +7,7 @@ import { EventEmitter } from "node:events";
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { execFileSync, execSync } from "node:child_process";
|
||||
import { createApiRoutes } from "./routes.js";
|
||||
import { GitHubClient } from "./github.js";
|
||||
import { githubRateLimiter } from "./github-poll.js";
|
||||
@@ -3792,6 +3792,94 @@ describe("POST /tasks/:id/reject-plan", () => {
|
||||
// --- Git Management route tests ---
|
||||
// These are integration tests that run against the actual git repository
|
||||
|
||||
describe("GET /tasks/:id/file-diffs", () => {
|
||||
let store: TaskStore;
|
||||
let worktreeDir: string;
|
||||
let testRoot: string;
|
||||
|
||||
beforeEach(() => {
|
||||
testRoot = mkdtempSync(join(tmpdir(), "kb-dashboard-file-diffs-"));
|
||||
worktreeDir = join(testRoot, "repo");
|
||||
mkdirSync(worktreeDir, { recursive: true });
|
||||
execFileSync("git", ["init", "-b", "main", worktreeDir]);
|
||||
execFileSync("git", ["-C", worktreeDir, "config", "user.email", "kb-tests@example.com"]);
|
||||
execFileSync("git", ["-C", worktreeDir, "config", "user.name", "KB Tests"]);
|
||||
writeFileSync(join(worktreeDir, "README.md"), "base\n");
|
||||
writeFileSync(join(worktreeDir, "keep.txt"), "keep\n");
|
||||
execFileSync("git", ["-C", worktreeDir, "add", "."]);
|
||||
execFileSync("git", ["-C", worktreeDir, "commit", "-m", "base"]);
|
||||
|
||||
store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "KB-651",
|
||||
worktree: worktreeDir,
|
||||
baseBranch: "main",
|
||||
}),
|
||||
getRootDir: vi.fn().mockReturnValue(worktreeDir),
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(testRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
it("returns changed files with statuses and diffs", async () => {
|
||||
writeFileSync(join(worktreeDir, "README.md"), "base\nchanged\n");
|
||||
writeFileSync(join(worktreeDir, "added.txt"), "new file\n");
|
||||
execFileSync("git", ["-C", worktreeDir, "rm", "keep.txt"]);
|
||||
expect(execSync("git status --short", { cwd: worktreeDir, encoding: "utf-8" })).not.toBe("");
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-651/file-diffs");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ path: "README.md", status: "modified", diff: expect.stringContaining("+changed") }),
|
||||
expect.objectContaining({ path: "added.txt", status: "added", diff: expect.stringContaining("+++ b/added.txt") }),
|
||||
expect.objectContaining({ path: "keep.txt", status: "deleted", diff: expect.stringContaining("--- a/keep.txt") }),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns renamed files with oldPath", async () => {
|
||||
execFileSync("git", ["-C", worktreeDir, "mv", "keep.txt", "renamed.txt"]);
|
||||
expect(execSync("git status --short", { cwd: worktreeDir, encoding: "utf-8" })).not.toBe("");
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-651/file-diffs");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual([
|
||||
expect.objectContaining({ path: "renamed.txt", oldPath: "keep.txt", status: "renamed" }),
|
||||
]);
|
||||
});
|
||||
|
||||
it("returns empty array when worktree is missing", async () => {
|
||||
store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({ ...FAKE_TASK_DETAIL, id: "KB-651", worktree: join(testRoot, "missing"), baseBranch: "main" }),
|
||||
});
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-651/file-diffs");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual([]);
|
||||
});
|
||||
|
||||
it("returns empty array when there are no changes", async () => {
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-651/file-diffs");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Git Management endpoints", () => {
|
||||
let store: TaskStore;
|
||||
let gitRepoDir: string;
|
||||
|
||||
@@ -1062,6 +1062,13 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
console.debug("[planning:routes:registered]", planningRoutes);
|
||||
}
|
||||
const sessionFilesCache = new Map<string, { files: string[]; expiresAt: number }>();
|
||||
const taskFileDiffsCache = new Map<
|
||||
string,
|
||||
{
|
||||
files: Array<{ path: string; status: "added" | "modified" | "deleted" | "renamed"; diff: string; oldPath?: string }>;
|
||||
expiresAt: number;
|
||||
}
|
||||
>();
|
||||
|
||||
// Get GitHub token from options or env
|
||||
const githubToken = options?.githubToken ?? process.env.GITHUB_TOKEN;
|
||||
@@ -1949,6 +1956,141 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
}
|
||||
});
|
||||
|
||||
router.get("/tasks/:id/file-diffs", async (req, res) => {
|
||||
try {
|
||||
const task = await store.getTask(req.params.id);
|
||||
if (!task.worktree || !existsSync(task.worktree)) {
|
||||
res.json([]);
|
||||
return;
|
||||
}
|
||||
|
||||
const cached = taskFileDiffsCache.get(task.id);
|
||||
if (cached && cached.expiresAt > Date.now()) {
|
||||
res.json(cached.files);
|
||||
return;
|
||||
}
|
||||
|
||||
const baseBranch = task.baseBranch ?? "main";
|
||||
type TaskFileDiff = { path: string; status: "added" | "modified" | "deleted" | "renamed"; diff: string; oldPath?: string };
|
||||
let files: TaskFileDiff[] = [];
|
||||
|
||||
const parseNameStatus = (output: string): TaskFileDiff[] => {
|
||||
return output
|
||||
.split("\n")
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
.map((line) => {
|
||||
const parts = line.split("\t");
|
||||
const rawStatus = parts[0] ?? "M";
|
||||
const statusCode = rawStatus[0];
|
||||
|
||||
if (statusCode === "R") {
|
||||
const oldPath = parts[1];
|
||||
const path = parts[2];
|
||||
return {
|
||||
path,
|
||||
oldPath,
|
||||
status: "renamed" as const,
|
||||
diff: "",
|
||||
};
|
||||
}
|
||||
|
||||
const path = parts[1];
|
||||
return {
|
||||
path,
|
||||
status:
|
||||
statusCode === "A"
|
||||
? ("added" as const)
|
||||
: statusCode === "D"
|
||||
? ("deleted" as const)
|
||||
: ("modified" as const),
|
||||
diff: "",
|
||||
};
|
||||
})
|
||||
.filter((entry): entry is TaskFileDiff => Boolean(entry.path));
|
||||
};
|
||||
|
||||
try {
|
||||
const output = execSync(`git diff --name-status ${baseBranch}...HEAD`, {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
|
||||
files = output ? parseNameStatus(output) : [];
|
||||
} catch {
|
||||
try {
|
||||
const fallback = execSync("git diff --name-status HEAD", {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
files = fallback ? parseNameStatus(fallback) : [];
|
||||
} catch {
|
||||
const workingTreeFallback = execSync("git status --short", {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
|
||||
files = workingTreeFallback
|
||||
? workingTreeFallback
|
||||
.split("\n")
|
||||
.map((line) => line.trimEnd())
|
||||
.filter(Boolean)
|
||||
.map((line) => {
|
||||
const indexStatus = line[0] ?? " ";
|
||||
const worktreeStatus = line[1] ?? " ";
|
||||
const statusCode = indexStatus !== " " ? indexStatus : worktreeStatus;
|
||||
const remainder = line.slice(3).trim();
|
||||
const normalized = statusCode === "R"
|
||||
? `R\t${remainder.replace(/\s+->\s+/, "\t")}`
|
||||
: `${statusCode || "M"}\t${remainder}`;
|
||||
return normalized;
|
||||
})
|
||||
.map((line) => parseNameStatus(line)[0])
|
||||
.filter((entry): entry is TaskFileDiff => Boolean(entry))
|
||||
: [];
|
||||
}
|
||||
}
|
||||
|
||||
if (files.length === 0) {
|
||||
taskFileDiffsCache.set(task.id, {
|
||||
files: [],
|
||||
expiresAt: Date.now() + 10000,
|
||||
});
|
||||
res.json([]);
|
||||
return;
|
||||
}
|
||||
|
||||
const filesWithDiffs = files.map((file) => {
|
||||
try {
|
||||
const diff = execSync(`git diff ${baseBranch}...HEAD -- "${file.path.replace(/"/g, '\\"')}"`, {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 10000,
|
||||
});
|
||||
return { ...file, diff };
|
||||
} catch {
|
||||
return file;
|
||||
}
|
||||
});
|
||||
|
||||
taskFileDiffsCache.set(task.id, {
|
||||
files: filesWithDiffs,
|
||||
expiresAt: Date.now() + 10000,
|
||||
});
|
||||
|
||||
res.json(filesWithDiffs);
|
||||
} catch (err: any) {
|
||||
if (err.code === "ENOENT") {
|
||||
res.status(404).json({ error: `Task ${req.params.id} not found` });
|
||||
} else {
|
||||
res.status(500).json({ error: err.message || "Internal server error" });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* GET /api/tasks/:id/diff
|
||||
* Get detailed diff information for files modified during task execution.
|
||||
|
||||
Reference in New Issue
Block a user