diff --git a/src/main/automation/automation-approval-handler.ts b/src/main/automation/automation-approval-handler.ts index 0506ea7f..b779ec69 100644 --- a/src/main/automation/automation-approval-handler.ts +++ b/src/main/automation/automation-approval-handler.ts @@ -83,6 +83,13 @@ export function handleApprovalResolved( if (deps.isQuitting()) return; const { db, logger, publishDomainChanged, notify, wakeAgentIfPossible, queueBrowserActionExecution } = deps; const { dispatchAutomation, handleTaskStatusChange, runWorkflowPipeline, trackTimer } = callbacks; + const explicitlyWokenAgentIds = new Set(); + + function wakeAgent(agentId: string | null | undefined, trigger: string): void { + if (!agentId) return; + wakeAgentIfPossible(agentId, companyId, trigger); + explicitlyWokenAgentIds.add(agentId); + } const snapshot = db.projection.snapshot.listSnapshot(); const approval = snapshot.approvals.find((entry) => entry.id === approvalId); @@ -134,11 +141,11 @@ export function handleApprovalResolved( companyId, agentId: agent.id, }); - wakeAgentIfPossible(agent.id, companyId, "assignment"); + wakeAgent(agent.id, "assignment"); const ceo = db.core.agents.getCeoForCompany(companyId); if (ceo && ceo.id !== agent.id) { - wakeAgentIfPossible(ceo.id, companyId, "assignment"); + wakeAgent(ceo.id, "assignment"); } publishDomainChanged(); @@ -186,20 +193,15 @@ export function handleApprovalResolved( db.core.tasks.advanceTaskStatus(task.id, "done", "Board approved deliverable."); handleTaskStatusChange(task.id, companyId, task.status, "done"); db.core.activity.addActivity({ companyId, actor: "board", action: "deliverable.approved", entityType: "task", entityId: task.id, detail: approval.decisionNote || "Deliverable approved by board." }); - } else if (decision === "rejected") { + } else if (decision === "rejected" || decision === "revision_requested") { + const previousStatus = task.status; + const statusMessage = decision === "rejected" + ? `Board rejected deliverable: ${approval.decisionNote || "No reason given."}` + : `Board requested revisions: ${approval.decisionNote || "No reason given."}`; recordReviewFeedbackKnowledge(db, approval, decision); - db.core.tasks.advanceTaskStatus(task.id, "in_progress", `Board rejected deliverable: ${approval.decisionNote || "No reason given."}`); - handleTaskStatusChange(task.id, companyId, task.status, "in_progress"); - if (task.assigneeAgentId) { - wakeAgentIfPossible(task.assigneeAgentId, companyId, "revision_needed"); - } - } else if (decision === "revision_requested") { - db.core.tasks.advanceTaskStatus(task.id, "in_progress", `Board requested revisions: ${approval.decisionNote || "No reason given."}`); - handleTaskStatusChange(task.id, companyId, task.status, "in_progress"); - recordReviewFeedbackKnowledge(db, approval, decision); - if (task.assigneeAgentId) { - wakeAgentIfPossible(task.assigneeAgentId, companyId, "revision_needed"); - } + db.core.tasks.advanceTaskStatus(task.id, "todo", statusMessage); + handleTaskStatusChange(task.id, companyId, previousStatus, "todo"); + wakeAgent(task.assigneeAgentId, "revision_needed"); } } else if (approval.requestedByAgentId) { const agent = db.core.entity.getAgent(approval.requestedByAgentId); @@ -208,7 +210,7 @@ export function handleApprovalResolved( logger.info(`[deliverable-review] Resumed heartbeat for ${agent.name} after user ${decision}.`); } if (decision !== "approved") { - wakeAgentIfPossible(approval.requestedByAgentId, companyId, "revision_needed"); + wakeAgent(approval.requestedByAgentId, "revision_needed"); } } @@ -366,6 +368,8 @@ export function handleApprovalResolved( } if (approval.requestedByAgentId) { - wakeAgentIfPossible(approval.requestedByAgentId, companyId, "approval_resolved"); + if (!explicitlyWokenAgentIds.has(approval.requestedByAgentId)) { + wakeAgentIfPossible(approval.requestedByAgentId, companyId, "approval_resolved"); + } } } diff --git a/tests/unit/approval-review-feedback-writeback.test.ts b/tests/unit/approval-review-feedback-writeback.test.ts index 3aeea474..b0863cdb 100644 --- a/tests/unit/approval-review-feedback-writeback.test.ts +++ b/tests/unit/approval-review-feedback-writeback.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { afterEach, describe, expect, it, vi } from "vitest"; import { AppDatabase } from "../../src/main/database"; import { handleApprovalResolved } from "../../src/main/automation/automation-approval-handler"; +import { createHeartbeat } from "../../src/main/orchestration/heartbeat"; const tempDirs: string[] = []; @@ -18,7 +19,9 @@ afterEach(async () => { }); describe("deliverable review feedback writeback", () => { - it("persists high-importance knowledge when the board requests revisions on a deliverable", async () => { + it.each(["rejected", "revision_requested"] as const)( + "persists high-importance knowledge when the board returns %s on a deliverable", + async (decision) => { const profileDir = await createTempDir("agentcompany-review-feedback-"); const db = new AppDatabase(profileDir); db.init(); @@ -87,7 +90,7 @@ describe("deliverable review feedback writeback", () => { db.core.approvals.decideApproval({ approvalId, - state: "revision_requested", + state: decision, decisionNote: "Add stronger competitor evidence and a clearer recommendation.", }); @@ -105,7 +108,7 @@ describe("deliverable review feedback writeback", () => { handleTaskStatusChange: vi.fn(), runWorkflowPipeline: async () => {}, trackTimer: vi.fn(), - }, approvalId, companyId, "revision_requested"); + }, approvalId, companyId, decision); const knowledgeEntries = db.projection.snapshot.listSnapshot().knowledgeBase.filter((entry) => entry.companyId === companyId); expect(knowledgeEntries).toHaveLength(1); @@ -113,13 +116,147 @@ describe("deliverable review feedback writeback", () => { expect(knowledgeEntries[0]?.importance).toBe("high"); expect(knowledgeEntries[0]?.referencedEntityType).toBe("task"); expect(knowledgeEntries[0]?.referencedEntityId).toBe(taskId); - expect(knowledgeEntries[0]?.content).toContain("revision_requested"); + expect(knowledgeEntries[0]?.content).toContain(decision); expect(knowledgeEntries[0]?.content).toContain("stronger competitor evidence"); expect(wakeAgentIfPossible).toHaveBeenCalledWith(agentId, companyId, "revision_needed"); - expect(db.core.entity.getTask(taskId).status).toBe("in_progress"); + expect(wakeAgentIfPossible).toHaveBeenCalledTimes(1); + expect(db.core.entity.getTask(taskId).status).toBe("todo"); const document = db.projection.snapshot.listSnapshot().documents.find((entry) => entry.id === documentId); expect(document?.status).toBe("draft"); + db.close(); + }, + ); + + it("makes a revision request immediately checkoutable by the assignee heartbeat", async () => { + const profileDir = await createTempDir("agentcompany-review-checkout-"); + const db = new AppDatabase(profileDir); + db.init(); + + db.core.connectors.saveConnector({ + id: "openai-codex", + model: "gpt-5.4", + envBindingText: "", + notes: "Test connector", + }); + + const companyId = db.core.company.saveCompany({ + name: "Review Checkout Co", + description: "Tests review revision checkout", + status: "active", + }); + const workspaceId = db.core.workspaces.saveWorkspace({ + companyId, + name: "Primary", + localPath: profileDir, + repoUrl: "", + repoRef: "", + isPrimary: true, + }); + const agentId = db.core.agents.saveAgent({ + companyId, + name: "Research Lead", + role: "Researcher", + title: "Research Lead", + status: "active", + reportsTo: null, + connectorId: "openai-codex", + workspaceId, + model: "gpt-5.4", + capabilities: "Research and synthesis", + budgetMonthlyUsd: 180, + department: "research", + }); + const taskId = db.core.tasks.saveTask({ + companyId, + title: "Draft market report", + description: "Produce a market report with concrete evidence and business insight.", + assigneeAgentId: agentId, + workspaceId, + priority: "high", + status: "in_review", + }); + const documentId = db.core.documents.saveDocument({ + companyId, + type: "knowledge_article", + title: "Market report", + content: "Draft report body", + authorAgentId: agentId, + status: "in_review", + }); + const approvalId = db.core.approvals.requestApproval({ + companyId, + relatedTaskId: taskId, + requestedByAgentId: agentId, + type: "deliverable_review", + payloadSummary: "Review market report deliverable", + impactSummary: "Board review required before the report is finalized.", + payloadJson: JSON.stringify({ documentIds: [documentId] }), + }); + + db.core.approvals.decideApproval({ + approvalId, + state: "revision_requested", + decisionNote: "Add stronger competitor evidence and a clearer recommendation.", + }); + + const dispatchRunToWorker = vi.fn(); + const heartbeat = createHeartbeat( + { + db, + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + publishDomainChanged: vi.fn(), + emitEvent: vi.fn(), + notify: vi.fn(), + createRunToken: () => "test-token", + getApiPort: () => 3210, + dispatchAutomation: vi.fn(), + hasWorkerReady: () => true, + enqueueWorkerRun: vi.fn(), + isQuitting: () => false, + listRuntimeSkillEntries: () => [], + budgetService: { + autoCloseExpiredIncidents: vi.fn(() => 0), + evaluateBudgetGate: vi.fn(() => ({ allowed: true, blockedBy: [] })), + } as never, + checkpointEngine: { shouldPauseHeartbeat: () => false }, + }, + () => ({ + workspace: null, + lockMode: "none", + source: "none", + observationPath: profileDir, + }), + () => null, + dispatchRunToWorker, + (filter) => db.core.runs.hasActiveRun(filter), + ); + + handleApprovalResolved({ + db, + logger: { info: () => {}, warn: () => {}, error: () => {} }, + publishDomainChanged: () => {}, + notify: vi.fn(), + wakeAgentIfPossible: heartbeat.wakeAgentIfPossible, + queueBrowserActionExecution: () => {}, + isQuitting: () => false, + }, { + dispatchAutomation: vi.fn(), + handleTaskStatusChange: vi.fn(), + runWorkflowPipeline: async () => {}, + trackTimer: vi.fn(), + }, approvalId, companyId, "revision_requested"); + + const task = db.core.entity.getTask(taskId); + expect(task.status).toBe("in_progress"); + expect(db.core.runs.hasActiveRun({ companyId, taskId, agentId })).toBe(true); + expect(dispatchRunToWorker).toHaveBeenCalledTimes(1); + expect(dispatchRunToWorker).toHaveBeenCalledWith(expect.objectContaining({ + companyId, + task: expect.objectContaining({ id: taskId }), + trigger: "revision_needed", + })); + db.close(); }); });