Merge branch 'worktree-agent-a205edfb'
# Conflicts: # .planning/STATE.md # src/main/orchestration-service.ts # src/main/task-quality-gates.ts # tests/unit/task-quality-gates.test.ts
This commit is contained in:
ZacharyZhang-NY committed
commit
fd3f5accc1
3 files changed
+378
-2
No files matched your search
@@ -6,6 +6,9 @@ import type { DesktopResult } from "@shared/contracts";
|
||||
import type { ConnectorRecord } from "@shared/types";
|
||||
import type { NotificationOptions } from "./notification-service";
|
||||
import type { createBudgetEnforcementService } from "./budget-enforcement-service";
|
||||
import { sweepStaleReviews } from "./task-quality-gates";
|
||||
import { resolveApprovalDecision } from "./approval-policy";
|
||||
import { dispatchAgentMessage } from "./message-dispatch";
|
||||
|
||||
type WorkspaceLockMode = "exclusive" | "shared" | "none";
|
||||
|
||||
@@ -789,6 +792,35 @@ export function createOrchestrationService({
|
||||
if (closedIncidents > 0) {
|
||||
logger.info(`[heartbeat] Auto-closed ${closedIncidents} expired budget incident(s) for company ${company.name}`);
|
||||
}
|
||||
|
||||
// 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)}`);
|
||||
}
|
||||
|
||||
@@ -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: <T>(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;
|
||||
}
|
||||
@@ -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> = {}): 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 unknown as { queryAll: ReturnType<typeof vi.fn>; getManagerForAgent: ReturnType<typeof vi.fn>; getChainOfCommand: ReturnType<typeof vi.fn> };
|
||||
|
||||
// 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 unknown as { queryAll: ReturnType<typeof vi.fn> };
|
||||
|
||||
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 unknown as { queryAll: ReturnType<typeof vi.fn>; getManagerForAgent: ReturnType<typeof vi.fn>; getChainOfCommand: ReturnType<typeof vi.fn> };
|
||||
|
||||
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 unknown as { queryAll: ReturnType<typeof vi.fn>; getManagerForAgent: ReturnType<typeof vi.fn>; getChainOfCommand: ReturnType<typeof vi.fn> };
|
||||
|
||||
// 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 unknown as { queryAll: ReturnType<typeof vi.fn>; getManagerForAgent: ReturnType<typeof vi.fn>; getChainOfCommand: ReturnType<typeof vi.fn> };
|
||||
|
||||
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<typeof vi.fn>).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 unknown as { queryAll: ReturnType<typeof vi.fn>; getManagerForAgent: ReturnType<typeof vi.fn>; getChainOfCommand: ReturnType<typeof vi.fn>; advanceTaskStatus: ReturnType<typeof vi.fn> };
|
||||
|
||||
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 unknown as { queryAll: ReturnType<typeof vi.fn> };
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user