Give the top-level workflow run its own durable identity - #130
Open
alexanderguy wants to merge 9 commits into
Open
Give the top-level workflow run its own durable identity#130alexanderguy wants to merge 9 commits into
alexanderguy wants to merge 9 commits into
Conversation
The old post-reconnect block fired a second distinct trigger and waited for RunCompleted under a runId hard-coded equal to the first. That re-observed the first run's own terminal and passed without exercising the real behavior: the deployment's one run is already terminal, so the second trigger is rejected with a workflow_run_terminal receipt and no new run starts. Assert that receipt and a single RunStarted/RunCompleted instead, read through the supervisor-minted run id rather than the mail address.
The sidecar's on-disk restore record persists a single workflow run's sources, session id, and hub key, but carried "deployment" naming left over from the dropped workflow_deployment table. Rename the type, file, and functions to name the run they serve. The persisted record keeps its deployment.json filename under workflow-runs/<deploymentId>, so a record written by an earlier sidecar is still found at boot. Only the in-memory naming moves; the on-disk format and the behavior are unchanged.
The top-level run id is re-derived independently in each consumer from the deployment mail address. Carry it as an explicit column on the workflow_run_dispatch row instead, so the reconcile path reads one authoritative value rather than re-deriving it. The value is unchanged (still the mail address), so delivery is byte-for-byte identical; this only removes the independent re-derivation.
The top-level run borrowed the deployment mail address as its id, so the run and the deployment were one identifier. Give the run its own run_ id: thread the id from the hub through the mail.inbound frame and the claim-check envelope, with the supervisor reading it instead of re-deriving; and mint it once per deployment under the anchor-run lock (read-or-mint, keyed by a new is_top_level marker whose partial unique index fails a double-mint loud). Remove deriveWorkflowRunId, the address-as-run-id function, so no code path can re-derive the old id. The deployment keeps its own id, address, public key, and running gate, so reconnect is unchanged.
The deployment anchor row borrowed the run status "running", so "running" meant both "this deployment is live and reconnectable" and the run-lifecycle "running" -- two concepts in one value. Split them: the anchor is born "deployed" (a deployment-phase status), while the top-level run and its children keep running and the terminal states. The liveness gates -- the reconnect key lookup, the anchor FOR UPDATE lock, the trigger and approval paths -- now treat "deployed" as live, funnelled through the shared run-state classifier so the constraint lives in one layer. A backfill flips existing live anchors.
The route lived at /workflows/instances but deploys and lists workflow deployments, not instances -- a leftover of the retired agent-instance vocabulary. Move it to /workflows/deployments so the path names what it returns. The only consumers are the in-repo hub-client and admin-UI, so there is no external contract to keep and no compatibility alias.
The anchor-locked read-or-mint of a deployment's top-level run was copy-pasted across the two mail-route reservation transactions and the sidecar mail-trigger materializer. A change to how the addressable run is resolved -- the is_top_level discriminator or the lock ordering -- had to be applied in all three or behavior would fork between them. Resolve it in one place so the three paths cannot drift.
The durable claim-check inbox is read across a hub upgrade, so the decode boundary can meet an entry written before the run id became a required envelope field. A strict decode rejects it and the in-flight trigger wedges. Derive the missing run id from the entry's address, the value a deployment's run was addressed by before it had its own id, so a legacy entry recovers instead of wedging.
Member
Author
Self-Review (
|
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
1 participant
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.
Summary
run_<hex>id, threaded from the hub through the mail-inbound frame and the claim-check envelope and read by the supervisor. The run id no longer derives from the deployment mail address, and the address-as-run-id derivation is removed.deployedstatus, distinct from the run lifecyclerunning. Reconnect key-lookup, signal correlation, and the reservation, trigger, and approval gates treatdeployedas live through one shared run-state classifier.workflow_run_dispatchrow carries the run id as an explicit column. The sidecar restore record — its type, file, and functions — names the run it serves, while keeping the persisteddeployment.jsonfilename so records written by an earlier sidecar still restore. The deploy-list route is/workflows/deployments.Verification
make all(build, lint, and both test suites) exits 0.is_top_levelmarker, and thedeployedanchor status, keyed on the legacy invariant that a top-level run's id equalled its address.Closes INTR-358
Closes INTR-359