From e90b15a7a2491cf1728686802f732b4803fe16f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 8 Oct 2026 17:54:28 -0400 Subject: [PATCH] Q109: a declined deliverable reopens its task and tells the next session why Co-Authored-By: Claude Opus 5.5 --- src/main/services/approval-service.ts | 15 +++++++++++++-- src/main/services/held-deliverables.ts | 12 +++++++----- src/main/services/run-journal.ts | 14 ++++++++++++++ tests/unit/execution-modes.test.ts | 25 +++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/main/services/approval-service.ts b/src/main/services/approval-service.ts index 776ee548..fa1b8c03 100644 --- a/src/main/services/approval-service.ts +++ b/src/main/services/approval-service.ts @@ -1,10 +1,11 @@ import type { DatabaseSync } from "node:sqlite"; import type { CreateApprovalInput, DecideApprovalInput } from "../../shared/commands"; -import { APPROVAL_STATE, ROLE_KEY } from "../../shared/domain"; +import { APPROVAL_STATE, ROLE_KEY, TASK_STATUS } from "../../shared/domain"; import { createId } from "../../shared/ids"; import { AuditService } from "./audit-service"; import { settleHire } from "./agent-admission"; import { settleHeldDeliverable } from "./held-deliverables"; +import { OrganizationService } from "./organization-service"; import { IdentityAccessService } from "./identity-access-service"; import { OrgGovernanceService } from "./org-governance-service"; @@ -104,10 +105,20 @@ export class ApprovalService { }); // Last, so nothing after it can fail once a file is on disk. if (approval.source_entity_type === "held_deliverable" && approval.source_entity_id) { - settleHeldDeliverable(this.db, approval.source_entity_id, input.state === APPROVAL_STATE.APPROVED); + const approved = input.state === APPROVAL_STATE.APPROVED; + const taskId = settleHeldDeliverable(this.db, approval.source_entity_id, approved); + if (!approved && taskId) this.reopenForRework(input.orgId, taskId); } } + /** Declined work goes back to its task's assignee: the task returns to in progress, wherever it had got to. */ + private reopenForRework(orgId: string, taskId: string): void { + const task = this.db.prepare("select status from tasks where id = ? and org_id = ?").get(taskId, orgId) as { status: string } | undefined; + if (!task || task.status === TASK_STATUS.IN_PROGRESS) return; + new OrganizationService(this.db).updateTaskStatus(orgId, taskId, TASK_STATUS.IN_PROGRESS); + console.info(`[deliverables] task ${taskId} reopened for rework after a declined deliverable`); + } + private assertRequesterBelongsToOrg(input: CreateApprovalInput): void { if (input.requestedByType === "employee") { if (!input.requestedById) throw new Error("Requesting employee is required."); diff --git a/src/main/services/held-deliverables.ts b/src/main/services/held-deliverables.ts index 7ce961eb..10060015 100644 --- a/src/main/services/held-deliverables.ts +++ b/src/main/services/held-deliverables.ts @@ -41,9 +41,10 @@ export function holdDeliverable( db: DatabaseSync, input: { id: string; approvalId: string; orgId: string; agentId: string; taskId: string | null; goalId: string | null; projectId: string | null; target: HeldTarget; content: string }, ): void { - const target = input.target; - // A file path that could never be written is refused now, not at approval. - if (target.kind === "file") resolveWorkspacePath(target.workspaceRoot, target.relativePath); + // A file path that could never be written is refused now, not at approval; one spelling of each path is kept. + const target = input.target.kind === "file" + ? { ...input.target, relativePath: resolveWorkspacePath(input.target.workspaceRoot, input.target.relativePath).relativePosix } + : input.target; db.prepare(` insert into held_deliverables( id, org_id, approval_id, kind, agent_id, task_id, goal_id, project_id, workspace_root, relative_path, @@ -64,14 +65,15 @@ export function holdDeliverable( console.info(`[deliverables] held ${target.kind} ${input.id} for review ${input.approvalId}`); } -/** Writes an approved deliverable where it was meant to go; a declined one is never written. */ -export function settleHeldDeliverable(db: DatabaseSync, heldId: string, approved: boolean): void { +/** Writes an approved deliverable where it was meant to go; a declined one is never written. Returns the task it belongs to. */ +export function settleHeldDeliverable(db: DatabaseSync, heldId: string, approved: boolean): string | null { const row = db.prepare("select * from held_deliverables where id = ? and state = 'held'").get(heldId) as HeldRow | undefined; if (!row) throw new Error("The held deliverable of this review was not found."); db.prepare("update held_deliverables set state = ? where id = ?").run(approved ? "published" : "declined", heldId); // Publication is the last write, so a file reaches disk only once every database change before it has succeeded. if (approved) publish(db, row); console.info(`[deliverables] ${row.kind} ${heldId} ${approved ? "published" : "declined"}`); + return row.task_id; } function publish(db: DatabaseSync, row: HeldRow): void { diff --git a/src/main/services/run-journal.ts b/src/main/services/run-journal.ts index f8b3a28f..502e1622 100644 --- a/src/main/services/run-journal.ts +++ b/src/main/services/run-journal.ts @@ -9,6 +9,9 @@ const READ_ONLY_TOOLS = new Set([ ]); const JOURNAL_LINES = 40; const FIELD_CHARS = 160; +const DECLINED_LINES = 5; +/** What a held deliverable would become, over a `held_deliverables` row alias: one file, one existing document, or a new document by type and title. */ +const heldTarget = (row: string) => `(case ${row}.kind when 'file' then ${row}.workspace_root || '/' || ${row}.relative_path when 'document_update' then ${row}.document_id else ${row}.doc_type || ':' || ${row}.title end)`; /** Records a call that tried to change something, with its result, so later sessions build on it instead of repeating it. */ export function recordRunAction(db: DatabaseSync, input: { orgId: string; runId: string; taskId: string; call: ToolCall; output: string }): void { @@ -35,6 +38,17 @@ export function runJournalLines(db: DatabaseSync, taskId: string): string[] { lines.push("", "Earlier sessions on this task already made these calls; each shows its result. Do not redo the ones that succeeded; retry or fix only what failed:"); for (const action of actions) lines.push(`- ${action.tool_name}(${action.arguments_excerpt}) -> ${action.result_excerpt}`); } + const declined = db.prepare(` + select coalesce(h.title, h.relative_path) as name, a.decision_notes as notes + from held_deliverables h join approvals a on a.id = h.approval_id + where h.task_id = ? and h.state = 'declined' + and not exists ( + select 1 from held_deliverables later + where later.task_id = h.task_id and later.rowid > h.rowid and later.kind = h.kind and ${heldTarget("later")} = ${heldTarget("h")} + ) + order by h.rowid desc limit ? + `).all(taskId, DECLINED_LINES) as Array<{ name: string; notes: string | null }>; + for (const row of declined) lines.push("", `The board declined ${row.name}${row.notes ? `: ${row.notes.trim().replace(/[.。!！]+$/, "")}` : ""}. Redo it before closing the task.`); if (lastFailure?.error_message) { lines.push("", `The last session on this task failed with: ${lastFailure.error_message}. Pick up from there instead of starting over.`); } diff --git a/tests/unit/execution-modes.test.ts b/tests/unit/execution-modes.test.ts index 8c2ac8bd..3681afa9 100644 --- a/tests/unit/execution-modes.test.ts +++ b/tests/unit/execution-modes.test.ts @@ -8,6 +8,7 @@ import { OrganizationService } from "../../src/main/services/organization-servic import { ChatService } from "../../src/main/services/chat-service"; import { dispatchAgentTool, type AgentToolContext } from "../../src/main/services/agent-tools"; import { ApprovalService } from "../../src/main/services/approval-service"; +import { runJournalLines } from "../../src/main/services/run-journal"; import { executionApprovalProblem, requireAssignedAgent } from "../../src/main/services/task-run-preparation"; import { TASK_PRIORITY } from "../../src/shared/domain"; import type { TaskExecutionTaskRow } from "../../src/main/services/task-prompts"; @@ -114,6 +115,30 @@ describe("execution modes", () => { finish(); }); + test("a declined deliverable sends its task back to work, with the board's reason in the next session", () => { + const h = harness("ai_cofounder"); + const db = h.connection.db; + const employeeId = (db.prepare("select id from employees where org_id = ?").get(h.orgId) as { id: string }).id; + const taskId = h.connection.transaction(() => h.deps.organization.createTask({ + orgId: h.orgId, title: "Write the plan", description: "d", status: "in_progress", assigneeAgentId: h.ceoId, risk: "low", + projectId: null, goalId: null, workspaceId: null, priority: TASK_PRIORITY.NORMAL, requiresFilesystem: false, + })); + dispatchAgentTool(h.deps, { ...h.ctx, taskId }, call("write_deliverable", { path: "plan.md", content: "draft" })); + // The operator closed the task while the draft waited; declining it still reopens the work. + h.connection.transaction(() => h.deps.organization.updateTaskStatus(h.orgId, taskId, "done")); + const review = (db.prepare("select id from approvals where org_id = ? and kind = 'deliverable_review'").get(h.orgId) as { id: string }).id; + h.connection.transaction(() => new ApprovalService(db).decide({ orgId: h.orgId, approvalId: review, state: "declined", decidedByEmployeeId: employeeId, notes: "Cite the market data." })); + expect(db.prepare("select status from tasks where id = ?").get(taskId)).toEqual({ status: "in_progress" }); + expect(runJournalLines(db, taskId).join("\n")).toContain("The board declined plan.md: Cite the market data. Redo it before closing the task."); + // Another deliverable under the same name elsewhere does not answer the decline; a resubmission of the same one does. + const elsewhere = join(h.ctx.workspace!.localPath, "other"); + mkdirSync(elsewhere); + dispatchAgentTool(h.deps, { ...h.ctx, taskId, workspace: { id: "wsp_other", localPath: elsewhere, writeAccess: true } }, call("write_deliverable", { path: "plan.md", content: "x" })); + expect(runJournalLines(db, taskId).join("\n")).toContain("The board declined plan.md"); + dispatchAgentTool(h.deps, { ...h.ctx, taskId }, call("write_deliverable", { path: "./plan.md", content: "with data" })); + expect(runJournalLines(db, taskId).join("\n")).not.toContain("The board declined plan.md"); + }); + test("an approved file whose registration fails leaves the workspace as it was", () => { const h = harness("ai_cofounder"); const db = h.connection.db;