Business rejections answer 'failed': 'rejected' is unwritable and the Postgres CHECK forbids it

[LukasGoTom]

Verified against the pinned vendor/adcp-sdk
(@adcp/sdk@14.0.0-beta.12, AdCP 3.2.0-beta.6). SDK issue, not a protocol
one — the spec already models this correctly.

Title

Business rejections answer 'failed': 'rejected' is unwritable and the Postgres CHECK forbids it

Body

Three layers of the SDK disagree about whether rejected exists.

• decisioning/runtime/task-registry.ts:45, :63 — rejected is a documented
  terminal status carrying a result artifact.
• task-registry.ts:247, postgres-task-registry.ts:322 — the only terminal
  writers are complete() and fail(), and fail() hardcodes failed.
  Nothing writes rejected.
• postgres-task-registry.ts:174 — CHECK allows only
  submitted, working, completed, failed (4 of the spec's 9), so an adopter
  willing to write the record directly still cannot.
• enums/task-status.json — failed is "an error during execution",
  rejected is a structured decline, "not a transport failure".

Net effect: an adopter declining a well-formed request from a handoffToTask
function answers failed on the wire, which is the wrong spec status. Buyers
cannot tell an execution error from a business decision.

Adopter context

We are a sales-guaranteed seller whose media buy waits on a human converting
an offer into a booking, which is the case submitted is defined for
("long-running execution (hours to days)"). When that offer is archived or
expires, the correct terminal answer is rejected — and we cannot emit it.

Our workaround is a custom TaskRegistry via opts.taskRegistry
(from-platform.ts:2525) that resolves status on read, which works because
get_task_status reads only through taskRegistry.getTask()
(create-adcp-server.ts:7464). It means we cannot use the shipped
createPostgresTaskRegistry, and list_tasks has to be wrapped too or it
serves stale rows.

Related

task-registry.ts:31 and postgres-task-registry.ts:171 point adopters at a
taskRegistry.transition() API "(v6.1)" for exactly these states. ## 6.1.0
is at CHANGELOG.md:10497 and the package is at 14.0.0-beta.12; transition
appears nowhere in src/ outside those two comments. Happy to file that
separately if you'd rather keep this issue single-concern.

Ask

1. Widen the Postgres CHECK to the full nine-state enum, with the migration.
2. Add a way to end a task as rejected — a reject() sibling of fail(), or
   fail() honouring a business-decline marker on AdcpError.

[bokelley]

Triage

Classification: Bug
Bucket(s): library
Status: ready-to-implement

What the experts said:

• code-reviewer: CHECK widening is non-breaking additive DDL; should widen to all 9 TaskStatus values at once (not just rejected) to avoid a second migration when transition() ships; reject() must be reject?() (optional) on the interface to stay minor; needs getDecisioningTaskRegistryStatusWidenV61Migration() helper + rejectScopedTask() free function.
• dx-expert: reject() sibling is clearly right — the type system already teaches the correct model, only the write path is missing; fail() JSDoc must warn adopters to use reject() for business declines; Postgres migration must ship in the same release.

My take: All claims verified — TaskStatus/TERMINAL_TASK_STATUSES already include rejected, but fail() hardcodes 'failed' and the Postgres CHECK blocks any other terminal write. Both asks are additive and must ship together.

• task-registry.ts — add optional reject?(taskId, scope, error: AdcpStructuredError, result?): Promise<TaskRegistryMutationResult> to TaskRegistry interface; impl in createInMemoryTaskRegistry() setting status = 'rejected'; rejectScopedTask() free function (call ensureWorkerSettlementIsSafe, mirror completeScopedTask/failScopedTask); add JSDoc on fail() directing business declines to reject().
• postgres-task-registry.ts — add getDecisioningTaskRegistryStatusWidenV61Migration() helper (ALTER TABLE DROP/ADD CONSTRAINT for all 9 TaskStatus values, with lockTimeoutMs safety valve, parallel to getDecisioningTaskRegistryScopeV1Upgrade); widen getDecisioningTaskRegistryBootstrap() CHECK to all 9 values; add reject() impl.
• Tests — both registries: reject() → applied; idempotent second call → already_terminal; complete()/fail()/updateProgress() on rejected task → already_terminal; rejectScopedTask() all three outcome paths.
• Changeset — minor; npm run ci:quick + npm run ci:schema-check (public types touched).

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01WuJekz4DaNsKqmZG6h1kau

---

Generated by Claude Code

[bokelley]

Thanks for filing! This is a well-evidenced report with precise line references.

Triage

Classification: Bug
Bucket(s): library
Status: ready-to-implement

What the experts said:

• code-reviewer: Confirmed completely — fail() hardcodes 'failed' at in-memory line 405 and Postgres line 762; Postgres DDL CHECK (lines 234-243) allows only 4 of 9 spec states; transition() referenced in comments does not exist in source. DDL migration must ship in the same PR or reject() becomes a silent prod landmine (compiles and tests pass, throws at runtime on Postgres).
• dx-expert: Blocker for sales-guaranteed / HITL adopters — this is the primary use case of the specialism and the workaround abandons createPostgresTaskRegistry entirely, losing idempotency, scope checks, and DDL management. Rated reject() > transition() on ergonomics: typed, self-documenting, mirrors fail(). The dead taskRegistry.transition() (v6.1) comment should be removed when reject() lands.

My take: Both experts converge. The fix is non-breaking (optional on the TaskRegistry interface) and clearly scoped. The DDL migration is inseparable from the method addition — they must ship together.

Implementation scope:

• task-registry.ts — add reject?(taskId, scope, opts: { message: string; result?: unknown }): Promise<TaskRegistryMutationResult> (optional ? for back-compat with existing custom registries); remove the dead v6.1 transition() comment
• In-memory TaskRegistry impl — add reject() setting status = 'rejected'
• postgres-task-registry.ts — add reject() with SET status = 'rejected'; widen getDecisioningTaskRegistryBootstrap() CHECK to all 9 states; add a getDecisioningTaskRegistryRejectedStatusUpgrade() DDL helper (drops old CHECK, adds wider one — no row migration needed since no rejected rows can exist) following the getDecisioningTaskRegistryScopeV1Upgrade() pattern
• Add rejectScopedTask() worker-settlement helper alongside completeScopedTask() / failScopedTask()
• Migration guide entry noting the ALTER TABLE acquires an ACCESS EXCLUSIVE lock (instant on small tables)
• Changeset: minor — new optional method, new DDL helper, new scoped helper

Non-breaking rationale: reject? is optional on the interface, so custom registries compile without change. CHECK widening is purely additive. No removals or signature changes.

Required checks: npm run ci:quick; changeset required per repo policy.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01MHoVYuUCCThb4hCzxPF4Rn

---

Generated by Claude Code

[bokelley]

Triage

Classification: Bug
Bucket(s): library
Status: ready-for-human

What the experts said:

• code-reviewer: Required reject() on TaskRegistry is the only safe design — optional creates a silent no-op; blockers are CHECK constraint widening (bootstrap + new migration export), new Postgres SQL (cannot reuse fail()), and missing rejectScopedTask() helper; canceled has the same structural gap and should be decided together.
• dx-expert: Severe — failed causes buyer retry storms on deliberate human declines in a financial protocol; rejectScopedPushTask() is also missing from postgres-task-settlement.ts; fix rejected + canceled together to share one Postgres migration; the transition() comments at task-registry.ts:35 and postgres-task-registry.ts:238-241 misdirect agents to a nonexistent API and must be updated in the same PR.

My take: Both experts converge on one design — required reject(taskId, scope, reason?) with no AdcpError (rejection is not a failure). Adding a required method to the public TaskRegistry interface is breaking for custom implementors; the right call on a Preview API, but needs @bokelley sign-off before a PR opens. Open call: scope to rejected only, or rejected + canceled together (one migration, symmetric surface — experts recommend the latter).

Implementation scope when authorized:

• Add required reject(taskId, scope, reason?) to TaskRegistry interface; implement in both built-in registries with new SQL (cannot route through fail() which hardcodes 'failed')
• Widen Postgres CHECK to all 9 spec states in getDecisioningTaskRegistryBootstrap(); export idempotent getDecisioningTaskRegistryStatusV2Migration()
• Add rejectScopedTask() to task-registry.ts and rejectScopedPushTask() to postgres-task-settlement.ts
• Update dead transition() comments in both files; npm run changeset (minor, Preview-breaking)

---

Triaged by Claude Code. Session: https://claude.ai/code/session_014nRkSkJi7nuRsqJqnsHJnZ

---

Generated by Claude Code

[bokelley]

Thanks for filing!

Triage

Classification: Bug
Bucket(s): library
Status: ready-to-implement

What the experts said:

• code-reviewer: Bug confirmed — rejected is in TERMINAL_TASK_STATUSES (line 48) and TaskRecord.result JSDoc (line 139) but has no write path; fail() hardcodes 'failed' in both in-memory and Postgres implementations; the Postgres CHECK constraint was deliberately narrowed (comment at lines 235–241) pending a v6.1 taskRegistry.transition() API that was never shipped; fix is non-breaking (optional interface method + DDL migration helper, minor changeset)
• dx-expert: Confirmed transition() does not exist in src/; stale comments at task-registry.ts:31 and postgres-task-registry.ts:171 should be cleaned up in the same PR; docs/guides/POSTGRES.md:123 needs updating

My take: The gap is real and well-contained. A reject() sibling to fail() plus a widened Postgres CHECK (with a migration helper) covers both asks. The v6.1 transition() stub in comments is a cleanup target in the same PR — it's the dead skeleton of the original plan and misleads adopters.

Implementation scope:

• Add optional reject(taskId, scope, result?, reason?): Promise<TaskRegistryMutationResult> to TaskRegistry interface; implement in in-memory registry and Postgres registry (task-registry.ts, postgres-task-registry.ts)
• Widen Postgres CHECK to all 9 spec states; export a getDecisioningTaskRegistryStatusV2Upgrade migration helper (same module as getDecisioningTaskRegistryBootstrap)
• Add rejectScopedTask worker helper alongside completeScopedTask / failScopedTask
• Remove stale (v6.1) taskRegistry.transition() comments at task-registry.ts:31 and postgres-task-registry.ts:171
• Tests: in-memory + Postgres reject() transitions, already-terminal idempotency, migration helper exercise, tasks_get surfaces rejected to buyers; update the seed-by-mutation test at lines 470–490
• Update docs/guides/POSTGRES.md:123; changeset: minor

Non-breaking: adding reject() as optional leaves existing custom TaskRegistry implementations interface-complete without it; the Postgres DDL migration is additive (widens allowed values, removes nothing).

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01P7yZWhdZoGG42StvsZ4JQe

---

Generated by Claude Code