From ab87d0d8038b2fc82fc152bb39f9c1d2f0f80fcb Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 26 Jul 2026 13:15:36 -0700 Subject: [PATCH] fix(api): return 404 for missing tasks, and make task deletions attributable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related fixes, all originating from a `[api:error] Request failed` log line showing a 500 on `GET /api/tasks/FN-8610/runtime-fallback`. 1. Missing/deleted tasks now return 404 instead of 500. `getTaskImpl` signalled a miss with a bare `Error`, and route catches only mapped errno `ENOENT` to 404 — a leftover from the file-backed storage era. In Postgres mode nothing sets an errno code, so every unknown/missing/soft-deleted/wrong-project read returned 500. Adds a typed `TaskNotFoundError` (message byte-identical) plus a shared `task-lookup-error` mapper applied across the task, session-diff, git/GitHub, workflow and file-workspace route registrars. The same bare throw existed on both archive-lifecycle delete paths, so `DELETE /tasks/:id` was affected too. 2. 5xx logs now carry the origin stack. `rethrowAsApiError` constructed a fresh `ApiError` from the message and discarded the original, so the `FNXC:ApiErrorDiagnostics` contract logged the rethrow site rather than the throw site — the reported log entry had no stack at all. Threads `cause` through the error factories and walks the chain (bounded, cycle-guarded). 3. Task deletions are attributable, and non-operator deletes notify. `task:deleted` audit rows recorded `agentId: "system"` for every HTTP delete, making an operator click indistinguishable from a script or an agent; the calling agent's task id was accepted by the store and then never persisted. Adds a `callerKind` union recorded in audit metadata, tags every delete call site, and stamps a self-reported `x-fusion-client` header from the dashboard client. When the caller is `agent-tool` or `api-unattributed`, a best-effort notice is sent to the operator mailbox; operator and engine deletes stay silent. `x-fusion-client` is attribution, not authentication — anything can send it. No delete-blocking, gating or permission logic is added here. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/task-delete-caller-attribution.md | 7 + .changeset/task-delete-operator-notice.md | 7 + .changeset/task-not-found-404.md | 7 + packages/cli/src/commands/task.ts | 3 + packages/cli/src/extension.ts | 8 + .../task-delete-caller-attribution.test.ts | 230 +++++++++++ .../src/__tests__/task-delete-notice.test.ts | 383 ++++++++++++++++++ .../__tests__/task-not-found-error.test.ts | 46 +++ packages/core/src/index.ts | 27 ++ packages/core/src/store.ts | 9 +- packages/core/src/task-delete-attribution.ts | 105 +++++ packages/core/src/task-delete-notice.ts | 207 ++++++++++ .../src/task-store/archive-lifecycle-2.ts | 28 +- .../core/src/task-store/archive-lifecycle.ts | 35 +- packages/core/src/task-store/errors.ts | 42 ++ packages/core/src/task-store/reads.ts | 17 +- .../__tests__/client-identity-header.test.ts | 73 ++++ packages/dashboard/app/api/client.ts | 27 +- .../dashboard/src/__tests__/api-error.test.ts | 174 ++++++++ packages/dashboard/src/api-error.ts | 111 ++++- packages/dashboard/src/file-service.ts | 12 +- ...w-routes.delete-caller-attribution.test.ts | 76 ++++ ...workflow-routes.task-not-found-404.test.ts | 149 +++++++ .../src/routes/register-git-github.ts | 44 +- .../register-planning-subtask-routes.ts | 3 + .../routes/register-session-diff-routes.ts | 10 +- .../routes/register-task-workflow-routes.ts | 142 ++++--- .../src/routes/register-workflow-routes.ts | 4 +- .../dashboard/src/routes/task-lookup-error.ts | 77 ++++ .../engine/src/runtimes/in-process-runtime.ts | 19 + packages/engine/src/self-healing.ts | 2 +- packages/engine/src/triage.ts | 6 +- 32 files changed, 1978 insertions(+), 112 deletions(-) create mode 100644 .changeset/task-delete-caller-attribution.md create mode 100644 .changeset/task-delete-operator-notice.md create mode 100644 .changeset/task-not-found-404.md create mode 100644 packages/core/src/__tests__/task-delete-caller-attribution.test.ts create mode 100644 packages/core/src/__tests__/task-delete-notice.test.ts create mode 100644 packages/core/src/__tests__/task-not-found-error.test.ts create mode 100644 packages/core/src/task-delete-attribution.ts create mode 100644 packages/core/src/task-delete-notice.ts create mode 100644 packages/dashboard/app/api/__tests__/client-identity-header.test.ts create mode 100644 packages/dashboard/src/routes/__tests__/register-task-workflow-routes.delete-caller-attribution.test.ts create mode 100644 packages/dashboard/src/routes/__tests__/register-task-workflow-routes.task-not-found-404.test.ts create mode 100644 packages/dashboard/src/routes/task-lookup-error.ts diff --git a/.changeset/task-delete-caller-attribution.md b/.changeset/task-delete-caller-attribution.md new file mode 100644 index 0000000000..5883ad53a1 --- /dev/null +++ b/.changeset/task-delete-caller-attribution.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Task deletions now record who asked — operator UI, CLI, agent tool, engine, or unattributed API. +category: fix +dev: Adds the `TaskDeleteCallerKind` union plus `callerKind`/`callerTaskId` in `task:deleted` run-audit metadata (both SQLite and PG delete paths). The dashboard client sends a self-reported `x-fusion-client: dashboard-ui` header that the DELETE route maps to `operator-ui`, defaulting to `api-unattributed`. Attribution only — not authentication, and no delete gating was added. diff --git a/.changeset/task-delete-operator-notice.md b/.changeset/task-delete-operator-notice.md new file mode 100644 index 0000000000..c73327ab83 --- /dev/null +++ b/.changeset/task-delete-operator-notice.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: You now get a mailbox notice whenever a task is deleted by someone other than you. +category: feature +dev: Adds `packages/core/src/task-delete-notice.ts` — a store-scoped `registerTaskDeleteNoticeMailbox` DI seam (mirroring the archive-worktree-disposer pattern) that the engine runtime wires to its `MessageStore`. Fires for `callerKind` `agent-tool` and `api-unattributed` only; `operator-ui`, `operator-cli`, and `engine` stay silent. Sent via `sendMessageOnce` keyed `task-delete-notice:`, from all three `task:deleted` emission sites (SQLite `deleteTaskImpl`/`deleteTaskIfImpl`, PG `deleteTaskBackendImpl`), always after the delete transaction commits and always swallowing its own failures so a mailbox write can never fail a delete. Notification only — no delete gating. Prose lives in the mailbox body; run-audit metadata is unchanged. diff --git a/.changeset/task-not-found-404.md b/.changeset/task-not-found-404.md new file mode 100644 index 0000000000..7de83bd339 --- /dev/null +++ b/.changeset/task-not-found-404.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Task API endpoints now return 404 for an unknown task id instead of a 500 error. +category: fix +dev: New typed `TaskNotFoundError` + `isTaskNotFoundError` guard in `@fusion/core` (`task-store/errors.ts`), thrown by both branches of `getTaskImpl` and the delete paths with a byte-identical `Task ${id} not found` message. Dashboard routes map it through the shared `packages/dashboard/src/routes/task-lookup-error.ts` helpers (`isTaskLookupMiss`, `taskLookupStatus`, `rethrowTaskApiError`); the legacy ENOENT check is retained as a fallback. diff --git a/packages/cli/src/commands/task.ts b/packages/cli/src/commands/task.ts index 87aaf5bf4c..5d29c988f5 100644 --- a/packages/cli/src/commands/task.ts +++ b/packages/cli/src/commands/task.ts @@ -1438,8 +1438,11 @@ export async function runTaskDelete(id: string, force?: boolean, allowResurrecti await retryBoardCall(context, id, "delete task", () => context.store.deleteTask(id, { allowResurrection: allowResurrection === true, auditContext: { + // FNXC:TaskDeleteAttribution 2026-07-26-14:30: `fn task delete` prompts a human for + // confirmation at a terminal, so it is an operator surface, not unattributed automation. agentId: "cli", runId: `synthetic-cli-delete-${id}-${Date.now()}`, + callerKind: "operator-cli", }, })); console.log(); diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 4105f99357..a9a88e1637 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -2174,9 +2174,17 @@ export default function kbExtension(pi: ExtensionAPI) { allowResurrection: params.allowResurrection === true, removeLineageReferences: params.removeLineageReferences === true, auditContext: { + /* + FNXC:TaskDeleteAttribution 2026-07-26-14:30: + `agentId` names the TOOL SURFACE, not the actor. Before callerKind/callerTaskId were + persisted, an agent deleting a task through this tool produced a row indistinguishable + from any other pi-extension write and the calling task was lost. `taskId` was already + passed here (the store's self-delete guard reads it) but never reached metadata. + */ agentId: "pi-extension", runId: `synthetic-pi-delete-${params.id}-${Date.now()}`, taskId: callerTaskId, + callerKind: "agent-tool", }, }); diff --git a/packages/core/src/__tests__/task-delete-caller-attribution.test.ts b/packages/core/src/__tests__/task-delete-caller-attribution.test.ts new file mode 100644 index 0000000000..161c99b2b7 --- /dev/null +++ b/packages/core/src/__tests__/task-delete-caller-attribution.test.ts @@ -0,0 +1,230 @@ +// @vitest-environment node + +/* +FNXC:TaskDeleteAttribution 2026-07-26-14:30: +Requirement under test: a `task:deleted` run-audit row must name WHO asked for the delete. + +Original symptom: four tasks were deleted inside one hour and the audit could not say which one the +operator deleted by hand. The rows recorded only the tool surface — `agentId:"pi-extension"` for an +AI agent's `fn_task_delete`, and a hardcoded `agentId:"system"` for every caller of +`DELETE /api/tasks/:id` (operator click and script alike). + +Per AGENTS.md "Fix the Invariant, Not the Repro", this asserts the invariant across every delete +surface rather than the one reported path: agent tool, engine lane, UI-labeled HTTP, unlabeled HTTP, +and the default when no audit context is supplied at all. Both SQLite emission sites are covered +(`deleteTaskImpl` and `deleteTaskIfImpl`); the PG mirror is covered by the shared +`buildDeleteCallerAuditFields` contract asserted below plus the PG suites' own delete coverage. + +Regression anchor: `callerTaskId` — `auditContext.taskId` was accepted by the signature and read by +the self-delete guard, yet never persisted, so the CALLING agent's task was lost forever. +*/ + +import { EventEmitter } from "node:events"; +import { describe, expect, it, vi } from "vitest"; +import { deleteTaskImpl, deleteTaskIfImpl } from "../task-store/archive-lifecycle.js"; +import { + FUSION_CLIENT_HEADER, + FUSION_DASHBOARD_UI_CLIENT, + TASK_DELETE_CALLER_KINDS, + buildDeleteCallerAuditFields, + resolveHttpDeleteCallerKind, + type TaskDeleteAuditContext, +} from "../task-delete-attribution.js"; +import type { Task } from "../types.js"; + +function createTask(id: string): Task { + const now = "2026-07-26T09:00:00.000Z"; + return { + id, + title: id, + description: id, + column: "todo", + dependencies: [], + createdAt: now, + updatedAt: now, + size: "M", + subtasks: [], + log: [], + tags: [], + blockedBy: [], + source: { sourceType: "api" }, + } as unknown as Task; +} + +type AuditRow = { mutationType: string; agentId?: string; metadata?: Record }; + +/** + * FNXC:TaskDeleteAttribution 2026-07-26-14:30: + * In-memory store fake — no DB, no network, no timers (AGENTS.md "Do Not Add Slow Tests"). Mirrors + * the fake in `task-delete-nonblocking-cleanup.test.ts` so the two delete suites share one shape. + */ +function makeDeleteStore(task: Task) { + const events = new EventEmitter(); + const tasks = new Map([[task.id, { ...task, log: [] }]]); + const auditEvents: AuditRow[] = []; + + return { + backendMode: false, + agentLogBuffer: [], + isWatching: false, + taskCache: new Map(), + missionStore: undefined, + db: { + transaction: (fn: () => void) => fn(), + prepare: () => ({ run: () => undefined }), + bumpLastModified: vi.fn(), + }, + withTaskLock: vi.fn(async (_id: string, fn: () => Promise) => fn()), + flushAgentLogBuffer: vi.fn(), + readTaskFromDb: vi.fn((id: string) => tasks.get(id) ?? null), + findLiveDependents: vi.fn(() => [] as string[]), + findLiveLineageChildren: vi.fn(async () => [] as string[]), + cleanupBranchForTask: vi.fn(async () => [] as string[]), + rewriteDependentsForRemoval: vi.fn(() => []), + rewriteBlockedByResidueDependentsForRemoval: vi.fn(() => []), + rewriteLineageChildrenForRemoval: vi.fn(() => []), + recordRunAuditEvent: vi.fn(async (event: AuditRow) => { + auditEvents.push(event); + }), + makeSyntheticDeleteRunId: vi.fn((id: string) => `synthetic-delete-${id}`), + clearLinkedAgentTaskIds: vi.fn(), + clearNearDuplicateReferencesToFailSoft: vi.fn(async () => undefined), + emit: vi.fn((event: string, ...args: unknown[]) => events.emit(event, ...args)), + on: events.on.bind(events), + deletedAuditRow: () => auditEvents.find((event) => event.mutationType === "task:deleted"), + }; +} + +/* +FNXC:TaskDeleteAttribution 2026-07-26-14:30: +Surface enumeration. Each row is a real delete caller in the tree, with the audit context it now +passes and the attribution the persisted row must carry. +*/ +const DELETE_SURFACES: ReadonlyArray<{ + name: string; + auditContext: TaskDeleteAuditContext | undefined; + expectedCallerKind: string; + expectedCallerTaskId: string | null; +}> = [ + { + name: "agent tool call (packages/cli extension fn_task_delete)", + auditContext: { agentId: "pi-extension", runId: "synthetic-pi-delete-1", taskId: "FN-CALLER", callerKind: "agent-tool" }, + expectedCallerKind: "agent-tool", + expectedCallerTaskId: "FN-CALLER", + }, + { + name: "engine lane (triage split-close / duplicate resolution / self-healing)", + auditContext: { agentId: "triage", runId: "triage-delete-1", callerKind: "engine" }, + expectedCallerKind: "engine", + expectedCallerTaskId: null, + }, + { + name: "HTTP DELETE labeled as the dashboard UI", + auditContext: { agentId: "system", runId: "synthetic-dashboard-delete-1", callerKind: "operator-ui" }, + expectedCallerKind: "operator-ui", + expectedCallerTaskId: null, + }, + { + name: "HTTP DELETE with no client label", + auditContext: { agentId: "system", runId: "synthetic-dashboard-delete-2", callerKind: "api-unattributed" }, + expectedCallerKind: "api-unattributed", + expectedCallerTaskId: null, + }, + { + name: "interactive `fn task delete` CLI", + auditContext: { agentId: "cli", runId: "synthetic-cli-delete-1", callerKind: "operator-cli" }, + expectedCallerKind: "operator-cli", + expectedCallerTaskId: null, + }, + { + name: "caller supplying no audit context at all", + auditContext: undefined, + expectedCallerKind: "api-unattributed", + expectedCallerTaskId: null, + }, +]; + +describe("task:deleted caller attribution", () => { + for (const surface of DELETE_SURFACES) { + it(`deleteTask persists callerKind/callerTaskId for ${surface.name}`, async () => { + const task = createTask("FN-TARGET"); + const store = makeDeleteStore(task); + + await deleteTaskImpl(store as never, task.id, { auditContext: surface.auditContext }); + + const row = store.deletedAuditRow(); + expect(row).toBeDefined(); + expect(row?.metadata?.callerKind).toBe(surface.expectedCallerKind); + expect(row?.metadata?.callerTaskId).toBe(surface.expectedCallerTaskId); + }); + + it(`deleteTaskIf persists callerKind/callerTaskId for ${surface.name}`, async () => { + const task = createTask("FN-TARGET-IF"); + const store = makeDeleteStore(task); + + await deleteTaskIfImpl(store as never, task.id, () => true, { auditContext: surface.auditContext }); + + const row = store.deletedAuditRow(); + expect(row).toBeDefined(); + expect(row?.metadata?.callerKind).toBe(surface.expectedCallerKind); + expect(row?.metadata?.callerTaskId).toBe(surface.expectedCallerTaskId); + }); + } + + /* + FNXC:TaskDeleteAttribution 2026-07-26-14:30: + Regression anchor for the exact field that was dropped. `fn_task_delete` already passed + `taskId: callerTaskId` and the self-delete guard already consumed it, but nothing wrote it to + metadata — so an agent-driven delete named only the tool surface. This asserts the calling task + survives into the persisted row even though the row's own `taskId`/`target` is the DELETED task. + */ + it("keeps the calling agent's task id distinct from the deleted task id", async () => { + const task = createTask("FN-8609"); + const store = makeDeleteStore(task); + + await deleteTaskImpl(store as never, task.id, { + auditContext: { agentId: "pi-extension", runId: "synthetic-pi-delete-FN-8609", taskId: "FN-8577", callerKind: "agent-tool" }, + }); + + const row = store.deletedAuditRow() as { taskId?: string; metadata?: Record } | undefined; + expect(row?.taskId).toBe("FN-8609"); + expect(row?.metadata?.callerTaskId).toBe("FN-8577"); + expect(row?.metadata?.callerKind).toBe("agent-tool"); + }); + + /* + FNXC:TaskDeleteAttribution 2026-07-26-14:30: + Run-audit metadata is ids/counts/outcomes-only. Guard that attribution never smuggles prose or a + user-agent string in: the two added fields must be a member of the closed union and a task id. + */ + it("records only enum/id attribution values", () => { + const fields = buildDeleteCallerAuditFields({ + agentId: "pi-extension", + runId: "run-1", + taskId: "FN-CALLER", + callerKind: "agent-tool", + }); + + expect(Object.keys(fields).sort()).toEqual(["callerKind", "callerTaskId"]); + expect(TASK_DELETE_CALLER_KINDS).toContain(fields.callerKind); + }); + + /* + FNXC:TaskDeleteAttribution 2026-07-26-14:30: + Header mapping is fail-closed: only the exact recognized dashboard-UI token becomes `operator-ui`. + A missing, duplicated (array-valued), or spoofed-looking unknown value stays `api-unattributed` so + an unidentified caller is never upgraded into an operator claim. This is attribution, not auth. + */ + it.each([ + [FUSION_DASHBOARD_UI_CLIENT, "operator-ui"], + [" Dashboard-UI ", "operator-ui"], + [undefined, "api-unattributed"], + ["", "api-unattributed"], + ["some-script", "api-unattributed"], + [["dashboard-ui", "dashboard-ui"], "api-unattributed"], + [42, "api-unattributed"], + ])("maps %o to %s", (headerValue, expected) => { + expect(resolveHttpDeleteCallerKind(headerValue)).toBe(expected); + expect(FUSION_CLIENT_HEADER).toBe("x-fusion-client"); + }); +}); diff --git a/packages/core/src/__tests__/task-delete-notice.test.ts b/packages/core/src/__tests__/task-delete-notice.test.ts new file mode 100644 index 0000000000..4ddbdc260f --- /dev/null +++ b/packages/core/src/__tests__/task-delete-notice.test.ts @@ -0,0 +1,383 @@ +// @vitest-environment node + +/* +FNXC:TaskDeleteNotice 2026-07-26-16:10: +Requirement under test: when a task is deleted by an actor that is NOT the operator, a notice lands +in the operator's mailbox. + +Original symptom: tasks disappeared from the board with no signal. Delete attribution +(task-delete-attribution.ts) made the actor discoverable in run-audit AFTER the fact, but nothing +ever told the operator that a delete they did not perform had happened. + +Scope the operator chose, asserted here as a closed decision table rather than as the one reported +case (AGENTS.md "Fix the Invariant, Not the Repro"): notify for `agent-tool` and `api-unattributed`; +stay silent for `operator-ui`, `operator-cli` (the operator did it themselves) and `engine` (triage +split-close fires on every decomposition — confirmed unwanted traffic). + +Surface enumeration — all three `task:deleted` emission sites, so the behavior cannot depend on +backend mode: + 1. `deleteTaskImpl` (SQLite) + 2. `deleteTaskIfImpl` (SQLite, conditional) + 3. `deleteTaskBackendImpl` (PostgreSQL; `deleteTaskIfBackendImpl` delegates here) +plus: no audit context at all, no mailbox registered, and a mailbox that throws. + +Best-effort anchor: a mailbox write that throws must never fail or roll back the delete. That is the +contract most likely to regress silently, so every path asserts it. +*/ + +import { EventEmitter } from "node:events"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../task-store/async-persistence.js", () => ({ + readTaskRow: vi.fn(async () => pgRow), + softDeleteTaskRowInTransaction: vi.fn(async () => undefined), +})); +vi.mock("../task-store/async-lifecycle.js", () => ({ + findLiveLineageChildren: vi.fn(async () => [] as string[]), + projectPartition: vi.fn(() => undefined), + removeLineageReferences: vi.fn(async () => undefined), +})); +vi.mock("../async-mission-store-queries.js", () => ({ + getFeatureByTaskId: vi.fn(async () => null), + unlinkFeatureFromTaskId: vi.fn(async () => undefined), + recordGeneratedFixOperatorStop: vi.fn(async () => undefined), +})); + +import { deleteTaskImpl, deleteTaskIfImpl } from "../task-store/archive-lifecycle.js"; +import { deleteTaskBackendImpl } from "../task-store/archive-lifecycle-2.js"; +import { TASK_DELETE_CALLER_KINDS, type TaskDeleteCallerKind } from "../task-delete-attribution.js"; +import { + NOTIFIED_TASK_DELETE_CALLER_KINDS, + buildTaskDeleteNoticeContent, + buildTaskDeleteNoticeIdempotencyKey, + registerTaskDeleteNoticeMailbox, + shouldNotifyOperatorOfDelete, +} from "../task-delete-notice.js"; +import { DASHBOARD_USER_ID } from "../types.js"; +import type { Task } from "../types.js"; + +/** Mutable row the mocked PG reader returns; each test resets it via `makeTask`. */ +let pgRow: unknown; + +function makeTask(id: string): Task { + const now = "2026-07-26T09:00:00.000Z"; + return { + id, + title: `Title of ${id}`, + description: id, + column: "in-progress", + status: "executing", + dependencies: [], + createdAt: now, + updatedAt: now, + size: "M", + subtasks: [], + log: [], + tags: [], + blockedBy: [], + source: { sourceType: "api" }, + } as unknown as Task; +} + +type SentNotice = { input: { toId: string; toType: string; type: string; content: string; metadata?: Record }; key: string }; + +/** + * FNXC:TaskDeleteNotice 2026-07-26-16:10: + * In-memory mailbox fake. `sendMessageOnce` is the entire seam core depends on (AGENTS.md + * "Do Not Add Slow Tests" — no DB, no timers, no network). + */ +function makeMailbox(options?: { throws?: boolean }) { + const sent: SentNotice[] = []; + return { + sent, + sendMessageOnce: vi.fn(async (input: SentNotice["input"], key: string) => { + if (options?.throws) throw new Error("mailbox is down"); + sent.push({ input, key }); + return { inserted: true }; + }), + }; +} + +/** In-memory SQLite-path TaskStore fake (same shape as task-delete-caller-attribution.test.ts). */ +function makeSqliteStore(task: Task) { + const events = new EventEmitter(); + const tasks = new Map([[task.id, { ...task, log: [] }]]); + return { + backendMode: false, + agentLogBuffer: [], + isWatching: false, + taskCache: new Map(), + missionStore: undefined, + db: { transaction: (fn: () => void) => fn(), prepare: () => ({ run: () => undefined }), bumpLastModified: vi.fn() }, + withTaskLock: vi.fn(async (_id: string, fn: () => Promise) => fn()), + flushAgentLogBuffer: vi.fn(), + readTaskFromDb: vi.fn((id: string) => tasks.get(id) ?? null), + findLiveDependents: vi.fn(() => [] as string[]), + findLiveLineageChildren: vi.fn(async () => [] as string[]), + cleanupBranchForTask: vi.fn(async () => [] as string[]), + rewriteDependentsForRemoval: vi.fn(() => []), + rewriteBlockedByResidueDependentsForRemoval: vi.fn(() => []), + rewriteLineageChildrenForRemoval: vi.fn(() => []), + recordRunAuditEvent: vi.fn(async () => undefined), + makeSyntheticDeleteRunId: vi.fn((id: string) => `synthetic-delete-${id}`), + clearLinkedAgentTaskIds: vi.fn(), + clearNearDuplicateReferencesToFailSoft: vi.fn(async () => undefined), + emit: vi.fn((event: string, ...args: unknown[]) => events.emit(event, ...args)), + on: events.on.bind(events), + }; +} + +/** In-memory PostgreSQL-path TaskStore fake for `deleteTaskBackendImpl`. */ +function makePgStore(task: Task) { + pgRow = task; + return { + backendMode: true, + asyncLayer: { + db: {}, + projectId: "project-1", + transactionImmediate: vi.fn(async (fn: (tx: unknown) => Promise) => fn({})), + }, + rowToTask: vi.fn((row: unknown) => row as Task), + pgRowToTaskRow: vi.fn((row: unknown) => row), + recordRunAuditEventBackend: vi.fn(async () => undefined), + makeSyntheticDeleteRunId: vi.fn((id: string) => `synthetic-delete-${id}`), + emit: vi.fn(), + }; +} + +/* +FNXC:TaskDeleteNotice 2026-07-26-16:10: +The three real delete entry points, driven through one table so a new emission site cannot be added +with notice coverage on only one backend. +*/ +const DELETE_PATHS: ReadonlyArray<{ + name: string; + run: (task: Task, auditContext: unknown) => Promise<{ store: object }>; +}> = [ + { + name: "deleteTaskImpl (SQLite)", + run: async (task, auditContext) => { + const store = makeSqliteStore(task); + registerMailboxFor(store); + await deleteTaskImpl(store as never, task.id, { auditContext: auditContext as never }); + return { store }; + }, + }, + { + name: "deleteTaskIfImpl (SQLite, conditional)", + run: async (task, auditContext) => { + const store = makeSqliteStore(task); + registerMailboxFor(store); + await deleteTaskIfImpl(store as never, task.id, () => true, { auditContext: auditContext as never }); + return { store }; + }, + }, + { + name: "deleteTaskBackendImpl (PostgreSQL)", + run: async (task, auditContext) => { + const store = makePgStore(task); + registerMailboxFor(store); + await deleteTaskBackendImpl(store as never, task.id, { auditContext: auditContext as never }); + return { store }; + }, + }, +]; + +/* +FNXC:TaskDeleteNotice 2026-07-26-16:10: +The operator's decision, hardcoded. This deliberately does NOT derive from +`NOTIFIED_TASK_DELETE_CALLER_KINDS` — deriving the expectation from the value under test makes the +suite agree with whatever the production constant happens to say, which is exactly how a widened +notify condition would slip through unnoticed. +*/ +const EXPECTED_NOTIFY: Record = { + "operator-ui": false, + "operator-cli": false, + "agent-tool": true, + engine: false, + "api-unattributed": true, +}; + +/** Current mailbox under test, installed onto whichever store fake the path builds. */ +let mailbox: ReturnType; +function registerMailboxFor(store: object): void { + registerTaskDeleteNoticeMailbox(store as never, mailbox); +} + +beforeEach(() => { + mailbox = makeMailbox(); +}); + +describe("operator mailbox notice for non-operator deletes", () => { + for (const path of DELETE_PATHS) { + for (const callerKind of TASK_DELETE_CALLER_KINDS) { + const expected = EXPECTED_NOTIFY[callerKind]; + it(`${path.name}: ${expected ? "notifies" : "stays silent"} for ${callerKind}`, async () => { + const task = makeTask("FN-9001"); + await path.run(task, { agentId: "a", runId: "r", callerKind }); + expect(mailbox.sendMessageOnce).toHaveBeenCalledTimes(expected ? 1 : 0); + }); + } + + /* No audit context at all resolves to `api-unattributed` (the attribution default), so it notifies. */ + it(`${path.name}: notifies when the caller supplied no audit context`, async () => { + const task = makeTask("FN-9002"); + await path.run(task, undefined); + expect(mailbox.sendMessageOnce).toHaveBeenCalledTimes(1); + expect(mailbox.sent[0]?.input.metadata?.callerKind).toBe("api-unattributed"); + }); + + /* + FNXC:TaskDeleteNotice 2026-07-26-16:10: + THE best-effort contract. The delete is primary; the notice is secondary. A throwing mailbox must + leave the delete completed and must not reject. Without this, one bad mailbox write turns every + agent-driven delete into a failure. + */ + it(`${path.name}: completes the delete when the mailbox write throws`, async () => { + mailbox = makeMailbox({ throws: true }); + const task = makeTask("FN-9003"); + await expect( + path.run(task, { agentId: "a", runId: "r", callerKind: "agent-tool" }), + ).resolves.toBeDefined(); + expect(mailbox.sendMessageOnce).toHaveBeenCalledTimes(1); + }); + + /* An unregistered mailbox (engine runtime not running) degrades to no notice, never to a failure. */ + it(`${path.name}: completes the delete when no mailbox is registered`, async () => { + const task = makeTask("FN-9004"); + const noop = { sendMessageOnce: vi.fn() }; + const store = path.name.startsWith("deleteTaskBackendImpl") ? makePgStore(task) : makeSqliteStore(task); + // deliberately register against a DIFFERENT store object: this store has no mailbox. + registerTaskDeleteNoticeMailbox({} as never, noop); + const options = { auditContext: { agentId: "a", runId: "r", callerKind: "agent-tool" } } as never; + if (path.name.startsWith("deleteTaskBackendImpl")) { + await expect(deleteTaskBackendImpl(store as never, task.id, options)).resolves.toBeDefined(); + } else if (path.name.startsWith("deleteTaskIfImpl")) { + await expect(deleteTaskIfImpl(store as never, task.id, () => true, options)).resolves.toBeDefined(); + } else { + await expect(deleteTaskImpl(store as never, task.id, options)).resolves.toBeDefined(); + } + expect(noop.sendMessageOnce).not.toHaveBeenCalled(); + }); + + it(`${path.name}: addresses the operator mailbox and names the deleted task`, async () => { + const task = makeTask("FN-9005"); + await path.run(task, { agentId: "pi-extension", runId: "r", taskId: "FN-CALLER", callerKind: "agent-tool" }); + + const notice = mailbox.sent[0]; + expect(notice).toBeDefined(); + expect(notice?.input.toId).toBe(DASHBOARD_USER_ID); + expect(notice?.input.toType).toBe("user"); + expect(notice?.input.type).toBe("system"); + expect(notice?.key).toBe(buildTaskDeleteNoticeIdempotencyKey("FN-9005")); + expect(notice?.input.content).toContain("FN-9005"); + expect(notice?.input.content).toContain("Title of FN-9005"); + expect(notice?.input.content).toContain("FN-CALLER"); + expect(notice?.input.content).toContain("agent-tool"); + // Previous column/status are the PRE-delete values, not `archived`. + expect(notice?.input.content).toContain("in-progress"); + expect(notice?.input.content).toContain("executing"); + expect(notice?.input.content).not.toContain("archived"); + }); + } + + /* + FNXC:TaskDeleteNotice 2026-07-26-16:10: + Closed-set guard. Adding a caller kind to TASK_DELETE_CALLER_KINDS without deciding whether the + operator wants to hear about it would otherwise silently inherit "notify" or "silent" by accident. + */ + it("has an explicit notify decision for every caller kind", () => { + expect(Object.keys(EXPECTED_NOTIFY).sort()).toEqual([...TASK_DELETE_CALLER_KINDS].sort()); + for (const kind of TASK_DELETE_CALLER_KINDS) { + expect(shouldNotifyOperatorOfDelete(kind)).toBe(EXPECTED_NOTIFY[kind]); + } + expect(TASK_DELETE_CALLER_KINDS.filter((kind) => shouldNotifyOperatorOfDelete(kind))).toEqual([ + "agent-tool", + "api-unattributed", + ]); + }); + + /* + FNXC:TaskDeleteNotice 2026-07-26-16:10: + Honesty requirement. `api-unattributed` means "nothing identified itself" — the `x-fusion-client` + header is self-reported. The prose must not present an unidentified caller as an agent. + */ + it("does not imply an agent when the caller merely failed to identify itself", () => { + const content = buildTaskDeleteNoticeContent( + { id: "FN-9006", title: "Unattributed target", previousColumn: "todo", previousStatus: null }, + "api-unattributed", + null, + ); + expect(content).toContain("did not identify itself"); + expect(content).toContain("self-reported"); + expect(content).not.toContain("AI agent"); + expect(content).not.toContain("fn_task_delete"); + }); + + it("names the agent tool and the calling task for an agent-driven delete", () => { + const content = buildTaskDeleteNoticeContent( + { id: "FN-9007", title: "Agent target", previousColumn: "todo", previousStatus: null }, + "agent-tool", + "FN-CALLER", + ); + expect(content).toContain("AI agent"); + expect(content).toContain("fn_task_delete"); + expect(content).toContain("FN-CALLER"); + expect(content).not.toContain("did not identify itself"); + }); + + /* + FNXC:TaskDeleteNotice 2026-07-26-16:10: + Prose belongs in the mailbox body only. The message metadata (which mirrors run-audit's + ids/counts/outcomes-only discipline) must carry ids and enums, never the sentence. + */ + it("keeps prose out of the notice metadata", async () => { + const task = makeTask("FN-9008"); + const store = makeSqliteStore(task); + registerMailboxFor(store); + await deleteTaskImpl(store as never, task.id, { + auditContext: { agentId: "pi-extension", runId: "r", taskId: "FN-CALLER", callerKind: "agent-tool" }, + } as never); + + const notice = mailbox.sent[0]; + expect(Object.keys(notice?.input.metadata ?? {}).sort()).toEqual([ + "callerKind", + "callerTaskId", + "kind", + "previousColumn", + "previousStatus", + "taskId", + ]); + for (const value of Object.values(notice?.input.metadata ?? {})) { + expect(String(value)).not.toContain(" "); + } + }); + + /* An idempotent re-delete of an already soft-deleted task must not re-notify. */ + it("stays silent when the delete short-circuits on an already-deleted task", async () => { + const task = { ...makeTask("FN-9009"), deletedAt: "2026-07-26T10:00:00.000Z" } as Task; + const store = makeSqliteStore(task); + registerMailboxFor(store); + await deleteTaskImpl(store as never, task.id, { + auditContext: { agentId: "a", runId: "r", callerKind: "agent-tool" }, + } as never); + expect(mailbox.sendMessageOnce).not.toHaveBeenCalled(); + }); + + /* A declined conditional delete deleted nothing, so it must not claim a deletion happened. */ + it("stays silent when deleteTaskIf's predicate declines the delete", async () => { + const task = makeTask("FN-9010"); + const store = makeSqliteStore(task); + registerMailboxFor(store); + await deleteTaskIfImpl(store as never, task.id, () => false, { + auditContext: { agentId: "a", runId: "r", callerKind: "agent-tool" }, + } as never); + expect(mailbox.sendMessageOnce).not.toHaveBeenCalled(); + }); + + it("resolves the notified set from the shared caller-kind union", () => { + for (const kind of NOTIFIED_TASK_DELETE_CALLER_KINDS) { + expect(TASK_DELETE_CALLER_KINDS).toContain(kind as TaskDeleteCallerKind); + } + }); +}); diff --git a/packages/core/src/__tests__/task-not-found-error.test.ts b/packages/core/src/__tests__/task-not-found-error.test.ts new file mode 100644 index 0000000000..3b10ff7920 --- /dev/null +++ b/packages/core/src/__tests__/task-not-found-error.test.ts @@ -0,0 +1,46 @@ +/* +FNXC:TaskLookup404 2026-07-26-12:40: +Guards the two contracts the dashboard 404 mapping depends on: +1. `TaskNotFoundError.message` stays byte-identical to the legacy + `Task ${id} not found` string, so message-matching callers/tests still work + after the switch from a bare `Error`. +2. `isTaskNotFoundError` is structural, so the guard survives a duplicated + `@fusion/core` module instance (bundled CLI vs workspace dist) and errors that + crossed a serialization boundary — an `instanceof`-only check would silently + regress those callers back to 500. +*/ +import { describe, it, expect } from "vitest"; +import { TaskNotFoundError, isTaskNotFoundError } from "../task-store/errors.js"; + +describe("TaskNotFoundError", () => { + it("keeps the legacy `Task not found` message byte-identical", () => { + expect(new TaskNotFoundError("FN-8610").message).toBe("Task FN-8610 not found"); + }); + + it("carries the task id and a stable code/name", () => { + const err = new TaskNotFoundError("FN-8610"); + expect(err.taskId).toBe("FN-8610"); + expect(err.name).toBe("TaskNotFoundError"); + expect(err.code).toBe("TASK_NOT_FOUND"); + }); + + it("recognizes its own instances", () => { + expect(isTaskNotFoundError(new TaskNotFoundError("FN-1"))).toBe(true); + }); + + it("recognizes a structurally equivalent error from a duplicate module instance", () => { + const cloned = Object.assign(new Error("Task FN-1 not found"), { + name: "TaskNotFoundError", + code: "TASK_NOT_FOUND", + taskId: "FN-1", + }); + expect(isTaskNotFoundError(cloned)).toBe(true); + }); + + it("does not match unrelated errors, including same-message bare Errors from other layers", () => { + expect(isTaskNotFoundError(new Error("Task FN-1 not found"))).toBe(false); + expect(isTaskNotFoundError(new Error("connection terminated unexpectedly"))).toBe(false); + expect(isTaskNotFoundError(undefined)).toBe(false); + expect(isTaskNotFoundError("Task FN-1 not found")).toBe(false); + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 04bede7f9e..864600d3e1 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -784,6 +784,10 @@ export { SelfDefeatingDependencyError, DependencyCycleError, TaskDeletedError, + // FNXC:TaskLookup404 2026-07-26-11:20: typed task-miss signal + guard so API + // boundaries can return 404 instead of 500 for an unknown task id. + TaskNotFoundError, + isTaskNotFoundError, TombstonedTaskResurrectionError, MergeQueueTaskNotFoundError, MergeQueueInvalidColumnError, @@ -2648,3 +2652,26 @@ export { getTotalAgentActiveMs, startPlanningSegment, finalizePlanningSegment } export { createLogger, type Logger } from "./logger.js"; export { ACTIVE_WORKFLOW_WORK_ITEM_STATES } from "./types.js"; export * from "./task-document-concurrency.js"; +/* +FNXC:TaskDeleteAttribution 2026-07-26-14:30: +Delete-caller attribution vocabulary. Exported from core because three packages must agree on it: +the CLI/pi extension tags `agent-tool`, the engine tags `engine`, and the dashboard's browser client +and Express route share the `x-fusion-client` header spelling across the wire. +*/ +export * from "./task-delete-attribution.js"; +/* +FNXC:TaskDeleteNotice 2026-07-26-16:10: +Operator mailbox notice for non-operator deletes. Exported because the mailbox lives outside core: +the engine runtime owns the MessageStore and registers it against its TaskStore through this seam. +*/ +export { + NOTIFIED_TASK_DELETE_CALLER_KINDS, + shouldNotifyOperatorOfDelete, + registerTaskDeleteNoticeMailbox, + getTaskDeleteNoticeMailbox, + buildTaskDeleteNoticeContent, + buildTaskDeleteNoticeIdempotencyKey, + notifyOperatorOfNonOperatorDelete, + type TaskDeleteNoticeMailbox, + type TaskDeleteNoticeSnapshot, +} from "./task-delete-notice.js"; diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 675e7cf681..969678706d 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -129,6 +129,7 @@ import { initImpl, setupActivityLogListenersImpl, reconcileOrphanedTaskDirsImpl, import { updateStepImpl, startStepImpl, acquireMergeQueueLeaseImpl, mergeTaskImpl } from "./task-store/merge-queue-ops.js"; import { addCommentImpl, publishArchivedTaskDocumentAdditionImpl, upsertTaskDocumentImpl } from "./task-store/comments-ops.js"; import { deleteTaskImpl, deleteTaskIfImpl, archiveTaskImpl, type DeleteTaskIfResult } from "./task-store/archive-lifecycle.js"; +import type { TaskDeleteAuditContext } from "./task-delete-attribution.js"; import { updateSettingsImpl, updateGlobalSettingsImpl } from "./task-store/settings-ops.js"; import { createTaskBackendImpl, _createTaskInternalBackendImpl, createTaskImpl, createTaskWithReservedIdImpl, _createTaskInternalImpl, _maybeAutoArchiveSameAgentDuplicateImpl } from "./task-store/task-creation.js"; import { getTaskImpl, listTasksImpl, searchTasksImpl, listTasksModifiedSinceImpl, getTaskVerificationRequestAsyncImpl } from "./task-store/reads.js"; @@ -194,6 +195,8 @@ export { TaskHasDependentsError, TaskSelfDeleteError, TaskDeletedError, + TaskNotFoundError, + isTaskNotFoundError, TombstonedTaskResurrectionError, TaskHasLineageChildrenError, InvalidFileScopeError, @@ -1955,7 +1958,7 @@ export class TaskStore extends EventEmitter { /** * FNXC:RuntimeLifecycleAsync 2026-06-24-12:05: */ - public async deleteTaskBackend( id: string, options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: { agentId: string; runId: string; sessionId?: string; taskId?: string }; }, ): Promise { + public async deleteTaskBackend( id: string, options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: TaskDeleteAuditContext; }, ): Promise { return deleteTaskBackendImpl(this, id, options); } @@ -1966,13 +1969,13 @@ export class TaskStore extends EventEmitter { */ public async recordRunAuditEventBackend( tx: DbTransaction, event: { domain: string; mutationType: string; target: string; taskId: string; agentId: string; runId: string; metadata: Record; }, ): Promise { return recordRunAuditEventBackendImpl(this, tx, event); } - async deleteTask( id: string, options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: { agentId: string; runId: string; sessionId?: string; taskId?: string }; }, ): Promise { + async deleteTask( id: string, options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: TaskDeleteAuditContext; }, ): Promise { return deleteTaskImpl(this, id, options); } async deleteTaskIf( id: string, predicate: (live: Task) => boolean | Promise, - options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: { agentId: string; runId: string; sessionId?: string; taskId?: string } }, + options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: TaskDeleteAuditContext }, ): Promise { if (this.backendMode) return deleteTaskIfBackendImpl(this, id, predicate, options); return deleteTaskIfImpl(this, id, predicate, options); diff --git a/packages/core/src/task-delete-attribution.ts b/packages/core/src/task-delete-attribution.ts new file mode 100644 index 0000000000..effbaff521 --- /dev/null +++ b/packages/core/src/task-delete-attribution.ts @@ -0,0 +1,105 @@ +/* +FNXC:TaskDeleteAttribution 2026-07-26-14:30: +Four tasks were deleted inside one hour in a live project and the run-audit rows could not answer +"which one did the human delete?". Two concrete holes produced that: (1) `task:deleted` metadata +persisted `auditContext.sessionId` but never `auditContext.taskId`, so an AI agent's `fn_task_delete` +call recorded only the tool surface (`agentId:"pi-extension"`) and the CALLING task/agent was lost +forever; (2) the dashboard `DELETE /api/tasks/:id` handler hardcoded `agentId:"system"`, so an +operator clicking Delete in the UI and a script hitting the same endpoint produced byte-identical +rows. This module is the single closed vocabulary that fixes both: `callerKind` names the class of +actor and `callerTaskId` names the calling task. + +TRUST MODEL — read before extending. `callerKind` is ATTRIBUTION, NOT AUTHENTICATION. The HTTP +variants are derived from a self-reported `x-fusion-client` request header, so all the row can +honestly claim is "the client identified itself as the dashboard UI" (`operator-ui`) versus +"nothing identified itself" (`api-unattributed`). Anything can send the header. Never gate a +permission, a delete, or any other decision on this value, and never describe it as proof of a +human. It exists so forensics has a starting hypothesis, not a verdict. + +Scope note: this is observability only. No delete-blocking, gating, or permission logic is added or +implied here; whether agents should hold `fn_task_delete` at all is a separate operator policy call. +*/ + +/** + * FNXC:TaskDeleteAttribution 2026-07-26-14:30: + * Closed set of delete-caller classes, ordered from most to least attributable. + * + * - `operator-ui` — an HTTP client that identified itself as the dashboard UI (see trust model). + * - `operator-cli` — the interactive `fn task delete` command, which prompts a human for + * confirmation at a terminal. Added beyond the original four because that surface is a real, + * distinct human actor already tagged `agentId:"cli"`; without its own member it would have to + * masquerade as `api-unattributed` and re-blur exactly the operator-vs-automation line this + * change exists to draw. + * - `agent-tool` — an AI agent's tool call (`fn_task_delete`). Pair with `callerTaskId`. + * - `engine` — an autonomous engine lane (triage split-close, duplicate resolution, self-healing). + * - `api-unattributed` — an HTTP caller that sent no recognized client header. The deliberate + * default: unknown is recorded as unknown rather than guessed as operator. + */ +export const TASK_DELETE_CALLER_KINDS = [ + "operator-ui", + "operator-cli", + "agent-tool", + "engine", + "api-unattributed", +] as const; + +export type TaskDeleteCallerKind = (typeof TASK_DELETE_CALLER_KINDS)[number]; + +/** + * FNXC:TaskDeleteAttribution 2026-07-26-14:30: + * Shared shape for every delete audit context. Previously this object literal was retyped inline at + * ten call signatures across `store.ts` and both archive-lifecycle branches, which is how `taskId` + * could exist on the type, be passed by `fn_task_delete`, be consumed by the self-delete guard, and + * still never reach persisted metadata without anything flagging it. + * + * `taskId` is the CALLER's task (the task whose agent is asking for the delete), not the target. + * It doubles as the self-delete guard input (`auditContext.taskId === id` is refused) and, from + * this change on, is persisted as `callerTaskId`. + */ +export interface TaskDeleteAuditContext { + agentId: string; + runId: string; + sessionId?: string; + taskId?: string; + callerKind?: TaskDeleteCallerKind; +} + +/** + * FNXC:TaskDeleteAttribution 2026-07-26-14:30: + * Request header a Fusion first-party HTTP client sets to identify itself. Self-reported; see the + * module trust-model note. Kept in core so the dashboard's browser client and the dashboard's + * Express route cannot drift on the spelling. + */ +export const FUSION_CLIENT_HEADER = "x-fusion-client"; + +/** Value the dashboard web/desktop UI sends in {@link FUSION_CLIENT_HEADER}. */ +export const FUSION_DASHBOARD_UI_CLIENT = "dashboard-ui"; + +/** + * FNXC:TaskDeleteAttribution 2026-07-26-14:30: + * Map a raw `x-fusion-client` header to a caller kind. Only the exact recognized dashboard-UI token + * maps to `operator-ui`; absent, array-valued (duplicate header), or unrecognized values all fall + * back to `api-unattributed` so an unknown caller is never upgraded into an operator claim. + */ +export function resolveHttpDeleteCallerKind(headerValue: unknown): TaskDeleteCallerKind { + if (typeof headerValue !== "string") return "api-unattributed"; + return headerValue.trim().toLowerCase() === FUSION_DASHBOARD_UI_CLIENT + ? "operator-ui" + : "api-unattributed"; +} + +/** + * FNXC:TaskDeleteAttribution 2026-07-26-14:30: + * Persisted metadata fragment for a `task:deleted` run-audit row. Returns enum/id values only — + * never prose, never a user-agent string — per the run-audit ids/counts/outcomes-only rule. + * `callerKind` defaults to `api-unattributed` so every row from here on carries a value and an old + * row's missing field is distinguishable from a new row's unknown caller. + */ +export function buildDeleteCallerAuditFields( + auditContext: TaskDeleteAuditContext | undefined, +): { callerKind: TaskDeleteCallerKind; callerTaskId: string | null } { + return { + callerKind: auditContext?.callerKind ?? "api-unattributed", + callerTaskId: auditContext?.taskId ?? null, + }; +} diff --git a/packages/core/src/task-delete-notice.ts b/packages/core/src/task-delete-notice.ts new file mode 100644 index 0000000000..fe6179f292 --- /dev/null +++ b/packages/core/src/task-delete-notice.ts @@ -0,0 +1,207 @@ +/* +FNXC:TaskDeleteNotice 2026-07-26-16:10: +Delete attribution (task-delete-attribution.ts) answered "who deleted this?" only for someone who +later went digging through run-audit rows. The operator's actual complaint was that tasks vanished +from the board with no signal at all. This module closes that loop: when a task is deleted by an +actor that is NOT the operator, a durable notice lands in the operator's mailbox. + +SCOPE — the operator chose this deliberately, do not widen it without asking: + - NOTIFY for `agent-tool` (an AI agent called `fn_task_delete`) and `api-unattributed` (an HTTP + caller that identified itself as nothing). + - DO NOT notify for `operator-ui` / `operator-cli` — the operator performed the delete themselves + and does not need to be told about their own click. + - DO NOT notify for `engine` — triage split-close deletes the parent on every decomposition, so + engine deletes are high-volume routine traffic. The operator confirmed that behavior is fine + and explicitly does not want the mailbox flooded with it. + +HONESTY — `callerKind` is attribution, not authentication (see task-delete-attribution.ts's trust +model). `api-unattributed` means "nothing identified itself", NOT "an automation did it": the +`x-fusion-client` header is self-reported, so a stale browser tab, a curl, or a script all land in +the same bucket. The notice prose must say that plainly and must never imply an agent was involved +when the caller was merely unidentified. + +BEST-EFFORT — the delete is the primary operation and the notice is strictly secondary. Every entry +point here swallows its own failures; a mailbox write must never surface as a failed delete, and it +must never run inside the delete transaction (a mailbox INSERT that throws would otherwise roll back +a committed soft-delete). Callers fire it AFTER the transaction commits. + +PROSE PLACEMENT — the message body is operator-facing prose and lives in the MAILBOX only. The +`task:deleted` run-audit row still carries ids/counts/outcomes only; nothing here writes to it. + +This module is observability/notification only. It adds no delete-blocking, gating, or permission +logic — an unattributed delete still succeeds, the operator just finds out about it. +*/ + +import { createLogger } from "./logger.js"; +import type { TaskStore } from "./store.js"; +import { + buildDeleteCallerAuditFields, + type TaskDeleteAuditContext, + type TaskDeleteCallerKind, +} from "./task-delete-attribution.js"; +import { DASHBOARD_USER_ID, type MessageCreateInput } from "./types.js"; + +const noticeLog = createLogger("task-delete-notice"); + +/** + * FNXC:TaskDeleteNotice 2026-07-26-16:10: + * The caller classes the operator wants to hear about. Kept as an exported constant (rather than + * inlined in the predicate) so a test can assert the closed set against + * `TASK_DELETE_CALLER_KINDS` and fail loudly if a new caller kind is added without a notify + * decision being made for it. + */ +export const NOTIFIED_TASK_DELETE_CALLER_KINDS: readonly TaskDeleteCallerKind[] = [ + "agent-tool", + "api-unattributed", +]; + +/** True when a delete by `callerKind` warrants an operator mailbox notice. */ +export function shouldNotifyOperatorOfDelete(callerKind: TaskDeleteCallerKind): boolean { + return NOTIFIED_TASK_DELETE_CALLER_KINDS.includes(callerKind); +} + +/** + * FNXC:TaskDeleteNotice 2026-07-26-16:10: + * Minimal mailbox seam. Core cannot import the engine and does not own a MessageStore, so the + * dependency is narrowed to the single method actually used. `MessageStore` structurally satisfies + * this, and a test fake is two lines. + */ +export interface TaskDeleteNoticeMailbox { + sendMessageOnce(input: MessageCreateInput, idempotencyKey: string): Promise; +} + +/* +FNXC:TaskDeleteNotice 2026-07-26-16:10: +Store-scoped registration, mirroring archive-worktree-disposer.ts. A process can host several +projects; a process-global mailbox would post one project's delete into another project's inbox. +The unregister closure is identity-guarded so a torn-down runtime cannot erase a newer one's +registration. An unregistered store degrades silently to "no notice" — losing a notice is +acceptable, losing a delete is not. +*/ +const mailboxes = new WeakMap(); + +export function registerTaskDeleteNoticeMailbox( + store: TaskStore, + mailbox: TaskDeleteNoticeMailbox, +): () => void { + mailboxes.set(store, mailbox); + return () => { + if (mailboxes.get(store) === mailbox) mailboxes.delete(store); + }; +} + +export function getTaskDeleteNoticeMailbox(store: TaskStore): TaskDeleteNoticeMailbox | undefined { + return mailboxes.get(store); +} + +/** Pre-delete snapshot the notice describes. Captured before the row is mutated to `archived`. */ +export interface TaskDeleteNoticeSnapshot { + id: string; + title?: string; + /** The column the task sat in BEFORE the delete moved it to `archived`. */ + previousColumn?: string; + previousStatus?: string | null; +} + +/** + * FNXC:TaskDeleteNotice 2026-07-26-16:10: + * Per-kind actor sentence. `api-unattributed` deliberately does NOT name an agent: the only true + * statement about that caller is that nothing identified itself. + */ +function describeActor(callerKind: TaskDeleteCallerKind, callerTaskId: string | null): string { + if (callerKind === "agent-tool") { + return callerTaskId + ? `An AI agent deleted it with the \`fn_task_delete\` tool while working on ${callerTaskId}.` + : "An AI agent deleted it with the `fn_task_delete` tool. The calling task was not recorded."; + } + return [ + "The caller did not identify itself, so Fusion recorded it as `api-unattributed`.", + "That is not evidence an automation did this: the `x-fusion-client` header is self-reported,", + "so an unidentified caller can equally be a script, a stale browser tab, or a direct API call.", + "All that is known is that nothing claimed responsibility.", + ].join(" "); +} + +/** Build the operator-facing mailbox body. Prose lives here and in the mailbox row only. */ +export function buildTaskDeleteNoticeContent( + task: TaskDeleteNoticeSnapshot, + callerKind: TaskDeleteCallerKind, + callerTaskId: string | null, +): string { + const title = task.title?.trim(); + const heading = title ? `**${task.id} — ${title}** was deleted` : `**${task.id}** was deleted`; + const where = task.previousColumn + ? `It was in the \`${task.previousColumn}\` column${task.previousStatus ? ` (status \`${task.previousStatus}\`)` : ""} before the delete.` + : "Its column before the delete was not recorded."; + return [ + `${heading} — not by you.`, + "", + describeActor(callerKind, callerTaskId), + where, + "", + `Caller class: \`${callerKind}\`${callerTaskId ? `, calling task: \`${callerTaskId}\`` : ""}.`, + "The task is soft-deleted, so it can still be inspected or restored from the archive.", + ].join("\n"); +} + +/* +FNXC:TaskDeleteNotice 2026-07-26-16:10: +`sendMessageOnce` rather than `sendMessage`, chosen for one concrete duplicate window: the PG +`deleteTaskBackendImpl` is reachable WITHOUT the per-task lock (only `deleteTaskIf` wraps it), so two +concurrent deletes of the same id can both pass the `task.deletedAt` short-circuit and both commit. +A plain send would post the same disappearance twice. The deterministic +`task-delete-notice:` key lets the DB's conflict handling arbitrate instead, and a task id is +deleted at most once so the key can never collapse two genuinely distinct events. +*/ +export function buildTaskDeleteNoticeIdempotencyKey(taskId: string): string { + return `task-delete-notice:${taskId}`; +} + +/** + * FNXC:TaskDeleteNotice 2026-07-26-16:10: + * Best-effort operator notice for a non-operator delete. NEVER throws and NEVER rejects — the + * returned promise resolves to whether a notice was actually written, purely so tests can assert + * the invariant. Call this only after the delete transaction has committed. + */ +export async function notifyOperatorOfNonOperatorDelete( + store: TaskStore, + task: TaskDeleteNoticeSnapshot, + auditContext: TaskDeleteAuditContext | undefined, +): Promise { + try { + // Resolve through the same helper the audit row uses so the notice can never claim a different + // caller class than the persisted `task:deleted` metadata. + const { callerKind, callerTaskId } = buildDeleteCallerAuditFields(auditContext); + if (!shouldNotifyOperatorOfDelete(callerKind)) return false; + const mailbox = mailboxes.get(store); + if (!mailbox) return false; + await mailbox.sendMessageOnce( + { + fromId: "system", + fromType: "system", + toId: DASHBOARD_USER_ID, + toType: "user", + type: "system", + content: buildTaskDeleteNoticeContent(task, callerKind, callerTaskId), + // Ids/enums only here too — the prose is the `content` field, not metadata. + metadata: { + kind: "task-delete-notice", + taskId: task.id, + callerKind, + ...(callerTaskId ? { callerTaskId } : {}), + ...(task.previousColumn ? { previousColumn: task.previousColumn } : {}), + ...(task.previousStatus ? { previousStatus: task.previousStatus } : {}), + }, + }, + buildTaskDeleteNoticeIdempotencyKey(task.id), + ); + return true; + } catch (error) { + // Swallowed on purpose: the delete already committed. Surfacing this would turn a successful + // delete into a 500 and (worse) invite a retry against an already-deleted task. + noticeLog.warn( + `Operator delete notice failed for ${task.id} (delete already committed): ${error instanceof Error ? error.message : String(error)}`, + ); + return false; + } +} diff --git a/packages/core/src/task-store/archive-lifecycle-2.ts b/packages/core/src/task-store/archive-lifecycle-2.ts index 66d7db3472..d09acaceee 100644 --- a/packages/core/src/task-store/archive-lifecycle-2.ts +++ b/packages/core/src/task-store/archive-lifecycle-2.ts @@ -8,12 +8,14 @@ */ import {TaskStore, storeLog} from "../store.js"; import {getFeatureByTaskId as getMissionFeatureByTaskId, unlinkFeatureFromTaskId as unlinkMissionFeatureFromTaskId, recordGeneratedFixOperatorStop} from "../async-mission-store-queries.js"; -import {TaskHasLineageChildrenError, TaskSelfDeleteError} from "./errors.js"; +import {TaskHasLineageChildrenError, TaskNotFoundError, TaskSelfDeleteError} from "./errors.js"; import {mkdir, writeFile} from "node:fs/promises"; import {join} from "node:path"; import {and, eq} from "drizzle-orm"; import * as schema from "../postgres/schema/index.js"; import type {Task, Column, ArchivedTaskEntry, GithubIssueAction} from "../types.js"; +import {buildDeleteCallerAuditFields, type TaskDeleteAuditContext} from "../task-delete-attribution.js"; +import {notifyOperatorOfNonOperatorDelete} from "../task-delete-notice.js"; import "../builtin-traits.js"; import {normalizeTaskPriority} from "../task-priority.js"; import {generateTaskLineageId} from "../task-lineage.js"; @@ -103,7 +105,7 @@ export async function taskToArchiveEntryImpl(store: TaskStore, task: Task, archi }; } -export async function deleteTaskBackendImpl(store: TaskStore, id: string, options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: { agentId: string; runId: string; sessionId?: string; taskId?: string }; },): Promise { +export async function deleteTaskBackendImpl(store: TaskStore, id: string, options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: TaskDeleteAuditContext; },): Promise { /* FNXC:TaskDeletion 2026-07-01-00:00: Task-bound runtime callers may never soft-delete the task they are executing; this guard is the PostgreSQL-backend mirror of the SQLite-path guard in deleteTaskImpl so direct callers of deleteTaskBackend inherit the same invariant before any mutation or audit. @@ -115,7 +117,9 @@ export async function deleteTaskBackendImpl(store: TaskStore, id: string, option // Read the task row (forensic: include soft-deleted). const pgRow = await readTaskRowAsync(layer, id, { includeDeleted: true }); if (!pgRow) { - throw new Error(`Task ${id} not found`); + // FNXC:TaskLookup404 2026-07-26-12:00: typed miss (message unchanged) so + // DELETE /api/tasks/:id answers 404 for an unknown id instead of 500. + throw new TaskNotFoundError(id); } const task = store.rowToTask(store.pgRowToTaskRow(pgRow)); @@ -177,12 +181,28 @@ export async function deleteTaskBackendImpl(store: TaskStore, id: string, option removeLineageReferences: !!options?.removeLineageReferences, allowResurrection, sessionId: options?.auditContext?.sessionId, + // FNXC:TaskDeleteAttribution 2026-07-26-14:30: caller class + calling + // task id; `taskId` reached this function but was never persisted. + ...buildDeleteCallerAuditFields(options?.auditContext), }, }); }); // Emit lifecycle event (best-effort, outside the transaction). store.emit("task:deleted", task, { githubIssueAction: options?.githubIssueAction ?? "auto" }); + /* + FNXC:TaskDeleteNotice 2026-07-26-16:10: + Operator mailbox notice for a delete the operator did not perform. Deliberately placed here, + beside the lifecycle emit and OUTSIDE `transactionImmediate`: a mailbox INSERT that threw inside + that callback would roll back the committed soft-delete, the lineage clear, the mission unlink, + and the audit row. `task` is still the pre-delete snapshot at this point, so `task.column` is the + real previous column. `deleteTaskIfBackendImpl` delegates here, so it is covered too. + */ + await notifyOperatorOfNonOperatorDelete( + store, + { id: task.id, title: task.title, previousColumn: task.column, previousStatus: task.status ?? null }, + options?.auditContext, + ); return task; } @@ -191,7 +211,7 @@ export async function deleteTaskIfBackendImpl( store: TaskStore, id: string, predicate: (live: Task) => boolean | Promise, - options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: { agentId: string; runId: string; sessionId?: string; taskId?: string } }, + options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: TaskDeleteAuditContext }, ): Promise<{ task: Task; deleted: boolean }> { if (options?.auditContext?.taskId === id) throw new TaskSelfDeleteError(id); return store.withTaskLock(id, async () => { diff --git a/packages/core/src/task-store/archive-lifecycle.ts b/packages/core/src/task-store/archive-lifecycle.ts index 97a89e3149..e31b865687 100644 --- a/packages/core/src/task-store/archive-lifecycle.ts +++ b/packages/core/src/task-store/archive-lifecycle.ts @@ -8,8 +8,10 @@ */ import {TaskStore, storeLog} from "../store.js"; import {MissionStore} from "../mission-store.js"; -import {TaskHasDependentsError, TaskHasLineageChildrenError, TaskSelfDeleteError} from "./errors.js"; +import {TaskHasDependentsError, TaskHasLineageChildrenError, TaskNotFoundError, TaskSelfDeleteError} from "./errors.js"; import {isWorkspaceTask, type Task, type Column, type GithubIssueAction} from "../types.js"; +import {buildDeleteCallerAuditFields, type TaskDeleteAuditContext} from "../task-delete-attribution.js"; +import {notifyOperatorOfNonOperatorDelete, type TaskDeleteNoticeSnapshot} from "../task-delete-notice.js"; import "../builtin-traits.js"; import {__setTaskActivityLogLimitsForTesting} from "../task-store/comments.js"; import {toJson} from "../db-helpers.js"; @@ -187,7 +189,7 @@ function scheduleDeleteBranchCleanup(store: TaskStore, task: Task): void { })(); } -export async function deleteTaskImpl(store: TaskStore, id: string, options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: { agentId: string; runId: string; sessionId?: string; taskId?: string }; },): Promise { +export async function deleteTaskImpl(store: TaskStore, id: string, options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: TaskDeleteAuditContext; },): Promise { // FNXC:RuntimeLifecycleAsync 2026-06-24-12:00: // Backend-mode deleteTask: delegate the core async operations (task read, // lineage gate, lineage clear, soft-delete, audit) to the async helpers. @@ -206,13 +208,24 @@ export async function deleteTaskImpl(store: TaskStore, id: string, options?: { r if (store.backendMode) { return store.deleteTaskBackend(id, options); } + /* + FNXC:TaskDeleteNotice 2026-07-26-16:10: + Pre-delete snapshot for the operator mailbox notice. It must be captured INSIDE the lock (the + row is mutated to `archived` a few lines below, so reading `task.column` afterwards would always + report `archived`) but the notice itself is sent AFTER the lock and after the transaction + commits — a mailbox write inside `store.db.transaction` would roll the soft-delete back if it + threw. Left `undefined` on the already-deleted short-circuit so an idempotent re-delete is silent. + */ + let noticeSnapshot: TaskDeleteNoticeSnapshot | undefined; const deletedTask = await store.withTaskLock(id, async () => { // Flush buffered agent logs inside the lock so no new appends for this // task can sneak in between flush and soft-delete mutation. store.flushAgentLogBuffer(); const task = store.readTaskFromDb(id, { includeDeleted: true }); if (!task) { - throw new Error(`Task ${id} not found`); + // FNXC:TaskLookup404 2026-07-26-12:00: typed miss (message unchanged) so + // DELETE /api/tasks/:id answers 404 for an unknown id instead of 500. + throw new TaskNotFoundError(id); } if (task.deletedAt) { @@ -233,6 +246,8 @@ export async function deleteTaskImpl(store: TaskStore, id: string, options?: { r throw new TaskHasLineageChildrenError(id, lineageChildIds); } + noticeSnapshot = { id: task.id, title: task.title, previousColumn: task.column, previousStatus: task.status ?? null }; + let rewrittenDependents: Task[] = []; let rewrittenBlockedByResidueDependents: Task[] = []; let rewrittenLineageChildren: Task[] = []; @@ -259,6 +274,9 @@ export async function deleteTaskImpl(store: TaskStore, id: string, options?: { r removeLineageReferences: !!options?.removeLineageReferences, allowResurrection: options?.allowResurrection === true, sessionId: options?.auditContext?.sessionId, + // FNXC:TaskDeleteAttribution 2026-07-26-14:30: caller class + calling + // task id; `taskId` reached this function but was never persisted. + ...buildDeleteCallerAuditFields(options?.auditContext), }, }); store.clearLinkedAgentTaskIds(id, deletedAt); @@ -319,6 +337,8 @@ export async function deleteTaskImpl(store: TaskStore, id: string, options?: { r deletedAt: deletedTask.deletedAt ?? new Date().toISOString(), reason: "deleted", }); + // FNXC:TaskDeleteNotice 2026-07-26-16:10: best-effort, post-commit, never throws. + if (noticeSnapshot) await notifyOperatorOfNonOperatorDelete(store, noticeSnapshot, options?.auditContext); return deletedTask; } @@ -337,10 +357,13 @@ export async function deleteTaskIfImpl( store: TaskStore, id: string, predicate: (live: Task) => boolean | Promise, - options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: { agentId: string; runId: string; sessionId?: string; taskId?: string } }, + options?: { removeDependencyReferences?: boolean; removeLineageReferences?: boolean; allowResurrection?: boolean; githubIssueAction?: GithubIssueAction; auditContext?: TaskDeleteAuditContext }, ): Promise { if (options?.auditContext?.taskId === id) throw new TaskSelfDeleteError(id); if (store.backendMode) return store.deleteTaskIf(id, predicate, options); + // FNXC:TaskDeleteNotice 2026-07-26-16:10: see deleteTaskImpl — snapshot inside the lock, notify + // after commit. Stays `undefined` when the predicate declines, so a skipped delete is silent. + let noticeSnapshot: TaskDeleteNoticeSnapshot | undefined; const result = await store.withTaskLock(id, async () => { store.flushAgentLogBuffer(); const task = store.readTaskFromDb(id, { includeDeleted: true }); @@ -351,6 +374,7 @@ export async function deleteTaskIfImpl( const lineageChildIds = await store.findLiveLineageChildren(id); if (lineageChildIds.length > 0 && !options?.removeLineageReferences) throw new TaskHasLineageChildrenError(id, lineageChildIds); if (!await predicate(task)) return { task, deleted: false }; + noticeSnapshot = { id: task.id, title: task.title, previousColumn: task.column, previousStatus: task.status ?? null }; let deletedAt = ""; let rewrittenDependents: Task[] = []; let rewrittenBlockedByResidueDependents: Task[] = []; @@ -362,7 +386,7 @@ export async function deleteTaskIfImpl( deletedAt = new Date().toISOString(); const allowResurrection = options?.allowResurrection === true ? 1 : 0; store.db.prepare("UPDATE tasks SET \"column\" = 'archived', deletedAt = ?, allowResurrection = ?, updatedAt = ? WHERE id = ?").run(deletedAt, allowResurrection, deletedAt, id); - void store.recordRunAuditEvent({ domain: "database", mutationType: "task:deleted", target: task.id, taskId: task.id, agentId: options?.auditContext?.agentId ?? "system", runId: options?.auditContext?.runId ?? store.makeSyntheticDeleteRunId(task.id), metadata: { previousColumn: task.column, previousStatus: task.status ?? null, githubIssueAction: options?.githubIssueAction ?? "auto", removeDependencyReferences: !!options?.removeDependencyReferences, removeLineageReferences: !!options?.removeLineageReferences, allowResurrection: options?.allowResurrection === true, sessionId: options?.auditContext?.sessionId } }); + void store.recordRunAuditEvent({ domain: "database", mutationType: "task:deleted", target: task.id, taskId: task.id, agentId: options?.auditContext?.agentId ?? "system", runId: options?.auditContext?.runId ?? store.makeSyntheticDeleteRunId(task.id), metadata: { previousColumn: task.column, previousStatus: task.status ?? null, githubIssueAction: options?.githubIssueAction ?? "auto", removeDependencyReferences: !!options?.removeDependencyReferences, removeLineageReferences: !!options?.removeLineageReferences, allowResurrection: options?.allowResurrection === true, sessionId: options?.auditContext?.sessionId, ...buildDeleteCallerAuditFields(options?.auditContext) } }); store.clearLinkedAgentTaskIds(id, deletedAt); store.db.bumpLastModified(); }); @@ -384,6 +408,7 @@ export async function deleteTaskIfImpl( return { task, deleted: true }; }); if (result.deleted) await store.clearNearDuplicateReferencesToFailSoft(id, { column: "archived", deletedAt: result.task.deletedAt ?? new Date().toISOString(), reason: "deleted" }); + if (result.deleted && noticeSnapshot) await notifyOperatorOfNonOperatorDelete(store, noticeSnapshot, options?.auditContext); return result; } diff --git a/packages/core/src/task-store/errors.ts b/packages/core/src/task-store/errors.ts index 0a9787e676..e8bda566f1 100644 --- a/packages/core/src/task-store/errors.ts +++ b/packages/core/src/task-store/errors.ts @@ -35,6 +35,48 @@ export class TaskSelfDeleteError extends Error { } } +/** + * FNXC:TaskLookup404 2026-07-26-11:20: + * Requirement: a task-detail read for a task that does not exist must surface as + * HTTP 404, never 500 — clients (dashboard task detail, polling widgets, CLI) + * must be able to distinguish "this task is gone" from "the server is broken". + * + * `getTaskImpl` previously signalled the miss with a bare `new Error(...)`, so + * the only thing routes could match on was an errno `code === "ENOENT"` — a + * leftover from the file-backed storage era. In Postgres/backend mode nothing on + * the read path sets an errno code, so EVERY missing/unknown/soft-deleted/ + * wrong-project task read returned 500 (reported repro: + * `GET /api/tasks/FN-8610/runtime-fallback`). + * + * `message` is deliberately byte-identical to the legacy string + * (`Task ${taskId} not found`) because existing code paths and tests match on + * it; the typed class is the new primary signal, the message is back-compat. + */ +export class TaskNotFoundError extends Error { + readonly code = "TASK_NOT_FOUND" as const; + readonly taskId: string; + + constructor(taskId: string) { + super(`Task ${taskId} not found`); + this.name = "TaskNotFoundError"; + this.taskId = taskId; + } +} + +/** + * FNXC:TaskLookup404 2026-07-26-11:20: + * Type guard used by API boundaries to map a task miss to 404. Structural (name + * + code) rather than `instanceof`-only so the check survives a duplicated + * `@fusion/core` module instance (bundled CLI vs workspace dist) and errors that + * crossed a serialization boundary. + */ +export function isTaskNotFoundError(error: unknown): error is TaskNotFoundError { + if (error instanceof TaskNotFoundError) return true; + if (!error || typeof error !== "object") return false; + const candidate = error as { name?: unknown; code?: unknown }; + return candidate.name === "TaskNotFoundError" || candidate.code === "TASK_NOT_FOUND"; +} + export class TaskDeletedError extends Error { constructor( public readonly taskId: string, diff --git a/packages/core/src/task-store/reads.ts b/packages/core/src/task-store/reads.ts index 5b9775b7ec..5220e83eb6 100644 --- a/packages/core/src/task-store/reads.ts +++ b/packages/core/src/task-store/reads.ts @@ -23,6 +23,9 @@ import {getStalePausedTodoSignal} from "../stale-paused-todo.js"; import {getTaskAgeStalenessSignal, type TaskAgeStalenessThresholds} from "../task-age-staleness.js"; import {detectStalledReview} from "../stalled-review-detector.js"; import {computeRetrySummary} from "../retry-summary.js"; +// FNXC:TaskLookup404 2026-07-26-11:20: typed miss signal so API boundaries can +// answer 404 instead of 500 (see TaskNotFoundError in task-store/errors.ts). +import {TaskNotFoundError} from "../task-store/errors.js"; /** Merge storage tiers while preserving primary-source authority and order. */ function mergePrimaryById(primary: T[], secondary: T[]): T[] { @@ -121,7 +124,15 @@ export async function getTaskImpl(store: TaskStore, id: string, options?: { acti */ const archived = await getArchivedTask(layer.db, id, layer.projectId); if (!archived) { - throw new Error(`Task ${id} not found`); + /* + FNXC:TaskLookup404 2026-07-26-11:20: + Backend/Postgres miss. Throw the typed TaskNotFoundError (message kept + byte-identical to the legacy `Task ${id} not found` string) so route + catches can map it to 404. Nothing on this path sets an errno `code`, + so the routes' legacy ENOENT check never fired and every unknown task + id 500'd. + */ + throw new TaskNotFoundError(id); } const archivedTask = store.archiveEntryToTask(archived, false); return { @@ -196,7 +207,9 @@ export async function getTaskImpl(store: TaskStore, id: string, options?: { acti if (!task) { const archived = store.archiveDb.get(id); if (!archived) { - throw new Error(`Task ${id} not found`); + // FNXC:TaskLookup404 2026-07-26-11:20: legacy/sync branch throws the same + // typed miss as the backend branch so 404 mapping is branch-independent. + throw new TaskNotFoundError(id); } const archivedTask = store.archiveEntryToTask(archived, false); return { diff --git a/packages/dashboard/app/api/__tests__/client-identity-header.test.ts b/packages/dashboard/app/api/__tests__/client-identity-header.test.ts new file mode 100644 index 0000000000..22e56a4768 --- /dev/null +++ b/packages/dashboard/app/api/__tests__/client-identity-header.test.ts @@ -0,0 +1,73 @@ +/* +FNXC:TaskDeleteAttribution 2026-07-26-14:30: +Client half of the delete-attribution invariant. The server can only label a request `operator-ui` +if the dashboard actually sends `x-fusion-client: dashboard-ui`; without this the UI's own Delete +click would report as `api-unattributed` and the four-delete incident would still be unattributable. + +Asserted centrally (on `api()`, not on `deleteTask`) because that is where the header is set — +covering the authenticated and unauthenticated branches, the with-caller-headers case, and the +task-delete call itself so no future route has to remember the header. +*/ + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FUSION_CLIENT_HEADER, FUSION_DASHBOARD_UI_CLIENT } from "@fusion/core"; +import { api } from "../client"; +import { deleteTask } from "../tasks-lifecycle"; +import * as auth from "../../auth"; + +function mockFetch() { + return vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({ ok: true }), { status: 200, headers: { "content-type": "application/json" } }), + ); +} + +function sentHeaders(spy: ReturnType): Headers { + const init = spy.mock.calls[0]?.[1] as RequestInit | undefined; + return new Headers(init?.headers ?? {}); +} + +describe("dashboard API client identity header", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("stamps the dashboard-ui client header on unauthenticated requests", async () => { + vi.spyOn(auth, "getAuthToken").mockReturnValue(undefined); + const spy = mockFetch(); + + await api("/tasks"); + + expect(sentHeaders(spy).get(FUSION_CLIENT_HEADER)).toBe(FUSION_DASHBOARD_UI_CLIENT); + }); + + it("stamps the client header on authenticated requests too", async () => { + vi.spyOn(auth, "getAuthToken").mockReturnValue("token-1"); + const spy = mockFetch(); + + await api("/tasks"); + + expect(sentHeaders(spy).get(FUSION_CLIENT_HEADER)).toBe(FUSION_DASHBOARD_UI_CLIENT); + }); + + it("keeps the client header when the caller supplies its own headers", async () => { + vi.spyOn(auth, "getAuthToken").mockReturnValue(undefined); + const spy = mockFetch(); + + await api("/tasks", { method: "POST", headers: { "X-Custom": "1" } }); + + const headers = sentHeaders(spy); + expect(headers.get(FUSION_CLIENT_HEADER)).toBe(FUSION_DASHBOARD_UI_CLIENT); + expect(headers.get("X-Custom")).toBe("1"); + }); + + it("sends the client header on the task delete call (the surface that was unattributable)", async () => { + vi.spyOn(auth, "getAuthToken").mockReturnValue(undefined); + const spy = mockFetch(); + + await deleteTask("FN-8600"); + + const init = spy.mock.calls[0]?.[1] as RequestInit | undefined; + expect(init?.method).toBe("DELETE"); + expect(sentHeaders(spy).get(FUSION_CLIENT_HEADER)).toBe(FUSION_DASHBOARD_UI_CLIENT); + }); +}); diff --git a/packages/dashboard/app/api/client.ts b/packages/dashboard/app/api/client.ts index e67cba186d..d2e0574e74 100644 --- a/packages/dashboard/app/api/client.ts +++ b/packages/dashboard/app/api/client.ts @@ -2,6 +2,7 @@ * FNXC:CodeOrganization 2026-07-15-16:00: * Dashboard API client core (fetch wrapper + ApiRequestError). */ +import { FUSION_CLIENT_HEADER, FUSION_DASHBOARD_UI_CLIENT } from "@fusion/core"; import { getAuthToken, withTokenHeader } from "../auth"; import type { DedupeOptions } from "./dedupe"; @@ -33,6 +34,24 @@ export function buildApiUrl(path: string): string { return `/api${path}`; } +/** + * FNXC:TaskDeleteAttribution 2026-07-26-14:30: + * Stamp every dashboard-originated request with `x-fusion-client: dashboard-ui` so server-side + * run-audit can tell an operator's click apart from an unlabeled script or agent hitting the same + * endpoint (the four-delete incident where `DELETE /api/tasks/:id` rows were byte-identical + * regardless of who called). Applied once here rather than per-call so no future mutation route + * has to remember it; the desktop shell mounts this same App and therefore inherits it. + * + * Self-reported and explicitly NOT a security boundary — anything can send this header. It + * separates "the client said it was the dashboard UI" from "nothing identified itself"; no + * authorization decision may depend on it. An existing explicit value is left alone. + */ +function applyClientIdentityHeader(headers: Headers): void { + if (!headers.has(FUSION_CLIENT_HEADER)) { + headers.set(FUSION_CLIENT_HEADER, FUSION_DASHBOARD_UI_CLIENT); + } +} + export async function api(path: string, opts: RequestInit = {}): Promise { const url = buildApiUrl(path); const token = getAuthToken(); @@ -42,17 +61,15 @@ export async function api(path: string, opts: RequestInit = {}): Pr if (!authenticatedHeaders.has("Content-Type")) { authenticatedHeaders.set("Content-Type", "application/json"); } + applyClientIdentityHeader(authenticatedHeaders); return withTokenHeader(authenticatedHeaders); } - if (!opts.headers) { - return { "Content-Type": "application/json" }; - } - - const defaultHeaders = new Headers(opts.headers); + const defaultHeaders = new Headers(opts.headers ?? {}); if (!defaultHeaders.has("Content-Type")) { defaultHeaders.set("Content-Type", "application/json"); } + applyClientIdentityHeader(defaultHeaders); return Object.fromEntries(defaultHeaders.entries()); })(); diff --git a/packages/dashboard/src/__tests__/api-error.test.ts b/packages/dashboard/src/__tests__/api-error.test.ts index dd1742b661..63869a5ff5 100644 --- a/packages/dashboard/src/__tests__/api-error.test.ts +++ b/packages/dashboard/src/__tests__/api-error.test.ts @@ -10,6 +10,7 @@ import { internalError, notFound, rateLimited, + rethrowAsApiError, sendErrorResponse, unauthorized, } from "../api-error.js"; @@ -262,4 +263,177 @@ describe("error factories", () => { expect(error.statusCode).toBe(500); expect(error.message).toBe("msg"); }); + + it("factories accept an optional trailing cause without changing status/message/details", () => { + const origin = new Error("origin"); + + expect((badRequest("msg", { field: "x" }, origin) as { cause?: unknown }).cause).toBe(origin); + expect((unauthorized("msg", origin) as { cause?: unknown }).cause).toBe(origin); + expect((notFound("msg", origin) as { cause?: unknown }).cause).toBe(origin); + expect((conflict("msg", { a: 1 }, origin) as { cause?: unknown }).cause).toBe(origin); + expect((rateLimited("msg", 60, origin) as { cause?: unknown }).cause).toBe(origin); + expect((internalError("msg", origin) as { cause?: unknown }).cause).toBe(origin); + + const withDetails = conflict("msg", { a: 1 }, origin); + expect(withDetails.statusCode).toBe(409); + expect(withDetails.message).toBe("msg"); + expect(withDetails.details).toEqual({ a: 1 }); + }); +}); + +/* +FNXC:ApiErrorDiagnostics 2026-07-26-11:20: +Invariant under test: ANY 5xx that reaches the boundary through `rethrowAsApiError` +logs the ORIGIN throw site's stack, not the rethrow site's, and preserves the wrapper +chain — across the Error branch, the non-Error fallback branch, and multi-level wraps. +Asserting only the single reported repro (a plain Error) would have passed before the +fix, since an unwrapped Error's own stack was already the origin. +*/ +describe("rethrowAsApiError diagnostics", () => { + function findApiErrorLog(): Record { + const event = runtimeLogEvents.find((e) => e.scope === "api:error" && e.message === "Request failed"); + expect(event, "expected an api:error 'Request failed' log entry").toBeDefined(); + return (event?.context ?? {}) as Record; + } + + /** Named so the origin frame is identifiable in a captured stack. */ + function originThrowSiteMarker(): never { + throw new Error("store read failed"); + } + + async function runThroughBoundary(throwing: () => unknown): Promise { + const handler = catchHandler(async () => { + try { + await throwing(); + } catch (error) { + rethrowAsApiError(error); + } + }); + const mock = createMockResponse(); + await handler({} as Request, mock.res, vi.fn()); + return mock; + } + + it("logs the origin stack, not the rethrow site, for the Error branch", async () => { + const { statusMock, jsonMock } = await runThroughBoundary(originThrowSiteMarker); + + expect(statusMock).toHaveBeenCalledWith(500); + expect(jsonMock).toHaveBeenCalledWith({ error: "store read failed" }); + + const context = findApiErrorLog(); + expect(context.statusCode).toBe(500); + expect(context.message).toBe("store read failed"); + expect(typeof context.stack).toBe("string"); + expect(context.stack).toContain("originThrowSiteMarker"); + expect(context.stack).not.toContain("rethrowAsApiError"); + }); + + it("preserves a multi-level cause chain and still reports the deepest origin stack", async () => { + const { jsonMock } = await runThroughBoundary(() => { + try { + originThrowSiteMarker(); + } catch (error) { + throw new Error("task detail load failed", { cause: error }); + } + }); + + expect(jsonMock).toHaveBeenCalledWith({ error: "task detail load failed" }); + + const context = findApiErrorLog(); + // Deepest link in the chain is the true origin. + expect(context.stack).toContain("originThrowSiteMarker"); + // Every wrapper past the boundary error survives in `cause`. + expect(context.cause).toContain("task detail load failed"); + expect(context.cause).toContain("store read failed"); + }); + + it("keeps the raw thrown value reachable on the non-Error fallback branch", async () => { + const { statusMock, jsonMock } = await runThroughBoundary(() => { + throw "raw string failure"; + }); + + expect(statusMock).toHaveBeenCalledWith(500); + expect(jsonMock).toHaveBeenCalledWith({ error: "Internal server error" }); + + const context = findApiErrorLog(); + expect(context.message).toBe("Internal server error"); + expect(context.cause).toContain("raw string failure"); + }); + + it("keeps the origin stack for an Error with an empty message (fallback branch)", async () => { + const empty = new Error(""); + const { jsonMock } = await runThroughBoundary(() => { + throw empty; + }); + + expect(jsonMock).toHaveBeenCalledWith({ error: "Internal server error" }); + + const context = findApiErrorLog(); + expect(context.cause).toBe(empty.stack); + expect(context.stack).toBe(empty.stack); + }); + + it("passes an ApiError through untouched, preserving status and details", async () => { + const handler = catchHandler(async () => { + try { + throw notFound("Task FN-8610 not found"); + } catch (error) { + rethrowAsApiError(error); + } + }); + const { res, statusMock, jsonMock } = createMockResponse(); + + await handler({} as Request, res, vi.fn()); + + expect(statusMock).toHaveBeenCalledWith(404); + expect(jsonMock).toHaveBeenCalledWith({ error: "Task FN-8610 not found" }); + // 4xx must remain unlogged. + expect(runtimeLogEvents).toHaveLength(0); + }); +}); + +describe("sendErrorResponse cause-chain logging", () => { + it("reports an unwrapped error's own stack with no cause", () => { + const { res } = createMockResponse(); + const error = new Error("plain"); + + sendErrorResponse(res, 500, "plain", { error }); + + const context = runtimeLogEvents[0]?.context as Record; + expect(context.stack).toBe(error.stack); + expect(context.cause).toBeUndefined(); + }); + + it("omits stack and cause when the thrown value is not an Error", () => { + const { res } = createMockResponse(); + + sendErrorResponse(res, 500, "weird", { error: { code: 500 } }); + + const context = runtimeLogEvents[0]?.context as Record; + expect(context.stack).toBeUndefined(); + expect(context.cause).toBeUndefined(); + }); + + it("terminates on a cyclic cause chain", () => { + const { res } = createMockResponse(); + const a = new Error("a"); + const b = new Error("b", { cause: a }); + (a as { cause?: unknown }).cause = b; + + sendErrorResponse(res, 500, "cycle", { error: b }); + + const context = runtimeLogEvents[0]?.context as Record; + expect(context.cause).toContain("a"); + expect(String(context.cause).split("Caused by:").length).toBeLessThanOrEqual(8); + }); + + it("does not log or alter the body for non-5xx even when an error is supplied", () => { + const { res, statusMock, jsonMock } = createMockResponse(); + + sendErrorResponse(res, 409, "Conflict", { error: new Error("origin"), details: { a: 1 } }); + + expect(statusMock).toHaveBeenCalledWith(409); + expect(jsonMock).toHaveBeenCalledWith({ error: "Conflict", details: { a: 1 } }); + expect(runtimeLogEvents).toHaveLength(0); + }); }); diff --git a/packages/dashboard/src/api-error.ts b/packages/dashboard/src/api-error.ts index bd6b25c199..b0b1688e22 100644 --- a/packages/dashboard/src/api-error.ts +++ b/packages/dashboard/src/api-error.ts @@ -16,10 +16,60 @@ export interface SendErrorOptions { *message* was logged and `rethrowAsApiError` discarded the stack, leaving the full-TaskDetail 500s on /api/tasks/:id (GET/DELETE/PATCH/retry/archive/reset) untraceable across releases. + + FNXC:ApiErrorDiagnostics 2026-07-26-11:20: + That contract was silently defeated on the most common path: `rethrowAsApiError` + built a fresh `ApiError(500, error.message)` and dropped the caught error, so the + boundary logged the rethrow site's stack (or, for a non-Error throw, no stack at + all — observed on GET /api/tasks/FN-8610/runtime-fallback, whose 500 log carried + only method/path/statusCode/message). The caught value is now threaded into + `ApiError.cause` end-to-end, and the boundary walks the chain so `stack` is the + ORIGIN error's stack and `cause` renders the wrappers between origin and boundary. */ error?: unknown; } +/* +FNXC:ApiErrorDiagnostics 2026-07-26-11:20: +Chain-walk bounds. Depth caps a pathological/self-referential wrap; the visited set +stops a cycle (`a.cause = b; b.cause = a`) from spinning the request thread. +*/ +const MAX_CAUSE_DEPTH = 8; + +/** + * FNXC:ApiErrorDiagnostics 2026-07-26-11:20: + * Flatten an error and its `cause` chain, nearest-wrapper first. Values are returned + * as-is (not stringified) so callers decide the log rendering; only Errors and raw + * thrown values enter the chain — never arbitrary object payloads. + */ +function collectCauseChain(error: unknown): unknown[] { + const chain: unknown[] = []; + const seen = new Set(); + let current = error; + + while (current !== undefined && chain.length < MAX_CAUSE_DEPTH) { + if (typeof current === "object" && current !== null) { + if (seen.has(current)) break; + seen.add(current); + } + chain.push(current); + current = current instanceof Error ? (current as { cause?: unknown }).cause : undefined; + } + + return chain; +} + +/** + * FNXC:ApiErrorDiagnostics 2026-07-26-11:20: + * Render one chain link for the log: an Error contributes its stack (message fallback + * when a runtime omits `stack`), any other thrown value its `String(...)` form. Keeps + * the log context ids/paths/stack-only. + */ +function renderChainLink(value: unknown): string { + if (value instanceof Error) return value.stack ?? value.message; + return String(value); +} + export class ApiError extends Error { public readonly statusCode: number; public readonly details?: Record; @@ -52,15 +102,26 @@ export function sendErrorResponse( const logger = options?.logger ?? createRuntimeLogger("api:error"); // FNXC:ApiErrorDiagnostics 2026-07-10-14:00: log the underlying stack and // cause (not just the message) so a 500 can be traced to its origin. + /* + FNXC:ApiErrorDiagnostics 2026-07-26-11:20: + Walk the whole `cause` chain rather than one level. `stack` reports the DEEPEST + Error in the chain — the origin throw site — because the boundary is handed the + wrapping `ApiError` whose own stack only names `rethrowAsApiError`. `cause` + reports every link past the boundary error so intermediate wrappers survive. An + unwrapped Error still logs its own stack and no cause, unchanged from before. + */ const originalError = options?.error; - const cause = originalError instanceof Error ? (originalError as { cause?: unknown }).cause : undefined; + const chain = collectCauseChain(originalError); + const errorLinks = chain.filter((link): link is Error => link instanceof Error); + const originError = errorLinks.length > 0 ? errorLinks[errorLinks.length - 1] : undefined; + const causeLinks = chain.slice(1); logger.error("Request failed", { method: request?.method, path: request?.originalUrl ?? request?.path, statusCode, message, - stack: originalError instanceof Error ? originalError.stack : undefined, - cause: cause instanceof Error ? (cause.stack ?? cause.message) : cause !== undefined ? String(cause) : undefined, + stack: originError?.stack, + cause: causeLinks.length > 0 ? causeLinks.map(renderChainLink).join("\nCaused by: ") : undefined, }); } @@ -99,38 +160,54 @@ export function catchHandler(fn: AsyncHandler): RequestHandler { }; } -export function badRequest(message: string, details?: Record): ApiError { - return new ApiError(400, message, details); +/* +FNXC:ApiErrorDiagnostics 2026-07-26-11:20: +Every factory takes an optional TRAILING `cause` so the hundreds of existing +single/double-argument call sites across the route registrars stay source-compatible; +callers that already hold the caught error can pass it and get an origin stack in the +5xx log for free. +*/ + +export function badRequest(message: string, details?: Record, cause?: unknown): ApiError { + return new ApiError(400, message, details, cause); } -export function unauthorized(message: string): ApiError { - return new ApiError(401, message); +export function unauthorized(message: string, cause?: unknown): ApiError { + return new ApiError(401, message, undefined, cause); } -export function notFound(message: string): ApiError { - return new ApiError(404, message); +export function notFound(message: string, cause?: unknown): ApiError { + return new ApiError(404, message, undefined, cause); } -export function conflict(message: string, details?: Record): ApiError { - return new ApiError(409, message, details); +export function conflict(message: string, details?: Record, cause?: unknown): ApiError { + return new ApiError(409, message, details, cause); } -export function rateLimited(message: string, retryAfter?: number): ApiError { - return new ApiError(429, message, { retryAfter }); +export function rateLimited(message: string, retryAfter?: number, cause?: unknown): ApiError { + return new ApiError(429, message, { retryAfter }, cause); } -export function internalError(message: string): ApiError { - return new ApiError(500, message); +export function internalError(message: string, cause?: unknown): ApiError { + return new ApiError(500, message, undefined, cause); } +/** + * FNXC:ApiErrorDiagnostics 2026-07-26-11:20: + * Thread the caught value into the wrapping `ApiError` on BOTH branches. Before this + * the caught error was read for its message and then discarded, so the 5xx boundary + * logged the stack of the `ApiError` constructed here (or nothing at all for a + * non-Error throw). Message, status, and response body are unchanged — this only + * adds `cause`. + */ export function rethrowAsApiError(error: unknown, fallbackMessage = "Internal server error"): never { if (error instanceof ApiError) { throw error; } if (error instanceof Error && error.message) { - throw internalError(error.message); + throw internalError(error.message, error); } - throw internalError(fallbackMessage); + throw internalError(fallbackMessage, error); } diff --git a/packages/dashboard/src/file-service.ts b/packages/dashboard/src/file-service.ts index 6cdcf3edf7..d0eb2d172c 100644 --- a/packages/dashboard/src/file-service.ts +++ b/packages/dashboard/src/file-service.ts @@ -1,4 +1,4 @@ -import { createLogger } from "@fusion/core"; +import { createLogger, isTaskNotFoundError } from "@fusion/core"; const severityAuditLog = createLogger("dashboard-file-service"); import { join, resolve, relative, dirname, basename } from "node:path"; @@ -92,7 +92,15 @@ async function getTaskBasePath(store: TaskStore, taskId: string): Promise { + const deleteTask = vi.fn(async (id: string, _options?: DeleteOptions) => ({ id, column: "archived" })); + + const store: TaskStore = { + getRootDir: vi.fn(() => process.cwd()), + deleteTask, + getProjectScopedPluginMcpServers: vi.fn().mockResolvedValue([]), + } as unknown as TaskStore; + + const app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store)); + return { app, deleteTask }; +}; + +async function callerKindFor(headers: Record): Promise { + const { app, deleteTask } = createHarness(); + const res = await REQUEST(app, "DELETE", `/api/tasks/${TARGET_ID}`, undefined, headers); + expect(res.status).toBe(200); + const options = deleteTask.mock.calls[0]?.[1] as DeleteOptions | undefined; + return options?.auditContext?.callerKind; +} + +describe("DELETE /api/tasks/:id caller attribution", () => { + it("labels a request that identifies itself as the dashboard UI as operator-ui", async () => { + expect(await callerKindFor({ [FUSION_CLIENT_HEADER]: FUSION_DASHBOARD_UI_CLIENT })).toBe("operator-ui"); + }); + + it("labels a request with no client header as api-unattributed", async () => { + expect(await callerKindFor({})).toBe("api-unattributed"); + }); + + it("labels a request with an unrecognized client header as api-unattributed", async () => { + expect(await callerKindFor({ [FUSION_CLIENT_HEADER]: "curl-script" })).toBe("api-unattributed"); + }); + + /* + FNXC:TaskDeleteAttribution 2026-07-26-14:30: + Observability only — this change adds no delete-blocking, gating, or permission logic. An + unattributed caller must still be allowed to delete exactly as before; only the audit row differs. + */ + it("does not block an unattributed delete", async () => { + const { app, deleteTask } = createHarness(); + const res = await REQUEST(app, "DELETE", `/api/tasks/${TARGET_ID}`); + expect(res.status).toBe(200); + expect(deleteTask).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/dashboard/src/routes/__tests__/register-task-workflow-routes.task-not-found-404.test.ts b/packages/dashboard/src/routes/__tests__/register-task-workflow-routes.task-not-found-404.test.ts new file mode 100644 index 0000000000..12df0659f3 --- /dev/null +++ b/packages/dashboard/src/routes/__tests__/register-task-workflow-routes.task-not-found-404.test.ts @@ -0,0 +1,149 @@ +// @vitest-environment node + +/* +FNXC:TaskLookup404 2026-07-26-12:20: +Requirement under test: a task-detail read for a task that does not exist must +return HTTP 404, never 500 — clients must be able to distinguish "this task is +gone" from "the server is broken". + +Reported repro: `GET /api/tasks/FN-8610/runtime-fallback` returned 500 with the +body `{"error":"Task FN-8610 not found"}`. Root cause: route catches detected a +missing task only via `(err as NodeJS.ErrnoException).code === "ENOENT"`, a +file-backed-storage-era leftover. In Postgres/backend mode nothing on the task +read path sets an errno code, so EVERY missing/unknown/soft-deleted/wrong-project +task read fell through to 500. + +Per AGENTS.md "Fix the Invariant, Not the Repro", this asserts the general +invariant across the affected route surfaces (task detail, runtime-fallback, +workflow-results, pause, PATCH, DELETE, session diff, PR status), not just the +one reported endpoint — plus the negative control that a genuine server fault +still surfaces as 500 rather than being laundered into a 404. +*/ + +import { describe, it, expect, vi } from "vitest"; +import express from "express"; +import type { TaskStore } from "@fusion/core"; +import { TaskNotFoundError } from "@fusion/core"; +import { createApiRoutes } from "../../routes.js"; +import { request as REQUEST } from "../../test-request.js"; + +const MISSING_ID = "FN-8610"; + +/** + * FNXC:TaskLookup404 2026-07-26-12:20: + * In-memory store fake (no DB, no network, no timers — AGENTS.md "Do Not Add + * Slow Tests"). Every task-lookup entry point throws the same typed + * `TaskNotFoundError` the real `getTaskImpl` now throws, so the test exercises + * the route-boundary mapping rather than the store. + */ +const createHarness = (taskLookupError: () => Error) => { + const throwMiss = vi.fn(async (_id?: unknown) => { + throw taskLookupError(); + }); + + const store: TaskStore = { + getRootDir: vi.fn(() => process.cwd()), + getTask: throwMiss, + updateTask: throwMiss, + deleteTask: throwMiss, + pauseTask: throwMiss, + getRunAuditEventsAsync: vi.fn(async () => []), + /* + FNXC:PluginMcpServers 2026-07-24-02:05 (mirrored from the runtime-fallback + suite): a store exposing getProjectScopedPluginMcpServers is treated as + runtime-owned, so resolveProjectContext skips the plugin-MCP binder that + would otherwise 500 on getPluginStore(). + */ + getProjectScopedPluginMcpServers: vi.fn().mockResolvedValue([]), + } as unknown as TaskStore; + + const app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store)); + return { app, store }; +}; + +/* +FNXC:TaskLookup404 2026-07-26-12:20: +Surface enumeration for the invariant. Each entry is a route whose handler +pre-checks (or loads) the task through `store.getTask(...)` and therefore +observed the 500. The list spans all four registrars that own per-task `:id` +routes: register-task-workflow-routes, register-session-diff-routes, +register-git-github, and the shared PATCH/DELETE task-mutation handlers. +*/ +const MISSING_TASK_SURFACES: ReadonlyArray<{ name: string; method: "GET" | "POST" | "PATCH" | "DELETE"; path: string; body?: Record }> = [ + { name: "GET /tasks/:id (task detail)", method: "GET", path: `/api/tasks/${MISSING_ID}` }, + { name: "GET /tasks/:id/runtime-fallback (reported repro)", method: "GET", path: `/api/tasks/${MISSING_ID}/runtime-fallback` }, + { name: "GET /tasks/:id/workflow-results", method: "GET", path: `/api/tasks/${MISSING_ID}/workflow-results` }, + { name: "GET /tasks/:id/session-files", method: "GET", path: `/api/tasks/${MISSING_ID}/session-files` }, + { name: "GET /tasks/:id/pr/status", method: "GET", path: `/api/tasks/${MISSING_ID}/pr/status` }, + { name: "POST /tasks/:id/pause", method: "POST", path: `/api/tasks/${MISSING_ID}/pause`, body: {} }, + { name: "PATCH /tasks/:id", method: "PATCH", path: `/api/tasks/${MISSING_ID}`, body: { title: "renamed" } }, + { name: "DELETE /tasks/:id", method: "DELETE", path: `/api/tasks/${MISSING_ID}` }, +]; + +describe("task-miss routes return 404 (not 500)", () => { + for (const surface of MISSING_TASK_SURFACES) { + it(`${surface.name} responds 404 for an unknown task id`, async () => { + const { app } = createHarness(() => new TaskNotFoundError(MISSING_ID)); + + const res = await REQUEST( + app, + surface.method, + surface.path, + surface.body ? JSON.stringify(surface.body) : undefined, + surface.body ? { "content-type": "application/json" } : {}, + ); + + expect(res.status).toBe(404); + expect(res.status).not.toBe(500); + }); + } + + /* + FNXC:TaskLookup404 2026-07-26-12:20: + Legacy safety: the file-backed-era ENOENT signal must keep mapping to 404 so + genuinely file-backed reads on the same handlers are unaffected by the switch + to the typed error. + */ + it("keeps mapping a legacy errno ENOENT task miss to 404", async () => { + const { app } = createHarness(() => { + const err = new Error(`Task ${MISSING_ID} not found`) as NodeJS.ErrnoException; + err.code = "ENOENT"; + return err; + }); + + const res = await REQUEST(app, "GET", `/api/tasks/${MISSING_ID}/runtime-fallback`); + + expect(res.status).toBe(404); + }); + + /* + FNXC:TaskLookup404 2026-07-26-12:20: + Negative control — the fix must not launder real server faults into 404s. A + non-miss failure on the same call path still has to surface as 500 so genuine + breakage stays visible/alertable. + */ + it("still returns 500 when the task read fails for a non-miss reason", async () => { + const { app } = createHarness(() => new Error("connection terminated unexpectedly")); + + const res = await REQUEST(app, "GET", `/api/tasks/${MISSING_ID}`); + + expect(res.status).toBe(500); + }); + + /* + FNXC:TaskLookup404 2026-07-26-12:20: + Response-body contract: the 404 keeps the byte-identical legacy message so + existing message-matching clients and tests are unaffected — only the status + code changes. + */ + it("preserves the legacy `Task not found` message on the 404", async () => { + const { app } = createHarness(() => new TaskNotFoundError(MISSING_ID)); + + const res = await REQUEST(app, "GET", `/api/tasks/${MISSING_ID}`); + + expect(res.status).toBe(404); + expect((res.body as { error?: string }).error).toBe(`Task ${MISSING_ID} not found`); + }); +}); diff --git a/packages/dashboard/src/routes/register-git-github.ts b/packages/dashboard/src/routes/register-git-github.ts index 6817407504..f5b60f4456 100644 --- a/packages/dashboard/src/routes/register-git-github.ts +++ b/packages/dashboard/src/routes/register-git-github.ts @@ -40,6 +40,8 @@ import { rateLimited, unauthorized, } from "../api-error.js"; +// FNXC:TaskLookup404 2026-07-26-11:40: shared task-miss -> 404 mapping seam. +import { isTaskLookupMiss, rethrowTaskApiError } from "./task-lookup-error.js"; import { GitHubClient, buildGitHubIssueSource, isGitHubIssueAlreadyImported, type PrReviewSnapshot, parseBadgeUrl } from "../github.js"; import { importIssueImageAttachments, githubImagePolicy } from "../issue-image-attachments.js"; import { GitHubIssueCommentService } from "../github-issue-comment.js"; @@ -102,6 +104,16 @@ function mapStructuredGhErrorToStatus(code: StructuredGhError["code"]): number { } function toPrApiError(err: unknown, fallbackMessage: string): ApiError { + /* + FNXC:TaskLookup404 2026-07-26-11:50: + Every PR route pre-checks its task with getTask, so a task miss can reach the + GitHub-error classifier. classifyGhError knows nothing about task ids and would + label the miss a generic PR failure (500). Map it to 404 first so an unknown + task id is reported as gone rather than as a broken PR integration. + */ + if (isTaskLookupMiss(err)) { + return notFound(err instanceof Error && err.message ? err.message : fallbackMessage); + } const githubError = classifyGhError(err); return new ApiError(mapStructuredGhErrorToStatus(githubError.code), githubError.message || fallbackMessage, { githubError, @@ -5171,7 +5183,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { appendBatchStatusError(results, taskId, `Task ${taskId} not found`); } else { appendBatchStatusError(results, taskId, err instanceof Error ? err.message : String(err) || `Failed to load task ${taskId}`); @@ -5399,7 +5411,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } else if ((err instanceof Error ? err.message : String(err)).includes("already exists")) { throw conflict(err instanceof Error ? err.message : String(err)); @@ -5469,7 +5481,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } if ((err instanceof Error ? err.message : String(err)).includes("already exists")) { @@ -5554,7 +5566,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } throw toPrApiError(err, "Failed to resolve PR conflicts"); @@ -5623,7 +5635,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err, "Failed to generate PR metadata"); @@ -5649,7 +5661,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err, "Failed to load PR preflight"); @@ -5740,7 +5752,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err, "Failed to load PR options"); @@ -5790,7 +5802,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } else { rethrowAsApiError(err); @@ -5945,7 +5957,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { }); } catch (err: unknown) { if (err instanceof ApiError) throw err; - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } throw toPrApiError(err, "Failed to refresh PR status"); @@ -5974,7 +5986,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { res.json({ task: updatedTask, prInfos: getTaskPrList(updatedTask) }); } catch (err: unknown) { if (err instanceof ApiError) throw err; - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err, "Failed to unlink pull request"); @@ -6005,7 +6017,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { res.json({ queued: true }); } catch (err: unknown) { if (err instanceof ApiError) throw err; - rethrowAsApiError(err, "Failed to queue PR conflict reclaim"); + rethrowTaskApiError(err, req.params.id, "Failed to queue PR conflict reclaim"); } }); @@ -6073,7 +6085,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { res.json({ prInfo }); } catch (err: unknown) { if (err instanceof ApiError) throw err; - rethrowAsApiError(err, "Failed to set PR auto-merge"); + rethrowTaskApiError(err, req.params.id, "Failed to set PR auto-merge"); } }); @@ -6144,7 +6156,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } throw toPrApiError(err, "Failed to fetch PR reviews"); @@ -6216,7 +6228,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } else { throw toPrApiError(err, "Failed to fetch PR checks"); @@ -6255,7 +6267,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } else { rethrowAsApiError(err); @@ -6331,7 +6343,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } else if ((err instanceof Error ? err.message : String(err)).includes("not found")) { throw notFound(err instanceof Error ? err.message : String(err)); diff --git a/packages/dashboard/src/routes/register-planning-subtask-routes.ts b/packages/dashboard/src/routes/register-planning-subtask-routes.ts index 476c6a464b..9e887e5ada 100644 --- a/packages/dashboard/src/routes/register-planning-subtask-routes.ts +++ b/packages/dashboard/src/routes/register-planning-subtask-routes.ts @@ -424,9 +424,12 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann try { await scopedStore.deleteTask(normalizedParentId, { auditContext: { + // FNXC:TaskDeleteAttribution 2026-07-26-14:30: subtask-breakdown parent close is + // automation running behind the planning session, not the operator's Delete click. agentId: "system", runId: `synthetic-planning-delete-${normalizedParentId}-${Date.now()}`, sessionId, + callerKind: "engine", }, }); parentTaskClosed = true; diff --git a/packages/dashboard/src/routes/register-session-diff-routes.ts b/packages/dashboard/src/routes/register-session-diff-routes.ts index dc5610770a..f00ea3b6f9 100644 --- a/packages/dashboard/src/routes/register-session-diff-routes.ts +++ b/packages/dashboard/src/routes/register-session-diff-routes.ts @@ -7,6 +7,8 @@ import type { Request, Router } from "express"; import type { RunAuditEvent, RunAuditEventFilter } from "@fusion/core"; import { isWorkspaceTask } from "@fusion/core"; import { ApiError, notFound, rethrowAsApiError } from "../api-error.js"; +// FNXC:TaskLookup404 2026-07-26-11:40: shared task-miss -> 404 mapping seam. +import { isTaskLookupMiss, rethrowTaskApiError } from "./task-lookup-error.js"; import { resolveDiffBase, runGitCommand } from "./resolve-diff-base.js"; import { countPatchLines } from "./diff-counts.js"; import { filterFilesToOwnTaskCommits } from "./attribute-done-range-files.js"; @@ -948,7 +950,7 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err, "Internal server error"); @@ -1189,7 +1191,7 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -1381,7 +1383,7 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err, "Internal server error"); @@ -1424,7 +1426,7 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err, "Internal server error"); diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index 5191063232..e50c3572bc 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -89,7 +89,10 @@ import { buildBoardWorkflowsPayload } from "./board-workflows.js"; import { resolveNativeStructurePreview } from "../native-structure-preview.js"; import { isBackwardMoveBlockedByOpenPr, PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE } from "./register-pull-requests-routes.js"; import { computePlanApprovalFingerprint, isWorkspaceTask, type RunAuditEventInput } from "@fusion/core"; +import { FUSION_CLIENT_HEADER, resolveHttpDeleteCallerKind } from "@fusion/core"; import { ApiError, badRequest, conflict, notFound } from "../api-error.js"; +// FNXC:TaskLookup404 2026-07-26-11:40: shared task-miss -> 404 mapping seam. +import { isTaskLookupMiss, rethrowTaskApiError } from "./task-lookup-error.js"; import type { ApiRoutesContext } from "./types.js"; import { deriveAutoTaskBranch, derivePerTaskBranch, getBranchSelectionMode, resolveBranchSelection } from "./branch-selection.js"; import { isDaemonAuthActive } from "../auth-middleware.js"; @@ -1750,6 +1753,11 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } + // FNXC:TaskLookup404 2026-07-26-11:45: moving an unknown task id is a 404, + // not a 500 — classify the miss before the transition-rejection mapping. + if (isTaskLookupMiss(err)) { + rethrowTaskApiError(err, req.params.id); + } // Flag-ON typed rejections surface as a structured 409 so the board can // resolve the i18n messageKey and decide snap-back vs no-move (U9/R17). // Flag-OFF legacy errors are unchanged (the legacy strings below). @@ -1834,7 +1842,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork retryable: err.rejection.retryable, }); } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -2453,7 +2461,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork const status = err.code === "dirty-working-tree" || err.code === "branch-mismatch" ? 409 : 500; throw new ApiError(status, err.message, { code: err.code }); } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -2676,7 +2684,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -2798,7 +2806,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -2813,7 +2821,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -2840,7 +2848,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 + const status = isTaskLookupMiss(errorWithCode) ? 404 : (err instanceof Error ? err.message : String(err)).includes("must be in 'done' or 'in-review'") ? 400 : (err instanceof Error ? err.message : String(err)).includes("Feedback is required") ? 400 : 500; @@ -3003,7 +3011,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT" || (err instanceof Error ? err.message : String(err)).includes("not found")) { + if (isTaskLookupMiss(err) || (err instanceof Error ? err.message : String(err)).includes("not found")) { throw notFound(`Task ${taskId} not found`); } throw err; @@ -3129,7 +3137,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound("Attachment not found"); } else { rethrowAsApiError(err); @@ -3147,7 +3155,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound("Attachment not found"); } else { rethrowAsApiError(err); @@ -3194,7 +3202,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } else { rethrowAsApiError(err); @@ -3216,7 +3224,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } else { rethrowAsApiError(err, "Internal server error"); @@ -3287,7 +3295,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } else { rethrowAsApiError(err); @@ -3356,7 +3364,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork res.json(await scopedStore.getTaskVerificationRequestAsync(req.params.id)); } catch (err: unknown) { if (err instanceof ApiError) throw err; - rethrowAsApiError(err, "Failed to read task verification status"); + rethrowTaskApiError(err, req.params.id, "Failed to read task verification status"); } }); @@ -3393,10 +3401,19 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - // ENOENT means the task directory/file genuinely doesn't exist → 404. - // Any other error (e.g. JSON parse failure from a concurrent partial write, - // or a transient FS error) should surface as 500 so clients can retry. - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + /* + FNXC:TaskLookup404 2026-07-26-11:55 (supersedes the ENOENT-only note): + A task that genuinely does not exist → 404; any other error (JSON parse + failure from a concurrent partial write, transient FS error) → 500 so + clients can retry. + + The previous check was `code === "ENOENT"` alone, a file-backed-storage-era + leftover. In Postgres/backend mode nothing on the task read path sets an + errno code, so EVERY unknown/missing/soft-deleted task id fell through to + 500. `isTaskLookupMiss` matches the typed `TaskNotFoundError` from + `@fusion/core` first and keeps ENOENT as a legacy fallback. + */ + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } else { rethrowAsApiError(err, "Internal server error"); @@ -3419,7 +3436,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -3434,7 +3451,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -3468,7 +3485,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -3485,7 +3502,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -3502,7 +3519,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -3525,7 +3542,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -3553,7 +3570,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -3613,7 +3630,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -3660,7 +3677,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -3721,7 +3738,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -3783,7 +3800,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -3798,7 +3815,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -3845,7 +3862,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -3867,7 +3884,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 + const status = isTaskLookupMiss(errorWithCode) ? 404 : (err instanceof Error ? err.message : String(err)).includes("not found") ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); @@ -3884,7 +3901,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 + const status = isTaskLookupMiss(errorWithCode) ? 404 : (err instanceof Error ? err.message : String(err)).includes("not found") ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); @@ -3907,7 +3924,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -3926,7 +3943,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -4005,7 +4022,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw new ApiError(409, err.message, { ...err.toDetails() }); } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -4411,7 +4428,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 : 500; + const status = isTaskLookupMiss(errorWithCode) ? 404 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } }); @@ -4507,7 +4524,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 + const status = isTaskLookupMiss(errorWithCode) ? 404 : (err instanceof Error ? err.message : String(err)).includes("Invalid transition") ? 400 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); @@ -4578,7 +4595,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork throw err; } const errorWithCode = err as NodeJS.ErrnoException; - const status = errorWithCode.code === "ENOENT" ? 404 + const status = isTaskLookupMiss(errorWithCode) ? 404 : (err instanceof Error ? err.message : String(err)).includes("Invalid transition") ? 400 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); @@ -5055,6 +5072,16 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } + /* + FNXC:TaskLookup404 2026-07-26-11:45: + PATCH pre-checks the row with getTask, so an unknown id reaches this catch. + Classify the miss as 404 BEFORE the 400-vs-500 message classifier — that + classifier only recognises validation strings, so a missing task fell + through to 500. + */ + if (isTaskLookupMiss(err)) { + rethrowTaskApiError(err, req.params.id); + } const status = (err instanceof Error ? err.message : String(err)).includes("must be a string") || (err instanceof Error ? err.message : String(err)).includes("must be a non-empty string") || (err instanceof Error ? err.message : String(err)).includes("must be a string or null") || (err instanceof Error ? err.message : String(err)).includes("must be an array of strings") || (err instanceof Error ? err.message : String(err)).includes("must be a boolean") || (err instanceof Error ? err.message : String(err)).includes("thinkingLevel must be one of") || (err instanceof Error ? err.message : String(err)).includes("validatorThinkingLevel must be one of") || (err instanceof Error ? err.message : String(err)).includes("planningThinkingLevel must be one of") || (err instanceof Error ? err.message : String(err)).includes("reviewLevel must be an integer") || (err instanceof Error ? err.message : String(err)).includes("executionMode must be one of") || (err instanceof Error ? err.message : String(err)).includes("priority must be one of") || (err instanceof Error ? err.message : String(err)).includes("sourceIssue") || (err instanceof Error ? err.message : String(err)).includes("gitlabTracking") || (err instanceof Error ? err.message : String(err)).includes("status may only be cleared") ? 400 : 500; throw new ApiError(status, err instanceof Error ? err.message : String(err)); } @@ -5109,7 +5136,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT" || (err instanceof Error ? err.message : String(err)).includes("not found")) { + if (isTaskLookupMiss(err) || (err instanceof Error ? err.message : String(err)).includes("not found")) { throw notFound(err instanceof Error ? err.message : String(err)); } else { rethrowAsApiError(err); @@ -5147,7 +5174,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT" || (err instanceof Error ? err.message : String(err)).includes("not found")) { + if (isTaskLookupMiss(err) || (err instanceof Error ? err.message : String(err)).includes("not found")) { throw notFound(err instanceof Error ? err.message : String(err)); } else { rethrowAsApiError(err); @@ -5195,7 +5222,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT" || (err instanceof Error ? err.message : String(err)).includes("not found")) { + if (isTaskLookupMiss(err) || (err instanceof Error ? err.message : String(err)).includes("not found")) { throw notFound(err instanceof Error ? err.message : String(err)); } rethrowAsApiError(err); @@ -5217,7 +5244,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT" || (err instanceof Error ? err.message : String(err)).includes("not found")) { + if (isTaskLookupMiss(err) || (err instanceof Error ? err.message : String(err)).includes("not found")) { throw notFound(err instanceof Error ? err.message : String(err)); } else { rethrowAsApiError(err); @@ -5247,7 +5274,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork res.json(reviewData); } catch (err: unknown) { if (err instanceof ApiError) throw err; - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err); @@ -5274,7 +5301,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork res.json(reviewData); } catch (err: unknown) { if (err instanceof ApiError) throw err; - if ((err as NodeJS.ErrnoException).code === "ENOENT") { + if (isTaskLookupMiss(err)) { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err); @@ -5447,7 +5474,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -5514,7 +5541,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -5535,7 +5562,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - if ((err as NodeJS.ErrnoException).code === "ENOENT" || (err instanceof Error ? err.message : String(err)).includes("not found")) { + if (isTaskLookupMiss(err) || (err instanceof Error ? err.message : String(err)).includes("not found")) { throw notFound(err instanceof Error ? err.message : String(err)); } else { rethrowAsApiError(err); @@ -5582,7 +5609,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if ((err instanceof Error ? err.message : String(err)).includes("not found")) { throw notFound(err instanceof Error ? err.message : String(err)); } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -5640,7 +5667,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if ((err instanceof Error ? err.message : String(err)).includes("not found")) { throw notFound(err instanceof Error ? err.message : String(err)); } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -5661,7 +5688,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (err instanceof ApiError) { throw err; } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); @@ -5690,8 +5717,21 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork allowResurrection, githubIssueAction, auditContext: { + /* + FNXC:TaskDeleteAttribution 2026-07-26-14:30: + This handler used to hardcode `agentId:"system"` with no caller field, so an operator + clicking Delete in the dashboard and any script or agent calling the same endpoint wrote + byte-identical audit rows — which is why a four-delete incident could not be attributed. + `callerKind` now records what the client SAID it was. + + This is attribution, not authentication: `x-fusion-client` is self-reported and anything + can send it. A row therefore distinguishes "the client identified itself as the dashboard + UI" from "nothing identified itself" (`api-unattributed`, the default for absent or + unrecognized values). Do not gate deletes or permissions on it. + */ agentId: "system", runId: `synthetic-dashboard-delete-${req.params.id}-${Date.now()}`, + callerKind: resolveHttpDeleteCallerKind(req.get(FUSION_CLIENT_HEADER)), }, }); scheduleReleaseExecutionAgentBindings(engine, req.params.id, runtimeLogger); @@ -5728,7 +5768,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork }); } - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.id); } }); diff --git a/packages/dashboard/src/routes/register-workflow-routes.ts b/packages/dashboard/src/routes/register-workflow-routes.ts index 4f278a8813..97533af792 100644 --- a/packages/dashboard/src/routes/register-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-workflow-routes.ts @@ -2,6 +2,8 @@ import type { WorkflowDefinition, WorkflowDefinitionKind, WorkflowIr, WorkflowIr import { ColumnTraitValidationError, OccupiedColumnsError, InvalidRehomeTargetError, WorkflowIrError, ColumnAgentBindingError, WorkflowSettingRejectionError, SCHEMA_VERSION, assertColumnTraitsValid, layoutForIr, listTraits, listStepParsers, parseWorkflowIr, resolvePlanningSettingsModel, stripApprovalBypassFlags, resolveWorkflowIrById, resolveEffectiveSettingValues, findOrphanedSettingValues, isBuiltinWorkflowId, getBuiltinWorkflow, BUILTIN_WORKFLOW_SETTINGS, AgentStore, validateColumnAgentBindings, resolveWorkflowOptionalSteps, enumeratePromptBearingWorkflowNodes, normalizeWorkflowIcon } from "@fusion/core"; import { buildSessionSkillContextSync, createFnAgent as engineCreateFnAgent, validateCodeNodeSources, validateWorkflowIrDryRun } from "@fusion/engine"; import { ApiError, badRequest, conflict, notFound, rateLimited } from "../api-error.js"; +// FNXC:TaskLookup404 2026-07-26-11:40: shared task-miss -> 404 mapping seam. +import { rethrowTaskApiError } from "./task-lookup-error.js"; import { emitWorkflowSseEvent } from "../sse.js"; import type { ApiRoutesContext } from "./types.js"; @@ -687,7 +689,7 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void { res.json({ approved: command }); } catch (err: unknown) { if (err instanceof ApiError) throw err; - rethrowAsApiError(err); + rethrowTaskApiError(err, req.params.taskId); } }); diff --git a/packages/dashboard/src/routes/task-lookup-error.ts b/packages/dashboard/src/routes/task-lookup-error.ts new file mode 100644 index 0000000000..5ce5ca22e2 --- /dev/null +++ b/packages/dashboard/src/routes/task-lookup-error.ts @@ -0,0 +1,77 @@ +/** + * FNXC:TaskLookup404 2026-07-26-11:35: + * Requirement: a task read for a task that does not exist must surface as HTTP + * 404 across EVERY per-task route, never 500 — clients (task detail view, the + * PR/review/diff panels, CLI callers) must be able to tell "this task is gone" + * apart from "the server is broken", and only the latter is worth retrying or + * paging on. + * + * Why this module exists: route catches used to detect a missing task solely by + * `(err as NodeJS.ErrnoException).code === "ENOENT"`. That was a leftover from + * the file-backed storage era. In Postgres/backend mode nothing on the task read + * path sets an errno code, so every missing / unknown / soft-deleted / + * wrong-project task read fell through to 500 (reported repro: + * `GET /api/tasks/FN-8610/runtime-fallback` returned 500 with the body + * `{"error":"Task FN-8610 not found"}` — a 404 wearing a 500 status). + * + * The fix is the typed `TaskNotFoundError` thrown by `getTaskImpl` in + * `@fusion/core`. This module is the single mapping seam every route catch + * shares, so the ~40 affected handlers cannot drift apart again (AGENTS.md + * "Reuse Components, Design Tokens, and Systems" / "Fix the Invariant, Not the + * Repro"). The legacy ENOENT test is deliberately retained as a second, harmless + * signal so genuinely file-backed reads (attachments, session files, worktree + * files) keep their 404 behavior. + */ +import { isTaskNotFoundError } from "@fusion/core"; +import { ApiError, notFound, rethrowAsApiError } from "../api-error.js"; + +/** + * FNXC:TaskLookup404 2026-07-26-11:35: + * True when `error` means "the task/file being read does not exist". Primary + * signal is the typed `TaskNotFoundError`; the errno `ENOENT` check stays for + * file-backed reads on the same handlers. + */ +export function isTaskLookupMiss(error: unknown): boolean { + if (isTaskNotFoundError(error)) return true; + return (error as NodeJS.ErrnoException | undefined)?.code === "ENOENT"; +} + +/** + * FNXC:TaskLookup404 2026-07-26-11:35: + * Status selector for the `const status = ... ? 404 : 500` catch shape. Returns + * 404 for a task/file miss, otherwise `fallbackStatus` (500 unless the handler + * has its own more specific classification). + */ +export function taskLookupStatus(error: unknown, fallbackStatus = 500): number { + return isTaskLookupMiss(error) ? 404 : fallbackStatus; +} + +/** + * FNXC:TaskLookup404 2026-07-26-11:35: + * Drop-in replacement for `rethrowAsApiError` on any handler whose call path can + * reach `store.getTask(...)`. Preserves an already-typed `ApiError`, maps a task + * miss to 404, and defers everything else to the existing 500 behavior. + * + * The 404 message reuses the thrown error's own message (byte-identical to the + * legacy `Task ${id} not found` string) so response bodies are unchanged for + * clients that match on it; `taskId` is only a fallback when the error carried + * no message. + */ +export function rethrowTaskApiError( + error: unknown, + taskId?: string, + fallbackMessage?: string, +): never { + if (error instanceof ApiError) { + throw error; + } + if (isTaskLookupMiss(error)) { + const message = error instanceof Error && error.message + ? error.message + : taskId + ? `Task ${taskId} not found` + : "Task not found"; + throw notFound(message); + } + rethrowAsApiError(error, fallbackMessage); +} diff --git a/packages/engine/src/runtimes/in-process-runtime.ts b/packages/engine/src/runtimes/in-process-runtime.ts index e15c7ece81..a16ad7699e 100644 --- a/packages/engine/src/runtimes/in-process-runtime.ts +++ b/packages/engine/src/runtimes/in-process-runtime.ts @@ -352,6 +352,8 @@ export class InProcessRuntime private workflowContinuationTimer?: ReturnType; private workflowContinuationDrainActive = false; private messageStore?: MessageStore; + /** FNXC:TaskDeleteNotice 2026-07-26-16:10: identity-guarded teardown for the delete-notice mailbox seam. */ + private unregisterTaskDeleteNoticeMailbox?: () => void; private chatStore?: ChatStore; private detachAgentLinkSync?: () => void; private concurrencyChangedListener?: (state: { globalMaxConcurrent: number }) => void; @@ -430,6 +432,7 @@ export class InProcessRuntime // the engine owns the result's shutdown() for process teardown. createTaskStoreForBackend, createProjectScopedPluginMcpProvider, + registerTaskDeleteNoticeMailbox, } = await import("@fusion/core"); if (this.config.externalTaskStore) { this.taskStore = this.config.externalTaskStore; @@ -487,6 +490,18 @@ export class InProcessRuntime this.messageStore = new MessageStoreClass(null, { asyncLayer: messageLayer }); + /* + FNXC:TaskDeleteNotice 2026-07-26-16:10: + Core owns the delete path but has no mailbox, so it exposes a store-scoped seam and the + runtime supplies the MessageStore. Registering here (rather than process-globally) keeps one + project's "a task was deleted by someone who is not you" notice out of another project's + inbox. A store with no registration degrades to no notice — never to a failed delete. + */ + this.unregisterTaskDeleteNoticeMailbox = registerTaskDeleteNoticeMailbox( + this.taskStore, + this.messageStore, + ); + await yieldEventLoop(); // 2. Initialize Plugin system (PluginStore + PluginLoader + PluginRunner) @@ -1469,6 +1484,10 @@ export class InProcessRuntime */ const backendShutdown = this.backendShutdown; this.backendShutdown = undefined; + // FNXC:TaskDeleteNotice 2026-07-26-16:10: drop the mailbox seam first so a stopping runtime + // cannot keep writing notices; the unregister is identity-guarded against a newer runtime. + this.unregisterTaskDeleteNoticeMailbox?.(); + this.unregisterTaskDeleteNoticeMailbox = undefined; let stopError: Error | undefined; try { if (this.workflowContinuationTimer) { diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 6531af8076..1d757fedaa 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -12784,7 +12784,7 @@ export class SelfHealingManager { continue; } if (resolution === "delete") { - await this.store.deleteTask(task.id, { removeLineageReferences: true, auditContext: { agentId: "self-healing", runId: generateSyntheticRunId("self-heal-explicit-duplicate", task.id) } }); + await this.store.deleteTask(task.id, { removeLineageReferences: true, auditContext: { agentId: "self-healing", runId: generateSyntheticRunId("self-heal-explicit-duplicate", task.id), callerKind: "engine" } }); } else if (resolution === "prompt") { await flagTriageDuplicate(this.store, task.id, canonicalTask.id); await this.store.updateTask(task.id, { paused: true, pausedReason: "duplicate-decision-required", status: null }); diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index bfc8acdede..e2136bbaf4 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -2295,8 +2295,11 @@ export class TriageProcessor { await this.store.deleteTask(task.id, { removeLineageReferences: true, auditContext: { + // FNXC:TaskDeleteAttribution 2026-07-26-14:30: labelling only — this + // split-close delete is intended engine behavior and is unchanged. agentId: task.assignedAgentId ?? "triage", runId: generateSyntheticRunId("triage-delete", task.id), + callerKind: "engine", }, }); planLog.log(`✓ ${task.id} split into subtasks (${childTaskIds}) and closed`); @@ -3311,7 +3314,8 @@ export class TriageProcessor { if (typeof deleteTaskIf !== "function") return; const result = await deleteTaskIf.call(this.store, task.id, isTaskStillInPlanningStage, { removeLineageReferences: true, - auditContext: { agentId: task.assignedAgentId ?? "triage", runId: generateSyntheticRunId("triage-delete", task.id) }, + // FNXC:TaskDeleteAttribution 2026-07-26-14:30: duplicate-resolution delete is engine-driven. + auditContext: { agentId: task.assignedAgentId ?? "triage", runId: generateSyntheticRunId("triage-delete", task.id), callerKind: "engine" }, }); if (!result.deleted) return; await this.store.recordActivity({