diff --git a/.changeset/retriage-renamed-intake-column.md b/.changeset/retriage-renamed-intake-column.md new file mode 100644 index 0000000000..294eaa5b31 --- /dev/null +++ b/.changeset/retriage-renamed-intake-column.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix user comments not invalidating spec approval on Coding (Ideas) cards. +category: fix +dev: `addComment`'s re-triage gate listed the legacy `todo`/`triage` column ids, which miss a workflow with a renamed intake column — `builtin:coding-ideas` uses `ideas`. An operator comment on such a card awaiting spec approval invalidated nothing. The gate now resolves the intake/hold roles from the card's own workflow, only for user comments, falling back to the legacy pair when no workflow resolves. diff --git a/packages/core/src/__tests__/postgres/store-comments.pg.test.ts b/packages/core/src/__tests__/postgres/store-comments.pg.test.ts index 6b87c961e3..acde5dfced 100644 --- a/packages/core/src/__tests__/postgres/store-comments.pg.test.ts +++ b/packages/core/src/__tests__/postgres/store-comments.pg.test.ts @@ -239,4 +239,68 @@ pgTest("TaskStore addSteeringComment (PostgreSQL)", () => { expect(final.comments).toHaveLength(2); expect(final.comments!.map((c) => c.text).sort()).toEqual(["Comment A", "Comment B"]); }); + + /* + FNXC:PostCommentRetriage 2026-07-30-00:20 (renamed intake column): + #2608 fixed the awaiting-approval invalidation by dropping the INNER column re-checks, keeping the + outer caller gate as `column === "todo" || column === "triage"` on the grounds that it "covers BOTH + vocabularies". + + It covers both LEGACY vocabularies. It does not cover a RENAMED one. `builtin:coding-ideas` places + its intake in `ideas`, which matches neither literal, so an operator comment on an Ideas card + awaiting spec approval still invalidates nothing: the approval stands and the task proceeds on the + spec the operator was correcting. Same defect as the reported one, one workflow over. + + The caller DOES have what it needs to ask (`store` + `id`), so the gate resolves the intake/hold + roles rather than listing ids. + */ + it("invalidates approval on a RENAMED intake column (Coding (Ideas) → \"ideas\")", async () => { + const store = h.store(); + const task = await store.createTask({ + description: "ideas awaiting approval", + workflowId: "builtin:coding-ideas", + }); + expect(task.column, "Ideas' intake role is `ideas` — matching neither legacy literal").toBe("ideas"); + await store.updateTask(task.id, { status: "awaiting-approval" }); + + await store.addTaskComment(task.id, "Narrow this to the import path only", "user"); + + const after = await store.getTask(task.id); + expect( + after.status, + "a renamed intake column must invalidate the approval too, not leave it standing", + ).not.toBe("awaiting-approval"); + }); + + it("re-triages an already-planned card in the hold column when the operator comments", async () => { + const store = h.store(); + const task = await store.createTask({ description: "planned card needing respec" }); + await store.moveTask(task.id, "todo"); + // A real (non-bootstrap) PROMPT.md is what distinguishes "planned" from "not yet specified". + const { writeFile } = await import("node:fs/promises"); + const { join } = await import("node:path"); + await writeFile( + join(store.taskDir(task.id), "PROMPT.md"), + `# ${task.id}\n\n## Context\nA real planned spec.\n\n## Steps\n1. Do the thing\n`, + "utf-8", + ); + + await store.addTaskComment(task.id, "The approach changed — replan this", "user"); + + const after = await store.getTask(task.id); + expect(after.status, "a planned hold-column card must be re-specified").toBe("needs-replan"); + }); + + it("does NOT re-triage when the comment is not from the user", async () => { + // The role conversion must not widen the gate to non-user authors. + const store = h.store(); + const task = await store.createTask({ description: "agent comment target" }); + await store.moveTask(task.id, "todo"); + await store.updateTask(task.id, { status: "awaiting-approval" }); + + await store.addTaskComment(task.id, "progress note from the agent", "agent"); + + const after = await store.getTask(task.id); + expect(after.status, "an agent comment must not invalidate the operator's approval").toBe("awaiting-approval"); + }); }); diff --git a/packages/core/src/task-store/comments-ops.ts b/packages/core/src/task-store/comments-ops.ts index 87ac628ce4..e50d7d3115 100644 --- a/packages/core/src/task-store/comments-ops.ts +++ b/packages/core/src/task-store/comments-ops.ts @@ -17,6 +17,8 @@ import type {ArchivedTaskDocumentAdditionInput, ArchivedTaskDocumentAdditionResu import {validateDocumentKey} from "../types.js"; import {ArchivedTaskDocumentPublicationRejectedError, validateArchivedTaskDocumentAddition, validateTaskDocumentPreconditions} from "../task-document-concurrency.js"; import "../builtin-traits.js"; +import {resolveWorkflowIrForTask} from "../workflow-ir-resolver.js"; +import {resolveLifecycleColumns} from "../workflow-lifecycle-traits.js"; import {__setTaskActivityLogLimitsForTesting, isBootstrapPromptStub} from "../task-store/comments.js"; import {getLiveTaskColumn, publishArchivedTaskDocumentAddition as publishArchivedTaskDocumentAdditionAsync, upsertTaskDocument as upsertTaskDocumentAsync} from "../task-store/async-comments-attachments.js"; @@ -170,7 +172,37 @@ export async function addCommentImpl(store: TaskStore, id: string, text: string, // Note: The `task` returned above reflects the state BEFORE this // transition. Callers that need the post-transition status should // re-read the task (e.g., via getTask). - if (author === "user" && (task.column === "todo" || task.column === "triage")) { + /* + FNXC:PostCommentRetriage 2026-07-30-00:30 (renamed intake column): + This gate previously listed the two LEGACY ids (`todo`, `triage`). That covers both legacy + vocabularies but NOT a renamed one: `builtin:coding-ideas` places its intake in `ideas`, matching + neither, so an operator comment on an Ideas card awaiting spec approval invalidated nothing — the + approval stood and the task proceeded on the spec being corrected. Identical defect to the one + #2608 fixed for default cards, one workflow over, and it stayed green because no test drove a + non-default workflow through here. + + Resolved from the card's own workflow rather than listing ids. The earlier note here said + narrowing this "needs an IR the caller does not have" — the caller has `store` and `id`, which is + all `resolveWorkflowIrForTask` needs. + + Two deliberate choices: + - The IR is only resolved for a USER comment, so agent/system comment traffic pays nothing. + - An unresolvable workflow falls back to the legacy pair. This is a best-effort re-triage whose + failure mode is a MISSED re-spec, so keeping the old behaviour beats dropping the card out of + the branch. Note `resolveLifecycleColumns` throws on a missing IR, so the null-check is + load-bearing, not defensive decoration. + */ + const retriageColumns = author === "user" + ? await (async () => { + const workflowIr = await resolveWorkflowIrForTask(store, id).catch(() => undefined); + const lifecycle = workflowIr ? resolveLifecycleColumns(workflowIr) : undefined; + return { + intake: lifecycle?.intake ?? "triage", + hold: lifecycle?.hold ?? "todo", + }; + })() + : undefined; + if (retriageColumns && (task.column === retriageColumns.hold || task.column === retriageColumns.intake)) { let hasRealPrompt = false; try { const promptPath = join(store.taskDir(id), "PROMPT.md");