From 4d1085fc04c3f7f5688fd33e007c6a5aa38e91bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 27 Mar 2026 00:37:36 -0400 Subject: [PATCH 1/4] chore(07-02): bring in plan 01 prerequisites Cherry-pick plan 01 task quality gate validation module and its integration into IPC, API server, and automation service. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/main/api-server.ts | 15 ++ src/main/automation-service.ts | 7 + src/main/ipc-core-work-graph-handlers.ts | 19 ++- src/main/task-quality-gates.ts | 163 ++++++++++++++++++ src/shared/contracts.ts | 7 +- tests/unit/task-quality-gates.test.ts | 203 +++++++++++++++++++++++ 6 files changed, 412 insertions(+), 2 deletions(-) create mode 100644 src/main/task-quality-gates.ts create mode 100644 tests/unit/task-quality-gates.test.ts diff --git a/src/main/api-server.ts b/src/main/api-server.ts index 61c274f9..106d8a28 100644 --- a/src/main/api-server.ts +++ b/src/main/api-server.ts @@ -8,6 +8,7 @@ import { canAssignTask, canControlAgent, canManageGoal, canManageOrganization, c import { resolveApprovalDecision, shouldAutoApproveApproval } from "./approval-policy"; import type { AppDatabase } from "./database"; import { inferTaskType } from "./connector-matching"; +import { validateTaskCreation } from "./task-quality-gates"; import { dispatchAgentMessage } from "./message-dispatch"; import type { DesktopEvent, SectionId } from "@shared/types"; @@ -561,6 +562,12 @@ export class AgentApiServer { this.json(res, 403, { error: "Current agent is not allowed to assign this task to the selected agent" }); return; } + const isHeartbeat = input.title.startsWith("[Heartbeat]"); + const taskValidation = validateTaskCreation({ title: input.title, description: input.description ?? "" }, isHeartbeat); + if (!taskValidation.valid) { + this.json(res, 422, { error: taskValidation.errors.join("; ") }); + return; + } let id: string; try { @@ -1582,6 +1589,14 @@ export class AgentApiServer { if (actionItems.length > 0) { for (const item of actionItems) { if (item.title && typeof item.title === "string") { + const meetingTaskValidation = validateTaskCreation( + { title: item.title, description: item.description ?? `Action item from meeting: ${data.title ?? existing.title}` }, + false, + ); + if (!meetingTaskValidation.valid) { + // Log but skip invalid meeting action items rather than failing the whole request + continue; + } const taskId = this.db.saveTask({ companyId: token.companyId, title: item.title, diff --git a/src/main/automation-service.ts b/src/main/automation-service.ts index 1638e33b..97a8dd19 100644 --- a/src/main/automation-service.ts +++ b/src/main/automation-service.ts @@ -8,6 +8,7 @@ import type { RunStatus, } from "@shared/types"; import { dispatchAgentMessage } from "./message-dispatch"; +import { validateTaskCreation } from "./task-quality-gates"; import type { NotificationOptions } from "./notification-service"; /** Document types considered important enough to gate behind user review when reviewDeliverables is enabled. */ @@ -268,6 +269,12 @@ export function createAutomationService({ const description = (config.description as string) ?? ""; const priority = (config.priority as string) ?? "medium"; const assigneeAgentId = (config.assigneeAgentId as string) ?? null; + const isHeartbeat = title.startsWith("[Heartbeat]"); + const taskValidation = validateTaskCreation({ title, description }, isHeartbeat); + if (!taskValidation.valid) { + logger.warn(`[automation] Skipping create_task — validation failed: ${taskValidation.errors.join("; ")}`); + break; + } const taskId = db.saveTask({ companyId: ctx.companyId, projectId: ctx.projectId ?? (config.projectId as string) ?? null, diff --git a/src/main/ipc-core-work-graph-handlers.ts b/src/main/ipc-core-work-graph-handlers.ts index 9ac2a941..1132e706 100644 --- a/src/main/ipc-core-work-graph-handlers.ts +++ b/src/main/ipc-core-work-graph-handlers.ts @@ -11,6 +11,7 @@ import { import { getConnectorExecutionReadinessIssue } from "@shared/connector-policy"; import { resolveApprovalDecision, shouldAutoApproveApproval } from "./approval-policy"; import { fail, ok, type RegisterCoreHandlersDependencies } from "./ipc-core-common"; +import { validateTaskCreation, checkCapabilityMatch } from "./task-quality-gates"; export function registerCoreWorkGraphHandlers({ registerHandle, @@ -314,6 +315,13 @@ export function registerCoreWorkGraphHandlers({ return fail("INVALID_COMPANY_REFERENCE", `The selected ${invalidReference.label} does not belong to the current company.`); } const isNew = !parsed.id; + if (isNew) { + const isHeartbeat = parsed.title.startsWith("[Heartbeat]"); + const validation = validateTaskCreation(parsed, isHeartbeat); + if (!validation.valid) { + return fail("TASK_VALIDATION_FAILED", validation.errors.join("; ")); + } + } let previousStatus: string | null = null; let previousAssigneeAgentId: string | null = null; if (!isNew) { @@ -355,8 +363,17 @@ export function registerCoreWorkGraphHandlers({ wakeAgentIfPossible(parsed.assigneeAgentId, parsed.companyId, "assignment"); } } + let capabilityWarning: string | undefined; + if (isNew && parsed.assigneeAgentId) { + try { + const match = checkCapabilityMatch(db, parsed.assigneeAgentId, parsed.description, parsed.companyId); + if (!match.matched) { + capabilityWarning = match.warning; + } + } catch { /* capability check is advisory — never block task creation */ } + } publishDomainChanged(); - return ok(id); + return ok({ id, capabilityWarning }); }); registerHandle(IPC_CHANNELS.deleteAgent, async (_event, payload) => { diff --git a/src/main/task-quality-gates.ts b/src/main/task-quality-gates.ts new file mode 100644 index 00000000..82bbc159 --- /dev/null +++ b/src/main/task-quality-gates.ts @@ -0,0 +1,163 @@ +/** + * Task Quality Gates — pure validation functions for task creation. + * + * Called by IPC handlers, API server, and automation service to enforce + * minimum description quality, acceptance criteria, and capability matching. + */ + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export interface TaskValidationResult { + valid: boolean; + errors: string[]; + warnings: string[]; +} + +export interface CapabilityMatchResult { + matched: boolean; + warning?: string; +} + +/** Minimal DB interface for capability matching — keeps the function testable with mocks. + * Skill methods are optional since the skill system may not be implemented yet (graceful degradation per D-07). */ +export interface CapabilityMatchDb { + getAgent(id: string): { name: string; capabilities: string }; + listAgentSkillAssignments?(id: string): string[]; + getCompanySkill?(companyId: string, skillId: string): { name: string; description: string | null; slug: string } | null; +} + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +const MIN_DESCRIPTION_LENGTH = 50; + +export const ACCEPTANCE_CRITERIA_KEYWORDS: readonly string[] = [ + "acceptance criteria", + "done when", + "success criteria", + "definition of done", + "requirements:", + "must:", + "should:", + "- [ ]", + "- [x]", + "deliverable:", + "expected outcome", + "exit criteria", +]; + +// --------------------------------------------------------------------------- +// checkAcceptanceCriteria +// --------------------------------------------------------------------------- + +export function checkAcceptanceCriteria(description: string): { hasCriteria: boolean; suggestion: string } { + const lower = description.toLowerCase(); + const found = ACCEPTANCE_CRITERIA_KEYWORDS.some((kw) => lower.includes(kw)); + + if (found) { + return { hasCriteria: true, suggestion: "" }; + } + + return { + hasCriteria: false, + suggestion: + 'Task description should include acceptance criteria. Add a "Done when:" or "Acceptance criteria:" section, or use checkbox items (- [ ]) to define success conditions.', + }; +} + +// --------------------------------------------------------------------------- +// validateTaskCreation +// --------------------------------------------------------------------------- + +export function validateTaskCreation( + input: { title: string; description: string }, + isHeartbeat: boolean, +): TaskValidationResult { + // Heartbeat tasks bypass all validation + if (isHeartbeat) { + return { valid: true, errors: [], warnings: [] }; + } + + const errors: string[] = []; + const warnings: string[] = []; + + // TASK-01: Minimum description length + if (input.description.length < MIN_DESCRIPTION_LENGTH) { + errors.push( + `Task description must be at least ${MIN_DESCRIPTION_LENGTH} characters (currently ${input.description.length}). Add details about what needs to be done, expected behavior, and any constraints.`, + ); + } + + // TASK-02: Acceptance criteria detection (only check if length passes) + if (input.description.length >= MIN_DESCRIPTION_LENGTH) { + const criteria = checkAcceptanceCriteria(input.description); + if (!criteria.hasCriteria) { + errors.push(criteria.suggestion); + } + } + + return { valid: errors.length === 0, errors, warnings }; +} + +// --------------------------------------------------------------------------- +// checkCapabilityMatch +// --------------------------------------------------------------------------- + +/** Tokenize text into words >3 chars, same pattern as database.ts autoAssignTask */ +function tokenize(text: string): string[] { + return text.toLowerCase().split(/\W+/).filter((w) => w.length > 3); +} + +export function checkCapabilityMatch( + db: CapabilityMatchDb, + agentId: string, + taskDescription: string, + companyId: string, +): CapabilityMatchResult { + const taskWords = tokenize(taskDescription); + + // Nothing to match against — task description too short/vague + if (taskWords.length === 0) { + return { matched: true }; + } + + const agent = db.getAgent(agentId); + const capWords = tokenize(agent.capabilities); + const capabilityOverlap = taskWords.filter((w) => capWords.includes(w)).length; + + // Check skill assignments for additional matching (graceful degradation) + let skillOverlap = 0; + try { + if (db.listAgentSkillAssignments && db.getCompanySkill) { + const skillIds = db.listAgentSkillAssignments(agentId); + for (const skillId of skillIds) { + const skill = db.getCompanySkill(companyId, skillId); + if (skill) { + const skillWords = tokenize(`${skill.name} ${skill.description ?? ""} ${skill.slug}`); + skillOverlap += taskWords.filter((w) => skillWords.includes(w)).length; + } + } + } + } catch { + // Skill system may not be available — graceful degradation per D-07 + } + + // Direct capability or skill overlap found + if (capabilityOverlap > 0 || skillOverlap > 0) { + return { matched: true }; + } + + // No capabilities defined at all — graceful degradation (Pitfall 4) + if (capWords.length === 0 && skillOverlap === 0) { + return { matched: true }; + } + + // Agent has capabilities but no overlap — warn + return { + matched: false, + warning: `Agent "${agent.name}" may lack skills for this task. Their capabilities (${agent.capabilities.slice(0, 80)}...) have no overlap with the task description. Consider reassigning.`, + }; +} diff --git a/src/shared/contracts.ts b/src/shared/contracts.ts index 5ff2552a..3be04d85 100644 --- a/src/shared/contracts.ts +++ b/src/shared/contracts.ts @@ -518,6 +518,11 @@ export type DesktopResult = | { ok: true; data: T } | { ok: false; error: { code: string; message: string } }; +export interface SaveTaskResult { + id: string; + capabilityWarning?: string; +} + export type DesktopApi = { loadSnapshot: () => Promise>; pickDirectory: () => Promise>; @@ -531,7 +536,7 @@ export type DesktopApi = { requestHire: (input: z.infer) => Promise>; saveGoal: (input: z.infer) => Promise>; saveProject: (input: z.infer) => Promise>; - saveTask: (input: z.infer) => Promise>; + saveTask: (input: z.infer) => Promise>; requestApproval: (input: z.infer) => Promise>; decideApproval: (input: z.infer) => Promise>; saveSecret: (input: z.infer) => Promise>; diff --git a/tests/unit/task-quality-gates.test.ts b/tests/unit/task-quality-gates.test.ts new file mode 100644 index 00000000..da7787e9 --- /dev/null +++ b/tests/unit/task-quality-gates.test.ts @@ -0,0 +1,203 @@ +import { describe, expect, it } from "vitest"; +import { + validateTaskCreation, + checkAcceptanceCriteria, + checkCapabilityMatch, + ACCEPTANCE_CRITERIA_KEYWORDS, +} from "../../src/main/task-quality-gates"; +import type { TaskValidationResult } from "../../src/main/task-quality-gates"; + +// --- TASK-01: validateTaskCreation tests --- + +describe("validateTaskCreation", () => { + it("rejects description with 5 chars when not heartbeat", () => { + const result = validateTaskCreation({ title: "Fix bug", description: "Short" }, false); + expect(result.valid).toBe(false); + expect(result.errors.some(e => e.includes("at least 50 characters"))).toBe(true); + }); + + it("rejects description with exactly 49 chars when not heartbeat", () => { + const desc = "A".repeat(49); + const result = validateTaskCreation({ title: "Fix bug", description: desc }, false); + expect(result.valid).toBe(false); + expect(result.errors.some(e => e.includes("at least 50 characters"))).toBe(true); + }); + + it("accepts description with exactly 50 chars and acceptance keyword when not heartbeat", () => { + const desc = "Done when: the login page renders correctly." + "......"; + // Ensure exactly 50 chars + const padded = desc.slice(0, 50).padEnd(50, "."); + expect(padded.length).toBe(50); + // Must contain "done when" + const result = validateTaskCreation({ title: "Build login page", description: padded }, false); + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("always returns valid for heartbeat tasks regardless of description", () => { + const result = validateTaskCreation({ title: "[Heartbeat] Check status", description: "Short" }, true); + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + expect(result.warnings).toHaveLength(0); + }); + + it("rejects empty description when not heartbeat", () => { + const result = validateTaskCreation({ title: "Some task", description: "" }, false); + expect(result.valid).toBe(false); + expect(result.errors.some(e => e.includes("at least 50 characters"))).toBe(true); + }); + + it("rejects 60-char description without acceptance criteria keywords", () => { + const desc = "This is a description that is long enough to pass length check"; + expect(desc.length).toBeGreaterThanOrEqual(50); + const result = validateTaskCreation({ title: "A task", description: desc }, false); + expect(result.valid).toBe(false); + expect(result.errors.some(e => e.toLowerCase().includes("acceptance criteria"))).toBe(true); + }); + + it("returns valid=true when description has both length and acceptance criteria", () => { + const desc = "Implement the authentication flow for the login page. Done when: all tests pass and the form submits."; + expect(desc.length).toBeGreaterThanOrEqual(50); + const result = validateTaskCreation({ title: "Auth flow", description: desc }, false); + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("includes current character count in length error message", () => { + const result = validateTaskCreation({ title: "Fix", description: "Short" }, false); + expect(result.errors.some(e => e.includes("currently 5"))).toBe(true); + }); +}); + +// --- TASK-02: checkAcceptanceCriteria tests --- + +describe("checkAcceptanceCriteria", () => { + it("detects 'done when' keyword", () => { + const result = checkAcceptanceCriteria("This task is done when all tests pass and deployment succeeds"); + expect(result.hasCriteria).toBe(true); + }); + + it("detects 'Acceptance Criteria:' case insensitive", () => { + const result = checkAcceptanceCriteria("Acceptance Criteria: the form validates all fields correctly"); + expect(result.hasCriteria).toBe(true); + }); + + it("detects '- [ ] first item' checkbox", () => { + const result = checkAcceptanceCriteria("Tasks to complete:\n- [ ] first item\n- [ ] second item"); + expect(result.hasCriteria).toBe(true); + }); + + it("detects '- [x] completed item' checked checkbox", () => { + const result = checkAcceptanceCriteria("Progress:\n- [x] completed item\n- [ ] pending item"); + expect(result.hasCriteria).toBe(true); + }); + + it("detects 'must:' keyword", () => { + const result = checkAcceptanceCriteria("The implementation must: handle all edge cases gracefully"); + expect(result.hasCriteria).toBe(true); + }); + + it("detects 'requirements:' keyword", () => { + const result = checkAcceptanceCriteria("Technical requirements: use React 19 with TypeScript strict"); + expect(result.hasCriteria).toBe(true); + }); + + it("detects 'success criteria' keyword", () => { + const result = checkAcceptanceCriteria("Success criteria for this task include passing all tests"); + expect(result.hasCriteria).toBe(true); + }); + + it("detects 'deliverable:' keyword", () => { + const result = checkAcceptanceCriteria("Primary deliverable: a working API endpoint with docs"); + expect(result.hasCriteria).toBe(true); + }); + + it("detects 'expected outcome' keyword", () => { + const result = checkAcceptanceCriteria("Expected outcome is a fully tested authentication module"); + expect(result.hasCriteria).toBe(true); + }); + + it("detects 'exit criteria' keyword", () => { + const result = checkAcceptanceCriteria("Exit criteria for this sprint includes shipped feature"); + expect(result.hasCriteria).toBe(true); + }); + + it("returns hasCriteria=false with suggestion for description with no keywords", () => { + const result = checkAcceptanceCriteria("Build a login page with email and password fields"); + expect(result.hasCriteria).toBe(false); + expect(result.suggestion).toContain("Done when:"); + }); +}); + +// --- Verify ACCEPTANCE_CRITERIA_KEYWORDS constant --- + +describe("ACCEPTANCE_CRITERIA_KEYWORDS", () => { + it("contains all required keywords", () => { + const required = [ + "acceptance criteria", + "done when", + "success criteria", + "definition of done", + "requirements:", + "must:", + "should:", + "- [ ]", + "- [x]", + "deliverable:", + "expected outcome", + "exit criteria", + ]; + for (const kw of required) { + expect(ACCEPTANCE_CRITERIA_KEYWORDS).toContain(kw); + } + }); +}); + +// --- TASK-03: checkCapabilityMatch tests --- + +describe("checkCapabilityMatch", () => { + function makeMockDb(agent: { name: string; capabilities: string }, skillAssignments: string[] = [], skills: Record = {}) { + return { + getAgent: (_id: string) => agent, + listAgentSkillAssignments: (_id: string) => skillAssignments, + getCompanySkill: (_companyId: string, skillId: string) => skills[skillId] ?? null, + }; + } + + it("returns matched=true when agent capabilities overlap with task", () => { + const db = makeMockDb({ name: "Frontend Dev", capabilities: "javascript react frontend" }); + const result = checkCapabilityMatch(db, "agent-1", "Build React login form with validation", "company-1"); + expect(result.matched).toBe(true); + }); + + it("returns matched=false with warning when capabilities do not overlap", () => { + const db = makeMockDb({ name: "Data Analyst", capabilities: "python data analysis" }); + const result = checkCapabilityMatch(db, "agent-1", "Build React login form with validation", "company-1"); + expect(result.matched).toBe(false); + expect(result.warning).toBeDefined(); + expect(result.warning).toContain("Data Analyst"); + }); + + it("returns matched=true when agent has empty capabilities and no skills (graceful degradation)", () => { + const db = makeMockDb({ name: "New Agent", capabilities: "" }); + const result = checkCapabilityMatch(db, "agent-1", "Build React login form", "company-1"); + expect(result.matched).toBe(true); + }); + + it("returns matched=true when agent has no capabilities but skill name overlaps with task", () => { + const db = makeMockDb( + { name: "Specialist", capabilities: "" }, + ["skill-1"], + { "skill-1": { name: "react development", description: "Building React components", slug: "react-dev" } }, + ); + const result = checkCapabilityMatch(db, "agent-1", "Build React component with hooks", "company-1"); + expect(result.matched).toBe(true); + }); + + it("returns matched=true when task description has no significant words", () => { + const db = makeMockDb({ name: "Agent", capabilities: "python" }); + const result = checkCapabilityMatch(db, "agent-1", "Do it", "company-1"); + // "do" and "it" are <= 3 chars, so taskWords is empty -> matched=true + expect(result.matched).toBe(true); + }); +}); From e1f8a3c9ab69b9943311dd8d6d09318d9b5b623f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 27 Mar 2026 00:38:57 -0400 Subject: [PATCH 2/4] test(07-02): add failing tests for sweepStaleReviews escalation - 8 tests covering TASK-04 stale review escalation logic - Escalation to manager's manager, CEO fallback notification - Auto-approve pending approvals, race condition handling - Task status advancement, no-assignee skip, timeout constant Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/unit/task-quality-gates.test.ts | 192 +++++++++++++++++++++++++- 1 file changed, 190 insertions(+), 2 deletions(-) diff --git a/tests/unit/task-quality-gates.test.ts b/tests/unit/task-quality-gates.test.ts index da7787e9..68255855 100644 --- a/tests/unit/task-quality-gates.test.ts +++ b/tests/unit/task-quality-gates.test.ts @@ -1,11 +1,16 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { validateTaskCreation, checkAcceptanceCriteria, checkCapabilityMatch, ACCEPTANCE_CRITERIA_KEYWORDS, + sweepStaleReviews, + REVIEW_TIMEOUT_MS, +} from "../../src/main/task-quality-gates"; +import type { + TaskValidationResult, + SweepStaleReviewsDeps, } from "../../src/main/task-quality-gates"; -import type { TaskValidationResult } from "../../src/main/task-quality-gates"; // --- TASK-01: validateTaskCreation tests --- @@ -201,3 +206,186 @@ describe("checkCapabilityMatch", () => { expect(result.matched).toBe(true); }); }); + +// --- TASK-04: sweepStaleReviews tests --- + +describe("sweepStaleReviews", () => { + const NOW = Date.now(); + const THIRTY_FIVE_MIN_AGO = new Date(NOW - 35 * 60 * 1000).toISOString(); + const TWENTY_FIVE_MIN_AGO = new Date(NOW - 25 * 60 * 1000).toISOString(); + + function makeSweepDeps(overrides: Partial = {}): SweepStaleReviewsDeps { + return { + db: { + queryAll: vi.fn().mockReturnValue([]), + getManagerForAgent: vi.fn().mockReturnValue(null), + getChainOfCommand: vi.fn().mockReturnValue([]), + advanceTaskStatus: vi.fn().mockReturnValue({ previousStatus: "in_review" }), + }, + dispatchAgentMessage: vi.fn().mockReturnValue({ ok: true }), + resolveApprovalDecision: vi.fn(), + notify: vi.fn(), + handleTaskStatusChange: vi.fn(), + handleApprovalResolved: vi.fn(), + emitEvent: vi.fn(), + publishDomainChanged: vi.fn(), + wakeAgentIfPossible: vi.fn(), + logger: { info: vi.fn(), warn: vi.fn() }, + ...overrides, + }; + } + + it("escalates to manager's manager when task in_review for 35 minutes", () => { + const deps = makeSweepDeps(); + const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; + + // Task in review for 35 min + dbMock.queryAll.mockReturnValueOnce([ + { id: "task-1", assigneeAgentId: "agent-1", updatedAt: THIRTY_FIVE_MIN_AGO, title: "Build login" }, + ]); + // Direct manager of the assignee (the reviewer) + dbMock.getManagerForAgent.mockReturnValueOnce({ id: "manager-1", name: "Manager One", companyId: "company-1" }); + // Chain of command for the reviewer — the reviewer's manager (escalation target) + dbMock.getChainOfCommand.mockReturnValueOnce([{ id: "higher-manager-1", name: "Higher Manager" }]); + + const count = sweepStaleReviews("company-1", deps, NOW); + expect(count).toBe(1); + expect(deps.dispatchAgentMessage).toHaveBeenCalledWith( + expect.objectContaining({ db: deps.db }), + expect.objectContaining({ + companyId: "company-1", + fromAgentId: "manager-1", + toAgentId: "higher-manager-1", + channel: "direct", + priority: "urgent", + }), + "subtask_completed", + ); + }); + + it("does not escalate when task in_review for only 25 minutes (under threshold)", () => { + const deps = makeSweepDeps(); + const dbMock = deps.db as { queryAll: ReturnType }; + + dbMock.queryAll.mockReturnValueOnce([ + { id: "task-1", assigneeAgentId: "agent-1", updatedAt: TWENTY_FIVE_MIN_AGO, title: "Build login" }, + ]); + + const count = sweepStaleReviews("company-1", deps, NOW); + expect(count).toBe(0); + expect(deps.dispatchAgentMessage).not.toHaveBeenCalled(); + expect(deps.notify).not.toHaveBeenCalled(); + }); + + it("notifies human when no higher manager (reviewer is CEO)", () => { + const deps = makeSweepDeps(); + const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; + + dbMock.queryAll + .mockReturnValueOnce([ + { id: "task-1", assigneeAgentId: "agent-1", updatedAt: THIRTY_FIVE_MIN_AGO, title: "Build login" }, + ]) + .mockReturnValueOnce([]); // no pending approvals + dbMock.getManagerForAgent.mockReturnValueOnce({ id: "ceo-1", name: "CEO", companyId: "company-1" }); + // Empty chain means reviewer is the top of hierarchy + dbMock.getChainOfCommand.mockReturnValueOnce([]); + + const count = sweepStaleReviews("company-1", deps, NOW); + expect(count).toBe(1); + expect(deps.notify).toHaveBeenCalledWith( + expect.objectContaining({ + title: "Review Timeout", + urgency: "critical", + batchKey: "review_escalation", + navigation: expect.objectContaining({ section: "tasks", entityId: "task-1" }), + }), + ); + expect(deps.dispatchAgentMessage).not.toHaveBeenCalled(); + }); + + it("auto-approves pending approval linked to stale review task", () => { + const deps = makeSweepDeps(); + const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; + + // First queryAll: stale task + dbMock.queryAll.mockReturnValueOnce([ + { id: "task-1", assigneeAgentId: "agent-1", updatedAt: THIRTY_FIVE_MIN_AGO, title: "Build login" }, + ]); + dbMock.getManagerForAgent.mockReturnValueOnce({ id: "manager-1", name: "Manager", companyId: "company-1" }); + dbMock.getChainOfCommand.mockReturnValueOnce([{ id: "higher-1", name: "Higher" }]); + // Second queryAll: pending approval + dbMock.queryAll.mockReturnValueOnce([{ id: "approval-1" }]); + + sweepStaleReviews("company-1", deps, NOW); + + expect(deps.resolveApprovalDecision).toHaveBeenCalledWith( + expect.objectContaining({ + db: deps.db, + companyId: "company-1", + approvalId: "approval-1", + state: "approved", + decisionNote: "Auto-approved: review timeout exceeded 30 minutes.", + actor: "system", + }), + ); + }); + + it("catches resolveApprovalDecision error gracefully (race condition)", () => { + const deps = makeSweepDeps(); + const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; + + dbMock.queryAll.mockReturnValueOnce([ + { id: "task-1", assigneeAgentId: "agent-1", updatedAt: THIRTY_FIVE_MIN_AGO, title: "Build login" }, + ]); + dbMock.getManagerForAgent.mockReturnValueOnce({ id: "manager-1", name: "Manager", companyId: "company-1" }); + dbMock.getChainOfCommand.mockReturnValueOnce([{ id: "higher-1", name: "Higher" }]); + dbMock.queryAll.mockReturnValueOnce([{ id: "approval-1" }]); + + // resolveApprovalDecision throws (already decided) + (deps.resolveApprovalDecision as ReturnType).mockImplementationOnce(() => { + throw new Error("Approval already decided"); + }); + + // Should not throw + expect(() => sweepStaleReviews("company-1", deps, NOW)).not.toThrow(); + }); + + it("advances task status to done after escalation", () => { + const deps = makeSweepDeps(); + const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType; advanceTaskStatus: ReturnType }; + + dbMock.queryAll.mockReturnValueOnce([ + { id: "task-1", assigneeAgentId: "agent-1", updatedAt: THIRTY_FIVE_MIN_AGO, title: "Build login" }, + ]); + dbMock.getManagerForAgent.mockReturnValueOnce({ id: "manager-1", name: "Manager", companyId: "company-1" }); + dbMock.getChainOfCommand.mockReturnValueOnce([{ id: "higher-1", name: "Higher" }]); + dbMock.queryAll.mockReturnValueOnce([]); // no approvals + + sweepStaleReviews("company-1", deps, NOW); + + expect(dbMock.advanceTaskStatus).toHaveBeenCalledWith( + "task-1", + "done", + "Auto-advanced: review timeout exceeded 30 minutes.", + ); + expect(deps.handleTaskStatusChange).toHaveBeenCalledWith("task-1", "company-1", "in_review", "done"); + }); + + it("skips task with no assigneeAgentId", () => { + const deps = makeSweepDeps(); + const dbMock = deps.db as { queryAll: ReturnType }; + + dbMock.queryAll.mockReturnValueOnce([ + { id: "task-1", assigneeAgentId: null, updatedAt: THIRTY_FIVE_MIN_AGO, title: "Orphan task" }, + ]); + + const count = sweepStaleReviews("company-1", deps, NOW); + expect(count).toBe(0); + expect(deps.dispatchAgentMessage).not.toHaveBeenCalled(); + expect(deps.notify).not.toHaveBeenCalled(); + }); + + it("exports REVIEW_TIMEOUT_MS as 30 minutes in milliseconds", () => { + expect(REVIEW_TIMEOUT_MS).toBe(30 * 60 * 1000); + }); +}); From 0bfbdec09b8ec3644bd253d2a5f27dec736ba80f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 27 Mar 2026 00:39:41 -0400 Subject: [PATCH 3/4] feat(07-02): implement sweepStaleReviews with escalation chain - sweepStaleReviews queries tasks in in_review status exceeding 30-min timeout - Escalation dispatched to reviewer's manager (manager's manager in chain) - When reviewer is CEO (no higher manager), desktop notification sent to human - Pending approvals auto-approved with documented timeout note - Task status advanced from in_review to done after escalation - Race condition with manual approval handled via try-catch (Pitfall 3) - Tasks without assigneeAgentId skipped gracefully - Exports REVIEW_TIMEOUT_MS (1800000ms) and SweepStaleReviewsDeps interface - 8 comprehensive unit tests covering all TASK-04 behaviors Co-Authored-By: Claude Opus 4.6 (1M context) --- src/main/task-quality-gates.ts | 156 +++++++++++++++++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/src/main/task-quality-gates.ts b/src/main/task-quality-gates.ts index 82bbc159..4e0e61d1 100644 --- a/src/main/task-quality-gates.ts +++ b/src/main/task-quality-gates.ts @@ -161,3 +161,159 @@ export function checkCapabilityMatch( warning: `Agent "${agent.name}" may lack skills for this task. Their capabilities (${agent.capabilities.slice(0, 80)}...) have no overlap with the task description. Consider reassigning.`, }; } + +// --------------------------------------------------------------------------- +// TASK-04: Stale Review Escalation +// --------------------------------------------------------------------------- + +export const REVIEW_TIMEOUT_MS = 30 * 60 * 1000; // 30 minutes (D-08) + +export interface SweepStaleReviewsDeps { + db: { + queryAll: (sql: string, ...params: unknown[]) => T[]; + getManagerForAgent: (agentId: string) => { id: string; name: string; companyId: string } | null; + getChainOfCommand: (agentId: string) => Array<{ id: string; name: string }>; + advanceTaskStatus: (taskId: string, newStatus: string, comment?: string) => { previousStatus: string }; + }; + dispatchAgentMessage: ( + deps: { + db: SweepStaleReviewsDeps["db"]; + emitEvent: (...args: unknown[]) => void; + publishDomainChanged: () => void; + wakeAgentIfPossible: (agentId: string, companyId: string, trigger: string) => void; + }, + message: { + companyId: string; + fromAgentId: string; + toAgentId: string | null; + channel: string; + subject: string; + body: string; + priority: string; + }, + wakeTrigger: string, + ) => { ok: boolean }; + resolveApprovalDecision: (params: { + db: SweepStaleReviewsDeps["db"]; + handleApprovalResolved: (approvalId: string, companyId: string, decision: string) => void; + companyId: string; + approvalId: string; + state: "approved" | "rejected" | "revision_requested"; + decisionNote?: string; + detail?: string | null; + actor?: string; + }) => void; + notify: (options: { + title: string; + body: string; + urgency?: string; + navigation?: { section: string; entityId?: string }; + batchKey?: string; + }) => void; + handleTaskStatusChange: (taskId: string, companyId: string, previousStatus: string, newStatus: string) => void; + handleApprovalResolved: (approvalId: string, companyId: string, decision: string) => void; + emitEvent: (...args: unknown[]) => void; + publishDomainChanged: () => void; + wakeAgentIfPossible: (agentId: string, companyId: string, trigger: string) => void; + logger: { info: (...args: unknown[]) => void; warn: (...args: unknown[]) => void }; +} + +export function sweepStaleReviews( + companyId: string, + deps: SweepStaleReviewsDeps, + now: number = Date.now(), +): number { + let escalatedCount = 0; + const reviewTasks = deps.db.queryAll<{ + id: string; + assigneeAgentId: string | null; + updatedAt: string; + title: string; + }>( + `SELECT id, assignee_agent_id as assigneeAgentId, updated_at as updatedAt, title + FROM tasks + WHERE company_id = ? AND status = 'in_review'`, + companyId, + ); + + for (const task of reviewTasks) { + const reviewAge = now - new Date(task.updatedAt).getTime(); + if (reviewAge < REVIEW_TIMEOUT_MS) continue; + if (!task.assigneeAgentId) continue; + + // The reviewer is the assignee's direct manager + const reviewer = deps.db.getManagerForAgent(task.assigneeAgentId); + if (!reviewer) continue; // No manager = CEO-assigned task, skip escalation + + // D-09: Escalate to the reviewer's manager (manager's manager) + const chain = deps.db.getChainOfCommand(reviewer.id); + const escalationTarget = chain.length > 0 ? chain[0] : null; + + if (escalationTarget) { + deps.dispatchAgentMessage( + { + db: deps.db, + emitEvent: deps.emitEvent, + publishDomainChanged: deps.publishDomainChanged, + wakeAgentIfPossible: deps.wakeAgentIfPossible, + }, + { + companyId, + fromAgentId: reviewer.id, + toAgentId: escalationTarget.id, + channel: "direct", + subject: `Review timeout: ${task.title}`, + body: `Review by ${reviewer.name} on "${task.title}" has been pending for over 30 minutes. Auto-escalating.`, + priority: "urgent", + }, + "subtask_completed", + ); + } else { + // D-09: No higher manager (reviewer is CEO) -- notify human + deps.notify({ + title: "Review Timeout", + body: `"${task.title}" review has been pending over 30 minutes. Auto-approving.`, + urgency: "critical", + navigation: { section: "tasks", entityId: task.id }, + batchKey: "review_escalation", + }); + } + + // D-10: Auto-approve any pending approval linked to this task + const pendingApprovals = deps.db.queryAll<{ id: string }>( + `SELECT id FROM approvals + WHERE company_id = ? AND related_task_id = ? AND state = 'pending' + LIMIT 1`, + companyId, + task.id, + ); + for (const approval of pendingApprovals) { + try { + deps.resolveApprovalDecision({ + db: deps.db, + handleApprovalResolved: deps.handleApprovalResolved, + companyId, + approvalId: approval.id, + state: "approved", + decisionNote: "Auto-approved: review timeout exceeded 30 minutes.", + detail: "Auto-approved by review timeout escalation", + actor: "system", + }); + } catch { + // Already decided (race condition with manual approval) -- skip silently (Pitfall 3) + } + } + + // D-10: Advance task status from in_review to done + try { + deps.db.advanceTaskStatus(task.id, "done", "Auto-advanced: review timeout exceeded 30 minutes."); + deps.handleTaskStatusChange(task.id, companyId, "in_review", "done"); + } catch { + // Task may have been updated by manual action + } + + escalatedCount++; + } + + return escalatedCount; +} From c4eb4423dbc921c58c12b4c618530c0399da2ff7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 27 Mar 2026 00:43:08 -0400 Subject: [PATCH 4/4] feat(07-02): wire sweepStaleReviews into heartbeat timer - Import sweepStaleReviews, resolveApprovalDecision, dispatchAgentMessage - Insert stale review sweep into per-company heartbeat loop after budget reset - Sweep runs on every 15-second heartbeat cycle for each active company - Local callback stubs for handleTaskStatusChange and handleApprovalResolved - Error handling prevents sweep failures from crashing the heartbeat loop - Fix test type casts to use as-unknown-as for vi.fn mock compatibility - TypeScript compiles cleanly, 249/249 tests pass Co-Authored-By: Claude Opus 4.6 (1M context) --- src/main/orchestration-service.ts | 31 +++++++++++++++++++++++++++ tests/unit/task-quality-gates.test.ts | 14 ++++++------ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/main/orchestration-service.ts b/src/main/orchestration-service.ts index e6e40a36..cc89df56 100644 --- a/src/main/orchestration-service.ts +++ b/src/main/orchestration-service.ts @@ -4,6 +4,9 @@ import { getConnectorExecutionReadinessIssue } from "@shared/connector-policy"; import type { DesktopResult } from "@shared/contracts"; import type { ConnectorRecord } from "@shared/types"; import type { NotificationOptions } from "./notification-service"; +import { sweepStaleReviews } from "./task-quality-gates"; +import { resolveApprovalDecision } from "./approval-policy"; +import { dispatchAgentMessage } from "./message-dispatch"; type WorkspaceLockMode = "exclusive" | "shared" | "none"; @@ -678,6 +681,34 @@ export function createOrchestrationService({ db.setLastBudgetResetMonth(company.id, currentMonth); logger.info(`[heartbeat] Reset monthly budgets for company ${company.name} (${currentMonth})`); } + + // TASK-04: Stale review escalation sweep + try { + const escalated = sweepStaleReviews(company.id, { + db: db as unknown as import("./task-quality-gates").SweepStaleReviewsDeps["db"], + dispatchAgentMessage: dispatchAgentMessage as unknown as import("./task-quality-gates").SweepStaleReviewsDeps["dispatchAgentMessage"], + resolveApprovalDecision: resolveApprovalDecision as unknown as import("./task-quality-gates").SweepStaleReviewsDeps["resolveApprovalDecision"], + notify: notify as unknown as import("./task-quality-gates").SweepStaleReviewsDeps["notify"], + handleTaskStatusChange: (_taskId: string, _companyId: string, _prev: string, _next: string) => { + publishDomainChanged(); + }, + handleApprovalResolved: (_approvalId: string, _companyId: string, _decision: string) => { + publishDomainChanged(); + }, + emitEvent: (..._args: unknown[]) => {}, + publishDomainChanged, + wakeAgentIfPossible: (agentId: string, cId: string, trigger: string) => { + wakeAgentIfPossible(agentId, cId, trigger); + }, + logger, + }); + if (escalated > 0) { + logger.info(`[heartbeat] Auto-escalated ${escalated} stale review(s) for company ${company.name}`); + publishDomainChanged(); + } + } catch (error) { + logger.error(`[heartbeat] Stale review sweep failed: ${error instanceof Error ? error.message : String(error)}`); + } } catch (error) { logger.error(`[heartbeat] Error processing company ${company.id}: ${error instanceof Error ? error.message : String(error)}`); } diff --git a/tests/unit/task-quality-gates.test.ts b/tests/unit/task-quality-gates.test.ts index 68255855..59be2811 100644 --- a/tests/unit/task-quality-gates.test.ts +++ b/tests/unit/task-quality-gates.test.ts @@ -237,7 +237,7 @@ describe("sweepStaleReviews", () => { it("escalates to manager's manager when task in_review for 35 minutes", () => { const deps = makeSweepDeps(); - const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; + const dbMock = deps.db as unknown as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; // Task in review for 35 min dbMock.queryAll.mockReturnValueOnce([ @@ -265,7 +265,7 @@ describe("sweepStaleReviews", () => { it("does not escalate when task in_review for only 25 minutes (under threshold)", () => { const deps = makeSweepDeps(); - const dbMock = deps.db as { queryAll: ReturnType }; + const dbMock = deps.db as unknown as { queryAll: ReturnType }; dbMock.queryAll.mockReturnValueOnce([ { id: "task-1", assigneeAgentId: "agent-1", updatedAt: TWENTY_FIVE_MIN_AGO, title: "Build login" }, @@ -279,7 +279,7 @@ describe("sweepStaleReviews", () => { it("notifies human when no higher manager (reviewer is CEO)", () => { const deps = makeSweepDeps(); - const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; + const dbMock = deps.db as unknown as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; dbMock.queryAll .mockReturnValueOnce([ @@ -305,7 +305,7 @@ describe("sweepStaleReviews", () => { it("auto-approves pending approval linked to stale review task", () => { const deps = makeSweepDeps(); - const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; + const dbMock = deps.db as unknown as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; // First queryAll: stale task dbMock.queryAll.mockReturnValueOnce([ @@ -332,7 +332,7 @@ describe("sweepStaleReviews", () => { it("catches resolveApprovalDecision error gracefully (race condition)", () => { const deps = makeSweepDeps(); - const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; + const dbMock = deps.db as unknown as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType }; dbMock.queryAll.mockReturnValueOnce([ { id: "task-1", assigneeAgentId: "agent-1", updatedAt: THIRTY_FIVE_MIN_AGO, title: "Build login" }, @@ -352,7 +352,7 @@ describe("sweepStaleReviews", () => { it("advances task status to done after escalation", () => { const deps = makeSweepDeps(); - const dbMock = deps.db as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType; advanceTaskStatus: ReturnType }; + const dbMock = deps.db as unknown as { queryAll: ReturnType; getManagerForAgent: ReturnType; getChainOfCommand: ReturnType; advanceTaskStatus: ReturnType }; dbMock.queryAll.mockReturnValueOnce([ { id: "task-1", assigneeAgentId: "agent-1", updatedAt: THIRTY_FIVE_MIN_AGO, title: "Build login" }, @@ -373,7 +373,7 @@ describe("sweepStaleReviews", () => { it("skips task with no assigneeAgentId", () => { const deps = makeSweepDeps(); - const dbMock = deps.db as { queryAll: ReturnType }; + const dbMock = deps.db as unknown as { queryAll: ReturnType }; dbMock.queryAll.mockReturnValueOnce([ { id: "task-1", assigneeAgentId: null, updatedAt: THIRTY_FIVE_MIN_AGO, title: "Orphan task" },