feat(FN-1259): add review handoff mechanism for user assignment
- Add assigneeUserId field to Task type and SQLite schema for human assignment - Add reviewHandoffPolicy setting to control automatic handoff behavior - Implement handoff detection in executor: detect user assignment during review and auto-transition task - Add dashboard API routes for user assignment, handoff queries, and completion - Add frontend API functions: getHandoffTask, assignTaskToUser, completeHandoff - Add comprehensive tests for store methods, API routes, and executor handoff logic - Update memory documentation with review handoff pattern
This commit is contained in:
@@ -98,6 +98,7 @@ const PROJECT_KEYS: (keyof ProjectSettings)[] = [
|
||||
"reflectionEnabled",
|
||||
"reflectionIntervalMs",
|
||||
"reflectionAfterTask",
|
||||
"reviewHandoffPolicy",
|
||||
];
|
||||
|
||||
function assertExactKeyCoverage(scopeName: string, actual: readonly string[], expected: readonly string[]): void {
|
||||
|
||||
@@ -51,7 +51,7 @@ describe("TaskStore task documents", () => {
|
||||
|
||||
expect(tableNames.has("task_documents")).toBe(true);
|
||||
expect(tableNames.has("task_document_revisions")).toBe(true);
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
|
||||
const index = db
|
||||
.prepare(
|
||||
|
||||
@@ -106,7 +106,7 @@ describe("Database", () => {
|
||||
});
|
||||
|
||||
it("seeds schema version", () => {
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
});
|
||||
|
||||
it("seeds lastModified", () => {
|
||||
@@ -129,7 +129,7 @@ describe("Database", () => {
|
||||
|
||||
it("is idempotent - calling init() twice does not fail", () => {
|
||||
expect(() => db.init()).not.toThrow();
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
});
|
||||
|
||||
it("does not overwrite existing config on re-init", () => {
|
||||
@@ -736,7 +736,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 22 (includes v1→v2 through v21→v22)
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -761,11 +761,11 @@ describe("schema migrations", () => {
|
||||
const db = new Database(kbDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
|
||||
// Re-init should not fail
|
||||
db.init();
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -781,7 +781,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "agentRatings" }]);
|
||||
@@ -805,7 +805,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "mission_events" }]);
|
||||
@@ -909,7 +909,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 22
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -1275,7 +1275,7 @@ describe("createDatabase factory", () => {
|
||||
const db = createDatabase(kbDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
expect(db.getLastModified()).toBeGreaterThan(0);
|
||||
|
||||
db.close();
|
||||
|
||||
@@ -59,7 +59,7 @@ export function fromJson<T>(json: string | null | undefined): T | undefined {
|
||||
|
||||
// ── Schema Definition ────────────────────────────────────────────────
|
||||
|
||||
const SCHEMA_VERSION = 25;
|
||||
const SCHEMA_VERSION = 26;
|
||||
|
||||
function normalizeTaskComments(
|
||||
steeringComments: SteeringComment[] | undefined,
|
||||
@@ -168,7 +168,8 @@ CREATE TABLE IF NOT EXISTS tasks (
|
||||
modifiedFiles TEXT DEFAULT '[]',
|
||||
missionId TEXT,
|
||||
sliceId TEXT,
|
||||
assignedAgentId TEXT
|
||||
assignedAgentId TEXT,
|
||||
assigneeUserId TEXT
|
||||
);
|
||||
|
||||
-- Config table (single row with project settings)
|
||||
@@ -935,6 +936,13 @@ export class Database {
|
||||
`);
|
||||
});
|
||||
}
|
||||
|
||||
if (version < 26) {
|
||||
this.applyMigration(26, () => {
|
||||
this.addColumnIfMissing("tasks", "assigneeUserId", "TEXT");
|
||||
this.db.exec(`CREATE INDEX IF NOT EXISTS idxTasksAssigneeUserId ON tasks(assigneeUserId)`);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -465,7 +465,7 @@ describe("Run Audit", () => {
|
||||
});
|
||||
|
||||
it("schema version is bumped to 25", () => {
|
||||
expect(db.getSchemaVersion()).toBe(25);
|
||||
expect(db.getSchemaVersion()).toBe(26);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1197,6 +1197,30 @@ describe("TaskStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("updateTask — assigneeUserId", () => {
|
||||
it("sets assigneeUserId via updateTask", async () => {
|
||||
const task = await store.createTask({ title: "User task", description: "A task" });
|
||||
const updated = await store.updateTask(task.id, { assigneeUserId: "requesting-user" });
|
||||
expect(updated.assigneeUserId).toBe("requesting-user");
|
||||
});
|
||||
|
||||
it("clears assigneeUserId when set to null", async () => {
|
||||
const task = await store.createTask({ title: "User task", description: "A task" });
|
||||
await store.updateTask(task.id, { assigneeUserId: "requesting-user" });
|
||||
const updated = await store.updateTask(task.id, { assigneeUserId: null });
|
||||
expect(updated.assigneeUserId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("sets and clears status: awaiting-user-review", async () => {
|
||||
const task = await store.createTask({ title: "Review task", description: "A task" });
|
||||
const updated = await store.updateTask(task.id, { status: "awaiting-user-review" });
|
||||
expect(updated.status).toBe("awaiting-user-review");
|
||||
|
||||
const cleared = await store.updateTask(task.id, { status: null });
|
||||
expect(cleared.status).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// ── Task prefix tests ──────────────────────────────────────────
|
||||
|
||||
describe("taskPrefix setting", () => {
|
||||
@@ -1576,6 +1600,21 @@ describe("TaskStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("createTask — assigneeUserId", () => {
|
||||
it("persists assigneeUserId on creation", async () => {
|
||||
const created = await store.createTask({
|
||||
title: "Task with user assignment",
|
||||
description: "A task assigned to a user",
|
||||
assigneeUserId: "requesting-user",
|
||||
});
|
||||
|
||||
expect(created.assigneeUserId).toBe("requesting-user");
|
||||
|
||||
const persisted = await store.getTask(created.id);
|
||||
expect(persisted.assigneeUserId).toBe("requesting-user");
|
||||
});
|
||||
});
|
||||
|
||||
describe("updateTask — model overrides", () => {
|
||||
it("sets executor model provider and id via updateTask", async () => {
|
||||
const task = await createTestTask();
|
||||
|
||||
@@ -233,6 +233,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
missionId: row.missionId || undefined,
|
||||
sliceId: row.sliceId || undefined,
|
||||
assignedAgentId: row.assignedAgentId || undefined,
|
||||
assigneeUserId: row.assigneeUserId || undefined,
|
||||
checkedOutBy: row.checkedOutBy || undefined,
|
||||
checkedOutAt: row.checkedOutAt || undefined,
|
||||
};
|
||||
@@ -284,10 +285,10 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
summary, thinkingLevel, createdAt, updatedAt, columnMovedAt,
|
||||
dependencies, steps, log, attachments, steeringComments,
|
||||
comments, workflowStepResults, prInfo, issueInfo, mergeDetails,
|
||||
breakIntoSubtasks, enabledWorkflowSteps, modifiedFiles, missionId, sliceId, assignedAgentId, checkedOutBy, checkedOutAt
|
||||
breakIntoSubtasks, enabledWorkflowSteps, modifiedFiles, missionId, sliceId, assignedAgentId, assigneeUserId, checkedOutBy, checkedOutAt
|
||||
) VALUES (
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
)
|
||||
`).run(
|
||||
task.id,
|
||||
@@ -337,6 +338,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
task.missionId ?? null,
|
||||
task.sliceId ?? null,
|
||||
task.assignedAgentId ?? null,
|
||||
task.assigneeUserId ?? null,
|
||||
task.checkedOutBy ?? null,
|
||||
task.checkedOutAt ?? null,
|
||||
);
|
||||
@@ -1059,6 +1061,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
enabledWorkflowSteps: resolvedWorkflowSteps,
|
||||
modelPresetId: input.modelPresetId,
|
||||
assignedAgentId: input.assignedAgentId,
|
||||
assigneeUserId: input.assigneeUserId,
|
||||
modelProvider: input.modelProvider,
|
||||
modelId: input.modelId,
|
||||
validatorModelProvider: input.validatorModelProvider,
|
||||
@@ -1484,7 +1487,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
async updateTask(
|
||||
id: string,
|
||||
updates: { title?: string; description?: string; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; blockedBy?: string | null; assignedAgentId?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; mergeRetries?: number; stuckKillCount?: number | null; recoveryRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
|
||||
updates: { title?: string; description?: string; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; blockedBy?: string | null; assignedAgentId?: string | null; assigneeUserId?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; mergeRetries?: number; stuckKillCount?: number | null; recoveryRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
|
||||
runContext?: RunMutationContext,
|
||||
): Promise<Task> {
|
||||
return this.withTaskLock(id, async () => {
|
||||
@@ -1545,6 +1548,11 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
} else if (updates.assignedAgentId !== undefined) {
|
||||
task.assignedAgentId = updates.assignedAgentId;
|
||||
}
|
||||
if (updates.assigneeUserId === null) {
|
||||
task.assigneeUserId = undefined;
|
||||
} else if (updates.assigneeUserId !== undefined) {
|
||||
task.assigneeUserId = updates.assigneeUserId;
|
||||
}
|
||||
if (updates.checkedOutBy === null) {
|
||||
task.checkedOutBy = undefined;
|
||||
task.checkedOutAt = undefined;
|
||||
|
||||
@@ -31,6 +31,16 @@ describe("getTaskMergeBlocker", () => {
|
||||
.toContain("failed");
|
||||
});
|
||||
|
||||
it("returns reason when task has awaiting-user-review status", () => {
|
||||
expect(getTaskMergeBlocker({ ...baseTask, status: "awaiting-user-review" }))
|
||||
.toContain("awaiting-user-review");
|
||||
});
|
||||
|
||||
it("returns reason when task has awaiting-inspection status", () => {
|
||||
expect(getTaskMergeBlocker({ ...baseTask, status: "awaiting-inspection" }))
|
||||
.toContain("awaiting-inspection");
|
||||
});
|
||||
|
||||
it("returns reason when task has incomplete steps", () => {
|
||||
expect(getTaskMergeBlocker({
|
||||
...baseTask,
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { Task, WorkflowStepResult } from "./types.js";
|
||||
const BLOCKING_TASK_STATUSES = new Set([
|
||||
"failed",
|
||||
"awaiting-inspection",
|
||||
"awaiting-user-review",
|
||||
]);
|
||||
|
||||
const NON_TERMINAL_STEP_STATUSES = new Set([
|
||||
|
||||
@@ -122,7 +122,7 @@ export interface WorkflowStep {
|
||||
|
||||
/** Input for creating a new workflow step. */
|
||||
/** Event types that can trigger ntfy notifications */
|
||||
export type NtfyNotificationEvent = "in-review" | "merged" | "failed" | "awaiting-approval";
|
||||
export type NtfyNotificationEvent = "in-review" | "merged" | "failed" | "awaiting-approval" | "awaiting-user-review";
|
||||
|
||||
export interface WorkflowStepInput {
|
||||
/** Built-in template source ID when creating a concrete step from a template. */
|
||||
@@ -684,6 +684,10 @@ export interface Task {
|
||||
thinkingLevel?: ThinkingLevel;
|
||||
/** Explicitly assigned agent ID for task-agent linking. Distinct from Agent.taskId active execution state. */
|
||||
assignedAgentId?: string;
|
||||
/** Explicitly assigned user ID for task-user linking. Used during review handoff to indicate
|
||||
* which user should review the task. The sentinel value "requesting-user" indicates the
|
||||
* user who created or steered the task. */
|
||||
assigneeUserId?: string;
|
||||
/** Agent ID currently holding the checkout lease for this task. Undefined when no active lease. */
|
||||
checkedOutBy?: string;
|
||||
/** ISO-8601 timestamp when the checkout lease was acquired. */
|
||||
@@ -758,6 +762,8 @@ export interface TaskCreateInput {
|
||||
sliceId?: string;
|
||||
/** Optional explicit agent assignment for this task */
|
||||
assignedAgentId?: string;
|
||||
/** Optional explicit user assignment for this task (used during review handoff) */
|
||||
assigneeUserId?: string;
|
||||
}
|
||||
|
||||
// ── Settings Scope Types ────────────────────────────────────────────────
|
||||
@@ -1125,6 +1131,13 @@ export interface ProjectSettings {
|
||||
reflectionIntervalMs?: number;
|
||||
/** When true, automatically trigger reflection after task completion. Default: true. */
|
||||
reflectionAfterTask?: boolean;
|
||||
/** Policy for agent-to-user review handoff. When enabled, agents can hand off
|
||||
* tasks to users for human review via steering comments.
|
||||
* - "disabled": No handoff detection (default)
|
||||
* - "comment-triggered": Detect handoff phrases in agent steering comments
|
||||
* - "always": Always handoff after completion (not implemented, reserved for future)
|
||||
*/
|
||||
reviewHandoffPolicy?: "disabled" | "comment-triggered" | "always";
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1153,7 +1166,7 @@ export const DEFAULT_GLOBAL_SETTINGS: Required<Pick<GlobalSettings, "themeMode"
|
||||
defaultThinkingLevel: undefined,
|
||||
ntfyEnabled: false,
|
||||
ntfyTopic: undefined,
|
||||
ntfyEvents: ["in-review", "merged", "failed", "awaiting-approval"],
|
||||
ntfyEvents: ["in-review", "merged", "failed", "awaiting-approval", "awaiting-user-review"],
|
||||
ntfyDashboardHost: undefined,
|
||||
defaultProjectId: undefined,
|
||||
setupComplete: undefined,
|
||||
@@ -1235,6 +1248,7 @@ export const DEFAULT_PROJECT_SETTINGS: ProjectSettings = {
|
||||
reflectionEnabled: false,
|
||||
reflectionIntervalMs: 3_600_000,
|
||||
reflectionAfterTask: true,
|
||||
reviewHandoffPolicy: "disabled",
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -1340,6 +1354,7 @@ export const PROJECT_SETTINGS_KEYS: ReadonlyArray<keyof ProjectSettings> = [
|
||||
"reflectionEnabled",
|
||||
"reflectionIntervalMs",
|
||||
"reflectionAfterTask",
|
||||
"reviewHandoffPolicy",
|
||||
] as const;
|
||||
|
||||
// ── Compile-time parity: ensures every interface key is listed exactly once ──
|
||||
@@ -1455,6 +1470,8 @@ export interface ArchivedTaskEntry {
|
||||
recoveryRetryCount?: number;
|
||||
nextRecoveryAt?: string;
|
||||
error?: string;
|
||||
/** User assigned to review this task (used during review handoff) */
|
||||
assigneeUserId?: string;
|
||||
}
|
||||
|
||||
/** Type of planning question presented to the user */
|
||||
|
||||
Reference in New Issue
Block a user