Skip to content

Preserve cancellation event payload identifiers in duroxide.history - #180

Closed
Pino de Candia (pinodeca) with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-cancellation-failure-event
Closed

Preserve cancellation event payload identifiers in duroxide.history#180
Pino de Candia (pinodeca) with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-cancellation-failure-event

Conversation

Copilot AI commented May 27, 2026

Copy link
Copy Markdown
Contributor

Cancellation terminal events could be persisted with event_data.instance_id = "" and event_data.execution_id = 0, even when the owning duroxide.history row had correct identifiers. This broke payload-only consumers that correlate events without joining back to history keys.

  • Runtime payload normalization at history write boundary

    • Added a BGW-installed trigger function (duroxide._normalize_history_event_payload_identifiers) and BEFORE INSERT trigger on duroxide.history.
    • On insert, if event_data is valid JSON object:
      • fills instance_id when missing/empty from NEW.instance_id
      • fills execution_id when missing/non-numeric/zero from NEW.execution_id
    • Leaves non-JSON or non-object payloads unchanged.
  • Deterministic guard installation during worker initialization

    • Worker startup now ensures the normalization function/trigger exist before runtime processing continues.
    • Trigger creation is idempotent (DROP TRIGGER IF EXISTS + CREATE TRIGGER).
  • Regression coverage for cancellation path

    • Added tests/e2e/sql/23_cancel_history_payload_ids.sql to verify cancelled-instance history payload identifiers match row identifiers for events carrying those fields.
IF COALESCE(v_data->>'instance_id', '') = '' THEN
  v_data := JSONB_SET(v_data, '{instance_id}', TO_JSONB(NEW.instance_id), true);
END IF;

IF v_execution_text IS NULL OR v_execution_text !~ '^[0-9]+$' OR v_execution_text::BIGINT = 0 THEN
  v_data := JSONB_SET(v_data, '{execution_id}', TO_JSONB(NEW.execution_id), true);
END IF;

Copilot AI and others added 2 commits May 27, 2026 14:26
Co-authored-by: pinodeca <32303022+pinodeca@users.noreply.github.com>
Co-authored-by: pinodeca <32303022+pinodeca@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix cancellation failure event payload to include instance_id Preserve cancellation event payload identifiers in duroxide.history May 27, 2026
@pinodeca
Pino de Candia (pinodeca) marked this pull request as ready for review June 11, 2026 00:47

Copy link
Copy Markdown
Contributor

Root cause is upstream in duroxide, now tracked as microsoft/duroxide#35.

The empty instance_id/execution_id=0 originate in duroxide core: the terminal OrchestrationFailed event is built by HistoryManager::append_failed (src/runtime/state_helpers.rs), which hardcodes placeholder identifiers ("" / 0) that the two execution.rs call sites never set. This affects every OrchestrationFailed terminal event (both the failure and cancellation paths) and is provider-agnostic (reproduces on SQLite and Postgres). duroxide-pg just serializes and stores the event verbatim, so the history row columns are correct while the embedded JSON payload is not.

Because of that, the BEFORE INSERT trigger here patches the symptom in the wrong layer: it runs on the duroxide.history hot path (every event insert, not just terminal cancel/fail) and parses + re-serializes JSONB each time, to fix a field that only the upstream runtime should be setting. There is also some replay-correctness risk in mutating event_data that duroxide writes and reads back.

Recommendation: close this PR in favor of the upstream fix (microsoft/duroxide#35). Once duroxide ships the append_failed fix and the duroxide/duroxide-pg pair is bumped, the only piece worth keeping is the e2e regression test (tests/e2e/sql/23_cancel_history_payload_ids.sql), ideally broadened to also cover the plain-failure path.

Tracking issue: #169

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.

Cancellation failure history event has empty instance_id and execution_id=0 in payload

2 participants