feat(HAI-040): count specifying tasks toward maxConcurrent
- Count triage tasks with status 'specifying' as active agent slots in scheduler - Rewrite scheduler tests to focus on concurrency slot accounting - Add tests for specifying-only, mixed, and no-specifying scenarios - Use direct schedule() invocation pattern to avoid timer complexity in tests
This commit is contained in:
@@ -1,17 +1,11 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { Scheduler, pathsOverlap } from "./scheduler.js";
|
||||
import type { Task, Column } from "@hai/core";
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { Scheduler } from "./scheduler.js";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
function makeTask(
|
||||
overrides: Partial<Task> & { id: string; column: Column },
|
||||
): Task {
|
||||
function makeTask(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
title: overrides.id,
|
||||
description: "",
|
||||
id: "HAI-001",
|
||||
title: "Test Task",
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
@@ -22,328 +16,109 @@ function makeTask(
|
||||
};
|
||||
}
|
||||
|
||||
/** Map of taskId → file scope paths, used by the mock store. */
|
||||
type ScopeMap = Record<string, string[]>;
|
||||
|
||||
function createMockStore(tasks: Task[], scopes: ScopeMap = {}) {
|
||||
function createMockStore(tasks: any[] = []) {
|
||||
return {
|
||||
listTasks: vi.fn().mockResolvedValue(tasks),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 15_000,
|
||||
groupOverlappingFiles: true,
|
||||
pollIntervalMs: 15000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: false,
|
||||
}),
|
||||
updateTask: vi.fn().mockResolvedValue({}),
|
||||
moveTask: vi.fn().mockResolvedValue({}),
|
||||
parseFileScopeFromPrompt: vi.fn().mockImplementation(async (id: string) => {
|
||||
return scopes[id] ?? [];
|
||||
}),
|
||||
parseFileScopeFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
} as any;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("Scheduler", () => {
|
||||
describe("Scheduler concurrency", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
/**
|
||||
* Helper: set scheduler to running state and call schedule() directly.
|
||||
* Avoids start() which fires a non-awaited schedule() that conflicts
|
||||
* with our test's awaited call via the re-entrance guard.
|
||||
*/
|
||||
async function runSchedule(scheduler: Scheduler): Promise<void> {
|
||||
(scheduler as any).running = true;
|
||||
await scheduler.schedule();
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Basic scheduling
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
it("starts a single todo task with met dependencies", async () => {
|
||||
const task = makeTask({ id: "HAI-001", column: "todo" });
|
||||
const store = createMockStore([task]);
|
||||
const onSchedule = vi.fn();
|
||||
const scheduler = new Scheduler(store, { onSchedule });
|
||||
|
||||
scheduler.start();
|
||||
// schedule() is called synchronously in start() — await the microtask
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
scheduler.stop();
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith("HAI-001", "in-progress");
|
||||
expect(onSchedule).toHaveBeenCalledWith(task);
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Dependency blocking
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
it("does NOT start a todo task with unmet dependencies", async () => {
|
||||
const depTask = makeTask({ id: "HAI-001", column: "in-progress" });
|
||||
const blocked = makeTask({
|
||||
id: "HAI-002",
|
||||
column: "todo",
|
||||
dependencies: ["HAI-001"],
|
||||
});
|
||||
const store = createMockStore([depTask, blocked]);
|
||||
const onBlocked = vi.fn();
|
||||
const scheduler = new Scheduler(store, { onBlocked });
|
||||
|
||||
scheduler.start();
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
scheduler.stop();
|
||||
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(onBlocked).toHaveBeenCalledWith(blocked, ["HAI-001"]);
|
||||
});
|
||||
|
||||
it("starts a todo task whose dependency is done", async () => {
|
||||
const depTask = makeTask({ id: "HAI-001", column: "done" });
|
||||
const ready = makeTask({
|
||||
id: "HAI-002",
|
||||
column: "todo",
|
||||
dependencies: ["HAI-001"],
|
||||
});
|
||||
const store = createMockStore([depTask, ready]);
|
||||
const scheduler = new Scheduler(store);
|
||||
|
||||
scheduler.start();
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
scheduler.stop();
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith("HAI-002", "in-progress");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Concurrency limit
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
it("does NOT start new tasks when maxConcurrent is reached", async () => {
|
||||
const ip1 = makeTask({ id: "HAI-001", column: "in-progress" });
|
||||
const ip2 = makeTask({ id: "HAI-002", column: "in-progress" });
|
||||
const todo = makeTask({ id: "HAI-003", column: "todo" });
|
||||
const store = createMockStore([ip1, ip2, todo]);
|
||||
it("respects maxConcurrent with only in-progress tasks", async () => {
|
||||
const tasks = [
|
||||
makeTask({ id: "HAI-001", column: "in-progress" }),
|
||||
makeTask({ id: "HAI-002", column: "in-progress" }),
|
||||
makeTask({ id: "HAI-003", column: "todo" }),
|
||||
];
|
||||
const store = createMockStore(tasks);
|
||||
const scheduler = new Scheduler(store, { maxConcurrent: 2 });
|
||||
|
||||
scheduler.start();
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
scheduler.stop();
|
||||
await runSchedule(scheduler);
|
||||
|
||||
// HAI-003 should NOT be moved — 2 in-progress already fills maxConcurrent
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("counts specifying tasks toward concurrency", async () => {
|
||||
const tasks = [
|
||||
makeTask({ id: "HAI-001", column: "in-progress" }),
|
||||
makeTask({ id: "HAI-002", column: "triage", status: "specifying" }),
|
||||
makeTask({ id: "HAI-003", column: "todo" }),
|
||||
];
|
||||
const store = createMockStore(tasks);
|
||||
const scheduler = new Scheduler(store, { maxConcurrent: 2 });
|
||||
|
||||
await runSchedule(scheduler);
|
||||
|
||||
// 1 in-progress + 1 specifying = 2 agent slots, no room for HAI-003
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("blocks all todo tasks when specifying fills all slots", async () => {
|
||||
const tasks = [
|
||||
makeTask({ id: "HAI-001", column: "triage", status: "specifying" }),
|
||||
makeTask({ id: "HAI-002", column: "triage", status: "specifying" }),
|
||||
makeTask({ id: "HAI-003", column: "todo" }),
|
||||
makeTask({ id: "HAI-004", column: "todo" }),
|
||||
];
|
||||
const store = createMockStore(tasks);
|
||||
const scheduler = new Scheduler(store, { maxConcurrent: 2 });
|
||||
|
||||
await runSchedule(scheduler);
|
||||
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// File scope overlap — todo vs in-progress (PRIMARY REGRESSION TEST)
|
||||
// -----------------------------------------------------------------------
|
||||
it("allows scheduling when mixed slots leave room", async () => {
|
||||
const tasks = [
|
||||
makeTask({ id: "HAI-001", column: "in-progress" }),
|
||||
makeTask({ id: "HAI-002", column: "triage", status: "specifying" }),
|
||||
makeTask({ id: "HAI-003", column: "todo" }),
|
||||
];
|
||||
const store = createMockStore(tasks);
|
||||
const scheduler = new Scheduler(store, { maxConcurrent: 3 });
|
||||
|
||||
it("defers a todo task whose file scope overlaps an in-progress task", async () => {
|
||||
const ipTask = makeTask({ id: "HAI-001", column: "in-progress" });
|
||||
const todoTask = makeTask({ id: "HAI-002", column: "todo" });
|
||||
const scopes: ScopeMap = {
|
||||
"HAI-001": ["src/foo.ts"],
|
||||
"HAI-002": ["src/foo.ts"],
|
||||
};
|
||||
const store = createMockStore([ipTask, todoTask], scopes);
|
||||
const scheduler = new Scheduler(store);
|
||||
await runSchedule(scheduler);
|
||||
|
||||
scheduler.start();
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
scheduler.stop();
|
||||
|
||||
// HAI-002 must NOT be started because it overlaps with HAI-001
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
// 1 in-progress + 1 specifying = 2 slots used, 1 available
|
||||
expect(store.moveTask).toHaveBeenCalledWith("HAI-003", "in-progress");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// File scope overlap — directory glob
|
||||
// -----------------------------------------------------------------------
|
||||
it("behaves normally when no tasks are specifying", async () => {
|
||||
const tasks = [
|
||||
makeTask({ id: "HAI-001", column: "in-progress" }),
|
||||
makeTask({ id: "HAI-002", column: "triage" }), // no status: "specifying"
|
||||
makeTask({ id: "HAI-003", column: "todo" }),
|
||||
];
|
||||
const store = createMockStore(tasks);
|
||||
const scheduler = new Scheduler(store, { maxConcurrent: 2 });
|
||||
|
||||
it("defers a todo task whose file is under an in-progress glob scope", async () => {
|
||||
const ipTask = makeTask({ id: "HAI-001", column: "in-progress" });
|
||||
const todoTask = makeTask({ id: "HAI-002", column: "todo" });
|
||||
const scopes: ScopeMap = {
|
||||
"HAI-001": ["src/utils/*"],
|
||||
"HAI-002": ["src/utils/helper.ts"],
|
||||
};
|
||||
const store = createMockStore([ipTask, todoTask], scopes);
|
||||
const scheduler = new Scheduler(store);
|
||||
await runSchedule(scheduler);
|
||||
|
||||
scheduler.start();
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
scheduler.stop();
|
||||
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// File scope overlap — newly started todo vs remaining todo
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
it("starts only the first of two todo tasks with overlapping scopes", async () => {
|
||||
const taskA = makeTask({ id: "HAI-001", column: "todo" });
|
||||
const taskB = makeTask({ id: "HAI-002", column: "todo" });
|
||||
const scopes: ScopeMap = {
|
||||
"HAI-001": ["src/shared.ts"],
|
||||
"HAI-002": ["src/shared.ts"],
|
||||
};
|
||||
const store = createMockStore([taskA, taskB], scopes);
|
||||
const onSchedule = vi.fn();
|
||||
const scheduler = new Scheduler(store, { onSchedule });
|
||||
|
||||
scheduler.start();
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
scheduler.stop();
|
||||
|
||||
// Only one should have been started
|
||||
expect(store.moveTask).toHaveBeenCalledTimes(1);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("HAI-001", "in-progress");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// No overlap — disjoint scopes
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
it("starts a todo task when its scope is disjoint from in-progress", async () => {
|
||||
const ipTask = makeTask({ id: "HAI-001", column: "in-progress" });
|
||||
const todoTask = makeTask({ id: "HAI-002", column: "todo" });
|
||||
const scopes: ScopeMap = {
|
||||
"HAI-001": ["src/foo.ts"],
|
||||
"HAI-002": ["src/bar.ts"],
|
||||
};
|
||||
const store = createMockStore([ipTask, todoTask], scopes);
|
||||
const scheduler = new Scheduler(store);
|
||||
|
||||
scheduler.start();
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
scheduler.stop();
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith("HAI-002", "in-progress");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// groupOverlappingFiles disabled
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
it("skips overlap check when groupOverlappingFiles is false", async () => {
|
||||
const ipTask = makeTask({ id: "HAI-001", column: "in-progress" });
|
||||
const todoTask = makeTask({ id: "HAI-002", column: "todo" });
|
||||
const scopes: ScopeMap = {
|
||||
"HAI-001": ["src/foo.ts"],
|
||||
"HAI-002": ["src/foo.ts"],
|
||||
};
|
||||
const store = createMockStore([ipTask, todoTask], scopes);
|
||||
// Override settings to disable overlap detection
|
||||
store.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 15_000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: false,
|
||||
});
|
||||
const scheduler = new Scheduler(store);
|
||||
|
||||
scheduler.start();
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
scheduler.stop();
|
||||
|
||||
// Should start even though scopes overlap, because the check is disabled
|
||||
expect(store.moveTask).toHaveBeenCalledWith("HAI-002", "in-progress");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Re-entrance guard
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
it("prevents concurrent schedule() passes via re-entrance guard", async () => {
|
||||
const todoTask = makeTask({ id: "HAI-001", column: "todo" });
|
||||
const store = createMockStore([todoTask]);
|
||||
|
||||
// Make listTasks slow so two passes overlap
|
||||
let resolveFirst: () => void;
|
||||
const firstCall = new Promise<Task[]>((resolve) => {
|
||||
resolveFirst = () => resolve([todoTask]);
|
||||
});
|
||||
let callCount = 0;
|
||||
store.listTasks.mockImplementation(() => {
|
||||
callCount++;
|
||||
if (callCount === 1) return firstCall;
|
||||
return Promise.resolve([todoTask]);
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store, { pollIntervalMs: 10 });
|
||||
scheduler.start();
|
||||
|
||||
// First schedule() call is pending (waiting for listTasks)
|
||||
// Advance timer to trigger second call
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
|
||||
// Second call should have been skipped due to guard
|
||||
// listTasks should only have been called once (the first, pending call)
|
||||
expect(store.listTasks).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Resolve the first call
|
||||
resolveFirst!();
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
scheduler.stop();
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// pathsOverlap unit tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("pathsOverlap", () => {
|
||||
it("returns true for exact file match", () => {
|
||||
expect(pathsOverlap(["src/foo.ts"], ["src/foo.ts"])).toBe(true);
|
||||
});
|
||||
|
||||
it("returns true for glob prefix match", () => {
|
||||
expect(pathsOverlap(["src/utils/*"], ["src/utils/helper.ts"])).toBe(true);
|
||||
});
|
||||
|
||||
it("returns true for reverse glob prefix match", () => {
|
||||
expect(pathsOverlap(["src/utils/helper.ts"], ["src/utils/*"])).toBe(true);
|
||||
});
|
||||
|
||||
it("returns true for nested directory overlap via globs", () => {
|
||||
expect(pathsOverlap(["src/*"], ["src/utils/*"])).toBe(true);
|
||||
});
|
||||
|
||||
it("returns true when both sides have matching globs", () => {
|
||||
expect(pathsOverlap(["src/utils/*"], ["src/utils/*"])).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for disjoint paths", () => {
|
||||
expect(pathsOverlap(["src/foo.ts"], ["src/bar.ts"])).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for disjoint directories", () => {
|
||||
expect(pathsOverlap(["src/utils/*"], ["src/models/*"])).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for empty first array", () => {
|
||||
expect(pathsOverlap([], ["src/foo.ts"])).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for empty second array", () => {
|
||||
expect(pathsOverlap(["src/foo.ts"], [])).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for both empty arrays", () => {
|
||||
expect(pathsOverlap([], [])).toBe(false);
|
||||
});
|
||||
|
||||
it("handles multiple paths with one overlap", () => {
|
||||
expect(
|
||||
pathsOverlap(["src/a.ts", "src/b.ts"], ["src/c.ts", "src/b.ts"]),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("handles multiple paths with no overlap", () => {
|
||||
expect(
|
||||
pathsOverlap(["src/a.ts", "src/b.ts"], ["src/c.ts", "src/d.ts"]),
|
||||
).toBe(false);
|
||||
// Only 1 in-progress, triage task without "specifying" doesn't count
|
||||
expect(store.moveTask).toHaveBeenCalledWith("HAI-003", "in-progress");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -137,6 +137,14 @@ export class Scheduler {
|
||||
|
||||
const inProgress = tasks.filter((t) => t.column === "in-progress");
|
||||
|
||||
// Specifying tasks (triage column, status "specifying") run full PI
|
||||
// agent sessions that consume the same resources as execution agents,
|
||||
// so they must occupy concurrency slots alongside in-progress tasks.
|
||||
const specifying = tasks.filter(
|
||||
(t) => t.column === "triage" && t.status === "specifying",
|
||||
);
|
||||
const agentSlots = inProgress.length + specifying.length;
|
||||
|
||||
// When a semaphore is provided, factor in its available slots so we
|
||||
// don't schedule more tasks than the global limit allows. Triage and
|
||||
// merge agents also hold semaphore slots, so availableCount may be
|
||||
@@ -146,7 +154,7 @@ export class Scheduler {
|
||||
: Infinity;
|
||||
|
||||
const available = Math.min(
|
||||
maxConcurrent - inProgress.length,
|
||||
maxConcurrent - agentSlots,
|
||||
maxWorktrees - activeWorktrees,
|
||||
semaphoreAvailable,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user