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); + }); +});