feat(FN-675): align session-files endpoint with diff endpoint
- Add git diff for session-files using baseCommitSha with double-dot syntax - Add merge-base fallback to find common ancestor when baseCommitSha is missing - Add HEAD~1 fallback when merge-base fails (e.g., no remote main) - Add 10-second in-memory cache for session-files computation - Fix scheduler to immediately schedule tasks created via dashboard - Add task:moved to done handler for immediate scheduling on status change - Add comprehensive unit tests for scheduler immediate scheduling - Add unit tests for session-files caching and base ref resolution
This commit is contained in:
@@ -1,10 +1,10 @@
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { EventEmitter, once } from "node:events";
|
||||
import http from "node:http";
|
||||
import type { Task } from "@fusion/core";
|
||||
import { createServer } from "../server.js";
|
||||
import * as childProcess from "node:child_process";
|
||||
import * as fs from "node:fs";
|
||||
import type { Task } from "@fusion/core";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { get } from "../test-request.js";
|
||||
|
||||
vi.mock("node:child_process", async () => {
|
||||
const actual = await vi.importActual<typeof import("node:child_process")>("node:child_process");
|
||||
@@ -32,10 +32,6 @@ class MockStore extends EventEmitter {
|
||||
return process.cwd();
|
||||
}
|
||||
|
||||
async listTasks(): Promise<Task[]> {
|
||||
return Array.from(this.tasks.values());
|
||||
}
|
||||
|
||||
async getTask(id: string): Promise<Task> {
|
||||
const task = this.tasks.get(id);
|
||||
if (!task) {
|
||||
@@ -48,38 +44,6 @@ class MockStore extends EventEmitter {
|
||||
addTask(task: Task): void {
|
||||
this.tasks.set(task.id, task);
|
||||
}
|
||||
|
||||
getMissionStore() {
|
||||
return {
|
||||
listMissions: vi.fn().mockResolvedValue([]),
|
||||
getMission: vi.fn(),
|
||||
createMission: vi.fn(),
|
||||
updateMission: vi.fn(),
|
||||
deleteMission: vi.fn(),
|
||||
getMissionWithHierarchy: vi.fn(),
|
||||
listMilestones: vi.fn().mockResolvedValue([]),
|
||||
getMilestone: vi.fn(),
|
||||
addMilestone: vi.fn(),
|
||||
updateMilestone: vi.fn(),
|
||||
deleteMilestone: vi.fn(),
|
||||
reorderMilestones: vi.fn(),
|
||||
listSlices: vi.fn().mockResolvedValue([]),
|
||||
getSlice: vi.fn(),
|
||||
addSlice: vi.fn(),
|
||||
updateSlice: vi.fn(),
|
||||
deleteSlice: vi.fn(),
|
||||
reorderSlices: vi.fn(),
|
||||
activateSlice: vi.fn(),
|
||||
listFeatures: vi.fn().mockResolvedValue([]),
|
||||
getFeature: vi.fn(),
|
||||
addFeature: vi.fn(),
|
||||
updateFeature: vi.fn(),
|
||||
deleteFeature: vi.fn(),
|
||||
linkFeatureToTask: vi.fn(),
|
||||
unlinkFeatureFromTask: vi.fn(),
|
||||
getFeatureRollups: vi.fn().mockResolvedValue([]),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
function createTask(overrides: Partial<Task> = {}): Task {
|
||||
@@ -100,9 +64,24 @@ function createTask(overrides: Partial<Task> = {}): Task {
|
||||
};
|
||||
}
|
||||
|
||||
async function requestSessionFiles(app: Parameters<typeof get>[0], taskId = "FN-675"): Promise<{ status: number; body: any }> {
|
||||
const response = await get(app, `/api/tasks/${taskId}/session-files`);
|
||||
return { status: response.status, body: response.body };
|
||||
async function requestSessionFiles(port: number, taskId = "FN-675"): Promise<{ status: number; body: any }> {
|
||||
return await new Promise((resolve, reject) => {
|
||||
const req = http.request(
|
||||
{
|
||||
hostname: "127.0.0.1",
|
||||
port,
|
||||
path: `/api/tasks/${taskId}/session-files`,
|
||||
method: "GET",
|
||||
},
|
||||
(res) => {
|
||||
let data = "";
|
||||
res.on("data", (chunk) => (data += chunk));
|
||||
res.on("end", () => resolve({ status: res.statusCode!, body: JSON.parse(data) }));
|
||||
},
|
||||
);
|
||||
req.on("error", reject);
|
||||
req.end();
|
||||
});
|
||||
}
|
||||
|
||||
describe("GET /api/tasks/:id/session-files", () => {
|
||||
@@ -118,15 +97,145 @@ describe("GET /api/tasks/:id/session-files", () => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("uses baseCommitSha with double-dot syntax when available", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ baseCommitSha: "abc123" }));
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
if (String(command) === "git diff --name-only abc123..HEAD") {
|
||||
return "src/a.ts\nsrc/b.ts\n" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${String(command)}`);
|
||||
});
|
||||
|
||||
const app = createServer(store as any);
|
||||
const server = app.listen(0);
|
||||
await once(server, "listening");
|
||||
const port = (server.address() as { port: number }).port;
|
||||
|
||||
const response = await requestSessionFiles(port);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual(["src/a.ts", "src/b.ts"]);
|
||||
expect(mockExecSync).toHaveBeenCalledWith("git diff --name-only abc123..HEAD", expect.objectContaining({ cwd: "/tmp/fn-675" }));
|
||||
expect(mockExecSync).not.toHaveBeenCalledWith(expect.stringContaining("...HEAD"), expect.anything());
|
||||
|
||||
server.close();
|
||||
await once(server, "close");
|
||||
});
|
||||
|
||||
it("computes fallback base ref with merge-base and returns matching file list", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ baseCommitSha: undefined }));
|
||||
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 diff --name-only mergebase123..HEAD") {
|
||||
return "packages/dashboard/src/routes.ts\npackages/dashboard/app/components/TaskCard.tsx\n" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${String(command)}`);
|
||||
});
|
||||
|
||||
const app = createServer(store as any);
|
||||
const server = app.listen(0);
|
||||
await once(server, "listening");
|
||||
const port = (server.address() as { port: number }).port;
|
||||
|
||||
const response = await requestSessionFiles(port);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual([
|
||||
"packages/dashboard/src/routes.ts",
|
||||
"packages/dashboard/app/components/TaskCard.tsx",
|
||||
]);
|
||||
expect(mockExecSync).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
"git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main",
|
||||
expect.objectContaining({ cwd: "/tmp/fn-675" }),
|
||||
);
|
||||
expect(mockExecSync).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
"git diff --name-only mergebase123..HEAD",
|
||||
expect.objectContaining({ cwd: "/tmp/fn-675" }),
|
||||
);
|
||||
|
||||
server.close();
|
||||
await once(server, "close");
|
||||
});
|
||||
|
||||
it("falls back to HEAD~1 when merge-base fails", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ baseCommitSha: undefined }));
|
||||
mockExecSync.mockImplementation((command) => {
|
||||
if (String(command) === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
|
||||
throw new Error("merge-base failed");
|
||||
}
|
||||
if (String(command) === "git rev-parse HEAD~1") {
|
||||
return "parent123\n" as any;
|
||||
}
|
||||
if (String(command) === "git diff --name-only parent123..HEAD") {
|
||||
return "src/only.ts\n" as any;
|
||||
}
|
||||
throw new Error(`Unexpected command: ${String(command)}`);
|
||||
});
|
||||
|
||||
const app = createServer(store as any);
|
||||
const server = app.listen(0);
|
||||
await once(server, "listening");
|
||||
const port = (server.address() as { port: number }).port;
|
||||
|
||||
const response = await requestSessionFiles(port);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual(["src/only.ts"]);
|
||||
|
||||
server.close();
|
||||
await once(server, "close");
|
||||
});
|
||||
|
||||
it("returns empty array when worktree is missing", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ worktree: undefined }));
|
||||
|
||||
const app = createServer(store as any);
|
||||
const response = await requestSessionFiles(app);
|
||||
const server = app.listen(0);
|
||||
await once(server, "listening");
|
||||
const port = (server.address() as { port: number }).port;
|
||||
|
||||
const response = await requestSessionFiles(port);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual([]);
|
||||
expect(mockExecSync).not.toHaveBeenCalled();
|
||||
|
||||
server.close();
|
||||
await once(server, "close");
|
||||
});
|
||||
|
||||
it("uses the 10-second cache before recomputing", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ baseCommitSha: "cachebase" }));
|
||||
mockExecSync.mockReturnValue("cached/file.ts\n" as any);
|
||||
|
||||
const app = createServer(store as any);
|
||||
const server = app.listen(0);
|
||||
await once(server, "listening");
|
||||
const port = (server.address() as { port: number }).port;
|
||||
|
||||
const first = await requestSessionFiles(port);
|
||||
const second = await requestSessionFiles(port);
|
||||
|
||||
expect(first.body).toEqual(["cached/file.ts"]);
|
||||
expect(second.body).toEqual(["cached/file.ts"]);
|
||||
expect(mockExecSync).toHaveBeenCalledTimes(1);
|
||||
|
||||
vi.advanceTimersByTime(10001);
|
||||
const third = await requestSessionFiles(port);
|
||||
|
||||
expect(third.body).toEqual(["cached/file.ts"]);
|
||||
expect(mockExecSync).toHaveBeenCalledTimes(2);
|
||||
|
||||
server.close();
|
||||
await once(server, "close");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1813,24 +1813,42 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
return;
|
||||
}
|
||||
|
||||
const baseBranch = task.baseBranch ?? "main";
|
||||
let files: string[] = [];
|
||||
|
||||
try {
|
||||
const output = execSync(`git diff --name-only ${baseBranch}...HEAD`, {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
let baseRef = task.baseCommitSha;
|
||||
|
||||
files = output ? output.split("\n").filter(Boolean) : [];
|
||||
if (!baseRef) {
|
||||
try {
|
||||
baseRef = execSync("git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main", {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
} catch {
|
||||
try {
|
||||
baseRef = execSync("git rev-parse HEAD~1", {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
} catch {
|
||||
baseRef = undefined;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (baseRef) {
|
||||
const output = execSync(`git diff --name-only ${baseRef}..HEAD`, {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
|
||||
files = output ? output.split("\n").filter(Boolean) : [];
|
||||
}
|
||||
} catch {
|
||||
const fallback = execSync("git diff --name-only HEAD", {
|
||||
cwd: task.worktree,
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
}).trim();
|
||||
files = fallback ? fallback.split("\n").filter(Boolean) : [];
|
||||
files = [];
|
||||
}
|
||||
|
||||
sessionFilesCache.set(task.id, {
|
||||
|
||||
@@ -138,6 +138,105 @@ describe("Scheduler", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("event-driven scheduling", () => {
|
||||
it("registers task:created event listener", () => {
|
||||
const store = createMockStore();
|
||||
new Scheduler(store);
|
||||
// Verify task:created listener is registered
|
||||
expect(store.on).toHaveBeenCalledWith("task:created", expect.any(Function));
|
||||
});
|
||||
|
||||
it("triggers scheduling immediately when task:created event fires", async () => {
|
||||
const store = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue([
|
||||
createMockTask({ id: "FN-001", column: "todo", dependencies: [] }),
|
||||
]),
|
||||
getSettings: vi.fn().mockResolvedValue({ maxConcurrent: 2, maxWorktrees: 4 }),
|
||||
updateTask: vi.fn().mockResolvedValue(undefined),
|
||||
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store);
|
||||
scheduler.start();
|
||||
|
||||
// Find and call the task:created handler
|
||||
const onCalls = (store.on as any).mock.calls;
|
||||
const createdHandler = onCalls.find((call: any) => call[0] === "task:created")?.[1];
|
||||
expect(createdHandler).toBeDefined();
|
||||
|
||||
// Simulate task:created event
|
||||
const newTask = createMockTask({ id: "FN-002", column: "todo" });
|
||||
await createdHandler(newTask);
|
||||
|
||||
// Verify schedule() was called (moveTask should be called since task can start)
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-002", "in-progress");
|
||||
});
|
||||
|
||||
it("registers task:moved event listener", () => {
|
||||
const store = createMockStore();
|
||||
new Scheduler(store);
|
||||
// Verify task:moved listener is registered
|
||||
expect(store.on).toHaveBeenCalledWith("task:moved", expect.any(Function));
|
||||
});
|
||||
|
||||
it("triggers scheduling immediately when task:moved to done event fires", async () => {
|
||||
const store = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue([
|
||||
createMockTask({ id: "FN-001", column: "done", dependencies: [] }), // Completed dep
|
||||
createMockTask({ id: "FN-002", column: "todo", dependencies: ["FN-001"] }), // Waiting on FN-001
|
||||
]),
|
||||
getSettings: vi.fn().mockResolvedValue({ maxConcurrent: 2, maxWorktrees: 4 }),
|
||||
updateTask: vi.fn().mockResolvedValue(undefined),
|
||||
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store);
|
||||
scheduler.start();
|
||||
|
||||
// Find and call the task:moved handler
|
||||
const onCalls = (store.on as any).mock.calls;
|
||||
const movedHandler = onCalls.find((call: any) => call[0] === "task:moved")?.[1];
|
||||
expect(movedHandler).toBeDefined();
|
||||
|
||||
// Simulate task:moved to done event
|
||||
const doneTask = createMockTask({ id: "FN-001", column: "todo" });
|
||||
await movedHandler({ task: doneTask, from: "in-progress", to: "done" });
|
||||
|
||||
// Verify schedule() was called - FN-002 should now be able to start
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-002", "in-progress");
|
||||
});
|
||||
|
||||
it("does not trigger scheduling for non-done task:moved events", async () => {
|
||||
const store = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue([
|
||||
createMockTask({ id: "FN-001", column: "todo", dependencies: [] }),
|
||||
]),
|
||||
getSettings: vi.fn().mockResolvedValue({ maxConcurrent: 2, maxWorktrees: 4 }),
|
||||
updateTask: vi.fn().mockResolvedValue(undefined),
|
||||
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store);
|
||||
scheduler.start();
|
||||
|
||||
// Clear previous calls
|
||||
(store.moveTask as any).mockClear();
|
||||
|
||||
// Find and call the task:moved handler
|
||||
const onCalls = (store.on as any).mock.calls;
|
||||
const movedHandler = onCalls.find((call: any) => call[0] === "task:moved")?.[1];
|
||||
|
||||
// Simulate task:moved to in-progress (not done)
|
||||
const task = createMockTask({ id: "FN-001", column: "in-progress" });
|
||||
await movedHandler({ task, from: "todo", to: "in-progress" });
|
||||
|
||||
// Should NOT have triggered additional scheduling (no new task moved to in-progress)
|
||||
// Note: The existing handler runs, but it doesn't call schedule() for non-done transitions
|
||||
// So moveTask won't be called for a task already in in-progress
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("start/stop", () => {
|
||||
it("starts and stops the scheduler", () => {
|
||||
const store = createMockStore();
|
||||
|
||||
@@ -94,6 +94,16 @@ export class Scheduler {
|
||||
private store: TaskStore,
|
||||
private options: SchedulerOptions = {},
|
||||
) {
|
||||
/**
|
||||
* Event-driven scheduling: when a task is created, trigger a scheduling
|
||||
* pass immediately instead of waiting for the next poll interval.
|
||||
* This reduces latency from up to 15 seconds to near-instant.
|
||||
*/
|
||||
this.store.on("task:created", () => {
|
||||
schedulerLog.log("Task created — triggering scheduling");
|
||||
this.schedule();
|
||||
});
|
||||
|
||||
/**
|
||||
* Immediate unpause resume: when `globalPause` transitions from `true`
|
||||
* to `false`, trigger a scheduling pass right away instead of waiting
|
||||
@@ -152,6 +162,14 @@ export class Scheduler {
|
||||
if (task.sliceId && this.options.missionStore && to === "in-progress") {
|
||||
void this.handleMissionTaskStart(task.id, task.sliceId);
|
||||
}
|
||||
|
||||
// Event-driven scheduling: when a dependency completes (task moves to "done"),
|
||||
// trigger scheduling immediately so waiting tasks can start without waiting
|
||||
// for the next poll interval (up to 15 seconds).
|
||||
if (to === "done") {
|
||||
schedulerLog.log("Task completed — triggering scheduling");
|
||||
this.schedule();
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user