From 87442b96648a2964b7684edb8ea3509b1083c2f9 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 10:32:12 -0700 Subject: [PATCH] test(engine): live-PG evidence that declared custom fields cannot be written (#2792) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What One new live-PostgreSQL E2E suite, 4 tests. **No production file is touched** — evidence, per the E2E worker's remit. Third in the series after #2789 and #2791; same root cause, materially worse consequence. `packages/engine/src/__tests__/workflow-custom-fields-sync-resolution-live-e2e.pg.test.ts` ## The finding **A workflow that declares custom fields cannot have any of them written.** `TaskStore.resolveTaskCustomFieldDefsSync` reads a task's field definitions through `store.resolveTaskWorkflowIrSync`, which under PostgreSQL answers `undefined` for every task and therefore resolves the **default** workflow IR. The default declares no `fields`, so the function returns `[]` for every task on every board. `task-update.ts` validates every write against that empty list: ```ts const defs = store.resolveTaskCustomFieldDefsSync(id); const result = validateCustomFieldPatch(defs, updates.customFields); if (!result.ok) throw new CustomFieldRejectionError(result.rejection); ``` Observed against a real store with a real persisted workflow declaring one `text` field: ``` STORED fields = [{"id":"risk","name":"Risk","type":"text"}] SYNC defs = [] WRITE threw = CustomFieldRejectionError custom field 'risk' rejected (no-fields-defined): the resolved workflow declares no custom fields; no values may be written ``` The rejection message is a true statement about the workflow that got resolved and a false one about the workflow the card is on. ### The two halves of the feature disagree in production The executor resolves the same definitions through the **async** resolver (`executor.ts` → `resolveTaskCustomFieldDefs` → `resolveWorkflowIrForTask`) and sees the real field. So an agent can be prompted to supply a value that the store will then refuse to store. The last case asserts both answers against **one store, one task, one workflow** — which is why this cannot be dismissed as a fixture artefact. This is a different severity from the previous two PRs in the series. #2789 and #2791 are wrong-lane defects, mostly latency, one of them unbounded. This one is a declared feature that does not function off the default board. ## Scope on record Three write paths share the sync resolver: `task-update.ts` (driven here), `workflow-task-create-ops.ts:394`, and `workflow-ops.ts:488`. Only the first is exercised; the other two are named in the file so the surface is recorded rather than implied. Also worth stating plainly: because the empty list *is* the default IR's `fields`, the same rejection is what a default-board card gets too. The feature is not merely renamed-board-broken. ## Evidence discipline - **Fixture integrity first.** The opening case asserts the stored workflow really does declare the field via the async resolver. Every other assertion is about a *missing* definition and would pass just as well against a workflow that never declared one — that case is what makes the rest mean something. - **Observed state.** The thrown typed rejection plus the **absence** of a persisted value on a re-read row. No spy on the validator. - **Mutation-verified.** Replacing the sync resolver's body with a hardcoded `[{id:"risk",…}]` fails **3 of 4** arms. The fourth is the fixture-integrity case, which exercises the async path by design and correctly survives. ## Not done, and why **No fix.** The async resolver already exists and is already used by the executor for the same data, so the shape of the fix is clear — but `task-update.ts`'s validation runs inside a synchronous update path, and making it async is a behaviour decision in `@fusion/core` that belongs to that file's owner, not to a smuggled edit in an evidence PR. The call-site allow-list entry for `task-store-helpers.ts` ("Synchronous helper shared by txn-hot paths") should cite this suite either way: the entry is accurate about the constraint and silent about the cost. ## Verification - new suite — **4/4 passed**, mutation-verified 3/4 (fourth by design) - full live-PG E2E surface, 20 suites — **125/125 passed** (121 on main + 4) - `pnpm lint` — clean Lane: `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- ...fields-sync-resolution-live-e2e.pg.test.ts | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 packages/engine/src/__tests__/workflow-custom-fields-sync-resolution-live-e2e.pg.test.ts diff --git a/packages/engine/src/__tests__/workflow-custom-fields-sync-resolution-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-custom-fields-sync-resolution-live-e2e.pg.test.ts new file mode 100644 index 0000000000..06a8c82658 --- /dev/null +++ b/packages/engine/src/__tests__/workflow-custom-fields-sync-resolution-live-e2e.pg.test.ts @@ -0,0 +1,134 @@ +/* +FNXC:WorkflowCustomFields 2026-07-30-22:40 (E2E evidence — custom fields are unwritable off the default board): + +This is the same inert-sync-resolution class as #2789 and #2791, but the consequence is not latency and +not a silently wrong id. It is a HARD WRITE REJECTION of a declared feature. + +`TaskStore.resolveTaskCustomFieldDefsSync` reads the task's field definitions through +`store.resolveTaskWorkflowIrSync`, which under PostgreSQL answers `undefined` for every task and so +resolves the DEFAULT workflow IR. The default declares no `fields`, so the function returns `[]` for +every task on every board. `task-update.ts` then validates every `customFields` write against that +empty list: + + const defs = store.resolveTaskCustomFieldDefsSync(id); + const result = validateCustomFieldPatch(defs, updates.customFields); + if (!result.ok) throw new CustomFieldRejectionError(result.rejection); + +so a workflow that DOES declare fields cannot have any of them written. The rejection reason is +`no-fields-defined`, and its message — "the resolved workflow declares no custom fields" — is a true +statement about the workflow that got resolved and a false one about the workflow the card is on. + +WHAT MAKES THIS WORTH ITS OWN FILE rather than a line in the ledger: the two halves of the feature +disagree with each other in production. The executor resolves the SAME definitions through the async +resolver (`executor.ts`'s `resolveTaskCustomFieldDefs` -> `resolveWorkflowIrForTask`) and sees the +real fields, so an agent can be prompted to supply a value that the store will then refuse to store. +The last case below asserts exactly that pair against one store and one task. + +THREE WRITE PATHS share the sync resolver — `task-update.ts` (the one driven here), +`workflow-task-create-ops.ts`, and `workflow-ops.ts`. Only the first is exercised; the other two are +named so the surface is on record rather than implied. See the PR body. + +OBSERVED STATE. The assertions are the thrown typed rejection and the ABSENCE of a persisted value on +a re-read row — not a spy on the validator. + +LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is +unaffected. Throwaway per-file database; never port 4040. +*/ +import { beforeAll, beforeEach, afterEach, afterAll, expect, it } from "vitest"; +import "@fusion/core"; // registers the built-in column traits +import { resolveWorkflowIrForTask, type TaskStore } from "@fusion/core"; + +import { + pgDescribe, + createSharedPgTaskStoreTestHarness, + type SharedPgTaskStoreHarness, +} from "../../../core/src/__test-utils__/pg-test-harness.js"; + +import { DEFAULT_VOCAB, lifecycleIr } from "./_workflow-vocabulary-fixture.js"; + +/** One declared field. `text` keeps the validator's type rules out of the subject. */ +const RISK_FIELD = { id: "risk", name: "Risk", type: "text" } as const; + +pgDescribe("custom field definitions resolved for a live task", () => { + const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ + prefix: "fusion_custom_fields", + }); + + beforeAll(h.beforeAll); + afterAll(h.afterAll); + beforeEach(async () => { await h.beforeEach(); }); + afterEach(async () => { await h.afterEach(); }); + + /** Persist a real workflow that DECLARES `risk`, and a task bound to it. + * + * `createWorkflowDefinition` allocates its own id and ignores one passed in, so the task binds to + * the id the STORE returned — binding to the requested id silently resolves the default builtin + * IR, which is a custom-field fixture that tests nothing. */ + async function taskOnFieldWorkflow(store: TaskStore, key: string): Promise { + const ir = { ...lifecycleIr(DEFAULT_VOCAB, `custom:${key}`), fields: [RISK_FIELD] }; + const created = await store.createWorkflowDefinition({ + name: `Fields ${key}`, + kind: "workflow", + ir, + } as never); + const workflowId = (created as { id: string }).id; + + const task = await store.createTask({ description: `field probe ${key}` }); + await store.writeTaskWorkflowSelection(task.id, workflowId, []); + store.taskCache.delete(task.id); + return task.id; + } + + it("the workflow definition really does declare the field (fixture integrity)", async () => { + /* First, because every assertion below is about a MISSING definition and would pass just as well + against a workflow that never declared one. */ + const store = h.store(); + const taskId = await taskOnFieldWorkflow(store, "wf-integrity"); + + const ir = await resolveWorkflowIrForTask(store, taskId); + + expect((ir as { fields?: unknown[] }).fields).toEqual([RISK_FIELD]); + }); + + it("CHARACTERIZATION — the sync resolver reports NO fields for that task", async () => { + const store = h.store(); + const taskId = await taskOnFieldWorkflow(store, "wf-sync-defs"); + + expect(store.resolveTaskCustomFieldDefsSync(taskId)).toEqual([]); + }); + + it("CHARACTERIZATION — so writing the declared field is REJECTED and nothing persists", async () => { + /* + The operator-visible failure. Not a wrong column id and not a missed wake: the write is refused + with a typed rejection whose reason is that no fields are defined, on a board that defines one. + */ + const store = h.store(); + const taskId = await taskOnFieldWorkflow(store, "wf-write"); + + await expect( + store.updateTask(taskId, { customFields: { risk: "high" } } as never), + ).rejects.toThrow(/no-fields-defined|declares no custom fields/); + + /* And the row is untouched — the rejection is not a partial write. */ + store.taskCache.delete(taskId); + const row = await store.getTask(taskId); + expect((row?.customFields as Record | undefined)?.risk).toBeUndefined(); + }); + + it("CHARACTERIZATION — the two halves of the feature disagree on the SAME task", async () => { + /* + The async resolver is the one the executor uses to decide what to ask an agent for + (`executor.ts` -> `resolveTaskCustomFieldDefs` -> `resolveWorkflowIrForTask`). It sees `risk`. + The sync resolver, which is what every write path validates against, sees nothing. One store, one + task, one workflow, two answers — which is why this cannot be dismissed as a fixture artefact. + */ + const store = h.store(); + const taskId = await taskOnFieldWorkflow(store, "wf-disagree"); + + const asyncIr = await resolveWorkflowIrForTask(store, taskId); + const asyncFields = (asyncIr as { fields?: { id: string }[] }).fields ?? []; + + expect(asyncFields.map((f) => f.id)).toEqual(["risk"]); + expect(store.resolveTaskCustomFieldDefsSync(taskId).map((f) => f.id)).toEqual([]); + }); +});