Skip to content

fix(examples): harden durable workflow finalization - #1109

Open
bokelley wants to merge 1 commit into
mainfrom
fix-durable-workflow-finalization
Open

fix(examples): harden durable workflow finalization#1109
bokelley wants to merge 1 commit into
mainfrom
fix-durable-workflow-finalization

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1107 after its merge raced the final expert review.

  • advertise request idempotency truthfully as unsupported until every mutating operation and durable handoff has operation-aware coverage
  • stage handler outcomes before registry publication so registry outages never rerun successful business work
  • renew live handler leases, separate handler attempts from registry retries, and keep transient lookup outages recoverable
  • preserve structured AdcpError payloads and retry only transient errors
  • scrub credential-bearing success results before durable staging and clear staged payloads after terminal reconciliation
  • propagate worker failures and finish cancellation before resource shutdown

Verification

  • make lint typecheck
  • reference seller suite: 85 passed, 2 deselected
  • PostgreSQL workflow queue suite: 14 passed
  • changed-file pre-commit hooks: all passed
  • full local CI before rebasing onto merged feat(examples): add durable seller production wiring #1107: 7489 passed, 42 skipped, 1 xfailed, 84.29% coverage

Expert review

Independent code, protocol, and security re-reviews completed with no remaining blockers.

await asyncio.wait_for(started.wait(), timeout=2)
processing.cancel()
with pytest.raises(asyncio.CancelledError):
await processing
stop_event=stop_event,
workflow_handler=AsyncMock(),
)
await stopper
staged_job = await staging
try:
await asyncio.shield(self._release(staged_job, "CancelledError"))
except WorkflowLeaseLostError:
await asyncio.gather(handler_task, heartbeat_task, return_exceptions=True)
try:
await asyncio.shield(self._release(job, "CancelledError"))
except WorkflowLeaseLostError:
except asyncio.CancelledError:
try:
await asyncio.shield(self._release(job, "CancelledError"))
except WorkflowLeaseLostError:
except asyncio.CancelledError:
try:
await asyncio.shield(self._release(job, "CancelledError"))
except WorkflowLeaseLostError:

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve.

No blocking findings. This PR (fix(examples): harden durable workflow finalization) is scoped to examples/v3_reference_seller, docs, and one PostgreSQL conformance suite — it does not touch the src/adcp public surface, so no semver, forward-compat, import-layering, generated-code, or ctx_metadata gates apply.

Checked against the decision table:

  • No critical/high findings (row 1 N/A).
  • gated_paths: false — row 2 does not fire despite review_decision: REVIEW_REQUIRED.
  • high_risk: false, no deletions in sensitive paths (rows 3–5 N/A).
  • No prior decision (row 6 N/A).
  • No no-auto-approve team match (row 7 N/A).
  • Zero medium findings (row 8 N/A).

Falls through to row 9 → approve. The reviewer verified the workflow_queue.py rewrite: stage-then-publish finalization, lease-token-guarded transitions with fail-closed WorkflowLeaseLostError, attempt-budget handling, credential scrub before JSONB staging, cancellation shielding, and transient-vs-terminal AdcpError routing — all backed by new conformance tests.

@bokelley
bokelley enabled auto-merge (squash) August 30, 2026 20:33
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.

1 participant