🐛 Restore the durable eval journal boundary - #394
Conversation
PR #394: 🐛 Restore the durable eval journal boundary25 files, +1999 / -374 Scope🔴 PR has 2373 lines changed. Split into focused PRs. 🟡 2373 lines changed. PRs under 400 receive more thorough review. 🟡 25 files changed. Are all changes related? StructuralOxlint structural signals:
Slop✅ Slop indicators look low. Static AnalysisOxlint: 204 diagnostics across 10 files (36 rules) capitalized-comments (58): packages/durable-streams/combinators.ts, packages/core/src/errors.ts, packages/durable-streams/guard.ts (+3) CorrectnessNo extraneous code patterns detected. |
Architecture review: REQUEST CHANGESThe central correction is right: durable eval's live-binding validation belongs in core, after transformation and before The new terminal boundary is not complete enough to uphold the architecture yet. Three related paths can still convert a durability/persistence failure into a recorded document outcome or accept incompatible retained history. 1. Preserve a durability failure before consulting replay exhaustionIn This contradicts Make durability-failure routing unconditional and prior to ordinary terminal handling. Add direct durable-stream tests in which:
2. A failed append is storage failure, not workflow failureBoth live effect factories currently resolve a failed There is an even smaller version at Keep journal I/O failure outside the document outcome. Structure the control flow and/or give failed event persistence a durable-stream-owned failure marker so neither root nor child terminal handling can journal it. Preserve the underlying adapter error as the cause. Add fail-once stream tests for both 3. Terminal alignment must cover the coroutine tree, including completed children
The terminal check cannot distinguish that orphaned completed child from a completed child actually recreated by the current scope tree, so it appends a root The same terminal policy also has to apply inside Track which retained coroutine identities the current run actually claims/visits, and apply one terminal-alignment/durability-failure policy before any root or child
Update the protocol and integration text with the resulting all-coroutine contract. It currently says all terminal paths are resolved while the implementation and Once these are addressed, the PR's eval validation boundary and immediate-root divergence behavior fit the architecture coherently. |
|
Addressed the architecture review in |
8bbdf5b to
9165cf9
Compare
…nup fatal Structural parsing runs from `fatalCause`, which every generic catch in expansion consults. A provider is free to hand back a Proxy that refuses to be inspected, and one of these parsers throwing would replace the failure being classified with a failure about classifying it. Every read is now total, and so is the cause traversal: a hostile wrapper narrows what discovery finds instead. Recognizing a Files fatal is a decision to let that exact object travel onward by identity, so it now requires the whole public contract — frozen data with no extra fields, the fixed diagnostic for its kind, and no cause. A candidate carrying a raw message or an errno chain is replaced by a fresh invariant rather than preserved. Durability recognition is unchanged: it stays #394's class-based mechanism, and only the Files boundary is crossed by a second loaded copy. A host cleanup that fails while cancellation is unwinding has no outcome to report beside it. It leaves the scope as a fixed teardown invariant instead of manufacturing a write result, carrying neither the platform's error nor the generated temporary's name. The loaded-copy claim is now proved by a real second copy: the Files module is bundled, imported as its own module, and its failures are recognized in both directions.
…nup fatal Structural parsing runs from `fatalCause`, which every generic catch in expansion consults. A provider is free to hand back a Proxy that refuses to be inspected, and one of these parsers throwing would replace the failure being classified with a failure about classifying it. Every read is now total, and so is the cause traversal: a hostile wrapper narrows what discovery finds instead. Recognizing a Files fatal is a decision to let that exact object travel onward by identity, so it now requires the whole public contract — frozen data with no extra fields, the fixed diagnostic for its kind, and no cause. A candidate carrying a raw message or an errno chain is replaced by a fresh invariant rather than preserved. Durability recognition is unchanged: it stays #394's class-based mechanism, and only the Files boundary is crossed by a second loaded copy. A host cleanup that fails while cancellation is unwinding has no outcome to report beside it. It leaves the scope as a fixed teardown invariant instead of manufacturing a write result, carrying neither the platform's error nor the generated temporary's name. The loaded-copy claim is now proved by a real second copy: the Files module is bundled, imported as its own module, and its failures are recognized in both directions.
…nup fatal Structural parsing runs from `fatalCause`, which every generic catch in expansion consults. A provider is free to hand back a Proxy that refuses to be inspected, and one of these parsers throwing would replace the failure being classified with a failure about classifying it. Every read is now total, and so is the cause traversal: a hostile wrapper narrows what discovery finds instead. Recognizing a Files fatal is a decision to let that exact object travel onward by identity, so it now requires the whole public contract — frozen data with no extra fields, the fixed diagnostic for its kind, and no cause. A candidate carrying a raw message or an errno chain is replaced by a fresh invariant rather than preserved. Durability recognition is unchanged: it stays #394's class-based mechanism, and only the Files boundary is crossed by a second loaded copy. A host cleanup that fails while cancellation is unwinding has no outcome to report beside it. It leaves the scope as a fixed teardown invariant instead of manufacturing a write result, carrying neither the platform's error nor the generated temporary's name. The loaded-copy claim is now proved by a real second copy: the Files module is bundled, imported as its own module, and its failures are recognized in both directions.
* 💥 Contain document filesystem access behind API.Files (#227) `<File>`, `<Glob>` and `<TempDir>` reached the host filesystem directly, so `xmd run` and a workflow run could not mean the same thing for one document. Document filesystem access now goes through `API.Files`, a contextual Api of whole semantic operations with no host default. The four CLI entrypoints install the host provider explicitly; a run with none installed fails rather than reaching the host. The three components make no filesystem call of their own and import no host path or fs module. What they keep is order: a write's lexical check runs before its children, and the semantic write that follows repeats admission and owns every later phase, so the earlier check authorizes nothing. Ordinary failures cross the boundary as frozen structural data — a reason from a fixed vocabulary and the phase it came from — and every printed message is byte-identical to before. A provider that is absent, refuses an operation, or breaks its own contract throws instead, and core's fatal traversal ranks it between a durability failure and a documentation failure, by identity and by structural tag. * 🐛 Keep core's error module out of the runtime's host graph `errors.ts` is in the graph a separately loaded copy of `printErrors` bundles, and importing the runtime's package root pulled the host Apis in with it — including a native addon no bundler can inline. The Files recognizer needs none of that, so it comes from the leaf module through a new `./files` subpath. The five-target job prepares with `deno install` rather than `deno task deps`: the task caches graphs this job does not need and reaches them by spawning a child, which does not survive the Windows runner's path handling. Its compile now carries the repository's isolation flags, and the rule that enforces them reads every compile in a workflow rather than letting the first invocation's flags answer for the rest. * 🐛 Sort the search results with the lib the Node typecheck targets `toSorted` needs es2023, which `tsconfig.node.json` does not select. The array is built from a Set on the line above, so nothing shared is being mutated. * 🔒 Make Files recognition total and strict, and keep cancellation cleanup fatal Structural parsing runs from `fatalCause`, which every generic catch in expansion consults. A provider is free to hand back a Proxy that refuses to be inspected, and one of these parsers throwing would replace the failure being classified with a failure about classifying it. Every read is now total, and so is the cause traversal: a hostile wrapper narrows what discovery finds instead. Recognizing a Files fatal is a decision to let that exact object travel onward by identity, so it now requires the whole public contract — frozen data with no extra fields, the fixed diagnostic for its kind, and no cause. A candidate carrying a raw message or an errno chain is replaced by a fresh invariant rather than preserved. Durability recognition is unchanged: it stays #394's class-based mechanism, and only the Files boundary is crossed by a second loaded copy. A host cleanup that fails while cancellation is unwinding has no outcome to report beside it. It leaves the scope as a fixed teardown invariant instead of manufacturing a write result, carrying neither the platform's error nor the generated temporary's name. The loaded-copy claim is now proved by a real second copy: the Files module is bundled, imported as its own module, and its failures are recognized in both directions. * 🔒 Rebuild every Files outcome from validated parts Recognizing a Files fatal hands that exact object onward, so the contract now covers the whole Error: the fixed name and diagnostic for its kind, frozen data with exactly the kind's fields, no cause, and no other enumerable member — string or symbol. A path riding on `name`, on an extra property, or under a symbol key fails the contract, and `invokeFiles` replaces the candidate rather than preserving it. A `Result` is only conventionally a Result. The TypeScript signature is a claim about the provider, not a guarantee, so a component that read `ok`, `value`, or `error` first would be the thing that ran a hostile accessor — outside anything that sanitizes. The core wrappers now inspect the container totally and rebuild every outcome from validated parts: no provider-originated container, error object, or payload reaches `<File>`, `<Glob>` or `<TempDir>`, and a search result is copied rather than passed along. A container that will not say how it settled, and a success it cannot describe, are provider-contract failures. A malformed non-write failure is not: the vocabulary already has a sentence for it, so a fresh generic failure is substituted and the document carries on. FA24 now runs all six orderings of the three fatal kinds, through both wrappers. * 🔒 Tell an unreadable Result member apart from an absent one `undefined` was standing for three different answers: a member read fine and held undefined, a member was absent, and reading a member threw. Collapsing them let two containers through that never described their outcome — `checkFilePath` accepted a success whose `value` refused to be read, and a non-write failure whose `error` refused was downgraded to the printable generic. Presence and readability are now asked separately, and every operation says which it requires. A search array is copied by index through the same reader, so length and element traps are covered too and the iterator is never consulted. The one place the contract bends is `checkFilePath`: Effection spells a payload-free success as its shared `Unit`, `{ ok: true }` with no `value` member at all, so absence there is the ordinary success rather than a failure. What is refused is a `value` that cannot be read, and one that is present but is something other than undefined. The seam now passes live Proxies around real `Ok`/`Err` values through `API.Files`, so the boundary is the first thing to run a hostile trap — the previous JSON round-trip invoked the getter inside the provider handler and proved only the already-covered handler-throw path. * 🔒 Recognize a search result's array brand totally `Array.isArray` is itself an operation on provider-controlled data: it throws on a revoked Proxy. Running it outside the total readers let a raw TypeError leave the boundary untagged, so a search whose result was revoked before it was returned surfaced the platform's message and let the document carry on. The brand check now answers instead of throwing, and a value whose array identity cannot be inspected is malformed success data like any other. The guard sits inside the payload contract rather than around the call site, so it stays the thing under test: restoring the unguarded call reds the regression. The specification catalog gains the rows for behavior already implemented — HF12b and FF11 through FF15 — and the provider-failure prose now separates an outcome that will not say what it is, which is a contract violation, from one that reads fine and reports a failure this version does not recognize, which is the generic sentence. * 📝 State the non-write failure rule for every non-write operation The provider-failure passages named only a read and a search, which left path admission and TempDir unstated even though they take the same path: readable data that does not validate selects the generic sentence there too. The FF14 row described a stricter rule than the boundary implements. It now matches FF14c and FF15: an unreadable settlement is a contract failure, so is an unreadable selected member, and an absent success value only where the operation carries one — path admission succeeds without one. A readable but unrecognized non-write failure stays printable. FF11 is renamed to what it asserts. Declining to recognize a hostile shape is not recognizing it as a valid structural failure. --------- Co-authored-by: Taras Mankovski <74687+taras@users.noreply.github.com>
1 participant
Why
Merged PR #390 put attached-service collision checks inside a generic durable-operation validation hook. Replay could therefore consume a recorded successful
Yieldand replace it with an unjournaled validation error. This draft is an architectural correction to #390; the attached-service product contract remains unchanged.Architecture review also identified terminal paths that could record durability or persistence failures as document outcomes, retained completed children that could escape replay alignment, and caught durability failures that allowed later durable work before termination. This revision closes those boundaries across durable entry, ordered persistence, and root/child termination.
What changes
Before:
createDurableOperation()accepted a generic validation callback on live and replay paths.Close(err)for stale, divergent, or failed-persistence execution.DurablePersistenceErrorwrapper.After:
createDurableOperation()again has only its description and executor and preserves persist-before-resume.DurablePersistenceErrorregardless of the adapter error's class, preserves the exact adapter value as cause, and activates shared fail-stop state across the durable coroutine tree.Close.guardDurableStream()keeps policy rejection distinct from backing-store failure, preserving the policy error and the existing guarded-journal contract.How it works
Replay restores only matching recorded results. The first durability failure is shared by the root and children, and durable entry plus the append boundary enforce it even if workflow code catches the error. Before any new root or child
Close, durable streams also confirms that the terminating coroutine's retained subtree is aligned and that no durability failure is active.Review guide
Start with:
packages/durable-streams/tests/fail-stop.test.tsanddurability.tsThen review:
packages/durable-streams/effect.tsfor pre-replay/pre-executor fail-stop checks and persist-before-resumepackages/durable-streams/run.ts,combinators.ts, andreplay-index.tsfor terminal alignment and shared child statepackages/durable-streams/tests/terminal-boundary.test.tsanddurable-run.test.tsfor genuine-history and fail-once regressionspackages/core/src/eval-handler.tsandpackages/core/tests/ephemeral-service.test.tsfor pre-effect collision validation from the earlier PR revisionarchitecture.mdand the executable/durable-stream specifications for the resulting contractLook carefully at:
YieldYieldor successfulCloseappend attempts no compensating terminal appendWhat must stay true
Yieldappend completes — enforced by both effect factories and paused/fail-once stream tests.API.Service,startService(),service=<binding>, host adapters, the XMD service handshake protocol, the live overlay, and scoped service ownership are unchanged.useWorkflowServiceDenial(); Add the xmd workflow start/resume filesystem vertical slice #366 will install it in futurexmd workflow startandresumescopes. No workflow CLI execution branch exists yet.How to verify it
1/0, adapter attempts1, stored events0, a newDurablePersistenceError, and the exact adapter error as cause.1/1, and persists exactlyYield(later)plusClose(root)without introducing a wrapper.--related=packages/durable-streams/durability.ts): 293 suites / 2,170 steps passed.1/0, adapter attempts1, stored events0; exactDurablePersistenceErrorand adapter cause escape.0, later executors0, appends0; retained bytes unchanged.1/0, adapter attempts1, stored events0; both catches and the run observe the same first error.1/1, adapter attempts1, stored events0; the queued append is fenced before storage.1/1, backend events2(Yield(later),Close(root)); the run completes normally.deno task lint— passed with 0 errors.deno task check— passed.deno task check:jsr—Success Dry run complete.deno task verify— all 9 applicable commands passed: vendor 3.1s, lint 2.7s, check 1.2s, Deno 648s, JSR 1.2s, TypeScript 22.3s, Node 235s, Bun 337.8s, docs 16s; tracked tree unchanged.Scope
Included
Intentionally unchanged
New abstractions
DurablePersistenceErrordistinguishes backing journal failure from a document failure while retaining the adapter error as its cause.DurableAppendFenceserializes the final admission to stream storage so queued work can recheck shared fail-stop state at the adapter boundary.Risks and limitations
Scope confirmation