From a081337cdc391c07b778e8f5f23d06d74457f4b1 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:01:37 +0100 Subject: [PATCH 01/17] Add durable Yieldstar reconciliation foundation Introduce 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. --- packages/reconciler/package.json | 7 +- .../reconciler/src/durable/coordination.ts | 55 ++ packages/reconciler/src/durable/deploy.ts | 37 ++ packages/reconciler/src/durable/destroy.ts | 46 ++ packages/reconciler/src/durable/emit.ts | 67 +++ packages/reconciler/src/durable/index.ts | 18 + packages/reconciler/src/durable/operations.ts | 495 ++++++++++++++++ .../reconciler/src/durable/state-backend.ts | 137 +++++ packages/reconciler/src/durable/stores.ts | 67 +++ packages/reconciler/src/durable/types.ts | 37 ++ packages/reconciler/src/durable/yieldstar.ts | 8 + packages/reconciler/src/events.ts | 53 ++ packages/reconciler/src/planner.ts | 81 +++ packages/reconciler/src/resource-registry.ts | 4 +- .../test/durable-reconciliation.test.ts | 531 ++++++++++++++++++ packages/reconciler/test/planner.test.ts | 36 ++ pnpm-lock.yaml | 65 ++- pnpm-workspace.yaml | 4 + 18 files changed, 1722 insertions(+), 26 deletions(-) create mode 100644 packages/reconciler/src/durable/coordination.ts create mode 100644 packages/reconciler/src/durable/deploy.ts create mode 100644 packages/reconciler/src/durable/destroy.ts create mode 100644 packages/reconciler/src/durable/emit.ts create mode 100644 packages/reconciler/src/durable/index.ts create mode 100644 packages/reconciler/src/durable/operations.ts create mode 100644 packages/reconciler/src/durable/state-backend.ts create mode 100644 packages/reconciler/src/durable/stores.ts create mode 100644 packages/reconciler/src/durable/types.ts create mode 100644 packages/reconciler/src/durable/yieldstar.ts create mode 100644 packages/reconciler/src/events.ts create mode 100644 packages/reconciler/src/planner.ts create mode 100644 packages/reconciler/test/durable-reconciliation.test.ts create mode 100644 packages/reconciler/test/planner.test.ts diff --git a/packages/reconciler/package.json b/packages/reconciler/package.json index 39ed2f0..3429d0d 100644 --- a/packages/reconciler/package.json +++ b/packages/reconciler/package.json @@ -14,7 +14,12 @@ "dependencies": { "@notation/resource": "workspace:*", "@notation/state": "workspace:*", + "@yieldstar/core": "0.5.0", "deep-object-diff": "^1.1.9", - "yieldstar": "^0.4.6" + "yieldstar": "0.5.0" + }, + "devDependencies": { + "@yieldstar/sqlite-runtime": "0.5.0", + "pino": "^9.9.0" } } diff --git a/packages/reconciler/src/durable/coordination.ts b/packages/reconciler/src/durable/coordination.ts new file mode 100644 index 0000000..7dac2d2 --- /dev/null +++ b/packages/reconciler/src/durable/coordination.ts @@ -0,0 +1,55 @@ +import type { ReconcilerEventEmitter } from "../events"; +import { emitOnce } from "./emit"; +import { deploymentCoordinationStore, type CoordinationState } from "./stores"; +import type { DurableStep, WorkflowStore } from "./yieldstar"; + +type CoordinationOptions = { + deploymentId: string; + executionId: string; + emit?: ReconcilerEventEmitter; +}; + +/** + * Prevents concurrent executions from mutating the same deployment. Names + * the holder so an operator can resume it after a crash. + */ +export async function* acquireDeploymentCoordination( + step: DurableStep, + opts: CoordinationOptions, +): AsyncGenerator, any> { + const coordination = yield* step.store(deploymentCoordinationStore, { + id: opts.deploymentId, + initial: { holder: null }, + }); + + const snapshot = yield* coordination.get("notation:coordination:inspect"); + const holder = snapshot.state.holder; + if (holder !== null && holder !== opts.executionId) { + yield* emitOnce(step, "notation:coordination:waiting", opts.emit, () => ({ + level: "warn", + event: "reconciler.coordination.waiting", + deploymentId: opts.deploymentId, + executionId: opts.executionId, + holderExecutionId: holder, + })); + } + + yield* coordination.take( + "notation:coordination:acquire", + (state) => state.holder === null || state.holder === opts.executionId, + (draft) => { + draft.holder = opts.executionId; + }, + ); + + return coordination; +} + +export function releaseDeploymentCoordination( + coordination: WorkflowStore, + executionId: string, +) { + return coordination.update("notation:coordination:release", (draft) => { + if (draft.holder === executionId) draft.holder = null; + }); +} diff --git a/packages/reconciler/src/durable/deploy.ts b/packages/reconciler/src/durable/deploy.ts new file mode 100644 index 0000000..3b93520 --- /dev/null +++ b/packages/reconciler/src/durable/deploy.ts @@ -0,0 +1,37 @@ +import { buildResourceDepthLevels } from "../dependency-graph"; +import { + acquireDeploymentCoordination, + releaseDeploymentCoordination, +} from "./coordination"; +import { reconcileResource, sweepOrphans } from "./operations"; +import type { DurableDeployOptions } from "./types"; +import type { DurableStep } from "./yieldstar"; + +export async function* deploy( + step: DurableStep, + opts: DurableDeployOptions, +): AsyncGenerator { + // Phase 1: take exclusive hold of the deployment. + const coordination = yield* acquireDeploymentCoordination(step, opts); + + try { + // Phase 2: reconcile in dependency order, so a resource only runs once + // its dependencies have converged. + for (const level of buildResourceDepthLevels(opts.resources)) { + for (const resource of level) { + yield* reconcileResource(step, resource, opts); + } + } + + // Phase 3: delete resources that are in state but no longer declared. + yield* sweepOrphans(step, opts, { + workflow: "deploy", + listKey: "notation:orphans:list", + warningKey: (nodeId) => `notation:orphan:${nodeId}:warning`, + deleteSuffix: "orphan", + }); + } finally { + // Phase 4: release the hold, even on error. + yield* releaseDeploymentCoordination(coordination, opts.executionId); + } +} diff --git a/packages/reconciler/src/durable/destroy.ts b/packages/reconciler/src/durable/destroy.ts new file mode 100644 index 0000000..8f01316 --- /dev/null +++ b/packages/reconciler/src/durable/destroy.ts @@ -0,0 +1,46 @@ +import { buildResourceDepthLevels } from "../dependency-graph"; +import { + acquireDeploymentCoordination, + releaseDeploymentCoordination, +} from "./coordination"; +import { deleteResource, sweepOrphans } from "./operations"; +import type { DurableDestroyOptions } from "./types"; +import type { DurableStep } from "./yieldstar"; + +/** Durably destroys persisted resources in reverse dependency order. */ +export async function* destroy( + step: DurableStep, + opts: DurableDestroyOptions, +): AsyncGenerator { + // Phase 1: take exclusive hold of the deployment. + const coordination = yield* acquireDeploymentCoordination(step, opts); + + try { + // Phase 2: delete in reverse dependency order, so dependents are gone + // before the resources they depend on. Resources with no persisted state + // were never created (or are already deleted) and are skipped. + const levels = buildResourceDepthLevels(opts.resources); + for (let index = levels.length - 1; index >= 0; index -= 1) { + for (const resource of levels[index]!) { + const stateNode = yield* step.run( + `notation:destroy:${resource.id}:state:lookup`, + () => opts.state.get(resource.id), + ); + if (!stateNode) continue; + resource.setOutput(stateNode.output); + yield* deleteResource(step, resource, opts, "destroy"); + } + } + + // Phase 3: delete resources that are in state but no longer declared. + yield* sweepOrphans(step, opts, { + workflow: "destroy", + listKey: "notation:destroy:orphans:list", + warningKey: (nodeId) => `notation:destroy:orphan:${nodeId}:warning`, + deleteSuffix: "destroy-orphan", + }); + } finally { + // Phase 4: release the hold, even on error. + yield* releaseDeploymentCoordination(coordination, opts.executionId); + } +} diff --git a/packages/reconciler/src/durable/emit.ts b/packages/reconciler/src/durable/emit.ts new file mode 100644 index 0000000..7be77b1 --- /dev/null +++ b/packages/reconciler/src/durable/emit.ts @@ -0,0 +1,67 @@ +import type { + OperationLifecycleEvent, + OperationLifecycleStatus, + OperationName, + ReconcilerEventEmitter, +} from "../events"; +import type { DurableStep } from "./yieldstar"; + +/** + * Emits an event inside a durable step so a replayed workflow does not + * re-emit events it already sent. + */ +export function emitOnce( + step: DurableStep, + key: string, + emit: ReconcilerEventEmitter | undefined, + event: () => Parameters[0], +) { + return step.run(key, async () => { + await emit?.(event()); + }); +} + +export function emitLifecycle( + step: DurableStep, + key: string, + emit: ReconcilerEventEmitter | undefined, + operation: OperationName, + status: OperationLifecycleStatus, + resource: LifecycleResource, + extra: { reason?: string; error?: unknown } = {}, +) { + return emitOnce(step, key, emit, () => + createLifecycleEvent(operation, status, resource, extra), + ); +} + +type LifecycleResource = { + id: string; + type: OperationLifecycleEvent["resourceType"]; +}; + +export function createLifecycleEvent( + operation: OperationName, + status: OperationLifecycleStatus, + resource: LifecycleResource, + extra: { reason?: string; error?: unknown } = {}, +): OperationLifecycleEvent { + const error = extra.error; + const details = + error === undefined + ? {} + : error instanceof Error + ? { errorName: error.name, errorMessage: error.message } + : { errorName: "UnknownError", errorMessage: String(error) }; + + return { + level: status === "error" ? "error" : "info", + event: "reconciler.operation.lifecycle", + operation, + status, + resourceId: resource.id, + resourceType: resource.type, + ...(extra.reason ? { reason: extra.reason } : {}), + ...details, + }; +} diff --git a/packages/reconciler/src/durable/index.ts b/packages/reconciler/src/durable/index.ts new file mode 100644 index 0000000..e42707f --- /dev/null +++ b/packages/reconciler/src/durable/index.ts @@ -0,0 +1,18 @@ +export { deploy } from "./deploy"; +export { destroy } from "./destroy"; +export { DurableStateBackend } from "./state-backend"; +export { + deploymentCoordinationStore, + resourceStateStore, + type CoordinationState, + type StoredResourceState, +} from "./stores"; +export { + DEFAULT_READ_POLL_OPTIONS, + DEFAULT_RETRY_OPTIONS, + type DurableDeployOptions, + type DurableDestroyOptions, + type DurableOperationOptions, + type PollOptions, +} from "./types"; +export type { DurableStep } from "./yieldstar"; diff --git a/packages/reconciler/src/durable/operations.ts b/packages/reconciler/src/durable/operations.ts new file mode 100644 index 0000000..6bca1d2 --- /dev/null +++ b/packages/reconciler/src/durable/operations.ts @@ -0,0 +1,495 @@ +import { + findErrorMatcher, + type BaseResource, + type ResourceType, +} from "@notation/resource"; +import { RevConflict } from "@notation/state"; +import { + createMissingResourceRegistryMatchWarningEvent, + createResourceRegistryFromResources, + resolveResourceClass, +} from "../resource-registry"; +import { decideAction, type ResourceAction } from "../plan"; +import { emitLifecycle, emitOnce } from "./emit"; +import type { DurableStateBackend } from "./state-backend"; +import { + resourceStateStore, + toStateNode, + type StoredResourceState, +} from "./stores"; +import { + DEFAULT_READ_POLL_OPTIONS, + DEFAULT_RETRY_OPTIONS, + type DurableDeployOptions, + type DurableOperationOptions, + type PollOptions, +} from "./types"; +import { + RetryableError, + type DurableStep, + type WorkflowStore, +} from "./yieldstar"; + +export async function* reconcileResource( + step: DurableStep, + resource: BaseResource, + opts: DurableDeployOptions, +): AsyncGenerator { + const prefix = `notation:resource:${resource.id}`; + + // Hydrate the resource from persisted state. The snapshot is kept so later + // writes can be conditional on the exact instance identity and version that + // was read here. + let stateNode = yield* step.run(`${prefix}:state:lookup`, () => + opts.state.get(resource.id), + ); + let stateStore: WorkflowStore | undefined; + let snapshot: + Awaited> | undefined; + if (stateNode) { + stateStore = yield* openResourceState(step, opts.state, resource.id); + snapshot = yield* stateStore.get(`${prefix}:state:get`); + stateNode = toStateNode(snapshot); + } + if (stateNode) resource.setOutput(stateNode.output); + + // Decide the operation from desired params vs persisted state. + const params = yield* step.run(`${prefix}:params`, () => + resource.getParams(), + ); + let action: ResourceAction = decideAction({ + resource, + stateNode: stateNode ?? undefined, + params, + }); + + // A noop is only trusted after the remote is read back: the provider may + // have drifted from persisted state, which upgrades the decision. + if (action.decision === "noop" && (opts.driftDetection ?? true)) { + const remote = yield* readRemote( + step, + resource, + opts, + `${prefix}:drift-read`, + ); + action = decideAction({ + resource, + stateNode: stateNode ?? undefined, + params, + driftRead: remote, + }); + } + + if (action.decision === "drift-update") { + const diff = action.patch; + yield* emitOnce(step, `${prefix}:drift-detected`, opts.emit, () => ({ + level: "info", + event: "reconciler.drift.detected", + resourceId: resource.id, + resourceType: resource.type, + diff, + })); + } + + yield* emitOnce(step, `${prefix}:decision`, opts.emit, () => ({ + level: "info", + event: "reconciler.deploy.decision", + resourceId: resource.id, + resourceType: resource.type, + decision: action.decision, + })); + + if (action.decision === "noop") return; + const operation = + action.decision === "create" || action.decision === "drift-recreate" + ? "create" + : "update"; + const patch = "patch" in action ? action.patch : {}; + yield* emitLifecycle( + step, + `${prefix}:${operation}:start`, + opts.emit, + operation, + "start", + resource, + ); + if (opts.dryRun) { + yield* emitLifecycle( + step, + `${prefix}:${operation}:dry-run`, + opts.emit, + operation, + "dry-run", + resource, + ); + return; + } + + try { + // Execute the provider call. Each call runs in its own durable step, so a + // replayed workflow never repeats a completed provider mutation. + if (operation === "create") { + const primaryKey = yield* runProviderCall( + step, + `${prefix}:create`, + () => resource.create(params), + resource, + opts.retryOptions, + ); + resource.setOutput(params); + if (primaryKey) resource.setOutput({ ...primaryKey, ...resource.output }); + } else { + if (!resource.update) { + yield* emitLifecycle( + step, + `${prefix}:update:skip`, + opts.emit, + "update", + "skip", + resource, + { reason: "update-not-implemented" }, + ); + return; + } + yield* runProviderCall( + step, + `${prefix}:update`, + () => + resource.update!( + resource.key, + patch, + params, + resource.toState(resource.output), + ), + resource, + opts.retryOptions, + ); + resource.setOutput({ ...resource.key, ...params }); + } + + // Read back the remote so persisted output reflects provider-assigned + // values, then persist conditionally against the snapshot read above. + const read = yield* readRemote( + step, + resource, + opts, + `${prefix}:read-after-write`, + ); + if (read.status === "found") + resource.setOutput({ ...resource.output, ...read.output }); + + const nextState: StoredResourceState = { + id: resource.id, + groupId: resource.groupId, + groupType: resource.groupType, + type: resource.type, + lastOperation: operation, + lastOperationAt: new Date().toISOString(), + config: resource.config, + params: resource.toState(params), + output: resource.toState(resource.output), + }; + + if (!stateStore || !snapshot) { + yield* step.store(resourceStateStore, { + id: opts.state.storeId(resource.id), + initial: nextState, + }); + } else { + const result = yield* stateStore.updateFrom( + `${prefix}:state:persist`, + snapshot, + () => nextState, + ); + if (!result.updated) + throw new RevConflict( + resource.id, + stateNode?.rev ?? 0, + result.actualVersion + 1, + ); + } + + yield* emitLifecycle( + step, + `${prefix}:${operation}:success`, + opts.emit, + operation, + "success", + resource, + ); + } catch (error) { + yield* emitLifecycle( + step, + `${prefix}:${operation}:error`, + opts.emit, + operation, + "error", + resource, + { error }, + ); + throw error; + } +} + +export async function* deleteResource( + step: DurableStep, + resource: BaseResource, + opts: DurableOperationOptions, + suffix: string, +): AsyncGenerator { + const prefix = `notation:${suffix}:${resource.id}`; + + // Hydrate output from persisted state; the delete call needs the primary + // key and the state removal must be conditional on this exact snapshot. + const stateStore = yield* openResourceState(step, opts.state, resource.id); + const snapshot = yield* stateStore.get(`${prefix}:state:get`); + const stateNode = toStateNode(snapshot); + resource.setOutput(stateNode.output); + + yield* emitLifecycle( + step, + `${prefix}:delete:start`, + opts.emit, + "delete", + "start", + resource, + ); + + if (opts.dryRun) { + yield* emitLifecycle( + step, + `${prefix}:delete:dry-run`, + opts.emit, + "delete", + "dry-run", + resource, + ); + return; + } + + try { + // An already-deleted remote is success, not failure: the goal state is + // absence, so a declared not-found error downgrades to a skip. + try { + yield* runProviderCall( + step, + `${prefix}:delete`, + () => resource.delete(resource.key, resource.toState(resource.output)), + resource, + opts.retryOptions, + ); + } catch (error) { + if (!findErrorMatcher(error, resource.notFoundOnError)) throw error; + yield* emitLifecycle( + step, + `${prefix}:delete:not-found`, + opts.emit, + "delete", + "skip", + resource, + { reason: "resource-not-found" }, + ); + } + + // State is removed only after the provider delete completes, and only if + // the store still matches the snapshot read before deleting. + const deleted = yield* stateStore.deleteFrom( + `${prefix}:state:delete`, + snapshot, + ); + if (!deleted.deleted) + throw new RevConflict(resource.id, stateNode.rev, undefined); + yield* emitLifecycle( + step, + `${prefix}:delete:success`, + opts.emit, + "delete", + "success", + resource, + ); + } catch (error) { + yield* emitLifecycle( + step, + `${prefix}:delete:error`, + opts.emit, + "delete", + "error", + resource, + { error }, + ); + throw error; + } +} + +export async function* readRemote( + step: DurableStep, + resource: BaseResource, + opts: DurableOperationOptions, + key: string, +): AsyncGenerator< + any, + | { status: "found"; output: Record } + | { status: "not-found" }, + any +> { + if (!resource.read) { + yield* emitLifecycle( + step, + `${key}:skip`, + opts.emit, + "read", + "skip", + resource, + { + reason: "read-not-implemented", + }, + ); + return { + status: "found", + output: { ...(await resource.getParams()), ...resource.output }, + }; + } + + yield* emitLifecycle( + step, + `${key}:start`, + opts.emit, + "read", + "start", + resource, + ); + try { + const output = yield* step.run(key, async () => { + const value = await resource.read!(resource.key); + // An unsettled read (a declared condition not yet met) re-polls + // durably instead of returning a half-provisioned remote. + const unsettled = (resource.retryReadOnCondition ?? []) + .filter(Boolean) + .find((condition) => { + const actual = value[condition!.key]; + return condition!.value === undefined + ? !actual + : actual !== condition!.value; + }); + if (unsettled) { + throw new RetryableError(unsettled.reason, { + ...(opts.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), + }); + } + return value; + }); + yield* emitLifecycle( + step, + `${key}:success`, + opts.emit, + "read", + "success", + resource, + ); + return { status: "found", output }; + } catch (error) { + if (findErrorMatcher(error, resource.notFoundOnError)) { + yield* emitLifecycle( + step, + `${key}:not-found`, + opts.emit, + "read", + "skip", + resource, + { reason: "resource-not-found" }, + ); + return { status: "not-found" }; + } + yield* emitLifecycle( + step, + `${key}:error`, + opts.emit, + "read", + "error", + resource, + { + error, + }, + ); + throw error; + } +} + +/** + * Runs a provider call in a durable step, converting errors the resource has + * declared retryable into durable retries. + */ +export function runProviderCall( + step: DurableStep, + key: string, + call: () => T | Promise, + resource: BaseResource, + retryOptions?: PollOptions, +) { + return step.run(key, async () => { + try { + return await call(); + } catch (error) { + const matcher = findErrorMatcher(error, resource.retryLaterOnError); + if (matcher) { + throw new RetryableError(matcher.reason, { + ...(retryOptions ?? DEFAULT_RETRY_OPTIONS), + }); + } + throw error; + } + }); +} + +/** + * Deletes persisted resources that are no longer in the desired set. A state + * node whose type has no registry entry is left in place and surfaced as a + * warning, because deleting it would need a resource class we cannot resolve. + */ +export async function* sweepOrphans( + step: DurableStep, + opts: DurableOperationOptions, + params: { + workflow: "deploy" | "destroy"; + listKey: string; + warningKey: (nodeId: string) => string; + deleteSuffix: string; + }, +): AsyncGenerator { + const resourceById = new Map( + opts.resources.map((resource) => [resource.id, resource]), + ); + const persisted = yield* step.run(params.listKey, () => opts.state.values()); + const registry = + opts.registry ?? createResourceRegistryFromResources(opts.resources); + + for (const node of persisted) { + if (resourceById.has(node.id)) continue; + + const Resource = resolveResourceClass(registry, node.type as ResourceType); + if (!Resource) { + yield* emitOnce(step, params.warningKey(node.id), opts.emit, () => + createMissingResourceRegistryMatchWarningEvent({ + workflow: params.workflow, + resourceId: node.id, + resourceType: node.type as ResourceType, + }), + ); + continue; + } + + const resource = new Resource({ id: node.id, config: node.config }); + resource.setOutput(node.output); + yield* deleteResource(step, resource, opts, params.deleteSuffix); + } +} + +export function openResourceState( + step: DurableStep, + state: DurableStateBackend, + resourceId: string, +) { + return step.store(resourceStateStore, { + id: state.storeId(resourceId), + }); +} diff --git a/packages/reconciler/src/durable/state-backend.ts b/packages/reconciler/src/durable/state-backend.ts new file mode 100644 index 0000000..e15d498 --- /dev/null +++ b/packages/reconciler/src/durable/state-backend.ts @@ -0,0 +1,137 @@ +import { RevConflict, type StateNode } from "@notation/state"; +import { + resourceStateStore, + toStateNode, + withoutRev, + type StoredResourceState, +} from "./stores"; +import type { StoreClient } from "./yieldstar"; + +export class DurableStateBackend { + readonly #client: StoreClient; + readonly #deploymentId: string; + readonly #prefix: string; + + constructor(client: StoreClient, deploymentId: string) { + this.#client = client; + this.#deploymentId = deploymentId; + // Keep deployment prefixes disjoint so orphan cleanup cannot delete + // another deployment's stores. + this.#prefix = `${encodeURIComponent(deploymentId)}:`; + } + + storeId(resourceId: string) { + return `${this.#prefix}${resourceId}`; + } + + async get(id: string): Promise { + const snapshot = await this.#tryGetSnapshot(this.storeId(id)); + return snapshot ? toStateNode(snapshot) : undefined; + } + + async #tryGetSnapshot( + storeId: string, + ): Promise< + | { state: StoredResourceState; instanceId: string; version: number } + | undefined + > { + try { + return await this.#client.getStore({ + definition: resourceStateStore, + id: storeId, + }); + } catch (error) { + const ids = await this.#client.listStores(resourceStateStore); + if (!ids.includes(storeId)) return undefined; + throw error; + } + } + + async has(id: string): Promise { + return (await this.get(id)) !== undefined; + } + + async update( + id: string, + expectedRev: number, + patch: Partial, + ): Promise<{ rev: number }> { + const storeId = this.storeId(id); + const snapshot = await this.#tryGetSnapshot(storeId); + if (!snapshot) { + if (expectedRev !== 0) throw new RevConflict(id, expectedRev, undefined); + const initial = { ...patch, id } as StoredResourceState; + const created = await this.#client.getOrCreateStore({ + definition: resourceStateStore, + id: storeId, + initial, + }); + return { rev: created.version + 1 }; + } + + const actualRev = snapshot.version + 1; + if (actualRev !== expectedRev) + throw new RevConflict(id, expectedRev, actualRev); + const result = await this.#client.updateStoreFrom({ + definition: resourceStateStore, + id: storeId, + snapshot, + updater: (draft) => { + Object.assign(draft, withoutRev(patch)); + }, + }); + if (!result.updated) throw new RevConflict(id, expectedRev, undefined); + return { rev: result.version + 1 }; + } + + async delete(id: string, expectedRev: number): Promise { + const storeId = this.storeId(id); + const snapshot = await this.#tryGetSnapshot(storeId); + if (!snapshot) { + if (expectedRev !== 0) throw new RevConflict(id, expectedRev, undefined); + return; + } + const actualRev = snapshot.version + 1; + if (actualRev !== expectedRev) + throw new RevConflict(id, expectedRev, actualRev); + const result = await this.#client.deleteStoreFrom({ + definition: resourceStateStore, + id: storeId, + snapshot, + }); + if (!result.deleted) throw new RevConflict(id, expectedRev, undefined); + } + + async values(): Promise { + const ids = await this.#client.listStores(resourceStateStore); + const snapshots = await Promise.all( + ids + .filter((id) => id.startsWith(this.#prefix)) + .map((id) => this.#tryGetSnapshot(id)), + ); + return snapshots + .filter((snapshot) => snapshot !== undefined) + .map(toStateNode); + } + + snapshot(id: string) { + return this.#client.getStore({ + definition: resourceStateStore, + id: this.storeId(id), + }); + } + + async clear(): Promise { + const ids = await this.#client.listStores(resourceStateStore); + await Promise.all( + ids + .filter((id) => id.startsWith(this.#prefix)) + .map((id) => + this.#client.deleteStore({ + definition: resourceStateStore, + id, + }), + ), + ); + } +} diff --git a/packages/reconciler/src/durable/stores.ts b/packages/reconciler/src/durable/stores.ts new file mode 100644 index 0000000..fba2cb3 --- /dev/null +++ b/packages/reconciler/src/durable/stores.ts @@ -0,0 +1,67 @@ +import type { StateNode } from "@notation/state"; +import { defineStore, type StandardSchemaV1 } from "./yieldstar"; + +export type StoredResourceState = Omit; +export type CoordinationState = { holder: string | null }; + +const storedResourceStateSchema = plainObjectSchema( + "Stored resource state", + (value) => + typeof value.id === "string" && + typeof value.type === "string" && + isPlainObject(value.config) && + isPlainObject(value.params) && + isPlainObject(value.output), +); +const coordinationStateSchema = plainObjectSchema( + "Deployment coordination state", + (value) => + "holder" in value && + (value.holder === null || typeof value.holder === "string"), +); + +export const resourceStateStore = defineStore( + "notation/resource-state", + storedResourceStateSchema, +); + +export const deploymentCoordinationStore = defineStore( + "notation/deployment-coordination", + coordinationStateSchema, +); + +export function toStateNode(snapshot: { + state: StoredResourceState; + version: number; +}): StateNode { + return { ...snapshot.state, rev: snapshot.version + 1 } as StateNode; +} + +export function withoutRev( + patch: Partial, +): Partial { + const { rev: _rev, ...stored } = patch; + return stored; +} + +function plainObjectSchema>( + label: string, + refine: (value: Record) => boolean, +): StandardSchemaV1 { + return { + "~standard": { + version: 1, + vendor: "notation", + validate(value) { + if (!isPlainObject(value) || !refine(value)) { + return { issues: [{ message: `${label} is invalid` }] }; + } + return { value: value as T }; + }, + }, + }; +} + +function isPlainObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} diff --git a/packages/reconciler/src/durable/types.ts b/packages/reconciler/src/durable/types.ts new file mode 100644 index 0000000..c139348 --- /dev/null +++ b/packages/reconciler/src/durable/types.ts @@ -0,0 +1,37 @@ +import type { BaseResource } from "@notation/resource"; +import type { ReconcilerEventEmitter } from "../events"; +import type { ResourceRegistry } from "../resource-registry"; +import type { DurableStateBackend } from "./state-backend"; + +export type PollOptions = { + maxAttempts: number; + retryInterval: number; +}; + +export const DEFAULT_RETRY_OPTIONS: PollOptions = { + maxAttempts: 10, + retryInterval: 1_000, +}; + +export const DEFAULT_READ_POLL_OPTIONS: PollOptions = { + maxAttempts: 30, + retryInterval: 1_000, +}; + +export type DurableOperationOptions = { + deploymentId: string; + executionId: string; + resources: BaseResource[]; + state: DurableStateBackend; + registry?: ResourceRegistry; + dryRun?: boolean; + emit?: ReconcilerEventEmitter; + retryOptions?: PollOptions; + readPollOptions?: PollOptions; +}; + +export type DurableDeployOptions = DurableOperationOptions & { + driftDetection?: boolean; +}; + +export type DurableDestroyOptions = DurableOperationOptions; diff --git a/packages/reconciler/src/durable/yieldstar.ts b/packages/reconciler/src/durable/yieldstar.ts new file mode 100644 index 0000000..13ea2dd --- /dev/null +++ b/packages/reconciler/src/durable/yieldstar.ts @@ -0,0 +1,8 @@ +import type { WorkflowFn } from "yieldstar"; + +export { RetryableError, defineStore } from "yieldstar"; +export type { WorkflowStore } from "yieldstar"; +export type { StandardSchemaV1, StoreClient } from "@yieldstar/core"; + +/** The durable step primitive the runtime hands to workflow functions. */ +export type DurableStep = Parameters>[0]; diff --git a/packages/reconciler/src/events.ts b/packages/reconciler/src/events.ts new file mode 100644 index 0000000..2cad300 --- /dev/null +++ b/packages/reconciler/src/events.ts @@ -0,0 +1,53 @@ +import type { ResourceType } from "@notation/resource"; + +export type OperationName = "create" | "read" | "update" | "delete"; + +export type OperationLifecycleStatus = + "start" | "success" | "error" | "skip" | "dry-run"; + +export type OperationLifecycleEvent = { + level: "info" | "error"; + event: "reconciler.operation.lifecycle"; + operation: OperationName; + status: OperationLifecycleStatus; + resourceId: string; + resourceType: ResourceType; + reason?: string; + errorName?: string; + errorMessage?: string; +}; + +export type ReconcilerDeployEvent = { + level: "info"; + event: "reconciler.deploy.decision"; + resourceId: string; + resourceType: string; + decision: "create" | "update" | "drift-update" | "drift-recreate" | "noop"; +}; + +export type ReconcilerDriftDetectedEvent = { + level: "info"; + event: "reconciler.drift.detected"; + resourceId: string; + resourceType: string; + diff: Record; +}; + +export type CoordinationWaitingEvent = { + level: "warn"; + event: "reconciler.coordination.waiting"; + deploymentId: string; + executionId: string; + holderExecutionId: string; +}; + +export type ReconcilerEvent = + | OperationLifecycleEvent + | CoordinationWaitingEvent + | ReconcilerDeployEvent + | ReconcilerDriftDetectedEvent + | import("./resource-registry").MissingResourceRegistryMatchWarningEvent; + +export type ReconcilerEventEmitter = ( + event: ReconcilerEvent, +) => void | Promise; diff --git a/packages/reconciler/src/planner.ts b/packages/reconciler/src/planner.ts new file mode 100644 index 0000000..6e90b56 --- /dev/null +++ b/packages/reconciler/src/planner.ts @@ -0,0 +1,81 @@ +import type { BaseResource } from "@notation/resource"; +import type { StateBackend } from "@notation/state"; +import { buildResourceDepthLevels } from "./dependency-graph"; +import { + decideAction, + getDependencyIds, + resolvePlanParams, + type Plan, + type PlanNode, +} from "./plan"; + +export type CreatePlanOptions = { + resources: BaseResource[]; + state: StateBackend; + driftDetection?: boolean; +}; + +export async function createPlan({ + resources, + state, + driftDetection = true, +}: CreatePlanOptions): Promise { + const resourceById = new Map( + resources.map((resource) => [resource.id, resource]), + ); + const nodes: PlanNode[] = []; + + for (const level of buildResourceDepthLevels(resources)) { + for (const resource of level) { + const stateNode = await state.get(resource.id); + if (stateNode) resource.setOutput(stateNode.output); + const params = await resolvePlanParams(resource); + let action = decideAction({ resource, stateNode, params }); + + if (action.decision === "noop" && driftDetection && resource.read) { + try { + const output = await resource.read(resource.key); + action = decideAction({ + resource, + stateNode, + params, + driftRead: { status: "found", output }, + }); + } catch (error) { + const notFound = resource.notFoundOnError?.some( + (matcher) => matcher.name === (error as Error)?.name, + ); + if (!notFound) throw error; + action = decideAction({ + resource, + stateNode, + params, + driftRead: { status: "not-found" }, + }); + } + } + + nodes.push({ + id: resource.id, + type: resource.type, + decision: action.decision, + ...("diff" in action ? { diff: action.diff } : {}), + params, + dependsOn: getDependencyIds(resource), + }); + } + } + + for (const stateNode of await state.values()) { + if (resourceById.has(stateNode.id)) continue; + nodes.push({ + id: stateNode.id, + type: stateNode.type, + decision: "delete-orphan", + params: stateNode.params, + dependsOn: [], + }); + } + + return { createdAt: new Date().toISOString(), nodes }; +} diff --git a/packages/reconciler/src/resource-registry.ts b/packages/reconciler/src/resource-registry.ts index 916c0ab..6916caa 100644 --- a/packages/reconciler/src/resource-registry.ts +++ b/packages/reconciler/src/resource-registry.ts @@ -6,7 +6,7 @@ export type MissingResourceRegistryMatchWarningEvent = { level: "warn"; event: "reconciler.orphan-deletion.skipped"; reason: "resource-type-not-registered"; - workflow: "deploy" | "refresh"; + workflow: "deploy" | "refresh" | "destroy"; resourceId: string; resourceType: ResourceType; }; @@ -46,7 +46,7 @@ export function resolveResourceClass( } export function createMissingResourceRegistryMatchWarningEvent(opts: { - workflow: "deploy" | "refresh"; + workflow: "deploy" | "refresh" | "destroy"; resourceId: string; resourceType: ResourceType; }): MissingResourceRegistryMatchWarningEvent { diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts new file mode 100644 index 0000000..1d55152 --- /dev/null +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -0,0 +1,531 @@ +import { + WorkflowRunner, + type HeapClient, + type WorkflowEvent, +} from "@yieldstar/core"; +import { + SqliteHeapClient, + SqliteStoreClient, + createSqliteDb, +} from "@yieldstar/sqlite-runtime/node"; +import { resource, type BaseResource } from "@notation/resource"; +import pino from "pino"; +import { createWorkflowRouter, workflow } from "yieldstar"; +import { describe, expect, it, vi } from "vitest"; +import * as durable from "../src/durable"; +import type { ReconcilerEvent } from "../src/events"; +import { + createResourceRegistry, + type ResourceRegistry, +} from "../src/resource-registry"; + +const logger = pino({ level: "silent" }); + +describe("durable execution and replay", () => { + it("waits durably for a retryable provider and persists after success", async () => { + let attempts = 0; + const PendingResource = resource({ type: "test/durable/pending" }) + .defineSchema({}) + .defineOperations({ + create: async () => { + attempts += 1; + if (attempts === 1) { + const error = new Error("provider is pending"); + error.name = "ProviderPending"; + throw error; + } + }, + delete: async () => undefined, + retryLaterOnError: [ + { name: "ProviderPending", reason: "provider is pending" }, + ], + }); + const runtime = createRuntime( + [new PendingResource({ id: "pending" })], + "durable-wait", + { retryOptions: { maxAttempts: 3, retryInterval: 1 } }, + ); + + await runtime.run("wait-execution"); + expect(attempts).toBe(1); + expect(runtime.scheduler.events).toHaveLength(1); + + await runtime.run("wait-execution"); + expect(attempts).toBe(2); + expect(await runtime.state.get("pending")).toMatchObject({ + id: "pending", + lastOperation: "create", + rev: 1, + }); + runtime.close(); + }); + + it("resumes after a crash without repeating a completed create", async () => { + const create = vi.fn(async () => undefined); + const TestResource = resource({ type: "test/durable/resume" }) + .defineSchema({}) + .defineOperations({ create, delete: async () => undefined }); + const runtime = createRuntime( + [new TestResource({ id: "resume" })], + "crash-resume", + { crashAfterStep: "notation:resource:resume:create" }, + ); + + await expect(runtime.run("resume-execution")).rejects.toThrow( + "simulated process crash", + ); + expect(create).toHaveBeenCalledOnce(); + expect(await runtime.state.get("resume")).toBeUndefined(); + + await runtime.run("resume-execution"); + expect(create).toHaveBeenCalledOnce(); + expect(await runtime.state.get("resume")).toMatchObject({ rev: 1 }); + runtime.close(); + }); + + it("resumes durable destroy after a crash without repeating delete", async () => { + const remove = vi.fn(async () => undefined); + const TestResource = resource({ type: "test/durable/destroy-resume" }) + .defineSchema({}) + .defineOperations({ create: async () => undefined, delete: remove }); + const runtime = createRuntime( + [new TestResource({ id: "destroyed" })], + "destroy-crash-resume", + { crashAfterStep: "notation:destroy:destroyed:delete" }, + ); + + await runtime.run("deploy-before-destroy"); + await expect(runtime.destroy("destroy-execution")).rejects.toThrow( + "simulated process crash", + ); + expect(remove).toHaveBeenCalledOnce(); + expect(await runtime.state.get("destroyed")).toBeDefined(); + + await runtime.destroy("destroy-execution"); + expect(remove).toHaveBeenCalledOnce(); + expect(await runtime.state.get("destroyed")).toBeUndefined(); + runtime.close(); + }); + + it("waits durably for a retryable delete before removing state", async () => { + let attempts = 0; + const PendingDelete = resource({ type: "test/durable/pending-delete" }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + delete: async () => { + attempts += 1; + if (attempts === 1) { + const error = new Error("delete is pending"); + error.name = "DeletePending"; + throw error; + } + }, + retryLaterOnError: [ + { name: "DeletePending", reason: "delete is pending" }, + ], + }); + const runtime = createRuntime( + [new PendingDelete({ id: "pending-delete" })], + "durable-destroy-wait", + { retryOptions: { maxAttempts: 3, retryInterval: 1 } }, + ); + + await runtime.run("deploy-before-wait"); + await runtime.destroy("destroy-wait"); + expect(attempts).toBe(1); + expect(await runtime.state.get("pending-delete")).toBeDefined(); + + await runtime.destroy("destroy-wait"); + expect(attempts).toBe(2); + expect(await runtime.state.get("pending-delete")).toBeUndefined(); + runtime.close(); + }); +}); + +describe("dependency ordering", () => { + it("destroys dependents before their dependencies", async () => { + const order: string[] = []; + const Dependency = resource({ type: "test/durable/dependency" }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + delete: async () => void order.push("dependency"), + }); + const Dependent = resource({ type: "test/durable/dependent" }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + delete: async () => void order.push("dependent"), + }); + const dependency = new Dependency({ id: "dependency" }); + const dependent = new Dependent({ + id: "dependent", + dependencies: { dependency }, + }); + const runtime = createRuntime([dependency, dependent], "destroy-order"); + + await runtime.run("deploy-before-ordered-destroy"); + await runtime.destroy("ordered-destroy"); + + expect(order).toEqual(["dependent", "dependency"]); + runtime.close(); + }); +}); + +describe("conditional state persistence", () => { + it("uses store identity and version for conditional update and delete", async () => { + const runtime = createRuntime([], "conditional-state"); + await runtime.state.update("resource", 0, statePatch("resource")); + const originalSnapshot = await runtime.state.snapshot("resource"); + + const first = runtime.state.update("resource", 1, { + output: { winner: "first" }, + }); + const second = runtime.state.update("resource", 1, { + output: { winner: "second" }, + }); + const results = await Promise.allSettled([first, second]); + + expect( + results.filter((result) => result.status === "fulfilled"), + ).toHaveLength(1); + expect( + results.filter((result) => result.status === "rejected"), + ).toHaveLength(1); + await expect(runtime.state.delete("resource", 1)).rejects.toMatchObject({ + name: "RevConflict", + }); + expect(await runtime.state.get("resource")).toMatchObject({ rev: 2 }); + + await runtime.state.clear(); + await runtime.state.update("resource", 0, statePatch("resource")); + const staleDelete = await runtime.storeClient.deleteStoreFrom({ + definition: durable.resourceStateStore, + id: runtime.state.storeId("resource"), + snapshot: originalSnapshot, + }); + expect(staleDelete).toMatchObject({ + deleted: false, + reason: "conflict", + }); + expect(await runtime.state.get("resource")).toMatchObject({ rev: 1 }); + runtime.close(); + }); +}); + +describe("deployment coordination", () => { + it("serializes concurrent deployments through durable store waiting", async () => { + let unblockCreate!: () => void; + const blocked = new Promise((resolve) => { + unblockCreate = resolve; + }); + let started!: () => void; + const createStarted = new Promise((resolve) => { + started = resolve; + }); + const create = vi.fn(async () => { + started(); + await blocked; + }); + const TestResource = resource({ type: "test/durable/concurrent" }) + .defineSchema({}) + .defineOperations({ create, delete: async () => undefined }); + const runtime = createRuntime( + [new TestResource({ id: "shared" })], + "concurrent", + ); + + const first = runtime.run("deployment-a"); + await createStarted; + await runtime.run("deployment-b"); + expect(create).toHaveBeenCalledOnce(); + + unblockCreate(); + await first; + const wake = runtime.scheduler.events.find( + (event) => event.executionId === "deployment-b", + ); + expect(wake).toBeDefined(); + await runtime.runner.run(wake!, logger); + + expect(create).toHaveBeenCalledOnce(); + expect(await runtime.state.values()).toHaveLength(1); + runtime.close(); + }); + + it("emits a coordination waiting event when another execution holds the deployment", async () => { + let unblockCreate!: () => void; + const blocked = new Promise((resolve) => { + unblockCreate = resolve; + }); + let started!: () => void; + const createStarted = new Promise((resolve) => { + started = resolve; + }); + const TestResource = resource({ type: "test/durable/coordination" }) + .defineSchema({}) + .defineOperations({ + create: async () => { + started(); + await blocked; + }, + delete: async () => undefined, + }); + const events: ReconcilerEvent[] = []; + const runtime = createRuntime( + [new TestResource({ id: "held" })], + "coordination-waiting", + { emit: (event) => void events.push(event) }, + ); + + const first = runtime.run("holder-execution"); + await createStarted; + await runtime.run("waiter-execution"); + + expect( + events.find((event) => event.event === "reconciler.coordination.waiting"), + ).toMatchObject({ + level: "warn", + deploymentId: "coordination-waiting", + executionId: "waiter-execution", + holderExecutionId: "holder-execution", + }); + + unblockCreate(); + await first; + runtime.close(); + }); +}); + +describe("deployment scoping", () => { + it("scopes store listing to the exact deployment despite prefix-like IDs", async () => { + const database = createSqliteDb({ path: ":memory:" }); + const storeClient = new SqliteStoreClient({ + db: database, + schedulerClient: new TestScheduler(), + }); + const app = new durable.DurableStateBackend(storeClient, "app"); + const appBlue = new durable.DurableStateBackend(storeClient, "app:blue"); + + await app.update("site", 0, statePatch("site")); + await appBlue.update("site", 0, statePatch("site")); + + expect(await app.values()).toHaveLength(1); + expect(await appBlue.values()).toHaveLength(1); + + await app.clear(); + expect(await app.values()).toHaveLength(0); + expect(await appBlue.values()).toHaveLength(1); + expect(await appBlue.get("site")).toBeDefined(); + database.close(); + }); +}); + +describe("orphan deletion", () => { + it("deletes orphaned resources through the registry on a later deployment", async () => { + const deleteSpy = vi.fn(async () => undefined); + const OrphanResource = resource({ type: "test/durable/orphan" }) + .defineSchema({}) + .defineOperations({ create: async () => undefined, delete: deleteSpy }); + const resources: BaseResource[] = [new OrphanResource({ id: "orphan" })]; + const runtime = createRuntime(resources, "orphan-deletion", { + registry: createResourceRegistry([OrphanResource]), + }); + + await runtime.run("deploy-1"); + expect(await runtime.state.values()).toHaveLength(1); + + resources.length = 0; + await runtime.run("deploy-2"); + + expect(deleteSpy).toHaveBeenCalledOnce(); + expect(await runtime.state.values()).toHaveLength(0); + expect(await runtime.state.get("orphan")).toBeUndefined(); + runtime.close(); + }); +}); + +describe("drift detection and repair", () => { + it("emits drift detection and repairs remote drift with update", async () => { + let remote = { name: "expected" }; + const updateSpy = vi.fn(async () => { + remote = { name: "expected" }; + }); + const DriftResource = resource({ type: "test/durable/drift" }) + .defineSchema({ + name: { + presence: "required", + propertyType: "param", + valueType: "string" as any, + }, + }) + .defineOperations({ + create: async () => remote, + read: async () => remote, + update: updateSpy, + delete: async () => undefined, + }); + const events: ReconcilerEvent[] = []; + const runtime = createRuntime( + [new DriftResource({ id: "drifted", config: { name: "expected" } })], + "drift-repair", + { driftDetection: true, emit: (event) => void events.push(event) }, + ); + + await runtime.run("deploy-1"); + remote = { name: "drifted" }; + await runtime.run("deploy-2"); + + expect(updateSpy).toHaveBeenCalledOnce(); + expect( + events.find((event) => event.event === "reconciler.drift.detected"), + ).toMatchObject({ resourceId: "drifted", diff: { name: "expected" } }); + expect( + events.filter( + (event) => + event.event === "reconciler.deploy.decision" && + event.decision === "drift-update", + ), + ).toHaveLength(1); + runtime.close(); + }); +}); + +function createRuntime( + resources: BaseResource[], + deploymentId: string, + options: { + retryOptions?: { maxAttempts: number; retryInterval: number }; + crashAfterStep?: string; + registry?: ResourceRegistry; + driftDetection?: boolean; + emit?: (event: ReconcilerEvent) => void; + } = {}, +) { + const database = createSqliteDb({ path: ":memory:" }); + const scheduler = new TestScheduler(); + const sqliteHeap = new SqliteHeapClient(database); + const heap = options.crashAfterStep + ? new CrashAfterWriteHeap(sqliteHeap, options.crashAfterStep) + : sqliteHeap; + const storeClient = new SqliteStoreClient({ + db: database, + schedulerClient: scheduler, + }); + const state = new durable.DurableStateBackend(storeClient, deploymentId); + const deploy = workflow(async function* (step, event) { + yield* durable.deploy(step, { + deploymentId, + executionId: event.executionId, + resources, + state, + registry: options.registry, + driftDetection: options.driftDetection ?? false, + emit: options.emit, + retryOptions: options.retryOptions, + }); + }); + const destroy = workflow(async function* (step, event) { + yield* durable.destroy(step, { + deploymentId, + executionId: event.executionId, + resources, + state, + registry: options.registry, + emit: options.emit, + retryOptions: options.retryOptions, + }); + }); + const router = createWorkflowRouter({ deploy, destroy }); + const runner = new WorkflowRunner({ + router, + heapClient: heap, + storeClient, + schedulerClient: scheduler, + logger, + }); + + return { + runner, + scheduler, + state, + storeClient, + run(executionId: string) { + return runner.run( + { + workflowId: "deploy", + executionId, + params: {}, + context: new Map(), + }, + logger, + ); + }, + destroy(executionId: string) { + return runner.run( + { + workflowId: "destroy", + executionId, + params: {}, + context: new Map(), + }, + logger, + ); + }, + close() { + database.close(); + }, + }; +} + +class TestScheduler { + readonly events: WorkflowEvent[] = []; + + async requestWakeUp(event: WorkflowEvent) { + this.events.push(event); + } +} + +class CrashAfterWriteHeap implements HeapClient { + #crashed = false; + + constructor( + private readonly inner: HeapClient, + private readonly crashAfterStep: string, + ) {} + + readStep(params: { executionId: string; stepKey: string }) { + return this.inner.readStep(params); + } + + async writeStep(params: { + executionId: string; + stepKey: string; + stepAttempt: number; + stepDone: boolean; + stepResponseJson: string; + }) { + await this.inner.writeStep(params); + if ( + !this.#crashed && + params.stepKey === this.crashAfterStep && + params.stepDone + ) { + this.#crashed = true; + throw new Error("simulated process crash"); + } + } +} + +function statePatch(id: string) { + return { + id, + type: "test/durable/state", + config: {}, + params: {}, + output: {}, + lastOperation: "create" as const, + lastOperationAt: new Date().toISOString(), + }; +} diff --git a/packages/reconciler/test/planner.test.ts b/packages/reconciler/test/planner.test.ts new file mode 100644 index 0000000..a215e82 --- /dev/null +++ b/packages/reconciler/test/planner.test.ts @@ -0,0 +1,36 @@ +import { resource } from "@notation/resource"; +import { MemoryStateBackend } from "@notation/state"; +import { describe, expect, it } from "vitest"; +import { createPlan } from "../src/planner"; + +describe("createPlan", () => { + it("plans desired creates and persisted orphans without mutation execution", async () => { + const TestResource = resource({ type: "test/planner/resource" }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + delete: async () => undefined, + }); + const state = new MemoryStateBackend(); + await state.update("orphan", 0, { + id: "orphan", + type: TestResource.type, + config: {}, + params: {}, + output: {}, + lastOperation: "create", + lastOperationAt: "2026-07-22T00:00:00.000Z", + }); + + const plan = await createPlan({ + resources: [new TestResource({ id: "desired" })], + state, + driftDetection: false, + }); + + expect(plan.nodes).toEqual([ + expect.objectContaining({ id: "desired", decision: "create" }), + expect.objectContaining({ id: "orphan", decision: "delete-orphan" }), + ]); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 82d8d14..82b4901 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -345,12 +345,25 @@ importers: '@notation/state': specifier: workspace:* version: link:../state + '@notation/utils': + specifier: workspace:* + version: link:../utils + '@yieldstar/core': + specifier: 0.5.0 + version: 0.5.0 deep-object-diff: specifier: ^1.1.9 version: 1.1.9 yieldstar: - specifier: ^0.4.6 - version: 0.4.6 + specifier: 0.5.0 + version: 0.5.0 + devDependencies: + '@yieldstar/sqlite-runtime': + specifier: 0.5.0 + version: 0.5.0 + pino: + specifier: ^9.9.0 + version: 9.14.0 packages/resource: {} @@ -1540,8 +1553,12 @@ packages: '@vitest/utils@4.1.10': resolution: {integrity: sha512-fy9am/HWxbaGt/Sawrp90vt6Y6jQwf1RX77cz3uwoJwJVMli/e1IEwRPnMNJ7vKfPTwo0diXifkpPvwH9v7nGA==} - '@yieldstar/core@0.4.6': - resolution: {integrity: sha512-6aJQ2NwA07YKdQpiyxdg+SCKwnHQsuxAGGW6uKZYydTS10ljL3uTE6mKdrt/22ehOtbXzjRa/xnECAWh3GM7pw==} + '@yieldstar/core@0.5.0': + resolution: {integrity: sha512-KaN1+AVg54W9G4VXHNmCRixV0MY123714YZgaA9EZx7mDgB0vfFA6T71rUnldGH86QEkocCwI4GCZzZ+0G4fyQ==} + + '@yieldstar/sqlite-runtime@0.5.0': + resolution: {integrity: sha512-zdG4kvOzEJFrTW52SY+WtdUuSvKhXs438H2/+TQotwi3fltLqva9EMgRJ/l9mcvC7nqAq1i0g2v+TZWjO/py4A==} + engines: {node: '>=22.6'} abstract-logging@2.0.1: resolution: {integrity: sha512-2BjRTZxTPvheOvGbBslFSYOUkr+SjPtOnrLP33f+VIWLzezQpZcqVg7ja3L4dBXmzzgwT+a029jRx5PCi3JuiA==} @@ -2228,9 +2245,6 @@ packages: pino-abstract-transport@3.0.0: resolution: {integrity: sha512-wlfUczU+n7Hy/Ha5j9a/gZNy7We5+cXp8YL+X+PG8S0KXxw7n/JXA3c46Y0zQznIJ83URJiwy7Lh56WLokNuxg==} - pino-std-serializers@7.0.0: - resolution: {integrity: sha512-e906FRY0+tV27iq4juKzSYPbUj2do2X2JX4EzSca1631EB2QJQUqGbDuERal7LCtOpxl6x3+nvo9NPZcmjkiFA==} - pino-std-serializers@7.1.0: resolution: {integrity: sha512-BndPH67/JxGExRgiX1dX0w1FvZck5Wa4aal9198SrRhZjH3GxKQUKIBnYJTdj2HDN3UQAS06HlfcSbQj2OHmaw==} @@ -2421,9 +2435,6 @@ packages: resolution: {integrity: sha512-g9Q1haeby36OSStwb4ntCGGGaKsaVSjQ68fBxoQcutl5fS1vuY18H3wSt3jFyFtrkx+Kz0V1G85A4MyAdDMi2Q==} engines: {node: '>=8'} - sonic-boom@4.2.0: - resolution: {integrity: sha512-INb7TM37/mAcsGmc9hyyI6+QR3rR1zVRu36B0NeGXKnOOLiZOfER5SA+N7X7k3yUYRzLWafduTDvJAfDswwEww==} - sonic-boom@4.2.1: resolution: {integrity: sha512-w6AxtubXa2wTXAUsZMMWERrsIRAdrK0Sc+FUytWvYAhBJLyuI4llrMIC1DtlNSdI99EI86KZum2MMq3EAZlF9Q==} @@ -2600,6 +2611,10 @@ packages: util-deprecate@1.0.2: resolution: {integrity: sha512-EPD5q1uXyFxJpCrLnCc1nHnq3gOa6DZBocAIiI2TaSCA7VCJ1UJDMagCzIkXNsUYfD1daK//LTEQ8xiIbrHtcw==} + uuid@11.1.1: + resolution: {integrity: sha512-vIYxrBCC/N/K+Js3qSN88go7kIfNPssr/hHCesKCQNAjmgvYS2oqr69kIufEG+O4+PfezOH4EbIeHCfFov8ZgQ==} + hasBin: true + vite@8.1.3: resolution: {integrity: sha512-Ds+gBRbj0lwRO2Y5hwnUBdxSwlAve9LeRyU4sNnAr0ewW0gWF0n5bgXgUzbgZ49MV9BVUAQUFYVcDUcilUExMA==} engines: {node: ^20.19.0 || >=22.12.0} @@ -2701,8 +2716,8 @@ packages: yallist@3.1.1: resolution: {integrity: sha512-a4UGQaWPH59mOXUYnAG2ewncQS4i4F43Tv3JoAM+s2VDAmS9NsK8GpDMLrCHPksFT7h3K6TOoUNn2pb7RoXx4g==} - yieldstar@0.4.6: - resolution: {integrity: sha512-toVxX0hHx+AlD5S3MT7voOW/Jw7AX5Mqejp28BJErSHGdZVcuKyVIfzYknbfnRnmBxAeZd3wfVnk7JQJQzZUZQ==} + yieldstar@0.5.0: + resolution: {integrity: sha512-MKuo2uaHYdy+1u0O0Q3Po3eVLGEmiNACsbaQ9aqOD4miZyRT63fXc/mhk0V4n8fNtoMN1BORD9QiXq5ZuOBtkQ==} yoctocolors@2.1.1: resolution: {integrity: sha512-GQHQqAopRhwU8Kt1DDM8NjibDXHC8eoh1erhGAJPEyveY9qqVeXvVikNKrDz69sHowPMorbPUrH/mx8c50eiBQ==} @@ -3835,7 +3850,15 @@ snapshots: convert-source-map: 2.0.0 tinyrainbow: 3.1.0 - '@yieldstar/core@0.4.6': {} + '@yieldstar/core@0.5.0': + dependencies: + '@standard-schema/spec': 1.1.0 + + '@yieldstar/sqlite-runtime@0.5.0': + dependencies: + '@yieldstar/core': 0.5.0 + pino: 9.14.0 + uuid: 11.1.1 abstract-logging@2.0.1: {} @@ -4461,8 +4484,6 @@ snapshots: dependencies: split2: 4.2.0 - pino-std-serializers@7.0.0: {} - pino-std-serializers@7.1.0: {} pino@10.3.1: @@ -4485,12 +4506,12 @@ snapshots: atomic-sleep: 1.0.0 on-exit-leak-free: 2.1.2 pino-abstract-transport: 2.0.0 - pino-std-serializers: 7.0.0 + pino-std-serializers: 7.1.0 process-warning: 5.0.0 quick-format-unescaped: 4.0.4 real-require: 0.2.0 safe-stable-stringify: 2.5.0 - sonic-boom: 4.2.0 + sonic-boom: 4.2.1 thread-stream: 3.1.0 pirates@4.0.7: {} @@ -4661,10 +4682,6 @@ snapshots: slash@3.0.0: {} - sonic-boom@4.2.0: - dependencies: - atomic-sleep: 1.0.0 - sonic-boom@4.2.1: dependencies: atomic-sleep: 1.0.0 @@ -4816,6 +4833,8 @@ snapshots: util-deprecate@1.0.2: {} + uuid@11.1.1: {} + vite@8.1.3(@types/node@22.13.4)(esbuild@0.28.1)(jiti@2.7.0): dependencies: lightningcss: 1.32.0 @@ -4869,9 +4888,9 @@ snapshots: yallist@3.1.1: {} - yieldstar@0.4.6: + yieldstar@0.5.0: dependencies: - '@yieldstar/core': 0.4.6 + '@yieldstar/core': 0.5.0 nanoid: 5.1.16 pino: 9.14.0 serialize-error: 11.0.3 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 8d0e4c7..d1d8a41 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -3,6 +3,10 @@ packages: - "packages/*" allowBuilds: esbuild: true +minimumReleaseAgeExclude: + - "@yieldstar/core@0.5.0" + - "@yieldstar/sqlite-runtime@0.5.0" + - yieldstar@0.5.0 overrides: lodash-es@<4.18.1: ^4.18.1 js-yaml@3: 3.15.0 From 351384eb144fd44f5842b018995a3b02e2733f99 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:22:35 +0100 Subject: [PATCH 02/17] Harden durable reconciliation semantics --- .../reconciler/src/durable/coordination.ts | 4 +- packages/reconciler/src/durable/emit.ts | 9 +-- packages/reconciler/src/durable/operations.ts | 27 +++++-- .../reconciler/src/durable/state-backend.ts | 52 ++++++++++---- packages/reconciler/src/durable/stores.ts | 10 ++- packages/reconciler/src/planner.ts | 6 +- .../test/durable-reconciliation.test.ts | 72 ++++++++++++++++++- packages/reconciler/test/planner.test.ts | 38 ++++++++++ 8 files changed, 182 insertions(+), 36 deletions(-) diff --git a/packages/reconciler/src/durable/coordination.ts b/packages/reconciler/src/durable/coordination.ts index 7dac2d2..2fb2579 100644 --- a/packages/reconciler/src/durable/coordination.ts +++ b/packages/reconciler/src/durable/coordination.ts @@ -1,5 +1,5 @@ import type { ReconcilerEventEmitter } from "../events"; -import { emitOnce } from "./emit"; +import { emitEvent } from "./emit"; import { deploymentCoordinationStore, type CoordinationState } from "./stores"; import type { DurableStep, WorkflowStore } from "./yieldstar"; @@ -25,7 +25,7 @@ export async function* acquireDeploymentCoordination( const snapshot = yield* coordination.get("notation:coordination:inspect"); const holder = snapshot.state.holder; if (holder !== null && holder !== opts.executionId) { - yield* emitOnce(step, "notation:coordination:waiting", opts.emit, () => ({ + yield* emitEvent(step, "notation:coordination:waiting", opts.emit, () => ({ level: "warn", event: "reconciler.coordination.waiting", deploymentId: opts.deploymentId, diff --git a/packages/reconciler/src/durable/emit.ts b/packages/reconciler/src/durable/emit.ts index 7be77b1..6cc14a9 100644 --- a/packages/reconciler/src/durable/emit.ts +++ b/packages/reconciler/src/durable/emit.ts @@ -6,11 +6,8 @@ import type { } from "../events"; import type { DurableStep } from "./yieldstar"; -/** - * Emits an event inside a durable step so a replayed workflow does not - * re-emit events it already sent. - */ -export function emitOnce( +/** Checkpoints delivery after the emitter returns. Emitters must tolerate a duplicate if the process crashes before that checkpoint. */ +export function emitEvent( step: DurableStep, key: string, emit: ReconcilerEventEmitter | undefined, @@ -30,7 +27,7 @@ export function emitLifecycle( resource: LifecycleResource, extra: { reason?: string; error?: unknown } = {}, ) { - return emitOnce(step, key, emit, () => + return emitEvent(step, key, emit, () => createLifecycleEvent(operation, status, resource, extra), ); } diff --git a/packages/reconciler/src/durable/operations.ts b/packages/reconciler/src/durable/operations.ts index 6bca1d2..c9bf94c 100644 --- a/packages/reconciler/src/durable/operations.ts +++ b/packages/reconciler/src/durable/operations.ts @@ -10,7 +10,7 @@ import { resolveResourceClass, } from "../resource-registry"; import { decideAction, type ResourceAction } from "../plan"; -import { emitLifecycle, emitOnce } from "./emit"; +import { emitEvent, emitLifecycle } from "./emit"; import type { DurableStateBackend } from "./state-backend"; import { resourceStateStore, @@ -82,7 +82,7 @@ export async function* reconcileResource( if (action.decision === "drift-update") { const diff = action.patch; - yield* emitOnce(step, `${prefix}:drift-detected`, opts.emit, () => ({ + yield* emitEvent(step, `${prefix}:drift-detected`, opts.emit, () => ({ level: "info", event: "reconciler.drift.detected", resourceId: resource.id, @@ -91,7 +91,7 @@ export async function* reconcileResource( })); } - yield* emitOnce(step, `${prefix}:decision`, opts.emit, () => ({ + yield* emitEvent(step, `${prefix}:decision`, opts.emit, () => ({ level: "info", event: "reconciler.deploy.decision", resourceId: resource.id, @@ -126,8 +126,8 @@ export async function* reconcileResource( } try { - // Execute the provider call. Each call runs in its own durable step, so a - // replayed workflow never repeats a completed provider mutation. + // Checkpoint successful provider calls. Provider mutations must be + // idempotent because a crash before the checkpoint can repeat them. if (operation === "create") { const primaryKey = yield* runProviderCall( step, @@ -174,6 +174,7 @@ export async function* reconcileResource( resource, opts, `${prefix}:read-after-write`, + { retryNotFound: true }, ); if (read.status === "found") resource.setOutput({ ...resource.output, ...read.output }); @@ -326,6 +327,7 @@ export async function* readRemote( resource: BaseResource, opts: DurableOperationOptions, key: string, + behaviour: { retryNotFound?: boolean } = {}, ): AsyncGenerator< any, | { status: "found"; output: Record } @@ -360,7 +362,18 @@ export async function* readRemote( ); try { const output = yield* step.run(key, async () => { - const value = await resource.read!(resource.key); + let value: Record; + try { + value = await resource.read!(resource.key); + } catch (error) { + const notFound = findErrorMatcher(error, resource.notFoundOnError); + if (behaviour.retryNotFound && notFound) { + throw new RetryableError(notFound.reason, { + ...(opts.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), + }); + } + throw error; + } // An unsettled read (a declared condition not yet met) re-polls // durably instead of returning a half-provisioned remote. const unsettled = (resource.retryReadOnCondition ?? []) @@ -468,7 +481,7 @@ export async function* sweepOrphans( const Resource = resolveResourceClass(registry, node.type as ResourceType); if (!Resource) { - yield* emitOnce(step, params.warningKey(node.id), opts.emit, () => + yield* emitEvent(step, params.warningKey(node.id), opts.emit, () => createMissingResourceRegistryMatchWarningEvent({ workflow: params.workflow, resourceId: node.id, diff --git a/packages/reconciler/src/durable/state-backend.ts b/packages/reconciler/src/durable/state-backend.ts index e15d498..b3703da 100644 --- a/packages/reconciler/src/durable/state-backend.ts +++ b/packages/reconciler/src/durable/state-backend.ts @@ -1,5 +1,7 @@ import { RevConflict, type StateNode } from "@notation/state"; +import { randomUUID } from "node:crypto"; import { + RESOURCE_CREATION_TOKEN, resourceStateStore, toStateNode, withoutRev, @@ -9,12 +11,10 @@ import type { StoreClient } from "./yieldstar"; export class DurableStateBackend { readonly #client: StoreClient; - readonly #deploymentId: string; readonly #prefix: string; constructor(client: StoreClient, deploymentId: string) { this.#client = client; - this.#deploymentId = deploymentId; // Keep deployment prefixes disjoint so orphan cleanup cannot delete // another deployment's stores. this.#prefix = `${encodeURIComponent(deploymentId)}:`; @@ -60,12 +60,20 @@ export class DurableStateBackend { const snapshot = await this.#tryGetSnapshot(storeId); if (!snapshot) { if (expectedRev !== 0) throw new RevConflict(id, expectedRev, undefined); - const initial = { ...patch, id } as StoredResourceState; + const creationToken = randomUUID(); + const initial = { + ...patch, + id, + [RESOURCE_CREATION_TOKEN]: creationToken, + } as StoredResourceState; const created = await this.#client.getOrCreateStore({ definition: resourceStateStore, id: storeId, initial, }); + if (created.state[RESOURCE_CREATION_TOKEN] !== creationToken) { + throw new RevConflict(id, expectedRev, created.version + 1); + } return { rev: created.version + 1 }; } @@ -80,7 +88,8 @@ export class DurableStateBackend { Object.assign(draft, withoutRev(patch)); }, }); - if (!result.updated) throw new RevConflict(id, expectedRev, undefined); + if (!result.updated) + throw new RevConflict(id, expectedRev, result.actualVersion + 1); return { rev: result.version + 1 }; } @@ -99,7 +108,12 @@ export class DurableStateBackend { id: storeId, snapshot, }); - if (!result.deleted) throw new RevConflict(id, expectedRev, undefined); + if (!result.deleted) + throw new RevConflict( + id, + expectedRev, + result.reason === "conflict" ? result.actualVersion + 1 : undefined, + ); } async values(): Promise { @@ -123,15 +137,27 @@ export class DurableStateBackend { async clear(): Promise { const ids = await this.#client.listStores(resourceStateStore); + const scopedIds = ids.filter((id) => id.startsWith(this.#prefix)); + const snapshots = await Promise.all( + scopedIds.map((id) => this.#tryGetSnapshot(id)), + ); await Promise.all( - ids - .filter((id) => id.startsWith(this.#prefix)) - .map((id) => - this.#client.deleteStore({ - definition: resourceStateStore, - id, - }), - ), + scopedIds.map(async (id, index) => { + const snapshot = snapshots[index]; + if (!snapshot) return; + const result = await this.#client.deleteStoreFrom({ + definition: resourceStateStore, + id, + snapshot, + }); + if (!result.deleted && result.reason === "conflict") { + throw new RevConflict( + id.slice(this.#prefix.length), + snapshot.version + 1, + result.actualVersion + 1, + ); + } + }), ); } } diff --git a/packages/reconciler/src/durable/stores.ts b/packages/reconciler/src/durable/stores.ts index fba2cb3..e2a1920 100644 --- a/packages/reconciler/src/durable/stores.ts +++ b/packages/reconciler/src/durable/stores.ts @@ -1,7 +1,11 @@ import type { StateNode } from "@notation/state"; import { defineStore, type StandardSchemaV1 } from "./yieldstar"; -export type StoredResourceState = Omit; +export const RESOURCE_CREATION_TOKEN = "$notationCreateToken"; + +export type StoredResourceState = Omit & { + [RESOURCE_CREATION_TOKEN]?: string; +}; export type CoordinationState = { holder: string | null }; const storedResourceStateSchema = plainObjectSchema( @@ -34,7 +38,9 @@ export function toStateNode(snapshot: { state: StoredResourceState; version: number; }): StateNode { - return { ...snapshot.state, rev: snapshot.version + 1 } as StateNode; + const { [RESOURCE_CREATION_TOKEN]: _creationToken, ...state } = + snapshot.state; + return { ...state, rev: snapshot.version + 1 } as StateNode; } export function withoutRev( diff --git a/packages/reconciler/src/planner.ts b/packages/reconciler/src/planner.ts index 6e90b56..1826703 100644 --- a/packages/reconciler/src/planner.ts +++ b/packages/reconciler/src/planner.ts @@ -1,4 +1,4 @@ -import type { BaseResource } from "@notation/resource"; +import { findErrorMatcher, type BaseResource } from "@notation/resource"; import type { StateBackend } from "@notation/state"; import { buildResourceDepthLevels } from "./dependency-graph"; import { @@ -42,9 +42,7 @@ export async function createPlan({ driftRead: { status: "found", output }, }); } catch (error) { - const notFound = resource.notFoundOnError?.some( - (matcher) => matcher.name === (error as Error)?.name, - ); + const notFound = findErrorMatcher(error, resource.notFoundOnError); if (!notFound) throw error; action = decideAction({ resource, diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index 1d55152..e6e44cc 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -60,7 +60,7 @@ describe("durable execution and replay", () => { runtime.close(); }); - it("resumes after a crash without repeating a completed create", async () => { + it("resumes after a crash following the create checkpoint", async () => { const create = vi.fn(async () => undefined); const TestResource = resource({ type: "test/durable/resume" }) .defineSchema({}) @@ -83,7 +83,7 @@ describe("durable execution and replay", () => { runtime.close(); }); - it("resumes durable destroy after a crash without repeating delete", async () => { + it("resumes after a crash following the delete checkpoint", async () => { const remove = vi.fn(async () => undefined); const TestResource = resource({ type: "test/durable/destroy-resume" }) .defineSchema({}) @@ -141,6 +141,46 @@ describe("durable execution and replay", () => { expect(await runtime.state.get("pending-delete")).toBeUndefined(); runtime.close(); }); + + it("retries a post-write not-found before persisting state", async () => { + let reads = 0; + const EventuallyReadable = resource({ + type: "test/durable/eventually-readable", + }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + read: async () => { + reads += 1; + if (reads === 1) { + const error = new Error("not visible yet"); + error.name = "NotFound"; + throw error; + } + return {}; + }, + delete: async () => undefined, + notFoundOnError: [ + { name: "NotFound", reason: "resource is not visible yet" }, + ], + }); + const runtime = createRuntime( + [new EventuallyReadable({ id: "eventually-readable" })], + "post-write-read", + { readPollOptions: { maxAttempts: 3, retryInterval: 1 } }, + ); + + await runtime.run("post-write-read-execution"); + expect(reads).toBe(1); + expect(await runtime.state.get("eventually-readable")).toBeUndefined(); + + await runtime.run("post-write-read-execution"); + expect(reads).toBe(2); + expect(await runtime.state.get("eventually-readable")).toMatchObject({ + rev: 1, + }); + runtime.close(); + }); }); describe("dependency ordering", () => { @@ -174,6 +214,31 @@ describe("dependency ordering", () => { }); describe("conditional state persistence", () => { + it("allows only one concurrent create-if-absent", async () => { + const runtime = createRuntime([], "conditional-create"); + const first = runtime.state.update("resource", 0, { + ...statePatch("resource"), + output: { winner: "first" }, + }); + const second = runtime.state.update("resource", 0, { + ...statePatch("resource"), + output: { winner: "second" }, + }); + + const results = await Promise.allSettled([first, second]); + + expect( + results.filter((result) => result.status === "fulfilled"), + ).toHaveLength(1); + expect( + results.filter((result) => result.status === "rejected"), + ).toHaveLength(1); + const state = await runtime.state.get("resource"); + expect(state).toMatchObject({ rev: 1 }); + expect(state).not.toHaveProperty("$notationCreateToken"); + runtime.close(); + }); + it("uses store identity and version for conditional update and delete", async () => { const runtime = createRuntime([], "conditional-state"); await runtime.state.update("resource", 0, statePatch("resource")); @@ -397,6 +462,7 @@ function createRuntime( deploymentId: string, options: { retryOptions?: { maxAttempts: number; retryInterval: number }; + readPollOptions?: { maxAttempts: number; retryInterval: number }; crashAfterStep?: string; registry?: ResourceRegistry; driftDetection?: boolean; @@ -424,6 +490,7 @@ function createRuntime( driftDetection: options.driftDetection ?? false, emit: options.emit, retryOptions: options.retryOptions, + readPollOptions: options.readPollOptions, }); }); const destroy = workflow(async function* (step, event) { @@ -435,6 +502,7 @@ function createRuntime( registry: options.registry, emit: options.emit, retryOptions: options.retryOptions, + readPollOptions: options.readPollOptions, }); }); const router = createWorkflowRouter({ deploy, destroy }); diff --git a/packages/reconciler/test/planner.test.ts b/packages/reconciler/test/planner.test.ts index a215e82..f0011e5 100644 --- a/packages/reconciler/test/planner.test.ts +++ b/packages/reconciler/test/planner.test.ts @@ -33,4 +33,42 @@ describe("createPlan", () => { expect.objectContaining({ id: "orphan", decision: "delete-orphan" }), ]); }); + + it("honours the message constraint in not-found matchers", async () => { + const TestResource = resource({ type: "test/planner/not-found" }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + read: async () => { + const error = new Error("access denied"); + error.name = "ProviderError"; + throw error; + }, + delete: async () => undefined, + notFoundOnError: [ + { + name: "ProviderError", + message: "not found", + reason: "resource does not exist", + }, + ], + }); + const state = new MemoryStateBackend(); + await state.update("existing", 0, { + id: "existing", + type: TestResource.type, + config: {}, + params: {}, + output: {}, + lastOperation: "create", + lastOperationAt: "2026-07-22T00:00:00.000Z", + }); + + await expect( + createPlan({ + resources: [new TestResource({ id: "existing" })], + state, + }), + ).rejects.toThrow("access denied"); + }); }); From 65d7badb78f7d9aa21c82c9d65d6032a246422c4 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:33:13 +0100 Subject: [PATCH 03/17] Protect durable state revisions --- packages/reconciler/src/durable/state-backend.ts | 2 +- packages/reconciler/test/durable-reconciliation.test.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/reconciler/src/durable/state-backend.ts b/packages/reconciler/src/durable/state-backend.ts index b3703da..88fa35c 100644 --- a/packages/reconciler/src/durable/state-backend.ts +++ b/packages/reconciler/src/durable/state-backend.ts @@ -62,7 +62,7 @@ export class DurableStateBackend { if (expectedRev !== 0) throw new RevConflict(id, expectedRev, undefined); const creationToken = randomUUID(); const initial = { - ...patch, + ...withoutRev(patch), id, [RESOURCE_CREATION_TOKEN]: creationToken, } as StoredResourceState; diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index e6e44cc..78ea78c 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -218,10 +218,12 @@ describe("conditional state persistence", () => { const runtime = createRuntime([], "conditional-create"); const first = runtime.state.update("resource", 0, { ...statePatch("resource"), + rev: 41, output: { winner: "first" }, }); const second = runtime.state.update("resource", 0, { ...statePatch("resource"), + rev: 42, output: { winner: "second" }, }); From 73546cf73349b27583b353365782b8a442f02763 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:10:14 +0100 Subject: [PATCH 04/17] Replace durable store schemas with Valibot --- packages/reconciler/package.json | 1 + packages/reconciler/src/durable/stores.ts | 72 +++++++------------- packages/reconciler/src/durable/yieldstar.ts | 2 +- pnpm-lock.yaml | 18 ++++- 4 files changed, 40 insertions(+), 53 deletions(-) diff --git a/packages/reconciler/package.json b/packages/reconciler/package.json index 3429d0d..d1ec1db 100644 --- a/packages/reconciler/package.json +++ b/packages/reconciler/package.json @@ -16,6 +16,7 @@ "@notation/state": "workspace:*", "@yieldstar/core": "0.5.0", "deep-object-diff": "^1.1.9", + "valibot": "^1.4.2", "yieldstar": "0.5.0" }, "devDependencies": { diff --git a/packages/reconciler/src/durable/stores.ts b/packages/reconciler/src/durable/stores.ts index e2a1920..d25f3ab 100644 --- a/packages/reconciler/src/durable/stores.ts +++ b/packages/reconciler/src/durable/stores.ts @@ -1,46 +1,42 @@ import type { StateNode } from "@notation/state"; -import { defineStore, type StandardSchemaV1 } from "./yieldstar"; +import * as v from "valibot"; +import { defineStore } from "./yieldstar"; export const RESOURCE_CREATION_TOKEN = "$notationCreateToken"; -export type StoredResourceState = Omit & { - [RESOURCE_CREATION_TOKEN]?: string; -}; -export type CoordinationState = { holder: string | null }; - -const storedResourceStateSchema = plainObjectSchema( - "Stored resource state", - (value) => - typeof value.id === "string" && - typeof value.type === "string" && - isPlainObject(value.config) && - isPlainObject(value.params) && - isPlainObject(value.output), -); -const coordinationStateSchema = plainObjectSchema( - "Deployment coordination state", - (value) => - "holder" in value && - (value.holder === null || typeof value.holder === "string"), -); - export const resourceStateStore = defineStore( - "notation/resource-state", - storedResourceStateSchema, + "resource-state", + v.looseObject({ + id: v.string(), + type: v.string(), + config: v.record(v.string(), v.unknown()), + params: v.record(v.string(), v.unknown()), + output: v.record(v.string(), v.unknown()), + lastOperation: v.picklist(["drift", "create", "update", "delete"]), + lastOperationAt: v.string(), + [RESOURCE_CREATION_TOKEN]: v.optional(v.string()), + }), ); export const deploymentCoordinationStore = defineStore( - "notation/deployment-coordination", - coordinationStateSchema, + "deployment-coordination", + v.object({ holder: v.nullable(v.string()) }), ); +export type StoredResourceState = v.InferOutput< + typeof resourceStateStore.schema +>; +export type CoordinationState = v.InferOutput< + typeof deploymentCoordinationStore.schema +>; + export function toStateNode(snapshot: { state: StoredResourceState; version: number; }): StateNode { const { [RESOURCE_CREATION_TOKEN]: _creationToken, ...state } = snapshot.state; - return { ...state, rev: snapshot.version + 1 } as StateNode; + return { ...state, rev: snapshot.version + 1 }; } export function withoutRev( @@ -49,25 +45,3 @@ export function withoutRev( const { rev: _rev, ...stored } = patch; return stored; } - -function plainObjectSchema>( - label: string, - refine: (value: Record) => boolean, -): StandardSchemaV1 { - return { - "~standard": { - version: 1, - vendor: "notation", - validate(value) { - if (!isPlainObject(value) || !refine(value)) { - return { issues: [{ message: `${label} is invalid` }] }; - } - return { value: value as T }; - }, - }, - }; -} - -function isPlainObject(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} diff --git a/packages/reconciler/src/durable/yieldstar.ts b/packages/reconciler/src/durable/yieldstar.ts index 13ea2dd..4fb26db 100644 --- a/packages/reconciler/src/durable/yieldstar.ts +++ b/packages/reconciler/src/durable/yieldstar.ts @@ -2,7 +2,7 @@ import type { WorkflowFn } from "yieldstar"; export { RetryableError, defineStore } from "yieldstar"; export type { WorkflowStore } from "yieldstar"; -export type { StandardSchemaV1, StoreClient } from "@yieldstar/core"; +export type { StoreClient } from "@yieldstar/core"; /** The durable step primitive the runtime hands to workflow functions. */ export type DurableStep = Parameters>[0]; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 82b4901..fd80fe6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -345,15 +345,15 @@ importers: '@notation/state': specifier: workspace:* version: link:../state - '@notation/utils': - specifier: workspace:* - version: link:../utils '@yieldstar/core': specifier: 0.5.0 version: 0.5.0 deep-object-diff: specifier: ^1.1.9 version: 1.1.9 + valibot: + specifier: ^1.4.2 + version: 1.4.2(typescript@6.0.3) yieldstar: specifier: 0.5.0 version: 0.5.0 @@ -2615,6 +2615,14 @@ packages: resolution: {integrity: sha512-vIYxrBCC/N/K+Js3qSN88go7kIfNPssr/hHCesKCQNAjmgvYS2oqr69kIufEG+O4+PfezOH4EbIeHCfFov8ZgQ==} hasBin: true + valibot@1.4.2: + resolution: {integrity: sha512-gjdCvJ6d3RyHAneqxMYMW9QMCwYMb3jpOO0IyHZV1bnRHFBHrX3VkIILt5XYR0WhwHiH7Mty8ovuPZ/O3gamrg==} + peerDependencies: + typescript: '>=5' + peerDependenciesMeta: + typescript: + optional: true + vite@8.1.3: resolution: {integrity: sha512-Ds+gBRbj0lwRO2Y5hwnUBdxSwlAve9LeRyU4sNnAr0ewW0gWF0n5bgXgUzbgZ49MV9BVUAQUFYVcDUcilUExMA==} engines: {node: ^20.19.0 || >=22.12.0} @@ -4835,6 +4843,10 @@ snapshots: uuid@11.1.1: {} + valibot@1.4.2(typescript@6.0.3): + optionalDependencies: + typescript: 6.0.3 + vite@8.1.3(@types/node@22.13.4)(esbuild@0.28.1)(jiti@2.7.0): dependencies: lightningcss: 1.32.0 From fcb06914c546f3d23febf8aac0e83b7d51ae37ab Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:47:28 +0100 Subject: [PATCH 05/17] Adapt the durable reconciler to undefined reads 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. --- packages/cli/src/plan.ts | 1 + packages/reconciler/src/durable/operations.ts | 127 +++++++----------- packages/reconciler/src/planner.ts | 35 +++-- .../test/durable-reconciliation.test.ts | 29 ++-- packages/reconciler/test/planner.test.ts | 80 +++++++++-- 5 files changed, 145 insertions(+), 127 deletions(-) diff --git a/packages/cli/src/plan.ts b/packages/cli/src/plan.ts index ef52ec8..787eaf0 100644 --- a/packages/cli/src/plan.ts +++ b/packages/cli/src/plan.ts @@ -79,6 +79,7 @@ function printPlanSummary(result: Plan, logger: Logger) { `${count("update") + count("drift-update")} to update`, `${count("drift-recreate")} to recreate`, `${count("delete-orphan")} to delete`, + `${count("indeterminate")} could not be checked`, `${count("noop")} unchanged`, ].join(", "); diff --git a/packages/reconciler/src/durable/operations.ts b/packages/reconciler/src/durable/operations.ts index c9bf94c..ec7027c 100644 --- a/packages/reconciler/src/durable/operations.ts +++ b/packages/reconciler/src/durable/operations.ts @@ -1,5 +1,5 @@ import { - findErrorMatcher, + ResourceNotReadyError, type BaseResource, type ResourceType, } from "@notation/resource"; @@ -9,7 +9,7 @@ import { createResourceRegistryFromResources, resolveResourceClass, } from "../resource-registry"; -import { decideAction, type ResourceAction } from "../plan"; +import { decideAction, type DriftRead, type ResourceAction } from "../plan"; import { emitEvent, emitLifecycle } from "./emit"; import type { DurableStateBackend } from "./state-backend"; import { @@ -129,11 +129,10 @@ export async function* reconcileResource( // Checkpoint successful provider calls. Provider mutations must be // idempotent because a crash before the checkpoint can repeat them. if (operation === "create") { - const primaryKey = yield* runProviderCall( + const primaryKey = yield* runResourceMutation( step, `${prefix}:create`, () => resource.create(params), - resource, opts.retryOptions, ); resource.setOutput(params); @@ -151,7 +150,7 @@ export async function* reconcileResource( ); return; } - yield* runProviderCall( + yield* runResourceMutation( step, `${prefix}:update`, () => @@ -161,7 +160,6 @@ export async function* reconcileResource( params, resource.toState(resource.output), ), - resource, opts.retryOptions, ); resource.setOutput({ ...resource.key, ...params }); @@ -174,9 +172,9 @@ export async function* reconcileResource( resource, opts, `${prefix}:read-after-write`, - { retryNotFound: true }, + { retryAbsent: true }, ); - if (read.status === "found") + if (read.kind === "present") resource.setOutput({ ...resource.output, ...read.output }); const nextState: StoredResourceState = { @@ -269,28 +267,12 @@ export async function* deleteResource( } try { - // An already-deleted remote is success, not failure: the goal state is - // absence, so a declared not-found error downgrades to a skip. - try { - yield* runProviderCall( - step, - `${prefix}:delete`, - () => resource.delete(resource.key, resource.toState(resource.output)), - resource, - opts.retryOptions, - ); - } catch (error) { - if (!findErrorMatcher(error, resource.notFoundOnError)) throw error; - yield* emitLifecycle( - step, - `${prefix}:delete:not-found`, - opts.emit, - "delete", - "skip", - resource, - { reason: "resource-not-found" }, - ); - } + yield* runResourceMutation( + step, + `${prefix}:delete`, + () => resource.delete(resource.key, resource.toState(resource.output)), + opts.retryOptions, + ); // State is removed only after the provider delete completes, and only if // the store still matches the snapshot read before deleting. @@ -327,13 +309,8 @@ export async function* readRemote( resource: BaseResource, opts: DurableOperationOptions, key: string, - behaviour: { retryNotFound?: boolean } = {}, -): AsyncGenerator< - any, - | { status: "found"; output: Record } - | { status: "not-found" }, - any -> { + behaviour: { retryAbsent?: boolean } = {}, +): AsyncGenerator { if (!resource.read) { yield* emitLifecycle( step, @@ -347,7 +324,7 @@ export async function* readRemote( }, ); return { - status: "found", + kind: "present", output: { ...(await resource.getParams()), ...resource.output }, }; } @@ -361,36 +338,40 @@ export async function* readRemote( resource, ); try { - const output = yield* step.run(key, async () => { - let value: Record; + const result = yield* step.run(key, async () => { try { - value = await resource.read!(resource.key); + const output = await resource.read!(resource.key); + if (output === undefined && behaviour.retryAbsent) { + throw new RetryableError("Waiting for resource to become visible", { + ...(opts.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), + }); + } + // Absence is null rather than undefined so that it survives the + // step's JSON round-trip when the run is replayed. + return output ?? null; } catch (error) { - const notFound = findErrorMatcher(error, resource.notFoundOnError); - if (behaviour.retryNotFound && notFound) { - throw new RetryableError(notFound.reason, { + // A tagged not-ready condition is the provider telling us to wait. + // Everything else is a genuine failure and must surface. + if (ResourceNotReadyError.is(error)) { + throw new RetryableError(error.message, { ...(opts.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), }); } throw error; } - // An unsettled read (a declared condition not yet met) re-polls - // durably instead of returning a half-provisioned remote. - const unsettled = (resource.retryReadOnCondition ?? []) - .filter(Boolean) - .find((condition) => { - const actual = value[condition!.key]; - return condition!.value === undefined - ? !actual - : actual !== condition!.value; - }); - if (unsettled) { - throw new RetryableError(unsettled.reason, { - ...(opts.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), - }); - } - return value; }); + if (result === null) { + yield* emitLifecycle( + step, + `${key}:absent`, + opts.emit, + "read", + "skip", + resource, + { reason: "resource-absent" }, + ); + return { kind: "absent" }; + } yield* emitLifecycle( step, `${key}:success`, @@ -399,20 +380,8 @@ export async function* readRemote( "success", resource, ); - return { status: "found", output }; + return { kind: "present", output: result }; } catch (error) { - if (findErrorMatcher(error, resource.notFoundOnError)) { - yield* emitLifecycle( - step, - `${key}:not-found`, - opts.emit, - "read", - "skip", - resource, - { reason: "resource-not-found" }, - ); - return { status: "not-found" }; - } yield* emitLifecycle( step, `${key}:error`, @@ -429,23 +398,21 @@ export async function* readRemote( } /** - * Runs a provider call in a durable step, converting errors the resource has - * declared retryable into durable retries. + * Runs a resource mutation in a durable step and adapts resource retry + * signals to the workflow runtime. */ -export function runProviderCall( +export function runResourceMutation( step: DurableStep, key: string, call: () => T | Promise, - resource: BaseResource, retryOptions?: PollOptions, ) { return step.run(key, async () => { try { return await call(); } catch (error) { - const matcher = findErrorMatcher(error, resource.retryLaterOnError); - if (matcher) { - throw new RetryableError(matcher.reason, { + if (ResourceNotReadyError.is(error)) { + throw new RetryableError(error.message, { ...(retryOptions ?? DEFAULT_RETRY_OPTIONS), }); } diff --git a/packages/reconciler/src/planner.ts b/packages/reconciler/src/planner.ts index 1826703..4b5a11e 100644 --- a/packages/reconciler/src/planner.ts +++ b/packages/reconciler/src/planner.ts @@ -1,4 +1,4 @@ -import { findErrorMatcher, type BaseResource } from "@notation/resource"; +import { ResourceNotReadyError, type BaseResource } from "@notation/resource"; import type { StateBackend } from "@notation/state"; import { buildResourceDepthLevels } from "./dependency-graph"; import { @@ -33,24 +33,31 @@ export async function createPlan({ let action = decideAction({ resource, stateNode, params }); if (action.decision === "noop" && driftDetection && resource.read) { + let output: Record | undefined; try { - const output = await resource.read(resource.key); - action = decideAction({ - resource, - stateNode, - params, - driftRead: { status: "found", output }, - }); + output = (await resource.read(resource.key)) as + Record | undefined; } catch (error) { - const notFound = findErrorMatcher(error, resource.notFoundOnError); - if (!notFound) throw error; - action = decideAction({ - resource, - stateNode, + // Planning cannot diff against a resource that has not settled, so + // it reports the condition rather than guessing at a decision. + if (!ResourceNotReadyError.is(error)) throw error; + nodes.push({ + id: resource.id, + type: resource.type, + decision: "indeterminate", + reason: error.message, params, - driftRead: { status: "not-found" }, + dependsOn: getDependencyIds(resource), }); + continue; } + + action = decideAction({ + resource, + stateNode, + params, + driftRead: output ? { kind: "present", output } : { kind: "absent" }, + }); } nodes.push({ diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index 78ea78c..f34e41b 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -8,7 +8,11 @@ import { SqliteStoreClient, createSqliteDb, } from "@yieldstar/sqlite-runtime/node"; -import { resource, type BaseResource } from "@notation/resource"; +import { + resource, + ResourceNotReadyError, + type BaseResource, +} from "@notation/resource"; import pino from "pino"; import { createWorkflowRouter, workflow } from "yieldstar"; import { describe, expect, it, vi } from "vitest"; @@ -30,15 +34,10 @@ describe("durable execution and replay", () => { create: async () => { attempts += 1; if (attempts === 1) { - const error = new Error("provider is pending"); - error.name = "ProviderPending"; - throw error; + throw new ResourceNotReadyError("provider is not ready"); } }, delete: async () => undefined, - retryLaterOnError: [ - { name: "ProviderPending", reason: "provider is pending" }, - ], }); const runtime = createRuntime( [new PendingResource({ id: "pending" })], @@ -116,14 +115,9 @@ describe("durable execution and replay", () => { delete: async () => { attempts += 1; if (attempts === 1) { - const error = new Error("delete is pending"); - error.name = "DeletePending"; - throw error; + throw new ResourceNotReadyError("delete is not ready"); } }, - retryLaterOnError: [ - { name: "DeletePending", reason: "delete is pending" }, - ], }); const runtime = createRuntime( [new PendingDelete({ id: "pending-delete" })], @@ -153,16 +147,11 @@ describe("durable execution and replay", () => { read: async () => { reads += 1; if (reads === 1) { - const error = new Error("not visible yet"); - error.name = "NotFound"; - throw error; + return undefined; } - return {}; + return {} as const; }, delete: async () => undefined, - notFoundOnError: [ - { name: "NotFound", reason: "resource is not visible yet" }, - ], }); const runtime = createRuntime( [new EventuallyReadable({ id: "eventually-readable" })], diff --git a/packages/reconciler/test/planner.test.ts b/packages/reconciler/test/planner.test.ts index f0011e5..35a854e 100644 --- a/packages/reconciler/test/planner.test.ts +++ b/packages/reconciler/test/planner.test.ts @@ -1,4 +1,4 @@ -import { resource } from "@notation/resource"; +import { ResourceNotReadyError, resource } from "@notation/resource"; import { MemoryStateBackend } from "@notation/state"; import { describe, expect, it } from "vitest"; import { createPlan } from "../src/planner"; @@ -34,24 +34,15 @@ describe("createPlan", () => { ]); }); - it("honours the message constraint in not-found matchers", async () => { - const TestResource = resource({ type: "test/planner/not-found" }) + it("propagates unexpected read failures", async () => { + const TestResource = resource({ type: "test/planner/read-failure" }) .defineSchema({}) .defineOperations({ create: async () => undefined, read: async () => { - const error = new Error("access denied"); - error.name = "ProviderError"; - throw error; + throw new Error("access denied"); }, delete: async () => undefined, - notFoundOnError: [ - { - name: "ProviderError", - message: "not found", - reason: "resource does not exist", - }, - ], }); const state = new MemoryStateBackend(); await state.update("existing", 0, { @@ -71,4 +62,67 @@ describe("createPlan", () => { }), ).rejects.toThrow("access denied"); }); + + it("plans recreation when the resource reports absence", async () => { + const TestResource = resource({ type: "test/planner/absent" }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + read: async () => undefined, + delete: async () => undefined, + }); + const state = new MemoryStateBackend(); + await state.update("existing", 0, { + id: "existing", + type: TestResource.type, + config: {}, + params: {}, + output: {}, + lastOperation: "create", + lastOperationAt: "2026-07-22T00:00:00.000Z", + }); + + const plan = await createPlan({ + resources: [new TestResource({ id: "existing" })], + state, + }); + + expect(plan.nodes[0]).toMatchObject({ + id: "existing", + decision: "drift-recreate", + }); + }); + + it("reports an indeterminate decision while the resource is not ready", async () => { + const TestResource = resource({ type: "test/planner/pending" }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + read: async () => { + throw new ResourceNotReadyError("Waiting for the provider"); + }, + delete: async () => undefined, + }); + const state = new MemoryStateBackend(); + await state.update("existing", 0, { + id: "existing", + type: TestResource.type, + config: {}, + params: {}, + output: {}, + lastOperation: "create", + lastOperationAt: "2026-07-22T00:00:00.000Z", + }); + + const plan = await createPlan({ + resources: [new TestResource({ id: "existing" })], + state, + }); + + expect(plan.nodes[0]).toMatchObject({ + id: "existing", + decision: "indeterminate", + reason: "Waiting for the provider", + }); + }); }); From 40d140c43106d0fc0d1ed9602415f856e3881df9 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:40:10 +0100 Subject: [PATCH 06/17] Use resource operation signals in durable workflows --- packages/cli/src/plan.ts | 1 - packages/reconciler/src/durable/index.ts | 3 - packages/reconciler/src/durable/operations.ts | 136 ++++++------------ packages/reconciler/src/durable/types.ts | 18 +-- packages/reconciler/src/durable/yieldstar.ts | 2 +- packages/reconciler/src/planner.ts | 31 ++-- .../test/durable-reconciliation.test.ts | 70 ++++++--- packages/reconciler/test/planner.test.ts | 26 +++- 8 files changed, 135 insertions(+), 152 deletions(-) diff --git a/packages/cli/src/plan.ts b/packages/cli/src/plan.ts index 787eaf0..ef52ec8 100644 --- a/packages/cli/src/plan.ts +++ b/packages/cli/src/plan.ts @@ -79,7 +79,6 @@ function printPlanSummary(result: Plan, logger: Logger) { `${count("update") + count("drift-update")} to update`, `${count("drift-recreate")} to recreate`, `${count("delete-orphan")} to delete`, - `${count("indeterminate")} could not be checked`, `${count("noop")} unchanged`, ].join(", "); diff --git a/packages/reconciler/src/durable/index.ts b/packages/reconciler/src/durable/index.ts index e42707f..c896eb8 100644 --- a/packages/reconciler/src/durable/index.ts +++ b/packages/reconciler/src/durable/index.ts @@ -8,11 +8,8 @@ export { type StoredResourceState, } from "./stores"; export { - DEFAULT_READ_POLL_OPTIONS, - DEFAULT_RETRY_OPTIONS, type DurableDeployOptions, type DurableDestroyOptions, type DurableOperationOptions, - type PollOptions, } from "./types"; export type { DurableStep } from "./yieldstar"; diff --git a/packages/reconciler/src/durable/operations.ts b/packages/reconciler/src/durable/operations.ts index ec7027c..d945403 100644 --- a/packages/reconciler/src/durable/operations.ts +++ b/packages/reconciler/src/durable/operations.ts @@ -1,5 +1,5 @@ import { - ResourceNotReadyError, + ResourceNotFoundError, type BaseResource, type ResourceType, } from "@notation/resource"; @@ -9,6 +9,7 @@ import { createResourceRegistryFromResources, resolveResourceClass, } from "../resource-registry"; +import { runPendingOperation } from "../operations/operation.pending"; import { decideAction, type DriftRead, type ResourceAction } from "../plan"; import { emitEvent, emitLifecycle } from "./emit"; import type { DurableStateBackend } from "./state-backend"; @@ -17,18 +18,8 @@ import { toStateNode, type StoredResourceState, } from "./stores"; -import { - DEFAULT_READ_POLL_OPTIONS, - DEFAULT_RETRY_OPTIONS, - type DurableDeployOptions, - type DurableOperationOptions, - type PollOptions, -} from "./types"; -import { - RetryableError, - type DurableStep, - type WorkflowStore, -} from "./yieldstar"; +import type { DurableDeployOptions, DurableOperationOptions } from "./types"; +import type { DurableStep, WorkflowStore } from "./yieldstar"; export async function* reconcileResource( step: DurableStep, @@ -129,11 +120,11 @@ export async function* reconcileResource( // Checkpoint successful provider calls. Provider mutations must be // idempotent because a crash before the checkpoint can repeat them. if (operation === "create") { - const primaryKey = yield* runResourceMutation( + const primaryKey = yield* runPendingOperation( step, `${prefix}:create`, - () => resource.create(params), - opts.retryOptions, + (context) => resource.create(params, context), + opts.maxOperationAttempts, ); resource.setOutput(params); if (primaryKey) resource.setOutput({ ...primaryKey, ...resource.output }); @@ -150,32 +141,31 @@ export async function* reconcileResource( ); return; } - yield* runResourceMutation( + yield* runPendingOperation( step, `${prefix}:update`, - () => + (context) => resource.update!( resource.key, patch, params, resource.toState(resource.output), + context, ), - opts.retryOptions, + opts.maxOperationAttempts, ); resource.setOutput({ ...resource.key, ...params }); } // Read back the remote so persisted output reflects provider-assigned // values, then persist conditionally against the snapshot read above. - const read = yield* readRemote( + const read = yield* readResource( step, resource, opts, `${prefix}:read-after-write`, - { retryAbsent: true }, ); - if (read.kind === "present") - resource.setOutput({ ...resource.output, ...read.output }); + resource.setOutput({ ...resource.output, ...read }); const nextState: StoredResourceState = { id: resource.id, @@ -267,11 +257,16 @@ export async function* deleteResource( } try { - yield* runResourceMutation( + yield* runPendingOperation( step, `${prefix}:delete`, - () => resource.delete(resource.key, resource.toState(resource.output)), - opts.retryOptions, + (context) => + resource.delete( + resource.key, + resource.toState(resource.output), + context, + ), + opts.maxOperationAttempts, ); // State is removed only after the provider delete completes, and only if @@ -309,8 +304,24 @@ export async function* readRemote( resource: BaseResource, opts: DurableOperationOptions, key: string, - behaviour: { retryAbsent?: boolean } = {}, ): AsyncGenerator { + try { + return { + kind: "present", + output: yield* readResource(step, resource, opts, key), + }; + } catch (error) { + if (ResourceNotFoundError.is(error)) return { kind: "absent" }; + throw error; + } +} + +async function* readResource( + step: DurableStep, + resource: BaseResource, + opts: DurableOperationOptions, + key: string, +): AsyncGenerator, any> { if (!resource.read) { yield* emitLifecycle( step, @@ -323,10 +334,7 @@ export async function* readRemote( reason: "read-not-implemented", }, ); - return { - kind: "present", - output: { ...(await resource.getParams()), ...resource.output }, - }; + return { ...(await resource.getParams()), ...resource.output }; } yield* emitLifecycle( @@ -338,40 +346,12 @@ export async function* readRemote( resource, ); try { - const result = yield* step.run(key, async () => { - try { - const output = await resource.read!(resource.key); - if (output === undefined && behaviour.retryAbsent) { - throw new RetryableError("Waiting for resource to become visible", { - ...(opts.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), - }); - } - // Absence is null rather than undefined so that it survives the - // step's JSON round-trip when the run is replayed. - return output ?? null; - } catch (error) { - // A tagged not-ready condition is the provider telling us to wait. - // Everything else is a genuine failure and must surface. - if (ResourceNotReadyError.is(error)) { - throw new RetryableError(error.message, { - ...(opts.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), - }); - } - throw error; - } - }); - if (result === null) { - yield* emitLifecycle( - step, - `${key}:absent`, - opts.emit, - "read", - "skip", - resource, - { reason: "resource-absent" }, - ); - return { kind: "absent" }; - } + const result = yield* runPendingOperation( + step, + key, + (context) => resource.read!(resource.key, context), + opts.maxOperationAttempts, + ); yield* emitLifecycle( step, `${key}:success`, @@ -380,7 +360,7 @@ export async function* readRemote( "success", resource, ); - return { kind: "present", output: result }; + return result; } catch (error) { yield* emitLifecycle( step, @@ -397,30 +377,6 @@ export async function* readRemote( } } -/** - * Runs a resource mutation in a durable step and adapts resource retry - * signals to the workflow runtime. - */ -export function runResourceMutation( - step: DurableStep, - key: string, - call: () => T | Promise, - retryOptions?: PollOptions, -) { - return step.run(key, async () => { - try { - return await call(); - } catch (error) { - if (ResourceNotReadyError.is(error)) { - throw new RetryableError(error.message, { - ...(retryOptions ?? DEFAULT_RETRY_OPTIONS), - }); - } - throw error; - } - }); -} - /** * Deletes persisted resources that are no longer in the desired set. A state * node whose type has no registry entry is left in place and surfaced as a diff --git a/packages/reconciler/src/durable/types.ts b/packages/reconciler/src/durable/types.ts index c139348..76f7000 100644 --- a/packages/reconciler/src/durable/types.ts +++ b/packages/reconciler/src/durable/types.ts @@ -3,21 +3,6 @@ import type { ReconcilerEventEmitter } from "../events"; import type { ResourceRegistry } from "../resource-registry"; import type { DurableStateBackend } from "./state-backend"; -export type PollOptions = { - maxAttempts: number; - retryInterval: number; -}; - -export const DEFAULT_RETRY_OPTIONS: PollOptions = { - maxAttempts: 10, - retryInterval: 1_000, -}; - -export const DEFAULT_READ_POLL_OPTIONS: PollOptions = { - maxAttempts: 30, - retryInterval: 1_000, -}; - export type DurableOperationOptions = { deploymentId: string; executionId: string; @@ -26,8 +11,7 @@ export type DurableOperationOptions = { registry?: ResourceRegistry; dryRun?: boolean; emit?: ReconcilerEventEmitter; - retryOptions?: PollOptions; - readPollOptions?: PollOptions; + maxOperationAttempts?: number; }; export type DurableDeployOptions = DurableOperationOptions & { diff --git a/packages/reconciler/src/durable/yieldstar.ts b/packages/reconciler/src/durable/yieldstar.ts index 4fb26db..6bbba63 100644 --- a/packages/reconciler/src/durable/yieldstar.ts +++ b/packages/reconciler/src/durable/yieldstar.ts @@ -1,6 +1,6 @@ import type { WorkflowFn } from "yieldstar"; -export { RetryableError, defineStore } from "yieldstar"; +export { defineStore } from "yieldstar"; export type { WorkflowStore } from "yieldstar"; export type { StoreClient } from "@yieldstar/core"; diff --git a/packages/reconciler/src/planner.ts b/packages/reconciler/src/planner.ts index 4b5a11e..ebfa064 100644 --- a/packages/reconciler/src/planner.ts +++ b/packages/reconciler/src/planner.ts @@ -1,6 +1,7 @@ -import { ResourceNotReadyError, type BaseResource } from "@notation/resource"; +import { ResourceNotFoundError, type BaseResource } from "@notation/resource"; import type { StateBackend } from "@notation/state"; import { buildResourceDepthLevels } from "./dependency-graph"; +import { readResourceOperation } from "./operations"; import { decideAction, getDependencyIds, @@ -8,6 +9,7 @@ import { type Plan, type PlanNode, } from "./plan"; +import { createStepRunner, runOperation } from "./reconciler"; export type CreatePlanOptions = { resources: BaseResource[]; @@ -33,30 +35,25 @@ export async function createPlan({ let action = decideAction({ resource, stateNode, params }); if (action.decision === "noop" && driftDetection && resource.read) { - let output: Record | undefined; + let driftRead; try { - output = (await resource.read(resource.key)) as - Record | undefined; + const output = await runOperation( + readResourceOperation(createStepRunner(), { + resource, + state, + }), + ); + driftRead = { kind: "present" as const, output }; } catch (error) { - // Planning cannot diff against a resource that has not settled, so - // it reports the condition rather than guessing at a decision. - if (!ResourceNotReadyError.is(error)) throw error; - nodes.push({ - id: resource.id, - type: resource.type, - decision: "indeterminate", - reason: error.message, - params, - dependsOn: getDependencyIds(resource), - }); - continue; + if (!ResourceNotFoundError.is(error)) throw error; + driftRead = { kind: "absent" as const }; } action = decideAction({ resource, stateNode, params, - driftRead: output ? { kind: "present", output } : { kind: "absent" }, + driftRead, }); } diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index f34e41b..1a51cd7 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -10,9 +10,11 @@ import { } from "@yieldstar/sqlite-runtime/node"; import { resource, - ResourceNotReadyError, + ResourceNotFoundError, + ResourceOperationPendingError, type BaseResource, } from "@notation/resource"; +import { setTimeout as sleep } from "node:timers/promises"; import pino from "pino"; import { createWorkflowRouter, workflow } from "yieldstar"; import { describe, expect, it, vi } from "vitest"; @@ -31,24 +33,30 @@ describe("durable execution and replay", () => { const PendingResource = resource({ type: "test/durable/pending" }) .defineSchema({}) .defineOperations({ - create: async () => { + create: async (_params, context) => { attempts += 1; if (attempts === 1) { - throw new ResourceNotReadyError("provider is not ready"); + expect(context).toBeUndefined(); + throw new ResourceOperationPendingError("provider is not ready", { + retryAfterMs: 1, + callbackContext: { requestId: "request-123" }, + }); } + expect(context).toEqual({ requestId: "request-123" }); }, delete: async () => undefined, }); const runtime = createRuntime( [new PendingResource({ id: "pending" })], "durable-wait", - { retryOptions: { maxAttempts: 3, retryInterval: 1 } }, + { maxOperationAttempts: 3 }, ); await runtime.run("wait-execution"); expect(attempts).toBe(1); expect(runtime.scheduler.events).toHaveLength(1); + await sleep(5); await runtime.run("wait-execution"); expect(attempts).toBe(2); expect(await runtime.state.get("pending")).toMatchObject({ @@ -67,7 +75,7 @@ describe("durable execution and replay", () => { const runtime = createRuntime( [new TestResource({ id: "resume" })], "crash-resume", - { crashAfterStep: "notation:resource:resume:create" }, + { crashAfterStep: "notation:resource:resume:create:attempt:0" }, ); await expect(runtime.run("resume-execution")).rejects.toThrow( @@ -90,7 +98,7 @@ describe("durable execution and replay", () => { const runtime = createRuntime( [new TestResource({ id: "destroyed" })], "destroy-crash-resume", - { crashAfterStep: "notation:destroy:destroyed:delete" }, + { crashAfterStep: "notation:destroy:destroyed:delete:attempt:0" }, ); await runtime.run("deploy-before-destroy"); @@ -115,14 +123,16 @@ describe("durable execution and replay", () => { delete: async () => { attempts += 1; if (attempts === 1) { - throw new ResourceNotReadyError("delete is not ready"); + throw new ResourceOperationPendingError("delete is not ready", { + retryAfterMs: 1, + }); } }, }); const runtime = createRuntime( [new PendingDelete({ id: "pending-delete" })], "durable-destroy-wait", - { retryOptions: { maxAttempts: 3, retryInterval: 1 } }, + { maxOperationAttempts: 3 }, ); await runtime.run("deploy-before-wait"); @@ -130,13 +140,14 @@ describe("durable execution and replay", () => { expect(attempts).toBe(1); expect(await runtime.state.get("pending-delete")).toBeDefined(); + await sleep(5); await runtime.destroy("destroy-wait"); expect(attempts).toBe(2); expect(await runtime.state.get("pending-delete")).toBeUndefined(); runtime.close(); }); - it("retries a post-write not-found before persisting state", async () => { + it("waits when a resource reports that its post-write read is pending", async () => { let reads = 0; const EventuallyReadable = resource({ type: "test/durable/eventually-readable", @@ -147,7 +158,10 @@ describe("durable execution and replay", () => { read: async () => { reads += 1; if (reads === 1) { - return undefined; + throw new ResourceOperationPendingError( + "resource is not visible yet", + { retryAfterMs: 1 }, + ); } return {} as const; }, @@ -156,13 +170,14 @@ describe("durable execution and replay", () => { const runtime = createRuntime( [new EventuallyReadable({ id: "eventually-readable" })], "post-write-read", - { readPollOptions: { maxAttempts: 3, retryInterval: 1 } }, + { maxOperationAttempts: 3 }, ); await runtime.run("post-write-read-execution"); expect(reads).toBe(1); expect(await runtime.state.get("eventually-readable")).toBeUndefined(); + await sleep(5); await runtime.run("post-write-read-execution"); expect(reads).toBe(2); expect(await runtime.state.get("eventually-readable")).toMatchObject({ @@ -170,6 +185,30 @@ describe("durable execution and replay", () => { }); runtime.close(); }); + + it("does not infer that not-found after a write is pending", async () => { + const MissingAfterCreate = resource({ + type: "test/durable/missing-after-create", + }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + read: async () => { + throw new ResourceNotFoundError("resource is absent"); + }, + delete: async () => undefined, + }); + const runtime = createRuntime( + [new MissingAfterCreate({ id: "missing-after-create" })], + "missing-after-create", + ); + + await expect(runtime.run("missing-after-create-execution")).rejects.toThrow( + "resource is absent", + ); + expect(await runtime.state.get("missing-after-create")).toBeUndefined(); + runtime.close(); + }); }); describe("dependency ordering", () => { @@ -452,8 +491,7 @@ function createRuntime( resources: BaseResource[], deploymentId: string, options: { - retryOptions?: { maxAttempts: number; retryInterval: number }; - readPollOptions?: { maxAttempts: number; retryInterval: number }; + maxOperationAttempts?: number; crashAfterStep?: string; registry?: ResourceRegistry; driftDetection?: boolean; @@ -480,8 +518,7 @@ function createRuntime( registry: options.registry, driftDetection: options.driftDetection ?? false, emit: options.emit, - retryOptions: options.retryOptions, - readPollOptions: options.readPollOptions, + maxOperationAttempts: options.maxOperationAttempts, }); }); const destroy = workflow(async function* (step, event) { @@ -492,8 +529,7 @@ function createRuntime( state, registry: options.registry, emit: options.emit, - retryOptions: options.retryOptions, - readPollOptions: options.readPollOptions, + maxOperationAttempts: options.maxOperationAttempts, }); }); const router = createWorkflowRouter({ deploy, destroy }); diff --git a/packages/reconciler/test/planner.test.ts b/packages/reconciler/test/planner.test.ts index 35a854e..8066578 100644 --- a/packages/reconciler/test/planner.test.ts +++ b/packages/reconciler/test/planner.test.ts @@ -1,4 +1,8 @@ -import { ResourceNotReadyError, resource } from "@notation/resource"; +import { + ResourceNotFoundError, + ResourceOperationPendingError, + resource, +} from "@notation/resource"; import { MemoryStateBackend } from "@notation/state"; import { describe, expect, it } from "vitest"; import { createPlan } from "../src/planner"; @@ -68,7 +72,9 @@ describe("createPlan", () => { .defineSchema({}) .defineOperations({ create: async () => undefined, - read: async () => undefined, + read: async () => { + throw new ResourceNotFoundError("resource is absent"); + }, delete: async () => undefined, }); const state = new MemoryStateBackend(); @@ -93,13 +99,21 @@ describe("createPlan", () => { }); }); - it("reports an indeterminate decision while the resource is not ready", async () => { + it("waits for a pending read before planning", async () => { + let attempts = 0; const TestResource = resource({ type: "test/planner/pending" }) .defineSchema({}) .defineOperations({ create: async () => undefined, read: async () => { - throw new ResourceNotReadyError("Waiting for the provider"); + attempts += 1; + if (attempts === 1) { + throw new ResourceOperationPendingError( + "Waiting for the provider", + { retryAfterMs: 0 }, + ); + } + return {}; }, delete: async () => undefined, }); @@ -121,8 +135,8 @@ describe("createPlan", () => { expect(plan.nodes[0]).toMatchObject({ id: "existing", - decision: "indeterminate", - reason: "Waiting for the provider", + decision: "noop", }); + expect(attempts).toBe(2); }); }); From c871428a6417d4257ec5776b9169d6126227c2c5 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 19:08:51 +0100 Subject: [PATCH 07/17] Share one operation library across reconciler drivers The 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. --- packages/reconciler/package.json | 10 + .../reconciler/src/durable/coordination.ts | 24 +- packages/reconciler/src/durable/deploy.ts | 29 +- packages/reconciler/src/durable/destroy.ts | 45 +- packages/reconciler/src/durable/emit.ts | 64 --- packages/reconciler/src/durable/operations.ts | 493 ++++++------------ .../reconciler/src/durable/state-backend.ts | 148 ++---- packages/reconciler/src/durable/step.ts | 84 +++ packages/reconciler/src/durable/stores.ts | 23 +- packages/reconciler/src/durable/yieldstar.ts | 2 +- packages/reconciler/src/events.ts | 22 +- packages/reconciler/src/index.ts | 2 + .../src/operations/operation.create.ts | 30 +- .../src/operations/operation.delete.ts | 12 +- .../src/operations/operation.read.ts | 32 +- .../src/operations/operation.types.ts | 77 ++- .../src/operations/operation.update.ts | 34 +- packages/reconciler/src/planner.ts | 40 +- packages/reconciler/src/protocol.ts | 2 +- packages/reconciler/src/reconciler.ts | 203 ++------ packages/reconciler/src/step-runner.ts | 40 ++ .../test/durable-reconciliation.test.ts | 153 +++--- .../test/operation.workflows.test.ts | 49 +- packages/reconciler/tsup.config.ts | 2 +- 24 files changed, 696 insertions(+), 924 deletions(-) delete mode 100644 packages/reconciler/src/durable/emit.ts create mode 100644 packages/reconciler/src/durable/step.ts create mode 100644 packages/reconciler/src/step-runner.ts diff --git a/packages/reconciler/package.json b/packages/reconciler/package.json index d1ec1db..58b0ab0 100644 --- a/packages/reconciler/package.json +++ b/packages/reconciler/package.json @@ -2,6 +2,16 @@ "type": "module", "name": "@notation/reconciler", "version": "0.12.0", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "default": "./dist/index.js" + }, + "./durable": { + "types": "./dist/durable/index.d.ts", + "default": "./dist/durable/index.js" + } + }, "main": "./dist/index.js", "types": "./dist/index.d.ts", "files": [ diff --git a/packages/reconciler/src/durable/coordination.ts b/packages/reconciler/src/durable/coordination.ts index 2fb2579..5c7af65 100644 --- a/packages/reconciler/src/durable/coordination.ts +++ b/packages/reconciler/src/durable/coordination.ts @@ -1,5 +1,5 @@ import type { ReconcilerEventEmitter } from "../events"; -import { emitEvent } from "./emit"; +import { durableEmitter, scopeStep } from "./step"; import { deploymentCoordinationStore, type CoordinationState } from "./stores"; import type { DurableStep, WorkflowStore } from "./yieldstar"; @@ -13,7 +13,7 @@ type CoordinationOptions = { * Prevents concurrent executions from mutating the same deployment. Names * the holder so an operator can resume it after a crash. */ -export async function* acquireDeploymentCoordination( +async function* acquireDeploymentCoordination( step: DurableStep, opts: CoordinationOptions, ): AsyncGenerator, any> { @@ -25,13 +25,13 @@ export async function* acquireDeploymentCoordination( const snapshot = yield* coordination.get("notation:coordination:inspect"); const holder = snapshot.state.holder; if (holder !== null && holder !== opts.executionId) { - yield* emitEvent(step, "notation:coordination:waiting", opts.emit, () => ({ + yield* durableEmitter(scopeStep(step, "notation:coordination"), opts.emit)({ level: "warn", event: "reconciler.coordination.waiting", deploymentId: opts.deploymentId, executionId: opts.executionId, holderExecutionId: holder, - })); + }); } yield* coordination.take( @@ -45,7 +45,7 @@ export async function* acquireDeploymentCoordination( return coordination; } -export function releaseDeploymentCoordination( +function releaseDeploymentCoordination( coordination: WorkflowStore, executionId: string, ) { @@ -53,3 +53,17 @@ export function releaseDeploymentCoordination( if (draft.holder === executionId) draft.holder = null; }); } + +/** Runs `body` while holding the deployment, releasing it even on error. */ +export async function* withDeploymentHold( + step: DurableStep, + opts: CoordinationOptions, + body: () => AsyncGenerator, +): AsyncGenerator { + const coordination = yield* acquireDeploymentCoordination(step, opts); + try { + yield* body(); + } finally { + yield* releaseDeploymentCoordination(coordination, opts.executionId); + } +} diff --git a/packages/reconciler/src/durable/deploy.ts b/packages/reconciler/src/durable/deploy.ts index 3b93520..b28c273 100644 --- a/packages/reconciler/src/durable/deploy.ts +++ b/packages/reconciler/src/durable/deploy.ts @@ -1,9 +1,7 @@ import { buildResourceDepthLevels } from "../dependency-graph"; -import { - acquireDeploymentCoordination, - releaseDeploymentCoordination, -} from "./coordination"; +import { withDeploymentHold } from "./coordination"; import { reconcileResource, sweepOrphans } from "./operations"; +import { scopeStep } from "./step"; import type { DurableDeployOptions } from "./types"; import type { DurableStep } from "./yieldstar"; @@ -11,27 +9,16 @@ export async function* deploy( step: DurableStep, opts: DurableDeployOptions, ): AsyncGenerator { - // Phase 1: take exclusive hold of the deployment. - const coordination = yield* acquireDeploymentCoordination(step, opts); - - try { - // Phase 2: reconcile in dependency order, so a resource only runs once - // its dependencies have converged. + yield* withDeploymentHold(step, opts, async function* () { + // Reconcile in dependency order, so a resource only runs once its + // dependencies have converged. for (const level of buildResourceDepthLevels(opts.resources)) { for (const resource of level) { yield* reconcileResource(step, resource, opts); } } - // Phase 3: delete resources that are in state but no longer declared. - yield* sweepOrphans(step, opts, { - workflow: "deploy", - listKey: "notation:orphans:list", - warningKey: (nodeId) => `notation:orphan:${nodeId}:warning`, - deleteSuffix: "orphan", - }); - } finally { - // Phase 4: release the hold, even on error. - yield* releaseDeploymentCoordination(coordination, opts.executionId); - } + // Then delete resources that are in state but no longer declared. + yield* sweepOrphans(scopeStep(step, "notation:orphans"), opts, "deploy"); + }); } diff --git a/packages/reconciler/src/durable/destroy.ts b/packages/reconciler/src/durable/destroy.ts index 8f01316..fa6158a 100644 --- a/packages/reconciler/src/durable/destroy.ts +++ b/packages/reconciler/src/durable/destroy.ts @@ -1,9 +1,7 @@ import { buildResourceDepthLevels } from "../dependency-graph"; -import { - acquireDeploymentCoordination, - releaseDeploymentCoordination, -} from "./coordination"; +import { withDeploymentHold } from "./coordination"; import { deleteResource, sweepOrphans } from "./operations"; +import { scopeStep } from "./step"; import type { DurableDestroyOptions } from "./types"; import type { DurableStep } from "./yieldstar"; @@ -12,35 +10,26 @@ export async function* destroy( step: DurableStep, opts: DurableDestroyOptions, ): AsyncGenerator { - // Phase 1: take exclusive hold of the deployment. - const coordination = yield* acquireDeploymentCoordination(step, opts); - - try { - // Phase 2: delete in reverse dependency order, so dependents are gone - // before the resources they depend on. Resources with no persisted state - // were never created (or are already deleted) and are skipped. + yield* withDeploymentHold(step, opts, async function* () { + // Delete in reverse dependency order, so dependents are gone before the + // resources they depend on. Resources with no persisted state were never + // created (or are already deleted) and are skipped by deleteResource. const levels = buildResourceDepthLevels(opts.resources); for (let index = levels.length - 1; index >= 0; index -= 1) { for (const resource of levels[index]!) { - const stateNode = yield* step.run( - `notation:destroy:${resource.id}:state:lookup`, - () => opts.state.get(resource.id), + yield* deleteResource( + scopeStep(step, `notation:destroy:${resource.id}`), + resource, + opts, ); - if (!stateNode) continue; - resource.setOutput(stateNode.output); - yield* deleteResource(step, resource, opts, "destroy"); } } - // Phase 3: delete resources that are in state but no longer declared. - yield* sweepOrphans(step, opts, { - workflow: "destroy", - listKey: "notation:destroy:orphans:list", - warningKey: (nodeId) => `notation:destroy:orphan:${nodeId}:warning`, - deleteSuffix: "destroy-orphan", - }); - } finally { - // Phase 4: release the hold, even on error. - yield* releaseDeploymentCoordination(coordination, opts.executionId); - } + // Then delete resources that are in state but no longer declared. + yield* sweepOrphans( + scopeStep(step, "notation:destroy:orphans"), + opts, + "destroy", + ); + }); } diff --git a/packages/reconciler/src/durable/emit.ts b/packages/reconciler/src/durable/emit.ts deleted file mode 100644 index 6cc14a9..0000000 --- a/packages/reconciler/src/durable/emit.ts +++ /dev/null @@ -1,64 +0,0 @@ -import type { - OperationLifecycleEvent, - OperationLifecycleStatus, - OperationName, - ReconcilerEventEmitter, -} from "../events"; -import type { DurableStep } from "./yieldstar"; - -/** Checkpoints delivery after the emitter returns. Emitters must tolerate a duplicate if the process crashes before that checkpoint. */ -export function emitEvent( - step: DurableStep, - key: string, - emit: ReconcilerEventEmitter | undefined, - event: () => Parameters[0], -) { - return step.run(key, async () => { - await emit?.(event()); - }); -} - -export function emitLifecycle( - step: DurableStep, - key: string, - emit: ReconcilerEventEmitter | undefined, - operation: OperationName, - status: OperationLifecycleStatus, - resource: LifecycleResource, - extra: { reason?: string; error?: unknown } = {}, -) { - return emitEvent(step, key, emit, () => - createLifecycleEvent(operation, status, resource, extra), - ); -} - -type LifecycleResource = { - id: string; - type: OperationLifecycleEvent["resourceType"]; -}; - -export function createLifecycleEvent( - operation: OperationName, - status: OperationLifecycleStatus, - resource: LifecycleResource, - extra: { reason?: string; error?: unknown } = {}, -): OperationLifecycleEvent { - const error = extra.error; - const details = - error === undefined - ? {} - : error instanceof Error - ? { errorName: error.name, errorMessage: error.message } - : { errorName: "UnknownError", errorMessage: String(error) }; - - return { - level: status === "error" ? "error" : "info", - event: "reconciler.operation.lifecycle", - operation, - status, - resourceId: resource.id, - resourceType: resource.type, - ...(extra.reason ? { reason: extra.reason } : {}), - ...details, - }; -} diff --git a/packages/reconciler/src/durable/operations.ts b/packages/reconciler/src/durable/operations.ts index d945403..e439df1 100644 --- a/packages/reconciler/src/durable/operations.ts +++ b/packages/reconciler/src/durable/operations.ts @@ -1,380 +1,107 @@ -import { - ResourceNotFoundError, - type BaseResource, - type ResourceType, -} from "@notation/resource"; -import { RevConflict } from "@notation/state"; +import type { BaseResource, ResourceType } from "@notation/resource"; +import { RevConflict, type StateNode } from "@notation/state"; import { createMissingResourceRegistryMatchWarningEvent, createResourceRegistryFromResources, resolveResourceClass, } from "../resource-registry"; -import { runPendingOperation } from "../operations/operation.pending"; -import { decideAction, type DriftRead, type ResourceAction } from "../plan"; -import { emitEvent, emitLifecycle } from "./emit"; +import { + createResourceOperation, + deleteResourceOperation, + readDriftOperation, + updateResourceOperation, + type PersistState, + type RemoveState, + type ResourceOperationBaseParams, +} from "../operations"; +import { decideAction, type ResourceAction } from "../plan"; import type { DurableStateBackend } from "./state-backend"; +import { durableEmitter, scopeStep } from "./step"; import { resourceStateStore, toStateNode, - type StoredResourceState, + type ResourceSnapshot, } from "./stores"; import type { DurableDeployOptions, DurableOperationOptions } from "./types"; -import type { DurableStep, WorkflowStore } from "./yieldstar"; +import type { DurableStep } from "./yieldstar"; export async function* reconcileResource( step: DurableStep, resource: BaseResource, opts: DurableDeployOptions, ): AsyncGenerator { - const prefix = `notation:resource:${resource.id}`; + const scope = scopeStep(step, `notation:resource:${resource.id}`); + const emit = durableEmitter(scope, opts.emit); - // Hydrate the resource from persisted state. The snapshot is kept so later - // writes can be conditional on the exact instance identity and version that - // was read here. - let stateNode = yield* step.run(`${prefix}:state:lookup`, () => - opts.state.get(resource.id), - ); - let stateStore: WorkflowStore | undefined; - let snapshot: - Awaited> | undefined; - if (stateNode) { - stateStore = yield* openResourceState(step, opts.state, resource.id); - snapshot = yield* stateStore.get(`${prefix}:state:get`); - stateNode = toStateNode(snapshot); - } - if (stateNode) resource.setOutput(stateNode.output); + const { stateNode, snapshot } = yield* hydrateResource(scope, resource, opts); + const shared = operationParams(scope, resource, opts, stateNode); // Decide the operation from desired params vs persisted state. - const params = yield* step.run(`${prefix}:params`, () => - resource.getParams(), - ); - let action: ResourceAction = decideAction({ - resource, - stateNode: stateNode ?? undefined, - params, - }); + const params = yield* scope.run("params", () => resource.getParams()); + let action: ResourceAction = decideAction({ resource, stateNode, params }); // A noop is only trusted after the remote is read back: the provider may // have drifted from persisted state, which upgrades the decision. if (action.decision === "noop" && (opts.driftDetection ?? true)) { - const remote = yield* readRemote( - step, - resource, - opts, - `${prefix}:drift-read`, + // Its own scope: the operation that follows reads the remote again, and + // the two reads must not share step keys. + const driftScope = scopeStep(scope, "drift-read"); + const driftRead = yield* readDriftOperation( + driftScope, + operationParams(driftScope, resource, opts, stateNode), ); - action = decideAction({ - resource, - stateNode: stateNode ?? undefined, - params, - driftRead: remote, - }); + action = decideAction({ resource, stateNode, params, driftRead }); } if (action.decision === "drift-update") { - const diff = action.patch; - yield* emitEvent(step, `${prefix}:drift-detected`, opts.emit, () => ({ + yield* emit({ level: "info", event: "reconciler.drift.detected", resourceId: resource.id, resourceType: resource.type, - diff, - })); + diff: action.patch, + }); } - yield* emitEvent(step, `${prefix}:decision`, opts.emit, () => ({ + yield* emit({ level: "info", event: "reconciler.deploy.decision", resourceId: resource.id, resourceType: resource.type, decision: action.decision, - })); + }); if (action.decision === "noop") return; - const operation = - action.decision === "create" || action.decision === "drift-recreate" - ? "create" - : "update"; - const patch = "patch" in action ? action.patch : {}; - yield* emitLifecycle( - step, - `${prefix}:${operation}:start`, - opts.emit, - operation, - "start", - resource, - ); - if (opts.dryRun) { - yield* emitLifecycle( - step, - `${prefix}:${operation}:dry-run`, - opts.emit, - operation, - "dry-run", - resource, - ); + + const persist = persistResourceState(scope, opts, resource, snapshot); + if (action.decision === "create" || action.decision === "drift-recreate") { + yield* createResourceOperation(scope, { ...shared, persist }); return; } - try { - // Checkpoint successful provider calls. Provider mutations must be - // idempotent because a crash before the checkpoint can repeat them. - if (operation === "create") { - const primaryKey = yield* runPendingOperation( - step, - `${prefix}:create`, - (context) => resource.create(params, context), - opts.maxOperationAttempts, - ); - resource.setOutput(params); - if (primaryKey) resource.setOutput({ ...primaryKey, ...resource.output }); - } else { - if (!resource.update) { - yield* emitLifecycle( - step, - `${prefix}:update:skip`, - opts.emit, - "update", - "skip", - resource, - { reason: "update-not-implemented" }, - ); - return; - } - yield* runPendingOperation( - step, - `${prefix}:update`, - (context) => - resource.update!( - resource.key, - patch, - params, - resource.toState(resource.output), - context, - ), - opts.maxOperationAttempts, - ); - resource.setOutput({ ...resource.key, ...params }); - } - - // Read back the remote so persisted output reflects provider-assigned - // values, then persist conditionally against the snapshot read above. - const read = yield* readResource( - step, - resource, - opts, - `${prefix}:read-after-write`, - ); - resource.setOutput({ ...resource.output, ...read }); - - const nextState: StoredResourceState = { - id: resource.id, - groupId: resource.groupId, - groupType: resource.groupType, - type: resource.type, - lastOperation: operation, - lastOperationAt: new Date().toISOString(), - config: resource.config, - params: resource.toState(params), - output: resource.toState(resource.output), - }; - - if (!stateStore || !snapshot) { - yield* step.store(resourceStateStore, { - id: opts.state.storeId(resource.id), - initial: nextState, - }); - } else { - const result = yield* stateStore.updateFrom( - `${prefix}:state:persist`, - snapshot, - () => nextState, - ); - if (!result.updated) - throw new RevConflict( - resource.id, - stateNode?.rev ?? 0, - result.actualVersion + 1, - ); - } - - yield* emitLifecycle( - step, - `${prefix}:${operation}:success`, - opts.emit, - operation, - "success", - resource, - ); - } catch (error) { - yield* emitLifecycle( - step, - `${prefix}:${operation}:error`, - opts.emit, - operation, - "error", - resource, - { error }, - ); - throw error; - } + yield* updateResourceOperation(scope, { + ...shared, + patch: action.patch, + persist, + }); } export async function* deleteResource( step: DurableStep, resource: BaseResource, opts: DurableOperationOptions, - suffix: string, ): AsyncGenerator { - const prefix = `notation:${suffix}:${resource.id}`; - // Hydrate output from persisted state; the delete call needs the primary // key and the state removal must be conditional on this exact snapshot. - const stateStore = yield* openResourceState(step, opts.state, resource.id); - const snapshot = yield* stateStore.get(`${prefix}:state:get`); + const snapshot = yield* readSnapshot(step, opts.state, resource.id); + if (!snapshot) return; const stateNode = toStateNode(snapshot); resource.setOutput(stateNode.output); - yield* emitLifecycle( - step, - `${prefix}:delete:start`, - opts.emit, - "delete", - "start", - resource, - ); - - if (opts.dryRun) { - yield* emitLifecycle( - step, - `${prefix}:delete:dry-run`, - opts.emit, - "delete", - "dry-run", - resource, - ); - return; - } - - try { - yield* runPendingOperation( - step, - `${prefix}:delete`, - (context) => - resource.delete( - resource.key, - resource.toState(resource.output), - context, - ), - opts.maxOperationAttempts, - ); - - // State is removed only after the provider delete completes, and only if - // the store still matches the snapshot read before deleting. - const deleted = yield* stateStore.deleteFrom( - `${prefix}:state:delete`, - snapshot, - ); - if (!deleted.deleted) - throw new RevConflict(resource.id, stateNode.rev, undefined); - yield* emitLifecycle( - step, - `${prefix}:delete:success`, - opts.emit, - "delete", - "success", - resource, - ); - } catch (error) { - yield* emitLifecycle( - step, - `${prefix}:delete:error`, - opts.emit, - "delete", - "error", - resource, - { error }, - ); - throw error; - } -} - -export async function* readRemote( - step: DurableStep, - resource: BaseResource, - opts: DurableOperationOptions, - key: string, -): AsyncGenerator { - try { - return { - kind: "present", - output: yield* readResource(step, resource, opts, key), - }; - } catch (error) { - if (ResourceNotFoundError.is(error)) return { kind: "absent" }; - throw error; - } -} - -async function* readResource( - step: DurableStep, - resource: BaseResource, - opts: DurableOperationOptions, - key: string, -): AsyncGenerator, any> { - if (!resource.read) { - yield* emitLifecycle( - step, - `${key}:skip`, - opts.emit, - "read", - "skip", - resource, - { - reason: "read-not-implemented", - }, - ); - return { ...(await resource.getParams()), ...resource.output }; - } - - yield* emitLifecycle( - step, - `${key}:start`, - opts.emit, - "read", - "start", - resource, - ); - try { - const result = yield* runPendingOperation( - step, - key, - (context) => resource.read!(resource.key, context), - opts.maxOperationAttempts, - ); - yield* emitLifecycle( - step, - `${key}:success`, - opts.emit, - "read", - "success", - resource, - ); - return result; - } catch (error) { - yield* emitLifecycle( - step, - `${key}:error`, - opts.emit, - "read", - "error", - resource, - { - error, - }, - ); - throw error; - } + yield* deleteResourceOperation(step, { + ...operationParams(step, resource, opts, stateNode), + remove: removeResourceState(step, opts, resource, snapshot), + }); } /** @@ -385,28 +112,25 @@ async function* readResource( export async function* sweepOrphans( step: DurableStep, opts: DurableOperationOptions, - params: { - workflow: "deploy" | "destroy"; - listKey: string; - warningKey: (nodeId: string) => string; - deleteSuffix: string; - }, + workflow: "deploy" | "destroy", ): AsyncGenerator { const resourceById = new Map( opts.resources.map((resource) => [resource.id, resource]), ); - const persisted = yield* step.run(params.listKey, () => opts.state.values()); + const persisted = yield* step.run("list", () => opts.state.values()); const registry = opts.registry ?? createResourceRegistryFromResources(opts.resources); for (const node of persisted) { if (resourceById.has(node.id)) continue; + const nodeScope = scopeStep(step, node.id); const Resource = resolveResourceClass(registry, node.type as ResourceType); if (!Resource) { - yield* emitEvent(step, params.warningKey(node.id), opts.emit, () => + const emit = durableEmitter(nodeScope, opts.emit); + yield* emit( createMissingResourceRegistryMatchWarningEvent({ - workflow: params.workflow, + workflow, resourceId: node.id, resourceType: node.type as ResourceType, }), @@ -416,16 +140,113 @@ export async function* sweepOrphans( const resource = new Resource({ id: node.id, config: node.config }); resource.setOutput(node.output); - yield* deleteResource(step, resource, opts, params.deleteSuffix); + yield* deleteResource(nodeScope, resource, opts); } } -export function openResourceState( +/** + * Reads the persisted record once. The snapshot is kept so that later writes + * can be made conditional on the exact instance identity and version read + * here, and is re-served to the operations so they need no second read. + */ +async function* hydrateResource( + step: DurableStep, + resource: BaseResource, + opts: DurableOperationOptions, +): AsyncGenerator< + any, + { stateNode?: StateNode; snapshot?: ResourceSnapshot }, + any +> { + const snapshot = yield* readSnapshot(step, opts.state, resource.id); + if (!snapshot) return {}; + + const stateNode = toStateNode(snapshot); + resource.setOutput(stateNode.output); + return { stateNode, snapshot }; +} + +function readSnapshot( step: DurableStep, state: DurableStateBackend, resourceId: string, -) { - return step.store(resourceStateStore, { - id: state.storeId(resourceId), - }); +): AsyncGenerator { + return step.run("state:snapshot", () => state.snapshot(resourceId)); +} + +/** The half of the operation params every durable driver call site shares. */ +function operationParams( + step: DurableStep, + resource: BaseResource, + opts: DurableOperationOptions, + stateNode: StateNode | undefined, +): ResourceOperationBaseParams { + return { + resource, + // Serve the record already read during hydration rather than reading it + // again; a workflow must see the same value on every replay anyway. + state: { get: async () => stateNode }, + dryRun: opts.dryRun, + emit: durableEmitter(step, opts.emit), + maxOperationAttempts: opts.maxOperationAttempts, + }; +} + +/** + * State writes go through the workflow store, never through the state backend: + * the store stamps the write with the step that made it, so the applied-step + * ledger and the state change commit together. Replaying then returns the + * recorded result instead of retrying a compare-and-set that would now fail. + */ +function persistResourceState( + step: DurableStep, + opts: DurableOperationOptions, + resource: BaseResource, + snapshot: ResourceSnapshot | undefined, +): PersistState { + return async function* (next) { + if (!snapshot) { + // Create-if-absent. A racing writer would win here and this record would + // be silently adopted rather than written, which is safe only because a + // deployment is held exclusively for the length of the workflow. + yield* step.store(resourceStateStore, { + id: opts.state.storeId(resource.id), + initial: next, + }); + return; + } + + const store = yield* step.store(resourceStateStore, { + id: opts.state.storeId(resource.id), + }); + const result = yield* store.updateFrom( + "state:persist", + snapshot, + () => next, + ); + if (!result.updated) { + throw new RevConflict( + resource.id, + snapshot.version + 1, + result.actualVersion + 1, + ); + } + }; +} + +function removeResourceState( + step: DurableStep, + opts: DurableOperationOptions, + resource: BaseResource, + snapshot: ResourceSnapshot, +): RemoveState { + return async function* () { + const store = yield* step.store(resourceStateStore, { + id: opts.state.storeId(resource.id), + }); + const result = yield* store.deleteFrom("state:delete", snapshot); + if (!result.deleted) { + throw new RevConflict(resource.id, snapshot.version + 1, undefined); + } + }; } diff --git a/packages/reconciler/src/durable/state-backend.ts b/packages/reconciler/src/durable/state-backend.ts index 88fa35c..6e1130d 100644 --- a/packages/reconciler/src/durable/state-backend.ts +++ b/packages/reconciler/src/durable/state-backend.ts @@ -1,14 +1,20 @@ -import { RevConflict, type StateNode } from "@notation/state"; -import { randomUUID } from "node:crypto"; +import type { StateNode } from "@notation/state"; import { - RESOURCE_CREATION_TOKEN, resourceStateStore, toStateNode, - withoutRev, - type StoredResourceState, + type ResourceSnapshot, } from "./stores"; import type { StoreClient } from "./yieldstar"; +/** + * Reads deployment state from outside a workflow, for the planner and for + * anything reporting on a deployment. + * + * Deliberately read-only. Writes belong to the workflow, which makes them + * through the store handle so they are stamped with the step that made them. + * There is nowhere on this interface to carry that idempotency key, so a + * write made here would be repeated on replay rather than recognised. + */ export class DurableStateBackend { readonly #client: StoreClient; readonly #prefix: string; @@ -25,95 +31,16 @@ export class DurableStateBackend { } async get(id: string): Promise { - const snapshot = await this.#tryGetSnapshot(this.storeId(id)); + const snapshot = await this.snapshot(id); return snapshot ? toStateNode(snapshot) : undefined; } - async #tryGetSnapshot( - storeId: string, - ): Promise< - | { state: StoredResourceState; instanceId: string; version: number } - | undefined - > { - try { - return await this.#client.getStore({ - definition: resourceStateStore, - id: storeId, - }); - } catch (error) { - const ids = await this.#client.listStores(resourceStateStore); - if (!ids.includes(storeId)) return undefined; - throw error; - } - } - async has(id: string): Promise { return (await this.get(id)) !== undefined; } - async update( - id: string, - expectedRev: number, - patch: Partial, - ): Promise<{ rev: number }> { - const storeId = this.storeId(id); - const snapshot = await this.#tryGetSnapshot(storeId); - if (!snapshot) { - if (expectedRev !== 0) throw new RevConflict(id, expectedRev, undefined); - const creationToken = randomUUID(); - const initial = { - ...withoutRev(patch), - id, - [RESOURCE_CREATION_TOKEN]: creationToken, - } as StoredResourceState; - const created = await this.#client.getOrCreateStore({ - definition: resourceStateStore, - id: storeId, - initial, - }); - if (created.state[RESOURCE_CREATION_TOKEN] !== creationToken) { - throw new RevConflict(id, expectedRev, created.version + 1); - } - return { rev: created.version + 1 }; - } - - const actualRev = snapshot.version + 1; - if (actualRev !== expectedRev) - throw new RevConflict(id, expectedRev, actualRev); - const result = await this.#client.updateStoreFrom({ - definition: resourceStateStore, - id: storeId, - snapshot, - updater: (draft) => { - Object.assign(draft, withoutRev(patch)); - }, - }); - if (!result.updated) - throw new RevConflict(id, expectedRev, result.actualVersion + 1); - return { rev: result.version + 1 }; - } - - async delete(id: string, expectedRev: number): Promise { - const storeId = this.storeId(id); - const snapshot = await this.#tryGetSnapshot(storeId); - if (!snapshot) { - if (expectedRev !== 0) throw new RevConflict(id, expectedRev, undefined); - return; - } - const actualRev = snapshot.version + 1; - if (actualRev !== expectedRev) - throw new RevConflict(id, expectedRev, actualRev); - const result = await this.#client.deleteStoreFrom({ - definition: resourceStateStore, - id: storeId, - snapshot, - }); - if (!result.deleted) - throw new RevConflict( - id, - expectedRev, - result.reason === "conflict" ? result.actualVersion + 1 : undefined, - ); + snapshot(id: string): Promise { + return this.#read(this.storeId(id)); } async values(): Promise { @@ -121,43 +48,26 @@ export class DurableStateBackend { const snapshots = await Promise.all( ids .filter((id) => id.startsWith(this.#prefix)) - .map((id) => this.#tryGetSnapshot(id)), + .map((id) => this.#read(id)), ); return snapshots .filter((snapshot) => snapshot !== undefined) .map(toStateNode); } - snapshot(id: string) { - return this.#client.getStore({ - definition: resourceStateStore, - id: this.storeId(id), - }); - } - - async clear(): Promise { - const ids = await this.#client.listStores(resourceStateStore); - const scopedIds = ids.filter((id) => id.startsWith(this.#prefix)); - const snapshots = await Promise.all( - scopedIds.map((id) => this.#tryGetSnapshot(id)), - ); - await Promise.all( - scopedIds.map(async (id, index) => { - const snapshot = snapshots[index]; - if (!snapshot) return; - const result = await this.#client.deleteStoreFrom({ - definition: resourceStateStore, - id, - snapshot, - }); - if (!result.deleted && result.reason === "conflict") { - throw new RevConflict( - id.slice(this.#prefix.length), - snapshot.version + 1, - result.actualVersion + 1, - ); - } - }), - ); + // getStore throws for a store that does not exist rather than returning + // undefined, and the error is not distinguishable from a real failure, so + // absence is confirmed by listing. Kept here so no caller has to know that. + async #read(storeId: string): Promise { + try { + return await this.#client.getStore({ + definition: resourceStateStore, + id: storeId, + }); + } catch (error) { + const ids = await this.#client.listStores(resourceStateStore); + if (!ids.includes(storeId)) return undefined; + throw error; + } } } diff --git a/packages/reconciler/src/durable/step.ts b/packages/reconciler/src/durable/step.ts new file mode 100644 index 0000000..ad52ade --- /dev/null +++ b/packages/reconciler/src/durable/step.ts @@ -0,0 +1,84 @@ +import type { + EmitStep, + ReconcilerEvent, + ReconcilerEventEmitter, +} from "../events"; +import type { DurableStep, WorkflowStore } from "./yieldstar"; + +/** + * Namespaces the step keys of `step` so an operation can be written once and + * replayed at several call sites without its keys colliding. + * + * Opening a store is not prefixed: yieldstar derives that key from the store + * name and store id, which is already unique. The keys the store *handle* + * takes are caller-supplied, so those are scoped like any other step. + */ +export function scopeStep(step: DurableStep, prefix: string): DurableStep { + const scoped = (key: string) => `${prefix}:${key}`; + + return { + ...step, + // The keyless overloads fall through untouched; yieldstar hashes the call + // site for those, and a prefix would not make them any more unique. + run: ((arg1: unknown, arg2?: unknown) => + typeof arg1 === "string" + ? (step.run as any)(scoped(arg1), arg2) + : (step.run as any)(arg1)) as DurableStep["run"], + delay: ((arg1: unknown, arg2?: unknown) => + typeof arg1 === "string" + ? (step.delay as any)(scoped(arg1), arg2) + : (step.delay as any)(arg1)) as DurableStep["delay"], + store: ((definition: any, params: any) => + (async function* () { + const store = yield* step.store(definition, params); + return scopeStore(store, prefix); + })()) as DurableStep["store"], + }; +} + +/** + * Checkpoints delivery so that replaying a workflow does not re-emit. The key + * is derived from the event itself, which keeps it deterministic across a + * replay; the enclosing scope is what keeps it unique, since an operation + * emits each (operation, status) pair at most once. + * + * Emitters must still tolerate a duplicate: the process can crash after the + * event is delivered but before the checkpoint is written. + */ +export function durableEmitter( + step: DurableStep, + emit: ReconcilerEventEmitter | undefined, +): EmitStep { + return async function* (event) { + if (!emit) return; + yield* step.run(emitKey(event), () => emit(event)); + }; +} + +function emitKey(event: ReconcilerEvent): string { + return event.event === "reconciler.operation.lifecycle" + ? `emit:${event.event}:${event.operation}:${event.status}` + : `emit:${event.event}`; +} + +function scopeStore( + store: WorkflowStore, + prefix: string, +): WorkflowStore { + const scoped = (key: string) => `${prefix}:${key}`; + + return { + ...store, + get: (key?: string) => store.get(key === undefined ? key : scoped(key)), + select: (key, selector) => store.select(scoped(key), selector), + update: (key, updater) => store.update(scoped(key), updater), + updateFrom: (key, snapshot, updater) => + store.updateFrom(scoped(key), snapshot, updater), + deleteFrom: (key, snapshot) => store.deleteFrom(scoped(key), snapshot), + when: ((arg1: any, arg2?: any) => + typeof arg1 === "string" + ? store.when(scoped(arg1), arg2) + : store.when(arg1)) as WorkflowStore["when"], + take: (key, selector, claim) => store.take(scoped(key), selector, claim), + }; +} diff --git a/packages/reconciler/src/durable/stores.ts b/packages/reconciler/src/durable/stores.ts index d25f3ab..aca56fd 100644 --- a/packages/reconciler/src/durable/stores.ts +++ b/packages/reconciler/src/durable/stores.ts @@ -1,8 +1,6 @@ import type { StateNode } from "@notation/state"; import * as v from "valibot"; -import { defineStore } from "./yieldstar"; - -export const RESOURCE_CREATION_TOKEN = "$notationCreateToken"; +import { defineStore, type StoreSnapshot } from "./yieldstar"; export const resourceStateStore = defineStore( "resource-state", @@ -14,7 +12,6 @@ export const resourceStateStore = defineStore( output: v.record(v.string(), v.unknown()), lastOperation: v.picklist(["drift", "create", "update", "delete"]), lastOperationAt: v.string(), - [RESOURCE_CREATION_TOKEN]: v.optional(v.string()), }), ); @@ -30,18 +27,10 @@ export type CoordinationState = v.InferOutput< typeof deploymentCoordinationStore.schema >; -export function toStateNode(snapshot: { - state: StoredResourceState; - version: number; -}): StateNode { - const { [RESOURCE_CREATION_TOKEN]: _creationToken, ...state } = - snapshot.state; - return { ...state, rev: snapshot.version + 1 }; -} +/** A read of a resource record, carrying the identity a write is made against. */ +export type ResourceSnapshot = StoreSnapshot; -export function withoutRev( - patch: Partial, -): Partial { - const { rev: _rev, ...stored } = patch; - return stored; +/** Store versions count from zero, state revisions from one. */ +export function toStateNode(snapshot: ResourceSnapshot): StateNode { + return { ...snapshot.state, rev: snapshot.version + 1 }; } diff --git a/packages/reconciler/src/durable/yieldstar.ts b/packages/reconciler/src/durable/yieldstar.ts index 6bbba63..afa9871 100644 --- a/packages/reconciler/src/durable/yieldstar.ts +++ b/packages/reconciler/src/durable/yieldstar.ts @@ -2,7 +2,7 @@ import type { WorkflowFn } from "yieldstar"; export { defineStore } from "yieldstar"; export type { WorkflowStore } from "yieldstar"; -export type { StoreClient } from "@yieldstar/core"; +export type { StoreClient, StoreSnapshot } from "@yieldstar/core"; /** The durable step primitive the runtime hands to workflow functions. */ export type DurableStep = Parameters>[0]; diff --git a/packages/reconciler/src/events.ts b/packages/reconciler/src/events.ts index 2cad300..46aacaf 100644 --- a/packages/reconciler/src/events.ts +++ b/packages/reconciler/src/events.ts @@ -1,4 +1,5 @@ import type { ResourceType } from "@notation/resource"; +import type { MissingResourceRegistryMatchWarningEvent } from "./resource-registry"; export type OperationName = "create" | "read" | "update" | "delete"; @@ -46,8 +47,27 @@ export type ReconcilerEvent = | CoordinationWaitingEvent | ReconcilerDeployEvent | ReconcilerDriftDetectedEvent - | import("./resource-registry").MissingResourceRegistryMatchWarningEvent; + | MissingResourceRegistryMatchWarningEvent; export type ReconcilerEventEmitter = ( event: ReconcilerEvent, ) => void | Promise; + +/** + * The seam a driver fills in to deliver an event. Emission is a step so that + * each driver decides how it is recorded: the in-process driver simply awaits + * the emitter, while the durable driver checkpoints it so that replaying a + * workflow does not re-emit events it has already delivered. + */ +export type EmitStep = ( + event: TEvent, +) => AsyncGenerator; + +/** Adapts a plain emitter to the driver seam, for drivers that just await. */ +export function toEmitStep( + emit: ((event: TEvent) => void | Promise) | undefined, +): EmitStep { + return async function* (event) { + await emit?.(event); + }; +} diff --git a/packages/reconciler/src/index.ts b/packages/reconciler/src/index.ts index 44fc402..b4e749a 100644 --- a/packages/reconciler/src/index.ts +++ b/packages/reconciler/src/index.ts @@ -6,7 +6,9 @@ export type YieldstarApi = typeof import("yieldstar"); export * from "./resource-registry"; export * from "./operations"; export * from "./dependency-graph"; +export * from "./events"; export * from "./plan"; +export * from "./planner"; export * from "./reconciler"; export * from "./logger-subscriber"; export * from "./protocol"; diff --git a/packages/reconciler/src/operations/operation.create.ts b/packages/reconciler/src/operations/operation.create.ts index e0aee4f..5cecb14 100644 --- a/packages/reconciler/src/operations/operation.create.ts +++ b/packages/reconciler/src/operations/operation.create.ts @@ -12,10 +12,10 @@ export async function* createResourceOperation( step: StepRunner, params: CreateResourceParams, ): AsyncGenerator { - await emitLifecycleEvent(params, "create", "start"); + yield* emitLifecycleEvent(params, "create", "start"); if (params.dryRun) { - await emitLifecycleEvent(params, "create", "dry-run"); + yield* emitLifecycleEvent(params, "create", "dry-run"); return; } @@ -51,23 +51,21 @@ export async function* createResourceOperation( ...readResult, }); - yield* step.run("create:persist-state", async () => { - await params.state.update(params.resource.id, params.expectedRev, { - id: params.resource.id, - groupId: params.resource.groupId, - groupType: params.resource.groupType, - type: params.resource.type, - lastOperation: "create", - lastOperationAt: new Date().toISOString(), - config: params.resource.config, - params: params.resource.toState(resourceParams), - output: params.resource.toState(params.resource.output), - }); + yield* params.persist({ + id: params.resource.id, + groupId: params.resource.groupId, + groupType: params.resource.groupType, + type: params.resource.type, + lastOperation: "create", + lastOperationAt: new Date().toISOString(), + config: params.resource.config, + params: params.resource.toState(resourceParams), + output: params.resource.toState(params.resource.output), }); - await emitLifecycleEvent(params, "create", "success"); + yield* emitLifecycleEvent(params, "create", "success"); } catch (err) { - await emitLifecycleEvent(params, "create", "error", getErrorDetails(err)); + yield* emitLifecycleEvent(params, "create", "error", getErrorDetails(err)); throw err; } } diff --git a/packages/reconciler/src/operations/operation.delete.ts b/packages/reconciler/src/operations/operation.delete.ts index 51975b6..170927f 100644 --- a/packages/reconciler/src/operations/operation.delete.ts +++ b/packages/reconciler/src/operations/operation.delete.ts @@ -11,10 +11,10 @@ export async function* deleteResourceOperation( step: StepRunner, params: DeleteResourceParams, ): AsyncGenerator { - await emitLifecycleEvent(params, "delete", "start"); + yield* emitLifecycleEvent(params, "delete", "start"); if (params.dryRun) { - await emitLifecycleEvent(params, "delete", "dry-run"); + yield* emitLifecycleEvent(params, "delete", "dry-run"); return; } @@ -31,13 +31,11 @@ export async function* deleteResourceOperation( params.maxOperationAttempts, ); - yield* step.run("delete:persist-state", () => - params.state.delete(params.resource.id, params.expectedRev), - ); + yield* params.remove(); - await emitLifecycleEvent(params, "delete", "success"); + yield* emitLifecycleEvent(params, "delete", "success"); } catch (err) { - await emitLifecycleEvent(params, "delete", "error", getErrorDetails(err)); + yield* emitLifecycleEvent(params, "delete", "error", getErrorDetails(err)); throw err; } } diff --git a/packages/reconciler/src/operations/operation.read.ts b/packages/reconciler/src/operations/operation.read.ts index e74196a..327e405 100644 --- a/packages/reconciler/src/operations/operation.read.ts +++ b/packages/reconciler/src/operations/operation.read.ts @@ -1,4 +1,6 @@ +import { ResourceNotFoundError } from "@notation/resource"; import { createWorkflow } from "yieldstar"; +import type { DriftRead } from "../plan"; import { type ReadResourceParams, type StepRunner, @@ -11,10 +13,10 @@ export async function* readResourceOperation( step: StepRunner, params: ReadResourceParams, ): AsyncGenerator, unknown> { - await emitLifecycleEvent(params, "read", "start"); + yield* emitLifecycleEvent(params, "read", "start"); if (params.dryRun) { - await emitLifecycleEvent(params, "read", "dry-run"); + yield* emitLifecycleEvent(params, "read", "dry-run"); return {}; } @@ -31,10 +33,10 @@ export async function* readResourceOperation( ? { ...stateNode.output, ...resourceParams } : resourceParams; - await emitLifecycleEvent(params, "read", "skip", { + yield* emitLifecycleEvent(params, "read", "skip", { reason: "read-not-implemented", }); - await emitLifecycleEvent(params, "read", "success"); + yield* emitLifecycleEvent(params, "read", "success"); return merged as Record; } @@ -50,14 +52,32 @@ export async function* readResourceOperation( ...remote, }; - await emitLifecycleEvent(params, "read", "success"); + yield* emitLifecycleEvent(params, "read", "success"); return mergedOutput; } catch (err) { - await emitLifecycleEvent(params, "read", "error", getErrorDetails(err)); + yield* emitLifecycleEvent(params, "read", "error", getErrorDetails(err)); throw err; } } +/** + * Reads the remote to compare it against persisted state. An absent resource + * is a fact about the world rather than a failure, so it is reported as such; + * every other error still propagates. + */ +export async function* readDriftOperation( + step: StepRunner, + params: ReadResourceParams, +): AsyncGenerator { + try { + const output = yield* readResourceOperation(step, params); + return { kind: "present", output }; + } catch (error) { + if (ResourceNotFoundError.is(error)) return { kind: "absent" }; + throw error; + } +} + export const readResourceWorkflow: unknown = createWorkflow( async function* (step, event) { return yield* readResourceOperation( diff --git a/packages/reconciler/src/operations/operation.types.ts b/packages/reconciler/src/operations/operation.types.ts index 6cbcd1c..5d9d640 100644 --- a/packages/reconciler/src/operations/operation.types.ts +++ b/packages/reconciler/src/operations/operation.types.ts @@ -1,26 +1,19 @@ -import type { BaseResource, ResourceType } from "@notation/resource"; -import type { State } from "@notation/state"; +import type { BaseResource } from "@notation/resource"; +import type { State, StateNode } from "@notation/state"; +import type { + EmitStep, + OperationLifecycleEvent, + OperationLifecycleStatus, + OperationName, +} from "../events"; -export type OperationName = "create" | "read" | "update" | "delete"; +export type { + OperationLifecycleEvent, + OperationLifecycleStatus, + OperationName, +} from "../events"; -export type OperationLifecycleStatus = - "start" | "success" | "error" | "skip" | "dry-run"; - -export type OperationLifecycleEvent = { - level: "info" | "error"; - event: "reconciler.operation.lifecycle"; - operation: OperationName; - status: OperationLifecycleStatus; - resourceId: string; - resourceType: ResourceType; - reason?: string; - errorName?: string; - errorMessage?: string; -}; - -export type OperationEventEmitter = ( - event: OperationLifecycleEvent, -) => void | Promise; +export type OperationEventEmitter = EmitStep; export type StepRunner = { run(fn: () => T | Promise): AsyncGenerator; @@ -32,27 +25,55 @@ export type StepRunner = { delay(key: string, ms: number): AsyncGenerator; }; +/** + * The record an operation wants persisted; the driver owns the revision. + * Spelled out rather than derived with Omit, which would collapse against + * StateNode's index signature and widen every field to unknown. + */ +export type PersistedResourceState = Pick< + StateNode, + | "id" + | "type" + | "config" + | "params" + | "output" + | "lastOperation" + | "lastOperationAt" +> & { [key: string]: unknown }; + +/** + * How a driver writes state. Both are steps so that each driver can carry its + * own concurrency control: in process that is a compare-and-set against the + * revision read before the operation, and in a workflow it is a store write + * stamped with the step that made it, so a replay does not repeat it. + */ +export type PersistState = ( + next: PersistedResourceState, +) => AsyncGenerator; + +export type RemoveState = () => AsyncGenerator; + export type ResourceOperationBaseParams = { resource: BaseResource; - state: Pick; + state: Pick; dryRun?: boolean; emit?: OperationEventEmitter; maxOperationAttempts?: number; }; export type CreateResourceParams = ResourceOperationBaseParams & { - expectedRev: number; + persist: PersistState; }; export type ReadResourceParams = ResourceOperationBaseParams; export type UpdateResourceParams = ResourceOperationBaseParams & { patch: Record; - expectedRev: number; + persist: PersistState; }; export type DeleteResourceParams = ResourceOperationBaseParams & { - expectedRev: number; + remove: RemoveState; }; export function getErrorDetails(err: unknown): { @@ -72,15 +93,15 @@ export function getErrorDetails(err: unknown): { }; } -export async function emitLifecycleEvent( +export async function* emitLifecycleEvent( params: ResourceOperationBaseParams, operation: OperationName, status: OperationLifecycleStatus, extra: Partial = {}, -) { +): AsyncGenerator { if (!params.emit) return; - await params.emit({ + yield* params.emit({ level: status === "error" ? "error" : "info", event: "reconciler.operation.lifecycle", operation, diff --git a/packages/reconciler/src/operations/operation.update.ts b/packages/reconciler/src/operations/operation.update.ts index fc62a8c..5f07609 100644 --- a/packages/reconciler/src/operations/operation.update.ts +++ b/packages/reconciler/src/operations/operation.update.ts @@ -12,18 +12,18 @@ export async function* updateResourceOperation( step: StepRunner, params: UpdateResourceParams, ): AsyncGenerator { - await emitLifecycleEvent(params, "update", "start"); + yield* emitLifecycleEvent(params, "update", "start"); if (params.dryRun) { - await emitLifecycleEvent(params, "update", "dry-run"); + yield* emitLifecycleEvent(params, "update", "dry-run"); return; } if (!params.resource.update) { - await emitLifecycleEvent(params, "update", "skip", { + yield* emitLifecycleEvent(params, "update", "skip", { reason: "update-not-implemented", }); - await emitLifecycleEvent(params, "update", "success"); + yield* emitLifecycleEvent(params, "update", "success"); return; } @@ -63,23 +63,21 @@ export async function* updateResourceOperation( ...readResult, }); - yield* step.run("update:persist-state", async () => { - await params.state.update(params.resource.id, params.expectedRev, { - id: params.resource.id, - groupId: params.resource.groupId, - groupType: params.resource.groupType, - type: params.resource.type, - lastOperation: "update", - lastOperationAt: new Date().toISOString(), - config: params.resource.config, - params: params.resource.toState(resourceParams), - output: params.resource.toState(params.resource.output), - }); + yield* params.persist({ + id: params.resource.id, + groupId: params.resource.groupId, + groupType: params.resource.groupType, + type: params.resource.type, + lastOperation: "update", + lastOperationAt: new Date().toISOString(), + config: params.resource.config, + params: params.resource.toState(resourceParams), + output: params.resource.toState(params.resource.output), }); - await emitLifecycleEvent(params, "update", "success"); + yield* emitLifecycleEvent(params, "update", "success"); } catch (err) { - await emitLifecycleEvent(params, "update", "error", getErrorDetails(err)); + yield* emitLifecycleEvent(params, "update", "error", getErrorDetails(err)); throw err; } } diff --git a/packages/reconciler/src/planner.ts b/packages/reconciler/src/planner.ts index ebfa064..ef7faca 100644 --- a/packages/reconciler/src/planner.ts +++ b/packages/reconciler/src/planner.ts @@ -1,7 +1,8 @@ -import { ResourceNotFoundError, type BaseResource } from "@notation/resource"; -import type { StateBackend } from "@notation/state"; +import type { BaseResource } from "@notation/resource"; +import type { State } from "@notation/state"; import { buildResourceDepthLevels } from "./dependency-graph"; -import { readResourceOperation } from "./operations"; +import { toEmitStep, type ReconcilerEventEmitter } from "./events"; +import { readDriftOperation } from "./operations"; import { decideAction, getDependencyIds, @@ -9,22 +10,30 @@ import { type Plan, type PlanNode, } from "./plan"; -import { createStepRunner, runOperation } from "./reconciler"; +import { createStepRunner, runOperation } from "./step-runner"; + +/** Planning reads state and never mutates it, so no lease is required. */ +export type PlannerState = Pick; export type CreatePlanOptions = { resources: BaseResource[]; - state: StateBackend; + state: PlannerState; driftDetection?: boolean; + emit?: ReconcilerEventEmitter; + maxOperationAttempts?: number; }; export async function createPlan({ resources, state, driftDetection = true, + emit, + maxOperationAttempts, }: CreatePlanOptions): Promise { const resourceById = new Map( resources.map((resource) => [resource.id, resource]), ); + const emitStep = emit ? toEmitStep(emit) : undefined; const nodes: PlanNode[] = []; for (const level of buildResourceDepthLevels(resources)) { @@ -35,19 +44,14 @@ export async function createPlan({ let action = decideAction({ resource, stateNode, params }); if (action.decision === "noop" && driftDetection && resource.read) { - let driftRead; - try { - const output = await runOperation( - readResourceOperation(createStepRunner(), { - resource, - state, - }), - ); - driftRead = { kind: "present" as const, output }; - } catch (error) { - if (!ResourceNotFoundError.is(error)) throw error; - driftRead = { kind: "absent" as const }; - } + const driftRead = await runOperation( + readDriftOperation(createStepRunner(), { + resource, + state, + emit: emitStep, + maxOperationAttempts, + }), + ); action = decideAction({ resource, diff --git a/packages/reconciler/src/protocol.ts b/packages/reconciler/src/protocol.ts index bf3706e..a8ab407 100644 --- a/packages/reconciler/src/protocol.ts +++ b/packages/reconciler/src/protocol.ts @@ -1,4 +1,4 @@ -import type { ReconcilerEvent, ReconcilerEventEmitter } from "./reconciler"; +import type { ReconcilerEvent, ReconcilerEventEmitter } from "./events"; export const EVENT_STREAM_VERSION = 1 as const; diff --git a/packages/reconciler/src/reconciler.ts b/packages/reconciler/src/reconciler.ts index fab6c0b..8db798b 100644 --- a/packages/reconciler/src/reconciler.ts +++ b/packages/reconciler/src/reconciler.ts @@ -1,4 +1,3 @@ -import { ResourceNotFoundError } from "@notation/resource"; import type { BaseResource, ResourceType } from "@notation/resource"; import { RevConflict, type State, type StateNode } from "@notation/state"; import { setTimeout as sleep } from "node:timers/promises"; @@ -15,8 +14,10 @@ import { import { createResourceOperation, deleteResourceOperation, - readResourceOperation, - type OperationLifecycleEvent, + readDriftOperation, + type OperationEventEmitter, + type PersistState, + type RemoveState, type StepRunner, updateResourceOperation, } from "./operations"; @@ -24,35 +25,19 @@ import { createMissingResourceRegistryMatchWarningEvent, createResourceRegistryFromResources, resolveResourceClass, - type MissingResourceRegistryMatchWarningEvent, type ResourceRegistry, } from "./resource-registry"; - -export type ReconcilerDeployEvent = { - level: "info"; - event: "reconciler.deploy.decision"; - resourceId: string; - resourceType: string; - decision: "create" | "update" | "drift-update" | "drift-recreate" | "noop"; -}; - -export type ReconcilerDriftDetectedEvent = { - level: "info"; - event: "reconciler.drift.detected"; - resourceId: string; - resourceType: string; - diff: Record; -}; - -export type ReconcilerEvent = - | OperationLifecycleEvent - | ReconcilerDeployEvent - | ReconcilerDriftDetectedEvent - | MissingResourceRegistryMatchWarningEvent; - -export type ReconcilerEventEmitter = ( - event: ReconcilerEvent, -) => void | Promise; +import { toEmitStep, type ReconcilerEventEmitter } from "./events"; +import { createPlan } from "./planner"; +import { createStepRunner, runOperation } from "./step-runner"; + +export type { + ReconcilerDeployEvent, + ReconcilerDriftDetectedEvent, + ReconcilerEvent, + ReconcilerEventEmitter, +} from "./events"; +export { createStepRunner, runOperation } from "./step-runner"; export type ReconcilerState = Pick< State, @@ -92,6 +77,7 @@ export class Reconciler { readonly #defaultDryRun: boolean; readonly #defaultDriftDetection: boolean; readonly #emit?: ReconcilerEventEmitter; + readonly #emitStep?: OperationEventEmitter; readonly #maxOperationAttempts?: number; readonly #mutationLeaseTtl: number; readonly #stepRunner: StepRunner; @@ -102,6 +88,8 @@ export class Reconciler { this.#defaultDryRun = opts.dryRun ?? false; this.#defaultDriftDetection = opts.driftDetection ?? true; this.#emit = opts.emit; + // Operations emit through a step seam; in process that is a plain await. + this.#emitStep = opts.emit ? toEmitStep(opts.emit) : undefined; this.#maxOperationAttempts = opts.maxOperationAttempts; this.#mutationLeaseTtl = opts.mutationLeaseTtl ?? 30_000; this.#stepRunner = createStepRunner(); @@ -130,36 +118,13 @@ export class Reconciler { } async plan(resources: BaseResource[], opts: PlanOptions = {}): Promise { - const driftDetection = opts.driftDetection ?? this.#defaultDriftDetection; - const resourceById = new Map( - resources.map((resource) => [resource.id, resource]), - ); - const nodes: PlanNode[] = []; - - const dependencyLevels = buildResourceDepthLevels(resources); - for (const level of dependencyLevels) { - for (const resource of level) { - nodes.push(await this.#planResource(resource, driftDetection)); - } - } - - const stateNodes = await this.#state.values(); - for (const stateNode of stateNodes) { - if (resourceById.has(stateNode.id)) continue; - - nodes.push({ - id: stateNode.id, - type: stateNode.type, - decision: "delete-orphan", - params: stateNode.params, - dependsOn: [], - }); - } - - return { - createdAt: new Date().toISOString(), - nodes, - }; + return createPlan({ + resources, + state: this.#state, + driftDetection: opts.driftDetection ?? this.#defaultDriftDetection, + emit: this.#emit, + maxOperationAttempts: this.#maxOperationAttempts, + }); } async destroy( @@ -310,9 +275,9 @@ export class Reconciler { resource, state: this.#state, dryRun, - emit: this.#emit, + emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, - expectedRev: stateNode?.rev ?? 0, + persist: this.#persist(resource.id, stateNode?.rev ?? 0), }), ); return; @@ -325,9 +290,9 @@ export class Reconciler { state: this.#state, patch: action.patch, dryRun, - emit: this.#emit, + emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, - expectedRev: stateNode!.rev, + persist: this.#persist(resource.id, stateNode!.rev), }), ); return; @@ -372,9 +337,9 @@ export class Reconciler { resource, state: this.#state, dryRun, - emit: this.#emit, + emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, - expectedRev: stateNode?.rev ?? 0, + persist: this.#persist(resource.id, stateNode?.rev ?? 0), }), ); return; @@ -386,9 +351,9 @@ export class Reconciler { state: this.#state, patch: action.patch, dryRun, - emit: this.#emit, + emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, - expectedRev: stateNode?.rev ?? 0, + persist: this.#persist(resource.id, stateNode?.rev ?? 0), }), ); return; @@ -409,48 +374,31 @@ export class Reconciler { } } - async #planResource( - resource: BaseResource, - driftDetection: boolean, - ): Promise { - const stateNode = await this.#state.get(resource.id); - if (stateNode) { - resource.setOutput(stateNode.output); - } - - const params = await resolvePlanParams(resource); - let action = decideAction({ resource, stateNode, params }); - - if (action.decision === "noop" && driftDetection) { - const driftRead = await this.#readForDrift(resource); - action = decideAction({ resource, stateNode, params, driftRead }); - } + // In process, concurrency control is a compare-and-set against the revision + // read before the operation started; the mutation lease keeps writers apart. + #persist(resourceId: string, expectedRev: number): PersistState { + const state = this.#state; + return async function* (next) { + await state.update(resourceId, expectedRev, next); + }; + } - return { - id: resource.id, - type: resource.type, - decision: action.decision, - ...("diff" in action ? { diff: action.diff } : {}), - params, - dependsOn: getDependencyIds(resource), + #remove(resourceId: string, expectedRev: number): RemoveState { + const state = this.#state; + return async function* () { + await state.delete(resourceId, expectedRev); }; } - async #readForDrift(resource: BaseResource): Promise { - try { - const output = await runOperation( - readResourceOperation(this.#stepRunner, { - resource, - state: this.#state, - emit: this.#emit, - maxOperationAttempts: this.#maxOperationAttempts, - }), - ); - return { kind: "present", output }; - } catch (error) { - if (ResourceNotFoundError.is(error)) return { kind: "absent" }; - throw error; - } + #readForDrift(resource: BaseResource): Promise { + return runOperation( + readDriftOperation(this.#stepRunner, { + resource, + state: this.#state, + emit: this.#emitStep, + maxOperationAttempts: this.#maxOperationAttempts, + }), + ); } async #deleteOrphans( @@ -539,24 +487,14 @@ export class Reconciler { resource, state: this.#state, dryRun, - emit: this.#emit, + emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, - expectedRev: stateNode.rev, + remove: this.#remove(resource.id, stateNode.rev), }), ); } } -export async function runOperation( - operation: AsyncGenerator, -) { - let next = await operation.next(); - while (!next.done) { - next = await operation.next(); - } - return next.value; -} - function hydrateResourceFromState( Resource: new (opts: { id: string; @@ -571,32 +509,3 @@ function hydrateResourceFromState( resource.setOutput(stateNode.output); return resource; } - -export function createStepRunner(): StepRunner { - return { - async *run( - arg1: string | (() => T | Promise), - arg2?: () => T | Promise, - ): AsyncGenerator { - const fn = (typeof arg1 === "string" ? arg2 : arg1) as - (() => T | Promise) | undefined; - - if (!fn) { - throw new Error("Missing run function"); - } - - return await fn(); - }, - async *delay( - arg1: string | number, - arg2?: number, - ): AsyncGenerator { - const ms = typeof arg1 === "number" ? arg1 : arg2; - if (ms === undefined) { - throw new Error("Missing delay duration"); - } - - await new Promise((resolve) => setTimeout(resolve, ms)); - }, - }; -} diff --git a/packages/reconciler/src/step-runner.ts b/packages/reconciler/src/step-runner.ts new file mode 100644 index 0000000..61cc6d5 --- /dev/null +++ b/packages/reconciler/src/step-runner.ts @@ -0,0 +1,40 @@ +import type { StepRunner } from "./operations"; + +export async function runOperation( + operation: AsyncGenerator, +) { + let next = await operation.next(); + while (!next.done) { + next = await operation.next(); + } + return next.value; +} + +export function createStepRunner(): StepRunner { + return { + async *run( + arg1: string | (() => T | Promise), + arg2?: () => T | Promise, + ): AsyncGenerator { + const fn = (typeof arg1 === "string" ? arg2 : arg1) as + (() => T | Promise) | undefined; + + if (!fn) { + throw new Error("Missing run function"); + } + + return await fn(); + }, + async *delay( + arg1: string | number, + arg2?: number, + ): AsyncGenerator { + const ms = typeof arg1 === "number" ? arg1 : arg2; + if (ms === undefined) { + throw new Error("Missing delay duration"); + } + + await new Promise((resolve) => setTimeout(resolve, ms)); + }, + }; +} diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index 1a51cd7..96e2ee9 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -75,7 +75,7 @@ describe("durable execution and replay", () => { const runtime = createRuntime( [new TestResource({ id: "resume" })], "crash-resume", - { crashAfterStep: "notation:resource:resume:create:attempt:0" }, + { crashAfterStep: "notation:resource:resume:create:remote:attempt:0" }, ); await expect(runtime.run("resume-execution")).rejects.toThrow( @@ -98,7 +98,7 @@ describe("durable execution and replay", () => { const runtime = createRuntime( [new TestResource({ id: "destroyed" })], "destroy-crash-resume", - { crashAfterStep: "notation:destroy:destroyed:delete:attempt:0" }, + { crashAfterStep: "notation:destroy:destroyed:delete:remote:attempt:0" }, ); await runtime.run("deploy-before-destroy"); @@ -242,69 +242,75 @@ describe("dependency ordering", () => { }); describe("conditional state persistence", () => { - it("allows only one concurrent create-if-absent", async () => { - const runtime = createRuntime([], "conditional-create"); - const first = runtime.state.update("resource", 0, { - ...statePatch("resource"), - rev: 41, - output: { winner: "first" }, - }); - const second = runtime.state.update("resource", 0, { - ...statePatch("resource"), - rev: 42, - output: { winner: "second" }, - }); + it("rejects a state write whose snapshot another writer has moved past", async () => { + const RaceResource = resource({ type: "test/durable/write-race" }) + .defineSchema({ + name: { + presence: "required", + propertyType: "param", + valueType: "string" as any, + }, + }) + .defineOperations({ + create: async () => undefined, + // Moves the store on between the workflow reading its snapshot and + // persisting against it, which is what the conditional write guards. + update: async () => { + await runtime.storeClient.updateStore({ + definition: durable.resourceStateStore, + id: runtime.state.storeId("raced"), + updater: (draft: any) => { + draft.lastOperationAt = "1999-01-01T00:00:00.000Z"; + }, + }); + }, + delete: async () => undefined, + }); + const resources = [ + new RaceResource({ id: "raced", config: { name: "before" } }), + ]; + const runtime = createRuntime(resources, "write-race"); - const results = await Promise.allSettled([first, second]); + await runtime.run("deploy-1"); + resources[0] = new RaceResource({ id: "raced", config: { name: "after" } }); - expect( - results.filter((result) => result.status === "fulfilled"), - ).toHaveLength(1); - expect( - results.filter((result) => result.status === "rejected"), - ).toHaveLength(1); - const state = await runtime.state.get("resource"); - expect(state).toMatchObject({ rev: 1 }); - expect(state).not.toHaveProperty("$notationCreateToken"); + await expect(runtime.run("deploy-2")).rejects.toMatchObject({ + name: "RevConflict", + }); + // The losing write left the other writer's record intact. + expect(await runtime.state.get("raced")).toMatchObject({ + rev: 2, + lastOperationAt: "1999-01-01T00:00:00.000Z", + }); runtime.close(); }); - it("uses store identity and version for conditional update and delete", async () => { - const runtime = createRuntime([], "conditional-state"); - await runtime.state.update("resource", 0, statePatch("resource")); - const originalSnapshot = await runtime.state.snapshot("resource"); - - const first = runtime.state.update("resource", 1, { - output: { winner: "first" }, - }); - const second = runtime.state.update("resource", 1, { - output: { winner: "second" }, - }); - const results = await Promise.allSettled([first, second]); + it("rejects a state removal whose snapshot another writer has moved past", async () => { + const RaceResource = resource({ type: "test/durable/delete-race" }) + .defineSchema({}) + .defineOperations({ + create: async () => undefined, + delete: async () => { + await runtime.storeClient.updateStore({ + definition: durable.resourceStateStore, + id: runtime.state.storeId("delete-raced"), + updater: (draft: any) => { + draft.lastOperationAt = "1999-01-01T00:00:00.000Z"; + }, + }); + }, + }); + const runtime = createRuntime( + [new RaceResource({ id: "delete-raced" })], + "delete-race", + ); - expect( - results.filter((result) => result.status === "fulfilled"), - ).toHaveLength(1); - expect( - results.filter((result) => result.status === "rejected"), - ).toHaveLength(1); - await expect(runtime.state.delete("resource", 1)).rejects.toMatchObject({ + await runtime.run("deploy-1"); + await expect(runtime.destroy("destroy-1")).rejects.toMatchObject({ name: "RevConflict", }); - expect(await runtime.state.get("resource")).toMatchObject({ rev: 2 }); - - await runtime.state.clear(); - await runtime.state.update("resource", 0, statePatch("resource")); - const staleDelete = await runtime.storeClient.deleteStoreFrom({ - definition: durable.resourceStateStore, - id: runtime.state.storeId("resource"), - snapshot: originalSnapshot, - }); - expect(staleDelete).toMatchObject({ - deleted: false, - reason: "conflict", - }); - expect(await runtime.state.get("resource")).toMatchObject({ rev: 1 }); + // State survives a removal that could not be proven safe. + expect(await runtime.state.get("delete-raced")).toBeDefined(); runtime.close(); }); }); @@ -403,16 +409,17 @@ describe("deployment scoping", () => { const app = new durable.DurableStateBackend(storeClient, "app"); const appBlue = new durable.DurableStateBackend(storeClient, "app:blue"); - await app.update("site", 0, statePatch("site")); - await appBlue.update("site", 0, statePatch("site")); - - expect(await app.values()).toHaveLength(1); - expect(await appBlue.values()).toHaveLength(1); - - await app.clear(); - expect(await app.values()).toHaveLength(0); - expect(await appBlue.values()).toHaveLength(1); - expect(await appBlue.get("site")).toBeDefined(); + await seedResourceState(storeClient, app.storeId("site"), "site"); + await seedResourceState(storeClient, appBlue.storeId("site"), "blue-site"); + + // A deployment named "app:blue" falls inside a naive "app:" prefix scan; + // encoding the deployment id is what keeps the two listings disjoint. + expect((await app.values()).map((node) => node.id)).toEqual(["site"]); + expect((await appBlue.values()).map((node) => node.id)).toEqual([ + "blue-site", + ]); + expect(await app.get("site")).toMatchObject({ id: "site" }); + expect(await appBlue.get("site")).toMatchObject({ id: "blue-site" }); database.close(); }); }); @@ -613,6 +620,18 @@ class CrashAfterWriteHeap implements HeapClient { } } +function seedResourceState( + storeClient: SqliteStoreClient, + storeId: string, + resourceId: string, +) { + return storeClient.getOrCreateStore({ + definition: durable.resourceStateStore, + id: storeId, + initial: statePatch(resourceId), + }); +} + function statePatch(id: string) { return { id, diff --git a/packages/reconciler/test/operation.workflows.test.ts b/packages/reconciler/test/operation.workflows.test.ts index 2afc6df..a721a29 100644 --- a/packages/reconciler/test/operation.workflows.test.ts +++ b/packages/reconciler/test/operation.workflows.test.ts @@ -11,6 +11,7 @@ import { type OperationLifecycleEvent, type StepRunner, } from "../src/operations"; +import { toEmitStep } from "../src/events"; function createStepRunnerDouble(): StepRunner { const run = vi.fn(async function* ( @@ -51,11 +52,8 @@ describe("operation workflows", () => { it("create performs create + read-after-create + state persistence", async () => { const step = createStepRunnerDouble(); const events: OperationLifecycleEvent[] = []; - const state = { - get: vi.fn(async () => undefined), - update: vi.fn(async () => undefined), - delete: vi.fn(async () => undefined), - }; + const state = { get: vi.fn(async () => undefined) }; + const persist = vi.fn(async function* () {}); let createAttempts = 0; const createMock = vi.fn(async (_params, context) => { @@ -86,15 +84,25 @@ describe("operation workflows", () => { createResourceOperation(step, { resource: testResource, state, - expectedRev: 0, - emit: async (event) => { - events.push(event); - }, + persist, + emit: toEmitStep((event) => void events.push(event)), }), ); expect(createAttempts).toBe(2); - expect(state.update).toHaveBeenCalledOnce(); + expect(persist).toHaveBeenCalledOnce(); + expect(persist).toHaveBeenCalledWith({ + id: "test-create", + groupId: testResource.groupId, + groupType: testResource.groupType, + type: TestResource.type, + lastOperation: "create", + lastOperationAt: expect.any(String), + config: testResource.config, + params: {}, + // The resource declares no schema, so nothing survives toState. + output: {}, + }); expect(createMock).toHaveBeenNthCalledWith( 1, await testResource.getParams(), @@ -231,11 +239,8 @@ describe("operation workflows", () => { it("delete treats an already-absent remote as success through its idempotent resource contract", async () => { const step = createStepRunnerDouble(); const events: OperationLifecycleEvent[] = []; - const state = { - get: vi.fn(async () => undefined), - update: vi.fn(async () => undefined), - delete: vi.fn(async () => undefined), - }; + const state = { get: vi.fn(async () => undefined) }; + const remove = vi.fn(async function* () {}); const TestResource = resource({ type: "test/service/delete" }) .defineSchema({}) @@ -250,14 +255,14 @@ describe("operation workflows", () => { deleteResourceOperation(step, { resource: testResource, state, - expectedRev: 1, - emit: async (event) => { - events.push(event); - }, + remove, + emit: toEmitStep((event) => void events.push(event)), }), ); - expect(state.delete).toHaveBeenCalledWith("test-delete", 1); + // State is removed only after the provider delete resolves; which record + // and revision that targets is the driver's concern, not the operation's. + expect(remove).toHaveBeenCalledOnce(); expect(events.map((event) => event.status)).toEqual(["start", "success"]); }); @@ -326,9 +331,7 @@ describe("operation workflows", () => { resource: testResource, state, expectedRev: 0, - emit: async (event) => { - events.push(event); - }, + emit: toEmitStep((event) => void events.push(event)), }), ), ).rejects.toMatchObject({ name: "CreateFailed", message: "boom" }); diff --git a/packages/reconciler/tsup.config.ts b/packages/reconciler/tsup.config.ts index f0ac238..2834ee4 100644 --- a/packages/reconciler/tsup.config.ts +++ b/packages/reconciler/tsup.config.ts @@ -1,7 +1,7 @@ import { defineConfig } from "tsup"; export default defineConfig({ - entry: ["src/index.ts"], + entry: ["src/index.ts", "src/durable/index.ts"], dts: true, format: ["esm"], }); From 551466a52ccf73e4919efa60f742093a9e83814a Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 19:15:33 +0100 Subject: [PATCH 08/17] Stop the durable retry tests racing the workflow loop 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. --- .../test/durable-reconciliation.test.ts | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index 96e2ee9..4cf80cd 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -27,6 +27,15 @@ import { const logger = pino({ level: "silent" }); +/** + * A retry delay has to outlive the heap write that follows it. The workflow + * loop continues inline for a delay that has already elapsed by the time it + * is reached, so a delay shorter than a SQLite write runs the retry in the + * same execution — which is the opposite of what these tests assert. + */ +const RETRY_AFTER_MS = 50; +const PAST_RETRY_MS = RETRY_AFTER_MS + 25; + describe("durable execution and replay", () => { it("waits durably for a retryable provider and persists after success", async () => { let attempts = 0; @@ -38,7 +47,7 @@ describe("durable execution and replay", () => { if (attempts === 1) { expect(context).toBeUndefined(); throw new ResourceOperationPendingError("provider is not ready", { - retryAfterMs: 1, + retryAfterMs: RETRY_AFTER_MS, callbackContext: { requestId: "request-123" }, }); } @@ -56,7 +65,7 @@ describe("durable execution and replay", () => { expect(attempts).toBe(1); expect(runtime.scheduler.events).toHaveLength(1); - await sleep(5); + await sleep(PAST_RETRY_MS); await runtime.run("wait-execution"); expect(attempts).toBe(2); expect(await runtime.state.get("pending")).toMatchObject({ @@ -124,7 +133,7 @@ describe("durable execution and replay", () => { attempts += 1; if (attempts === 1) { throw new ResourceOperationPendingError("delete is not ready", { - retryAfterMs: 1, + retryAfterMs: RETRY_AFTER_MS, }); } }, @@ -140,7 +149,7 @@ describe("durable execution and replay", () => { expect(attempts).toBe(1); expect(await runtime.state.get("pending-delete")).toBeDefined(); - await sleep(5); + await sleep(PAST_RETRY_MS); await runtime.destroy("destroy-wait"); expect(attempts).toBe(2); expect(await runtime.state.get("pending-delete")).toBeUndefined(); @@ -160,7 +169,7 @@ describe("durable execution and replay", () => { if (reads === 1) { throw new ResourceOperationPendingError( "resource is not visible yet", - { retryAfterMs: 1 }, + { retryAfterMs: RETRY_AFTER_MS }, ); } return {} as const; @@ -177,7 +186,7 @@ describe("durable execution and replay", () => { expect(reads).toBe(1); expect(await runtime.state.get("eventually-readable")).toBeUndefined(); - await sleep(5); + await sleep(PAST_RETRY_MS); await runtime.run("post-write-read-execution"); expect(reads).toBe(2); expect(await runtime.state.get("eventually-readable")).toMatchObject({ From 1fb03c4d4826112dd92a8d548bc71c4d26e52ead Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:40:30 +0100 Subject: [PATCH 09/17] Release the deployment hold only on success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../reconciler/src/durable/coordination.ts | 73 +++++++++++++++++-- packages/reconciler/src/durable/index.ts | 4 + .../test/durable-reconciliation.test.ts | 45 ++++++++++++ 3 files changed, 116 insertions(+), 6 deletions(-) diff --git a/packages/reconciler/src/durable/coordination.ts b/packages/reconciler/src/durable/coordination.ts index 5c7af65..094e6f3 100644 --- a/packages/reconciler/src/durable/coordination.ts +++ b/packages/reconciler/src/durable/coordination.ts @@ -1,7 +1,7 @@ import type { ReconcilerEventEmitter } from "../events"; import { durableEmitter, scopeStep } from "./step"; import { deploymentCoordinationStore, type CoordinationState } from "./stores"; -import type { DurableStep, WorkflowStore } from "./yieldstar"; +import type { DurableStep, StoreClient, WorkflowStore } from "./yieldstar"; type CoordinationOptions = { deploymentId: string; @@ -54,16 +54,77 @@ function releaseDeploymentCoordination( }); } -/** Runs `body` while holding the deployment, releasing it even on error. */ +/** + * Runs `body` while holding the deployment, releasing the hold only once + * `body` has completed. A failed or suspended execution keeps the hold, which + * is what makes it safe to resume: the resumed execution replays `take` from + * the step cache and so never re-acquires anything, so a hold released on the + * way out would leave the resumption mutating a deployment it does not hold. + * + * The cost is that an execution which will never be resumed holds its + * deployment indefinitely. That is deliberate — nothing here can tell "will + * retry" from "abandoned" — and it is resolved by an operator calling + * `takeOverDeploymentHold`. + */ export async function* withDeploymentHold( step: DurableStep, opts: CoordinationOptions, body: () => AsyncGenerator, ): AsyncGenerator { const coordination = yield* acquireDeploymentCoordination(step, opts); - try { - yield* body(); - } finally { - yield* releaseDeploymentCoordination(coordination, opts.executionId); + yield* body(); + yield* releaseDeploymentCoordination(coordination, opts.executionId); +} + +export type DeploymentHoldTakeover = + | { taken: true; previousHolder: string } + | { taken: false; holder: string | null }; + +/** + * Clears a deployment hold left by an execution that will not be resumed, so + * that later deployments are not blocked behind it. Named separately from the + * workflow path because it is the only supported way out of that state: the + * hold is otherwise released solely by an execution completing. + * + * The write is conditional on `fromExecutionId` still being the named holder, + * so it cannot clear a hold that has since been released and re-taken by + * another execution. Confirm the holder is genuinely dead first: it may still + * be mid-flight, and taking its hold away permits a concurrent mutation of + * the same deployment. + * + * Throws if the deployment has no coordination store, i.e. if it has never + * been deployed. + */ +export async function takeOverDeploymentHold(params: { + storeClient: StoreClient; + deploymentId: string; + fromExecutionId: string; + toExecutionId?: string | null; +}): Promise { + const { storeClient, deploymentId, fromExecutionId } = params; + const read = () => + storeClient.getStore({ + definition: deploymentCoordinationStore, + id: deploymentId, + }); + + const snapshot = await read(); + if (snapshot.state.holder !== fromExecutionId) { + return { taken: false, holder: snapshot.state.holder }; + } + + const result = await storeClient.updateStoreFrom({ + definition: deploymentCoordinationStore, + id: deploymentId, + snapshot, + updater: (draft) => { + draft.holder = params.toExecutionId ?? null; + }, + }); + + if (!result.updated) { + return { taken: false, holder: (await read()).state.holder }; } + + return { taken: true, previousHolder: fromExecutionId }; } diff --git a/packages/reconciler/src/durable/index.ts b/packages/reconciler/src/durable/index.ts index c896eb8..74c9a22 100644 --- a/packages/reconciler/src/durable/index.ts +++ b/packages/reconciler/src/durable/index.ts @@ -1,5 +1,9 @@ export { deploy } from "./deploy"; export { destroy } from "./destroy"; +export { + takeOverDeploymentHold, + type DeploymentHoldTakeover, +} from "./coordination"; export { DurableStateBackend } from "./state-backend"; export { deploymentCoordinationStore, diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index 4cf80cd..23e65b8 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -364,6 +364,51 @@ describe("deployment coordination", () => { runtime.close(); }); + it("still holds the deployment when a failed execution is resumed", async () => { + // The failure has to live in plain generator code. A step that fails + // caches a StepError, and a cached StepError is rethrown on replay before + // the step's function is reached, so no later work would ever run + // uncached. decideAction calls toComparable outside any step, after the + // resource's reads have been checkpointed. + let failBeforeSecond = true; + const holders: Array = []; + const Resource = resource({ type: "test/durable/hold-replay" }) + .defineSchema({}) + .defineOperations({ + create: async () => { + const snapshot = await runtime.storeClient.getStore({ + definition: durable.deploymentCoordinationStore, + id: "hold-replay", + }); + holders.push(snapshot.state.holder); + }, + delete: async () => undefined, + }); + + const first = new Resource({ id: "first" }); + const second = new Resource({ id: "second" }); + const toComparable = second.toComparable.bind(second); + second.toComparable = (output) => { + if (failBeforeSecond) throw new Error("simulated mid-deployment failure"); + return toComparable(output); + }; + const runtime = createRuntime([first, second], "hold-replay"); + + await expect(runtime.run("replayed-execution")).rejects.toThrow( + "simulated mid-deployment failure", + ); + expect(holders).toEqual(["replayed-execution"]); + + failBeforeSecond = false; + await runtime.run("replayed-execution"); + + // The second resource's create is the first uncached work after the + // failure, so it observes whichever hold the resumed execution is running + // under. Releasing on the way out of a failure would leave it null here. + expect(holders).toEqual(["replayed-execution", "replayed-execution"]); + runtime.close(); + }); + it("emits a coordination waiting event when another execution holds the deployment", async () => { let unblockCreate!: () => void; const blocked = new Promise((resolve) => { From 355c3634d8cd885b7e308ed904c25abf2f48479e Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:41:19 +0100 Subject: [PATCH 10/17] Delete the unused per-operation workflow exports 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/. --- packages/reconciler/src/index.ts | 1 - packages/reconciler/src/operations/operation.create.ts | 10 ---------- packages/reconciler/src/operations/operation.delete.ts | 10 ---------- packages/reconciler/src/operations/operation.read.ts | 10 ---------- packages/reconciler/src/operations/operation.update.ts | 10 ---------- 5 files changed, 41 deletions(-) diff --git a/packages/reconciler/src/index.ts b/packages/reconciler/src/index.ts index b4e749a..cdc37c8 100644 --- a/packages/reconciler/src/index.ts +++ b/packages/reconciler/src/index.ts @@ -1,7 +1,6 @@ export type ResourceApi = typeof import("@notation/resource"); export type StateApi = typeof import("@notation/state"); export type DeepObjectDiffApi = typeof import("deep-object-diff"); -export type YieldstarApi = typeof import("yieldstar"); export * from "./resource-registry"; export * from "./operations"; diff --git a/packages/reconciler/src/operations/operation.create.ts b/packages/reconciler/src/operations/operation.create.ts index 5cecb14..2a1c8d1 100644 --- a/packages/reconciler/src/operations/operation.create.ts +++ b/packages/reconciler/src/operations/operation.create.ts @@ -1,4 +1,3 @@ -import { createWorkflow } from "yieldstar"; import { type CreateResourceParams, type StepRunner, @@ -69,12 +68,3 @@ export async function* createResourceOperation( throw err; } } - -export const createResourceWorkflow: unknown = createWorkflow( - async function* (step, event) { - return yield* createResourceOperation( - step as StepRunner, - event.params as CreateResourceParams, - ); - }, -); diff --git a/packages/reconciler/src/operations/operation.delete.ts b/packages/reconciler/src/operations/operation.delete.ts index 170927f..3bb4e8b 100644 --- a/packages/reconciler/src/operations/operation.delete.ts +++ b/packages/reconciler/src/operations/operation.delete.ts @@ -1,4 +1,3 @@ -import { createWorkflow } from "yieldstar"; import { type DeleteResourceParams, type StepRunner, @@ -39,12 +38,3 @@ export async function* deleteResourceOperation( throw err; } } - -export const deleteResourceWorkflow: unknown = createWorkflow( - async function* (step, event) { - return yield* deleteResourceOperation( - step as StepRunner, - event.params as DeleteResourceParams, - ); - }, -); diff --git a/packages/reconciler/src/operations/operation.read.ts b/packages/reconciler/src/operations/operation.read.ts index 327e405..306af17 100644 --- a/packages/reconciler/src/operations/operation.read.ts +++ b/packages/reconciler/src/operations/operation.read.ts @@ -1,5 +1,4 @@ import { ResourceNotFoundError } from "@notation/resource"; -import { createWorkflow } from "yieldstar"; import type { DriftRead } from "../plan"; import { type ReadResourceParams, @@ -77,12 +76,3 @@ export async function* readDriftOperation( throw error; } } - -export const readResourceWorkflow: unknown = createWorkflow( - async function* (step, event) { - return yield* readResourceOperation( - step as StepRunner, - event.params as ReadResourceParams, - ); - }, -); diff --git a/packages/reconciler/src/operations/operation.update.ts b/packages/reconciler/src/operations/operation.update.ts index 5f07609..84698ee 100644 --- a/packages/reconciler/src/operations/operation.update.ts +++ b/packages/reconciler/src/operations/operation.update.ts @@ -1,4 +1,3 @@ -import { createWorkflow } from "yieldstar"; import { type StepRunner, type UpdateResourceParams, @@ -81,12 +80,3 @@ export async function* updateResourceOperation( throw err; } } - -export const updateResourceWorkflow: unknown = createWorkflow( - async function* (step, event) { - return yield* updateResourceOperation( - step as StepRunner, - event.params as UpdateResourceParams, - ); - }, -); From 2e84f7029429efdd8211cb46532a0900666b12f4 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:42:04 +0100 Subject: [PATCH 11/17] Declare groupId and groupType in the durable state schema 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. --- packages/reconciler/src/durable/stores.ts | 11 +++++++++++ packages/reconciler/src/operations/operation.types.ts | 7 ++++++- .../reconciler/test/durable-reconciliation.test.ts | 2 ++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/reconciler/src/durable/stores.ts b/packages/reconciler/src/durable/stores.ts index aca56fd..c5f03b5 100644 --- a/packages/reconciler/src/durable/stores.ts +++ b/packages/reconciler/src/durable/stores.ts @@ -2,11 +2,22 @@ import type { StateNode } from "@notation/state"; import * as v from "valibot"; import { defineStore, type StoreSnapshot } from "./yieldstar"; +/** + * Loose on purpose: PersistedResourceState carries an index signature so a + * driver can persist fields this schema does not name yet, and v.object would + * strip those at the store boundary rather than carry them forward. Every + * field an operation actually writes is still declared, so the schema is a + * floor on the record rather than a description of one. + */ export const resourceStateStore = defineStore( "resource-state", v.looseObject({ id: v.string(), type: v.string(), + // Written by create and update. -1 and "" are BaseResource's defaults for + // a resource that belongs to no group. + groupId: v.number(), + groupType: v.string(), config: v.record(v.string(), v.unknown()), params: v.record(v.string(), v.unknown()), output: v.record(v.string(), v.unknown()), diff --git a/packages/reconciler/src/operations/operation.types.ts b/packages/reconciler/src/operations/operation.types.ts index 5d9d640..ad11676 100644 --- a/packages/reconciler/src/operations/operation.types.ts +++ b/packages/reconciler/src/operations/operation.types.ts @@ -39,7 +39,12 @@ export type PersistedResourceState = Pick< | "output" | "lastOperation" | "lastOperationAt" -> & { [key: string]: unknown }; +> & { + // Not on StateNode itself, where they arrive through its index signature. + groupId: number; + groupType: string; + [key: string]: unknown; +}; /** * How a driver writes state. Both are steps so that each driver can carry its diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index 23e65b8..595ab82 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -690,6 +690,8 @@ function statePatch(id: string) { return { id, type: "test/durable/state", + groupId: -1, + groupType: "", config: {}, params: {}, output: {}, From 7ab8201453a4688f4f8b146e5088448af5dc7f9d Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:49:47 +0100 Subject: [PATCH 12/17] Freeze an operation's inputs before it starts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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` 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. --- .../test/provisioner/operation.create.test.ts | 6 +- packages/reconciler/package.json | 1 + packages/reconciler/src/durable/operations.ts | 26 ++++--- .../src/operations/operation.create.ts | 7 +- .../src/operations/operation.read.ts | 11 +-- .../src/operations/operation.types.ts | 20 +++-- .../src/operations/operation.update.ts | 7 +- packages/reconciler/src/planner.ts | 3 +- packages/reconciler/src/reconciler.ts | 73 ++++++++++++++----- .../test/durable-reconciliation.test.ts | 22 +++--- .../test/operation.workflows.test.ts | 67 ++++++----------- .../reconciler/test/reconciler.deploy.test.ts | 22 +++--- .../reconciler/test/reconciler.plan.test.ts | 40 +++++----- .../reconciler/test/resource-registry.test.ts | 4 +- 14 files changed, 162 insertions(+), 147 deletions(-) diff --git a/packages/core/test/provisioner/operation.create.test.ts b/packages/core/test/provisioner/operation.create.test.ts index 2e4f506..84ed0e3 100644 --- a/packages/core/test/provisioner/operation.create.test.ts +++ b/packages/core/test/provisioner/operation.create.test.ts @@ -34,8 +34,10 @@ describe("resource creation", () => { await runOperation( createResourceOperation(step, { resource: testResource, - state: stateBackend, - expectedRev: 0, + resourceParams: await testResource.getParams(), + persist: async function* (next) { + await stateBackend.update(testResource.id, 0, next); + }, }), ); diff --git a/packages/reconciler/package.json b/packages/reconciler/package.json index 58b0ab0..c36012c 100644 --- a/packages/reconciler/package.json +++ b/packages/reconciler/package.json @@ -19,6 +19,7 @@ ], "scripts": { "build": "tsup --clean", + "typecheck": "tsc --noEmit", "dev": "tsup --watch" }, "dependencies": { diff --git a/packages/reconciler/src/durable/operations.ts b/packages/reconciler/src/durable/operations.ts index e439df1..fca0036 100644 --- a/packages/reconciler/src/durable/operations.ts +++ b/packages/reconciler/src/durable/operations.ts @@ -34,10 +34,17 @@ export async function* reconcileResource( const emit = durableEmitter(scope, opts.emit); const { stateNode, snapshot } = yield* hydrateResource(scope, resource, opts); - const shared = operationParams(scope, resource, opts, stateNode); - // Decide the operation from desired params vs persisted state. + // Decide the operation from desired params vs persisted state. The params + // are resolved once and then carried: deriveParams is user code and need + // not be deterministic, so an operation resolving them again could persist + // params other than the ones the decision was taken against. const params = yield* scope.run("params", () => resource.getParams()); + const shared = { + ...operationParams(scope, resource, opts), + resourceParams: params, + persistedOutput: stateNode?.output, + }; let action: ResourceAction = decideAction({ resource, stateNode, params }); // A noop is only trusted after the remote is read back: the provider may @@ -46,10 +53,11 @@ export async function* reconcileResource( // Its own scope: the operation that follows reads the remote again, and // the two reads must not share step keys. const driftScope = scopeStep(scope, "drift-read"); - const driftRead = yield* readDriftOperation( - driftScope, - operationParams(driftScope, resource, opts, stateNode), - ); + const driftRead = yield* readDriftOperation(driftScope, { + ...operationParams(driftScope, resource, opts), + resourceParams: params, + persistedOutput: stateNode?.output, + }); action = decideAction({ resource, stateNode, params, driftRead }); } @@ -99,7 +107,7 @@ export async function* deleteResource( resource.setOutput(stateNode.output); yield* deleteResourceOperation(step, { - ...operationParams(step, resource, opts, stateNode), + ...operationParams(step, resource, opts), remove: removeResourceState(step, opts, resource, snapshot), }); } @@ -179,13 +187,9 @@ function operationParams( step: DurableStep, resource: BaseResource, opts: DurableOperationOptions, - stateNode: StateNode | undefined, ): ResourceOperationBaseParams { return { resource, - // Serve the record already read during hydration rather than reading it - // again; a workflow must see the same value on every replay anyway. - state: { get: async () => stateNode }, dryRun: opts.dryRun, emit: durableEmitter(step, opts.emit), maxOperationAttempts: opts.maxOperationAttempts, diff --git a/packages/reconciler/src/operations/operation.create.ts b/packages/reconciler/src/operations/operation.create.ts index 2a1c8d1..d76d06f 100644 --- a/packages/reconciler/src/operations/operation.create.ts +++ b/packages/reconciler/src/operations/operation.create.ts @@ -19,9 +19,7 @@ export async function* createResourceOperation( } try { - const resourceParams = yield* step.run("create:get-params", () => - params.resource.getParams(), - ); + const resourceParams = params.resourceParams; const computedPrimaryKey = yield* runPendingOperation( step, @@ -40,7 +38,8 @@ export async function* createResourceOperation( const readResult = yield* readResourceOperation(step, { resource: params.resource, - state: params.state, + resourceParams, + persistedOutput: params.persistedOutput, emit: params.emit, maxOperationAttempts: params.maxOperationAttempts, }); diff --git a/packages/reconciler/src/operations/operation.read.ts b/packages/reconciler/src/operations/operation.read.ts index 306af17..76e02da 100644 --- a/packages/reconciler/src/operations/operation.read.ts +++ b/packages/reconciler/src/operations/operation.read.ts @@ -20,16 +20,11 @@ export async function* readResourceOperation( } try { - const resourceParams = yield* step.run("read:get-params", () => - params.resource.getParams(), - ); + const resourceParams = params.resourceParams; if (!params.resource.read) { - const stateNode = yield* step.run("read:get-state-node", () => - params.state.get(params.resource.id), - ); - const merged = stateNode - ? { ...stateNode.output, ...resourceParams } + const merged = params.persistedOutput + ? { ...params.persistedOutput, ...resourceParams } : resourceParams; yield* emitLifecycleEvent(params, "read", "skip", { diff --git a/packages/reconciler/src/operations/operation.types.ts b/packages/reconciler/src/operations/operation.types.ts index ad11676..1c247dc 100644 --- a/packages/reconciler/src/operations/operation.types.ts +++ b/packages/reconciler/src/operations/operation.types.ts @@ -1,5 +1,5 @@ import type { BaseResource } from "@notation/resource"; -import type { State, StateNode } from "@notation/state"; +import type { StateNode } from "@notation/state"; import type { EmitStep, OperationLifecycleEvent, @@ -60,19 +60,27 @@ export type RemoveState = () => AsyncGenerator; export type ResourceOperationBaseParams = { resource: BaseResource; - state: Pick; dryRun?: boolean; emit?: OperationEventEmitter; maxOperationAttempts?: number; }; -export type CreateResourceParams = ResourceOperationBaseParams & { - persist: PersistState; +/** + * Everything a read needs is resolved before the operation starts: the + * desired params, and — for a resource with no read operation — the output + * the last write persisted. An operation that resolved either itself could + * see a different answer from the one the decision was taken against. + */ +export type ReadResourceParams = ResourceOperationBaseParams & { + resourceParams: Record; + persistedOutput?: Record; }; -export type ReadResourceParams = ResourceOperationBaseParams; +export type CreateResourceParams = ReadResourceParams & { + persist: PersistState; +}; -export type UpdateResourceParams = ResourceOperationBaseParams & { +export type UpdateResourceParams = ReadResourceParams & { patch: Record; persist: PersistState; }; diff --git a/packages/reconciler/src/operations/operation.update.ts b/packages/reconciler/src/operations/operation.update.ts index 84698ee..838161e 100644 --- a/packages/reconciler/src/operations/operation.update.ts +++ b/packages/reconciler/src/operations/operation.update.ts @@ -27,9 +27,7 @@ export async function* updateResourceOperation( } try { - const resourceParams = yield* step.run("update:get-params", () => - params.resource.getParams(), - ); + const resourceParams = params.resourceParams; yield* runPendingOperation( step, @@ -52,7 +50,8 @@ export async function* updateResourceOperation( const readResult = yield* readResourceOperation(step, { resource: params.resource, - state: params.state, + resourceParams, + persistedOutput: params.persistedOutput, emit: params.emit, maxOperationAttempts: params.maxOperationAttempts, }); diff --git a/packages/reconciler/src/planner.ts b/packages/reconciler/src/planner.ts index ef7faca..8ac8b27 100644 --- a/packages/reconciler/src/planner.ts +++ b/packages/reconciler/src/planner.ts @@ -47,7 +47,8 @@ export async function createPlan({ const driftRead = await runOperation( readDriftOperation(createStepRunner(), { resource, - state, + resourceParams: params, + persistedOutput: stateNode?.output, emit: emitStep, maxOperationAttempts, }), diff --git a/packages/reconciler/src/reconciler.ts b/packages/reconciler/src/reconciler.ts index 8db798b..634bf73 100644 --- a/packages/reconciler/src/reconciler.ts +++ b/packages/reconciler/src/reconciler.ts @@ -163,11 +163,23 @@ export class Reconciler { dryRun: boolean, driftDetection: boolean, ) { - await this.#withMutationLease(resource.id, () => - this.#retryOnRevConflict((conflict) => - this.#deployResourceOnce(resource, dryRun, driftDetection, conflict), - ), - ); + await this.#withMutationLease(resource.id, async () => { + // Resolved once for the whole scheduled reconciliation, recovery + // included: deriveParams is user code and need not be deterministic, so + // a second resolution could decide against one set of params and + // persist another, with nothing to reconcile the two afterwards. + const params = (await resource.getParams()) as Record; + + await this.#retryOnRevConflict((conflict) => + this.#deployResourceOnce( + resource, + params, + dryRun, + driftDetection, + conflict, + ), + ); + }); } async #withMutationLease(resourceId: string, fn: () => Promise) { @@ -224,12 +236,13 @@ export class Reconciler { async #deployResourceOnce( resource: BaseResource, + params: Record, dryRun: boolean, driftDetection: boolean, conflict?: RevConflict, ) { if (conflict) { - await this.#recoverDeployResource(resource, dryRun, conflict); + await this.#recoverDeployResource(resource, params, dryRun, conflict); return; } @@ -237,14 +250,17 @@ export class Reconciler { let action: ResourceAction; if (!stateNode) { - action = decideAction({ resource }); + action = decideAction({ resource, params }); } else { resource.setOutput(stateNode.output); - const params = await resource.getParams(); action = decideAction({ resource, stateNode, params }); if (action.decision === "noop" && driftDetection) { - const driftRead = await this.#readForDrift(resource); + const driftRead = await this.#readForDrift( + resource, + params, + stateNode.output, + ); action = decideAction({ resource, stateNode, params, driftRead }); } } @@ -273,7 +289,8 @@ export class Reconciler { await runOperation( createResourceOperation(this.#stepRunner, { resource, - state: this.#state, + resourceParams: params, + persistedOutput: stateNode?.output, dryRun, emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, @@ -287,7 +304,8 @@ export class Reconciler { await runOperation( updateResourceOperation(this.#stepRunner, { resource, - state: this.#state, + resourceParams: params, + persistedOutput: stateNode?.output, patch: action.patch, dryRun, emit: this.#emitStep, @@ -303,6 +321,7 @@ export class Reconciler { async #recoverDeployResource( resource: BaseResource, + params: Record, dryRun: boolean, conflict: RevConflict, ) { @@ -311,8 +330,11 @@ export class Reconciler { const stateNode = await this.#state.get(resource.id); if (stateNode) resource.setOutput(stateNode.output); - const params = await resource.getParams(); - const remote = await this.#readForDrift(resource); + const remote = await this.#readForDrift( + resource, + params, + stateNode?.output, + ); const action = decideAction({ resource, stateNode, @@ -335,7 +357,8 @@ export class Reconciler { await runOperation( createResourceOperation(this.#stepRunner, { resource, - state: this.#state, + resourceParams: params, + persistedOutput: stateNode?.output, dryRun, emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, @@ -348,7 +371,8 @@ export class Reconciler { await runOperation( updateResourceOperation(this.#stepRunner, { resource, - state: this.#state, + resourceParams: params, + persistedOutput: stateNode?.output, patch: action.patch, dryRun, emit: this.#emitStep, @@ -390,11 +414,16 @@ export class Reconciler { }; } - #readForDrift(resource: BaseResource): Promise { + #readForDrift( + resource: BaseResource, + params: Record, + persistedOutput: Record | undefined, + ): Promise { return runOperation( readDriftOperation(this.#stepRunner, { resource, - state: this.#state, + resourceParams: params, + persistedOutput, emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, }), @@ -474,7 +503,14 @@ export class Reconciler { if (conflict) { if (!resource.read) throw conflict; - const remote = await this.#readForDrift(resource); + // Deletion never needs the desired params, so they are resolved here + // rather than for every delete: only the recovery read consumes them. + const params = (await resource.getParams()) as Record; + const remote = await this.#readForDrift( + resource, + params, + stateNode.output, + ); if (remote.kind !== "present") { if (!dryRun) await this.#state.delete(resource.id, stateNode.rev); return; @@ -485,7 +521,6 @@ export class Reconciler { await runOperation( deleteResourceOperation(this.#stepRunner, { resource, - state: this.#state, dryRun, emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index 595ab82..7b87cca 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -253,13 +253,11 @@ describe("dependency ordering", () => { describe("conditional state persistence", () => { it("rejects a state write whose snapshot another writer has moved past", async () => { const RaceResource = resource({ type: "test/durable/write-race" }) + // Cast: a resource declared without API types constrains every schema + // key to be a key of an `any` API schema, which no named key satisfies. .defineSchema({ - name: { - presence: "required", - propertyType: "param", - valueType: "string" as any, - }, - }) + name: { presence: "required", propertyType: "param" }, + } as any) .defineOperations({ create: async () => undefined, // Moves the store on between the workflow reading its snapshot and @@ -509,15 +507,13 @@ describe("drift detection and repair", () => { remote = { name: "expected" }; }); const DriftResource = resource({ type: "test/durable/drift" }) + // Cast: a resource declared without API types constrains every schema + // key to be a key of an `any` API schema, which no named key satisfies. .defineSchema({ - name: { - presence: "required", - propertyType: "param", - valueType: "string" as any, - }, - }) + name: { presence: "required", propertyType: "param" }, + } as any) .defineOperations({ - create: async () => remote, + create: (async () => remote) as any, read: async () => remote, update: updateSpy, delete: async () => undefined, diff --git a/packages/reconciler/test/operation.workflows.test.ts b/packages/reconciler/test/operation.workflows.test.ts index a721a29..245cb00 100644 --- a/packages/reconciler/test/operation.workflows.test.ts +++ b/packages/reconciler/test/operation.workflows.test.ts @@ -52,11 +52,10 @@ describe("operation workflows", () => { it("create performs create + read-after-create + state persistence", async () => { const step = createStepRunnerDouble(); const events: OperationLifecycleEvent[] = []; - const state = { get: vi.fn(async () => undefined) }; const persist = vi.fn(async function* () {}); let createAttempts = 0; - const createMock = vi.fn(async (_params, context) => { + const createMock = vi.fn(async (_params: unknown, context: unknown) => { createAttempts += 1; if (createAttempts === 1) { expect(context).toBeUndefined(); @@ -73,7 +72,8 @@ describe("operation workflows", () => { const TestResource = resource({ type: "test/service/create" }) .defineSchema({}) .defineOperations({ - create: createMock, + // Cast: an empty schema declares no primary key to return. + create: createMock as any, read: async () => ({ remoteId: "abc", status: "ready" }), delete: async () => undefined, }); @@ -83,7 +83,7 @@ describe("operation workflows", () => { await runOperation( createResourceOperation(step, { resource: testResource, - state, + resourceParams: await testResource.getParams(), persist, emit: toEmitStep((event) => void events.push(event)), }), @@ -126,17 +126,12 @@ describe("operation workflows", () => { it("read follows pending retry instructions", async () => { const step = createStepRunnerDouble(); - const state = { - get: vi.fn(async () => undefined), - update: vi.fn(async () => undefined), - delete: vi.fn(async () => undefined), - }; let readAttempts = 0; const TestResource = resource({ type: "test/service/read" }) .defineSchema({}) .defineOperations({ - create: async () => ({}), + create: (async () => ({})) as any, read: async (_key, context) => { readAttempts += 1; if (readAttempts < 3) { @@ -156,7 +151,7 @@ describe("operation workflows", () => { const result = await runOperation( readResourceOperation(step, { resource: testResource, - state, + resourceParams: await testResource.getParams(), }), ); @@ -176,11 +171,6 @@ describe("operation workflows", () => { it("fails when an operation remains pending past the safety limit", async () => { const step = createStepRunnerDouble(); - const state = { - get: vi.fn(async () => undefined), - update: vi.fn(async () => undefined), - delete: vi.fn(async () => undefined), - }; const read = vi.fn(async () => { throw new ResourceOperationPendingError("still pending", { retryAfterMs: 10, @@ -189,7 +179,7 @@ describe("operation workflows", () => { const TestResource = resource({ type: "test/service/pending-limit" }) .defineSchema({}) .defineOperations({ - create: async () => ({}), + create: (async () => ({})) as any, read, delete: async () => undefined, }); @@ -198,7 +188,7 @@ describe("operation workflows", () => { runOperation( readResourceOperation(step, { resource: new TestResource({ id: "pending-limit" }), - state, + resourceParams: {}, maxOperationAttempts: 2, }), ), @@ -209,15 +199,11 @@ describe("operation workflows", () => { it("does not infer that not-found after creation is retryable", async () => { const step = createStepRunnerDouble(); - const state = { - get: vi.fn(async () => undefined), - update: vi.fn(async () => undefined), - delete: vi.fn(async () => undefined), - }; + const persist = vi.fn(async function* () {}); const TestResource = resource({ type: "test/service/eventually-visible" }) .defineSchema({}) .defineOperations({ - create: async () => ({}), + create: (async () => ({})) as any, read: async () => { throw new ResourceNotFoundError("resource is absent"); }, @@ -228,24 +214,23 @@ describe("operation workflows", () => { runOperation( createResourceOperation(step, { resource: new TestResource({ id: "eventually-visible" }), - state, - expectedRev: 0, + resourceParams: {}, + persist, }), ), ).rejects.toThrowError("resource is absent"); - expect(state.update).not.toHaveBeenCalled(); + expect(persist).not.toHaveBeenCalled(); }); it("delete treats an already-absent remote as success through its idempotent resource contract", async () => { const step = createStepRunnerDouble(); const events: OperationLifecycleEvent[] = []; - const state = { get: vi.fn(async () => undefined) }; const remove = vi.fn(async function* () {}); const TestResource = resource({ type: "test/service/delete" }) .defineSchema({}) .defineOperations({ - create: async () => ({}), + create: (async () => ({})) as any, delete: async () => undefined, }); @@ -254,7 +239,6 @@ describe("operation workflows", () => { await runOperation( deleteResourceOperation(step, { resource: testResource, - state, remove, emit: toEmitStep((event) => void events.push(event)), }), @@ -268,16 +252,12 @@ describe("operation workflows", () => { it("delete rethrows an unclassified resource error", async () => { const step = createStepRunnerDouble(); - const state = { - get: vi.fn(async () => undefined), - update: vi.fn(async () => undefined), - delete: vi.fn(async () => undefined), - }; + const remove = vi.fn(async function* () {}); const TestResource = resource({ type: "test/service/delete-miss" }) .defineSchema({}) .defineOperations({ - create: async () => ({}), + create: (async () => ({})) as any, delete: async () => { const err = new Error("still exists"); err.name = "DifferentError"; @@ -291,8 +271,7 @@ describe("operation workflows", () => { runOperation( deleteResourceOperation(step, { resource: testResource, - state, - expectedRev: 1, + remove, }), ), ).rejects.toMatchObject({ @@ -300,17 +279,13 @@ describe("operation workflows", () => { message: "still exists", }); - expect(state.delete).not.toHaveBeenCalled(); + expect(remove).not.toHaveBeenCalled(); }); it("emits structured error details on operation failure", async () => { const step = createStepRunnerDouble(); const events: OperationLifecycleEvent[] = []; - const state = { - get: vi.fn(async () => undefined), - update: vi.fn(async () => undefined), - delete: vi.fn(async () => undefined), - }; + const persist = vi.fn(async function* () {}); const TestResource = resource({ type: "test/service/create-error" }) .defineSchema({}) @@ -329,8 +304,8 @@ describe("operation workflows", () => { runOperation( createResourceOperation(step, { resource: testResource, - state, - expectedRev: 0, + resourceParams: {}, + persist, emit: toEmitStep((event) => void events.push(event)), }), ), diff --git a/packages/reconciler/test/reconciler.deploy.test.ts b/packages/reconciler/test/reconciler.deploy.test.ts index eaa5ce5..61d0209 100644 --- a/packages/reconciler/test/reconciler.deploy.test.ts +++ b/packages/reconciler/test/reconciler.deploy.test.ts @@ -72,15 +72,15 @@ function createTestResourceClass(opts: { ) => Promise; }) { return resource({ type: opts.type }) + // Cast: a resource declared without API types constrains every schema key + // to be a key of an `any` API schema, which no named key can satisfy. .defineSchema({ - name: { - presence: "required", - propertyType: "param", - valueType: "string" as any, - }, - }) + name: { presence: "required", propertyType: "param" }, + } as any) .defineOperations({ - create: opts.create ?? (async () => ({})), + // Cast: with no API types the schema resolves to no primary key, so the + // inferred create signature returns void. + create: (opts.create ?? (async () => ({}))) as any, read: opts.read, update: opts.update, delete: opts.delete ?? (async () => undefined), @@ -93,7 +93,9 @@ const found = (output: Record) => output; describe("reconciler deploy", () => { it("chooses create vs update from desired params vs state", async () => { const createSpy = vi.fn(async () => ({ name: "new" })); - const updateSpy = vi.fn(async () => undefined); + const updateSpy = vi.fn( + async (_key: unknown, _patch: Record) => undefined, + ); const CreateResource = createTestResourceClass({ type: "test/service/create-choice", @@ -397,7 +399,9 @@ describe("reconciler deploy", () => { }); it("detects drift using live read output and converges with update", async () => { - const updateSpy = vi.fn(async () => undefined); + const updateSpy = vi.fn( + async (_key: unknown, _patch: Record) => undefined, + ); const events: Array> = []; const TestResource = createTestResourceClass({ type: "test/service/drift", diff --git a/packages/reconciler/test/reconciler.plan.test.ts b/packages/reconciler/test/reconciler.plan.test.ts index d98e55a..3ea8752 100644 --- a/packages/reconciler/test/reconciler.plan.test.ts +++ b/packages/reconciler/test/reconciler.plan.test.ts @@ -13,16 +13,19 @@ function createMemoryState(initial: Record = {}) { return { store, - get: vi.fn(async (id: string) => store[id]), + get: vi.fn(async (id: string): Promise => store[id]), update: vi.fn( - async (id: string, expectedRev: number, patch: Partial) => { + async (id: string, _expectedRev: number, patch: Partial) => { + const rev = (store[id]?.rev ?? 0) + 1; store[id] = { ...(store[id] ?? {}), ...patch, + rev, } as StateNode; + return { rev }; }, ), - delete: vi.fn(async (id: string) => { + delete: vi.fn(async (id: string, _expectedRev: number) => { delete store[id]; }), values: vi.fn(async () => Object.values(store)), @@ -55,20 +58,16 @@ function createTestResourceClass(opts: { ) => Promise; }) { return resource({ type: opts.type }) + // Cast: a resource declared without API types constrains every schema key + // to be a key of an `any` API schema, which no named key can satisfy. .defineSchema({ - name: { - presence: "required", - propertyType: "param", - valueType: "string" as any, - }, - tag: { - presence: "optional", - propertyType: "param", - valueType: "string" as any, - }, - }) + name: { presence: "required", propertyType: "param" }, + tag: { presence: "optional", propertyType: "param" }, + } as any) .defineOperations({ - create: opts.create ?? (async () => ({})), + // Cast: with no API types the schema resolves to no primary key, so the + // inferred create signature returns void. + create: (opts.create ?? (async () => ({}))) as any, read: opts.read, update: opts.update, delete: opts.delete ?? (async () => undefined), @@ -82,6 +81,7 @@ function createStateNode( output: Record = params, ): StateNode { return { + rev: 1, id, groupId: -1, groupType: "", @@ -369,14 +369,10 @@ describe("reconciler plan", () => { type: "test/service/plan-derive-failure", }) .defineSchema({ - name: { - presence: "required", - propertyType: "param", - valueType: "string" as any, - }, - }) + name: { presence: "required", propertyType: "param" }, + } as any) .defineOperations({ - create: async () => ({}), + create: (async () => ({})) as any, delete: async () => undefined, deriveParams: () => { throw new Error("invalid derived configuration"); diff --git a/packages/reconciler/test/resource-registry.test.ts b/packages/reconciler/test/resource-registry.test.ts index dd63f48..de3fdf8 100644 --- a/packages/reconciler/test/resource-registry.test.ts +++ b/packages/reconciler/test/resource-registry.test.ts @@ -9,14 +9,14 @@ import { const TestResourceA = resource({ type: "test/service/a" }) .defineSchema({}) .defineOperations({ - create: async () => ({}), + create: (async () => ({})) as any, delete: async () => undefined, }); const TestResourceB = resource({ type: "test/service/b" }) .defineSchema({}) .defineOperations({ - create: async () => ({}), + create: (async () => ({})) as any, delete: async () => undefined, }); From f2959144d6f1e61a8334133eb7d3646aebc890c9 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:53:10 +0100 Subject: [PATCH 13/17] Move step scoping onto the step runner seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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:, state:delete: 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. --- packages/reconciler/src/durable/index.ts | 19 +++++ packages/reconciler/src/durable/operations.ts | 27 ++++--- packages/reconciler/src/durable/step.ts | 77 ++++++++----------- .../src/operations/operation.types.ts | 15 +++- packages/reconciler/src/step-runner.ts | 29 ++----- .../test/operation.workflows.test.ts | 30 ++++---- 6 files changed, 103 insertions(+), 94 deletions(-) diff --git a/packages/reconciler/src/durable/index.ts b/packages/reconciler/src/durable/index.ts index 74c9a22..336d548 100644 --- a/packages/reconciler/src/durable/index.ts +++ b/packages/reconciler/src/durable/index.ts @@ -1,3 +1,22 @@ +/** + * Step keys are a public contract: they are what a resumed execution matches + * its cached work against, so changing one re-executes the work behind it. + * The shapes in use are: + * + * notation:resource::* per-resource reconciliation steps + * notation:destroy::* per-resource deletion steps + * notation:orphans::* orphan sweep, per persisted record + * *:remote:attempt: one provider call attempt + * *:remote:retry-delay: the wait between two attempts + * emit:[::] event delivery checkpoint + * notation:coordination:* deployment hold: inspect/acquire/release + * state:persist: conditional write of a resource record + * state:delete: conditional removal of one + * + * The state: keys are store-handle keys and so are not scope-prefixed: a + * store outlives the scope that opened it, which is why they carry the + * resource id themselves. + */ export { deploy } from "./deploy"; export { destroy } from "./destroy"; export { diff --git a/packages/reconciler/src/durable/operations.ts b/packages/reconciler/src/durable/operations.ts index fca0036..2912922 100644 --- a/packages/reconciler/src/durable/operations.ts +++ b/packages/reconciler/src/durable/operations.ts @@ -16,7 +16,7 @@ import { } from "../operations"; import { decideAction, type ResourceAction } from "../plan"; import type { DurableStateBackend } from "./state-backend"; -import { durableEmitter, scopeStep } from "./step"; +import { durableEmitter, scopeStep, type DurableStepRunner } from "./step"; import { resourceStateStore, toStateNode, @@ -52,7 +52,7 @@ export async function* reconcileResource( if (action.decision === "noop" && (opts.driftDetection ?? true)) { // Its own scope: the operation that follows reads the remote again, and // the two reads must not share step keys. - const driftScope = scopeStep(scope, "drift-read"); + const driftScope = scope.scope("drift-read"); const driftRead = yield* readDriftOperation(driftScope, { ...operationParams(driftScope, resource, opts), resourceParams: params, @@ -95,7 +95,7 @@ export async function* reconcileResource( } export async function* deleteResource( - step: DurableStep, + step: DurableStepRunner, resource: BaseResource, opts: DurableOperationOptions, ): AsyncGenerator { @@ -118,7 +118,7 @@ export async function* deleteResource( * warning, because deleting it would need a resource class we cannot resolve. */ export async function* sweepOrphans( - step: DurableStep, + step: DurableStepRunner, opts: DurableOperationOptions, workflow: "deploy" | "destroy", ): AsyncGenerator { @@ -131,7 +131,7 @@ export async function* sweepOrphans( for (const node of persisted) { if (resourceById.has(node.id)) continue; - const nodeScope = scopeStep(step, node.id); + const nodeScope = step.scope(node.id); const Resource = resolveResourceClass(registry, node.type as ResourceType); if (!Resource) { @@ -158,7 +158,7 @@ export async function* sweepOrphans( * here, and is re-served to the operations so they need no second read. */ async function* hydrateResource( - step: DurableStep, + step: DurableStepRunner, resource: BaseResource, opts: DurableOperationOptions, ): AsyncGenerator< @@ -175,7 +175,7 @@ async function* hydrateResource( } function readSnapshot( - step: DurableStep, + step: DurableStepRunner, state: DurableStateBackend, resourceId: string, ): AsyncGenerator { @@ -184,7 +184,7 @@ function readSnapshot( /** The half of the operation params every durable driver call site shares. */ function operationParams( - step: DurableStep, + step: DurableStepRunner, resource: BaseResource, opts: DurableOperationOptions, ): ResourceOperationBaseParams { @@ -203,7 +203,7 @@ function operationParams( * recorded result instead of retrying a compare-and-set that would now fail. */ function persistResourceState( - step: DurableStep, + step: DurableStepRunner, opts: DurableOperationOptions, resource: BaseResource, snapshot: ResourceSnapshot | undefined, @@ -224,7 +224,7 @@ function persistResourceState( id: opts.state.storeId(resource.id), }); const result = yield* store.updateFrom( - "state:persist", + `state:persist:${resource.id}`, snapshot, () => next, ); @@ -239,7 +239,7 @@ function persistResourceState( } function removeResourceState( - step: DurableStep, + step: DurableStepRunner, opts: DurableOperationOptions, resource: BaseResource, snapshot: ResourceSnapshot, @@ -248,7 +248,10 @@ function removeResourceState( const store = yield* step.store(resourceStateStore, { id: opts.state.storeId(resource.id), }); - const result = yield* store.deleteFrom("state:delete", snapshot); + const result = yield* store.deleteFrom( + `state:delete:${resource.id}`, + snapshot, + ); if (!result.deleted) { throw new RevConflict(resource.id, snapshot.version + 1, undefined); } diff --git a/packages/reconciler/src/durable/step.ts b/packages/reconciler/src/durable/step.ts index ad52ade..a3c78ae 100644 --- a/packages/reconciler/src/durable/step.ts +++ b/packages/reconciler/src/durable/step.ts @@ -3,36 +3,49 @@ import type { ReconcilerEvent, ReconcilerEventEmitter, } from "../events"; -import type { DurableStep, WorkflowStore } from "./yieldstar"; +import type { StepRunner } from "../operations"; +import type { DurableStep } from "./yieldstar"; + +/** + * A durable step runner: the operation seam plus the store handle, which only + * the durable driver uses. + */ +export type DurableStepRunner = { + run( + key: string, + fn: () => T | Promise, + ): AsyncGenerator; + delay(key: string, ms: number): AsyncGenerator; + /** Narrower than StepRunner's, so a scope keeps its store handle. */ + scope(prefix: string): DurableStepRunner; + store: DurableStep["store"]; +}; + +// A durable runner is one of the step runners the operations accept. +type AssertStepRunner = DurableStepRunner extends StepRunner ? true : never; +export type DurableStepRunnerIsStepRunner = AssertStepRunner; /** * Namespaces the step keys of `step` so an operation can be written once and * replayed at several call sites without its keys colliding. * * Opening a store is not prefixed: yieldstar derives that key from the store - * name and store id, which is already unique. The keys the store *handle* - * takes are caller-supplied, so those are scoped like any other step. + * name and store id, which is already unique. The keys a store *handle* takes + * are caller-supplied and are left alone too — a store outlives the scope + * that opened it, so its call sites qualify their own keys. */ -export function scopeStep(step: DurableStep, prefix: string): DurableStep { +export function scopeStep( + step: DurableStep, + prefix: string, +): DurableStepRunner { const scoped = (key: string) => `${prefix}:${key}`; return { - ...step, - // The keyless overloads fall through untouched; yieldstar hashes the call - // site for those, and a prefix would not make them any more unique. - run: ((arg1: unknown, arg2?: unknown) => - typeof arg1 === "string" - ? (step.run as any)(scoped(arg1), arg2) - : (step.run as any)(arg1)) as DurableStep["run"], - delay: ((arg1: unknown, arg2?: unknown) => - typeof arg1 === "string" - ? (step.delay as any)(scoped(arg1), arg2) - : (step.delay as any)(arg1)) as DurableStep["delay"], - store: ((definition: any, params: any) => - (async function* () { - const store = yield* step.store(definition, params); - return scopeStore(store, prefix); - })()) as DurableStep["store"], + run: ((key: string, fn: any) => + step.run(scoped(key), fn)) as DurableStepRunner["run"], + delay: (key: string, ms: number) => step.delay(scoped(key), ms), + store: step.store, + scope: (nested: string) => scopeStep(step, scoped(nested)), }; } @@ -46,7 +59,7 @@ export function scopeStep(step: DurableStep, prefix: string): DurableStep { * event is delivered but before the checkpoint is written. */ export function durableEmitter( - step: DurableStep, + step: DurableStepRunner, emit: ReconcilerEventEmitter | undefined, ): EmitStep { return async function* (event) { @@ -60,25 +73,3 @@ function emitKey(event: ReconcilerEvent): string { ? `emit:${event.event}:${event.operation}:${event.status}` : `emit:${event.event}`; } - -function scopeStore( - store: WorkflowStore, - prefix: string, -): WorkflowStore { - const scoped = (key: string) => `${prefix}:${key}`; - - return { - ...store, - get: (key?: string) => store.get(key === undefined ? key : scoped(key)), - select: (key, selector) => store.select(scoped(key), selector), - update: (key, updater) => store.update(scoped(key), updater), - updateFrom: (key, snapshot, updater) => - store.updateFrom(scoped(key), snapshot, updater), - deleteFrom: (key, snapshot) => store.deleteFrom(scoped(key), snapshot), - when: ((arg1: any, arg2?: any) => - typeof arg1 === "string" - ? store.when(scoped(arg1), arg2) - : store.when(arg1)) as WorkflowStore["when"], - take: (key, selector, claim) => store.take(scoped(key), selector, claim), - }; -} diff --git a/packages/reconciler/src/operations/operation.types.ts b/packages/reconciler/src/operations/operation.types.ts index 1c247dc..492f4c9 100644 --- a/packages/reconciler/src/operations/operation.types.ts +++ b/packages/reconciler/src/operations/operation.types.ts @@ -15,14 +15,25 @@ export type { export type OperationEventEmitter = EmitStep; +/** + * How an operation runs a step, and how it namespaces the steps it runs. + * + * Keys are mandatory. A driver may ignore them — the in-process one does — + * but a keyless step would have to derive a key from its call site, which for + * a step reached through `scope` is the scoping wrapper's call site rather + * than the caller's, so any two keyless steps under one scope would collide. + * + * `scope` is the seam that lets one operation run at several call sites in a + * single execution: in process it is the identity, and in a workflow it + * prefixes the keys the runtime caches against. + */ export type StepRunner = { - run(fn: () => T | Promise): AsyncGenerator; run( key: string, fn: () => T | Promise, ): AsyncGenerator; - delay(ms: number): AsyncGenerator; delay(key: string, ms: number): AsyncGenerator; + scope(prefix: string): StepRunner; }; /** diff --git a/packages/reconciler/src/step-runner.ts b/packages/reconciler/src/step-runner.ts index 61cc6d5..ff39fdc 100644 --- a/packages/reconciler/src/step-runner.ts +++ b/packages/reconciler/src/step-runner.ts @@ -11,30 +11,17 @@ export async function runOperation( } export function createStepRunner(): StepRunner { - return { - async *run( - arg1: string | (() => T | Promise), - arg2?: () => T | Promise, - ): AsyncGenerator { - const fn = (typeof arg1 === "string" ? arg2 : arg1) as - (() => T | Promise) | undefined; - - if (!fn) { - throw new Error("Missing run function"); - } - + const runner: StepRunner = { + async *run(_key: string, fn: () => T | Promise) { return await fn(); }, - async *delay( - arg1: string | number, - arg2?: number, - ): AsyncGenerator { - const ms = typeof arg1 === "number" ? arg1 : arg2; - if (ms === undefined) { - throw new Error("Missing delay duration"); - } - + async *delay(_key: string, ms: number) { await new Promise((resolve) => setTimeout(resolve, ms)); }, + // Nothing is replayed in process, so no step key is ever read and a scope + // has nothing to namespace: one runner serves every scope. + scope: () => runner, }; + + return runner; } diff --git a/packages/reconciler/test/operation.workflows.test.ts b/packages/reconciler/test/operation.workflows.test.ts index 245cb00..1d4c410 100644 --- a/packages/reconciler/test/operation.workflows.test.ts +++ b/packages/reconciler/test/operation.workflows.test.ts @@ -13,31 +13,29 @@ import { } from "../src/operations"; import { toEmitStep } from "../src/events"; -function createStepRunnerDouble(): StepRunner { +function createStepRunnerDouble() { const run = vi.fn(async function* ( - arg1: string | (() => T | Promise), - arg2?: () => T | Promise, + _key: string, + fn: () => T | Promise, ): AsyncGenerator { - const fn = (typeof arg1 === "string" ? arg2 : arg1) as () => T | Promise; - if (!fn) { - throw new Error("Missing run function"); - } - return await fn(); }); - const delay = vi.fn(async function* (): AsyncGenerator< - unknown, - void, - unknown - > { + const delay = vi.fn(async function* ( + _key: string, + _ms: number, + ): AsyncGenerator { return; }); - return { - run, - delay, + // vi.fn erases the generic, so the seam's signature is restored here. + const runner: StepRunner = { + run: run as unknown as StepRunner["run"], + delay: delay as unknown as StepRunner["delay"], + scope: () => runner, }; + + return runner; } async function runOperation(operation: AsyncGenerator) { From 4b3bd59e86b06b3344eb8c03e7926915e9ae3b14 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:54:58 +0100 Subject: [PATCH 14/17] Read the remote during a durable dry run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/reconciler/src/durable/operations.ts | 5 ++ .../test/durable-reconciliation.test.ts | 49 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/packages/reconciler/src/durable/operations.ts b/packages/reconciler/src/durable/operations.ts index 2912922..b33c49c 100644 --- a/packages/reconciler/src/durable/operations.ts +++ b/packages/reconciler/src/durable/operations.ts @@ -55,6 +55,11 @@ export async function* reconcileResource( const driftScope = scope.scope("drift-read"); const driftRead = yield* readDriftOperation(driftScope, { ...operationParams(driftScope, resource, opts), + // A dry run suppresses mutations, not reads. Threading dryRun in here + // would make the read return {} without touching the provider, which + // decideAction would then diff against the desired params and report as + // drift that is not there. + dryRun: undefined, resourceParams: params, persistedOutput: stateNode?.output, }); diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index 7b87cca..f4e3ba3 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -542,6 +542,52 @@ describe("drift detection and repair", () => { ).toHaveLength(1); runtime.close(); }); + + it("reads the remote during a dry run rather than reporting false drift", async () => { + const remote = { name: "expected" }; + const read = vi.fn(async () => remote); + const DryRunDriftResource = resource({ type: "test/durable/dry-run-drift" }) + .defineSchema({ + name: { presence: "required", propertyType: "param" }, + } as any) + .defineOperations({ + create: (async () => remote) as any, + read, + update: async () => undefined, + delete: async () => undefined, + }); + const events: ReconcilerEvent[] = []; + const options = { + driftDetection: true, + dryRun: false, + emit: (event: ReconcilerEvent) => void events.push(event), + }; + const runtime = createRuntime( + [new DryRunDriftResource({ id: "steady", config: { name: "expected" } })], + "dry-run-drift", + options, + ); + + await runtime.run("deploy-1"); + const readsAfterDeploy = read.mock.calls.length; + + options.dryRun = true; + events.length = 0; + await runtime.run("dry-run"); + + // Suppressing the drift read under dryRun would leave decideAction + // diffing an empty read against the desired params, which reports every + // param as drift; skipping it entirely would make a dry run unable to + // report the drift it exists to report. + expect(read.mock.calls.length).toBeGreaterThan(readsAfterDeploy); + expect( + events.filter((event) => event.event === "reconciler.drift.detected"), + ).toEqual([]); + expect( + events.find((event) => event.event === "reconciler.deploy.decision"), + ).toMatchObject({ resourceId: "steady", decision: "noop" }); + runtime.close(); + }); }); function createRuntime( @@ -552,6 +598,7 @@ function createRuntime( crashAfterStep?: string; registry?: ResourceRegistry; driftDetection?: boolean; + dryRun?: boolean; emit?: (event: ReconcilerEvent) => void; } = {}, ) { @@ -574,6 +621,8 @@ function createRuntime( state, registry: options.registry, driftDetection: options.driftDetection ?? false, + // Read at execution time, so a test can switch it between runs. + dryRun: options.dryRun, emit: options.emit, maxOperationAttempts: options.maxOperationAttempts, }); From 1c4de0e3bf8f9ce4e5a7a0279fdaa54eb964a19f Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:20:24 +0100 Subject: [PATCH 15/17] Emit drift detection when recovery adopts drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/reconciler/src/reconciler.ts | 13 ++++ .../reconciler/test/reconciler.deploy.test.ts | 73 +++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/packages/reconciler/src/reconciler.ts b/packages/reconciler/src/reconciler.ts index 634bf73..e2b4a79 100644 --- a/packages/reconciler/src/reconciler.ts +++ b/packages/reconciler/src/reconciler.ts @@ -343,6 +343,19 @@ export class Reconciler { }); if (remote.kind === "present") resource.setOutput(remote.output); + // Recovery is drift adoption: the remote moved while the attempt that + // conflicted was in flight, so the same event a first-pass drift-update + // emits is owed here too. + if (action.decision === "drift-update") { + await this.#emit?.({ + level: "info", + event: "reconciler.drift.detected", + resourceId: resource.id, + resourceType: resource.type, + diff: action.patch, + }); + } + await this.#emit?.({ level: "info", event: "reconciler.deploy.decision", diff --git a/packages/reconciler/test/reconciler.deploy.test.ts b/packages/reconciler/test/reconciler.deploy.test.ts index 61d0209..9987a90 100644 --- a/packages/reconciler/test/reconciler.deploy.test.ts +++ b/packages/reconciler/test/reconciler.deploy.test.ts @@ -263,6 +263,79 @@ describe("reconciler deploy", () => { }); }); + it("reports the drift it adopts when recovering from a conflict", async () => { + // The provider ignored the update, so the remote still holds the old + // value while another writer has moved persisted state to the desired + // one: recovery re-decides as drift rather than as a repeat update. + const readSpy = vi.fn(async () => found({ name: "stale" })); + const updateSpy = vi.fn( + async (_key: unknown, _patch: Record) => undefined, + ); + const UpdateResource = createTestResourceClass({ + type: "test/service/recovery-drift", + read: readSpy, + update: updateSpy, + }); + const state = createMemoryState({ + existing: { + rev: 1, + id: "existing", + groupId: -1, + groupType: "", + type: UpdateResource.type, + config: { name: "old" }, + params: { name: "old" }, + output: { name: "old" }, + lastOperation: "create", + lastOperationAt: new Date().toISOString(), + }, + }); + const updateState = state.update.getMockImplementation()!; + state.update + .mockImplementationOnce(async () => { + state.store.existing = { + ...state.store.existing!, + rev: 2, + config: { name: "new" }, + params: { name: "new" }, + output: { name: "new" }, + }; + throw new RevConflict("existing", 1, 2); + }) + .mockImplementation(updateState); + + const events: Array> = []; + const reconciler = new Reconciler({ + state, + driftDetection: false, + emit: async (event) => { + events.push(event as unknown as Record); + }, + }); + await reconciler.deploy([ + new UpdateResource({ id: "existing", config: { name: "new" } }), + ]); + + expect( + events.filter((event) => event.event === "reconciler.drift.detected"), + ).toEqual([ + { + level: "info", + event: "reconciler.drift.detected", + resourceId: "existing", + resourceType: UpdateResource.type, + diff: { name: "new" }, + }, + ]); + expect( + events.filter( + (event) => + event.event === "reconciler.deploy.decision" && + event.decision === "drift-update", + ), + ).toHaveLength(1); + }); + it("reads remote state after a create conflict instead of creating twice", async () => { let remoteName: string | undefined; const createSpy = vi.fn(async (params) => { From efd75c502c4c28eab94f8c6027cbdfa37cbdb487 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:20:41 +0100 Subject: [PATCH 16/17] Test the deployment hold takeover, and finish the key map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/reconciler/src/durable/index.ts | 8 ++- .../test/durable-reconciliation.test.ts | 52 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/packages/reconciler/src/durable/index.ts b/packages/reconciler/src/durable/index.ts index 336d548..72646b2 100644 --- a/packages/reconciler/src/durable/index.ts +++ b/packages/reconciler/src/durable/index.ts @@ -5,7 +5,8 @@ * * notation:resource::* per-resource reconciliation steps * notation:destroy::* per-resource deletion steps - * notation:orphans::* orphan sweep, per persisted record + * notation:orphans::* orphan sweep on deploy, per record + * notation:destroy:orphans::* orphan sweep on destroy, per record * *:remote:attempt: one provider call attempt * *:remote:retry-delay: the wait between two attempts * emit:[::] event delivery checkpoint @@ -16,6 +17,11 @@ * The state: keys are store-handle keys and so are not scope-prefixed: a * store outlives the scope that opened it, which is why they carry the * resource id themselves. + * + * Resource ids are spliced in unescaped, so the delimiter is ambiguous: a + * resource named "orphans" sits in the same key space as the sweep's own + * segment. That predates the key map and is recorded here rather than fixed, + * since changing the composition invalidates in-flight executions. */ export { deploy } from "./deploy"; export { destroy } from "./destroy"; diff --git a/packages/reconciler/test/durable-reconciliation.test.ts b/packages/reconciler/test/durable-reconciliation.test.ts index f4e3ba3..516663b 100644 --- a/packages/reconciler/test/durable-reconciliation.test.ts +++ b/packages/reconciler/test/durable-reconciliation.test.ts @@ -451,6 +451,58 @@ describe("deployment coordination", () => { }); }); +describe("deployment hold takeover", () => { + it("clears a hold its named holder still has, and unblocks the deployment", async () => { + const create = vi.fn(async () => undefined); + const Resource = resource({ type: "test/durable/takeover" }) + .defineSchema({}) + .defineOperations({ create, delete: async () => undefined }); + const runtime = createRuntime([new Resource({ id: "held" })], "takeover"); + await runtime.storeClient.getOrCreateStore({ + definition: durable.deploymentCoordinationStore, + id: "takeover", + initial: { holder: "abandoned-execution" }, + }); + + const result = await durable.takeOverDeploymentHold({ + storeClient: runtime.storeClient, + deploymentId: "takeover", + fromExecutionId: "abandoned-execution", + }); + + expect(result).toEqual({ + taken: true, + previousHolder: "abandoned-execution", + }); + await runtime.run("later-execution"); + expect(create).toHaveBeenCalledOnce(); + runtime.close(); + }); + + it("refuses to clear a hold that has moved to another execution", async () => { + const runtime = createRuntime([], "takeover-race"); + await runtime.storeClient.getOrCreateStore({ + definition: durable.deploymentCoordinationStore, + id: "takeover-race", + initial: { holder: "current-execution" }, + }); + + const result = await durable.takeOverDeploymentHold({ + storeClient: runtime.storeClient, + deploymentId: "takeover-race", + fromExecutionId: "abandoned-execution", + }); + + expect(result).toEqual({ taken: false, holder: "current-execution" }); + const snapshot = await runtime.storeClient.getStore({ + definition: durable.deploymentCoordinationStore, + id: "takeover-race", + }); + expect(snapshot.state.holder).toBe("current-execution"); + runtime.close(); + }); +}); + describe("deployment scoping", () => { it("scopes store listing to the exact deployment despite prefix-like IDs", async () => { const database = createSqliteDb({ path: ":memory:" }); From 8b0f0e445f1001f30a797ba1bd37e44f1e85bef2 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:20:58 +0100 Subject: [PATCH 17/17] Run one reconciliation algorithm in both drivers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../provisioner/workflows/workflow.destroy.ts | 10 +- packages/reconciler/src/durable/operations.ts | 173 +++----- packages/reconciler/src/durable/step.ts | 2 +- packages/reconciler/src/reconcile.ts | 251 ++++++++++++ packages/reconciler/src/reconciler.ts | 368 +++++------------- .../reconciler/test/reconciler.deploy.test.ts | 53 +++ 6 files changed, 453 insertions(+), 404 deletions(-) create mode 100644 packages/reconciler/src/reconcile.ts diff --git a/packages/core/src/provisioner/workflows/workflow.destroy.ts b/packages/core/src/provisioner/workflows/workflow.destroy.ts index 813239a..d5ee58e 100644 --- a/packages/core/src/provisioner/workflows/workflow.destroy.ts +++ b/packages/core/src/provisioner/workflows/workflow.destroy.ts @@ -7,7 +7,6 @@ import { import type { StateBackend } from "@notation/state"; import { getResourceGraph } from "src/orchestrator/graph"; import { createDefaultStateBackend } from "../state-backend"; -import { refreshState } from "./workflow.refresh"; export type DestroyAppOptions = { entryPoint: string; @@ -23,11 +22,16 @@ export async function destroyApp({ emit = createLoggerReconcilerSubscriber(), }: DestroyAppOptions) { const state = stateBackend ?? createDefaultStateBackend(); - await refreshState({ entryPoint, registry, state, emit }); - const graph = await getResourceGraph(entryPoint); + + // The registry has to be threaded through: destroy sweeps orphans itself + // now, and without one the sweep falls back to the types of the resources + // still declared — so an orphan whose type the app no longer declares would + // be skipped with a warning instead of deleted. The parameter is optional, + // so nothing but this would catch it. const reconciler = new Reconciler({ state, + registry, emit, }); diff --git a/packages/reconciler/src/durable/operations.ts b/packages/reconciler/src/durable/operations.ts index b33c49c..5ccc5c0 100644 --- a/packages/reconciler/src/durable/operations.ts +++ b/packages/reconciler/src/durable/operations.ts @@ -1,21 +1,17 @@ import type { BaseResource, ResourceType } from "@notation/resource"; -import { RevConflict, type StateNode } from "@notation/state"; +import { RevConflict } from "@notation/state"; import { createMissingResourceRegistryMatchWarningEvent, createResourceRegistryFromResources, resolveResourceClass, } from "../resource-registry"; +import type { PersistState, RemoveState, StepRunner } from "../operations"; import { - createResourceOperation, - deleteResourceOperation, - readDriftOperation, - updateResourceOperation, - type PersistState, - type RemoveState, - type ResourceOperationBaseParams, -} from "../operations"; -import { decideAction, type ResourceAction } from "../plan"; -import type { DurableStateBackend } from "./state-backend"; + destroyResource, + reconcileResource as reconcile, + type EmitFromStep, + type OpenStateSession, +} from "../reconcile"; import { durableEmitter, scopeStep, type DurableStepRunner } from "./step"; import { resourceStateStore, @@ -31,71 +27,21 @@ export async function* reconcileResource( opts: DurableDeployOptions, ): AsyncGenerator { const scope = scopeStep(step, `notation:resource:${resource.id}`); - const emit = durableEmitter(scope, opts.emit); - const { stateNode, snapshot } = yield* hydrateResource(scope, resource, opts); - - // Decide the operation from desired params vs persisted state. The params - // are resolved once and then carried: deriveParams is user code and need - // not be deterministic, so an operation resolving them again could persist - // params other than the ones the decision was taken against. + // Resolved once and then carried: deriveParams is user code and need not be + // deterministic, so an operation resolving them again could persist params + // other than the ones the decision was taken against. The step also pins + // the answer across a replay. const params = yield* scope.run("params", () => resource.getParams()); - const shared = { - ...operationParams(scope, resource, opts), - resourceParams: params, - persistedOutput: stateNode?.output, - }; - let action: ResourceAction = decideAction({ resource, stateNode, params }); - - // A noop is only trusted after the remote is read back: the provider may - // have drifted from persisted state, which upgrades the decision. - if (action.decision === "noop" && (opts.driftDetection ?? true)) { - // Its own scope: the operation that follows reads the remote again, and - // the two reads must not share step keys. - const driftScope = scope.scope("drift-read"); - const driftRead = yield* readDriftOperation(driftScope, { - ...operationParams(driftScope, resource, opts), - // A dry run suppresses mutations, not reads. Threading dryRun in here - // would make the read return {} without touching the provider, which - // decideAction would then diff against the desired params and report as - // drift that is not there. - dryRun: undefined, - resourceParams: params, - persistedOutput: stateNode?.output, - }); - action = decideAction({ resource, stateNode, params, driftRead }); - } - if (action.decision === "drift-update") { - yield* emit({ - level: "info", - event: "reconciler.drift.detected", - resourceId: resource.id, - resourceType: resource.type, - diff: action.patch, - }); - } - - yield* emit({ - level: "info", - event: "reconciler.deploy.decision", - resourceId: resource.id, - resourceType: resource.type, - decision: action.decision, - }); - - if (action.decision === "noop") return; - - const persist = persistResourceState(scope, opts, resource, snapshot); - if (action.decision === "create" || action.decision === "drift-recreate") { - yield* createResourceOperation(scope, { ...shared, persist }); - return; - } - - yield* updateResourceOperation(scope, { - ...shared, - patch: action.patch, - persist, + yield* reconcile(scope, { + resource, + resourceParams: params, + openSession: durableSession(scope, opts), + emit: durableEmit(opts), + dryRun: opts.dryRun, + driftDetection: opts.driftDetection, + maxOperationAttempts: opts.maxOperationAttempts, }); } @@ -104,16 +50,15 @@ export async function* deleteResource( resource: BaseResource, opts: DurableOperationOptions, ): AsyncGenerator { - // Hydrate output from persisted state; the delete call needs the primary - // key and the state removal must be conditional on this exact snapshot. - const snapshot = yield* readSnapshot(step, opts.state, resource.id); - if (!snapshot) return; - const stateNode = toStateNode(snapshot); - resource.setOutput(stateNode.output); - - yield* deleteResourceOperation(step, { - ...operationParams(step, resource, opts), - remove: removeResourceState(step, opts, resource, snapshot), + // No recovery pass: a workflow's conditional writes are stamped with the + // step that made them, so a replay is served the recorded result rather + // than losing a race with itself. + yield* destroyResource(step, { + resource, + openSession: durableSession(step, opts), + emit: durableEmit(opts), + dryRun: opts.dryRun, + maxOperationAttempts: opts.maxOperationAttempts, }); } @@ -157,47 +102,35 @@ export async function* sweepOrphans( } } -/** - * Reads the persisted record once. The snapshot is kept so that later writes - * can be made conditional on the exact instance identity and version read - * here, and is re-served to the operations so they need no second read. - */ -async function* hydrateResource( - step: DurableStepRunner, - resource: BaseResource, - opts: DurableOperationOptions, -): AsyncGenerator< - any, - { stateNode?: StateNode; snapshot?: ResourceSnapshot }, - any -> { - const snapshot = yield* readSnapshot(step, opts.state, resource.id); - if (!snapshot) return {}; - - const stateNode = toStateNode(snapshot); - resource.setOutput(stateNode.output); - return { stateNode, snapshot }; +/** Delivery is checkpointed per scope, so the scope decides the key. */ +function durableEmit(opts: DurableOperationOptions): EmitFromStep { + return (step: StepRunner) => durableEmitter(step, opts.emit); } -function readSnapshot( - step: DurableStepRunner, - state: DurableStateBackend, - resourceId: string, -): AsyncGenerator { - return step.run("state:snapshot", () => state.snapshot(resourceId)); -} - -/** The half of the operation params every durable driver call site shares. */ -function operationParams( +/** + * Reads the persisted record once and binds the writes conditional on it. + * + * The snapshot is the precondition: it names the exact store instance and + * version the record was read at, so a write made against it cannot land on a + * record another writer has moved on. It is re-served to the operations so + * they need no second read. + */ +function durableSession( step: DurableStepRunner, - resource: BaseResource, opts: DurableOperationOptions, -): ResourceOperationBaseParams { - return { - resource, - dryRun: opts.dryRun, - emit: durableEmitter(step, opts.emit), - maxOperationAttempts: opts.maxOperationAttempts, +): OpenStateSession { + return async function* (resource: BaseResource) { + const snapshot = yield* step.run("state:snapshot", () => + opts.state.snapshot(resource.id), + ); + const persist = persistResourceState(step, opts, resource, snapshot); + if (!snapshot) return { node: undefined, persist }; + + return { + node: toStateNode(snapshot), + persist, + remove: removeResourceState(step, opts, resource, snapshot), + }; }; } diff --git a/packages/reconciler/src/durable/step.ts b/packages/reconciler/src/durable/step.ts index a3c78ae..3a37e6f 100644 --- a/packages/reconciler/src/durable/step.ts +++ b/packages/reconciler/src/durable/step.ts @@ -59,7 +59,7 @@ export function scopeStep( * event is delivered but before the checkpoint is written. */ export function durableEmitter( - step: DurableStepRunner, + step: Pick, emit: ReconcilerEventEmitter | undefined, ): EmitStep { return async function* (event) { diff --git a/packages/reconciler/src/reconcile.ts b/packages/reconciler/src/reconcile.ts new file mode 100644 index 0000000..2b125e9 --- /dev/null +++ b/packages/reconciler/src/reconcile.ts @@ -0,0 +1,251 @@ +import type { BaseResource } from "@notation/resource"; +import type { RevConflict, StateNode } from "@notation/state"; +import type { EmitStep, ReconcilerEvent } from "./events"; +import { + createResourceOperation, + deleteResourceOperation, + readDriftOperation, + updateResourceOperation, + type PersistState, + type RemoveState, + type StepRunner, +} from "./operations"; +import { decideAction } from "./plan"; + +/** + * A read of a resource's persisted record, together with the writes that are + * conditional on that exact read. + * + * The union is the point: `remove` exists only alongside a `node`, because + * removing a record that was never read is not a thing either driver can do + * safely. Keeping the writes bound to the read that produced them is what + * stops a node being combined with a precondition from a different read — a + * revision in process, a store snapshot in a workflow. + */ +export type ResourceStateSession = + | { node: undefined; persist: PersistState; remove?: never } + | { node: StateNode; persist: PersistState; remove: RemoveState }; + +/** + * Opens a session. A factory rather than an open session because recovery + * re-reads: the first pass opens once, and a recovering pass opens again + * against whatever the winning writer left behind. + */ +export type OpenStateSession = ( + resource: BaseResource, +) => AsyncGenerator; + +/** + * How a driver delivers an event from a given scope. Emission is scoped + * because a workflow checkpoints it, and two reads of the same resource in + * one execution must not share a checkpoint key. + */ +export type EmitFromStep = (step: StepRunner) => EmitStep; + +export type ReconcileResourceOptions = { + resource: BaseResource; + /** Resolved once per scheduled reconciliation, recovery included. */ + resourceParams: Record; + openSession: OpenStateSession; + emit?: EmitFromStep; + dryRun?: boolean; + driftDetection?: boolean; + maxOperationAttempts?: number; + /** + * Set when a previous attempt lost a conditional write. The remote is then + * read unconditionally and the decision retaken against it, so the attempt + * adopts what the winning writer did rather than repeating its own work. + */ + recoverFrom?: RevConflict; +}; + +/** + * Reconciles one resource: hydrate, decide, read the remote when the decision + * needs it, announce the decision, then act. + * + * Both drivers run this same generator. What stays outside it is scheduling + * (dependency levels, concurrency), the conflict retry policy, opening state + * sessions, and how a step is run — in process a plain await under a mutation + * lease, in a workflow a checkpointed step under a deployment hold. + */ +export async function* reconcileResource( + step: StepRunner, + opts: ReconcileResourceOptions, +): AsyncGenerator { + const { resource, resourceParams: params } = opts; + const recovering = opts.recoverFrom !== undefined; + + // Recovery is built on re-reading the remote, so a resource that cannot be + // read cannot be recovered: the conflict is the caller's answer. + if (recovering && !resource.read) throw opts.recoverFrom; + + const emit = opts.emit?.(step); + const session = yield* opts.openSession(resource); + if (session.node) resource.setOutput(session.node.output); + + let action = decideAction({ resource, stateNode: session.node, params }); + + // A noop is only trusted once the remote has been read back: the provider + // may have drifted from persisted state, which upgrades the decision. + // Recovery always reads, because the point of it is to see what the writer + // that won the race actually left behind. + if ( + recovering || + (action.decision === "noop" && (opts.driftDetection ?? true)) + ) { + // Its own scope: the operation that follows reads the remote again, and + // the two reads must not share step keys. + const driftStep = step.scope("drift-read"); + const driftRead = yield* readDriftOperation(driftStep, { + resource, + resourceParams: params, + persistedOutput: session.node?.output, + // Deliberately no dryRun: a dry run suppresses mutations, not reads. + // Reading is how a dry run reports drift at all. + emit: opts.emit?.(driftStep), + maxOperationAttempts: opts.maxOperationAttempts, + }); + action = decideAction({ + resource, + stateNode: session.node, + params, + driftRead, + }); + if (recovering && driftRead.kind === "present") { + resource.setOutput(driftRead.output); + } + } + + if (action.decision === "drift-update") { + yield* emitEvent(emit, { + level: "info", + event: "reconciler.drift.detected", + resourceId: resource.id, + resourceType: resource.type, + diff: action.patch, + }); + } + + yield* emitEvent(emit, { + level: "info", + event: "reconciler.deploy.decision", + resourceId: resource.id, + resourceType: resource.type, + decision: action.decision, + }); + + const shared = { + resource, + resourceParams: params, + persistedOutput: session.node?.output, + dryRun: opts.dryRun, + emit, + maxOperationAttempts: opts.maxOperationAttempts, + }; + + switch (action.decision) { + case "create": + case "drift-recreate": + yield* createResourceOperation(step, { + ...shared, + persist: session.persist, + }); + return; + case "update": + case "drift-update": + yield* updateResourceOperation(step, { + ...shared, + patch: action.patch, + persist: session.persist, + }); + return; + case "noop": + // A first-pass noop writes nothing. A recovering noop has to: it read + // the remote, found it already converged, and owes a record of that + // adoption at the revision it re-read, or the next deployment would + // reconcile against the losing writer's view. + if (!recovering || opts.dryRun) return; + yield* session.persist({ + id: resource.id, + groupId: resource.groupId, + groupType: resource.groupType, + type: resource.type, + lastOperation: "drift", + lastOperationAt: new Date().toISOString(), + config: resource.config, + params: resource.toState(params), + output: resource.toState(resource.output), + }); + } +} + +export type DestroyResourceOptions = { + resource: BaseResource; + openSession: OpenStateSession; + emit?: EmitFromStep; + dryRun?: boolean; + maxOperationAttempts?: number; + /** + * Set when a previous attempt lost its conditional removal. Carries the + * params because the recovery read needs them, and deletion is the one path + * that never resolves them otherwise. + */ + recoverFrom?: { + conflict: RevConflict; + resourceParams: Record; + }; +}; + +/** + * Deletes one resource. A resource with no persisted record was never created + * — or has already been deleted — and is skipped, which is also what makes + * the sweep of a partly-deleted deployment idempotent. + */ +export async function* destroyResource( + step: StepRunner, + opts: DestroyResourceOptions, +): AsyncGenerator { + const { resource } = opts; + const session = yield* opts.openSession(resource); + if (!session.node) return; + resource.setOutput(session.node.output); + + if (opts.recoverFrom) { + const { conflict, resourceParams } = opts.recoverFrom; + if (!resource.read) throw conflict; + + const driftStep = step.scope("drift-read"); + const remote = yield* readDriftOperation(driftStep, { + resource, + resourceParams, + persistedOutput: session.node.output, + emit: opts.emit?.(driftStep), + maxOperationAttempts: opts.maxOperationAttempts, + }); + + // Already gone remotely: the delete succeeded and only the record is + // left, so drop the record rather than calling the provider again. + if (remote.kind !== "present") { + if (!opts.dryRun) yield* session.remove(); + return; + } + resource.setOutput(remote.output); + } + + yield* deleteResourceOperation(step, { + resource, + dryRun: opts.dryRun, + emit: opts.emit?.(step), + maxOperationAttempts: opts.maxOperationAttempts, + remove: session.remove, + }); +} + +function emitEvent( + emit: EmitStep | undefined, + event: ReconcilerEvent, +): AsyncGenerator { + return emit ? emit(event) : noSteps(); +} + +async function* noSteps(): AsyncGenerator {} diff --git a/packages/reconciler/src/reconciler.ts b/packages/reconciler/src/reconciler.ts index e2b4a79..8c119cf 100644 --- a/packages/reconciler/src/reconciler.ts +++ b/packages/reconciler/src/reconciler.ts @@ -2,25 +2,14 @@ import type { BaseResource, ResourceType } from "@notation/resource"; import { RevConflict, type State, type StateNode } from "@notation/state"; import { setTimeout as sleep } from "node:timers/promises"; import { buildResourceDepthLevels } from "./dependency-graph"; +import type { Plan } from "./plan"; +import type { PersistState, RemoveState, StepRunner } from "./operations"; import { - decideAction, - getDependencyIds, - resolvePlanParams, - type DriftRead, - type Plan, - type PlanNode, - type ResourceAction, -} from "./plan"; -import { - createResourceOperation, - deleteResourceOperation, - readDriftOperation, - type OperationEventEmitter, - type PersistState, - type RemoveState, - type StepRunner, - updateResourceOperation, -} from "./operations"; + destroyResource, + reconcileResource, + type EmitFromStep, + type OpenStateSession, +} from "./reconcile"; import { createMissingResourceRegistryMatchWarningEvent, createResourceRegistryFromResources, @@ -77,7 +66,7 @@ export class Reconciler { readonly #defaultDryRun: boolean; readonly #defaultDriftDetection: boolean; readonly #emit?: ReconcilerEventEmitter; - readonly #emitStep?: OperationEventEmitter; + readonly #emitFromStep?: EmitFromStep; readonly #maxOperationAttempts?: number; readonly #mutationLeaseTtl: number; readonly #stepRunner: StepRunner; @@ -88,8 +77,10 @@ export class Reconciler { this.#defaultDryRun = opts.dryRun ?? false; this.#defaultDriftDetection = opts.driftDetection ?? true; this.#emit = opts.emit; - // Operations emit through a step seam; in process that is a plain await. - this.#emitStep = opts.emit ? toEmitStep(opts.emit) : undefined; + // Everything the shared generator emits — decisions, drift and operation + // lifecycle alike — goes through one seam; in process it is a plain + // await, and the scope it is handed is ignored because nothing is keyed. + this.#emitFromStep = opts.emit ? emitDirectly(opts.emit) : undefined; this.#maxOperationAttempts = opts.maxOperationAttempts; this.#mutationLeaseTtl = opts.mutationLeaseTtl ?? 30_000; this.#stepRunner = createStepRunner(); @@ -132,6 +123,9 @@ export class Reconciler { opts: DestroyOptions = {}, ): Promise { const dryRun = opts.dryRun ?? this.#defaultDryRun; + const resourceById = new Map( + resources.map((resource) => [resource.id, resource]), + ); const dependencyLevels = buildResourceDepthLevels(resources); for ( @@ -144,6 +138,10 @@ export class Reconciler { level.map((resource) => this.#destroyResource(resource, dryRun)), ); } + + // Then the records that were never declared, so a destroy leaves the + // deployment empty rather than leaving orphans for a later refresh. + await this.#deleteOrphans(resources, resourceById, dryRun, "destroy"); } async refresh( @@ -171,12 +169,17 @@ export class Reconciler { const params = (await resource.getParams()) as Record; await this.#retryOnRevConflict((conflict) => - this.#deployResourceOnce( - resource, - params, - dryRun, - driftDetection, - conflict, + runOperation( + reconcileResource(this.#stepRunner, { + resource, + resourceParams: params, + openSession: this.#openSession(), + emit: this.#emitFromStep, + dryRun, + driftDetection, + maxOperationAttempts: this.#maxOperationAttempts, + recoverFrom: conflict, + }), ), ); }); @@ -234,220 +237,34 @@ export class Reconciler { } } - async #deployResourceOnce( - resource: BaseResource, - params: Record, - dryRun: boolean, - driftDetection: boolean, - conflict?: RevConflict, - ) { - if (conflict) { - await this.#recoverDeployResource(resource, params, dryRun, conflict); - return; - } - - const stateNode = await this.#state.get(resource.id); - - let action: ResourceAction; - if (!stateNode) { - action = decideAction({ resource, params }); - } else { - resource.setOutput(stateNode.output); - action = decideAction({ resource, stateNode, params }); - - if (action.decision === "noop" && driftDetection) { - const driftRead = await this.#readForDrift( - resource, - params, - stateNode.output, - ); - action = decideAction({ resource, stateNode, params, driftRead }); - } - } - - if (action.decision === "drift-update") { - await this.#emit?.({ - level: "info", - event: "reconciler.drift.detected", - resourceId: resource.id, - resourceType: resource.type, - diff: action.patch, - }); - } - - await this.#emit?.({ - level: "info", - event: "reconciler.deploy.decision", - resourceId: resource.id, - resourceType: resource.type, - decision: action.decision, - }); - - switch (action.decision) { - case "create": - case "drift-recreate": - await runOperation( - createResourceOperation(this.#stepRunner, { - resource, - resourceParams: params, - persistedOutput: stateNode?.output, - dryRun, - emit: this.#emitStep, - maxOperationAttempts: this.#maxOperationAttempts, - persist: this.#persist(resource.id, stateNode?.rev ?? 0), - }), - ); - return; - case "update": - case "drift-update": - // decideAction only returns update decisions for an existing stateNode - await runOperation( - updateResourceOperation(this.#stepRunner, { - resource, - resourceParams: params, - persistedOutput: stateNode?.output, - patch: action.patch, - dryRun, - emit: this.#emitStep, - maxOperationAttempts: this.#maxOperationAttempts, - persist: this.#persist(resource.id, stateNode!.rev), - }), - ); - return; - case "noop": - return; - } - } - - async #recoverDeployResource( - resource: BaseResource, - params: Record, - dryRun: boolean, - conflict: RevConflict, - ) { - if (!resource.read) throw conflict; - - const stateNode = await this.#state.get(resource.id); - if (stateNode) resource.setOutput(stateNode.output); - - const remote = await this.#readForDrift( - resource, - params, - stateNode?.output, - ); - const action = decideAction({ - resource, - stateNode, - params, - driftRead: remote, - }); - if (remote.kind === "present") resource.setOutput(remote.output); - - // Recovery is drift adoption: the remote moved while the attempt that - // conflicted was in flight, so the same event a first-pass drift-update - // emits is owed here too. - if (action.decision === "drift-update") { - await this.#emit?.({ - level: "info", - event: "reconciler.drift.detected", - resourceId: resource.id, - resourceType: resource.type, - diff: action.patch, - }); - } - - await this.#emit?.({ - level: "info", - event: "reconciler.deploy.decision", - resourceId: resource.id, - resourceType: resource.type, - decision: action.decision, - }); - - switch (action.decision) { - case "create": - case "drift-recreate": - await runOperation( - createResourceOperation(this.#stepRunner, { - resource, - resourceParams: params, - persistedOutput: stateNode?.output, - dryRun, - emit: this.#emitStep, - maxOperationAttempts: this.#maxOperationAttempts, - persist: this.#persist(resource.id, stateNode?.rev ?? 0), - }), - ); - return; - case "update": - case "drift-update": - await runOperation( - updateResourceOperation(this.#stepRunner, { - resource, - resourceParams: params, - persistedOutput: stateNode?.output, - patch: action.patch, - dryRun, - emit: this.#emitStep, - maxOperationAttempts: this.#maxOperationAttempts, - persist: this.#persist(resource.id, stateNode?.rev ?? 0), - }), - ); - return; - case "noop": - if (dryRun) return; - await this.#state.update(resource.id, stateNode?.rev ?? 0, { - id: resource.id, - groupId: resource.groupId, - groupType: resource.groupType, - type: resource.type, - lastOperation: "drift", - lastOperationAt: new Date().toISOString(), - config: resource.config, - params: resource.toState(params), - output: resource.toState(resource.output), - }); - return; - } - } - - // In process, concurrency control is a compare-and-set against the revision - // read before the operation started; the mutation lease keeps writers apart. - #persist(resourceId: string, expectedRev: number): PersistState { - const state = this.#state; - return async function* (next) { - await state.update(resourceId, expectedRev, next); - }; - } - - #remove(resourceId: string, expectedRev: number): RemoveState { + /** + * Reads a resource's record and binds the writes that are conditional on + * that read. In process the precondition is the revision read here, which + * the mutation lease keeps other writers away from. + */ + #openSession(): OpenStateSession { const state = this.#state; - return async function* () { - await state.delete(resourceId, expectedRev); + return async function* (resource: BaseResource) { + const node = await state.get(resource.id); + const expectedRev = node?.rev ?? 0; + const persist: PersistState = async function* (next) { + await state.update(resource.id, expectedRev, next); + }; + + if (!node) return { node: undefined, persist }; + + const remove: RemoveState = async function* () { + await state.delete(resource.id, node.rev); + }; + return { node, persist, remove }; }; } - #readForDrift( - resource: BaseResource, - params: Record, - persistedOutput: Record | undefined, - ): Promise { - return runOperation( - readDriftOperation(this.#stepRunner, { - resource, - resourceParams: params, - persistedOutput, - emit: this.#emitStep, - maxOperationAttempts: this.#maxOperationAttempts, - }), - ); - } - async #deleteOrphans( resources: BaseResource[], resourceById: Map, dryRun: boolean, - workflow: "deploy" | "refresh", + workflow: "deploy" | "refresh" | "destroy", ) { await this.#withLease("reconciler:orphan-deletion", async () => { const stateNodes = await this.#state.values(); @@ -471,23 +288,17 @@ export class Reconciler { continue; } + // Built from the listing, so its config is as of that read rather + // than of the lease taken below — the session re-reads and refreshes + // output, but not this. Config reaches nothing but deriveParams on + // the delete-recovery read, so a racing write can only make that read + // one revision stale; the removal itself is still conditional on the + // session's own read. + const orphanResource = hydrateResourceFromState(Resource, stateNode); await this.#withMutationLease(stateNode.id, () => - this.#retryOnRevConflict(async (conflict) => { - const currentNode = await this.#state.get(stateNode.id); - if (!currentNode) return; - - const orphanResource = hydrateResourceFromState( - Resource, - currentNode, - ); - - await this.#deleteResourceOnce( - orphanResource, - currentNode, - dryRun, - conflict, - ); - }), + this.#retryOnRevConflict((conflict) => + this.#deleteResource(orphanResource, dryRun, conflict), + ), ); } }); @@ -495,49 +306,37 @@ export class Reconciler { async #destroyResource(resource: BaseResource, dryRun: boolean) { await this.#withMutationLease(resource.id, () => - this.#retryOnRevConflict(async (conflict) => { - const stateNode = await this.#state.get(resource.id); - if (!stateNode) { - return; - } - - resource.setOutput(stateNode.output); - await this.#deleteResourceOnce(resource, stateNode, dryRun, conflict); - }), + this.#retryOnRevConflict((conflict) => + this.#deleteResource(resource, dryRun, conflict), + ), ); } - async #deleteResourceOnce( + async #deleteResource( resource: BaseResource, - stateNode: StateNode, dryRun: boolean, conflict?: RevConflict, ) { - if (conflict) { - if (!resource.read) throw conflict; - - // Deletion never needs the desired params, so they are resolved here - // rather than for every delete: only the recovery read consumes them. - const params = (await resource.getParams()) as Record; - const remote = await this.#readForDrift( - resource, - params, - stateNode.output, - ); - if (remote.kind !== "present") { - if (!dryRun) await this.#state.delete(resource.id, stateNode.rev); - return; - } - resource.setOutput(remote.output); - } + // Deletion never needs the desired params, so they are resolved only for + // the recovery read, which is their sole consumer on this path. + const recoverFrom = conflict + ? { + conflict, + resourceParams: (await resource.getParams()) as Record< + string, + unknown + >, + } + : undefined; await runOperation( - deleteResourceOperation(this.#stepRunner, { + destroyResource(this.#stepRunner, { resource, + openSession: this.#openSession(), + emit: this.#emitFromStep, dryRun, - emit: this.#emitStep, maxOperationAttempts: this.#maxOperationAttempts, - remove: this.#remove(resource.id, stateNode.rev), + recoverFrom, }), ); } @@ -557,3 +356,12 @@ function hydrateResourceFromState( resource.setOutput(stateNode.output); return resource; } + +/** + * The in-process emit seam: a scope carries no meaning here, since nothing + * keys anything, so every scope delivers through the same emitter. + */ +function emitDirectly(emit: ReconcilerEventEmitter): EmitFromStep { + const step = toEmitStep(emit); + return () => step; +} diff --git a/packages/reconciler/test/reconciler.deploy.test.ts b/packages/reconciler/test/reconciler.deploy.test.ts index 9987a90..61c328b 100644 --- a/packages/reconciler/test/reconciler.deploy.test.ts +++ b/packages/reconciler/test/reconciler.deploy.test.ts @@ -766,6 +766,59 @@ describe("reconciler destroy + refresh", () => { expect(state.delete).toHaveBeenCalledWith("c", 1); }); + it("destroy sweeps orphans after the resources it was given", async () => { + const order: string[] = []; + const DeclaredResource = createTestResourceClass({ + type: "test/service/destroy-sweep-declared", + delete: async () => void order.push("declared"), + }); + // Its type is only in the registry: an app that stopped declaring it is + // exactly the case where falling back to the declared types would skip it. + const OrphanResource = createTestResourceClass({ + type: "test/service/destroy-sweep-orphan", + delete: async () => void order.push("orphan"), + }); + + const state = createMemoryState({ + declared: { + rev: 1, + id: "declared", + groupId: -1, + groupType: "", + type: DeclaredResource.type, + config: { name: "declared" }, + params: { name: "declared" }, + output: { name: "declared" }, + lastOperation: "create", + lastOperationAt: new Date().toISOString(), + }, + orphan: { + rev: 1, + id: "orphan", + groupId: -1, + groupType: "", + type: OrphanResource.type, + config: { name: "orphan" }, + params: { name: "orphan" }, + output: { name: "orphan" }, + lastOperation: "create", + lastOperationAt: new Date().toISOString(), + }, + }); + + const reconciler = new Reconciler({ + state, + registry: createResourceRegistry([DeclaredResource, OrphanResource]), + }); + + await reconciler.destroy([ + new DeclaredResource({ id: "declared", config: { name: "declared" } }), + ]); + + expect(order).toEqual(["declared", "orphan"]); + expect(state.store).toEqual({}); + }); + it("refresh removes orphan state entries", async () => { const deleteSpy = vi.fn(async () => undefined); const OrphanResource = createTestResourceClass({