Skip to content

fix(workflows): reject an automation transition with no recorded cause - #792

Merged
arantespp merged 1 commit into
mainfrom
claude/issue-fix-alboqp
Jul 31, 2026
Merged

fix(workflows): reject an automation transition with no recorded cause#792
arantespp merged 1 commit into
mainfrom
claude/issue-fix-alboqp

Conversation

@arantespp

Copy link
Copy Markdown
Member

Summary

Fixes the bug reported in the "transition writer missing cause" report (split out of #791): an automation transition could be persisted with principal_id, generation_id, and orchestration_run_id all null — a task move with no recorded cause whatsoever.

Investigation

I audited every TaskTransition writer (tasksTransition.ts, tasks.ts, tasksApprovalGate.ts). The routeOnComplete / handleFailure paths in tasksAutomation.ts already never duplicate a run/generation id into principal_id — that was fixed by #786 before this repo's current state (confirmed by existing tests at tasks.test.ts asserting principal_id is null while generation_id/orchestration_run_id carry the cause). So the literal writer bug described in the report (duplicating an orchestration run id into the principal column) is already fixed on main.

What was not covered: if a dispatch fails before any generation/run id exists (e.g. an error thrown ahead of recordGenerationFailure), failedDispatchIds legitimately returns { generationId: null, orchestrationRunId: null }, and the on_failure transition was still silently written with zero provenance — exactly the "meaningless record" the report's suggested action #3 called out.

Fix

Added a defensive check (assertAutomationHasProvenance in tasksTransition.ts) that rejects — via a new TASK_AUTOMATION_PROVENANCE_MISSING (500) error — persisting any automation-kind transition where principal_id, generation_id, and orchestration_run_id would all be null. The write is atomic (inside the existing row-locked transaction), so a rejected write also rolls back the state-change side of the same transaction: the task stays in its current state, flagged automation_status: failed, instead of silently moving to a state whose history carries no cause.

Open Questions Gate

Q: Should the defensive check throw (loud failure, task stays parked) or silently skip firing on_failure while still marking automation_status?
A: Throw — resolved by long-term; checked: the issue's own suggested action #3 explicitly asks for a "loud failure" over a silently-recorded meaningless row, and automation_status already flags the task for board queries whether or not the transition fires, so throwing loses no visibility while fully preventing the corrupt write.

Tests

  • Updated a failed dispatch sets automation_status and follows on_failure to reject with a DomainError carrying generation_id in its meta, matching how createGeneration actually fails in production (a generation row always exists before the LLM call, so real failures always carry an id) — the previous plain-Error mock no longer exercises a realistic path now that the new guard exists.
  • Added a failed dispatch with no recoverable cause id never persists a provenance-less automation transition (#792): simulates a dispatch failure with zero metadata and asserts the task stays in its current state (automation_status: failed) instead of transitioning, and that no to_failed history row is written.

Docs

  • packages/website/docs/modules/workflows.md: added the new TASK_AUTOMATION_PROVENANCE_MISSING error code to the Error Codes table.

Verification

  • pnpm typecheck — passes.
  • pnpm eslint --fix on changed files — passes.
  • pnpm --filter @soat/server test --testPathPatterns=tasks.test.tscould not be run to completion in this sandbox: the integration suite requires a live PostgreSQL testcontainer, and Docker's daemon is unavailable in this environment (confirmed the same pre-existing failure occurs on an untouched file, e.g. users.test.ts, so it is an environment limitation, not something this change caused). CI's build-and-test job will run the full suite against the real Postgres container.

Generated by Claude Code

#792)

An automation-caused transition writes principal_id: null (#786) — the
cause lives in generation_id / orchestration_run_id instead. If a dispatch
fails before either id exists (e.g. an error thrown ahead of
recordGenerationFailure), the on_failure transition could still be
persisted with all three columns null: a transition history row with no
recorded cause at all. Reject the write instead of silently persisting it;
the task stays in its current state with automation_status: failed for a
human to investigate.

Audited every TaskTransition writer (tasksTransition.ts, tasks.ts,
tasksApprovalGate.ts) — the routeOnComplete/handleFailure paths already
never duplicate an orchestration run id into principal_id (fixed by #786
before this repo state), so this closes the remaining gap the issue's own
suggested action #3 called out.
@arantespp
arantespp enabled auto-merge (squash) July 31, 2026 14:23
@github-actions

Copy link
Copy Markdown

Deploy Outputs

Package Stack Output Key Output Value
@soat/website SoatWebsite-claude-issue-fix-alboqp BucketWebsiteURL http://soatwebsite-claude-issue-fix-alboqp-staticbucket-bmyr8xqgffbu.s3-website-us-east-1.amazonaws.com

@arantespp
arantespp merged commit c65bb2a into main Jul 31, 2026
6 checks passed
@arantespp
arantespp deleted the claude/issue-fix-alboqp branch July 31, 2026 14:30
arantespp added a commit that referenced this pull request Jul 31, 2026
Patch release 0.18.0 -> 0.18.1.

- fix(webhooks,triggers): encrypt signing secrets at rest (#793)
- fix(workflows): reject an automation transition with no recorded cause (#792)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants