Uh oh!
There was an error while loading. Please reload this page.
Add durable Yieldstar reconciliation foundation - #42
Closed
djgrant wants to merge 17 commits into
Closed
Conversation
This was referenced Jul 22, 2026
djgrantforce-pushed
the
yieldstar-reconciliation-foundation
branch
5 times, most recently
from
July 27, 2026 13:47
12aa7a4 to
3f7198dComparedjgrantforce-pushed
the
yieldstar-reconciliation-foundation
branch
from
July 27, 2026 14:42
3f7198d to
5028cd3CompareIntroduce the durable reconciliation modules under durable/, with the Yieldstar runtime confined to a single adapter seam. Adds @notation/utils and moves error-matcher classification next to ErrorMatcher in @notation/resource as findErrorMatcher. The legacy reconciler remains in place until the runtime cutover.
The durable planner and operations now treat an undefined read as absence, and a tagged ResourceNotReadyError as a wait: durable operations retry it, while planning reports an indeterminate node carrying the message.
djgrantforce-pushed
the
yieldstar-reconciliation-foundation
branch
from
July 27, 2026 16:40
5028cd3 to
40d140cCompareThe durable path reimplemented create, update, read and delete inline rather than using the StepRunner seam those operations were built for, leaving two copies of the same provider protocol to keep in step. Persistence becomes a driver-supplied step. In process that is a compare-and-set against the revision read before the operation; in a workflow it is a store write stamped with the step that made it, so the applied-step ledger and the state change commit together and a replay returns the recorded result instead of retrying a stale compare-and-set. Event emission becomes a step for the same reason. Scoped step keys let one operation body run at several call sites, which replaces the hand-threaded key prefixes and the parameter bags that carried them. With writes owned by the workflow, DurableStateBackend is read-only for the planner and reporting, so the revision arithmetic and the creation token that guarded create-if-absent both go. Also collapses the three copies of the reconciler event types, delegates Reconciler.plan to createPlan, and exports the durable subsystem, which no consumer could previously import.
A delay whose deadline has already passed is continued inline rather than suspended, so a one millisecond retry could resume inside the execution that scheduled it once the heap write outlived it. The tests then saw two provider calls where they expected the workflow to have parked after one. Names the interval and puts it comfortably beyond a SQLite write. The assertions are unchanged: the provider is still called once before the workflow parks, and once more after it resumes.
The hold was released in a finally, so every failure mode released it — except a crash, where the runner abandons the workflow generator without calling .throw() and the finally never runs. That split matters because yieldstar caches steps per execution: a resumed execution replays `take` from the cache without re-acquiring, and replays `release` as a cached no-op, so a resumption that followed the release path proceeds holding nothing while it mutates the deployment. Unify every failure mode on the crash path's semantics. The hold is released in exactly one place, after the body completes. An execution that will never resume now holds its deployment forever, which is why takeOverDeploymentHold lands with it: force-clearing the coordination store through storeClient.updateStore is unconditional and undocumented, so without a named, compare-holder takeover this commit would turn any transient provider error into a wedge with no runbook. The test cannot be written against a failing step: a cached StepError is rethrown before the step's function runs again, so the resumption would never reach uncached work. It fails in plain generator code between two resources instead, and asserts the holder observed from inside the second resource's create — null before this change, the execution id after it.
createResourceWorkflow, readResourceWorkflow, updateResourceWorkflow and deleteResourceWorkflow had no consumers, and each was typed `unknown`, so nothing could have called one without casting anyway. They were also the only runtime yieldstar import reachable from the base entry point, which made the durable runtime look like a dependency of the in-process driver. YieldstarApi goes with them: `export type`, so never a runtime import, and equally unused. operation.workflows.test.ts exercises the operations directly despite its name, so it keeps its coverage. After this, the only "yieldstar" imports under src are in durable/.
Both are persisted by create and update, but the schema was loose, so neither appeared in StoredResourceState and nothing checked that a durable record carried the same fields as an in-process one. The store stays loose. PersistedResourceState deliberately carries an index signature, and v.object strips unknown entries on parse, so tightening the object would silently drop forward-compatible fields at the store boundary. The schema is a floor on the record, not a description of it. Validation runs on write rather than read, so records persisted before this commit are still readable; the next write to one supplies both fields. The test fixture that seeds a record directly now supplies them.
Operations resolved two things for themselves that the driver had already
decided against: the desired params, through resource.getParams(), and
the persisted record, through a `state: Pick<State, "get">` seam that
existed for a single read in operation.read.ts.
Both are now passed in. getParams is resolved once per scheduled
reconciliation — conflict recovery included, so recovery compares the
remote against the same params the failed attempt decided on — and
persistedOutput replaces the state seam, which also deletes the fake
`state: { get: async () => stateNode }` adapter the durable driver had to
construct.
This is not a live correctness fix. getParams spreads config over
deriveParams(id, config, deps) and never reads this.output, so repeated
calls do not diverge today. The win is that deriveParams is user code:
a non-deterministic one would let the emitted decision diff and the
persisted params record disagree with nothing able to reconcile them.
Deletion keeps resolving params lazily, on the recovery path only, since
that is the sole consumer there and an orphan hydrated from state should
not be asked to derive params just to be deleted.
Three construction sites changed: reconciler.ts, durable/operations.ts
and planner.ts, which already had both the state node and the resolved
plan params in hand and was resolving params a second time inside the
drift read.
Also adds the typecheck script the package never had, which is how the
call sites still passing the long-deleted expectedRev field surfaced.
Making the package typecheck exposed two more things: the durable store
schema now requires groupId and groupType that PersistedResourceState did
not declare (declared here, since StateNode only carries them through its
index signature), and a batch of test fixtures that were never type
checked. The fixture casts are annotated: a resource declared without API
types cannot express a named schema key at all.Scoping was a durable-only wrapper that rebuilt a step object. It is now part of the StepRunner contract: scope(prefix) is the identity in process, because createStepRunner ignores keys entirely, and the prefixing wrapper in a workflow. scopeStore goes with it. A store outlives the scope that opened it, so prefixing its caller-supplied keys with whichever scope happened to open the handle was arbitrary; the three call sites now qualify their own keys, as state:persist:<id>, state:delete:<id> and the notation:coordination:* keys, which were already fully qualified. The keyless run/delay overloads are gone. Yieldstar derives a key from the call site for those, and through a scoping wrapper that call site is the wrapper's, not the caller's — so any two keyless steps reached through one scope would have collided. Nothing called them; removing them makes that structural rather than a convention. Keys cost nothing in process, where they are ignored. BLAST RADIUS. Store keys change, so an execution that was in flight before this commit and resumes after it must be drained or abandoned first. It will not refuse cleanly: plain run steps have no idempotency ledger, so a resumed execution takes a cache miss and RE-EXECUTES the provider mutations behind those keys. It then fails safe at the first conditional state write, because updateFrom/deleteFrom carry a snapshot that is now stale and abort with RevConflict — but the remote calls will already have happened. Registered store waiters are keyed by stepKey too, so a `take` suspended on the deployment hold re-registers under its new key when it next wakes. The key shapes now in use are documented at the durable entry point, where they can be read as the contract they are.
The durable driver threaded dryRun into the drift read's params, so
readResourceOperation returned {} without calling the provider. decideAction
then diffed toComparable({}) against the desired params, found every param
"missing" from the remote, and upgraded a noop into a drift-update carrying
a fabricated diff. Every dry-run test set driftDetection: false, so nothing
caught it.
The in-process driver never had this: #readForDrift deliberately omits
dryRun, and createPlan does the same. The contract is that a dry run
suppresses mutations, not reads — a dry run that cannot read the remote
cannot report drift, which is most of what it is for. So this matches the
in-process behaviour rather than guarding the read.
The new test pins both halves: the provider read happens during the dry
run, and the decision stays noop with no drift event.Conflict recovery re-reads the remote, re-decides against it, and can land on drift-update — but it emitted only deploy.decision, so the drift.detected event that the same decision produces on a first pass was missing. Recovery after a conflict is drift adoption, so it is owed. This is a visible change for anything diffing event sequences: a recovering deployment now emits reconciler.drift.detected before its deploy.decision, where before it emitted only the decision. It lands on its own so that it is not buried in the driver unification that follows.
takeOverDeploymentHold is the only exit from a hold left by an execution that will never resume, so it ships tested: it takes when the named holder still holds, and refuses — reporting the actual holder, leaving the record alone — when the hold has moved on. The key map gains the destroy-path orphan sweep, which it omitted, and a note that resource ids are spliced in unescaped. That ambiguity predates the map and is recorded rather than fixed, since changing key composition invalidates in-flight executions.
The per-resource algorithm existed twice — #deployResourceOnce plus #recoverDeployResource in process, reconcileResource in the workflow — and the copies had already drifted apart. There is now one generator, src/reconcile.ts, that owns hydration, the decision, the drift read, event emission, dispatch, recovery adoption and deletion recovery. Both drivers run it. What stays with a driver is what genuinely differs: dependency-level scheduling and concurrency (Promise.all under a mutation lease against a sequential yield* under a deployment hold), the conflict retry policy, how a state session is opened, and how a step is run. Two seams carry that split. The driver passes openSession(resource), a factory rather than an open session, because recovery re-reads and persists against what the winning writer left; the first pass opens once and a recovering pass opens again. And ResourceStateSession is a discriminated union, so `remove` exists only alongside a `node` — the precondition deleteResource expressed as an early return and destroy expressed as skip-when-absent, now expressed in the type. Keeping the writes bound to the read that produced them is what stops a node being paired with a precondition from a different read. Recovery is a recoverFrom parameter rather than a second near-copy of the algorithm. The behaviours that were easy to lose in that collapse are each still pinned by an unchanged test: a recovering noop persists the adopted record with lastOperation "drift" at the re-read revision (guarded by dryRun), a resource that cannot be read cannot be recovered, and the drift read carries no dryRun. Orphan sweeping is aligned too. In-process destroy now sweeps, which also means destroyApp no longer runs a refresh pass first — but the Reconciler it builds must be given the registry, or the sweep silently falls back to the types of the resources still declared and skips, with a warning, exactly the orphans a destroy exists to remove. registry is optional, so nothing else would have caught that. Note the ordering flip: in-process destroy used to sweep before deleting declared resources and now sweeps after, matching the durable driver and deploy's act-then-sweep shape. Public refresh is unchanged, for standalone cleanup. The #emit/#emitStep split collapses with it: OperationLifecycleEvent is a subset of ReconcilerEvent, so one scoped emit seam serves decisions, drift and operation lifecycle alike. reconciler.ts 511 -> 361 lines, durable/operations.ts 252 -> 197, with 248 shared between them.
djgrant
commented
Jul 28, 2026
CollaboratorAuthor
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 freeto 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.
#44 defines how resources report three outcomes: finished, not found, or still pending. This PR makes those outcomes durable.
It adds a Yieldstar deploy and destroy engine alongside the current
Reconciler. Core and the CLI do not use the new engine yet; #43 performs that cutover.What it does
ResourceNotFoundErrorfrom a drift read as an absent resource.A provider call and its workflow checkpoint are not atomic. Provider create, update, and delete handlers must remain safe to repeat if the process dies between those two events.
Scope
The current
Reconciler, core workflows, and CLI remain unchanged in this PR. #43 switches them to this engine and removes the old implementation.Verification
pnpm build,pnpm typecheck, andpnpm test:oncepass. The suite contains 132 passing tests and 4 skipped tests.