From efb4bddab6ef303df8fc1287d36f8b4109683926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sun, 19 Apr 2026 14:59:05 -0400 Subject: [PATCH] Count work_products toward a run's deliverable judgment The deliverable check used to compare a run against knowledge entries, documents, and substantial task comments only, so a research or content task that produced its output as a markdown file on disk (via write/edit/bash) stayed pinned in_progress even though the deliverable was already sitting in `work_products`. Split the DB query into `collectRunDeliverableSignals`, which returns the four surfaces (knowledge / document / work product / substantial comment) explicitly, and push the per-task-type acceptance logic into `automation-run-completion`. Each task type declares which signal set counts as a valid deliverable: content tasks now require a document or an on-disk artifact and will not advance on a knowledge entry alone; research and general accept any of the four; code and management preserve their broader surface. Tests updated to cover the new signal and the content/knowledge rejection case. --- .../automation/automation-run-completion.ts | 53 +++++++++++++--- src/main/database.ts | 4 +- src/main/database/knowledge.ts | 61 +++++++++++++------ tests/unit/task-completion.test.ts | 46 ++++++++++++-- 4 files changed, 132 insertions(+), 32 deletions(-) diff --git a/src/main/automation/automation-run-completion.ts b/src/main/automation/automation-run-completion.ts index 4be1602a..73574412 100644 --- a/src/main/automation/automation-run-completion.ts +++ b/src/main/automation/automation-run-completion.ts @@ -34,13 +34,46 @@ export function needsDeliverableCheck(taskType: DeliverableTaskType | string): b return taskType === "research" || taskType === "content"; } -export interface DeliverableCheckInput { - taskType: DeliverableTaskType | string; +export interface DeliverableSignals { hasKnowledgeEntry: boolean; hasDocument: boolean; + hasWorkProduct: boolean; hasSubstantialComment: boolean; } +export interface DeliverableCheckInput extends DeliverableSignals { + taskType: DeliverableTaskType | string; +} + +/** + * Valid artifact surfaces for each task type. A task advances to review + * only when at least one signal from the set is present. The `work_product` + * surface covers filesystem deliverables the agent writes via bash / write + * / edit; before this table included it, a research task that produced a + * markdown file instead of a knowledge entry would stay in `in_progress` + * forever even though the deliverable was sitting on disk. + */ +const VALID_DELIVERABLE_SIGNALS: Record> = { + research: new Set(["hasKnowledgeEntry", "hasDocument", "hasWorkProduct", "hasSubstantialComment"]), + content: new Set(["hasDocument", "hasWorkProduct"]), + code: new Set(["hasWorkProduct", "hasDocument"]), + management: new Set(["hasKnowledgeEntry", "hasDocument", "hasWorkProduct", "hasSubstantialComment"]), + general: new Set(["hasKnowledgeEntry", "hasDocument", "hasWorkProduct", "hasSubstantialComment"]), +}; + +function validSignalsForTaskType(taskType: string): ReadonlySet { + return VALID_DELIVERABLE_SIGNALS[taskType as DeliverableTaskType] + ?? VALID_DELIVERABLE_SIGNALS.general; +} + +export function hasValidDeliverableFor(taskType: string, signals: DeliverableSignals): boolean { + const valid = validSignalsForTaskType(taskType); + for (const key of valid) { + if (signals[key]) return true; + } + return false; +} + /** Returns the next task status and comment after a successful run. */ export function determinePostRunStatus(input: DeliverableCheckInput): { nextStatus: "in_review" | "in_progress"; @@ -52,8 +85,7 @@ export function determinePostRunStatus(input: DeliverableCheckInput): { comment: "Run completed successfully. Awaiting review.", }; } - const delivered = input.hasKnowledgeEntry || input.hasDocument || input.hasSubstantialComment; - if (delivered) { + if (hasValidDeliverableFor(input.taskType, input)) { return { nextStatus: "in_review", comment: "Run completed with deliverable. Awaiting review.", @@ -112,8 +144,15 @@ export function checkBudgetThreshold( // ─── Internal DB-backed deliverable check ─────────────────────────────────── -function checkRunProducedDeliverable(db: AppDatabase, runId: string, companyId: string, taskId: string): boolean { - return db.runProducedDeliverable(runId, companyId, taskId); +function checkRunProducedDeliverable( + db: AppDatabase, + runId: string, + companyId: string, + taskId: string, + taskType: string, +): boolean { + const signals = db.collectRunDeliverableSignals(runId, companyId, taskId); + return hasValidDeliverableFor(taskType, signals); } /** Returns important documents created during this run (matching IMPORTANT_DOCUMENT_TYPES). */ @@ -238,7 +277,7 @@ export function processRunCompletion( const taskType = task.taskType ?? "general"; if (needsDeliverableCheck(taskType)) { - const hasDeliverable = checkRunProducedDeliverable(db, run.id, run.companyId, task.id); + const hasDeliverable = checkRunProducedDeliverable(db, run.id, run.companyId, task.id, taskType); if (!hasDeliverable) { // Don't auto-advance -- flag for manager review db.advanceTaskStatus(task.id, "in_progress", diff --git a/src/main/database.ts b/src/main/database.ts index 00342e4f..4ffa948b 100644 --- a/src/main/database.ts +++ b/src/main/database.ts @@ -124,7 +124,9 @@ export class AppDatabase extends AppDatabaseBase { saveSprint(input: { id?: string | null; companyId: string; name: string; goal?: string; startDate: string; endDate: string; status?: string; retrospectiveNotes?: string; velocityPoints?: number; completedPoints?: number }) { return this.knowledgeMod.saveSprint(input); } deleteSprint(id: string, companyId: string) { this.knowledgeMod.deleteSprint(id, companyId); } listSprints(companyId: string) { return this.knowledgeMod.listSprints(companyId); } - runProducedDeliverable(runId: string, companyId: string, taskId: string) { return this.knowledgeMod.runProducedDeliverable(runId, companyId, taskId); } + collectRunDeliverableSignals(runId: string, companyId: string, taskId: string) { + return this.knowledgeMod.collectRunDeliverableSignals(runId, companyId, taskId); + } // ─── Automation ──────────────────────────────────────────────────────────── diff --git a/src/main/database/knowledge.ts b/src/main/database/knowledge.ts index c67a6b83..bcb838f8 100644 --- a/src/main/database/knowledge.ts +++ b/src/main/database/knowledge.ts @@ -173,45 +173,66 @@ export function buildKnowledgeMethods( }, /** - * Returns true when the given run caused a durable deliverable — a - * knowledge entry, a document, or a substantial task comment (>100 - * chars). Causal binding via source_run_id is preferred; time-window - * fallback handles legacy rows written before that column existed. + * Collects the set of deliverable signals produced by a single run. + * Each signal reflects a distinct artifact surface: * - * Encapsulates the five raw SELECTs that automation-run-completion - * previously issued via db.queryAll, so schema changes to knowledge_base - * / documents / task_comments surface here rather than silently. + * - knowledge entry bound to the run (or written during its window) + * - document bound to the run (or written during its window) + * - filesystem `work_product` registered with `created_by_run_id = run` + * - substantial task comment (>100 chars) posted during the run window + * + * The caller in `automation-run-completion` decides which signals are + * acceptable for a given task type, so schema changes to any of the + * source tables surface here rather than in an opaque boolean. */ - runProducedDeliverable(runId: string, companyId: string, taskId: string): boolean { + collectRunDeliverableSignals( + runId: string, + companyId: string, + taskId: string, + ): { + hasKnowledgeEntry: boolean; + hasDocument: boolean; + hasWorkProduct: boolean; + hasSubstantialComment: boolean; + } { const causalKnowledge = readAll<{ id: string }>( "select id from knowledge_base where company_id = ? and source_run_id = ? limit 1", companyId, runId, ); - if (causalKnowledge.length > 0) return true; + const legacyKnowledge = causalKnowledge.length === 0 + ? readAll<{ id: string }>( + "select id from knowledge_base where company_id = ? and source_run_id is null and created_at >= (select started_at from runs where id = ?) limit 1", + companyId, runId, + ) + : []; const causalDocs = readAll<{ id: string }>( "select id from documents where company_id = ? and source_run_id = ? limit 1", companyId, runId, ); - if (causalDocs.length > 0) return true; + const legacyDocs = causalDocs.length === 0 + ? readAll<{ id: string }>( + "select id from documents where company_id = ? and source_run_id is null and created_at >= (select started_at from runs where id = ?) limit 1", + companyId, runId, + ) + : []; - const legacyKnowledge = readAll<{ id: string }>( - "select id from knowledge_base where company_id = ? and source_run_id is null and created_at >= (select started_at from runs where id = ?) limit 1", + const workProducts = readAll<{ id: string }>( + "select id from work_products where company_id = ? and created_by_run_id = ? limit 1", companyId, runId, ); - if (legacyKnowledge.length > 0) return true; - - const legacyDocs = readAll<{ id: string }>( - "select id from documents where company_id = ? and source_run_id is null and created_at >= (select started_at from runs where id = ?) limit 1", - companyId, runId, - ); - if (legacyDocs.length > 0) return true; const recentComments = readAll<{ body: string }>( "select body from task_comments where task_id = ? and created_at >= (select started_at from runs where id = ?) and length(body) > 100", taskId, runId, ); - return recentComments.length > 0; + + return { + hasKnowledgeEntry: causalKnowledge.length > 0 || legacyKnowledge.length > 0, + hasDocument: causalDocs.length > 0 || legacyDocs.length > 0, + hasWorkProduct: workProducts.length > 0, + hasSubstantialComment: recentComments.length > 0, + }; }, /** diff --git a/tests/unit/task-completion.test.ts b/tests/unit/task-completion.test.ts index a64911e8..599d9ac9 100644 --- a/tests/unit/task-completion.test.ts +++ b/tests/unit/task-completion.test.ts @@ -9,8 +9,11 @@ import { * * When a run completes successfully on a non-code task (research, content), * the system checks whether the agent produced a tangible deliverable - * (knowledge entry, document, or substantial comment). If not, the task - * stays in_progress and the manager is notified. + * (knowledge entry, document, filesystem work product, or substantial + * comment). The valid signal set is defined per task type: content tasks + * require a document or filesystem artifact, research tasks accept any of + * the four surfaces. If no valid signal is present the task stays + * in_progress and the manager is notified. */ describe("non-code task deliverable enforcement", () => { @@ -42,6 +45,7 @@ describe("non-code task deliverable enforcement", () => { taskType: "code", hasKnowledgeEntry: false, hasDocument: false, + hasWorkProduct: false, hasSubstantialComment: false, }); expect(result.nextStatus).toBe("in_review"); @@ -53,6 +57,7 @@ describe("non-code task deliverable enforcement", () => { taskType: "general", hasKnowledgeEntry: false, hasDocument: false, + hasWorkProduct: false, hasSubstantialComment: false, }); expect(result.nextStatus).toBe("in_review"); @@ -63,6 +68,7 @@ describe("non-code task deliverable enforcement", () => { taskType: "management", hasKnowledgeEntry: false, hasDocument: false, + hasWorkProduct: false, hasSubstantialComment: false, }); expect(result.nextStatus).toBe("in_review"); @@ -73,6 +79,7 @@ describe("non-code task deliverable enforcement", () => { taskType: "research", hasKnowledgeEntry: false, hasDocument: false, + hasWorkProduct: false, hasSubstantialComment: false, }); expect(result.nextStatus).toBe("in_progress"); @@ -85,6 +92,7 @@ describe("non-code task deliverable enforcement", () => { taskType: "research", hasKnowledgeEntry: true, hasDocument: false, + hasWorkProduct: false, hasSubstantialComment: false, }); expect(result.nextStatus).toBe("in_review"); @@ -96,6 +104,18 @@ describe("non-code task deliverable enforcement", () => { taskType: "research", hasKnowledgeEntry: false, hasDocument: true, + hasWorkProduct: false, + hasSubstantialComment: false, + }); + expect(result.nextStatus).toBe("in_review"); + }); + + it("advances research task to in_review when work product exists", () => { + const result = determinePostRunStatus({ + taskType: "research", + hasKnowledgeEntry: false, + hasDocument: false, + hasWorkProduct: true, hasSubstantialComment: false, }); expect(result.nextStatus).toBe("in_review"); @@ -106,6 +126,7 @@ describe("non-code task deliverable enforcement", () => { taskType: "research", hasKnowledgeEntry: false, hasDocument: false, + hasWorkProduct: false, hasSubstantialComment: true, }); expect(result.nextStatus).toBe("in_review"); @@ -116,6 +137,7 @@ describe("non-code task deliverable enforcement", () => { taskType: "content", hasKnowledgeEntry: false, hasDocument: false, + hasWorkProduct: false, hasSubstantialComment: false, }); expect(result.nextStatus).toBe("in_progress"); @@ -127,19 +149,35 @@ describe("non-code task deliverable enforcement", () => { taskType: "content", hasKnowledgeEntry: false, hasDocument: true, + hasWorkProduct: false, hasSubstantialComment: false, }); expect(result.nextStatus).toBe("in_review"); }); - it("advances content task when any deliverable exists", () => { + it("advances content task to in_review when work product exists", () => { const result = determinePostRunStatus({ taskType: "content", - hasKnowledgeEntry: true, + hasKnowledgeEntry: false, hasDocument: false, + hasWorkProduct: true, hasSubstantialComment: false, }); expect(result.nextStatus).toBe("in_review"); }); + + it("keeps content task in_progress when only a knowledge entry exists", () => { + // content deliverables live in documents or on disk — a knowledge + // entry alone does not constitute a publishable content artifact, + // so the task stays in_progress for manager follow-up. + const result = determinePostRunStatus({ + taskType: "content", + hasKnowledgeEntry: true, + hasDocument: false, + hasWorkProduct: false, + hasSubstantialComment: false, + }); + expect(result.nextStatus).toBe("in_progress"); + }); }); });