refactor: unify queue and workflow runtime#4
Merged
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
cmilesio
marked this pull request as ready for review
July 19, 2026 02:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This PR collapses the overlapping queue and workflow models into one queue-owned runtime.
queue.Queueis the canonical application surface,internal/workflow.Engineowns orchestration, drivers own physical delivery, andbusremains as a deprecated compatibility facade.Fresh review hardening
Eight fresh-context review passes were used as release gates. The sixth pass reduced its findings to two root invariants: native shutdown had to keep resources open for handler-admitted continuations, and SQL producers had to honor both exact historical and canonical
UniqueForclaims during transition. Both are fixed. A bounded follow-up found no production defect and tightened the documented limit for old high-level envelopes whose generated correlation changes the historical key. Remaining deferred boundaries stay explicit inplan.md.The seventh bounded pass found two inherited defects outside the architectural core: a handler could finish registering while an activating worker still lacked that job type, and the production guide named SQL recovery fields that do not exist on
queue.Config. Both are fixed in53d7539. A targeted fix-only verification then caught a draining-gate regression and a false-positive concurrency test in the first patch. Both were corrected before the release candidate was frozen, and the settled diff returned clean.The eighth bounded release-candidate audit split three new reviewers across API and architecture, runtime and lifecycle, and driver durability. It found one release-blocking inherited defect: the workflow SQL store used
sync.Onceto cache its first schema result, so caller cancellation, a transient database outage, a lock or permission error, or partial DDL could permanently poison that store instance. Initialization remains serialized, but only complete success is now cached. A canceled-first-use regression proves the same SQLite store retries, finishes migration, and persists state, and two fix-only reviewers returned clean.A post-review dependency audit identified GO-2026-5004 in pgx v5.8.0. The PostgreSQL driver and integration suite now use pgx v5.9.2, the first fixed release. pgx v5.9 requires Go 1.25, so only
driver/postgresqueueand the modules that compose it (examples,integration, and the repository workspace) move to Go 1.25. The root queue module, documentation module, and every non-PostgreSQL driver remain on Go 1.24.4.A checked dependency floor now applies to every direct pgx owner and rejects module or workspace replacements that could bypass it. The minimum-toolchain workflow derives its module cohorts from one manifest and compiles them with exact Go 1.24.4 and Go 1.25.0 toolchains. A patched Go 1.26.5
govulncheckpass reports no vulnerabilities in the PostgreSQL driver. pgx v5.9 also changes runtime behavior: it reduces prepared-statement protocol traffic, discards pooled connections returned with an open transaction during session reset, and uses the current operating-system user when the database user is omitted. Consumers should set the database user explicitly when they do not want that default.PostgreSQL security before and after
A separate changed-line audit by NATS, RabbitMQ, Redis, and documentation specialists added the one remaining deterministic production contract: a real NATS subscription failure must leave startup retryable. The other uncovered markers require impossible fixed-shape serialization errors, invalid dependency wiring, global entropy hooks, concrete-client transport fault injection, or artificial process timeouts, so no production-only test seams were added.
The first exact-head CI run also exposed an awk portability warning in the new release planner. The parser expressions are now portable across the local and GitHub runner implementations, and the release contract requires a successful inventory run to emit no parser diagnostics.
The fifth review found that managed SQL could report readiness and start inert workers against an absent or incomplete schema.
ReadyandStartWorkersnow share one read-only gate for both base-table relations and all runtime-used columns, and the gate survives the internal driver bridge. The all-backend evidence run then exposed that NATS rejected deadline-free readiness contexts; every Core NATS server roundtrip now receives a five-second bound while retaining any shorter caller deadline.The exact-head run for SHA
2ab5313passes all 26 CI checks. That includes both exact minimum-toolchain cohorts, all nine module race jobs plus their fan-in, all ten backend integration jobs, the full multi-module unit and vet pass, generated-document parity, and the complete coverage fan-in. The Codecov freshness gate confirms that both immutable reports belong to this base and head: 87.16% project coverage and 97.13% patch coverage, with 29 missing and 18 partial lines. Generated evidence records 970 unit and 631 integration test executions. A mutable GitHub coverage comment may render an older run, but it is not used as release evidence.Final lifecycle and SQL transition invariants
Handler activation during startup
Before, startup copied the logical handler registry, released the lifecycle lock, and then activated the backend. A concurrent registration could complete while the activating worker still lacked that type:
After, the worker generation is published and caught up while holding the same mutex used by
Register, then activated. Native runtimes use the same ordering against their fixed backend:flowchart LR REGISTER["Register handler"] --> LEDGER["Update logical registry under lifecycle mutex"] START["StartWorkers"] --> PUBLISH["Publish worker generation under same mutex"] LEDGER --> STATE{"Worker generation published?"} STATE -->|No| CATCHUP["Startup catches up the complete registry"] STATE -->|Yes| INSTALL["Register installs the handler physically"] PUBLISH --> CATCHUP CATCHUP --> ACTIVATE["Activate backend"] INSTALL --> RETURN["Register returns"] ACTIVATE --> CONSUME["Backend may consume"]Retryable workflow schema initialization
Workflow SQL schema initialization previously used
sync.Once, so its first error became the lifetime result of that store instance. The settled change keeps one initializer at a time but publishes readiness only after complete success:flowchart LR CALL["Store operation"] --> LOCK["Serialize schema initialization"] LOCK --> READY{"Schema ready?"} READY -->|Yes| USE["Run store operation"] READY -->|No| INIT["Run complete dialect initialization"] INIT --> RESULT{"Succeeded?"} RESULT -->|No| RETRY["Return current error and leave store retryable"] RETRY --> LATER["Later operation retries"] LATER --> LOCK RESULT -->|Yes| PUBLISH["Publish MySQL limits and schemaReady"] PUBLISH --> USEflowchart LR subgraph SHUTDOWN["Native shutdown"] STOP["Latch shutdown"] --> DRAIN["Drain workers"] DRAIN --> WAIT["Wait stable late-operation generation"] PERMIT["Handler continuation permit"] --> LATE["Admitted late dispatch"] LATE --> WAIT WAIT --> CLOSE["Close producer and storage resources"] end subgraph UNIQUE["SQL producer transition"] OLD["Legacy producer"] --> LEGACY["Exact historical physical key"] NEW["Current producer"] --> LEGACY NEW --> CANONICAL["Canonical logical key"] LEGACY --> TX["One acceptance transaction"] CANONICAL --> TX TX --> ROW["Exactly one queue row"] endA replacement decorator now changes application context behavior without discarding runtime authority:
Before:
After:
The important before-and-after changes from those reviews are concrete:
sync.OnceDisableAutoMigrateaccepted an empty schema and checked only two additive columns when tables existedReady(context.Background())reached a client flush that requires a deadline and failed before probing the serverprocess_failedfollowed bysettlement_failedcould decrementActivetwice or consume a newer duplicateActivelive and produce no failed-attempt telemetryprocess_failedis emitted, correlation state closes, and the original panic value is rethrown so backend semantics stay intactObserverFuncsignaturefunc(context.Context, queue.Event)and a guard pins their exact manual textGOWORK=off; an always-run fan-in preserves the historical requiredracecheckManaged SQL readiness and startup now share exactly one structural gate:
flowchart LR CALL["Ready or StartWorkers"] --> PING["Database ping"] PING --> MODE{"Automatic migration?"} MODE -->|Yes| AUTO["Ready returns after connectivity;<br/>startup owns migration"] MODE -->|No| TABLES["Require queue_jobs and<br/>queue_unique_locks base tables"] TABLES --> COLUMNS["Read one column snapshot<br/>per table"] COLUMNS --> COMPLETE{"All 18 runtime-used<br/>columns present?"} COMPLETE -->|Yes| READY["Ready or start workers"] COMPLETE -->|No| FAIL["Fail closed without DDL;<br/>same runtime may retry"]The release path now makes every mutation boundary explicit:
Settlement-aware metrics now have one unambiguous ownership rule:
flowchart TD START["process_started with opaque<br/>physical identity"] --> ACTIVE["Open exactly one Active execution"] TERMINAL{"Terminal fact"} ACTIVE --> TERMINAL TERMINAL -->|process success or failure<br/>with same identity| CLOSE["Close that exact execution"] TERMINAL -->|settlement failure<br/>with same identity| CLOSE TERMINAL -->|identity missing| KEEP["Leave identity-bearing Active unchanged"] CLOSE --> COUNTERS["Settlement failure changes neither<br/>Processed nor application Failed"] KEEP --> SAFE["Conservative overcount is safer than<br/>undercounting a newer live execution"]The nil path changed from forwarding an invalid target:
to preserving the existing registry exactly:
The reliability evidence now has an explicit provenance chain:
flowchart LR SRC["Integration and root tagged sources"] --> HASH["SHA-256 evidence manifest"] FULL["All-backend executed run count"] --> HASH HASH --> BADGE["README integration badge"] GEN["README, examples, counts,<br/>and benchmark generators"] --> PARITY["Checked-in parity plus<br/>second-run idempotency"] JSON["Exact go test JSON event"] --> OUTCOME{"Terminal action"} OUTCOME -->|pass| PASS["Executed pass"] OUTCOME -->|skip| SKIP["Capability skip"] OUTCOME -->|fail| FAIL["Executed failure"] OUTCOME -->|absent| MISSING["Missing event and failing job"]Architecture before and after
flowchart LR subgraph BEFORE["Before: split ownership"] direction TB BAPP["Application"] --> BQ["queue.Queue facade"] BAPP -. legacy calls .-> BB["bus surface"] BQ --> BQR["Queue registry, lifecycle,<br/>and observer"] BQ --> BWE["bus-owned workflow engine, models,<br/>store, middleware, fake, and observer"] BB --> BWE BQR --> BRT["Runtime seam"] BWE --> BRT BRT --> BDR["Queue drivers"] end subgraph AFTER["After: queue-owned composition"] direction TB AAPP["Application"] --> AQ["queue.Queue"] ALEG["Legacy bus caller"] --> AB["Deprecated bus facade"] AB --> AQ AQ --> AOWN["Shared registry, middleware,<br/>and lifecycle"] AQ --> AWE["Canonical workflow engine"] AQ --> AOBS["Unified queue.Event stream"] AAPP -. tests .-> AFAKE["Root queue fake"] AFAKE --> AWE AWE --> AST["Root workflow models and store"] AOWN --> ART["Runtime seam"] AWE --> ART ART --> ADR["Queue drivers"] endThe root queue already exposed chains and batches before this PR. The change is ownership and continuity: those calls now use root
queuetypes and one shared engine instead of crossing into a second public model. The low-level raw runtime route remains for compatibility, but it is not the recommended application path.Application code before and after
Before, root workflow configuration still depended on public
busmodels:After, the same application uses one root model.
Event.Layerdistinguishes queue and workflow facts without creating a second observer pipeline:The queue constructor and workflow call stay familiar:
An option-free
bus.New(existingQueue)now returns a compatibility view over this same engine. It shares handlers, middleware, stores, workflow state, observers, and shutdown state. Options that would create conflicting ownership are rejected withbus.ErrQueueOptionsUnsupportedand must be applied when the root queue is constructed.Direct jobs now remain direct
This application call is unchanged:
Before, the driver received a workflow protocol message and a JSON wrapper:
{ "schema_version": 1, "dispatch_id": "dsp_0123456789abcdef", "kind": "job", "job_id": "job_0123456789abcdef", "attempt": 0, "job": { "type": "emails:send", "payload": "eyJpZCI6MX0=", "options": { "Queue": "critical", "Delay": 0, "Timeout": 0, "Retry": 0, "Backoff": 0, "UniqueFor": 0 } } }After, the driver receives the application type and exact application bytes. Correlation travels beside the payload in versioned metadata:
{ "schema_version": 1, "dispatch_id": "dsp_0123456789abcdef", "job_id": "job_0123456789abcdef", "queue": "critical" }Arbitrary bytes are preserved, and an absent payload stays absent. Chains, batches, callbacks, reserved
bus:*types, and the raw runtime route intentionally retain the version-one workflow envelope.flowchart LR A["Queue.Dispatch"] --> B{"Legacy mode or<br/>reserved type?"} B -->|Yes| C["Version 1 workflow envelope"] B -->|No| D["Application type<br/>and exact payload"] D --> E["Versioned correlation metadata<br/>beside the payload"] C --> OLD["Old and new workers"] E --> NEW["New workers only"] OLD --> PIPE["Canonical handler and middleware pipeline"] NEW --> PIPE PIPE --> SETTLE["Backend settlement boundary"] SETTLE --> OBS["Best-effort observer facts"]Compatibility is intentionally one-way during rollout. New workers read old and new deliveries; old workers cannot safely consume the new direct format. For non-SQL backends, deploy upgraded workers while producer instances keep
queue.WithLegacyDirectEnvelope(), then remove that producer option after old consumers are gone. SQL requires a fleet replacement: quiesce every old SQL worker before any new SQL worker starts, even while legacy producer emission remains enabled.One runtime behavior map
busmodels and orchestrationqueueowns the public model; one internal engine owns orchestrationbus.New(existingQueue)shares the root enginequeue.Eventstream usesEvent.Layer, one effective physical queue name, and opaque identity for settlement-aware Active gaugesbus:jobJSON envelopeSQL receipt-backed handler replay suppression
On the built-in SQL queue plus SQL workflow-store path, the logical workflow outcome and an immutable transition receipt commit in one database transaction. The memory store implements the same contract when complete delivery provenance is supplied, but its receipts are process-local. SQL settlement is separately generation-fenced. If a failed settlement leaves or restores a recoverable row, a later delivery can prove that application work already committed without pretending queue settlement was atomic with it.
flowchart TD A["SQL worker claims a delivery<br/>and processing generation"] --> B["Run workflow handler"] B --> C["Store transaction commits<br/>workflow outcome plus receipt"] C --> D["Attempt successor work<br/>and physical settlement"] D --> E{"Settlement commits?"} E -->|Yes| F["Publish eligible observer facts<br/>best effort"] E -->|No| R{"Delivery remains or<br/>becomes recoverable?"} R -->|No| X["No receipt-based replay<br/>on that path"] R -->|Yes| G["Redelivery enters recovery"] G --> H["Validate receipt version, identity,<br/>fingerprint, and workflow state"] H --> I{"Proof valid?"} I -->|No| J["Return uncommitted<br/>no ack and no effects"] I -->|Yes| K["Suppress duplicate handler execution"] K --> L["Restore eligible outcome<br/>and retry settlement"] L --> DThis is not an exactly-once claim. Receipts do not make settlement, successor enqueue, closure callbacks, batch fanout, or observer delivery part of the store transaction. Only the exact physical receipt owner can reconstruct predecessor success facts. Lineage repair is best effort, and chain successor repair remains at least once.
Compatibility and rollout
metadata_jsonandprocessing_token; managed readiness now rejects absent, view-backed, or incomplete queue tablesbus_workflow_transition_receiptsand validates connected MySQL identity columnsRetry(0)now means zero instead of Asynq's default 25Retry(25)explicitly if that was the intended policybus.Observerandbus.Eventtoqueue.Observerandqueue.Eventqueue.ObserverFuncand useEvent.Layer; the legacy bus facade still translates eventsbusruntime.DeliverySettlementIdentity; identity-less terminal facts cannot close an identity-bearing startprocess_failedbefore the original panic is rethrown; backend panic recovery and retry behavior is unchangedUniqueForidentity/busto root/queueFailChainkeeps the first terminal causeWorkflowStorecallsv0.0.0, so a prospective next tag family is intentionally blockedSchema, durability, and test boundaries
queue.NewSQLStoreWithManagedSchemaperforms no workflow DDL, so managed deployments must precreate the complete receipt table. Incompatible existing MySQL workflow identities require a quiescent persisted-schema migration and a fresh store; compatible established rows remain readable. Queue database migration remains enabled by default and can be disabled withDisableAutoMigrate. Managed queue readiness performs no DDL and requires both queue objects to be base-table relations with all 18 runtime-used columns; it does not certify permissions, exact types, constraints, or indexes. Existing queue rows remain readable after the two nullable queue columns are added. Automatic migration also adds the SQL queue uniqueness-lock expiry index needed for bounded pruning.plan.md.The branch includes literal legacy wire and SQL fixtures, public compile contracts, race coverage, store and lifecycle fault injection, and real SQLite, MySQL, and PostgreSQL recovery scenarios. CI validates all 12 Go modules and runs null, Sync, Workerpool, Redis, NATS, SQS, RabbitMQ, MySQL, PostgreSQL, and SQLite independently. Coverage fans in every buildable module plus all ten backend profiles, rejects incomplete or duplicate inputs before upload, and submits one complete report under the repository's Codecov project and patch policy. The coverage check binds that upload to the exact workflow run and stays pending until Codecov's commit aggregate and pull request comparison are processed; fast contracts cover reruns, pagination, transient responses, and stale comparisons.
The complete fan-in also drove focused scripted SQL and driver tests for migration races, settlement repair, uniqueness compensation, retry decisions, malformed deliveries, readiness failures, and shutdown boundaries. A follow-up pass proves NATS shutdown waits for accepted handlers, Redis ownership reaches real command semantics, and RabbitMQ immediate retry reaches attempt one, commits, and leaves its broker queue empty. Defensive invariant tests verify ambiguous SQL claim results roll back and absent SQS client results fail closed without presenting those results as production client behavior. Fixed-structure serialization, entropy-source failures, and failures emitted only inside concrete broker clients remain explicit instead of adding production hooks solely for coverage.
Why
The repository had two application models over the same delivery layer. Root
queueowned direct jobs and worker lifecycle, whilebusseparately owned workflow types, stores, middleware, observers, fakes, and orchestration state. The same physical queue could therefore have two registries, two observer pipelines, and different retry or shutdown behavior depending on the facade a caller used.That duplication also blurred three separate facts: application state committed, the physical delivery settled, and an observer received an event. During retries or finalization failures those facts can diverge. Treating them as one fact risks repeated handlers, contradictory workflow outcomes, premature success events, or lost continuation work.
This PR gives each fact one owner. Durable store state determines workflow truth, the driver boundary determines physical completion, and observation remains telemetry. The result is one ergonomic root API with explicit compatibility and durability boundaries instead of two partially overlapping systems.