[Type Design · High] XOR invariant is unenforced. task_id (UUID, Celery) and queue_message_id (bigint, PG) are two nullable columns where, post-dispatch, exactly one should be set. That invariant lives only in prose (db_comment + the _record_dispatch_handle docstring) and in the happy-path helper — nothing stops another writer from setting both or neither. This is the same "illegal state is representable" shape that produced UN-3602.

Consider expressing it at the DB level so all writers are covered:

class Meta:
    constraints = [
        models.CheckConstraint(
            name="exec_task_xor_queue_msg",
            check=~(Q(task_id__isnull=False) & Q(queue_message_id__isnull=False)),
        ),
    ]

(Negated AND rather than strict XOR, since both are legitimately NULL pre-dispatch.) Cheapest change with the strongest guarantee.

Deferring this one, with reasoning. A CheckConstraint is added via an AddConstraint migration that performs a validating full-table scan of workflow_execution under a SHARE ROW EXCLUSIVE lock — exactly the large-table migration cost we chose the additive-nullable-column path to avoid (the UUIDField → CharField type-change alternative was rejected for the same reason). The invariant is already guaranteed by construction here: _record_dispatch_handle branches on transport and sets exactly one column, and it's the sole writer of queue_message_id. I'd rather not add a scanning migration on this hot table in this PR — happy to file it as a standalone constraint migration (or a NOT VALID + later VALIDATE) if you want the DB-level belt-and-suspenders.

[Silent Failure · Medium] int(dispatch_handle) can raise ValueError, now swallowed generically. This is the same failure class the PR fixes (a type mismatch on the PG handle), just relocated. If pg_enqueue_task ever returns a non-numeric string (version skew, future handle format, error sentinel), int() raises ValueError, which the outer except Exception (lines 667-672) catches and logs as the generic "Failed to record dispatch handle" — giving operators no hint the handle format was the problem, and the PG row silently loses its queue_message_id (cancellation/reaper later can't find the row).

Parse defensively and log the offending value:

if is_pg_transport(transport):
    try:
        msg_id = int(dispatch_handle)
    except (TypeError, ValueError):
        logger.error(
            f"[{org_schema}] PG dispatch handle {dispatch_handle!r} is not a "
            f"valid bigint msg_id for execution_id '{execution_id}'; "
            "queue_message_id not recorded"
        )
        return
    WorkflowExecutionServiceHelper.update_execution_queue_message_id(
        execution_id=execution_id, queue_message_id=msg_id
    )

Fixed in 52704e2 — defensive parse with a handle-specific logger.error (offending value included), so the generic outer guard no longer hides a malformed-handle cause. Same change addresses the greptile P2 on this line.

[Type Design · Medium] transport: str is stringly-typed at the internal helper boundary. A WorkflowTransport (str-Enum) already exists in unstract/core/.../data_models.py and is_pg_transport is the single-source predicate. With a bare str, a typo or garbage value reaches here and is_pg_transport("pg-queue") silently returns False, routing a PG handle to task_id — re-introducing exactly the bug class this PR closes. Wire-strings can stay at the edge, but normalize once and type this internal param as WorkflowTransport so the discriminant is a closed set.

Deferring, with reasoning. resolve_transport returns str (the WorkflowTransport.value), not the enum member, so annotating this internal param as WorkflowTransport would be inaccurate unless resolve_transport's return contract changes — which ripples to every caller and the task-payload wire shape (transport is carried as a plain string), out of scope for this fix. In practice the discriminant is already closed: the only caller passes resolve_transport's output, and is_pg_transport is the single normalization predicate. Happy to do the enum-normalization as a separate refactor of resolve_transport if you'd like that hardening.

[Type Design · Medium] Annotation/contract mismatch. The signature says queue_message_id: int, but the body (lines 437-442) guards if queue_message_id is None. The annotation lies — either it's int | None (matching the runtime guard) or the guard is dead code. Note the only caller, _record_dispatch_handle, already returns early on a falsy handle and always passes int(dispatch_handle), so the None branch is unreachable from the real path. Either make it int | None for honesty (mirrors the defensive update_execution_task), or drop the guard. Minor: the sibling guards if not task_id (catches 0/empty) while this guards is None only — queue_message_id=0 would slip through, though 0 is not a valid msg_id.

Fixed in 52704e2 — annotation is now int | None, matching the is None guard in the body and mirroring update_execution_task. (msg_id is a BigAutoField starting at 1, so the queue_message_id=0 slip-through is moot in practice.)

[Tests · High] The actual regression is untested. This inner try/except is the heart of the fix, but no test exercises it. The new tests verify the routing of _record_dispatch_handle in isolation (helper mocked); they do not verify the defense-in-depth guarantee that a raise here still lets execute_workflow_async fall through to the if timeout > -1: poll loop instead of short-circuiting to the post-dispatch handler that returns EXECUTING immediately.

Without this test, someone could move the recording back outside the try/except (or delete it) and every existing test would still pass while UN-3602 silently returns. Suggested: patch cls._record_dispatch_handle to raise, patch _dispatch_orchestrator_task / WorkflowExecution.objects.get / _get_execution_status, and assert the poll loop runs (i.e. status is polled) rather than an immediate bare EXECUTING. This is the one test that would actually catch a re-regression.

Fixed in 52704e2 — added tests/test_execute_workflow_async_wait.py. It patches _record_dispatch_handle to raise and asserts execute_workflow_async still enters the poll loop (_get_execution_status is called) rather than short-circuiting to an immediate EXECUTING. Moving the recording back outside the inner try/except (or deleting it) fails this test — exactly the re-regression guard you described.

[Tests + Comment · Low] Two small follow-ups for this file:

1. Scope the docstring. It describes the production crash (UUID coercion → ValueError → skipped wait) in detail, but these tests mock WorkflowExecutionServiceHelper and assert routing only — they never reproduce the ValueError or the sync-wait skip. A future maintainer could read this and assume the crash itself is covered, then delete the real guard. Add: "These tests assert routing only (PG→queue_message_id as int, Celery→task_id); the UUID-coercion crash and the timeout sync-wait are not exercised here (helper is mocked)."

2. Add a non-numeric-handle case. int(dispatch_handle) on the PG path raises ValueError for a malformed handle. A test_non_numeric_pg_handle asserting that behavior (or the graceful no-record after the fix in workflow_helper.py:547) pins the contract — today it's only protected by _dispatch_orchestrator_task always returning a numeric str(msg_id).

Both fixed in 52704e2 — added test_non_numeric_pg_handle_records_nothing (asserts the defensive parse records nothing and doesn't raise), and scoped the module docstring to "routing only (mocked); the UUID-coercion crash and the timeout sync-wait are covered in test_execute_workflow_async_wait.py".

[Comment · Low] Minor imprecision. "used to raise ValueError here" reads as if the throw originated at this call site, but the ValueError was actually raised inside update_execution_task → execution.save() (UUID coercion). Suggest: "...used to raise ValueError inside update_execution_task (UUID coercion on save), which bubbled to the post-dispatch handler..." Helps anyone grepping for the throw origin. (The comment is otherwise an excellent "why" comment — keep it.)

Fixed in 52704e2 — comment now reads "...used to raise ValueError inside update_execution_task (UUID coercion on save), which bubbled to the post-dispatch handler and silently skipped the wait...".