From 1901a07e39a2ee0d953fabea3caf8b78ccd776bb Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Wed, 26 Aug 2026 19:35:34 +0300 Subject: [PATCH 1/3] =?UTF-8?q?feat(core):=20add=20the=20execution=20conte?= =?UTF-8?q?xt=20model=20=E2=80=94=20Phase=204a=20(CTX-1..CTX-20,=20XCUT-14?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships the per-call correlation state the pipeline is built on, per product-spec/07-execution-context-model.md and docs/superpowers/specs/2026-07-25-phase4a-execution-context-design.md. New `packages/core/src/context/`, layered instrumentation → errors → context → store, with no `index.ts` barrel (docs/knowledge/module-organization.md:18 bans internal barrels; 4c imports the files directly): - `instrumentation.ts` — the `InstrumentationBundle` shape (CTX-14) and its frozen no-op default (CTX-15, CTX-20). `activeSpan`/`tracerFactory` stay typed `unknown`: a real tracing adapter owns their shape, deferred to Phase 7. - `errors.ts` — `DuplicateContextKeyError extends DexpaceError`, carrying the offending `key` as a readonly field (CTX-8). - `context.ts` — `DispatchContext`/`RequestContext`/`ExchangeContext` as a frozen discriminated union over plain data, with `create*` factories and the two one-way promotions (CTX-1, CTX-2, CTX-3, CTX-5, CTX-6, CTX-7, CTX-16). No classes: nothing here owns a lifecycle. Call keys are `Symbol()`, never a trace-derived string. The factories and both promotions freeze the instrumentation bundle in place — `Object.freeze` is shallow, so freezing only the context would leave a caller-supplied bundle writable behind the `instrumentation` slot, and the flavors are interfaces, so a literal-built context can reach a promotion without passing a factory. - `store.ts` — `ContextStore`, a bounded `Map` with a post-insert drain loop, plus the process-wide `contextStore` singleton (CTX-7..13, CTX-18, CTX-19). Also the subject of XCUT-14, which names "context registries" first among the caller-keyed process-lived maps that must be capped — and is the only appendix-B conformance row this code satisfies, since appendix B has no CTX section at all. Nothing enters the public barrel: `context/` is SDK-internal correlation plumbing, and `packages/core/etc/core.api.md` is byte-identical. Tests: 45 across four colocated files, each header citing the IDs it exercises. A 23-mutant sweep over the module kills 21; one survivor was an equivalent mutant, and the other — collapsing `#drain`'s loop into a single check-then-evict — is unkillable by construction, since both callers set one key before draining so the map never exceeds cap + 1. The loop is kept because CTX-12 and XCUT-14 mandate the shape for runtimes with real concurrency; both `#drain` and its describe block say so, and it is registered as open item A6. Verified against the CI-pinned toolchain rather than the local one: every `ci` job step under bun 1.3.14 (`.bun-version`), and node-conformance on both matrix legs — the 20.3.0 floor and lts/* (v24.20.0) — 31 pass each. The context module itself was additionally exercised against the built artifact on both Node versions, confirming the plan's claim that nothing here is runtime-divergent. Also fixes bunfig.toml, where merging the Phase 3 and knowledge-CLI branches left `root = "packages"` twice under `[test]`; Bun refuses a config with a redefined key, so `bun test` failed to start at all. Deliberate deferrals, all registered in docs/open-items.md rather than left silent: CTX-17's positive half (install-on-first-promotion) belongs to 4c, which owns the store handle; real W3C Trace Context generation to Phase 7; `contextsEqual()` unscheduled; and CTX-8's message clause (a Symbol's description names the flavor, not the instance) awaiting a decision as A5. --- .changeset/2026-08-26-execution-context.md | 5 + docs/open-items.md | 83 +++++- packages/core/src/context/context.test.ts | 261 +++++++++++++++++ packages/core/src/context/context.ts | 198 +++++++++++++ packages/core/src/context/errors.test.ts | 29 ++ packages/core/src/context/errors.ts | 17 ++ .../core/src/context/instrumentation.test.ts | 44 +++ packages/core/src/context/instrumentation.ts | 40 +++ packages/core/src/context/store.test.ts | 269 ++++++++++++++++++ packages/core/src/context/store.ts | 133 +++++++++ 10 files changed, 1076 insertions(+), 3 deletions(-) create mode 100644 .changeset/2026-08-26-execution-context.md create mode 100644 packages/core/src/context/context.test.ts create mode 100644 packages/core/src/context/context.ts create mode 100644 packages/core/src/context/errors.test.ts create mode 100644 packages/core/src/context/errors.ts create mode 100644 packages/core/src/context/instrumentation.test.ts create mode 100644 packages/core/src/context/instrumentation.ts create mode 100644 packages/core/src/context/store.test.ts create mode 100644 packages/core/src/context/store.ts diff --git a/.changeset/2026-08-26-execution-context.md b/.changeset/2026-08-26-execution-context.md new file mode 100644 index 0000000..5f619e3 --- /dev/null +++ b/.changeset/2026-08-26-execution-context.md @@ -0,0 +1,5 @@ +--- +"@dexpace/core": patch +--- + +Internal: execution context promotion chain and bounded store for product-spec §7 (CTX-1..20). No public API change. diff --git a/docs/open-items.md b/docs/open-items.md index cc9fcc7..07ec5f4 100644 --- a/docs/open-items.md +++ b/docs/open-items.md @@ -1,9 +1,10 @@ # Open Items Running register of everything known to be unmet, unverified, misreported, or deliberately deferred across the -implemented portion of this project. Reviewed state: **scaffold milestone** (committed, `0ebdc79`) and +implemented portion of this project. Reviewed state: **scaffold milestone** (committed, `0ebdc79`), **Phase 1 — Core HTTP Domain Model** (branch `2-phase-1-core-http-domain-model`, uncommitted at time of -review). Last reviewed **2026-07-30**. +review), and **Phase 4a — Execution Context** (branch `7-phase-4a-execution-context`, three review passes). +Last reviewed **2026-08-26**. A requirement absent from this file is either satisfied or belongs to a phase that has not started. The point of the file is that nothing is unmet *silently* — every gap below is either scheduled against a named phase or @@ -84,6 +85,63 @@ add an import scan over `packages/core/src` allowing only relative specifiers an --- +### A5 — CTX-8: the duplicate-key error's *message* does not identify the key — **DECIDE** + +Appendix C states CTX-8 more strictly than `product-spec/07` §7.3 does. §7.3 says the reject-on-duplicate +insert "fails all others with an error naming the key"; appendix C +(`appendix-c-consolidated-normative-requirement-index.md:176`) says "an error **whose message** identifies the +key." + +`DuplicateContextKeyError`'s message is `` `context key already registered: ${String(key)}` ``. Call keys are +`Symbol()`s whose description is the flavor, not the identity, so every default-constructed context of a given +flavor renders identically: + +``` +context key already registered: Symbol(dispatch-context) +``` + +The message therefore names the *kind* of key, not *which* key. The error does carry the offending symbol as a +`readonly key: symbol` field — strictly more identifying than any string, and asserted in +`store.test.ts` — so the requirement's intent is met by the field while its letter is not met by the message. + +Phase 4a's design already ledgers the `Symbol()` key choice with the cost "debuggability (opaque when logged or +printed)", but that row does not connect itself to CTX-8's message clause, so nothing currently records this as +a known partial deviation. + +Two ways out, both defensible: +1. **Give default keys a distinguishing description** — `Symbol('dispatch-context#' + n)` from a module-scoped + counter. The counter would label only the description; `Symbol()` remains the identity, so CTX-4/5/6's + uniqueness is untouched and the ledger's rejection of a `traceId:spanId`+counter *string key* still stands. + Costs a second module-level mutable binding (`docs/knowledge/variables-and-declarations.md:22`), on top of + the `contextStore` singleton that already takes that deviation. +2. **Record a deliberate partial deviation** in the Phase 4a design's Deviation Ledger, on the grounds that a + symbol has no unique rendering and the typed `.key` field identifies the key more precisely than a message + can. + +Either way the Phase 4 checklist's CTX-8 row should stop reading as an unqualified ✅. + +### A6 — CTX-12 / XCUT-14: the drain **loop**'s shape is unverifiable, and untested — **WATCH** + +`ContextStore.#drain` is a post-insert loop, as CTX-12 (SHOULD) and XCUT-14 (MUST) require. No test proves it +is a loop, and none can: `install` and `installIfAbsent` each set exactly one key before draining, so the map +is never more than one over the cap at drain entry and a second pass is unreachable. Replacing the loop body +with a single check-then-evict breaks nothing — confirmed by mutation testing across the module (that mutant is +the only meaningful survivor of 23). + +The Phase 4a plan's Self-Review claims CTX-12 is covered by "a property-style burst test [that] asserts the +size never overshoots after any single insert". That test is real and passing, but it pins the **bound**, not +the drain's shape. + +Not a defect today: the loop is present, the bound holds, and on a single-threaded runtime the two shapes are +behaviorally identical. `#drain` and the drain `describe` block both now carry a note saying so, so the loop is +not "simplified" away by a later reader. + +**Trigger:** a runtime where inserts can stack more than one overshoot before a drain runs (worker threads, a +future concurrent store), or any change that lets the map exceed `cap + 1`. At that point the shape becomes +observable and owes a real test. + +--- + ## B. Gates and tooling ### B1 — NFR-10 / NFR-17: CI never runs on the declared minimum runtime — **ACT** (trigger has now fired) @@ -144,6 +202,21 @@ not deliberate reflection abuse (`Object.create(Request.prototype)`), and states `sdk-design-nodejs/10-deliberate-deviations-from-the-reference-contract.md` when that phase is reached." Listed here so the promise survives until then. +### C3 — The Phase 4 checklist under-reports Phase 4a — **ACT** + +`docs/superpowers/plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md` still carries its +banner: "the plans are reviewed and corrected as of 2026-07-26 but **not yet executed**. Every ✅ means 'the +plan builds and tests it,' not 'it is on `main`.'" Phase 4a's rows are now built, tested, and committed on +`7-phase-4a-execution-context`, so the banner understates them while 4b and 4c remain unbuilt. + +The same checklist maps only `CTX-*`. It has no `XCUT-14` row, even though +`docs/superpowers/specs/2026-07-28-phase9-cross-cutting-conformance-design.md:66` names "4a's context registry" +as an XCUT-14 site and appendix B's only conformance row that `ContextStore` satisfies is B.8's +"Caller/server-keyed maps bounded with drain-to-cap loop (XCUT-14)" — appendix B has no CTX section at all. The +ID is now cited in `store.ts` and `store.test.ts`; the checklist is the remaining gap. + +Split the banner per sub-phase, and add an `XCUT-14` row pointing at 4a Task 4 (qualified by A6 above). + --- ## D. Scheduled deferrals @@ -161,7 +234,11 @@ No action now. Each is already owned by a named phase; this table exists so none | Seam contracts (byte-stream, transport, codec, projection) | SEAM-2 – SEAM-30 | 2–8 | | | Adapter packages, peer-dependency dedup | NFR-2 | 8 | | | Shrink-survival regression guard | NFR-9 | 9 | | -| Concurrency-model agnosticism check | NFR-11 | 4 | No async code exists yet | +| Concurrency-model agnosticism check | NFR-11 | 4c | Retargeted from "Phase 4" by the 4a design: everything in 4a is synchronous, and 4c's stage pipeline is where async-facing surface appears | +| `CTX-17`'s positive half — the first store entry installed by the first promotion | CTX-17 | 4c | 4a satisfies only the negative half (constructing a head context must not auto-register it), which holds structurally because `context.ts` never imports `store.ts`. Wiring the store into the promotions would invert the layering and make every promotion a global side effect | +| Real W3C Trace Context generation behind `InstrumentationBundle` | CTX-14, CTX-15 | 7 | 4a ships the bundle's frozen shape and the no-op default only. `activeSpan`/`tracerFactory` stay typed `unknown`, and `activeSpan` is `undefined` rather than a no-op span object, until a tracing adapter defines `Span` | +| `contextsEqual()`, value equality over `ExecutionContext` | CTX-5 (equality framing) | none | Built only if 4b or 4c needs one. `CTX-5`'s operative half — pinning an explicit shared key — ships via `ContextInit.key` | +| `FakeTransport` test double | — | 4c | 4a never touches `Transport`; `PIPE-9`'s empty-pipeline dispatch is the likely first real consumer | | Self-identifying version metadata (real `User-Agent`) | NFR-15 | 7/8 | | | Publish + provenance CI job | NFR-16 | release | `prepublishOnly` wired; nothing published yet | | NFR-8 re-confirmed as a documented non-applicability | NFR-8 | 10 | No reflection-driven discovery surface exists by design | diff --git a/packages/core/src/context/context.test.ts b/packages/core/src/context/context.test.ts new file mode 100644 index 0000000..5cd576b --- /dev/null +++ b/packages/core/src/context/context.test.ts @@ -0,0 +1,261 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/context/context.test.ts +// Exercises: CTX-1 (one-way promotion, incl. the compile-time no-promote-back check), CTX-2 (additive, +// non-mutating, carries forward instrumentation + key), CTX-3 (one shared call key across the whole +// chain), CTX-5/CTX-6 (off-chain construction, fresh key per default call at population scale, explicit +// key pinning), CTX-7 (immutable), CTX-15 (keys stay call-unique though every bundle field is identical), +// CTX-16 (operationName absent at dispatch, introduced at request, carried forward, never keyed on) +import {describe, expect, test} from 'bun:test'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import { + type DispatchContext, + createDispatchContext, + createExchangeContext, + createRequestContext, + promoteToExchange, + promoteToRequest, +} from './context.js'; +import {noopInstrumentationBundle} from './instrumentation.js'; + +function aRequest(): Request { + return Request.newBuilder().url('https://example.com').build(); +} + +function aResponse(request: Request): Response { + return Response.newBuilder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .build(); +} + +describe('promotion chain (CTX-1, CTX-2, CTX-3)', () => { + test('dispatch exposes exactly its expected artifacts', () => { + const dispatch = createDispatchContext(); + expect(dispatch.kind).toBe('dispatch'); + expect(dispatch.key).toBeDefined(); + expect(dispatch.instrumentation).toBe(noopInstrumentationBundle); + }); + + test('promoting dispatch to request adds exactly the request, carrying key and instrumentation forward by reference', () => { + const dispatch = createDispatchContext(); + const request = aRequest(); + const requestCtx = promoteToRequest(dispatch, request, 'GetWidget'); + + expect(requestCtx.kind).toBe('request'); + expect(requestCtx.key).toBe(dispatch.key); + expect(requestCtx.instrumentation).toBe(dispatch.instrumentation); + expect(requestCtx.request).toBe(request); + expect(requestCtx.operationName).toBe('GetWidget'); + }); + + test('the source context is unchanged by promotion', () => { + const dispatch = createDispatchContext(); + const before = {...dispatch}; + promoteToRequest(dispatch, aRequest()); + expect(dispatch).toEqual(before); + }); + + test('promoting request to exchange adds exactly the response, carrying everything else forward', () => { + const request = aRequest(); + const requestCtx = promoteToRequest( + createDispatchContext(), + request, + 'GetWidget', + ); + const response = aResponse(request); + const exchangeCtx = promoteToExchange(requestCtx, response); + + expect(exchangeCtx.kind).toBe('exchange'); + expect(exchangeCtx.key).toBe(requestCtx.key); + expect(exchangeCtx.instrumentation).toBe(requestCtx.instrumentation); + expect(exchangeCtx.operationName).toBe('GetWidget'); + expect(exchangeCtx.request).toBe(request); + expect(exchangeCtx.response).toBe(response); + }); + + test('the whole chain shares one call key across all three flavors', () => { + const dispatch = createDispatchContext(); + const requestCtx = promoteToRequest(dispatch, aRequest()); + const exchangeCtx = promoteToExchange( + requestCtx, + aResponse(requestCtx.request), + ); + expect(requestCtx.key).toBe(dispatch.key); + expect(exchangeCtx.key).toBe(dispatch.key); + }); +}); + +describe('promotion is one-way (CTX-1)', () => { + test('no promotion function accepts an ExchangeContext, so there is no way back', () => { + const requestCtx = promoteToRequest(createDispatchContext(), aRequest()); + const exchangeCtx = promoteToExchange( + requestCtx, + aResponse(requestCtx.request), + ); + + // CTX-1's "the exchange type exposes no method promoting back" is a compile-time guarantee in this + // design, not a runtime one: promoteToRequest/promoteToExchange are free functions typed to accept + // only DispatchContext/RequestContext respectively, and there is no third promotion function. These + // two @ts-expect-error lines are the assertion -- `bun run typecheck` FAILS if either promotion ever + // widens to accept a terminal context, which a prose-only comment would not catch. + // @ts-expect-error -- ExchangeContext is terminal; it is not a DispatchContext + promoteToRequest(exchangeCtx, aRequest()); + // @ts-expect-error -- ExchangeContext is terminal; it is not a RequestContext + promoteToExchange(exchangeCtx, aResponse(requestCtx.request)); + + expect(exchangeCtx.kind).toBe('exchange'); + }); +}); + +describe('off-chain construction (CTX-5, CTX-6)', () => { + test('default construction mints a fresh, distinct key every call', () => { + const a = createDispatchContext(); + const b = createDispatchContext(); + expect(a.key).not.toBe(b.key); + }); + + test('N default-constructed contexts across all three flavors are pairwise key-distinct', () => { + // CTX-5's "globally distinct across the whole process and all three flavors" is a property over the + // whole population, not just a pair -- a keying scheme that collided every Nth call would pass the + // pairwise test above. Every bundle field is identical here (all use noopInstrumentationBundle), so + // this is also CTX-15's "call-key derivation MUST remain call-unique even when every bundle field is + // identical" at scale. + const request = aRequest(); + const keys = new Set(); + for (let i = 0; i < 1000; i += 1) { + keys.add(createDispatchContext().key); + keys.add(createRequestContext(request).key); + keys.add(createExchangeContext(request, aResponse(request)).key); + } + expect(keys.size).toBe(3000); + }); + + test('an explicit key can be pinned so two contexts share one slot', () => { + const key = Symbol('shared'); + const a = createDispatchContext({key}); + const b = createDispatchContext({key}); + expect(a.key).toBe(b.key); + }); + + test('an explicit instrumentation bundle is carried onto the context verbatim', () => { + const instrumentation = { + ...noopInstrumentationBundle, + traceId: 'a'.repeat(32), + isValid: true, + }; + expect(createDispatchContext({instrumentation}).instrumentation).toBe( + instrumentation, + ); + }); + + test('a caller-supplied instrumentation bundle is frozen by the factory (CTX-7)', () => { + // Object.freeze on the context is shallow, so without this the bundle behind `instrumentation` stays + // writable and the caller can mutate a "immutable" context out from under the whole chain. + const instrumentation = { + ...noopInstrumentationBundle, + traceId: 'a'.repeat(32), + }; + const dispatch = createDispatchContext({instrumentation}); + + expect(Object.isFrozen(dispatch.instrumentation)).toBe(true); + expect(Object.isFrozen(instrumentation)).toBe(true); // frozen in place, so the reference stays shared + }); + + test('createRequestContext and createExchangeContext also default to a fresh key per call', () => { + const request = aRequest(); + const a = createRequestContext(request); + const b = createRequestContext(request); + expect(a.key).not.toBe(b.key); + + const c = createExchangeContext(request, aResponse(request)); + const d = createExchangeContext(request, aResponse(request)); + expect(c.key).not.toBe(d.key); + }); +}); + +describe('operationName (CTX-16)', () => { + test('is absent at the dispatch stage', () => { + expect('operationName' in createDispatchContext()).toBe(false); + }); + + test('defaults to undefined when not supplied at promotion', () => { + const requestCtx = promoteToRequest(createDispatchContext(), aRequest()); + expect(requestCtx.operationName).toBeUndefined(); + }); + + test('is carried forward unchanged across the request-to-exchange promotion', () => { + const requestCtx = promoteToRequest( + createDispatchContext(), + aRequest(), + 'GetWidget', + ); + const exchangeCtx = promoteToExchange( + requestCtx, + aResponse(requestCtx.request), + ); + expect(exchangeCtx.operationName).toBe('GetWidget'); + }); + + test('is advisory only -- it never influences the call key', () => { + // CTX-16: "never influencing the request, dispatch decision, or store key." Two otherwise-identical + // promotions differing only in operationName keep their source keys; and pinning one key across two + // different operation names still yields one slot, proving the name is not folded into it. + const key = Symbol('shared'); + const a = promoteToRequest( + createDispatchContext({key}), + aRequest(), + 'GetWidget', + ); + const b = promoteToRequest( + createDispatchContext({key}), + aRequest(), + 'DeleteWidget', + ); + expect(a.key).toBe(b.key); + expect(a.operationName).not.toBe(b.operationName); + }); +}); + +describe('immutability (CTX-7)', () => { + test('a promotion freezes a bundle that never passed through a factory', () => { + // The context flavors are interfaces, not classes, so 4b/4c can hand a promotion a + // literal-constructed context whose bundle was never frozen. Without this the promoted context is + // "immutable" in name only: the caller keeps a writable reference to its trace state. + const instrumentation = { + ...noopInstrumentationBundle, + traceId: 'a'.repeat(32), + }; + const forged: DispatchContext = { + kind: 'dispatch', + key: Symbol('forged'), + instrumentation, + }; + + const requestCtx = promoteToRequest(forged, aRequest()); + + expect(Object.isFrozen(requestCtx.instrumentation)).toBe(true); + expect(requestCtx.instrumentation).toBe(instrumentation); // frozen in place -- CTX-2 still holds + expect( + Object.isFrozen( + promoteToExchange(requestCtx, aResponse(requestCtx.request)) + .instrumentation, + ), + ).toBe(true); + }); + + test('every context flavor is frozen', () => { + const dispatch = createDispatchContext(); + expect(Object.isFrozen(dispatch)).toBe(true); + const requestCtx = promoteToRequest(dispatch, aRequest()); + expect(Object.isFrozen(requestCtx)).toBe(true); + expect( + Object.isFrozen( + promoteToExchange(requestCtx, aResponse(requestCtx.request)), + ), + ).toBe(true); + }); +}); diff --git a/packages/core/src/context/context.ts b/packages/core/src/context/context.ts new file mode 100644 index 0000000..99f6ce2 --- /dev/null +++ b/packages/core/src/context/context.ts @@ -0,0 +1,198 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/context/context.ts +import type {Request} from '../http/request.js'; +import type {Response} from '../http/response.js'; +import { + noopInstrumentationBundle, + type InstrumentationBundle, +} from './instrumentation.js'; + +/** + * Before any request (CTX-1). No `operationName` — CTX-16 introduces it at the request stage. + * + * @internal + */ +export interface DispatchContext { + readonly kind: 'dispatch'; + readonly key: symbol; + readonly instrumentation: InstrumentationBundle; +} + +/** + * An outgoing request assembled (CTX-1). + * + * @internal + */ +export interface RequestContext { + readonly kind: 'request'; + readonly key: symbol; + readonly instrumentation: InstrumentationBundle; + readonly operationName: string | undefined; + readonly request: Request; +} + +/** + * A response arrived; terminal — no further promotion exists (CTX-1). + * + * @internal + */ +export interface ExchangeContext { + readonly kind: 'exchange'; + readonly key: symbol; + readonly instrumentation: InstrumentationBundle; + readonly operationName: string | undefined; + readonly request: Request; + readonly response: Response; +} + +/** + * The three promotion-chain stages as one discriminated union, branched on `kind`. + * + * @internal + */ +export type ExecutionContext = + DispatchContext | RequestContext | ExchangeContext; + +/** + * Optional inputs shared by the three off-chain `create*` factories. One options object rather than + * positional parameters: `createExchangeContext` would otherwise take five, and ESLint's `max-params` is 3 + * and counts optional parameters. Every field is spelled `?: T | undefined` for + * `exactOptionalPropertyTypes`. + * + * @internal + */ +export interface ContextInit { + /** Advisory operation label (CTX-16); never influences the request, dispatch, or store key. */ + readonly operationName?: string | undefined; + /** @defaultValue `noopInstrumentationBundle` */ + readonly instrumentation?: InstrumentationBundle | undefined; + /** + * Pin to make two contexts share one store slot (CTX-5). + * + * @defaultValue a fresh `Symbol()` per call + */ + readonly key?: symbol | undefined; +} + +/** + * Off-chain construction (CTX-5): `key` defaults to a fresh Symbol() per call unless pinned, which is also + * what makes default keys globally distinct across the process and all three flavors (CTX-6). Takes + * `Omit` — CTX-16 introduces the operation name at the request stage, so the + * dispatch factory does not offer it. + * + * @internal + */ +export function createDispatchContext( + init: Omit = {}, +): DispatchContext { + const { + instrumentation = noopInstrumentationBundle, + key = Symbol('dispatch-context'), + } = init; + return Object.freeze({ + kind: 'dispatch', + key, + instrumentation: freezeBundle(instrumentation), + }); +} + +/** + * Off-chain construction (CTX-5/6) — see `promoteToRequest` for the normal promotion path. + * + * @internal + */ +export function createRequestContext( + request: Request, + init: ContextInit = {}, +): RequestContext { + const { + operationName, + instrumentation = noopInstrumentationBundle, + key = Symbol('request-context'), + } = init; + return Object.freeze({ + kind: 'request', + key, + instrumentation: freezeBundle(instrumentation), + operationName, + request, + }); +} + +/** + * Off-chain construction (CTX-5/6) — see `promoteToExchange` for the normal promotion path. + * + * @internal + */ +export function createExchangeContext( + request: Request, + response: Response, + init: ContextInit = {}, +): ExchangeContext { + const { + operationName, + instrumentation = noopInstrumentationBundle, + key = Symbol('exchange-context'), + } = init; + return Object.freeze({ + kind: 'exchange', + key, + instrumentation: freezeBundle(instrumentation), + operationName, + request, + response, + }); +} + +/** + * dispatch -\> request (CTX-1/2/3): adds the request, carries key + instrumentation forward verbatim — + * `freezeBundle` is idempotent and freezes in place, so the bundle reference CTX-2 carries forward is + * unchanged; it is re-run because `DispatchContext` is an interface, so a caller can hand a + * literal-constructed context whose bundle never passed through a `create*` factory. + * + * @internal + */ +export function promoteToRequest( + context: DispatchContext, + request: Request, + operationName?: string, +): RequestContext { + return Object.freeze({ + kind: 'request', + key: context.key, + instrumentation: freezeBundle(context.instrumentation), + operationName, + request, + }); +} + +/** + * request -\> exchange (CTX-1/2/3): adds the response, carries everything else forward verbatim; the + * bundle is re-frozen for the same reason as `promoteToRequest`. + * + * @internal + */ +export function promoteToExchange( + context: RequestContext, + response: Response, +): ExchangeContext { + return Object.freeze({ + kind: 'exchange', + key: context.key, + instrumentation: freezeBundle(context.instrumentation), + operationName: context.operationName, + request: context.request, + response, + }); +} + +/** + * CTX-7: a context must be immutable, but `Object.freeze` on the context object is shallow, so a + * caller-supplied bundle would stay writable behind the `instrumentation` slot. Frozen in place rather than + * copied, so the reference the promotions carry forward (CTX-2) is the one the caller handed in. + * `noopInstrumentationBundle` is already frozen, so the default path costs nothing. Idempotent, which is + * what lets both the factories and the two promotions call it unconditionally. + */ +function freezeBundle(bundle: InstrumentationBundle): InstrumentationBundle { + return Object.isFrozen(bundle) ? bundle : Object.freeze(bundle); +} diff --git a/packages/core/src/context/errors.test.ts b/packages/core/src/context/errors.test.ts new file mode 100644 index 0000000..efea243 --- /dev/null +++ b/packages/core/src/context/errors.test.ts @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/context/errors.test.ts +// Exercises: CTX-8 (reject-on-duplicate insert failure, naming the key) +import {describe, expect, test} from 'bun:test'; +import {DexpaceError} from '../http/errors.js'; +import {DuplicateContextKeyError} from './errors.js'; + +describe('DuplicateContextKeyError', () => { + test('descends from DexpaceError and names the offending key', () => { + const key = Symbol('call-1'); + const error = new DuplicateContextKeyError(key); + expect(error).toBeInstanceOf(DexpaceError); + expect(error.key).toBe(key); + expect(error.message).toContain('call-1'); + }); + + test('sets name from its own constructor', () => { + expect(new DuplicateContextKeyError(Symbol('x')).name).toBe( + 'DuplicateContextKeyError', + ); + }); + + test('cause chains through', () => { + const cause = new Error('boom'); + expect(new DuplicateContextKeyError(Symbol('x'), {cause}).cause).toBe( + cause, + ); + }); +}); diff --git a/packages/core/src/context/errors.ts b/packages/core/src/context/errors.ts new file mode 100644 index 0000000..eacab55 --- /dev/null +++ b/packages/core/src/context/errors.ts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/context/errors.ts +import {DexpaceError} from '../http/errors.js'; + +/** + * `installIfAbsent` found the key already occupied (CTX-8). + * + * @internal + */ +export class DuplicateContextKeyError extends DexpaceError { + readonly key: symbol; + + constructor(key: symbol, options?: ErrorOptions) { + super(`context key already registered: ${String(key)}`, options); + this.key = key; + } +} diff --git a/packages/core/src/context/instrumentation.test.ts b/packages/core/src/context/instrumentation.test.ts new file mode 100644 index 0000000..acd5f3f --- /dev/null +++ b/packages/core/src/context/instrumentation.test.ts @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/context/instrumentation.test.ts +// Exercises: CTX-14 (bundle shape), CTX-15 (no-op default: invalid sentinels, isValid/isRemote false, +// no-op span/tracer factory), CTX-20 (tracer factory safe to invoke concurrently, emits nothing) +import {describe, expect, test} from 'bun:test'; +import {noopInstrumentationBundle} from './instrumentation.js'; + +describe('noopInstrumentationBundle (CTX-15)', () => { + test('reserves all-zero trace/span ids and zero flags', () => { + expect(noopInstrumentationBundle.traceId).toBe( + '00000000000000000000000000000000', + ); + expect(noopInstrumentationBundle.spanId).toBe('0000000000000000'); + expect(noopInstrumentationBundle.traceFlags).toBe(0); + expect(noopInstrumentationBundle.traceState).toBe(''); + }); + + test('names its trace-id encoding flavor', () => { + // CTX-14 requires the flavor field; CTX-15 fixes no sentinel for it, so the disabled bundle says + // 'none' rather than claiming an encoding it never produced ids in. + expect(noopInstrumentationBundle.traceIdEncoding).toBe('none'); + }); + + test('is invalid and not remote', () => { + expect(noopInstrumentationBundle.isValid).toBe(false); + expect(noopInstrumentationBundle.isRemote).toBe(false); + }); + + // CTX-15 says "a no-op span". With `activeSpan` typed `unknown` until a real tracing adapter lands + // (Phase 7), there is no Span shape to build a no-op instance of, so absence is the encoding. Logged as a + // partial deviation in the design's Deviation Ledger -- revisit when the adapter defines Span. + test('has no active span', () => { + expect(noopInstrumentationBundle.activeSpan).toBeUndefined(); + }); + + test('tracerFactory emits nothing and is safe to invoke repeatedly (CTX-20)', () => { + expect(noopInstrumentationBundle.tracerFactory('op-a')).toBeUndefined(); + expect(noopInstrumentationBundle.tracerFactory('op-b')).toBeUndefined(); + }); + + test('is frozen', () => { + expect(Object.isFrozen(noopInstrumentationBundle)).toBe(true); + }); +}); diff --git a/packages/core/src/context/instrumentation.ts b/packages/core/src/context/instrumentation.ts new file mode 100644 index 0000000..8a14524 --- /dev/null +++ b/packages/core/src/context/instrumentation.ts @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/context/instrumentation.ts + +/** + * Correlation/instrumentation bundle every execution context carries (CTX-14). `activeSpan` and + * `tracerFactory` are typed `unknown` rather than a Span/Tracer interface — nothing in this phase + * consumes either, and a real tracing adapter (deferred to Phase 7) owns their eventual shape. + * + * @internal + */ +export interface InstrumentationBundle { + readonly traceId: string; + readonly spanId: string; + readonly traceFlags: number; + readonly traceState: string; + readonly traceIdEncoding: string; + readonly isValid: boolean; + readonly isRemote: boolean; + readonly activeSpan: unknown; + readonly tracerFactory: (operationName: string) => unknown; +} + +/** + * The disabled-tracing default (CTX-15): reserved invalid sentinels, no-op span and tracer factory. Every + * field is constant, so call-key uniqueness (CTX-4) must not depend on any of them — see `context.ts`'s + * `Symbol()`-based keys. + * + * @internal + */ +export const noopInstrumentationBundle: InstrumentationBundle = Object.freeze({ + traceId: '00000000000000000000000000000000', + spanId: '0000000000000000', + traceFlags: 0, + traceState: '', + traceIdEncoding: 'none', + isValid: false, + isRemote: false, + activeSpan: undefined, + tracerFactory: () => undefined, +}); diff --git a/packages/core/src/context/store.test.ts b/packages/core/src/context/store.test.ts new file mode 100644 index 0000000..f040c9f --- /dev/null +++ b/packages/core/src/context/store.test.ts @@ -0,0 +1,269 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/context/store.test.ts +// Exercises: CTX-3 (all three flavors collapse to one slot, successive promotions overwriting it), +// CTX-4 (two contexts sharing identical trace AND span id get distinct keys and both +// register), CTX-8 (install-or-replace never throws; reject-on-duplicate fails naming the key), +// CTX-9/CTX-10 (identity-conditional close, intermediate-link close is a no-op), CTX-11/CTX-12 (bounded, +// post-insert drain loop), CTX-17 (a never-promoted dispatch context leaves no entry; its close is a +// harmless no-op), CTX-13 (arbitrary victim; no entry is promised to survive), CTX-18 (unknown-key +// lookup/close are well-defined no-ops), CTX-19 (strong refs), +// XCUT-14 (a caller-keyed process-lived map -- "context registries" is the requirement's own first +// example -- carries a hard cap and a post-insert drain loop, and a burst never leaves it stuck above) +// +// Every test builds its own `new ContextStore()`. The exported `contextStore` singleton is module-level +// mutable state shared by every test file in a `bun test` run -- 4c's runtime.test.ts installs into that +// same object -- so an absolute `size` assertion against it reads a counter a sibling file can move, and a +// blanket clear() wipes a sibling's entries. docs/knowledge/testing.md:50,52. The singleton gets exactly +// one assertion here: that it is a ContextStore. +import {describe, expect, test} from 'bun:test'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {InvariantViolation} from '../invariant.js'; +import { + createDispatchContext, + promoteToExchange, + promoteToRequest, +} from './context.js'; +import {DuplicateContextKeyError} from './errors.js'; +import {ContextStore, contextStore} from './store.js'; + +function aRequest(): Request { + return Request.newBuilder().url('https://example.com').build(); +} + +function aResponse(request: Request): Response { + return Response.newBuilder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .build(); +} + +describe('install / installIfAbsent (CTX-8)', () => { + test('install never throws and is retrievable by key', () => { + const store = new ContextStore(); + const context = createDispatchContext(); + store.install(context); + expect(store.get(context.key)).toBe(context); + }); + + test('install unconditionally overwrites an existing occupant', () => { + const store = new ContextStore(); + const context = createDispatchContext(); + store.install(context); + const promoted = promoteToRequest(context, aRequest()); + store.install(promoted); + expect(store.get(context.key)).toBe(promoted); + }); + + test('installIfAbsent succeeds when the key is free', () => { + const store = new ContextStore(); + const context = createDispatchContext(); + store.installIfAbsent(context); + expect(store.get(context.key)).toBe(context); + }); + + test('installIfAbsent on an occupied key throws DuplicateContextKeyError naming the key', () => { + const store = new ContextStore(); + const context = createDispatchContext(); + store.installIfAbsent(context); + const other = createDispatchContext({ + instrumentation: context.instrumentation, + key: context.key, + }); + + let caught: unknown; + try { + store.installIfAbsent(other); + } catch (error) { + caught = error; + } + + // CTX-8 says the error names the key, so assert the field, not only the class -- the store passing + // the wrong symbol through would otherwise be invisible here, and a symbol does not survive the + // message-substring check the rest of the suite uses for named-field errors. + expect(caught).toBeInstanceOf(DuplicateContextKeyError); + expect((caught as DuplicateContextKeyError).key).toBe(context.key); + }); + + test('a rejected installIfAbsent leaves the incumbent in the slot', () => { + // CTX-8's "admits exactly one winner": the loser must not have displaced or corrupted the winner. + const store = new ContextStore(); + const winner = createDispatchContext(); + store.installIfAbsent(winner); + const loser = createDispatchContext({key: winner.key}); + + expect(() => { + store.installIfAbsent(loser); + }).toThrow(DuplicateContextKeyError); + + expect(store.get(winner.key)).toBe(winner); + expect(store.size).toBe(1); + }); +}); + +describe('call-key uniqueness under an identical bundle (CTX-4)', () => { + test('two contexts sharing identical trace AND span id get distinct keys and both register', () => { + // §7's own Conformance clause for CTX-4, transcribed. Both contexts carry the very same + // noopInstrumentationBundle -- identical traceId, spanId, flags, state -- which is exactly the + // disabled-tracing case CTX-15 warns about. Symbol() keys make them distinct anyway, so neither + // evicts the other. + const store = new ContextStore(); + const a = createDispatchContext(); + const b = createDispatchContext(); + expect(a.instrumentation).toBe(b.instrumentation); + expect(a.key).not.toBe(b.key); + + store.install(a); + store.install(b); + expect(store.get(a.key)).toBe(a); + expect(store.get(b.key)).toBe(b); + expect(store.size).toBe(2); + }); +}); + +describe('one slot for the whole chain (CTX-3)', () => { + test('all three flavors register under the identical slot, each promotion overwriting the last', () => { + // CTX-3's store-level clause: "all three flavors register under the identical store slot and + // successive promotions overwrite one entry." Asserted here rather than in context.test.ts, which + // can only show the keys match -- that they collapse to ONE entry needs a store. + const store = new ContextStore(); + const dispatch = createDispatchContext(); + const request = aRequest(); + const requestCtx = promoteToRequest(dispatch, request, 'GetWidget'); + const exchangeCtx = promoteToExchange(requestCtx, aResponse(request)); + + store.install(dispatch); + store.install(requestCtx); + store.install(exchangeCtx); + + expect(store.size).toBe(1); + expect(store.get(dispatch.key)).toBe(exchangeCtx); + }); +}); + +describe('no auto-registration at construction (CTX-17)', () => { + test('a freshly constructed dispatch context is not in the store', () => { + const store = new ContextStore(); + const context = createDispatchContext(); + expect(store.get(context.key)).toBeUndefined(); + expect(store.size).toBe(0); + }); + + test('promoting registers nothing either, and closing the unregistered source is a harmless no-op', () => { + const store = new ContextStore(); + const context = createDispatchContext(); + promoteToRequest(context, aRequest()); // promotion alone registers nothing in 4a -- see below + expect(store.size).toBe(0); + expect(() => { + store.close(context); + }).not.toThrow(); + }); + + // CTX-17's other half -- "the first store entry is installed by the first promotion" -- is NOT + // satisfied here: promoteToRequest/promoteToExchange are pure and never touch the store, so an + // explicit store.install(...) is what registers anything. That call belongs to 4c's pipeline, + // which owns the store handle. Tracked as a deferral in this plan's Self-Review, not an omission. +}); + +describe('close (CTX-9, CTX-10)', () => { + test('evicts when the closing context is the current occupant', () => { + const store = new ContextStore(); + const context = createDispatchContext(); + store.install(context); + store.close(context); + expect(store.get(context.key)).toBeUndefined(); + }); + + test('closing an intermediate link already superseded by promotion is a no-op', () => { + const store = new ContextStore(); + const dispatch = createDispatchContext(); + store.install(dispatch); + const promoted = promoteToRequest(dispatch, aRequest()); + store.install(promoted); // furthest-reached link now occupies the slot + + store.close(dispatch); // intermediate link -- must not evict the live promoted occupant + expect(store.get(dispatch.key)).toBe(promoted); + }); + + test('closing an unknown or already-removed key is a well-defined no-op (CTX-18)', () => { + const store = new ContextStore(); + const context = createDispatchContext(); + expect(() => { + store.close(context); + }).not.toThrow(); + store.install(context); + store.close(context); + expect(() => { + store.close(context); + }).not.toThrow(); + }); +}); + +describe('lookup (CTX-18)', () => { + test('an unknown key returns undefined, never throws', () => { + expect(new ContextStore().get(Symbol('unknown'))).toBeUndefined(); + }); +}); + +describe('bounded drain (CTX-11, CTX-12, CTX-13)', () => { + // These pin the BOUND, not the drain's shape. `install`/`installIfAbsent` each set one key before + // draining, so the map is never more than one over the cap and a single check-then-evict would pass + // every assertion here -- verified by mutation. CTX-12/XCUT-14's loop is retained for runtimes where + // concurrent inserts stack overshoots; see the note on `#drain`. + + test('a burst of inserts past the cap converges the store to at or under the cap', () => { + const store = new ContextStore(5); + for (let i = 0; i < 50; i += 1) { + store.install(createDispatchContext()); + expect(store.size).toBeLessThanOrEqual(5); // drains after every single insert, never overshoots + } + // Negative space: bounding only from above would also pass for a store that retained nothing at all. + // 50 distinct keys against a cap of 5 must leave the store saturated, not empty. + expect(store.size).toBe(5); + }); + + test('installIfAbsent also drains after a successful insert', () => { + const store = new ContextStore(2); + for (let i = 0; i < 10; i += 1) { + store.installIfAbsent(createDispatchContext()); + } + expect(store.size).toBe(2); + }); + + test('a cap below 1 is rejected at construction', () => { + // The constructor is the only place this is checked, which is what lets #drain skip an unreachable + // in-loop undefined guard. A bad cap is a violated precondition -- a programmer error -- so it fails + // through invariant (assertions.md:4, error-handling.md:36), not an ad-hoc throw. + expect(() => new ContextStore(0)).toThrow(InvariantViolation); + expect(() => new ContextStore(-1)).toThrow(InvariantViolation); + expect(() => new ContextStore(1.5)).toThrow(InvariantViolation); + expect(() => new ContextStore(1)).not.toThrow(); + }); +}); + +describe('clear', () => { + test('drops every entry, leaving the store reusable', () => { + const store = new ContextStore(); + const kept = createDispatchContext(); + store.install(kept); + store.install(createDispatchContext()); + + store.clear(); + + expect(store.size).toBe(0); + expect(store.get(kept.key)).toBeUndefined(); + store.install(kept); // still usable afterwards -- clear() resets entries, not the cap + expect(store.get(kept.key)).toBe(kept); + }); +}); + +describe('the process-wide singleton', () => { + test('is a real ContextStore instance', () => { + // The only assertion this file makes against the singleton: it is shared with every other test file + // in the run, so nothing behavioural may be asserted through it. + expect(contextStore).toBeInstanceOf(ContextStore); + }); +}); diff --git a/packages/core/src/context/store.ts b/packages/core/src/context/store.ts new file mode 100644 index 0000000..54e103f --- /dev/null +++ b/packages/core/src/context/store.ts @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/context/store.ts +import {invariant} from '../invariant.js'; +import type {ExecutionContext} from './context.js'; +import {DuplicateContextKeyError} from './errors.js'; + +// Backstop cap (CTX-11, XCUT-14); a leaked context pins its whole request/response graph, including a +// possibly unread body holding a connection. +const DEFAULT_MAX_ENTRIES = 10_000; + +/** + * A bounded, keyed store of in-flight execution contexts (CTX-7..13, CTX-18, CTX-19). Also the textbook + * subject of `XCUT-14`, which names "context registries" first among the caller-keyed process-lived maps + * that MUST carry a hard cap and drain back under it in a loop after each insert — an unbounded one is a + * memory-exhaustion vector, not merely a leak. Thread-safety is + * satisfied by construction: Node's single-threaded event loop means no two synchronous Map mutations + * ever interleave, collapsing the reference's concurrent-map requirement into a plain Map. The Map holds + * strong references — never WeakRef/WeakMap — so a registered context keeps its whole Request+Response + * graph reachable and the cap, not the collector, is the leak backstop (CTX-19). + * + * @internal + */ +export class ContextStore { + readonly #entries = new Map(); + readonly #maxEntries: number; + + /** + * @throws InvariantViolation when `maxEntries` is not a positive integer — a violated precondition, + * never an operational failure a caller recovers from. + */ + constructor(maxEntries: number = DEFAULT_MAX_ENTRIES) { + // A bad cap is a violated precondition — a programmer error — so it crashes at the fault via the + // project's one assertion primitive rather than an ad-hoc `if (!x) throw` + // (docs/knowledge/assertions.md:4, docs/knowledge/error-handling.md:36). + invariant( + Number.isInteger(maxEntries) && maxEntries >= 1, + `maxEntries must be a positive integer, got ${String(maxEntries)}`, + ); + this.#maxEntries = maxEntries; + } + + /** + * Install-or-replace; never throws (CTX-8). Nothing in 4a calls this — the promotion functions are + * pure and never touch a store (CTX-17's negative half); 4c's pipeline is the first caller. + */ + install(context: ExecutionContext): void { + this.#entries.set(context.key, context); + this.#drain(); + invariant( + this.#entries.size <= this.#maxEntries, + 'context store above its cap after a drain', + ); + } + + /** + * Install only if absent; every other concurrent caller fails (CTX-8). + * + * @throws DuplicateContextKeyError when the key is already occupied. The error carries the offending + * `key` as a field — the symbol itself, not just its rendering in the message. + */ + installIfAbsent(context: ExecutionContext): void { + if (this.#entries.has(context.key)) { + throw new DuplicateContextKeyError(context.key); + } + this.#entries.set(context.key, context); + this.#drain(); + invariant( + this.#entries.size <= this.#maxEntries, + 'context store above its cap after a drain', + ); + } + + /** Absent key returns undefined, never throws (CTX-18). */ + get(key: symbol): ExecutionContext | undefined { + return this.#entries.get(key); + } + + /** + * Evicts the slot only when the current occupant IS `context` (reference identity, CTX-9). Closing an + * intermediate link already superseded by a later promotion, or an unknown/already-removed key, is a + * well-defined no-op (CTX-10, CTX-18). + */ + close(context: ExecutionContext): void { + if (this.#entries.get(context.key) === context) { + this.#entries.delete(context.key); + } + } + + /** + * Drops every entry. Not part of `§7`'s contract — it exists so a test that must observe the shared + * singleton (4c's runtime tests) can reset it. Prefer constructing an isolated `ContextStore`. + */ + clear(): void { + this.#entries.clear(); + } + + /** Entries currently tracked; at or below the cap once inserts quiesce (CTX-11, CTX-13). */ + get size(): number { + return this.#entries.size; + } + + #drain(): void { + // CTX-12 / XCUT-14: a loop, not a single check-then-evict, so an insert burst converges to the cap. + // + // DO NOT "simplify" this loop into an `if`. On this runtime the two are behaviorally identical and + // no test can tell them apart: both callers set exactly one key before draining, so the map is never + // more than one over the cap at entry and the loop never needs a second pass. The loop survives + // because CTX-12 and XCUT-14 mandate the shape for runtimes where concurrent inserts can stack + // several overshoots before any drain runs — the burst test below pins the bound, not the shape. + // + // CTX-13: victim selection is arbitrary — oldest-inserted (Map iteration order) is the cheapest + // choice, not a retention promise; callers must not rely on any particular entry surviving. + // + // No undefined-guard inside the loop: the constructor rejects maxEntries < 1, so `size > maxEntries` + // proves size >= 2 and the iterator always yields. A guard here would be unreachable code the + // coverage gate could never exercise. + for (const oldestKey of this.#entries.keys()) { + if (this.#entries.size <= this.#maxEntries) return; + this.#entries.delete(oldestKey); + } + } +} + +/** + * The one registry 4c's `Runtime.send()` installs into. Module-level mutable state, which + * `docs/knowledge/variables-and-declarations.md:22` bans — accepted here because threading a store handle + * through builder → runtime → every step would be a wide API change for no observable gain, and logged in + * the design's Deviation Ledger for Phase 10. Tests must build their own `new ContextStore()` rather than + * asserting through this one: it is shared by every test file in a `bun test` run. + * + * @internal + */ +export const contextStore = new ContextStore(); From 3b7eb4cefbafc8b376bbf45c3e323785cb6c178a Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Wed, 26 Aug 2026 19:38:26 +0300 Subject: [PATCH 2/3] =?UTF-8?q?feat(core):=20recovery-chain=20primitives?= =?UTF-8?q?=20=E2=80=94=20product-spec=20=C2=A78.2=20(RECOV-1..RECOV-16)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4b. Ships `packages/core/src/recovery/` — six files, no folder barrel, nothing on the published API surface (`packages/core/etc/core.api.md` is byte-identical): - `outcome.ts` Outcome, success/failure/fold (RECOV-1) - `request-chain.ts` sequential fold, empty = identity, throw propagates for the orchestrator to convert (RECOV-3, 14) - `response-chain.ts` response phase on Success only, recovery phase always, close-on-throw exactly once with the original throwable primary, no auto-close on a deliberately returned substitute (RECOV-4..9, 12..14) - `cancellation.ts` wrapCancellation — never throws, which is what keeps RECOV-2 absolute (RECOV-11) - `status-mapping.ts` a thin response step over 3b's unchanged toHttpError() (RECOV-15, 16) - `orchestrator.ts` dispatchWithRecovery — one try/catch over the request chain and the transport hop; the final unwrap rethrows by identity (RECOV-2, 10) Plus two package-root helpers: `assertNever` in `invariant.ts` (the codebase's first discriminated-union `default`) and `suppress()` in `suppress.ts`. F1, the cross-phase blocker, resolved to branch (b) --------------------------------------------------- RECOV-12 pairs a step's throwable with a close failure, which is what `SuppressedError` is for — and `SuppressedError` reached Node only in 24.0.0, against this package's `engines.node >=20.3` floor (set by `AbortSignal.any()`), and is absent from the `lib` it compiles against, so the direct form neither type-checks nor runs there. Raising the floor would drop Node 18, 20 and 22 for one error class. `suppress()` uses the native class where the runtime has one and a shape-compatible stand-in where it does not, reading the global per call. Callers assert the shape, never `instanceof SuppressedError` — that form would silently assert nothing on the floor. Phases 5a, 6a, 6b and 6c reached for the native class on the same premise; their docs now point at the helper. F2 resolved as a Deviation Ledger row: the zero-vs-fifteen `invariant()` split with 4c is project-wide, so Phase 10 settles the density rule once. Three review passes ------------------- Pass 1, against the knowledge corpus: a dead `statusMappingStep;` statement reaching the published dist/ (`satisfies` erases to its operand, not to nothing); two test files that could not survive parallel execution because they deleted a global; no type-level test for the exported generic `Outcome`; the RECOV-15 conformance clause tested on the step in isolation rather than through the chain; two step-down-rule violations. Pass 2, against the normative text: **a RECOV-8 violation** — `apply()` could raise `TypeError: undefined is not an object` when a step returned a non-outcome, against "MUST NOT throw under any input". `toFailureClosingSuccess` is now total: the discriminant read and the `close()` call share one `try`, so a misbehaving step becomes a Failure with its own throwable still primary. Also an unguarded `String()` in `assertNever`'s default message, which throws on a null-prototype object. Pass 3: re-ran every step of both CI jobs, swept the structure, and wrote what survives into `docs/open-items.md` section F. Also fixes a merge residue: the phase-3 merge left `bunfig.toml` with a duplicated `[test] root` key, which TOML rejects, so `bun test` failed to load bunfig at all on this branch. Verification (all exit 0) ------------------------- `bun install --frozen-lockfile`, typecheck, lint, build, `bun test --coverage` (588 tests / 50 files, 98.68% funcs / 99.73% lines against the 80% floor), api, lint:publish, verify:dual-consumption, verify:consumer-types, verify:seam-1, verify:runtime-floor, audit, test:node (36 cases, +1 file covering the SuppressedError guard and RECOV-12 over Node's own Web Streams), test:knowledge. No `node:` import, no `enum`, no internal barrel, SPDX on line 1 of all 15 new files, no import cycle under `packages/core/src`. Refs: dexpace/nodejs-sdk#8 --- .../2026-08-26-recovery-chain-primitives.md | 24 + bunfig.toml | 6 +- docs/open-items.md | 123 ++++- .../2026-07-25-phase4b-recovery-chain.md | 214 +++++---- ...ecution-context-and-pipelines-checklist.md | 36 +- .../plans/2026-07-26-phase5a-retry.md | 12 +- .../plans/2026-07-28-phase6b-sse.md | 26 +- ...2026-07-23-nodejs-sdk-v1-roadmap-design.md | 22 +- ...026-07-25-phase4b-recovery-chain-design.md | 49 +- .../specs/2026-07-28-phase6a-serde-design.md | 10 +- .../specs/2026-07-28-phase6b-sse-design.md | 8 +- .../2026-07-28-phase6c-pagination-design.md | 6 +- packages/core/src/invariant.test.ts | 43 +- packages/core/src/invariant.ts | 29 ++ .../core/src/recovery/cancellation.test.ts | 43 ++ packages/core/src/recovery/cancellation.ts | 33 ++ .../core/src/recovery/orchestrator.test.ts | 370 +++++++++++++++ packages/core/src/recovery/orchestrator.ts | 80 ++++ packages/core/src/recovery/outcome.test.ts | 157 +++++++ packages/core/src/recovery/outcome.ts | 72 +++ .../core/src/recovery/request-chain.test.ts | 152 +++++++ packages/core/src/recovery/request-chain.ts | 53 +++ .../core/src/recovery/response-chain.test.ts | 426 ++++++++++++++++++ packages/core/src/recovery/response-chain.ts | 147 ++++++ .../core/src/recovery/status-mapping.test.ts | 154 +++++++ packages/core/src/recovery/status-mapping.ts | 41 ++ packages/core/src/suppress.test.ts | 85 ++++ packages/core/src/suppress.ts | 92 ++++ test/node-conformance/recovery-chain.test.mjs | 159 +++++++ 29 files changed, 2524 insertions(+), 148 deletions(-) create mode 100644 .changeset/2026-08-26-recovery-chain-primitives.md create mode 100644 packages/core/src/recovery/cancellation.test.ts create mode 100644 packages/core/src/recovery/cancellation.ts create mode 100644 packages/core/src/recovery/orchestrator.test.ts create mode 100644 packages/core/src/recovery/orchestrator.ts create mode 100644 packages/core/src/recovery/outcome.test.ts create mode 100644 packages/core/src/recovery/outcome.ts create mode 100644 packages/core/src/recovery/request-chain.test.ts create mode 100644 packages/core/src/recovery/request-chain.ts create mode 100644 packages/core/src/recovery/response-chain.test.ts create mode 100644 packages/core/src/recovery/response-chain.ts create mode 100644 packages/core/src/recovery/status-mapping.test.ts create mode 100644 packages/core/src/recovery/status-mapping.ts create mode 100644 packages/core/src/suppress.test.ts create mode 100644 packages/core/src/suppress.ts create mode 100644 test/node-conformance/recovery-chain.test.mjs diff --git a/.changeset/2026-08-26-recovery-chain-primitives.md b/.changeset/2026-08-26-recovery-chain-primitives.md new file mode 100644 index 0000000..a444a15 --- /dev/null +++ b/.changeset/2026-08-26-recovery-chain-primitives.md @@ -0,0 +1,24 @@ +--- +'@dexpace/core': patch +--- + +Add the recovery-chain primitives for product-spec §8.2 (`RECOV-1`–`RECOV-16`). No public API change. + +Everything this adds lives under `packages/core/src/recovery/` plus two package-root helpers, and none of it is +re-exported from `src/index.ts` — `packages/core/etc/core.api.md` is byte-identical before and after. `patch` +rather than an empty changeset because files under `packages/` did change: the published tarball carries the +new `dist/recovery/*.js` and `dist/suppress.js`, and a consumer stepping through the package in a debugger will +see them. + +What landed: `Outcome` with `success`/`failure`/`fold`; `RequestRecoveryChain` and `ResponseRecoveryChain` +(defensive copies on both, concurrency-safe by construction); `dispatchWithRecovery`, whose single `try`/`catch` +wraps both the request chain and the transport hop so no throwable from either can bypass the recovery hooks; +`wrapCancellation`; and `statusMappingStep`, a thin response step over Phase 3b's unchanged `toHttpError()`. +`assertNever` joins `invariant.ts` as the codebase's first discriminated-union `default` case. + +One consumer-visible-in-principle detail worth recording: `RECOV-12` pairs a step's throwable with a close +failure, which is what `SuppressedError` is for — and `SuppressedError` reached Node only in 24.0.0, against +this package's `>=20.3` floor. Rather than raise the floor and drop Node 18, 20 and 22 for one error class, +`suppress()` uses the native class where the runtime has one and returns a shape-compatible stand-in (`name`, +`error`, `suppressed`) where it does not. Code that catches one of these should read its fields, not test +`instanceof SuppressedError`. diff --git a/bunfig.toml b/bunfig.toml index dbf396f..956ed24 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -2,9 +2,9 @@ # Scope discovery to the workspace packages. Without this, `bun test` also collects # test/node-conformance/*.test.mjs -- the Node-only layer that exists precisely because it must NOT # run on Bun (checkpoint 5.9). Running it under both runners would inflate the unit count and quietly -# erase the distinction the suite was added to draw. It also keeps `scripts/*.test.mjs` — repo -# tooling, run via `bun run test:knowledge` (`node --test`) — out of both the run and the 80% floor, -# which is a statement about `packages/core`. +# erase the distinction the suite was added to draw. It likewise keeps `scripts/*.test.mjs` -- repo +# tooling, run via `bun run test:knowledge` (`node --test`) -- out of both the run and the coverage +# floor, which is a statement about `packages/core`. root = "packages" coverage = true coverageThreshold = 0.8 diff --git a/docs/open-items.md b/docs/open-items.md index 07ec5f4..1f8d44d 100644 --- a/docs/open-items.md +++ b/docs/open-items.md @@ -3,9 +3,13 @@ Running register of everything known to be unmet, unverified, misreported, or deliberately deferred across the implemented portion of this project. Reviewed state: **scaffold milestone** (committed, `0ebdc79`), **Phase 1 — Core HTTP Domain Model** (branch `2-phase-1-core-http-domain-model`, uncommitted at time of -review), and **Phase 4a — Execution Context** (branch `7-phase-4a-execution-context`, three review passes). +review), **Phase 3a/3b**, **Phase 4a — Execution Context** (branch `7-phase-4a-execution-context`, three +review passes), and **Phase 4b — Recovery-Chain Primitives** (branch +`8-phase-4b-recovery-chain-primitives`). 4a and 4b are both merged into `9-phase-4c-stage-based-pipeline`. Last reviewed **2026-08-26**. +Sections A–E below were written against Phase 1 and are re-verified at each review; section F is Phase 4b's. + A requirement absent from this file is either satisfied or belongs to a phase that has not started. The point of the file is that nothing is unmet *silently* — every gap below is either scheduled against a named phase or awaiting a decision. @@ -144,7 +148,13 @@ observable and owes a real test. ## B. Gates and tooling -### B1 — NFR-10 / NFR-17: CI never runs on the declared minimum runtime — **ACT** (trigger has now fired) +### B1 — NFR-10 / NFR-17: CI never runs on the declared minimum runtime — **RESOLVED** (2026-08-26) + +Closed by the `node-conformance` job in `.github/workflows/ci.yml`, which runs `test:node` against the built +artifact as a matrix over `['20.3.0', 'lts/*']` — the declared floor and current LTS. `test/node-conformance/` +holds 36 cases. Re-verified 2026-08-26. Original finding kept below for provenance. + +#### Original finding — **ACT** (trigger has now fired) The scaffold checklist deferred this explicitly: *"recommend adding an `actions/setup-node@v4` step pinned to `18.17` running `scripts/verify-dual-consumption.mjs` once real Node-API usage lands (Phase 1 onward), rather @@ -161,13 +171,14 @@ run" is still missing. Phase 1 established the convention ("every new source file opens with `// SPDX-License-Identifier: MIT` on line 1") and every file under `packages/core/src/http/` complies. Three files predating it do not: -- `scripts/verify-runtime-floor.mjs` -- `scripts/verify-seam-1.mjs` -- `eslint.config.js` +- ~~`scripts/verify-runtime-floor.mjs`~~ — fixed +- ~~`scripts/verify-seam-1.mjs`~~ — fixed +- `eslint.config.js` — **still missing** (re-verified 2026-08-26; line 1 is + `import {createRequire} from 'node:module';`) `scripts/verify-dual-consumption.mjs` gained one during Phase 1, which is what makes the omission of its two siblings look accidental rather than scoped. NFR-13 is a review convention, not a mechanical gate, so this is a -one-line-per-file cleanup. +one-line cleanup on the one file left. Phase 9's `NFR-13` sweep owns it if it is not done sooner. ### B3 — NFR-12: reproducible builds asserted, never proven — **WATCH** @@ -177,7 +188,8 @@ it. Becomes real at first publish (~Phase 10): build twice, diff artifact digest ### B4 — NFR-14: `expect-type` breaks the single-source-of-versions convention — **WATCH** Every other devDependency is centralized at the workspace root; Phase 1 added `expect-type` to -`packages/core/package.json`'s own `devDependencies`. Harmless with one package — it is exactly the restatement +`packages/core/package.json`'s own `devDependencies` (re-verified 2026-08-26 — still there, and Phase 4b added +three more call sites, so the convention is now load-bearing in four files rather than two). Harmless with one package — it is exactly the restatement NFR-14 warns about once a second package exists (Phase 8). Either hoist it to the root now or fold it into the NFR-14 decision at Phase 8. @@ -225,16 +237,16 @@ No action now. Each is already owned by a named phase; this table exists so none | Item | Requirement | Owner phase | Note | |---|---|---|---| -| Body lifecycle: write/replayability, single-use, close, charset | HTTP-36 – HTTP-43 | 3b | `Request`/`Response` `body` is typed `unknown` as an explicit placeholder | -| Lazy `TypedResponse` with parse-once memoization | HTTP-44, HTTP-45 | 3b | | -| `MultipartBody` — the one builder-based model HTTP-3 lists that Phase 1 did not build | HTTP-51 | 3b | Depends on body-lifecycle contracts | -| 1 MiB error-body buffering cap | HTTP-52 | 3b | | +| ~~Body lifecycle: write/replayability, single-use, close, charset~~ | HTTP-36 – HTTP-43 | 3b | **Done** — `packages/core/src/body/`, merged 2026-08-26 | +| ~~Lazy `TypedResponse` with parse-once memoization~~ | HTTP-44, HTTP-45 | 3b | **Done** — `body/typed-response.ts` | +| ~~`MultipartBody`~~ | HTTP-51 | 3b | **Done** — `body/multipart-body.ts`. Its non-appearance clause stays partial; see the roadmap's Phase-3-owned residuals | +| ~~1 MiB error-body buffering cap~~ | HTTP-52 | 3b | **Done** — `body/http-status-error.ts`; `RECOV-16` reuses it unchanged | | `Request.equals` compares body by reference, not by value | HTTP-46 (body clause) | 3b | Blocked on a real `Body` model supplying value equality | | `RequestConditions.applyTo` cannot emit an obs-text ETag | HTTP-18 vs HTTP-48/50 | 10 | Spec text in scope does not resolve the tension; strict outbound path kept rather than guessed. Documented in `applyTo`'s TSDoc | | Seam contracts (byte-stream, transport, codec, projection) | SEAM-2 – SEAM-30 | 2–8 | | | Adapter packages, peer-dependency dedup | NFR-2 | 8 | | | Shrink-survival regression guard | NFR-9 | 9 | | -| Concurrency-model agnosticism check | NFR-11 | 4c | Retargeted from "Phase 4" by the 4a design: everything in 4a is synchronous, and 4c's stage pipeline is where async-facing surface appears | +| Concurrency-model agnosticism check | NFR-11 | 4c | Retargeted from "Phase 4" by the 4a design: everything in 4a is synchronous, 4b's surface is `Promise`-only, and 4c's stage pipeline is where async-facing surface appears. 4c's plan claims closure; re-verify when 4c executes | | `CTX-17`'s positive half — the first store entry installed by the first promotion | CTX-17 | 4c | 4a satisfies only the negative half (constructing a head context must not auto-register it), which holds structurally because `context.ts` never imports `store.ts`. Wiring the store into the promotions would invert the layering and make every promotion a global side effect | | Real W3C Trace Context generation behind `InstrumentationBundle` | CTX-14, CTX-15 | 7 | 4a ships the bundle's frozen shape and the no-op default only. `activeSpan`/`tracerFactory` stay typed `unknown`, and `activeSpan` is `undefined` rather than a no-op span object, until a tracing adapter defines `Span` | | `contextsEqual()`, value equality over `ExecutionContext` | CTX-5 (equality framing) | none | Built only if 4b or 4c needs one. `CTX-5`'s operative half — pinning an explicit shared key — ships via `ContextInit.key` | @@ -260,6 +272,93 @@ departure. --- +## F. Phase 4b — Recovery-Chain Primitives + +Three review passes ran over this phase; everything they found is either fixed in the branch or listed here. +Nothing below blocks the phase — the `RECOV-1`–`RECOV-16` mapping is satisfied and every CI step is green. + +### F1 — `ResponseRecoveryChain.apply()` still trusts its *seed* outcome — **WATCH** + +`RECOV-8` is absolute: "the response recovery chain's apply operation MUST NOT throw under any input." Pass 2 +found and closed the reachable half — a *step* returning a non-outcome used to raise +`TypeError: undefined is not an object` out of `apply()`, because `toFailureClosingSuccess` read `.kind` outside +its `try`. That function is now total, and three regression tests pin it. + +What is not guarded is the seed: `apply(garbage)` with at least one response step installed throws on the +`current.kind !== 'success'` read at the loop head. Left alone deliberately — `current` is only ever the +caller's argument at that point, and the sole caller is `dispatchWithRecovery`, which constructs it with +`success()` or `wrapCancellation()`. Guarding it needs either a cast plus an optional chain (which +`no-unnecessary-condition` rejects on a typed value) or the postcondition assertions F3 defers. + +**Trigger:** `recovery/` gaining a public export, or any JavaScript caller reaching `apply()` directly. Either +makes the seed a third-party value and this a real defect. + +### F2 — A step returning a non-outcome poisons the fold silently when nothing downstream reads it — **WATCH** + +The mirror of F1 on the value side. A response step returning `undefined` yields `success(undefined)`; if no +later step touches it, `apply()` resolves with a malformed Success and `dispatchWithRecovery` hands `undefined` +back as the response. Nothing throws, so `RECOV-8` holds — the failure surfaces layers away, in the caller. + +This is the concrete cost named in the roadmap's finding F2 (assertion density), and it is why that finding is +recorded as a Deviation Ledger row rather than as "no assertions needed." **Trigger:** the same as F1, or +Phase 5's retry step being the first real third-party-shaped consumer. + +### F3 — Zero `invariant()` assertions across `recovery/` — **SCHEDULED** (Phase 10) + +`docs/knowledge/assertions.md:6-7` sets a 2-per-function module average; this phase ships none across roughly a +dozen functions. Project-wide inconsistency rather than 4b's — Phases 1/2/3b/4a ship zero, 4c's plan ships +fifteen — so adding them to 4b alone would deepen the split. Recorded in the phase design's Deviation Ledger. + +One constraint Phase 10 must carry into the decision: **at the fold sites, `invariant()` is the wrong tool.** +An `invariant()` inside `apply()` throws, and `RECOV-8` forbids `apply()` from throwing. The correct shape +there is to convert a broken postcondition into a Failure, which is what the F1 fix already does. + +### F4 — The chains are classes where `data-modeling.md:10` asks for free functions — **SCHEDULED** (Phase 10) + +`RequestRecoveryChain` / `ResponseRecoveryChain` own no lifecycle and hold no mutable state, so the corpus +would have them be plain data plus free functions. Kept as classes because `RECOV-14`'s text is written about +the chain and step *instances*, and because the defensive copy wants a construction boundary. Ledgered in the +phase design. + +### F5 — `#private` fields carry no per-use justification — **SCHEDULED** (Phase 10) + +`data-modeling.md:20-23` makes `private` the default and requires a comment justifying each `#private` as a +genuine runtime-privacy requirement. No such claim is made for either chain class — unlike 3b's `Response`, +whose `#closed` must survive `Object.freeze(this)`. `#private` is the package-wide style (Phases 1, 3b, 4a), so +this is one project-wide reconciliation, not a 4b edit. + +### F6 — `RECOV-11` is a no-op in this port — **SCHEDULED** (Phase 10, ledgered) + +`wrapCancellation(error)` is `failure(error)`. The reference re-asserts a clearable `Thread.interrupt()` flag; +`AbortSignal.aborted` is durable once fired and the SDK never holds the caller's `AbortController`, so there is +nothing to re-assert. The helper exists as the one named site where the disposition lives. If Phase 5's retry +step lands without giving it behavior, inline it there and carry the disposition wholly in the ledger. + +### F7 — `suppress()`'s branch selection is only ever half-covered on any single runtime — **WATCH** + +`suppress()` returns the native `SuppressedError` where the runtime has one and `FallbackSuppressedError` where +it does not. No test forces the other branch by deleting the global — that cannot survive parallel execution +(`docs/knowledge/testing.md:50`). Coverage comes from the `test:node` matrix instead: `lts/*` exercises the +native branch, the pinned `20.3.0` exercises the fallback. **Trigger:** if the matrix ever collapses to one +runtime, or the floor rises past Node 24 (where the fallback becomes dead code to be deleted, not guarded). + +### F8 — 4b did not depend on 4a — **RESOLVED** (2026-08-26) + +The issue lists Phases 0–4a as 4b's dependency and 4b's design says "4a (execution context, done)". Neither +was true on the `8-phase-4b-recovery-chain-primitives` branch, where `packages/core/src/context/` did not +exist: 4b turned out not to depend on it — its only imports outside `recovery/` are `http/`, +`body/http-status-error.js`, `seams/transport.js`, `invariant.js` and `suppress.js`. The design's +parenthetical is corrected. The sequencing half is now closed too: 4a and 4b are both merged into +`9-phase-4c-stage-based-pipeline`, so `context/` is present ahead of 4c, which does depend on it. + +### F9 — The Phase 4 checklist's ✅ marks for 4a and 4c are still plan-level — **WATCH** + +`plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md` now says so explicitly in its Status +line, but the §7.x and §8.1 tables read identically to the §8.2 ones that are now real. Re-scan both when their +phases execute, per this file's own maintenance rule. + +--- + ## Maintaining this file Add an entry the moment a gap is found, not when it is fixed — the failure mode this file prevents is a diff --git a/docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md b/docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md index 0316f97..cc64d7c 100644 --- a/docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md +++ b/docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md @@ -16,46 +16,38 @@ reuse Phase 3b's already-async `toHttpError()` directly. **Nothing in this phase barrel** — `recovery/` is resilience-layer plumbing; `api-extractor`'s committed report must come back byte-identical. -**Tech Stack:** TypeScript 5.8+, `fast-check` for the invariant-bearing-function property tests. No new runtime +**Tech Stack:** TypeScript 5.8+, `fast-check` for the invariant-bearing-function property tests, and the +runtime-guarded `suppress()` helper this phase adds for `RECOV-12` (see the notice below). No new runtime dependencies — `SEAM-1` untouched. -> ### ⛔ BLOCKED — do not execute this plan yet +> ### ✅ UNBLOCKED — executed 2026-08-26 > -> **`RECOV-12`'s `SuppressedError` is not available on the declared runtime floor.** An earlier draft of this -> plan claimed it was "already available since Phase 3b's checkpoint lib bump" — that is false and has been -> removed. The checkpoint raised `engines.node` only to the first release exposing `Symbol.dispose`/ -> `Symbol.asyncDispose` (`plans/2026-07-25-checkpoint-scaffold-through-phase3a.md:57`, believed `18.18.0`, -> which also forbids any further floor movement as "unreviewed drift"). Node backported those two symbols on -> its own; `SuppressedError` is a V8 global from the full Explicit Resource Management proposal and is absent -> on every 18.x runtime. +> **F1 (`SuppressedError`) resolved to branch (b): a runtime-guarded `suppress()` helper.** The roadmap's +> "F1 resolution — the verified version facts" settled the choice on evidence: `SuppressedError` belongs to the +> full Explicit Resource Management proposal, which reached Node only in **24.0.0**. Branch (a) is therefore not +> a patch bump — it means `engines.node >= 24`, dropping Node 18, 20 and 22 outright for one error class. The +> floor stayed at `>=20.3` (set by `AbortSignal.any()`), and this package's `lib` is `["ES2023", "DOM", +> "DOM.AsyncIterable"]`, which does not supply `SuppressedError`'s type either. > -> Adding `esnext.disposable` to `lib` supplies the *type* only. So `new SuppressedError(...)` at Task 3 type- -> checks, passes `bun test` locally, and then throws `ReferenceError: SuppressedError is not defined` at call -> time — precisely the `NFR-10` trap `docs/knowledge/tooling-and-quality-gates.md:60-61` describes. Task 7's -> `bun run verify:node-floor`, `bun run test:node`, and the `node-floor-conformance` job pinned to `18.17.0` -> would all fail. +> `packages/core/src/suppress.ts` ships `suppress(error, suppressed, message)`: it constructs the native class +> when `globalThis.SuppressedError` exists and a shape-compatible stand-in (`name`, `error`, `suppressed`) when +> it does not, reading the global per call rather than capturing it at module load. Task 3 calls it instead of +> `new SuppressedError(...)`, and every assertion is written against the shape rather than +> `toBeInstanceOf(SuppressedError)` — the `instanceof` form would silently assert nothing on the floor runtime. +> Both branches are forced in `suppress.test.ts`, and `test/node-conformance/recovery-chain.test.mjs` re-forces +> the guarded branch from real Node, since `bun test` alone only ever exercises whichever branch Bun takes. > -> **Needs a decision, and it is cross-phase** — Phases 5a, 6a, 6b and 6c reach for `SuppressedError` on the same -> premise (`plans/2026-07-26-phase5a-retry.md:36`, `plans/2026-07-28-phase6a-serde.md`'s `closingAfter` helper, -> `specs/2026-07-28-phase6b-sse-design.md:163`, `specs/2026-07-28-phase6c-pagination-design.md:192`), so -> whichever option lands must land in all five: +> **The same helper is the fix for Phases 5a, 6a, 6b and 6c**, which reach for `SuppressedError` on the same +> premise. Their plans now point here; each replaces `new SuppressedError(...)` with `suppress(...)` when it +> executes. > -> - **(a) Raise `engines.node`** past the first release shipping Explicit Resource Management. Consumer-visible -> breaking change, and the checkpoint forbids unsanctioned floor moves. Confirm the exact release first. -> - **(b) A runtime-guarded `suppress(primary, secondary)` helper** in `packages/core/src/`, using native -> `SuppressedError` when `globalThis.SuppressedError` exists and attaching a `suppressed` property otherwise — -> the same guarded shape the roadmap already sanctioned for `Symbol.asyncDispose`. Changes Task 3's -> `expect(wrapped).toBeInstanceOf(SuppressedError)` assertion. -> -> **Second open decision, non-blocking: assertion density.** This phase ships zero `invariant()` calls across -> roughly a dozen functions, against `docs/knowledge/assertions.md:6-7`'s 2-per-function module average. The -> concrete cost: no `apply()` checks that a step returned a value at all, so a step returning `undefined` -> poisons the fold silently and surfaces layers away. Project-wide inconsistency rather than 4b's alone — -> Phases 1/2/3b/4a ship zero, 4c ships fifteen. Resolve as either postcondition assertions at the fold sites -> or a Deviation Ledger row, ideally project-wide at Phase 10. -> -> Both items are tracked in the roadmap's "Open Findings — Phase 4b Validation Review (2026-07-28)" section. -> Everything else that review raised (F3–F10) is already applied to this plan and its design. +> **F2 (assertion density) resolved to a Deviation Ledger row**, the alternative F2 itself named. This phase +> ships zero `invariant()` calls across roughly a dozen functions, against `docs/knowledge/assertions.md:6-7`'s +> 2-per-function module average, with a named cost: no `apply()` postcondition checks that a step returned a +> value at all, so a step returning `undefined` poisons the fold silently. It is a project-wide inconsistency +> rather than 4b's — Phases 1/2/3b/4a ship zero, 4c ships fifteen — so assertions added to 4b alone would deepen +> the split rather than close it. The design's ledger carries the row; Phase 10 settles the density rule once, +> project-wide. **Prerequisite:** This plan assumes Phases 0, 1, 2, 3a, 3b, and 4a are already implemented exactly as their own plans specify. Concretely: `packages/core/src/http/*` exports `DexpaceError`, `Request`, `Response`, @@ -92,8 +84,11 @@ addition alongside the existing `invariant()`/`InvariantViolation` it already ex - **`RECOV-12`'s close-on-throw is a hand-written `try`/`catch`, never `using`/`await using`.** Native disposal's auto-generated `SuppressedError` puts the *later* error (the disposal failure) first, making it primary and the original body error `.suppressed` — the opposite of what `RECOV-12` wants (the step's original - throwable stays primary; a close failure rides along as `.suppressed`). Construct - `new SuppressedError(originalError, closeError, message)` by hand — original first. + throwable stays primary; a close failure rides along as `.suppressed`). Build it with + `suppress(originalError, closeError, message)` from `../suppress.js` — original first. **Never + `new SuppressedError(...)`:** it is absent on the declared floor (Node 24.0.0 and up only) and absent from + this package's `lib`, so the direct form neither type-checks nor runs there. Assert its shape (`name`, + `error`, `suppressed`), never `toBeInstanceOf(SuppressedError)`. - **`RECOV-13`: a step that deliberately *returns* a different outcome (no throw) is never auto-closed.** Only a caught throw triggers the close-and-wrap path. Do not add a "close whenever the outcome changes" check — that would violate `RECOV-13` by closing a response a step meant to keep alive or already closed itself. @@ -151,6 +146,8 @@ addition alongside the existing `invariant()`/`InvariantViolation` it already ex ``` packages/core/src/invariant.ts # MODIFY: add assertNever() (Task 1) packages/core/src/invariant.test.ts # MODIFY: add assertNever coverage +packages/core/src/suppress.ts # NEW: suppress(), the guarded SuppressedError (F1 (b)) (Task 1b) +packages/core/src/suppress.test.ts # NEW: both branches of the guard, forced packages/core/src/recovery/ outcome.ts # Outcome, success(), failure(), fold() (Task 1) @@ -186,7 +183,7 @@ No `recovery/index.ts` (see Global Constraints). Task 7 runs the full gate seque onFailure): R` (from `recovery/outcome.ts`). Every later task in this plan imports `Outcome`/`success`/`failure` from `outcome.js`. -- [ ] **Step 1: Write the failing test for `assertNever`** +- [x] **Step 1: Write the failing test for `assertNever`** ```typescript // packages/core/src/invariant.test.ts @@ -212,12 +209,12 @@ describe('assertNever', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/invariant.test.ts` Expected: FAIL — `assertNever is not a function` (or similar export error). -- [ ] **Step 3: Add `assertNever` to `invariant.ts`** +- [x] **Step 3: Add `assertNever` to `invariant.ts`** Append to the existing file (do not touch the existing `invariant()`/`InvariantViolation` exports): @@ -234,19 +231,19 @@ export function assertNever(value: never, message = `unreachable case: ${String( } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/invariant.test.ts` Expected: PASS, including the 2 new tests. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/invariant.ts packages/core/src/invariant.test.ts git commit -m "feat(core): add assertNever exhaustiveness helper" ``` -- [ ] **Step 6: Write the failing test for `Outcome`** +- [x] **Step 6: Write the failing test for `Outcome`** ```typescript // packages/core/src/recovery/outcome.test.ts @@ -329,12 +326,12 @@ describe('fold identity law (RECOV-1)', () => { }); ``` -- [ ] **Step 7: Run and confirm it fails** +- [x] **Step 7: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/outcome.test.ts` Expected: FAIL — `Cannot find module './outcome.js'`. -- [ ] **Step 8: Write `outcome.ts`** +- [x] **Step 8: Write `outcome.ts`** ```typescript // packages/core/src/recovery/outcome.ts @@ -379,12 +376,12 @@ export function fold(outcome: Outcome, onSuccess: (value: T) => R, onFa } ``` -- [ ] **Step 9: Run and confirm it passes** +- [x] **Step 9: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/outcome.test.ts` Expected: PASS, 7 tests. -- [ ] **Step 10: Commit** +- [x] **Step 10: Commit** ```bash git add packages/core/src/recovery/outcome.ts packages/core/src/recovery/outcome.test.ts @@ -393,6 +390,43 @@ git commit -m "feat(core): add Outcome, success/failure/fold (RECOV-1)" --- +### Task 1b: `suppress()` — the runtime-guarded `SuppressedError` (F1 branch (b)) + +**Files:** +- Create: `packages/core/src/suppress.ts` +- Create: `packages/core/src/suppress.test.ts` + +**Interfaces:** +- Consumes: nothing. +- Produces: `interface SuppressedErrorLike`, `suppress(error: unknown, suppressed: unknown, message: string): + SuppressedErrorLike`. Task 3's `toFailureClosingSuccess` is its first call site; Phases 5a, 6a, 6b and 6c are + the next ones. + +- [x] **Step 1: Write the failing test** — both branches of the guard, forced. The native branch is skipped when + the runtime has no `SuppressedError`; the fallback branch is reached by deleting the global inside a + `try`/`finally` that restores the original property descriptor, with a following test asserting the restore + actually happened. Without that, whichever branch the test runtime happens to take is the only one ever + covered, and the floor runtime takes the *other* one. + +- [x] **Step 2: Run and confirm it fails** — `Cannot find module './suppress.js'`. + +- [x] **Step 3: Write `suppress.ts`** — read `globalThis.SuppressedError` **per call**, not at module load, via + an intersection cast (`globalThis as typeof globalThis & {SuppressedError?: SuppressedErrorConstructor}`); a + cast to a bare optional-property type trips TS's weak-type check. Fall back to a module-private class + extending `Error` that sets `name = 'SuppressedError'` and assigns `error`/`suppressed` in the constructor + body — no parameter properties (`erasableSyntaxOnly`). + +- [x] **Step 4: Run and confirm it passes** — 5 tests. + +- [x] **Step 5: Commit** + +```bash +git add packages/core/src/suppress.ts packages/core/src/suppress.test.ts +git commit -m "feat(core): add a runtime-guarded suppress() helper for RECOV-12" +``` + +--- + ### Task 2: `recovery/request-chain.ts` **Files:** @@ -405,7 +439,7 @@ git commit -m "feat(core): add Outcome, success/failure/fold (RECOV-1)" - Produces: `type RequestStep = (request: Request) => Promise`, `class RequestRecoveryChain`. Task 6 (`orchestrator.ts`) imports `RequestRecoveryChain`. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/request-chain.test.ts @@ -489,12 +523,12 @@ describe('RequestRecoveryChain.apply fold law', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/request-chain.test.ts` Expected: FAIL — `Cannot find module './request-chain.js'`. -- [ ] **Step 3: Write `request-chain.ts`** +- [x] **Step 3: Write `request-chain.ts`** ```typescript // packages/core/src/recovery/request-chain.ts @@ -529,12 +563,12 @@ export class RequestRecoveryChain { } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/request-chain.test.ts` Expected: PASS, 5 tests (including the fast-check property, which itself runs 100 cases by default). -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/request-chain.ts packages/core/src/recovery/request-chain.test.ts @@ -556,7 +590,7 @@ git commit -m "feat(core): add RequestRecoveryChain (RECOV-3, RECOV-14)" Outcome) => Promise>`, `class ResponseRecoveryChain`. Task 6 imports `ResponseRecoveryChain`; Task 5's `statusMappingStep` is typed as a `ResponseStep`. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/response-chain.test.ts @@ -756,9 +790,12 @@ describe('RECOV-12: close-on-throw while holding a Success', () => { expect(result.kind).toBe('failure'); const wrapped = result.kind === 'failure' ? result.error : undefined; - expect(wrapped).toBeInstanceOf(SuppressedError); - expect((wrapped as SuppressedError).error).toBe(originalError); - expect((wrapped as SuppressedError).suppressed).toBe(closeError); + // Shape, not `toBeInstanceOf(SuppressedError)`: the native class does not exist on the floor + // runtime, so the instanceof form would assert nothing there. + expect(wrapped).toBeInstanceOf(Error); + expect((wrapped as SuppressedErrorShape).name).toBe('SuppressedError'); + expect((wrapped as SuppressedErrorShape).error).toBe(originalError); + expect((wrapped as SuppressedErrorShape).suppressed).toBe(closeError); }); }); @@ -867,16 +904,17 @@ describe('RECOV-14: steps are safe for concurrent invocation', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/response-chain.test.ts` Expected: FAIL — `Cannot find module './response-chain.js'`. -- [ ] **Step 3: Write `response-chain.ts`** +- [x] **Step 3: Write `response-chain.ts`** ```typescript // packages/core/src/recovery/response-chain.ts import type {Response} from '../http/response.js'; +import {suppress} from '../suppress.js'; import {failure, success, type Outcome} from './outcome.js'; /** @internal */ @@ -887,15 +925,17 @@ export type RecoveryStep = (outcome: Outcome) => Promise): Promise> { if (current.kind === 'success') { try { await current.value.close(); } catch (closeError) { - return failure(new SuppressedError(thrownError, closeError, 'response close failed while handling step error')); + return failure( + suppress(thrownError, closeError, 'response close failed while handling a step error'), + ); } } return failure(thrownError); @@ -953,12 +993,12 @@ export class ResponseRecoveryChain { } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/response-chain.test.ts` Expected: PASS, 13 tests (including the fast-check property and the RECOV-14 concurrency test). -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/response-chain.ts packages/core/src/recovery/response-chain.test.ts @@ -980,7 +1020,7 @@ git commit -m "feat(core): add ResponseRecoveryChain (RECOV-4..RECOV-9, RECOV-12 disposition lives — if it is still a pure pass-through once Phase 5 lands, inline it there and move the disposition wholly into Phase 10's deviation ledger rather than keeping an abstraction with no behavior. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/cancellation.test.ts @@ -1026,12 +1066,12 @@ describe('wrapCancellation (RECOV-11)', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/cancellation.test.ts` Expected: FAIL — `Cannot find module './cancellation.js'`. -- [ ] **Step 3: Write `cancellation.ts`** +- [x] **Step 3: Write `cancellation.ts`** ```typescript // packages/core/src/recovery/cancellation.ts @@ -1060,7 +1100,7 @@ export function wrapCancellation(error: unknown): Outcome { } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/cancellation.test.ts` Expected: PASS, 4 tests. @@ -1068,7 +1108,7 @@ Expected: PASS, 4 tests. `CancellationError` is imported by the *test* only (to prove a classified cancellation gets no special treatment); `cancellation.ts` itself no longer needs it, so do not add the import back to the module. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/cancellation.ts packages/core/src/recovery/cancellation.test.ts @@ -1090,7 +1130,7 @@ git commit -m "feat(core): add wrapCancellation (RECOV-11)" task in this plan -- a future consumer (Phase 5 or 4c) installs it into a `ResponseRecoveryChain`'s response-step list. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/status-mapping.test.ts @@ -1149,12 +1189,12 @@ describe('statusMappingStep (RECOV-15)', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/status-mapping.test.ts` Expected: FAIL — `Cannot find module './status-mapping.js'`. -- [ ] **Step 3: Write `status-mapping.ts`** +- [x] **Step 3: Write `status-mapping.ts`** ```typescript // packages/core/src/recovery/status-mapping.ts @@ -1191,12 +1231,12 @@ statusMappingStep satisfies ResponseStep; `Response` is now imported (type-only) because the explicit parameter and return annotations a `function` declaration needs replace the inference the `: ResponseStep` annotation was supplying. -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/status-mapping.test.ts` Expected: PASS, 4 tests. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/status-mapping.ts packages/core/src/recovery/status-mapping.test.ts @@ -1219,7 +1259,7 @@ git commit -m "feat(core): add statusMappingStep, wiring 3b's toHttpError into t - Produces: `interface DispatchConfig`, `dispatchWithRecovery(request: Request, config: DispatchConfig): Promise`. Terminal task of this plan -- no later task consumes this file. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/orchestrator.test.ts @@ -1409,12 +1449,12 @@ describe('RECOV-11: the catch routes every throwable through wrapCancellation', `CancellationError` is imported from `../seams/transport.js` (Phase 2) alongside the `Transport` type. -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/orchestrator.test.ts` Expected: FAIL — `Cannot find module './orchestrator.js'`. -- [ ] **Step 3: Write `orchestrator.ts`** +- [x] **Step 3: Write `orchestrator.ts`** ```typescript // packages/core/src/recovery/orchestrator.ts @@ -1476,12 +1516,12 @@ export async function dispatchWithRecovery(request: Request, config: DispatchCon } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/orchestrator.test.ts` Expected: PASS, 7 tests. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/orchestrator.ts packages/core/src/recovery/orchestrator.test.ts @@ -1498,7 +1538,7 @@ git commit -m "feat(core): add dispatchWithRecovery orchestrator (RECOV-2, RECOV - Consumes: every preceding task. - Produces: nothing new; verifies the whole phase is green and the public surface did not move. -- [ ] **Step 1: Run the full gate sequence** +- [x] **Step 1: Run the full gate sequence** ```bash cd /home/mohammad/Projects/dexpace/nodejs-sdk @@ -1517,7 +1557,17 @@ bun run audit Expected: all exit 0. Coverage at or above the 80% aggregate floor (`NFR-5`). -- [ ] **Step 2: Verify no `node:` import crept in** +- [x] **Step 1b: Add the Node-runtime conformance case** + +`test/node-conformance/README.md`'s membership rule: a phase touching a runtime-divergent surface adds a case +there, not only to `bun test`. `SuppressedError`'s presence is exactly that divergence — Bun and current Node +ship it, the declared 20.3 floor does not — so `test/node-conformance/recovery-chain.test.mjs` forces the +guarded branch from real Node (including with the global deleted) and re-runs `RECOV-12`'s +release-exactly-once over Node's own Web Streams implementation, whose `cancel()` and reader-lock timing are +independent of Bun's. `suppress` and `recovery/` are `@internal` with no public subpath, so it imports them by +direct `dist/` path, the way `io-byte-stream.test.mjs` does. + +- [x] **Step 2: Verify no `node:` import crept in** ```bash ! grep -rn "from 'node:" packages/core/src/recovery/ @@ -1525,7 +1575,7 @@ Expected: all exit 0. Coverage at or above the 80% aggregate floor (`NFR-5`). Expected: exit 0, no matches. -- [ ] **Step 3: Verify the public API surface did not move** +- [x] **Step 3: Verify the public API surface did not move** Step 1 already regenerated the report via `bun run api`; this only inspects the result. Run from the repo root: @@ -1538,7 +1588,7 @@ Expected: **no output, exit 0.** Nothing from `src/recovery/` reached the publis 3a's/4a's gate. If this fails, remove whatever export leaked into `packages/core/src/index.ts` rather than accepting the report change. -- [ ] **Step 4: Add a changeset** +- [x] **Step 4: Add a changeset** Because nothing enters the public API, this is a patch-level, no-consumer-impact change: @@ -1549,7 +1599,7 @@ bun run changeset Select `@dexpace/core`, choose **patch**, summary: `Internal: recovery-chain primitives for product-spec §8.2 (RECOV-1..16). No public API change.` -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add .changeset/ diff --git a/docs/superpowers/plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md b/docs/superpowers/plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md index c4d49d1..856b7d2 100644 --- a/docs/superpowers/plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md +++ b/docs/superpowers/plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md @@ -10,8 +10,9 @@ Verification of the three Phase 4 implementation plans — **Legend:** ✅ Planned, implemented and tested — 🚫 Not built (permanent simplification, named reason) — ⏳ Deferred (named target phase) — N/A Not applicable in this port. -**Status:** the plans are reviewed and corrected as of 2026-07-26 (see *Review findings applied*, below) but -**not yet executed**. Every ✅ means "the plan builds and tests it," not "it is on `main`." +**Status:** the plans are reviewed and corrected as of 2026-07-26 (see *Review findings applied*, below). **4b +is executed as of 2026-08-26**; 4a and 4c are not. For the §8.2 table a ✅ now means "built, tested and on the +branch"; everywhere else it still means "the plan builds and tests it." --- @@ -77,7 +78,7 @@ Verification of the three Phase 4 implementation plans — | RECOV-9 | SHOULD | Recovery steps should return a Failure rather than throw | ✅ | Satisfied structurally — both shapes are handled identically, documented rather than enforced | | RECOV-10 | MUST | Unwrap: Success returns the response; Failure rethrows the throwable **unchanged** | ✅ | 4b Task 6 — asserted with `rejects.toBe(typedError)`, identity not message | | RECOV-11 | MUST | Wrapping a cancellation throwable re-asserts the cancellation signal | ✅ (reframed) | 4b Task 4. An `AbortSignal` is durable once fired and the SDK never holds the caller's `AbortController`, so there is nothing to re-assert; the helper is `failure(error)` and **never throws**, which is what keeps RECOV-2 absolute. Ledgered | -| RECOV-12 | MUST | A step throwing while holding a Success closes that response exactly once, close error `suppressed`, original primary | ✅ | 4b Task 3 (`toFailureClosingSuccess`, hand-built `SuppressedError` — never `using`, whose auto-generated one inverts the priority). Close observed via the body stream's `cancel()` hook, since `Response` is frozen | +| RECOV-12 | MUST | A step throwing while holding a Success closes that response exactly once, close error `suppressed`, original primary | ✅ | 4b Task 3 (`toFailureClosingSuccess`) over Task 1b's guarded `suppress()` — never `using`, whose auto-generated `SuppressedError` inverts the priority, and never `new SuppressedError(...)`, which is absent on the declared floor. Close observed via the body stream's `cancel()` hook, since `Response` is frozen. Re-forced from real Node in `test/node-conformance/recovery-chain.test.mjs` | | RECOV-13 | MUST | A deliberately *returned* different outcome is never auto-closed | ✅ | 4b Task 3 — only a caught throw reaches the close path; asserted for both a substitute Failure and a substitute Success | | RECOV-14 | MUST | Step lists immutable; response chain copies both | ✅ | 4b Tasks 2 and 3 — the request chain is copied too, which the reference does not do and the requirement's own text recommends. Ledgered | | RECOV-15 | MUST | Only 400..599 map to the typed exception; every other status passes through | ✅ | 4b Task 5, delegating to Phase 3b's unchanged `toHttpError()` | @@ -155,6 +156,7 @@ Verification of the three Phase 4 implementation plans — | Negative-space assertions | styleguide 11.9 | ✅ | Duplicate-key install, no-op closes, cross-stage edits, missing anchors, reserved SEND, continuation reuse, transport `close()` never called | | Options object over positional params | `max-params: 3` | ✅ | `ContextInit` (4a), `DispatchConfig` (4b), `CursorInit` (4c). No `eslint-disable` anywhere in Phase 4 | | Fakes over mocks; no owned interface mocked | styleguide 11.3 | ✅ | File-local `Transport` stubs throughout; no `FakeTransport`, no `mock.module`, and (as of the 2026-07-26 review) no patched `Response` method and no patched `contextStore` singleton | +| A runtime-divergent surface gets a `test/node-conformance/` case | `test/node-conformance/README.md` membership rule | ✅ | 4b: `recovery-chain.test.mjs` — `SuppressedError`'s presence is the divergence (Bun and current Node have it, the 20.3 floor does not), plus `RECOV-12`'s release-exactly-once over Node's own Web Streams | | Every test file cites its requirement IDs | Phase 1 convention, for Phase 9 | ✅ | Top-of-file comment in every test file across all three plans | | 80% aggregate coverage floor | `NFR-5` | ✅ | Each phase's gate task | @@ -197,6 +199,34 @@ stay accurate except where noted here. Full text in the roadmap's *Open Findings --- +## Phase 4b execution (2026-08-26) + +Both of 4b's open decisions closed before execution; neither changed a `RECOV-*` disposition above. + +| Item | Resolution | +|---|---| +| **F1 (blocker, cross-phase):** `SuppressedError` absent on the declared floor | Branch (b) — `packages/core/src/suppress.ts` ships `suppress(error, suppressed, message)`, native class where the runtime has one and a shape-compatible stand-in where it does not, global read per call. Branch (a) was disqualified on evidence: `SuppressedError` reached Node in **24.0.0**, so raising the floor means dropping Node 18, 20 and 22 for one error class, against a floor of `>=20.3` set by `AbortSignal.any()`. The helper discharges the obligation for 5a, 6a, 6b and 6c too — they substitute the call when they execute | +| **F2:** zero `invariant()` assertions across `recovery/` | Deviation Ledger row in 4b's design, naming the concrete cost (a step returning `undefined` poisons the fold silently). Project-wide inconsistency — 1/2/3b/4a ship zero, 4c ships fifteen — so Phase 10 settles the density rule once rather than 4b becoming the one module that differs | +| Merge residue | The phase-3 merge left `bunfig.toml` with a duplicated `[test] root` key, which TOML rejects — `bun test` failed to load bunfig at all on this branch. Fixed in its own commit before any 4b work | + +**Gate evidence (all exit 0), every step both CI jobs run, in order:** `bun install --frozen-lockfile`, +`typecheck`, `lint`, `build`, `bun test --coverage` (588 tests across 50 files; 98.68% funcs / 99.73% lines +against the 80% floor), `api` with `packages/core/etc/core.api.md` byte-identical, `lint:publish`, +`verify:dual-consumption`, `verify:consumer-types`, `verify:seam-1`, `verify:runtime-floor`, `audit`, and the +`node-conformance` job's `test:node` (36 cases, 35 before this phase). Also `test:knowledge`, which CI does not +run. Structural: no `node:` import, no `enum`, no `recovery/index.ts`, SPDX on line 1 of all 15 new files, no +import cycle anywhere under `packages/core/src`. + +**Three review passes ran before this was called done.** Pass 1 (corpus-driven) found a dead `satisfies` +statement reaching the published `dist/`, two test files that could not survive parallel execution, a missing +type-level test for the exported generic `Outcome`, an untranscribed `RECOV-15` conformance clause, and two +step-down-rule violations. Pass 2 (normative-text-driven) found a **`RECOV-8` violation**: `apply()` could +throw a `TypeError` when a step returned a non-outcome, against "MUST NOT throw under any input" — closed by +making `toFailureClosingSuccess` total; plus an unguarded `String()` in `assertNever`'s default message. Pass 3 +re-ran every CI step and swept the structure. What survives is in `docs/open-items.md` under Phase 4b. + +--- + ## Deferred out of Phase 4 | Item | Target | Note | diff --git a/docs/superpowers/plans/2026-07-26-phase5a-retry.md b/docs/superpowers/plans/2026-07-26-phase5a-retry.md index 0be4314..5ea52be 100644 --- a/docs/superpowers/plans/2026-07-26-phase5a-retry.md +++ b/docs/superpowers/plans/2026-07-26-phase5a-retry.md @@ -38,11 +38,21 @@ classification, backoff math, and pacing parsing take no I/O and no clock — dr engine, not two stacks** — `RETRY-28` explicitly instructs a unifying port to make the total-timeout opt-in, which `RetrySettings.totalTimeoutMs` does. -**Tech Stack:** TypeScript 5.8+, native `SuppressedError`, `fast-check` for the four invariant-bearing pure +**Tech Stack:** TypeScript 5.8+, Phase 4b's guarded `suppress()` helper (never native `SuppressedError` — see below), `fast-check` for the four invariant-bearing pure functions, `bun test`. No new runtime dependencies — `SEAM-1` untouched. No `node:` imports — core's zero-`node:` invariant, mechanically enforced since the scaffold, still holds (the RFC 1123 parser and the timer are both platform-neutral). +> ### ✅ F1 CLOSED — use `suppress()`, not `new SuppressedError(...)` +> +> Resolved 2026-08-26 in Phase 4b as branch (b): `packages/core/src/suppress.ts` ships +> `suppress(error, suppressed, message)` — native `SuppressedError` when `globalThis.SuppressedError` exists, a +> shape-compatible stand-in (`name`, `error`, `suppressed`) when it does not. The native class reached Node only +> in **24.0.0** and `engines.node` is `>=20.3`, so the direct form neither type-checks (not in this package's +> `lib`) nor runs on the floor. Every `new SuppressedError(...)` below becomes `suppress(...)`, and every +> `toBeInstanceOf(SuppressedError)` becomes an assertion on that shape — the `instanceof` form would silently +> assert nothing on the floor runtime. + **Prerequisite:** This plan assumes Phases 0, 1, 2, 3a, 3b, 4a, 4b, and 4c are implemented exactly as their plans specify, **plus Phase 7a's `Clock` seam** (added by the 2026-07-28 Phase 7a brainstorm's retrofit — see the "`Clock` retrofit" note in `docs/superpowers/specs/2026-07-28-phase7a-configuration-design.md`, Scope section). diff --git a/docs/superpowers/plans/2026-07-28-phase6b-sse.md b/docs/superpowers/plans/2026-07-28-phase6b-sse.md index 306c580..f2076eb 100644 --- a/docs/superpowers/plans/2026-07-28-phase6b-sse.md +++ b/docs/superpowers/plans/2026-07-28-phase6b-sse.md @@ -14,26 +14,18 @@ The parser is a stateful class (`SSE-15`/`SSE-16` need observable state, `SSE-17 generator lives one layer up in the facade, where ownership belongs. **Tech Stack:** TypeScript 5.8+, `bun test`, `fast-check` for the two chunk-independence/round-trip properties, -native `SuppressedError`. No new runtime dependencies. No `node:` imports. **No serde imports at all** — enforced +Phase 4b's guarded `suppress()` helper. No new runtime dependencies. No `node:` imports. **No serde imports at all** — enforced by a new build script, not by review. -> ### ⛔ BLOCKED on the same cross-phase item as Phase 4b — do not execute Tasks 5–7 yet +> ### ✅ F1 CLOSED — use `suppress()`, not `new SuppressedError(...)` > -> **`SuppressedError` does not exist on the declared runtime floor.** `SSE-29` and `SSE-36` are implemented here -> with `new SuppressedError(...)`, and `engines.node` is `">=18.17"`. `SuppressedError` is a V8 global from the -> full Explicit Resource Management proposal and is absent on every 18.x runtime — Node backported -> `Symbol.dispose`/`Symbol.asyncDispose` on their own, not the error type. Adding `esnext.disposable` to `lib` -> supplies the *type* only, so `new SuppressedError(...)` type-checks, passes `bun test` locally, and then throws -> `ReferenceError: SuppressedError is not defined` under Task 9's `bun run verify:node-floor` / `bun run -> test:node` on the pinned 18.17.0 runner. That is exactly the `NFR-10` trap -> `docs/knowledge/tooling-and-quality-gates.md:60-61` describes. -> -> This is **not 6b's decision to make**: `plans/2026-07-25-phase4b-recovery-chain.md:24-48` already raised it as -> a blocker naming Phases 5a, 6a, 6b and 6c, with two options on the table — raise `engines.node`, or add a -> runtime-guarded `suppress(primary, secondary)` helper in `packages/core/src/`. Whichever lands, lands in all -> five. If the guarded-helper option is chosen, every `new SuppressedError(...)` below becomes -> `suppress(primary, secondary, message)` and the `toBeInstanceOf(SuppressedError)` assertions become assertions -> on that helper's shape. Tasks 1–4 and 8 are unaffected and can proceed. +> Resolved 2026-08-26 in Phase 4b as branch (b): `packages/core/src/suppress.ts` ships +> `suppress(error, suppressed, message)`, which constructs the native `SuppressedError` when +> `globalThis.SuppressedError` exists and a shape-compatible stand-in (`name`, `error`, `suppressed`) when it +> does not. `SuppressedError` reached Node only in **24.0.0** and `engines.node` is `>=20.3`, so the direct form +> neither type-checks (it is not in this package's `lib`) nor runs on the floor. Every `new SuppressedError(...)` +> below becomes `suppress(...)`, and every `toBeInstanceOf(SuppressedError)` becomes an assertion on that shape — +> the `instanceof` form would silently assert nothing on the floor runtime. No decision left to make here. **Prerequisite:** Phases 0 through **5c** implemented as their plans specify. **6a is deliberately *not* a prerequisite** — `SSE-37` (MUST) forbids any serde dependency in core SSE, so this phase imports nothing 6a diff --git a/docs/superpowers/specs/2026-07-23-nodejs-sdk-v1-roadmap-design.md b/docs/superpowers/specs/2026-07-23-nodejs-sdk-v1-roadmap-design.md index 855ed04..c6fabf7 100644 --- a/docs/superpowers/specs/2026-07-23-nodejs-sdk-v1-roadmap-design.md +++ b/docs/superpowers/specs/2026-07-23-nodejs-sdk-v1-roadmap-design.md @@ -376,8 +376,8 @@ had. **Verify a prerequisite against the artifact it was supposed to produce, no 2. **E1** (§5.4's three parts, which do not work separately). Cheaper now than when the checkpoint was written: the new `verify:consumer-types` gate mechanically proves a `lib` entry that is declared but whose floor was not raised, and proves the reverse too. -3. **Read 4b's F1 before designing against it** — amended 2026-08-26 with the verified `SuppressedError` - version facts, which resolve it to branch (b). See "F1 resolution — the verified version facts" under +3. **F1 is closed** — resolved to branch (b) and implemented 2026-08-26 as `packages/core/src/suppress.ts`. + Read it before designing against `SuppressedError` anywhere. See "F1 resolution — the verified version facts" under "Open Findings — Phase 4b Validation Review" further down this document. That amendment changes 4b's design input, not just its wording, and F1 already notes the resolution has to land in 5a, 6b and 6c at the same time. @@ -417,18 +417,28 @@ checks out against the earlier phase plans — `toHttpError(): Promise=18.17"`, raised at most to `18.18.0` at the 2026-07-25 checkpoint (which exposes `Symbol.dispose`/`Symbol.asyncDispose` only — Node backported those two symbols; `SuppressedError` is a V8 global from the full Explicit Resource Management proposal). `esnext.disposable` in `lib` supplies its *type*, so `new SuppressedError(...)` type-checks and then throws `ReferenceError` at call time — the exact `NFR-10` trap `tooling-and-quality-gates.md:60-61` describes. `bun test` passes locally; the `node-floor-conformance` job pinned to `18.17.0`, `verify:node-floor` and `test:node` all fail | PLAN:19-20 (Tech Stack, claims it is "already available since Phase 3b's checkpoint lib bump" — false), PLAN:804, SPEC:124; also 5a plan:36, 6b design:163, 6c design:192 | **Resolved 2026-08-26: take branch (b)** — the runtime-guarded `suppress()` helper. The "confirm the first supporting Node release" condition this row left open is now discharged, and it settles the choice rather than merely informing it; two of this row's own premises also turn out to be false. See "F1 resolution — the verified version facts" below the table. **Partially applied 2026-07-28:** the false Tech Stack claim is deleted and replaced with a blocking notice at the top of the plan stating the real constraint; the mechanism itself is untouched pending implementation of (b) | -| F2 | major — OPEN | Zero assertions across the whole `recovery/` module — a dozen functions, no `invariant()` call, against `assertions.md:6-7`'s 2-per-function module average (and `styleguide-overview.md:22-23` Rule 8). Neither document acknowledges the rule or argues an exemption. Concretely: no `apply()` checks that a step returned a value at all, so a step returning `undefined` poisons the fold silently. Project-wide inconsistency, not 4b's alone — Phases 1/2/3b/4a ship zero, 4c ships fifteen | PLAN:463-479, 818-859, 964-966, 1352-1370 | **Undecided.** Either postcondition assertions at the fold sites, or a Deviation Ledger row. Worth settling at the project level (Phase 10) rather than per-phase | +| F1 | **blocker** — ✅ closed | `SuppressedError` does not exist on the declared runtime floor. `engines.node` is `">=18.17"`, raised at most to `18.18.0` at the 2026-07-25 checkpoint (which exposes `Symbol.dispose`/`Symbol.asyncDispose` only — Node backported those two symbols; `SuppressedError` is a V8 global from the full Explicit Resource Management proposal). `esnext.disposable` in `lib` supplies its *type*, so `new SuppressedError(...)` type-checks and then throws `ReferenceError` at call time — the exact `NFR-10` trap `tooling-and-quality-gates.md:60-61` describes. `bun test` passes locally; the `node-floor-conformance` job pinned to `18.17.0`, `verify:node-floor` and `test:node` all fail | PLAN:19-20 (Tech Stack, claims it is "already available since Phase 3b's checkpoint lib bump" — false), PLAN:804, SPEC:124; also 5a plan:36, 6b design:163, 6c design:192 | **Resolved 2026-08-26: take branch (b)** — the runtime-guarded `suppress()` helper. The "confirm the first supporting Node release" condition this row left open is now discharged, and it settles the choice rather than merely informing it; two of this row's own premises also turn out to be false. See "F1 resolution — the verified version facts" below the table. **Partially applied 2026-07-28:** the false Tech Stack claim is deleted and replaced with a blocking notice at the top of the plan stating the real constraint; **Applied 2026-08-26:** `packages/core/src/suppress.ts` ships the guarded helper, `response-chain.ts` calls it, and assertions are written against its shape rather than `instanceof SuppressedError` — the `instanceof` form would silently assert nothing on the floor runtime | +| F2 | major — ✅ closed | Zero assertions across the whole `recovery/` module — a dozen functions, no `invariant()` call, against `assertions.md:6-7`'s 2-per-function module average (and `styleguide-overview.md:22-23` Rule 8). Neither document acknowledges the rule or argues an exemption. Concretely: no `apply()` checks that a step returned a value at all, so a step returning `undefined` poisons the fold silently. Project-wide inconsistency, not 4b's alone — Phases 1/2/3b/4a ship zero, 4c ships fifteen | PLAN:463-479, 818-859, 964-966, 1352-1370 | **Resolved 2026-08-26: Deviation Ledger row.** Recorded in 4b's design with the concrete cost named (a step returning `undefined` poisons the fold silently). Assertions added to 4b alone would deepen the 0-vs-15 split with 4c rather than close it, so the density rule is settled once at Phase 10 and applied project-wide | | F3 | major — ✅ applied | SPEC:270 still says "the only new failure surface is `wrapCancellation()`'s `invariant()` crash" — stale text from a superseded draft. SPEC:194-204, SPEC:279 and PLAN:63-74 all state the opposite. An agent executing from the File Layout section would restore the `invariant()`, and because the helper runs inside `dispatchWithRecovery`'s own `catch`, that throw bypasses the response and recovery chains — the one failure mode `RECOV-2` exists to prevent | SPEC:270-271 | Replace with `assertNever`'s `InvariantViolation` crash, matching the already-correct PLAN:89-90 | | F4 | minor — ✅ applied | Spec never designs the `assertNever` addition Task 1 builds. PLAN modifies `packages/core/src/invariant.ts` (new exported symbol, two tests, its own commit); SPEC's File Layout lists only `recovery/` | SPEC:258-268 vs PLAN:102-103, 124-197 | Add the `invariant.ts` line to the spec's File Layout with a one-line note that `fold()` is the codebase's first discriminated-union `switch` | | F5 | minor — ✅ applied | `RECOV-14`'s second normative sentence (steps safe for concurrent invocation; per-request state never on the step instance) is claimed but neither designed nor tested — both documents cite `RECOV-14` for the defensive copy only. The design does satisfy it (all per-call state is local), but nothing records or guards that | SPEC:141-144, PLAN:49-51 | One sentence in the design + one plan test interleaving two `apply()` calls on one chain | diff --git a/docs/superpowers/specs/2026-07-25-phase4b-recovery-chain-design.md b/docs/superpowers/specs/2026-07-25-phase4b-recovery-chain-design.md index 00202d7..76b5b8b 100644 --- a/docs/superpowers/specs/2026-07-25-phase4b-recovery-chain-design.md +++ b/docs/superpowers/specs/2026-07-25-phase4b-recovery-chain-design.md @@ -1,17 +1,19 @@ # Phase 4b — Recovery-Chain Primitives — Design -**Status:** Draft, approved for planning. **⛔ Two open decisions block execution** — see the blocking notice at -the top of `docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md`. In short: `RECOV-12`'s -`SuppressedError` does not exist on the declared `engines.node` floor (a cross-phase problem shared with 5a, 6b -and 6c), and this phase's zero-assertion module contradicts the corpus's 2-per-function average. Both are -tracked in the roadmap's "Open Findings — Phase 4b Validation Review (2026-07-28)" section. The rest of that -review's findings are applied to this document. +**Status:** Implemented 2026-08-26. **Both open decisions are closed.** `RECOV-12`'s `SuppressedError` is +reached through a runtime-guarded `suppress()` helper — branch (b) of the cross-phase F1 decision, which the +roadmap resolved on the verified version facts (`SuppressedError` reached Node only in 24.0.0; branch (a) would +mean `>=24`, dropping Node 18/20/22 outright). F2 (this phase's zero `invariant()` assertions) is recorded as a +Deviation Ledger row for Phase 10's project-wide pass, the disposition F2 itself named as the alternative to +fold-site postconditions. Both are tracked in the roadmap's "Open Findings — Phase 4b Validation Review +(2026-07-28)" section. **Purpose:** Implement the recovery-chain primitives — `Outcome`, the request and response recovery chains, the unified dispatch orchestrator, the cancellation-wrapping helper, and the status→typed-exception mapping step — satisfying `docs/product-spec/08-execution-pipelines.md` §8.2 (`RECOV-1`–`RECOV-16`). This is the second of three sub-phases the roadmap's Phase 4 ("Execution Context & Pipelines") splits into: 4a (execution context, -done), **4b** (this document, `§8.2`), 4c (stage-based pipeline, `§8.1`, built on 4a+4b). +**not yet implemented** — 4b turned out not to depend on it; see `docs/open-items.md` F8), **4b** (this +document, `§8.2`), 4c (stage-based pipeline, `§8.1`, which does depend on both 4a and 4b). **Governing documents:** `docs/product-spec/08-execution-pipelines.md` §8.2/§8.3 (normative, cited by ID throughout), `docs/sdk-design-nodejs/05-pipeline-architecture.md` (Node-port mapping for both pipeline layers), @@ -113,8 +115,16 @@ declared order within each group. than throw; both are handled identically by `apply()`. - **`RECOV-12`:** when a step throws while the current outcome is a `Success` holding a response, `apply()` closes that response (`response.close()`, from Phase 3b) before wrapping the throwable into a `Failure`, - attaching any close error as `suppressed` via a manually-constructed `SuppressedError` so a close failure never - masks the primary throwable. The response is released exactly once. + attaching any close error as `suppressed` through the `suppress()` helper so a close failure never masks the + primary throwable. The response is released exactly once. + + **`SuppressedError` is not a global on the declared floor.** It belongs to the full Explicit Resource + Management proposal, which reached Node only in **24.0.0**; `engines.node` is `>=20.3` and this package's + `lib` (`ES2023`, `DOM`, `DOM.AsyncIterable`) does not even supply the *type*. `packages/core/src/suppress.ts` + wraps that gap: `suppress(error, suppressed, message)` constructs the native class when + `globalThis.SuppressedError` exists and a shape-compatible stand-in (`name`, `error`, `suppressed`) when it + does not, reading the global per call rather than at module load. Callers never branch on which one they got, + and assertions are written against the shape, never `instanceof SuppressedError`. **This must be a hand-written `try`/`catch` around the `close()` call, not `using`/`await using`.** Native disposal's own `SuppressedError` construction puts the *later* error first — when a body already threw and @@ -133,7 +143,7 @@ declared order within each group. await current.value.close(); // Response.close() is Promise (3b) -- must be awaited to be catchable } catch (closeError) { return failure( - new SuppressedError(originalError, closeError, 'response close failed while handling step error'), + suppress(originalError, closeError, 'response close failed while handling a step error'), ); } } @@ -287,6 +297,7 @@ the violation — retrofit it before Phase 3b is executed, or record it in Phase ``` packages/core/src/invariant.ts # MODIFY: add assertNever() +packages/core/src/suppress.ts # NEW: suppress(), the runtime-guarded SuppressedError (F1 branch (b)) packages/core/src/recovery/ outcome.ts # Outcome, success(), failure(), fold() @@ -297,7 +308,7 @@ packages/core/src/recovery/ status-mapping.ts # statusMappingStep() ``` -`invariant.ts` is the one file outside `recovery/` this sub-phase touches. `docs/knowledge/data-modeling.md` +`invariant.ts` and `suppress.ts` are the two files outside `recovery/` this sub-phase touches. `docs/knowledge/data-modeling.md` requires every discriminated-union `switch` to close with `default: return assertNever(x)`, "defined once and imported everywhere," and no prior phase plan actually adds it — `fold()` is the codebase's first such `switch`, so `assertNever` lands here as a small addition alongside the `invariant()`/`InvariantViolation` that module @@ -317,6 +328,9 @@ see its section above for why an `invariant()` there would violate `RECOV-2`. | No new per-status typed-exception hierarchy for `RECOV-15` | `RECOV-15`'s "matching typed exception" (which some ports read as a per-status class family) | Phase 3b's flat `HttpStatusError` (carrying `status` + buffered body) already satisfies this, and the corpus caps custom error hierarchies at two levels; a per-status class family would violate that cap | | No default/preset recovery chain shipped in 4b | none — scope decision | Matches 4a's "primitives only" discipline; Phase 5 (retry) is the first real consumer and decides its own composition | | `#private` fields and methods on both chain classes (`#steps`, `#responseSteps`, `#recoverySteps`, `#runResponsePhase`, `#runRecoveryPhase`) | `docs/knowledge/data-modeling.md:20-23` — `private` is the default; `#private` requires a comment justifying a genuine runtime-privacy requirement | **No runtime-privacy claim is made.** These classes are unfrozen holders of a readonly array, unlike 3b's `Response`, whose `#closed` genuinely must survive `Object.freeze(this)`. `#private` is the established package-wide field style (Phase 1, 3b, and 4a's `ContextStore` all use it), so switching 4b alone would fragment the package and trip the corpus's own "never mix two styles within a module/package" rule. Recorded as a project-wide deviation for Phase 10 to reconcile in one pass, not fixed here | +| `RECOV-12`'s suppressed-error pairing goes through a runtime-guarded `suppress()` helper rather than `new SuppressedError(...)` | none — a runtime-floor constraint, not a spec deviation. Listed so Phase 10 sees the shape | `SuppressedError` reached Node in 24.0.0; `engines.node` is `>=20.3` and `lib` does not supply the type. Raising the floor to reach one error class would drop Node 18, 20 and 22. The helper returns the native class where it exists and a shape-compatible stand-in where it does not, so nothing downstream branches. Phases 5a, 6a, 6b and 6c share the helper | +| `RequestRecoveryChain` / `ResponseRecoveryChain` are classes holding an immutable step array | `docs/knowledge/data-modeling.md:10` — classes are reserved for things that own a lifecycle or hold mutable runtime state behind an invariant; everything else is plain data transformed by free functions | Neither chain owns a lifecycle or mutable state — a free `applyRequestChain(steps, request)` would satisfy the corpus directly. Kept as classes because `RECOV-14`'s second clause is written about the *step instance* and the chain instance ("per-request state never on the step instance"), and because the defensive copy has to happen once at a construction boundary rather than on every call. Recorded rather than corrected: the shape is what `§8.2` describes and what Phase 5's retry step will compose against | +| Zero `invariant()` assertions across `recovery/` | `docs/knowledge/assertions.md:6-7`'s 2-per-function module average (Rule 8) | F2, deliberately not closed here. The concrete cost is named: no `apply()` postcondition checks that a step returned a value at all, so a step returning `undefined` poisons the fold silently and surfaces layers away. It is a project-wide inconsistency rather than 4b's — Phases 1/2/3b/4a ship zero, 4c ships fifteen — so adding assertions to 4b alone would deepen the split rather than close it. Phase 10 settles the density rule once and applies it everywhere | | `fold(outcome, onSuccess, onFailure)` takes three positional parameters | `docs/knowledge/function-design.md:22-23` — "an options object when it has 3 or more parameters" | The prose rule is one parameter stricter than its own stated enforcement (`max-params: ['error', 3]` errors at four), so this passes lint while violating the corpus text — flagged as a corpus conflict in the roadmap, not silently ignored. Three positional parameters match Phase 2's already-shipped `Transport.send(request, options?, signal?)`; `fold(outcome, {onSuccess, onFailure})` would make 4b the only module in the package reading differently for a canonical two-branch fold | ## Testing @@ -350,6 +364,19 @@ trigger an auto-close of the original (`RECOV-13`); `dispatchWithRecovery` rethr byte-for-byte unchanged, no wrapping (`RECOV-10`); a transport-raised `CancellationError` with no caller signal still reaches the recovery steps rather than escaping the orchestrator (`RECOV-2`/`RECOV-11`). +**Type-level tests.** `Outcome` is an exported generic type, so it ships `expectTypeOf` assertions +(styleguide 11.6): the `kind` union is closed, each variant's payload is reachable only after narrowing, and two +`@ts-expect-error` lines prove the negative — a narrowed `Success` has no `error` and a narrowed `Failure` has +no `value`. `statusMappingStep`'s conformance to `ResponseStep` is asserted the same way, in the test file +rather than as a module-level `satisfies` statement: `satisfies` erases to its operand, not to nothing, so the +module-level form leaves a dead `statusMappingStep;` expression statement in the published `dist/`. + +**Node-runtime conformance.** `SuppressedError`'s presence is exactly the kind of runtime divergence +`test/node-conformance/`'s membership rule exists for — Bun and current Node ship it, the declared floor does +not — so `recovery-chain.test.mjs` forces the guarded branch from real Node and re-runs `RECOV-12`'s +release-exactly-once over Node's own Web Streams. `bun test` alone would only ever exercise whichever branch +Bun's runtime happens to take. + **A `Response` is frozen** (Phase 1's `Object.freeze(this)`, preserved by 3b's retrofit), so no test may patch `response.close` — the assignment throws `TypeError` under an ES module's strict mode. `RECOV-12`/`RECOV-13`'s close assertions observe the body stream's `cancel()` hook instead, the way 3b's own `response.test.ts` does. diff --git a/docs/superpowers/specs/2026-07-28-phase6a-serde-design.md b/docs/superpowers/specs/2026-07-28-phase6a-serde-design.md index 264c7c7..fc96789 100644 --- a/docs/superpowers/specs/2026-07-28-phase6a-serde-design.md +++ b/docs/superpowers/specs/2026-07-28-phase6a-serde-design.md @@ -261,10 +261,12 @@ serde semantics) rather than methods. **Runtime-floor note on the close-failure path.** Preserving a decode failure as primary while carrying the close failure alongside it is what `SuppressedError` is for, and `SuppressedError` is **not available on the - declared `engines.node` floor** — it is a V8 global from the full Explicit Resource Management proposal, - absent on every 18.x runtime, and `esnext.disposable` in `lib` supplies only the type. This is the open - cross-phase decision recorded at `plans/2026-07-25-phase4b-recovery-chain.md:22-47`; 6a is a fourth site - alongside 5a, 6b and 6c, and whichever option lands there lands here unchanged. + declared `engines.node` floor** — it belongs to the full Explicit Resource Management proposal, which reached + Node only in 24.0.0, against a floor of `>=20.3`, and this package's `lib` does not supply its type either. + The cross-phase decision is **closed**: Phase 4b resolved it to branch (b) and shipped + `suppress(error, suppressed, message)` in `packages/core/src/suppress.ts` — native class where the runtime has + one, shape-compatible stand-in where it does not. 6a calls that helper and asserts its shape, never + `instanceof SuppressedError`. - `SERDE-28`: 2xx decodes. **4xx/5xx delegates to 3b's `toHttpError()`** — that function already buffers a bounded error body inside the response's own close-guaranteeing scope, at the shared 1 MiB cap `BODY-30`/`HTTP-52` define and `§14` itself points at. Building a second cap here would be a defect. Other non-2xx (1xx, an diff --git a/docs/superpowers/specs/2026-07-28-phase6b-sse-design.md b/docs/superpowers/specs/2026-07-28-phase6b-sse-design.md index 460183d..8be7cf6 100644 --- a/docs/superpowers/specs/2026-07-28-phase6b-sse-design.md +++ b/docs/superpowers/specs/2026-07-28-phase6b-sse-design.md @@ -193,9 +193,9 @@ internal release routine, not inferred from context: consumer already received, which is the specific harm `SSE-30` names. - Release on an **explicit `close()`** — a failing close propagates. The caller asked; the caller hears. - Release with **an error already in flight** (`SSE-29`, `SSE-36`) — the primary error propagates with the close - failure attached via native `SuppressedError`, the same mechanism 5a uses. + failure attached via Phase 4b's guarded `suppress()` helper, the same mechanism 5a uses. -> **`SuppressedError` is blocked on a cross-phase decision, and 6b does not get to make it.** +> **`SuppressedError` was blocked on a cross-phase decision; it is now closed and 6b just calls the helper.** > `plans/2026-07-25-phase4b-recovery-chain.md:24-48` establishes that `SuppressedError` is a V8 global from the > full Explicit Resource Management proposal, absent on every 18.x runtime — Node backported > `Symbol.dispose`/`Symbol.asyncDispose` alone — while `engines.node` is `">=18.17"` and `verify:node-floor` @@ -325,7 +325,7 @@ Phase 9's sweep reads this table rather than re-deriving it. |---|---|---| | `BufferedSource` (`overStream`, `exhausted`, `readByte`, `readExactly`, `peek`, `close`) | 3a | `peek()` is `SSE-12`'s lookahead; `close()` is already idempotent and already rejects later reads, covering most of `SSE-27`/`SSE-28`. `exhausted()` — not a sentinel from `readByte()` — is how end of stream is detected, because `readByte()` rejects rather than returning one | | `IoError` | 3a | Every read-path failure surfaces as one shape, including the torn-down-mid-read case | -| `SuppressedError` usage pattern | 5a | `SSE-29`/`SSE-36`'s "close failure attached to the primary" is the identical mechanism — **and inherits 5a's unresolved blocker**, see below | +| `suppress()` usage pattern | 4b, 5a | `SSE-29`/`SSE-36`'s "close failure attached to the primary" is the identical mechanism, over 4b's guarded helper. The former cross-phase blocker is closed | | `Response.body` / `Response.close()` | 3b | `sseStreamFrom` binds to them; it does not reach for a transport — which is also half of why `SSE-38` holds by construction | | The `kind`-discriminated union idiom | 4b | `MapperOutcome` mirrors `Outcome`'s shape without extending its type | @@ -408,4 +408,4 @@ would publish a way to violate `SSE-17`'s non-ownership contract by accident. | `SSE-37` and `SSE-38` enforced by a build script, not by module-graph structure | `SSE-37`, `SSE-38` | The reference gets it free from package boundaries; this port puts serde in the same package, so the invariant needs a mechanical guard or it is only a convention. The script strips comments before the `SSE-38` marker scan and skips `*.test.ts` for markers only — a gate that failed on a TSDoc *documenting* the absence of reconnection would be deleted rather than obeyed | | `[Symbol.asyncDispose]` on `SseStream` is optional and runtime-guarded, not an `implements AsyncDisposable` | `styleguide/typescript/13` §13.1–13.2 | The symbol postdates the declared `>=18.17` floor that `verify:node-floor` pins, and TypeScript does not polyfill it for a declaring library. `close()` stays the supported path everywhere; dispose delegates to it. Cost: `await using` does not type-check against an optional member. Unconditional once the floor moves past 18.18 | | Byte-at-a-time line framing via `readByte()` | `docs/knowledge/performance.md:16-17` | `readByte()` is `readExactly(1)` underneath, so framing allocates per byte on the parse path. Deferred deliberately on the guide's own terms (`performance.md:4,24` — no micro-fix before a profile names the bottleneck) and recorded so Phase 10 revisits it with a `*.bench.ts` instead of rediscovering it. A bulk-`read()` framing is the fix if a profile calls for one; no observable contract changes | -| `SSE-29`/`SSE-36` construct a native `SuppressedError` | `NFR-10` / the declared `>=18.17` floor | Not 6b's deviation to take or reverse — inherited from the cross-phase blocker at `plans/2026-07-25-phase4b-recovery-chain.md:24-48`, which must resolve across 4b/5a/6a/6b/6c together. Listed here so Phase 10 sees 6b in that set | +| `SSE-29`/`SSE-36` pair errors through `suppress()` rather than native `SuppressedError` | none — a runtime-floor constraint, not a spec deviation | Inherited from 4b's F1 resolution (branch (b), 2026-08-26): the native class reached Node in 24.0.0, against a `>=20.3` floor. Listed here so Phase 10 sees 6b in that set | diff --git a/docs/superpowers/specs/2026-07-28-phase6c-pagination-design.md b/docs/superpowers/specs/2026-07-28-phase6c-pagination-design.md index f57f798..b540d3b 100644 --- a/docs/superpowers/specs/2026-07-28-phase6c-pagination-design.md +++ b/docs/superpowers/specs/2026-07-28-phase6c-pagination-design.md @@ -197,7 +197,7 @@ one of them would get an error instead of a walk. So `pages()` mints a fresh sin `items()` is deliberately not single-use at either level: `PAGE-14` scopes the restriction to the page-level view, and `PAGE-8` requires two independent iterations to work. -`PAGE-15`'s two-close-failures case uses native `SuppressedError`, the same mechanism 5a and 6b use — and it +`PAGE-15`'s two-close-failures case uses Phase 4b's guarded `suppress()` helper (never native `SuppressedError`, which reached Node only in 24.0.0 against a `>=20.3` floor), the same mechanism 5a and 6b use — and it applies to the walk's *own* failure paths too, not only to back-to-back close failures. A generator whose body throws and whose `finally` then fails to close the held page would surface the close error and lose the transport or parse failure the caller actually needs. So the drive routine catches, releases with the walk's failure kept @@ -377,7 +377,7 @@ async generator *is* the engine.** | `Transport` | 2 | `PAGE-25`'s transport-agnosticism; `Runtime` (4c) satisfies it, so a resilience pipeline drops in unchanged | | `RequestOptions` | 1 | `PAGE-36` threads the caller's instance; the engine never constructs one | | `FakeTransport`, `countingResponse()` | 5a | Scripted multi-response sequences, wire-send counting, and per-response close observation are exactly what `PAGE-6`/`PAGE-9`/`PAGE-27` need. 5a's design names `countingResponse()`'s `cancel()` hook as the **only** sanctioned way to observe a close — responses are frozen, so a spy assignment throws | -| `SuppressedError` usage pattern | 5a, 6b | `PAGE-13` and `PAGE-15` both need primary-plus-suppressed | +| `suppress()` usage pattern (never native `SuppressedError`) | 4b, 5a, 6b | `PAGE-13` and `PAGE-15` both need primary-plus-suppressed | ## File Layout @@ -442,7 +442,7 @@ implementation details; publishing them would publish a second URL-manipulation - The same iterator-level single-use assertion on `paginateWithFetchers()`' returned view, paired with a `firstCalls === 1` check — an unguarded view re-runs the first-page fetcher and breaks `PAGE-34` outright. - A transport failure surfacing unwrapped (`PAGE-28`), and a transport failure whose held-page release *also* - fails surfacing as `SuppressedError` with the transport failure primary (`PAGE-15`). + fails surfacing as a `suppress()` pairing with the transport failure primary (`PAGE-15`). - The capped fetcher walk asserting the next-page fetcher ran `N-1` times, not `N`, and that every page fetched was also closed — a cap checked at the wrong end of the loop over-fetches by one and leaks the page it refuses to deliver. diff --git a/packages/core/src/invariant.test.ts b/packages/core/src/invariant.test.ts index be3fd82..118975c 100644 --- a/packages/core/src/invariant.test.ts +++ b/packages/core/src/invariant.test.ts @@ -1,8 +1,10 @@ // SPDX-License-Identifier: MIT // packages/core/src/invariant.test.ts -// Exercises: the project's sole assertion primitive (styleguide 5.6) and its error class. +// Exercises: the project's sole assertion primitive (styleguide 5.6), its error class, and the +// discriminated-union exhaustiveness helper docs/knowledge/data-modeling.md requires every switch to +// close with. import {describe, expect, test} from 'bun:test'; -import {invariant, InvariantViolation} from './invariant.js'; +import {assertNever, invariant, InvariantViolation} from './invariant.js'; describe('invariant', () => { test('does not throw when the condition is truthy', () => { @@ -27,3 +29,40 @@ describe('invariant', () => { expect(error.message).toBe('boom'); }); }); + +describe('assertNever', () => { + test('throws InvariantViolation naming the unreachable value', () => { + expect(() => { + // @ts-expect-error -- deliberately calling with a value that is not `never`, to exercise the + // runtime path a newly-added union variant would reach. + assertNever('unexpected-variant'); + }).toThrow(InvariantViolation); + expect(() => { + // @ts-expect-error -- same as above. + assertNever('unexpected-variant'); + }).toThrow('unexpected-variant'); + }); + + test('does not throw from its own message construction on an unstringifiable value', () => { + // `String()` is not total: a null-prototype object has no `toString` to reach, and a value + // whose `toString` throws propagates that throw. An assertion helper that reported THOSE + // instead of the invariant violation would name the wrong failure at the worst moment. + expect(() => { + assertNever(Object.create(null) as never); + }).toThrow(InvariantViolation); + expect(() => { + assertNever({ + toString() { + throw new Error('boom'); + }, + } as never); + }).toThrow(InvariantViolation); + }); + + test('accepts a custom message', () => { + expect(() => { + // @ts-expect-error -- same as above. + assertNever('x', 'custom message'); + }).toThrow('custom message'); + }); +}); diff --git a/packages/core/src/invariant.ts b/packages/core/src/invariant.ts index f0da862..d450d00 100644 --- a/packages/core/src/invariant.ts +++ b/packages/core/src/invariant.ts @@ -29,3 +29,32 @@ export class InvariantViolation extends Error { export function invariant(cond: unknown, msg: string): asserts cond { if (!cond) throw new InvariantViolation(msg); } + +/** + * Closes an exhaustive discriminated-union `switch`'s `default` case + * (`docs/knowledge/data-modeling.md`). If a new union variant is ever added without a matching + * `case`, the call stops type-checking; if one reaches this at runtime anyway — a value crossing a + * seam that the type says cannot exist — it crashes loudly rather than falling through silently. + * + * @internal + */ +export function assertNever(value: never, message?: string): never { + throw new InvariantViolation( + message ?? `unreachable case: ${describe(value)}`, + ); +} + +/** + * `String(value)` is not total: it throws on a null-prototype object (no `toString` to reach) and + * on any value whose `toString`/`Symbol.toPrimitive` throws — the same hazard + * `docs/knowledge/error-handling.md:18` makes `toError` guard. An assertion helper that throws from + * its own message construction reports the wrong failure at the worst moment, so the fallback is a + * fixed string. + */ +function describe(value: unknown): string { + try { + return String(value); + } catch { + return 'an unstringifiable value'; + } +} diff --git a/packages/core/src/recovery/cancellation.test.ts b/packages/core/src/recovery/cancellation.test.ts new file mode 100644 index 0000000..c275b53 --- /dev/null +++ b/packages/core/src/recovery/cancellation.test.ts @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/cancellation.test.ts +// Exercises: RECOV-11 (wrapping a cancellation throwable into a Failure), reframed for Node — an +// AbortSignal is durable once aborted and the SDK never holds the caller's AbortController, so +// there is nothing to re-assert. What the requirement still buys is the guarantee that a +// cancellation surfaces through the SAME Failure channel as every other throwable, never through a +// side exit (RECOV-2). +import {describe, expect, test} from 'bun:test'; +import {CancellationError} from '../seams/transport.js'; +import {wrapCancellation} from './cancellation.js'; + +describe('wrapCancellation (RECOV-11)', () => { + test('wraps a CancellationError into a Failure carrying it unchanged', () => { + const error = new CancellationError('cancelled by caller'); + + const outcome = wrapCancellation(error); + + expect(outcome.kind).toBe('failure'); + expect(outcome.kind === 'failure' && outcome.error).toBe(error); + }); + + test('wraps an ordinary error into a Failure carrying it unchanged', () => { + const error = new Error('an ordinary failure'); + + const outcome = wrapCancellation(error); + + expect(outcome.kind).toBe('failure'); + expect(outcome.kind === 'failure' && outcome.error).toBe(error); + }); + + test('wraps a non-Error throw unchanged — a JS throw can raise any value', () => { + const outcome = wrapCancellation('a string throw'); + + expect(outcome.kind === 'failure' && outcome.error).toBe('a string throw'); + }); + + test('never throws, for any input', () => { + // RECOV-2 depends on this: dispatchWithRecovery calls it from inside its own catch, so a throw + // here would let a transport failure bypass the response and recovery chains entirely. + expect(() => wrapCancellation(new CancellationError('x'))).not.toThrow(); + expect(() => wrapCancellation(undefined)).not.toThrow(); + }); +}); diff --git a/packages/core/src/recovery/cancellation.ts b/packages/core/src/recovery/cancellation.ts new file mode 100644 index 0000000..edd053b --- /dev/null +++ b/packages/core/src/recovery/cancellation.ts @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/cancellation.ts +import {failure, type Outcome} from './outcome.js'; + +/** + * Wraps a cancellation or interruption throwable into a Failure (RECOV-11). + * + * The reference requires re-asserting the cancellation signal on the current context when wrapping, + * so code later blocked on the outcome still observes cancellation — a concern specific to a + * clearable `Thread.interrupt()` flag. Node has nothing to re-assert: an `AbortSignal` stays + * aborted once fired, and the SDK holds a signal, never the caller's `AbortController`, so it could + * not set one anyway. The helper therefore degenerates to `failure(error)`, and exists as the one + * named, findable site where RECOV-11's Node disposition lives. + * + * It deliberately does **not** crash on a `CancellationError` whose paired signal never aborted. + * `Transport` is a pluggable seam, so that mismatch is a misbehaving third-party implementation — + * an operational failure, not a violated precondition of this codebase, and crash-loud treatment is + * reserved for the latter (`docs/knowledge/error-handling.md`). It would also break RECOV-2: this + * runs inside `dispatchWithRecovery`'s own `catch`, so throwing here would let a transport failure + * skip the response and recovery chains entirely, which is precisely what RECOV-2 forbids. A + * transport that aborts its in-flight requests from `close()` — which SEAM-14 permits — produces + * exactly that shape while the caller passed no signal at all. + * + * This function never throws, for any input. + * + * @param error - whatever the request chain or the transport raised. + * @returns a failure outcome carrying `error` unchanged. + * + * @internal + */ +export function wrapCancellation(error: unknown): Outcome { + return failure(error); +} diff --git a/packages/core/src/recovery/orchestrator.test.ts b/packages/core/src/recovery/orchestrator.test.ts new file mode 100644 index 0000000..aae4ec0 --- /dev/null +++ b/packages/core/src/recovery/orchestrator.test.ts @@ -0,0 +1,370 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/orchestrator.test.ts +// Exercises: RECOV-2 (one try/catch wraps the request chain AND the transport invocation; no +// throwable from either bypasses the recovery hooks), RECOV-10 (unwrap: a Success returns the +// response, a Failure rethrows the throwable unchanged, no wrapping or substitution), RECOV-11 (the +// catch routes every throwable through wrapCancellation, so the helper sits on the real dispatch +// path rather than being an unwired primitive) +import {describe, expect, test} from 'bun:test'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {RequestOptions} from '../http/request-options.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {CancellationError, type Transport} from '../seams/transport.js'; +import {dispatchWithRecovery} from './orchestrator.js'; +import {success} from './outcome.js'; +import {RequestRecoveryChain, type RequestStep} from './request-chain.js'; +import { + ResponseRecoveryChain, + type RecoveryStep, + type ResponseStep, +} from './response-chain.js'; + +function aRequest(): Request { + return Request.newBuilder().url('https://example.com').build(); +} + +function aResponse( + request: Request, + body: ReadableStream | null = null, +): Response { + return Response.newBuilder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .body(body) + .build(); +} + +/** Close is observed through the body stream's `cancel()` — `Response` is frozen. */ +function countingCloseBody(): { + body: ReadableStream; + closeCount: () => number; +} { + let cancels = 0; + return { + body: new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + cancels += 1; + }, + }), + closeCount: () => cancels, + }; +} + +/** Awaits `promise`, returning whatever it rejected with — `undefined` when it resolved. */ +async function rejection(promise: Promise): Promise { + try { + await promise; + return undefined; + } catch (error) { + return error; + } +} + +type SendImpl = ( + request: Request, + options?: RequestOptions, + signal?: AbortSignal, +) => Promise; + +/** A minimal, file-local Transport stub — no shared FakeTransport exists yet. */ +class StubTransport implements Transport { + readonly #impl: SendImpl; + + constructor(impl: SendImpl) { + this.#impl = impl; + } + + send( + request: Request, + options?: RequestOptions, + signal?: AbortSignal, + ): Promise { + return this.#impl(request, options, signal); + } + + close(): Promise { + return Promise.resolve(); + } +} + +function emptyChains(): { + requestChain: RequestRecoveryChain; + responseChain: ResponseRecoveryChain; +} { + return { + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], []), + }; +} + +describe('dispatchWithRecovery happy path', () => { + test('returns the transport response when everything succeeds', async () => { + const request = aRequest(); + const response = aResponse(request); + const transport = new StubTransport(() => Promise.resolve(response)); + + const result = await dispatchWithRecovery(request, { + transport, + ...emptyChains(), + }); + + expect(result).toBe(response); + }); + + test('sends the request the request chain produced, not the one the caller handed in', async () => { + const tagStep: RequestStep = request => + Promise.resolve( + request + .newBuilder() + .headers( + request.headers.newBuilder().set('X-Trace', 'tagged').build(), + ) + .build(), + ); + let sent: Request | undefined; + const transport = new StubTransport(request => { + sent = request; + return Promise.resolve(aResponse(request)); + }); + + await dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([tagStep]), + responseChain: new ResponseRecoveryChain([], []), + }); + + expect(sent?.headers.get('X-Trace')).toBe('tagged'); + }); + + test('threads per-call options and signal through to the transport unchanged', async () => { + const request = aRequest(); + const options = RequestOptions.EMPTY; + const controller = new AbortController(); + let receivedOptions: RequestOptions | undefined; + let receivedSignal: AbortSignal | undefined; + const transport = new StubTransport((req, opts, signal) => { + receivedOptions = opts; + receivedSignal = signal; + return Promise.resolve(aResponse(req)); + }); + + await dispatchWithRecovery(request, { + transport, + ...emptyChains(), + options, + signal: controller.signal, + }); + + expect(receivedOptions).toBe(options); + expect(receivedSignal).toBe(controller.signal); + }); +}); + +describe('RECOV-2: every throwable from the request chain or the transport is caught', () => { + test('a throwing request step surfaces as a Failure to a recovery hook, not an unhandled throw', async () => { + const thrownError = new Error('request step failed'); + const failingStep: RequestStep = () => { + throw thrownError; + }; + const seenByRecovery: unknown[] = []; + const recoveryStep: RecoveryStep = outcome => { + if (outcome.kind === 'failure') seenByRecovery.push(outcome.error); + return Promise.resolve(outcome); + }; + const transport = new StubTransport(() => { + throw new Error('must not run — the request chain already failed'); + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([failingStep]), + responseChain: new ResponseRecoveryChain([], [recoveryStep]), + }), + ); + + expect(error).toBe(thrownError); + expect(seenByRecovery).toEqual([thrownError]); + }); + + test('a throwing transport surfaces as a Failure to a recovery hook, not an unhandled throw', async () => { + const thrownError = new Error('transport failed'); + const seenByRecovery: unknown[] = []; + const recoveryStep: RecoveryStep = outcome => { + if (outcome.kind === 'failure') seenByRecovery.push(outcome.error); + return Promise.resolve(outcome); + }; + const transport = new StubTransport(() => { + throw thrownError; + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], [recoveryStep]), + }), + ); + + expect(error).toBe(thrownError); + expect(seenByRecovery).toEqual([thrownError]); + }); + + test('a recovery step can turn a transport failure back into a Success', async () => { + const request = aRequest(); + const fallback = aResponse(request); + const recoverStep: RecoveryStep = outcome => + Promise.resolve(outcome.kind === 'failure' ? success(fallback) : outcome); + const transport = new StubTransport(() => { + throw new Error('transport failed'); + }); + + const result = await dispatchWithRecovery(request, { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], [recoverStep]), + }); + + expect(result).toBe(fallback); + }); +}); + +describe('RECOV-10: the final unwrap is unchanged, no wrapping or substitution', () => { + test('a response step throwing a typed error surfaces exactly that error, by identity', async () => { + class MyTypedError extends Error {} + const typedError = new MyTypedError('mapped'); + const mapToTypedError: ResponseStep = () => { + throw typedError; + }; + const transport = new StubTransport(request => + Promise.resolve(aResponse(request)), + ); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([mapToTypedError], []), + }), + ); + + expect(error).toBe(typedError); + }); + + test('a non-Error throwable is rethrown as-is, not coerced into an Error', async () => { + const transport = new StubTransport(() => { + // The point of the case: a JS throw can legally raise any value, and RECOV-10 requires the + // orchestrator to rethrow it by identity rather than coercing it into an Error. Re-enable if + // the transport seam ever narrows what an implementation may reject with. + // eslint-disable-next-line @typescript-eslint/only-throw-error -- see the comment above + throw 'a string throw'; + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), {transport, ...emptyChains()}), + ); + + expect(error).toBe('a string throw'); + }); +}); + +describe('RECOV-11: the catch routes every throwable through wrapCancellation', () => { + test('a transport CancellationError paired with an aborted signal surfaces unchanged', async () => { + const controller = new AbortController(); + const cancellation = new CancellationError('aborted by caller'); + const transport = new StubTransport(() => { + controller.abort(); + throw cancellation; + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], []), + signal: controller.signal, + }), + ); + + expect(error).toBe(cancellation); + }); + + test('a CancellationError raised with no caller signal still reaches the recovery chain (RECOV-2)', async () => { + // A transport may abort its own in-flight requests for reasons the caller never signalled — + // SEAM-14 permits close() to cancel them. That must surface as an ordinary Failure through the + // recovery hooks, not as a side exit: RECOV-2 admits no throwable from the transport bypassing + // them. + const cancellation = new CancellationError( + 'aborted by the transport itself', + ); + const seenByRecovery: unknown[] = []; + const recoveryStep: RecoveryStep = outcome => { + if (outcome.kind === 'failure') seenByRecovery.push(outcome.error); + return Promise.resolve(outcome); + }; + const transport = new StubTransport(() => { + throw cancellation; + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], [recoveryStep]), + }), + ); + + expect(error).toBe(cancellation); + expect(seenByRecovery).toEqual([cancellation]); + }); +}); + +describe('negative space: the orchestrator releases nothing of its own', () => { + test('the response it hands back is left open for the caller to close', async () => { + // RECOV-10 returns the contained response; ownership passes to the caller. A future + // "helpful" close here would hand back a response whose body is already cancelled. + const request = aRequest(); + const {body, closeCount} = countingCloseBody(); + const transport = new StubTransport(() => + Promise.resolve(aResponse(request, body)), + ); + + const result = await dispatchWithRecovery(request, { + transport, + ...emptyChains(), + }); + + expect(closeCount()).toBe(0); + expect(result.body).not.toBeNull(); + }); + + test('it never closes the transport it was handed', async () => { + // SEAM-14/PIPE-27's discipline, asserted early: the orchestrator borrows the transport, it + // does not own it. + let closeCalls = 0; + const request = aRequest(); + const transport = new StubTransport(() => + Promise.resolve(aResponse(request)), + ); + const countingTransport: Transport = { + send: (...args) => transport.send(...args), + close: () => { + closeCalls += 1; + return Promise.resolve(); + }, + }; + + await dispatchWithRecovery(request, { + transport: countingTransport, + ...emptyChains(), + }); + + expect(closeCalls).toBe(0); + }); +}); diff --git a/packages/core/src/recovery/orchestrator.ts b/packages/core/src/recovery/orchestrator.ts new file mode 100644 index 0000000..a8acac4 --- /dev/null +++ b/packages/core/src/recovery/orchestrator.ts @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/orchestrator.ts +import type {Request} from '../http/request.js'; +import type {RequestOptions} from '../http/request-options.js'; +import type {Response} from '../http/response.js'; +import type {Transport} from '../seams/transport.js'; +import {wrapCancellation} from './cancellation.js'; +import {fold, success, type Outcome} from './outcome.js'; +import type {RequestRecoveryChain} from './request-chain.js'; +import type {ResponseRecoveryChain} from './response-chain.js'; + +/** + * Everything {@link dispatchWithRecovery} needs beyond the request itself, bundled into one + * trailing object. Five positional parameters would fail ESLint's `max-params: 3`. + * + * @internal + */ +export interface DispatchConfig { + /** The terminal transport hop. */ + readonly transport: Transport; + /** Run before the transport hop; its throwables become a Failure (RECOV-2). */ + readonly requestChain: RequestRecoveryChain; + /** Run on the outcome, whatever it is (RECOV-4 … RECOV-8). */ + readonly responseChain: ResponseRecoveryChain; + /** Per-call operational overrides, threaded to the transport unchanged. */ + readonly options?: RequestOptions | undefined; + /** The caller's abort signal, threaded to the transport unchanged. */ + readonly signal?: AbortSignal | undefined; +} + +/** + * The unified recovery-chain orchestrator (RECOV-2, RECOV-10, RECOV-11). + * + * One `try`/`catch` wraps both the request chain's `apply()` and the transport invocation, so every + * throwable from either is caught and converted into a Failure before the response chain runs — a + * before-request throw cannot skip after-error handling. That conversion goes through + * {@link wrapCancellation} (RECOV-11), this orchestrator's catch being its only call site; RECOV-2's + * guarantee rests on that helper never throwing, since this catch clause is the last place a + * throwable could escape without meeting the recovery hooks. + * + * The final unwrap returns the response on a Success, or rethrows the Failure's throwable + * **unchanged** — no wrapping, no substitution (RECOV-10). Surfacing a typed exception is a + * recovery step's own responsibility, never this function's. + * + * @param request - the request to prepare and send. + * @param config - transport, chains, and the per-call options and signal. + * @returns the response the terminal outcome carries. + * @throws Whatever the terminal Failure carries, by identity — any value, not necessarily an + * `Error`. + * + * @internal + */ +export async function dispatchWithRecovery( + request: Request, + config: DispatchConfig, +): Promise { + let outcome: Outcome; + try { + const preparedRequest = await config.requestChain.apply(request); + outcome = success( + await config.transport.send( + preparedRequest, + config.options, + config.signal, + ), + ); + } catch (error) { + // RECOV-11: `Outcome` widens to `Outcome` without a cast, and never throws, + // which is what keeps RECOV-2 absolute. + outcome = wrapCancellation(error); + } + const finalOutcome = await config.responseChain.apply(outcome); + return fold( + finalOutcome, + response => response, + error => { + throw error; + }, + ); +} diff --git a/packages/core/src/recovery/outcome.test.ts b/packages/core/src/recovery/outcome.test.ts new file mode 100644 index 0000000..9cc38d4 --- /dev/null +++ b/packages/core/src/recovery/outcome.test.ts @@ -0,0 +1,157 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/outcome.test.ts +// Exercises: RECOV-1 (closed two-variant sum type, mutually exclusive and jointly exhaustive, with a +// fold that applies exactly one of two branches at most once per call) +import {describe, expect, test} from 'bun:test'; +import {expectTypeOf} from 'expect-type'; +import fc from 'fast-check'; +import {failure, fold, success, type Outcome} from './outcome.js'; + +describe('success / failure (RECOV-1)', () => { + test('success carries its value under kind "success"', () => { + const outcome = success(42); + + expect(outcome.kind).toBe('success'); + expect(outcome.kind === 'success' && outcome.value).toBe(42); + }); + + test('failure carries its error under kind "failure", typed unknown', () => { + // A JS throw can legally raise any value, not only an Error (sdk-design-nodejs/05). + const outcome = failure('a string throw'); + + expect(outcome.kind).toBe('failure'); + expect(outcome.kind === 'failure' && outcome.error).toBe('a string throw'); + }); +}); + +describe('fold (RECOV-1)', () => { + test('applies onSuccess for a success outcome', () => { + const result = fold( + success(10), + v => v * 2, + () => -1, + ); + + expect(result).toBe(20); + }); + + test('applies onFailure for a failure outcome', () => { + const error = new Error('boom'); + + const result = fold( + failure(error), + () => 'unreachable', + e => e, + ); + + expect(result).toBe(error); + }); + + test('invokes exactly one branch, never both, for either variant', () => { + let successCalls = 0; + let failureCalls = 0; + const onSuccess = (): string => { + successCalls += 1; + return 'ok'; + }; + const onFailure = (): string => { + failureCalls += 1; + return 'err'; + }; + + fold(success(1), onSuccess, onFailure); + fold(failure(new Error('x')), onSuccess, onFailure); + + expect(successCalls).toBe(1); + expect(failureCalls).toBe(1); + }); +}); + +describe('fold identity law (RECOV-1)', () => { + // Canonical law for an invariant-bearing function (docs/knowledge/testing.md): folding a success + // through the identity success-handler, and a failure through the identity failure-handler, must + // each recover the original payload, for arbitrary values. + test('fold(success(x), id, _) === x for arbitrary x', () => { + fc.assert( + fc.property(fc.anything(), value => { + expect( + fold( + success(value), + v => v, + () => 'unreachable', + ), + ).toBe(value); + }), + ); + }); + + test('fold(failure(e), _, id) === e for arbitrary e', () => { + fc.assert( + fc.property(fc.anything(), error => { + expect( + fold( + failure(error), + () => 'unreachable', + e => e, + ), + ).toBe(error); + }), + ); + }); +}); + +describe('Outcome as a type (RECOV-1)', () => { + // An exported generic type ships with a type-level test (styleguide 11.6). These only fire under + // `bun run typecheck` — `bun test` executes this file but strips its types without checking them. + test('the two variants are closed and jointly exhaustive', () => { + expectTypeOf['kind']>().toEqualTypeOf< + 'success' | 'failure' + >(); + }); + + test('narrowing on kind reaches the variant payload, and only that payload', () => { + expectTypeOf< + Extract, {kind: 'success'}>['value'] + >().toEqualTypeOf(); + expectTypeOf< + Extract, {kind: 'failure'}>['error'] + >().toEqualTypeOf(); + }); + + test('a narrowed success has no error field (negative case)', () => { + const outcome: Outcome = success(1); + if (outcome.kind !== 'success') throw new Error('unreachable seed'); + + // @ts-expect-error -- RECOV-1: the variants are mutually exclusive, so a narrowed success has + // no `error` to read. If this line ever compiles, the union has stopped being closed. + const read: unknown = outcome.error; + + expect(read).toBeUndefined(); + }); + + test('a narrowed failure has no value field (negative case)', () => { + const outcome: Outcome = failure(new Error('x')); + if (outcome.kind !== 'failure') throw new Error('unreachable seed'); + + // @ts-expect-error -- the mirror of the case above. + const read: unknown = outcome.value; + + expect(read).toBeUndefined(); + }); + + test('fold collapses both branches to one result type', () => { + expectTypeOf( + fold( + success(1), + v => v, + () => 0, + ), + ).toEqualTypeOf(); + }); + + test('failure() infers the caller-declared payload type, not the error type', () => { + expectTypeOf(failure(new Error('x'))).toEqualTypeOf< + Outcome + >(); + }); +}); diff --git a/packages/core/src/recovery/outcome.ts b/packages/core/src/recovery/outcome.ts new file mode 100644 index 0000000..671bf4a --- /dev/null +++ b/packages/core/src/recovery/outcome.ts @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/outcome.ts +import {assertNever} from '../invariant.js'; + +/** + * The recovery chain's closed two-variant outcome (RECOV-1): a success carrying a value, or a + * failure carrying whatever was thrown. + * + * `error` is `unknown`, not `Error` — a JavaScript `throw` can legally raise any value, and this + * type sits directly under a `catch`. The discriminated union is what RECOV-1's "derivable + * accessors" buys in TypeScript: narrowing on `kind` is compiler-checked, so no `isSuccess()` / + * `getOrThrow()` pair is shipped. + * + * @internal + */ +export type Outcome = + | {readonly kind: 'success'; readonly value: T} + | {readonly kind: 'failure'; readonly error: unknown}; + +/** + * Builds the success variant. + * + * @param value - the value carried by the outcome. + * @returns a success outcome holding `value`. + * + * @internal + */ +export function success(value: T): Outcome { + return {kind: 'success', value}; +} + +/** + * Builds the failure variant. + * + * @param error - whatever was thrown; any value, not necessarily an `Error`. + * @returns a failure outcome holding `error`. + * + * @internal + */ +export function failure(error: unknown): Outcome { + return {kind: 'failure', error}; +} + +/** + * Applies exactly one of `onSuccess` / `onFailure`, never both, satisfying RECOV-1's "a fold that + * applies exactly one of two branches at most once per call." + * + * Three positional parameters rather than an options object: `max-params` errors at four, and this + * matches Phase 2's already-shipped `Transport.send(request, options?, signal?)`. Recorded as a + * corpus deviation in the phase design's ledger. + * + * @param outcome - the outcome to fold. + * @param onSuccess - applied to the value of a success outcome. + * @param onFailure - applied to the error of a failure outcome. + * @returns whichever branch ran. + * + * @internal + */ +export function fold( + outcome: Outcome, + onSuccess: (value: T) => R, + onFailure: (error: unknown) => R, +): R { + switch (outcome.kind) { + case 'success': + return onSuccess(outcome.value); + case 'failure': + return onFailure(outcome.error); + default: + return assertNever(outcome); + } +} diff --git a/packages/core/src/recovery/request-chain.test.ts b/packages/core/src/recovery/request-chain.test.ts new file mode 100644 index 0000000..a63a69a --- /dev/null +++ b/packages/core/src/recovery/request-chain.test.ts @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/request-chain.test.ts +// Exercises: RECOV-3 (sequential left-to-right fold, empty chain is the identity, a throwing step +// aborts the remainder and propagates), RECOV-14 (defensive copy of the step list at construction) +import {describe, expect, test} from 'bun:test'; +import fc from 'fast-check'; +import {Request} from '../http/request.js'; +import {RequestRecoveryChain, type RequestStep} from './request-chain.js'; + +function aRequest(): Request { + return Request.newBuilder().url('https://example.com').build(); +} + +function tagAppendStep(char: string): RequestStep { + return request => { + const current = request.headers.get('X-Trace') ?? ''; + return Promise.resolve( + request + .newBuilder() + .headers( + request.headers + .newBuilder() + .set('X-Trace', current + char) + .build(), + ) + .build(), + ); + }; +} + +/** Awaits `promise`, returning whatever it rejected with — `undefined` when it resolved. */ +async function rejection(promise: Promise): Promise { + try { + await promise; + return undefined; + } catch (error) { + return error; + } +} + +describe('RequestRecoveryChain.apply (RECOV-3)', () => { + test('an empty chain returns the input unchanged', async () => { + const chain = new RequestRecoveryChain([]); + const request = aRequest(); + + const result = await chain.apply(request); + + expect(result).toBe(request); + }); + + test('applies steps as a sequential left-to-right fold', async () => { + const chain = new RequestRecoveryChain([ + tagAppendStep('a'), + tagAppendStep('b'), + tagAppendStep('c'), + ]); + + const result = await chain.apply(aRequest()); + + expect(result.headers.get('X-Trace')).toBe('abc'); + }); + + test('a throwing step aborts the remainder and propagates', async () => { + const reached: string[] = []; + const thrownError = new Error('step failed'); + const failingStep: RequestStep = () => { + throw thrownError; + }; + const laterStep: RequestStep = request => { + reached.push('later'); + return Promise.resolve(request); + }; + const chain = new RequestRecoveryChain([ + tagAppendStep('a'), + failingStep, + laterStep, + ]); + + expect(await rejection(chain.apply(aRequest()))).toBe(thrownError); + expect(reached).toEqual([]); + }); +}); + +describe('RequestRecoveryChain construction (RECOV-14)', () => { + test('defensively copies its step list — mutating the source after construction has no effect', async () => { + const steps: RequestStep[] = [tagAppendStep('a')]; + const chain = new RequestRecoveryChain(steps); + steps.push(tagAppendStep('b')); + + const result = await chain.apply(aRequest()); + + expect(result.headers.get('X-Trace')).toBe('a'); + }); +}); + +describe('RequestRecoveryChain.apply fold law', () => { + // Canonical law for an invariant-bearing function: applying the chain equals manually reducing + // the same steps in order, for an arbitrary sequence of single-character append steps. + test('apply() equals a manual left-to-right reduce, for arbitrary step sequences', async () => { + await fc.assert( + // `fc.string({minLength: 1, maxLength: 1})` rather than `fc.char()`: the latter is deprecated + // in fast-check 3.22+ and would print a deprecation warning on every run. + fc.asyncProperty( + fc.array(fc.string({minLength: 1, maxLength: 1}), {maxLength: 10}), + async chars => { + const steps = chars.map(tagAppendStep); + const chain = new RequestRecoveryChain(steps); + + const chained = await chain.apply(aRequest()); + let manual = aRequest(); + for (const step of steps) manual = await step(manual); + + expect(chained.headers.get('X-Trace')).toBe( + manual.headers.get('X-Trace'), + ); + }, + ), + ); + }); +}); + +describe('RECOV-14: steps are safe for concurrent invocation', () => { + // RECOV-14's second normative clause binds both chains, not only the response one: per-request + // state lives in the value being transformed, never on the step or the chain instance. Guards + // the structural property that `apply()`'s only per-call state is its `current` local. + test('two interleaved apply() calls on ONE chain instance do not observe each other', async () => { + const gate: (() => void)[] = []; + const slowStep: RequestStep = async request => { + await new Promise(resolve => gate.push(resolve)); + return request; + }; + const chain = new RequestRecoveryChain([slowStep, tagAppendStep('x')]); + const first = Request.newBuilder().url('https://example.com/first').build(); + const second = Request.newBuilder() + .url('https://example.com/second') + .build(); + + const firstCall = chain.apply(first); + const secondCall = chain.apply(second); + await Promise.resolve(); + for (const release of gate) release(); + const [firstResult, secondResult] = await Promise.all([ + firstCall, + secondCall, + ]); + + expect(firstResult.url.pathname).toBe('/first'); + expect(secondResult.url.pathname).toBe('/second'); + expect(firstResult.headers.get('X-Trace')).toBe('x'); + expect(secondResult.headers.get('X-Trace')).toBe('x'); + }); +}); diff --git a/packages/core/src/recovery/request-chain.ts b/packages/core/src/recovery/request-chain.ts new file mode 100644 index 0000000..ca40301 --- /dev/null +++ b/packages/core/src/recovery/request-chain.ts @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/request-chain.ts +import type {Request} from '../http/request.js'; + +/** + * One link of the request-preparation chain. Async like every other step type in this layer — Node + * has a single execution model, so the phase does not mix sync and async step shapes. + * + * @internal + */ +export type RequestStep = (request: Request) => Promise; + +/** + * A sequential left-to-right fold over request steps (RECOV-3): the output of step N is the input + * of step N+1, an empty chain returns its input unchanged, and a throwing step aborts the remainder + * and propagates — `dispatchWithRecovery` (`orchestrator.ts`) converts that propagation into a + * `Failure` per RECOV-2, which is the only reason propagating here is safe. + * + * Safe under concurrent `apply()` calls (RECOV-14): after construction the instance holds nothing + * but its step array, and every piece of per-call state lives in `apply()`'s locals. A later phase + * must not move per-call bookkeeping onto a field here. + * + * @internal + */ +export class RequestRecoveryChain { + readonly #steps: readonly RequestStep[]; + + /** + * Defensively copies `steps` (RECOV-14). The reference implementation retains the caller's array + * by reference on this chain only — an asymmetry the requirement's own text recommends a port not + * reproduce. + * + * @param steps - the ordered request steps. + */ + constructor(steps: readonly RequestStep[]) { + this.#steps = [...steps]; + } + + /** + * Folds the request through every step in order. + * + * @param request - the request to prepare. + * @returns the request produced by the last step, or the input when the chain is empty. + * @throws Whatever a step throws, aborting the remaining steps (RECOV-3). + */ + async apply(request: Request): Promise { + let current = request; + for (const step of this.#steps) { + current = await step(current); + } + return current; + } +} diff --git a/packages/core/src/recovery/response-chain.test.ts b/packages/core/src/recovery/response-chain.test.ts new file mode 100644 index 0000000..a9e14ae --- /dev/null +++ b/packages/core/src/recovery/response-chain.test.ts @@ -0,0 +1,426 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/response-chain.test.ts +// Exercises: RECOV-4 (response steps run only on a Success), RECOV-5/RECOV-6 (recovery steps run on +// every outcome; fold order is all response steps then all recovery steps), RECOV-7 (a throwing +// response step becomes a Failure fed to recovery, never propagated), RECOV-8 (a throwing recovery +// step becomes a Failure fed to the NEXT recovery step; apply() never throws), RECOV-12 +// (close-on-throw while holding a Success, close failure attached as `suppressed` with the original +// throwable staying primary), RECOV-13 (a deliberately returned substitute outcome is never +// auto-closed), RECOV-14 (both step lists defensively copied, and steps safe for concurrent +// invocation — no per-call state on the chain instance) +import {describe, expect, test} from 'bun:test'; +import fc from 'fast-check'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {failure, success, type Outcome} from './outcome.js'; +import { + ResponseRecoveryChain, + type RecoveryStep, + type ResponseStep, +} from './response-chain.js'; + +function aResponse(body: ReadableStream | null = null): Response { + return Response.newBuilder() + .request(Request.newBuilder().url('https://example.com').build()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .body(body) + .build(); +} + +/** + * Close is observed through the body stream's `cancel()`, exactly the way Phase 3b's own + * `response.test.ts` observes it — NOT by patching `response.close`. `Response` calls + * `Object.freeze(this)` at the end of its constructor, so `response.close = ...` throws + * `TypeError: Cannot add property close, object is not extensible` in an ES module's strict mode. + * `Response.close()` is memoized and cancels the body at most once, so the cancel count IS the + * effective-close count RECOV-12's "released exactly once" asks about. + */ +function countingCloseResponse(): { + response: Response; + closeCount: () => number; +} { + let cancels = 0; + const body = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + cancels += 1; + }, + }); + return {response: aResponse(body), closeCount: () => cancels}; +} + +/** + * A response whose `close()` rejects. `Response.close()` awaits `body.cancel()` and swallows only + * `TypeError` (the locked-stream case), so a plain `Error` propagates out of `close()`. + */ +function failingCloseResponse(closeError: Error): Response { + return aResponse( + new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + throw closeError; + }, + }), + ); +} + +describe('response-step phase (RECOV-4, RECOV-6)', () => { + test('response steps run in order on a Success outcome', async () => { + const seen: string[] = []; + const stepA: ResponseStep = r => { + seen.push('a'); + return Promise.resolve(r); + }; + const stepB: ResponseStep = r => { + seen.push('b'); + return Promise.resolve(r); + }; + const chain = new ResponseRecoveryChain([stepA, stepB], []); + + await chain.apply(success(aResponse())); + + expect(seen).toEqual(['a', 'b']); + }); + + test('response steps do not run when the input outcome is already a Failure', async () => { + const original = new Error('original'); + const stepShouldNotRun: ResponseStep = () => { + throw new Error('must not run'); + }; + const chain = new ResponseRecoveryChain([stepShouldNotRun], []); + + const result = await chain.apply(failure(original)); + + expect(result.kind).toBe('failure'); + expect(result.kind === 'failure' && result.error).toBe(original); + }); +}); + +describe('recovery-step phase (RECOV-5, RECOV-6)', () => { + test('recovery steps run on every outcome, successes and failures, in order', async () => { + const seenKinds: string[] = []; + const record: RecoveryStep = outcome => { + seenKinds.push(outcome.kind); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain([], [record, record]); + + await chain.apply(success(aResponse())); + await chain.apply(failure(new Error('x'))); + + expect(seenKinds).toEqual(['success', 'success', 'failure', 'failure']); + }); + + test('fold order is all response steps first, then all recovery steps', async () => { + const order: string[] = []; + const responseStep: ResponseStep = r => { + order.push('response'); + return Promise.resolve(r); + }; + const recoveryStep: RecoveryStep = outcome => { + order.push('recovery'); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain([responseStep], [recoveryStep]); + + await chain.apply(success(aResponse())); + + expect(order).toEqual(['response', 'recovery']); + }); +}); + +describe('RECOV-7: a throwing response step converts to a Failure fed to recovery', () => { + test('the throwable never propagates out of apply(), and recovery observes the Failure', async () => { + const stepAfterThatMustNotRun: ResponseStep = () => { + throw new Error( + 'must not run — the response phase stops after the throw', + ); + }; + const thrownError = new Error('response step failed'); + const throwingStep: ResponseStep = () => { + throw thrownError; + }; + const seenByRecovery: Outcome[] = []; + const recoveryStep: RecoveryStep = outcome => { + seenByRecovery.push(outcome); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain( + [throwingStep, stepAfterThatMustNotRun], + [recoveryStep], + ); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + expect(result.kind === 'failure' && result.error).toBe(thrownError); + expect(seenByRecovery).toHaveLength(1); + expect(seenByRecovery[0]?.kind).toBe('failure'); + }); +}); + +describe('RECOV-8: a throwing recovery step wraps into a Failure fed to the next step', () => { + test('apply() never throws, and the remaining recovery steps still run', async () => { + const secondStepSeen: Outcome[] = []; + const throwingRecoveryStep: RecoveryStep = () => { + throw new Error('recovery step failed'); + }; + const secondRecoveryStep: RecoveryStep = outcome => { + secondStepSeen.push(outcome); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain( + [], + [throwingRecoveryStep, secondRecoveryStep], + ); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + expect(secondStepSeen).toHaveLength(1); + expect(secondStepSeen[0]?.kind).toBe('failure'); + }); +}); + +describe('RECOV-12: close-on-throw while holding a Success', () => { + test('closes the in-hand response exactly once before wrapping the throwable', async () => { + const {response, closeCount} = countingCloseResponse(); + const thrownError = new Error('step failed'); + const throwingStep: ResponseStep = () => { + throw thrownError; + }; + const chain = new ResponseRecoveryChain([throwingStep], []); + + const result = await chain.apply(success(response)); + + expect(closeCount()).toBe(1); + expect(result.kind === 'failure' && result.error).toBe(thrownError); + }); + + test('a close failure is attached as suppressed, with the original throwable staying primary', async () => { + const closeError = new Error('close failed'); + const response = failingCloseResponse(closeError); + const originalError = new Error('step failed'); + const throwingStep: ResponseStep = () => { + throw originalError; + }; + const chain = new ResponseRecoveryChain([throwingStep], []); + + const result = await chain.apply(success(response)); + + expect(result.kind).toBe('failure'); + const wrapped = result.kind === 'failure' ? result.error : undefined; + expect(wrapped).toBeInstanceOf(Error); + expect((wrapped as SuppressedErrorShape).name).toBe('SuppressedError'); + expect((wrapped as SuppressedErrorShape).error).toBe(originalError); + expect((wrapped as SuppressedErrorShape).suppressed).toBe(closeError); + }); + + test('a throwing recovery step holding a Success also closes it exactly once', async () => { + const {response, closeCount} = countingCloseResponse(); + const thrownError = new Error('recovery step failed'); + const throwingRecoveryStep: RecoveryStep = () => { + throw thrownError; + }; + const chain = new ResponseRecoveryChain([], [throwingRecoveryStep]); + + const result = await chain.apply(success(response)); + + expect(closeCount()).toBe(1); + expect(result.kind === 'failure' && result.error).toBe(thrownError); + }); + + test('a throwing step holding a Failure closes nothing — there is no response in hand', async () => { + const thrownError = new Error('recovery step failed'); + const throwingRecoveryStep: RecoveryStep = () => { + throw thrownError; + }; + const chain = new ResponseRecoveryChain([], [throwingRecoveryStep]); + + const result = await chain.apply(failure(new Error('seed'))); + + expect(result.kind === 'failure' && result.error).toBe(thrownError); + }); +}); + +interface SuppressedErrorShape extends Error { + readonly error: unknown; + readonly suppressed: unknown; +} + +describe('RECOV-13: a deliberate outcome substitution is never auto-closed', () => { + test('a recovery step returning a different Failure does not trigger a close', async () => { + const {response, closeCount} = countingCloseResponse(); + const substituteStep: RecoveryStep = () => + Promise.resolve(failure(new Error('substituted, not thrown'))); + const chain = new ResponseRecoveryChain([], [substituteStep]); + + await chain.apply(success(response)); + + expect(closeCount()).toBe(0); + }); + + test('a recovery step substituting a different Success does not trigger a close', async () => { + const {response: original, closeCount} = countingCloseResponse(); + const substitute = aResponse(); + const substituteStep: RecoveryStep = () => + Promise.resolve(success(substitute)); + const chain = new ResponseRecoveryChain([], [substituteStep]); + + const result = await chain.apply(success(original)); + + expect(closeCount()).toBe(0); + expect(result.kind === 'success' && result.value).toBe(substitute); + }); +}); + +describe('RECOV-14: both step lists are defensively copied', () => { + test('mutating the source arrays after construction has no effect on apply()', async () => { + const responseSteps: ResponseStep[] = []; + const recoverySteps: RecoveryStep[] = []; + const chain = new ResponseRecoveryChain(responseSteps, recoverySteps); + responseSteps.push(() => { + throw new Error('must not run — pushed after construction'); + }); + recoverySteps.push(outcome => Promise.resolve(outcome)); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('success'); + }); +}); + +describe('RECOV-8: apply() never throws, including on a step that lies about its type', () => { + // RECOV-8 is absolute — "MUST NOT throw under any input" — and `recovery/` is plumbing a later + // phase (and eventually a caller) installs steps into. TypeScript cannot enforce the return type + // across that seam, so the two shapes a mistyped step produces are pinned here: a step whose + // return value is not an outcome at all, and a step that then trips over it. Before this was + // guarded, the second case raised `TypeError: undefined is not an object` out of `apply()`. + test('a recovery step returning a non-outcome does not make apply() throw', async () => { + const returnsNothing = (() => + Promise.resolve(undefined)) as unknown as RecoveryStep; + const readsTheOutcome: RecoveryStep = outcome => + Promise.resolve(outcome.kind === 'failure' ? outcome : outcome); + const chain = new ResponseRecoveryChain( + [], + [returnsNothing, readsTheOutcome], + ); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + }); + + test('a response step returning a non-response does not make apply() throw', async () => { + const returnsNothing = (() => + Promise.resolve(undefined)) as unknown as ResponseStep; + const readsTheResponse: ResponseStep = response => + Promise.resolve(response.status.isError ? response : response); + const chain = new ResponseRecoveryChain( + [returnsNothing, readsTheResponse], + [], + ); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + }); + + test("the step's own throwable stays primary when the plumbing also fails", async () => { + const returnsNothing = (() => + Promise.resolve(undefined)) as unknown as ResponseStep; + const thrownError = new Error('step failed on a poisoned outcome'); + const throwingStep: ResponseStep = () => { + throw thrownError; + }; + const chain = new ResponseRecoveryChain([returnsNothing, throwingStep], []); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + const error = result.kind === 'failure' ? result.error : undefined; + expect((error as SuppressedErrorShape).name).toBe('SuppressedError'); + expect((error as SuppressedErrorShape).error).toBe(thrownError); + }); +}); + +describe('apply() never throws (RECOV-8 property)', () => { + // Canonical law for an invariant-bearing function: for an arbitrary mix of throwing and + // non-throwing RESPONSE AND RECOVERY steps, over a seed outcome that is arbitrarily a Success or + // a Failure, apply() always settles and never re-raises a step's throw (RECOV-8) — and no + // response step runs on any generated case whose seed was already a Failure (RECOV-4). + // + // BOTH phases and BOTH seed variants must be generated: a generator emitting recovery steps only, + // or seeding Success only, proves the RECOV-8 law and silently leaves RECOV-4 to the example + // tests above. + test('apply() settles and skips the response phase on a Failure seed, for arbitrary step sequences', async () => { + await fc.assert( + fc.asyncProperty( + fc.array(fc.boolean(), {maxLength: 4}), + fc.array(fc.boolean(), {maxLength: 4}), + fc.boolean(), + async (responseFlags, recoveryFlags, seedIsSuccess) => { + let responseStepRuns = 0; + const responseSteps: ResponseStep[] = responseFlags.map( + (shouldThrow, index) => response => { + responseStepRuns += 1; + if (shouldThrow) + throw new Error(`response step ${String(index)} failed`); + return Promise.resolve(response); + }, + ); + const recoverySteps: RecoveryStep[] = recoveryFlags.map( + (shouldThrow, index) => outcome => { + if (shouldThrow) + throw new Error(`recovery step ${String(index)} failed`); + return Promise.resolve(outcome); + }, + ); + const chain = new ResponseRecoveryChain(responseSteps, recoverySteps); + const seed = seedIsSuccess + ? success(aResponse()) + : failure(new Error('seed failure')); + + const result = await chain.apply(seed); + + expect(['success', 'failure']).toContain(result.kind); + if (!seedIsSuccess) expect(responseStepRuns).toBe(0); // RECOV-4 + }, + ), + ); + }); +}); + +describe('RECOV-14: steps are safe for concurrent invocation', () => { + // RECOV-14's SECOND normative clause: per-request state lives in the passed value, never on the + // step or the chain. Guards the structural property that apply()'s only per-call state is its + // `current` local — a later phase adding per-call bookkeeping to a chain field would fail here. + test('two interleaved apply() calls on ONE chain instance do not observe each other', async () => { + const gate: (() => void)[] = []; + const slowStep: RecoveryStep = async outcome => { + await new Promise(resolve => gate.push(resolve)); + return outcome; + }; + const chain = new ResponseRecoveryChain([], [slowStep]); + const successSeed = success(aResponse()); + const failureSeed = failure(new Error('second call')); + + const first = chain.apply(successSeed); + const second = chain.apply(failureSeed); + await Promise.resolve(); + for (const release of gate) release(); + const [firstResult, secondResult] = await Promise.all([first, second]); + + expect(firstResult.kind).toBe('success'); + expect( + secondResult.kind === 'failure' && (secondResult.error as Error).message, + ).toBe('second call'); + }); +}); diff --git a/packages/core/src/recovery/response-chain.ts b/packages/core/src/recovery/response-chain.ts new file mode 100644 index 0000000..f62349f --- /dev/null +++ b/packages/core/src/recovery/response-chain.ts @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/response-chain.ts +import type {Response} from '../http/response.js'; +import {suppress} from '../suppress.js'; +import {failure, success, type Outcome} from './outcome.js'; + +/** + * One link of the response phase, run only while the outcome is a Success (RECOV-4). + * + * @internal + */ +export type ResponseStep = (response: Response) => Promise; + +/** + * One link of the recovery phase, run on every outcome, success or failure (RECOV-5). + * + * A recovery step SHOULD return a `Failure` rather than throw (RECOV-9); both are handled + * identically, so this is a convention rather than something the chain enforces. + * + * @internal + */ +export type RecoveryStep = ( + outcome: Outcome, +) => Promise>; + +/** + * The response and recovery step folds (RECOV-4 … RECOV-9, RECOV-12, RECOV-13). + * + * Response steps run first and only while the outcome is a Success, in declared order; a throwing + * response step becomes a Failure fed to the recovery phase (RECOV-7) rather than propagating. + * Recovery steps then run on whatever the outcome is by then, always, in declared order; a throwing + * recovery step becomes a Failure fed to the NEXT recovery step (RECOV-8). `apply()` itself never + * throws, for any input. + * + * A step that *returns* a substitute outcome is never auto-closed (RECOV-13) — only a caught throw + * reaches {@link toFailureClosingSuccess}. A transforming step owns releasing whatever it drops. + * + * Safe under concurrent `apply()` calls (RECOV-14): after construction the instance holds nothing + * but its two step arrays, and all per-call state lives in the phase methods' locals. + * + * @internal + */ +export class ResponseRecoveryChain { + readonly #responseSteps: readonly ResponseStep[]; + readonly #recoverySteps: readonly RecoveryStep[]; + + /** + * Defensively copies both lists (RECOV-14). + * + * @param responseSteps - steps run on a Success, in order. + * @param recoverySteps - steps run on every outcome, in order. + */ + constructor( + responseSteps: readonly ResponseStep[], + recoverySteps: readonly RecoveryStep[], + ) { + this.#responseSteps = [...responseSteps]; + this.#recoverySteps = [...recoverySteps]; + } + + /** + * Folds `outcome` through the response phase and then the recovery phase (RECOV-6). + * + * @param outcome - the outcome produced by the transport, or by an earlier failure. + * @returns the terminal outcome. Never throws (RECOV-8). + */ + async apply(outcome: Outcome): Promise> { + const afterResponsePhase = await this.#runResponsePhase(outcome); + return this.#runRecoveryPhase(afterResponsePhase); + } + + async #runResponsePhase( + outcome: Outcome, + ): Promise> { + let current = outcome; + for (const step of this.#responseSteps) { + // RECOV-4: the whole response phase is skipped once the outcome is not a Success. + if (current.kind !== 'success') break; + try { + current = success(await step(current.value)); + } catch (thrownError) { + current = await toFailureClosingSuccess(thrownError, current); // RECOV-7, RECOV-12 + break; // the remaining response steps do not run once converted to a Failure + } + } + return current; + } + + async #runRecoveryPhase( + outcome: Outcome, + ): Promise> { + let current = outcome; + for (const step of this.#recoverySteps) { + try { + // RECOV-13: a normal return substituting the outcome is never auto-closed. + current = await step(current); + } catch (thrownError) { + current = await toFailureClosingSuccess(thrownError, current); // RECOV-8, RECOV-12 + // RECOV-8: the remaining recovery steps still run — deliberately no `break` here. + } + } + return current; + } +} + +/** + * Shared close-on-throw handling for both phases (RECOV-12): when the outcome held at the moment of + * the throw was a Success, its response is released before the throwable is wrapped into a Failure, + * exactly once. + * + * A close failure rides along as `suppressed` on the ORIGINAL throwable — built by hand through + * {@link suppress}, original first — never via `using` / `await using`, whose auto-generated + * `SuppressedError` puts the *teardown* failure first and would silently invert which error the + * caller ends up seeing (`docs/knowledge/resource-management.md:72`). + * + * **This function is total: it never throws, for any argument.** RECOV-8 makes "`apply()` MUST NOT + * throw under any input" absolute, and this runs inside both phases' `catch` blocks — the last place + * a throwable could escape the chain. The discriminant read and the `close()` call are therefore + * inside the same `try`, not just the `close()`: a step that lies about its return type (a JS caller, + * or one returning `undefined`) can leave `current` holding something with no `kind` and no + * `close()`, and reading through it would otherwise raise a `TypeError` out of `apply()`. Handling it + * here rather than crashing is the same call `wrapCancellation` makes — a step is a pluggable seam, + * so a step that misbehaves is an operational failure, not a violated precondition of this codebase. + * The step's own throwable stays primary either way; the plumbing failure rides along as + * `suppressed`. + */ +async function toFailureClosingSuccess( + thrownError: unknown, + current: Outcome, +): Promise> { + try { + if (current.kind === 'success') { + // Awaited, not fire-and-forget: `Response.close()` returns a promise, so an un-awaited call + // would settle outside this try with nothing to catch its rejection. + await current.value.close(); + } + } catch (closeError) { + return failure( + suppress( + thrownError, + closeError, + 'response close failed while handling a step error', + ), + ); + } + return failure(thrownError); +} diff --git a/packages/core/src/recovery/status-mapping.test.ts b/packages/core/src/recovery/status-mapping.test.ts new file mode 100644 index 0000000..0963257 --- /dev/null +++ b/packages/core/src/recovery/status-mapping.test.ts @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/status-mapping.test.ts +// Exercises: RECOV-15 (only 400..599 map to the matching typed exception; every other status passes +// through unchanged, and §8.2's conformance clause that an error status reaches a recovery hook as +// a Failure), RECOV-16 (the mapping reuses Phase 3b's already-bounded, replayable buffering — this +// file proves the wiring only; the 1 MiB cap and its truncation are 3b's own suite's job, at +// `body/http-status-error.test.ts`), RECOV-7 and RECOV-4 where the step meets the chain +import {describe, expect, test} from 'bun:test'; +import {expectTypeOf} from 'expect-type'; +import {HttpStatusError} from '../body/http-status-error.js'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {failure, success, type Outcome} from './outcome.js'; +import { + ResponseRecoveryChain, + type RecoveryStep, + type ResponseStep, +} from './response-chain.js'; +import {statusMappingStep} from './status-mapping.js'; + +function aResponse( + status: number, + body: ReadableStream | null = null, +): Response { + return Response.newBuilder() + .request(Request.newBuilder().url('https://example.com').build()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(status)) + .body(body) + .build(); +} + +function bodyOf(text: string): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(text)); + controller.close(); + }, + }); +} + +/** Awaits `promise`, returning whatever it rejected with — `undefined` when it resolved. */ +async function rejection(promise: Promise): Promise { + try { + await promise; + return undefined; + } catch (error) { + return error; + } +} + +describe('statusMappingStep (RECOV-15)', () => { + test('returns a 2xx response unchanged', async () => { + const response = aResponse(200); + + const result = await statusMappingStep(response); + + expect(result).toBe(response); + }); + + test('returns a 3xx response unchanged', async () => { + const response = aResponse(304); + + const result = await statusMappingStep(response); + + expect(result).toBe(response); + }); + + test('returns a non-standard 6xx response unchanged — only 400..599 map', async () => { + const response = aResponse(600); + + const result = await statusMappingStep(response); + + expect(result).toBe(response); + }); + + test('throws HttpStatusError naming the status for a 404', async () => { + const error = await rejection(statusMappingStep(aResponse(404))); + + expect(error).toBeInstanceOf(HttpStatusError); + expect((error as HttpStatusError).status).toBe(404); + }); + + test('throws HttpStatusError for a 500', async () => { + const error = await rejection(statusMappingStep(aResponse(500))); + + expect(error).toBeInstanceOf(HttpStatusError); + expect((error as HttpStatusError).status).toBe(500); + }); +}); + +describe('statusMappingStep buffering (RECOV-16)', () => { + test('the error body survives on the thrown exception, replayable after the response is closed', async () => { + const error = await rejection( + statusMappingStep(aResponse(422, bodyOf('{"detail":"nope"}'))), + ); + + expect(error).toBeInstanceOf(HttpStatusError); + expect((error as HttpStatusError).preview()).toBe('{"detail":"nope"}'); + }); +}); + +describe('statusMappingStep inside a recovery chain (RECOV-15, RECOV-7)', () => { + // §8.2's own conformance clause for RECOV-15 is about the outcome the chain produces, not about + // the step in isolation: a 400..599 must reach a recovery hook as a Failure carrying the typed + // exception, exactly the way a transport error does. The step throwing is the mechanism; this is + // the requirement. + test('a 404 surfaces to a recovery hook as a Failure carrying the typed exception', async () => { + const seen: Outcome[] = []; + const recorder: RecoveryStep = outcome => { + seen.push(outcome); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain([statusMappingStep], [recorder]); + + const result = await chain.apply(success(aResponse(404, bodyOf('nope')))); + + expect(result.kind).toBe('failure'); + const error = result.kind === 'failure' ? result.error : undefined; + expect(error).toBeInstanceOf(HttpStatusError); + expect((error as HttpStatusError).status).toBe(404); + expect((error as HttpStatusError).preview()).toBe('nope'); + expect(seen).toEqual([result]); + }); + + test('a 200 passes through the chain as a Success carrying the same response', async () => { + const response = aResponse(200); + const chain = new ResponseRecoveryChain([statusMappingStep], []); + + const result = await chain.apply(success(response)); + + expect(result.kind === 'success' && result.value).toBe(response); + }); + + test('the step never runs on a Failure input — RECOV-4 governs, not the status', async () => { + const seedError = new Error('transport failed'); + const chain = new ResponseRecoveryChain([statusMappingStep], []); + + const result = await chain.apply(failure(seedError)); + + expect(result.kind === 'failure' && result.error).toBe(seedError); + }); +}); + +describe('statusMappingStep conforms to ResponseStep', () => { + // The compile-time proof the discarded `: ResponseStep` annotation used to provide, kept out of + // the module so nothing dead reaches `dist/`. Only fires under `bun run typecheck` — `bun test` + // executes this file but strips its types without checking them (styleguide 11.6). + test('its signature is exactly the ResponseStep signature', () => { + expectTypeOf().toEqualTypeOf(); + }); +}); diff --git a/packages/core/src/recovery/status-mapping.ts b/packages/core/src/recovery/status-mapping.ts new file mode 100644 index 0000000..40678ff --- /dev/null +++ b/packages/core/src/recovery/status-mapping.ts @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/status-mapping.ts +import {toHttpError} from '../body/http-status-error.js'; +import type {Response} from '../http/response.js'; + +/** + * The status → typed-exception mapping response step (RECOV-15, RECOV-16). + * + * Phase 3b's `toHttpError()` already satisfies both requirements in full: it treats only 400..599 + * as errors and hands every other status back unchanged (RECOV-15), and it buffers the error body + * into a bounded, replayable in-memory copy inside the response's own close-guaranteeing scope + * before mapping, sharing the same 1 MiB cap 3b's logging tees use (RECOV-16). `HttpStatusError` — + * flat, carrying `status` and the buffered body — IS the "matching typed exception"; no new + * buffering, no per-status class hierarchy. + * + * The `throw` is deliberate: it lets RECOV-7 in `response-chain.ts` convert an error status into a + * Failure exactly the way any other response-step throw is handled, rather than this step + * special-casing its own error path. + * + * @param response - the response to inspect. + * @returns the response unchanged when its status is not an error status. + * @throws HttpStatusError when the status is in 400..599. + * + * @internal + */ +export async function statusMappingStep(response: Response): Promise { + const httpError = await toHttpError(response); + if (httpError === null) return response; + throw httpError; +} + +// A named declaration, not `const statusMappingStep: ResponseStep = async response => ...`: arrows +// are reserved for inline callbacks (docs/knowledge/function-design.md:18-21), and a named +// declaration survives in stack traces — which a function whose whole job is to throw actually +// depends on. `func-style`'s `allowArrowFunctions: true` would not have flagged the arrow form, so +// this is on the author, not the gate. +// +// The proof that the signature still conforms to `ResponseStep` lives in the test file, as an +// `expectTypeOf` assertion. A module-level `statusMappingStep satisfies ResponseStep;` would do the +// same job, but `satisfies` erases to its operand rather than to nothing, leaving a dead +// `statusMappingStep;` expression statement in the published `dist/`. diff --git a/packages/core/src/suppress.test.ts b/packages/core/src/suppress.test.ts new file mode 100644 index 0000000..e675796 --- /dev/null +++ b/packages/core/src/suppress.test.ts @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/suppress.test.ts +// Exercises: the runtime-guarded stand-in for `SuppressedError` that RECOV-12 (and, later, Phases +// 5a/6a/6b/6c) need to attach a teardown failure to a primary throwable without inverting their +// priority. +// +// Neither branch of the guard is forced here by mutating `globalThis` — a test that deletes a +// global does not survive parallel execution, which docs/knowledge/testing.md:50 requires. The +// branch selection is covered where it is real instead: `suppress()` is asserted on its shape, +// which holds on either runtime, `FallbackSuppressedError` is constructed directly, and the +// `test:node` matrix runs both legs — `lts/*` has the native class, the pinned `20.3.0` floor does +// not. +import {describe, expect, test} from 'bun:test'; +import {expectTypeOf} from 'expect-type'; +import { + FallbackSuppressedError, + suppress, + type SuppressedErrorLike, +} from './suppress.js'; + +describe('suppress', () => { + test('keeps the primary error primary and the secondary suppressed', () => { + const primary = new Error('primary'); + const secondary = new Error('secondary'); + + const result = suppress(primary, secondary, 'teardown failed'); + + expect(result.error).toBe(primary); + expect(result.suppressed).toBe(secondary); + expect(result.message).toBe('teardown failed'); + }); + + test('reports the same identity on either branch of the guard', () => { + const result = suppress( + new Error('primary'), + new Error('secondary'), + 'msg', + ); + + expect(result).toBeInstanceOf(Error); + expect(result.name).toBe('SuppressedError'); + }); + + test('carries non-Error throwables unchanged — a JS throw can raise any value', () => { + const result = suppress('a string throw', undefined, 'teardown failed'); + + expect(result.error).toBe('a string throw'); + expect(result.suppressed).toBeUndefined(); + }); + + test('uses the native SuppressedError when the runtime provides one', () => { + const native = (globalThis as {SuppressedError?: unknown}).SuppressedError; + if (typeof native !== 'function') return; // the floor runtime has no native class to use + + const result = suppress(new Error('a'), new Error('b'), 'msg'); + + expect(result).toBeInstanceOf(native); + }); +}); + +describe('FallbackSuppressedError — the branch the declared floor takes', () => { + test('mirrors the native shape rather than reporting its own class name', () => { + const primary = new Error('primary'); + const secondary = new Error('secondary'); + + const result = new FallbackSuppressedError( + primary, + secondary, + 'teardown failed', + ); + + expect(result).toBeInstanceOf(Error); + expect(result.name).toBe('SuppressedError'); + expect(result.error).toBe(primary); + expect(result.suppressed).toBe(secondary); + expect(result.message).toBe('teardown failed'); + }); + + test('satisfies the SuppressedErrorLike shape suppress() promises', () => { + expectTypeOf().toExtend(); + expectTypeOf< + ReturnType + >().toEqualTypeOf(); + }); +}); diff --git a/packages/core/src/suppress.ts b/packages/core/src/suppress.ts new file mode 100644 index 0000000..99c407f --- /dev/null +++ b/packages/core/src/suppress.ts @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/suppress.ts + +/** + * The shape of an error carrying a suppressed secondary throwable — structurally identical to the + * ECMAScript `SuppressedError` this module produces when the runtime has one. + * + * @internal + */ +export interface SuppressedErrorLike extends Error { + /** The primary throwable — the one the caller actually cares about. */ + readonly error: unknown; + /** The secondary throwable raised while unwinding, riding along rather than masking. */ + readonly suppressed: unknown; +} + +type SuppressedErrorConstructor = new ( + error: unknown, + suppressed: unknown, + message: string, +) => SuppressedErrorLike; + +/** + * Pairs a primary throwable with a secondary one raised while unwinding, keeping the **primary** + * primary (RECOV-12). + * + * `SuppressedError` is a V8 global from the full Explicit Resource Management proposal, absent on + * this package's declared floor (`engines.node >=20.3`, set by `AbortSignal.any()`) and absent from + * the `lib` this package compiles against. Raising the floor to reach it would drop every Node 20 + * and 22 consumer for one error class, so the class is used when the runtime happens to provide it + * and {@link FallbackSuppressedError} is built when it does not — the same guarded shape the + * roadmap already sanctioned for `Symbol.asyncDispose`. The global is read per call, not captured at + * module load, so the choice tracks the runtime rather than the import order. + * + * Both branches return the same observable shape, so no caller branches on which one it got, and no + * caller may test `instanceof SuppressedError` — that would silently assert nothing on the floor. + * CI covers both: the `lts/*` leg of the `test:node` matrix takes the native branch, the pinned + * `20.3.0` leg takes the fallback. + * + * Never built via `using`/`await using`: native disposal constructs + * `new SuppressedError(disposalError, originalError)`, making the *teardown* failure primary + * (`docs/knowledge/resource-management.md:72`) — the inverse of what RECOV-12 requires. + * + * @param error - the primary throwable; stays primary. + * @param suppressed - the secondary throwable raised while unwinding. + * @param message - describes the unwinding that produced `suppressed`. + * @returns an error carrying both, with `error` primary. + * + * @internal + */ +export function suppress( + error: unknown, + suppressed: unknown, + message: string, +): SuppressedErrorLike { + const {SuppressedError: native} = globalThis as typeof globalThis & { + SuppressedError?: SuppressedErrorConstructor; + }; + return typeof native === 'function' + ? new native(error, suppressed, message) + : new FallbackSuppressedError(error, suppressed, message); +} + +/** + * The stand-in {@link suppress} builds on runtimes without the native class. Mirrors its observable + * shape — `name`, `error`, `suppressed` — so a caller never has to branch on which one it received. + * + * `name` is pinned to `'SuppressedError'` rather than following `docs/knowledge/error-handling.md`'s + * `this.name = new.target.name`: the point of this class is to be indistinguishable from the native + * one, and reporting `FallbackSuppressedError` in a stack trace would make the runtime the reader is + * on part of the error's identity. + * + * Exported so its shape is unit-testable directly. The alternative — deleting + * `globalThis.SuppressedError` inside a test to force the fallback branch — would not survive + * parallel execution, which `docs/knowledge/testing.md:50` requires of every test. + * + * @internal + */ +export class FallbackSuppressedError + extends Error + implements SuppressedErrorLike +{ + override readonly name = 'SuppressedError'; + readonly error: unknown; + readonly suppressed: unknown; + + constructor(error: unknown, suppressed: unknown, message: string) { + super(message); + this.error = error; + this.suppressed = suppressed; + } +} diff --git a/test/node-conformance/recovery-chain.test.mjs b/test/node-conformance/recovery-chain.test.mjs new file mode 100644 index 0000000..de0a215 --- /dev/null +++ b/test/node-conformance/recovery-chain.test.mjs @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: MIT +// test/node-conformance/recovery-chain.test.mjs +// +// Phase 4b (`RECOV-12`) is a runtime-divergent surface for one specific reason: the `SuppressedError` +// global. Bun ships it and so does current Node, but it is a V8 global from the full Explicit Resource +// Management proposal and is absent on this package's declared floor (`engines.node ">=20.3"`). A +// `new SuppressedError(...)` written straight into `response-chain.ts` would pass `bun test` and then throw +// `ReferenceError: SuppressedError is not defined` at a consumer's call time — exactly the `NFR-10` trap +// `docs/knowledge/tooling-and-quality-gates.md:60-61` describes. `suppress()` guards on the global; this +// file is what proves the guarded path actually works on the runtime the SDK ships to, at both ends of the +// matrix. +// +// The close-on-throw half also exercises `RECOV-12`'s "released exactly once" over Node's own Web Streams +// implementation, whose `cancel()` and reader-lock timing are independent of Bun's. +import assert from 'node:assert/strict'; +import {describe, it} from 'node:test'; +import {Protocol, Request, Response, Status} from '@dexpace/core'; +import { + FallbackSuppressedError, + suppress, +} from '../../packages/core/dist/suppress.js'; +import {ResponseRecoveryChain} from '../../packages/core/dist/recovery/response-chain.js'; +import {success} from '../../packages/core/dist/recovery/outcome.js'; + +function aResponse(body = null) { + return Response.newBuilder() + .request(Request.newBuilder().url('https://example.com').build()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .body(body) + .build(); +} + +describe('suppress() on the declared Node floor', () => { + it('produces a shape-compatible error whether or not the runtime has SuppressedError', () => { + const primary = new Error('primary'); + const secondary = new Error('secondary'); + + const result = suppress(primary, secondary, 'teardown failed'); + + assert.ok(result instanceof Error, 'suppress() must return an Error'); + assert.equal(result.name, 'SuppressedError'); + assert.equal( + result.error, + primary, + 'the primary throwable must stay primary', + ); + assert.equal(result.suppressed, secondary); + assert.equal(result.message, 'teardown failed'); + }); + + it('takes the branch this runtime actually has, and both legs of the matrix are covered', () => { + // Not forced by deleting the global — that would not survive parallel execution + // (docs/knowledge/testing.md:50). The matrix is the forcing function: the pinned 20.3.0 leg has + // no native class and takes the fallback, `lts/*` has one and takes the native branch. Either + // way the result must be usable without the caller knowing which. + const native = globalThis.SuppressedError; + const result = suppress( + new Error('primary'), + new Error('secondary'), + 'teardown failed', + ); + + if (typeof native === 'function') { + assert.ok( + result instanceof native, + 'a runtime with SuppressedError must produce the native class', + ); + } else { + assert.ok( + result instanceof FallbackSuppressedError, + 'a runtime without SuppressedError must produce the stand-in', + ); + } + assert.equal(result.name, 'SuppressedError'); + assert.equal(result.message, 'teardown failed'); + }); + + it('builds the fallback stand-in with the same observable shape', () => { + const primary = new Error('primary'); + const secondary = new Error('secondary'); + + const result = new FallbackSuppressedError( + primary, + secondary, + 'teardown failed', + ); + + assert.ok(result instanceof Error); + assert.equal(result.name, 'SuppressedError'); + assert.equal(result.error, primary); + assert.equal(result.suppressed, secondary); + }); +}); + +describe('RECOV-12 close-on-throw over Node Web Streams', () => { + it('closes the in-hand response exactly once and keeps the step error primary', async () => { + let cancels = 0; + const body = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + cancels += 1; + }, + }); + const thrownError = new Error('step failed'); + const chain = new ResponseRecoveryChain( + [ + () => { + throw thrownError; + }, + ], + [], + ); + + const result = await chain.apply(success(aResponse(body))); + + assert.equal(cancels, 1, 'the response must be released exactly once'); + assert.equal(result.kind, 'failure'); + assert.equal( + result.error, + thrownError, + 'the step error must survive by identity', + ); + }); + + it('attaches a close failure as suppressed without displacing the step error', async () => { + const closeError = new Error('close failed'); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + throw closeError; + }, + }); + const originalError = new Error('step failed'); + const chain = new ResponseRecoveryChain( + [ + () => { + throw originalError; + }, + ], + [], + ); + + const result = await chain.apply(success(aResponse(body))); + + assert.equal(result.kind, 'failure'); + assert.equal(result.error.name, 'SuppressedError'); + assert.equal( + result.error.error, + originalError, + 'RECOV-12: the original stays primary', + ); + assert.equal(result.error.suppressed, closeError); + }); +}); From c78c327d646600e7c713b99c78df43eebc9508bc Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Wed, 26 Aug 2026 20:26:01 +0300 Subject: [PATCH 3/3] =?UTF-8?q?feat(core):=20stage-based=20pipeline=20?= =?UTF-8?q?=E2=80=94=20product-spec=20=C2=A78.1=20(PIPE-1..PIPE-40)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4c. Ships `packages/core/src/pipeline/` — the fixed-stage step composition runtime, its builder, the per-call cursor/fork mechanism, and the execution-context-store wiring 4a deferred here. Plumbing only: no pillar step bodies, no standard-resilience preset, nothing added to the public barrel. - `stage.ts` — `Stage` as a string-literal union plus `STAGE_ORDER` and `PILLAR_STAGES`. No TS `enum` (`erasableSyntaxOnly`); inserting a stage later is one splice and touches no existing stage identity (PIPE-1..4, PIPE-8). - `step.ts` — `Step`/`StepContext`/`Next`/`StepDescriptor`. A step is a function wrapped in a descriptor carrying a `type` symbol, which is what PIPE-6's reference identity and PIPE-18/19's anchor matching key off. - `cursor.ts` — one recursive dispatcher per call. `ctx.next` and every `ctx.fork()` are one-shot closures over it, pinned to the same target position, sharing a single mutable in-flight request so a substitution sticks for the whole call (PIPE-9..PIPE-17). - `runtime.ts` — `Runtime implements Transport`: empty-pipeline fast path, context install/promote/evict-in-`finally` on both paths, and `exchangeSource` so the exchange context describes the request that was actually sent (PIPE-9, PIPE-10, PIPE-25..27, CTX-17). - `builder.ts` — stage-bucketed surgical edits with fail-fast validation at the mutating call, flattened once at `build()` (PIPE-7, PIPE-18..PIPE-25, PIPE-38). - `errors.ts` — five flat `DexpaceError` leaves, each rendering its identifying symbols into its own message. Tests are colocated and cite their PIPE IDs, including fast-check properties for the builder's ordering laws (PIPE-22, PIPE-38) and the driven probe test for PIPE-1/PIPE-2's stage ordering. Deliberately deferred, each named in the design doc or the roadmap's Deferred Items Log: PIPE-17's "readable by any step" clause and `StepContext.signal` (Phase 5a Task 1), PIPE-24/PIPE-35/PIPE-39 (Phase 5+), PIPE-2's redirect/retry half and PIPE-40's 2-hop clause (Phase 5b/5c). Open finding F9 — the cursor does not observe the caller's `AbortSignal` between steps — stays undecided in the roadmap and must be settled before 5a Task 1. Full CI sequence green locally: typecheck, lint, build, test --coverage (690 tests, pipeline files at 100%), api (report byte-identical), lint:publish, verify:dual-consumption, verify:consumer-types, verify:seam-1, verify:runtime-floor, audit, test:node. --- .changeset/2026-08-26-stage-based-pipeline.md | 5 + ...026-07-25-phase4c-stage-pipeline-design.md | 19 + packages/core/src/pipeline/builder.test.ts | 436 ++++++++++++++++++ packages/core/src/pipeline/builder.ts | 273 +++++++++++ packages/core/src/pipeline/cursor.test.ts | 381 +++++++++++++++ packages/core/src/pipeline/cursor.ts | 117 +++++ packages/core/src/pipeline/errors.test.ts | 72 +++ packages/core/src/pipeline/errors.ts | 112 +++++ packages/core/src/pipeline/runtime.test.ts | 309 +++++++++++++ packages/core/src/pipeline/runtime.ts | 124 +++++ packages/core/src/pipeline/stage.test.ts | 57 +++ packages/core/src/pipeline/stage.ts | 64 +++ packages/core/src/pipeline/step.ts | 62 +++ 13 files changed, 2031 insertions(+) create mode 100644 .changeset/2026-08-26-stage-based-pipeline.md create mode 100644 packages/core/src/pipeline/builder.test.ts create mode 100644 packages/core/src/pipeline/builder.ts create mode 100644 packages/core/src/pipeline/cursor.test.ts create mode 100644 packages/core/src/pipeline/cursor.ts create mode 100644 packages/core/src/pipeline/errors.test.ts create mode 100644 packages/core/src/pipeline/errors.ts create mode 100644 packages/core/src/pipeline/runtime.test.ts create mode 100644 packages/core/src/pipeline/runtime.ts create mode 100644 packages/core/src/pipeline/stage.test.ts create mode 100644 packages/core/src/pipeline/stage.ts create mode 100644 packages/core/src/pipeline/step.ts diff --git a/.changeset/2026-08-26-stage-based-pipeline.md b/.changeset/2026-08-26-stage-based-pipeline.md new file mode 100644 index 0000000..663c3bd --- /dev/null +++ b/.changeset/2026-08-26-stage-based-pipeline.md @@ -0,0 +1,5 @@ +--- +"@dexpace/core": patch +--- + +Internal: stage-based pipeline runtime for product-spec §8.1 (PIPE-1..40). No public API change. diff --git a/docs/superpowers/specs/2026-07-25-phase4c-stage-pipeline-design.md b/docs/superpowers/specs/2026-07-25-phase4c-stage-pipeline-design.md index 774dd37..c2e1325 100644 --- a/docs/superpowers/specs/2026-07-25-phase4c-stage-pipeline-design.md +++ b/docs/superpowers/specs/2026-07-25-phase4c-stage-pipeline-design.md @@ -456,6 +456,7 @@ first ships a pillar step. | `PIPE-26`'s "delegate execute/execute-async to its own send/send-async" satisfied by one `send()` method | `PIPE-26`'s literal two-method framing | Follows directly from the row above: `Transport` has one method, so there is nothing to delegate to beyond it | | Steps are functions wrapped in a `StepDescriptor`, not classes implementing an interface | Reference's class-based step modeling (`PIPE-36`'s subclass-locking framing) | `sdk-design-nodejs/05`'s existing precedent; `StepDescriptor.type` (a `symbol`) carries the identity/anchor-matching role a class hierarchy would otherwise provide | | `Stage` is a string-literal union plus an explicit `STAGE_ORDER` array, not numeric enum values with gaps | `PIPE-3`'s "sparse numeric order keys" (SHOULD, naming a mechanism) | The styleguide bars TS `enum` outright (erasable-syntax rule, binding since Phase 0); a string union + ordered array satisfies the same underlying goal (inserting a stage never touches existing stages' identities) without a numeric type at all | +| Two `eslint-disable` directives ship in this phase | The plan's lint-gate pre-check, which asserted "No `eslint-disable` anywhere in this phase" | Added during implementation (2026-08-26). (1) `PillarCollisionError(stage, existingType, incomingType, options?)` is four parameters and `max-params` counts them — the plan's audit covered the builder's methods and `Cursor`/`Runtime`'s constructors but not the error leaves. `PIPE-5` fixes the first three and `DexpaceError`'s contract fixes the trailing `options?: ErrorOptions`, so the shape is not reducible; `HttpStatusError` (Phase 3) established the same exemption for the same reason. (2) `runtime.test.ts` disables `@typescript-eslint/require-await` on a step that throws before its first `await` — that shape *is* the case under test (`PIPE-29`/`PIPE-30`'s structural claim), and rewriting it as `Promise.reject` would exercise something else. Both carry the `-- reason` that `eslint-comments/require-description` demands (`NFR-7`) | ## Deferred Items @@ -514,6 +515,24 @@ ships (plumbing, no pillar steps — a test that needs redirect or retry *behavi conformance clause describes the handle silently resuming past already-visited steps — the behavior the one-shot guard makes unreachable, so that clause is not transcribable as written. +**Added during the 2026-08-26 implementation review**, all testable with plumbing alone and none of them +covered by the list above as written: + +- `PIPE-12`'s two remaining clauses, each its own case: a step that short-circuits (returns without calling + `next`) never reaches the terminal transport, and a step may substitute the outbound response on the way + back out. +- `PIPE-26`'s "a configured pipeline can stand in wherever a transport is expected... and options survive the + indirection": a `Runtime` nested as another `Runtime`'s transport — step order across both hops, and the + caller's `options`/`signal` reaching the terminal transport by reference. +- `PIPE-14`'s stickiness *across forks* (the design's "visible to every subsequent fork" claim, distinct from + the downstream-within-one-drive case): a substitution made inside one fork is what the next fork dispatches. +- `PIPE-10`/`PIPE-11`'s concurrency claim: two interleaved `send()` calls on one `Runtime` each reach the + transport with their own in-flight request, which is what a shared mutable `#request` would break. +- `PIPE-25`/`PIPE-10`'s immutability, structurally rather than by equality: `Runtime.steps` is frozen, and + mutating the array handed to the constructor afterwards cannot reach the built runtime. +- `PIPE-23`'s all-or-nothing on the `SEND` rejection path, not only on a pillar collision; and `PIPE-20`/ + `PIPE-5`'s interaction — a pillar emptied by `remove` accepts a step of a different type. + `PIPE-40`'s response-release discipline is a contract on wrapping steps, not on `Cursor`/`Runtime` (see "Cursor and fork"), and its conformance clause is a 2-hop redirect — untestable without a redirect step. It moves to the phase that ships one, alongside `PIPE-2`'s second half. diff --git a/packages/core/src/pipeline/builder.test.ts b/packages/core/src/pipeline/builder.test.ts new file mode 100644 index 0000000..bea746c --- /dev/null +++ b/packages/core/src/pipeline/builder.test.ts @@ -0,0 +1,436 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/builder.test.ts +// Exercises: PIPE-4/5/6 (a pillar admits at most one step; a distinct collision throws; the same type is +// idempotent), PIPE-7 (non-pillar stages preserve insertion order through append/prepend), PIPE-8 (SEND +// rejects any insertion), PIPE-18/19 (insertAfter/insertBefore/replace act relative to the first anchor +// instance; cross-stage is rejected), PIPE-20 (remove deletes every instance, no-op when absent), PIPE-21 +// (a missing anchor fails), PIPE-22 (an edit sequence flattens the same as constructing the final set from +// scratch), PIPE-23 (a colliding reload leaves prior content untouched, and a same-type pillar repeat inside +// one batch seats only one step), PIPE-25 (flatten order), PIPE-38 (appendAll preserves batch order; +// prependAll reverses it), PIPE-1/PIPE-2 (a built pipeline, driven: entry in STAGE_ORDER, exit reversed) +import {describe, expect, test} from 'bun:test'; +import fc from 'fast-check'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import type {Transport} from '../seams/transport.js'; +import {PipelineBuilder} from './builder.js'; +import { + AnchorNotFoundError, + CrossStageEditError, + PillarCollisionError, + ReservedStageError, +} from './errors.js'; +import type {Runtime} from './runtime.js'; +import {PILLAR_STAGES, STAGE_ORDER, type Stage} from './stage.js'; +import type {Step, StepDescriptor} from './step.js'; + +function aRequest(url: string): Request { + return Request.newBuilder().url(url).build(); +} + +function aResponse(status: number): Response { + return Response.newBuilder() + .request(aRequest('https://example.com')) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(status)) + .build(); +} + +class StubTransport implements Transport { + #response: Response; + + constructor(response: Response) { + this.#response = response; + } + + send(): Promise { + return Promise.resolve(this.#response); + } + + close(): Promise { + return Promise.resolve(); + } +} + +// A driven pipeline installs a context per call and evicts it again in `Runtime.send()`'s own `finally`, so +// this file leaves the process-wide `contextStore` exactly as it found it -- no `afterEach(clear)`, which +// would wipe entries a sibling test file installed (4a's plan Global Constraints; testing.md:50,52). + +const noopStep: Step = async (_request, ctx) => ctx.next(); + +function descriptor( + label: string, + stage: StepDescriptor['stage'], +): StepDescriptor { + return {type: Symbol(label), stage, fn: noopStep}; +} + +function labelsOf(runtime: Runtime): (string | undefined)[] { + return runtime.steps.map(d => d.type.description); +} + +function aBuilder(): PipelineBuilder { + return new PipelineBuilder(new StubTransport(aResponse(200))); +} + +describe('PipelineBuilder pillar rules (PIPE-4, PIPE-5, PIPE-6)', () => { + test('a pillar stage admits at most one step', () => { + const builder = aBuilder().append(descriptor('a', 'RETRY')); + + expect(builder.build().steps).toHaveLength(1); + }); + + test('installing a distinct second step onto an occupied pillar throws, naming both types', () => { + const builder = aBuilder(); + const a = descriptor('a', 'RETRY'); + const b = descriptor('b', 'RETRY'); + builder.append(a); + + try { + builder.append(b); + throw new Error( + 'unreachable -- append must throw for a distinct pillar collision', + ); + } catch (error) { + expect(error).toBeInstanceOf(PillarCollisionError); + expect((error as PillarCollisionError).existingType).toBe(a.type); + expect((error as PillarCollisionError).incomingType).toBe(b.type); + } + }); + + test('re-installing the identical descriptor type onto its own pillar is an idempotent no-op', () => { + const builder = aBuilder(); + const a = descriptor('a', 'RETRY'); + + builder.append(a).append(a); + + expect(builder.build().steps).toHaveLength(1); + }); +}); + +describe('PipelineBuilder remove then re-install (PIPE-20, PIPE-5)', () => { + test('a pillar emptied by remove accepts a step of a different type', () => { + const first = descriptor('first', 'RETRY'); + const builder = aBuilder().append(first); + + builder.remove(first.type); + builder.append(descriptor('second', 'RETRY')); + + // The emptied bucket must not read as still occupied: PIPE-5's collision is about an occupant, and + // remove left none. + expect(labelsOf(builder.build())).toEqual(['second']); + }); +}); + +describe('PipelineBuilder non-pillar ordering (PIPE-7)', () => { + test('append adds to the tail, prepend adds to the head, within one stage', () => { + const a = descriptor('a', 'PRE_LOGGING'); + const b = descriptor('b', 'PRE_LOGGING'); + const c = descriptor('c', 'PRE_LOGGING'); + + const runtime = aBuilder().append(a).append(c).prepend(b).build(); + + expect(labelsOf(runtime)).toEqual(['b', 'a', 'c']); + }); +}); + +describe('PipelineBuilder batch edits (PIPE-38)', () => { + test('appendAll preserves the batch iteration order', () => { + const steps = ['a', 'b', 'c'].map(label => + descriptor(label, 'PRE_LOGGING'), + ); + + const runtime = aBuilder().appendAll(steps).build(); + + expect(labelsOf(runtime)).toEqual(['a', 'b', 'c']); + }); + + test('prependAll results in the reversed batch order', () => { + const steps = ['a', 'b', 'c'].map(label => + descriptor(label, 'PRE_LOGGING'), + ); + + const runtime = aBuilder().prependAll(steps).build(); + + expect(labelsOf(runtime)).toEqual(['c', 'b', 'a']); + }); +}); + +describe('PipelineBuilder anchor edits (PIPE-18, PIPE-19, PIPE-21)', () => { + test('insertAfter/insertBefore act relative to the FIRST existing instance of the anchor type', () => { + const a = descriptor('a', 'PRE_LOGGING'); + const b = descriptor('b', 'PRE_LOGGING'); + const builder = aBuilder().append(a).append(b); + + builder.insertAfter(a.type, descriptor('c', 'PRE_LOGGING')); + builder.insertBefore(a.type, descriptor('d', 'PRE_LOGGING')); + + expect(labelsOf(builder.build())).toEqual(['d', 'a', 'c', 'b']); + }); + + test('insertAfter/insertBefore/replace reject a cross-stage edit', () => { + const a = descriptor('a', 'PRE_LOGGING'); + const builder = aBuilder().append(a); + const wrongStage = descriptor('x', 'POST_LOGGING'); + + expect(() => builder.insertAfter(a.type, wrongStage)).toThrow( + CrossStageEditError, + ); + expect(() => builder.insertBefore(a.type, wrongStage)).toThrow( + CrossStageEditError, + ); + expect(() => builder.replace(a.type, wrongStage)).toThrow( + CrossStageEditError, + ); + }); + + test('an anchor edit against a missing type throws AnchorNotFoundError', () => { + const builder = aBuilder(); + const missing = Symbol('missing'); + + expect(() => + builder.insertAfter(missing, descriptor('x', 'PRE_LOGGING')), + ).toThrow(AnchorNotFoundError); + expect(() => + builder.replace(missing, descriptor('x', 'PRE_LOGGING')), + ).toThrow(AnchorNotFoundError); + }); + + test('replace swaps the anchor step in place, same stage, same position', () => { + const a = descriptor('a', 'PRE_LOGGING'); + const b = descriptor('b', 'PRE_LOGGING'); + const builder = aBuilder().append(a).append(b); + + builder.replace(a.type, descriptor('a2', 'PRE_LOGGING')); + + expect(labelsOf(builder.build())).toEqual(['a2', 'b']); + }); +}); + +describe('PipelineBuilder remove (PIPE-20)', () => { + test('deletes every instance of a type, preserving relative order of the rest', () => { + const a1 = descriptor('a', 'PRE_LOGGING'); + const b = descriptor('b', 'PRE_LOGGING'); + const a2: StepDescriptor = { + type: a1.type, + stage: 'POST_LOGGING', + fn: noopStep, + }; + const builder = aBuilder().appendAll([a1, b]).append(a2); + + builder.remove(a1.type); + + expect(labelsOf(builder.build())).toEqual(['b']); + }); + + test('is a no-op when the type is absent', () => { + const a = descriptor('a', 'PRE_LOGGING'); + const builder = aBuilder().append(a); + + expect(() => builder.remove(Symbol('absent'))).not.toThrow(); + expect(labelsOf(builder.build())).toEqual(['a']); + }); +}); + +describe('PipelineBuilder reload (PIPE-23)', () => { + test('a colliding batch leaves the existing collection completely unchanged', () => { + const builder = aBuilder().append(descriptor('original', 'PRE_LOGGING')); + + expect(() => + builder.reload([descriptor('x', 'RETRY'), descriptor('y', 'RETRY')]), + ).toThrow(PillarCollisionError); + expect(labelsOf(builder.build())).toEqual(['original']); + }); + + test('a valid batch fully replaces the prior collection', () => { + const builder = aBuilder().append(descriptor('stale', 'PRE_LOGGING')); + + builder.reload([descriptor('fresh', 'POST_LOGGING')]); + + expect(labelsOf(builder.build())).toEqual(['fresh']); + }); + + test('a batch rejected on a later element leaves the existing collection untouched', () => { + const builder = aBuilder().append(descriptor('original', 'PRE_LOGGING')); + + expect(() => + builder.reload([descriptor('ok', 'PRE_AUTH'), descriptor('bad', 'SEND')]), + ).toThrow(ReservedStageError); + + // PIPE-23: validation runs over the whole batch before `#buckets.clear()`, so a rejection that + // surfaces on the second element cannot leave the builder half-rebuilt. + expect(labelsOf(builder.build())).toEqual(['original']); + }); + + test('a batch repeating the SAME pillar type installs it once, not twice (PIPE-4, PIPE-6)', () => { + const retry = descriptor('retry', 'RETRY'); + const builder = aBuilder(); + + builder.reload([retry, retry]); + + // PIPE-4: a pillar admits at most one step. The incremental `append` path already treats a same-type + // re-install as an idempotent no-op (PIPE-6); a bulk reload must not be the back door that seats two. + expect(labelsOf(builder.build())).toEqual(['retry']); + }); +}); + +describe('PipelineBuilder reserved SEND stage (PIPE-8)', () => { + test('rejects any insertion targeting SEND', () => { + const sendShaped = descriptor('x', 'SEND'); + + expect(() => aBuilder().append(sendShaped)).toThrow(ReservedStageError); + expect(() => aBuilder().prepend(sendShaped)).toThrow(ReservedStageError); + expect(() => aBuilder().reload([sendShaped])).toThrow(ReservedStageError); + }); +}); + +describe('PipelineBuilder.build() flatten order (PIPE-1, PIPE-25)', () => { + test('flattens stages in declaration order regardless of append order', () => { + const preRedirect = descriptor('pre-redirect', 'PRE_REDIRECT'); + const postSerde = descriptor('post-serde', 'POST_SERDE'); + + const runtime = aBuilder().append(postSerde).append(preRedirect).build(); + + expect(labelsOf(runtime)).toEqual(['pre-redirect', 'post-serde']); + }); + + // PIPE-1/PIPE-2's conformance clause, in the one place that can express it: a built pipeline actually + // driven. Entry is the stage list top-down, exit is its exact reverse, with insertion order deliberately + // the reverse of declaration order so a flatten that leaked insertion order would fail loudly. + test('one probe step per stage enters in STAGE_ORDER and exits in its exact reverse', async () => { + const stages = STAGE_ORDER.filter(stage => stage !== 'SEND'); + const log: string[] = []; + const builder = aBuilder(); + for (const stage of [...stages].reverse()) { + builder.append({ + type: Symbol(stage), + stage, + fn: async (_request, ctx) => { + log.push(`enter:${stage}`); + const response = await ctx.next(); + log.push(`exit:${stage}`); + return response; + }, + }); + } + + await builder.build().send(aRequest('https://example.com')); + + expect(log).toEqual([ + ...stages.map(stage => `enter:${stage}`), + ...[...stages].reverse().map(stage => `exit:${stage}`), + ]); + }); +}); + +describe('PipelineBuilder edit-order independence (PIPE-22)', () => { + test('an edit sequence flattens the same as constructing the final set from scratch', () => { + const a = descriptor('a', 'PRE_LOGGING'); + const b = descriptor('b', 'PRE_LOGGING'); + const c = descriptor('c', 'POST_LOGGING'); + + const edited = new PipelineBuilder(new StubTransport(aResponse(200))) + .append(a) + .append(c) + .prepend(b) + .build(); + const fromScratch = new PipelineBuilder(new StubTransport(aResponse(200))) + .appendAll([b, a, c]) + .build(); + + expect(labelsOf(edited)).toEqual(labelsOf(fromScratch)); + expect(labelsOf(edited)).toEqual(['b', 'a', 'c']); + }); +}); + +// The two ordering laws the design calls for (PIPE-38's split across an append and a prepend test, one act +// each). `build()` is an invariant-bearing assembler, which +// docs/knowledge/testing.md:29 puts in property-test territory; the examples above pin concrete regressions, +// these prove the law over generated input. Generated over the non-pillar stages only: a generator that also +// emitted pillar stages would spend most of its cases hitting PIPE-5's collision instead of exercising order. +const editableStages = STAGE_ORDER.filter( + stage => stage !== 'SEND' && !PILLAR_STAGES.has(stage), +); + +describe('PipelineBuilder ordering properties (PIPE-22)', () => { + test('any append/prepend sequence flattens the same as building the final set from scratch (PIPE-22)', () => { + fc.assert( + fc.property( + fc.array( + fc.record({ + stage: fc.constantFrom(...editableStages), + where: fc.constantFrom('append' as const, 'prepend' as const), + }), + {maxLength: 24}, + ), + edits => { + const edited = aBuilder(); + const model = new Map(); + for (const [index, edit] of edits.entries()) { + const step = descriptor(`s${String(index)}`, edit.stage); + const bucket = model.get(edit.stage) ?? []; + if (edit.where === 'append') { + bucket.push(step); + edited.append(step); + } else { + bucket.unshift(step); + edited.prepend(step); + } + model.set(edit.stage, bucket); + } + const finalSet = editableStages.flatMap( + stage => model.get(stage) ?? [], + ); + + const fromScratch = aBuilder().appendAll(finalSet).build(); + + expect(labelsOf(edited.build())).toEqual(labelsOf(fromScratch)); + }, + ), + ); + }); +}); + +describe('PipelineBuilder batch-order properties (PIPE-38)', () => { + test('appendAll preserves the batch order within a stage, for a batch of any size (PIPE-38)', () => { + fc.assert( + fc.property( + fc.integer({min: 1, max: 12}), + fc.constantFrom(...editableStages), + (size, stage) => { + const batch = Array.from({length: size}, (_unused, index) => + descriptor(`s${String(index)}`, stage), + ); + + const runtime = aBuilder().appendAll(batch).build(); + + expect(labelsOf(runtime)).toEqual( + batch.map(step => step.type.description), + ); + }, + ), + ); + }); + + test('prependAll reverses the batch order within a stage, for a batch of any size (PIPE-38)', () => { + fc.assert( + fc.property( + fc.integer({min: 1, max: 12}), + fc.constantFrom(...editableStages), + (size, stage) => { + const batch = Array.from({length: size}, (_unused, index) => + descriptor(`s${String(index)}`, stage), + ); + + const runtime = aBuilder().prependAll(batch).build(); + + expect(labelsOf(runtime)).toEqual( + batch.map(step => step.type.description).reverse(), + ); + }, + ), + ); + }); +}); diff --git a/packages/core/src/pipeline/builder.ts b/packages/core/src/pipeline/builder.ts new file mode 100644 index 0000000..537d2ec --- /dev/null +++ b/packages/core/src/pipeline/builder.ts @@ -0,0 +1,273 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/builder.ts +import {invariant} from '../invariant.js'; +import type {Transport} from '../seams/transport.js'; +import { + AnchorNotFoundError, + CrossStageEditError, + PillarCollisionError, + ReservedStageError, +} from './errors.js'; +import {Runtime} from './runtime.js'; +import {PILLAR_STAGES, STAGE_ORDER, type Stage} from './stage.js'; +import type {StepDescriptor} from './step.js'; + +interface AnchorLocation { + readonly stage: Stage; + readonly index: number; +} + +/** + * Assembles a stage-based pipeline via surgical edits (PIPE-7, PIPE-18..PIPE-24), flattening into an + * immutable Runtime at build() time (PIPE-25). Mutable while being built; the produced Runtime is frozen. + * + * @internal + */ +export class PipelineBuilder { + readonly #buckets = new Map(); + readonly #transport: Transport; + + constructor(transport: Transport) { + this.#transport = transport; + } + + /** + * Seats `descriptor` at the tail of its own stage bucket (PIPE-7). + * + * @throws ReservedStageError when the descriptor declares the terminal `SEND` stage (PIPE-8). + * @throws PillarCollisionError when its pillar stage already holds a step of a different type + * (PIPE-5); re-seating the same `type` symbol is an idempotent no-op instead (PIPE-6). + */ + append(descriptor: StepDescriptor): this { + this.#rejectReservedStage(descriptor.stage, 'append'); + if (this.#pillarSlot(descriptor) === 'occupied-same-type') return this; + this.#insertAt(descriptor.stage, descriptor, 'tail'); + return this; + } + + /** + * Seats `descriptor` at the head of its own stage bucket (PIPE-7). + * + * @throws ReservedStageError when the descriptor declares the terminal `SEND` stage (PIPE-8). + * @throws PillarCollisionError when its pillar stage already holds a step of a different type + * (PIPE-5); re-seating the same `type` symbol is an idempotent no-op instead (PIPE-6). + */ + prepend(descriptor: StepDescriptor): this { + this.#rejectReservedStage(descriptor.stage, 'prepend'); + if (this.#pillarSlot(descriptor) === 'occupied-same-type') return this; + this.#insertAt(descriptor.stage, descriptor, 'head'); + return this; + } + + /** + * PIPE-38: batch iteration order preserved within a stage. + * + * @throws ReservedStageError as {@link PipelineBuilder.append}, on the first offending descriptor. + * @throws PillarCollisionError as {@link PipelineBuilder.append}. Not all-or-nothing: descriptors + * before the offending one are already seated — `reload` is the transactional bulk path (PIPE-23). + */ + appendAll(descriptors: readonly StepDescriptor[]): this { + for (const descriptor of descriptors) this.append(descriptor); + return this; + } + + /** + * PIPE-38: each element prepended individually -- the batch order comes out reversed, by + * construction. This asymmetry with {@link PipelineBuilder.appendAll} is the documented one PIPE-38 + * requires a port to state. + * + * @throws ReservedStageError as {@link PipelineBuilder.prepend}, on the first offending descriptor. + * @throws PillarCollisionError as {@link PipelineBuilder.prepend}. Not all-or-nothing, as above. + */ + prependAll(descriptors: readonly StepDescriptor[]): this { + for (const descriptor of descriptors) this.prepend(descriptor); + return this; + } + + /** + * Seats `descriptor` immediately after the first existing instance of `anchorType` (PIPE-18). + * + * @throws ReservedStageError when the descriptor declares the terminal `SEND` stage (PIPE-8). + * @throws AnchorNotFoundError when no step of `anchorType` is present (PIPE-21). + * @throws CrossStageEditError when the descriptor's stage differs from the anchor's (PIPE-18). + * @throws PillarCollisionError when the anchor's pillar stage already holds a different type (PIPE-5). + */ + insertAfter(anchorType: symbol, descriptor: StepDescriptor): this { + this.#rejectReservedStage(descriptor.stage, 'insertAfter'); + const anchor = this.#requireAnchor(anchorType, 'insertAfter'); + this.#requireSameStage(anchor.stage, descriptor.stage); + if (this.#pillarSlot(descriptor) === 'occupied-same-type') return this; + const bucket = this.#buckets.get(anchor.stage); + invariant( + bucket !== undefined, + 'anchor stage bucket must exist -- #requireAnchor just located an entry in it', + ); + bucket.splice(anchor.index + 1, 0, descriptor); + return this; + } + + /** + * Seats `descriptor` immediately before the first existing instance of `anchorType` (PIPE-18). + * + * @throws ReservedStageError when the descriptor declares the terminal `SEND` stage (PIPE-8). + * @throws AnchorNotFoundError when no step of `anchorType` is present (PIPE-21). + * @throws CrossStageEditError when the descriptor's stage differs from the anchor's (PIPE-18). + * @throws PillarCollisionError when the anchor's pillar stage already holds a different type (PIPE-5). + */ + insertBefore(anchorType: symbol, descriptor: StepDescriptor): this { + this.#rejectReservedStage(descriptor.stage, 'insertBefore'); + const anchor = this.#requireAnchor(anchorType, 'insertBefore'); + this.#requireSameStage(anchor.stage, descriptor.stage); + if (this.#pillarSlot(descriptor) === 'occupied-same-type') return this; + const bucket = this.#buckets.get(anchor.stage); + invariant( + bucket !== undefined, + 'anchor stage bucket must exist -- #requireAnchor just located an entry in it', + ); + bucket.splice(anchor.index, 0, descriptor); + return this; + } + + /** + * Swaps the first existing instance of `anchorType` for `descriptor`, in place (PIPE-19). The + * sanctioned way past a pillar collision: PIPE-5 exempts `replace` from the pillar check, since it + * swaps one occupant 1:1 within its own stage and the incoming type is distinct by definition. + * + * @throws ReservedStageError when the descriptor declares the terminal `SEND` stage (PIPE-8). + * @throws AnchorNotFoundError when no step of `anchorType` is present (PIPE-21). + * @throws CrossStageEditError when the descriptor's stage differs from the anchor's (PIPE-19). + */ + replace(anchorType: symbol, descriptor: StepDescriptor): this { + this.#rejectReservedStage(descriptor.stage, 'replace'); + const anchor = this.#requireAnchor(anchorType, 'replace'); + this.#requireSameStage(anchor.stage, descriptor.stage); + const bucket = this.#buckets.get(anchor.stage); + invariant( + bucket !== undefined, + 'anchor stage bucket must exist -- #requireAnchor just located an entry in it', + ); + bucket.splice(anchor.index, 1, descriptor); + return this; + } + + /** + * PIPE-20: deletes every instance of `type`, preserving relative order; a no-op when absent. A stage + * left with no steps keeps an empty bucket, which flattening and the pillar check both read as absent. + */ + remove(type: symbol): this { + for (const [stage, bucket] of this.#buckets) { + const filtered = bucket.filter(entry => entry.type !== type); + if (filtered.length !== bucket.length) this.#buckets.set(stage, filtered); + } + return this; + } + + /** + * PIPE-23: all-or-nothing -- validated fully before any existing content is touched, so a rejected + * batch leaves the builder exactly as it was. + * + * @throws ReservedStageError when any descriptor declares the terminal `SEND` stage (PIPE-8). + * @throws PillarCollisionError when two descriptors of different types claim one pillar stage + * (PIPE-5); a repeat of the same `type` on one pillar is dropped instead, so the bulk path cannot + * seat two steps where `append` would seat one (PIPE-4/PIPE-6). + */ + reload(descriptors: readonly StepDescriptor[]): this { + const admitted: StepDescriptor[] = []; + const pillarTypes = new Map(); + for (const desc of descriptors) { + this.#rejectReservedStage(desc.stage, 'reload'); + if (!PILLAR_STAGES.has(desc.stage)) { + admitted.push(desc); + continue; + } + const seenType = pillarTypes.get(desc.stage); + if (seenType === desc.type) continue; // PIPE-6: a repeat of the SAME type is idempotent, not a second step. + if (seenType !== undefined) { + throw new PillarCollisionError(desc.stage, seenType, desc.type); // PIPE-5 + } + pillarTypes.set(desc.stage, desc.type); + admitted.push(desc); + } + // PIPE-4: `admitted` holds at most one entry per pillar stage by construction -- a same-type repeat was + // skipped above rather than pushed, so a batch cannot install two steps onto one pillar the way the + // incremental `append` path already refuses to. + this.#buckets.clear(); + for (const desc of admitted) { + const bucket = this.#buckets.get(desc.stage); + if (bucket === undefined) this.#buckets.set(desc.stage, [desc]); + else bucket.push(desc); + } + return this; + } + + /** PIPE-25: flattens stage buckets in declaration order, skipping SEND, into an immutable Runtime. */ + build(): Runtime { + const flattened: StepDescriptor[] = []; + for (const stage of STAGE_ORDER) { + if (stage === 'SEND') continue; // PIPE-8: terminal, reserved, flattening skips it. + const bucket = this.#buckets.get(stage); + if (bucket !== undefined) flattened.push(...bucket); + } + return new Runtime(flattened, this.#transport); // Runtime copies and freezes -- PIPE-10/PIPE-25. + } + + #rejectReservedStage(stage: Stage, operation: string): void { + if (stage === 'SEND') throw new ReservedStageError(operation); // PIPE-8 + } + + /** + * PIPE-4/5/6: `'ok'` when `descriptor` may be seated, `'occupied-same-type'` when its pillar already + * holds that exact `type` and the edit is an idempotent no-op. A bucket emptied by `remove` counts as + * unoccupied. + * + * @throws PillarCollisionError when the pillar holds a step of a different type (PIPE-5). + */ + #pillarSlot(descriptor: StepDescriptor): 'ok' | 'occupied-same-type' { + const {stage, type} = descriptor; + if (!PILLAR_STAGES.has(stage)) return 'ok'; + const bucket = this.#buckets.get(stage); + if (bucket === undefined || bucket.length === 0) return 'ok'; + const occupant = bucket[0]; + invariant( + occupant !== undefined, + 'pillar bucket has non-zero length but its first element is undefined', + ); + if (occupant.type === type) return 'occupied-same-type'; // PIPE-6: idempotent re-installation. + throw new PillarCollisionError(stage, occupant.type, type); // PIPE-5 + } + + #insertAt( + stage: Stage, + descriptor: StepDescriptor, + where: 'head' | 'tail', + ): void { + const bucket = this.#buckets.get(stage); + if (bucket === undefined) { + this.#buckets.set(stage, [descriptor]); + return; + } + if (where === 'tail') bucket.push(descriptor); + else bucket.unshift(descriptor); + } + + /** + * PIPE-18's "first existing instance", resolved in flattened order -- `STAGE_ORDER` first, then + * position within the stage bucket. A type installed in more than one stage therefore anchors on its + * earliest-staged instance, and an edit declaring one of the later stages is a cross-stage edit even + * though an instance does sit in that stage. + */ + #requireAnchor(type: symbol, operation: string): AnchorLocation { + for (const stage of STAGE_ORDER) { + const bucket = this.#buckets.get(stage); + if (bucket === undefined) continue; + const index = bucket.findIndex(entry => entry.type === type); + if (index !== -1) return {stage, index}; + } + throw new AnchorNotFoundError(type, operation); // PIPE-21 + } + + #requireSameStage(anchorStage: Stage, incomingStage: Stage): void { + if (anchorStage !== incomingStage) + throw new CrossStageEditError(anchorStage, incomingStage); // PIPE-18/19 + } +} diff --git a/packages/core/src/pipeline/cursor.test.ts b/packages/core/src/pipeline/cursor.test.ts new file mode 100644 index 0000000..27285c4 --- /dev/null +++ b/packages/core/src/pipeline/cursor.test.ts @@ -0,0 +1,381 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/cursor.test.ts +// Exercises: PIPE-9 (Cursor-level: an exhausted position dispatches to the terminal transport), PIPE-11/15 +// (a reused next()/fork() continuation throws CursorAlreadyAdvancedError), PIPE-12 (ctx.context, ctx.fork +// gated by pillar stage, short-circuiting without invoking the chain, substituting the outbound response), +// PIPE-13 (terminal dispatch threads request/options/signal), PIPE-14 (a substituted request sticks +// downstream, across every later fork, and into the terminal dispatch), PIPE-15/16 (fork() returns +// independent, position-pinned one-shot continuations; a step that forks twice re-visits every downstream +// step both times), PIPE-17 (the caller's options are carried unchanged across every fork and into each +// dispatch) +import {describe, expect, test} from 'bun:test'; +import { + createRequestContext, + type ExecutionContext, +} from '../context/context.js'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {RequestOptions} from '../http/request-options.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {invariant} from '../invariant.js'; +import type {Transport} from '../seams/transport.js'; +import {Cursor} from './cursor.js'; +import {CursorAlreadyAdvancedError} from './errors.js'; +import type {Next, Step, StepDescriptor} from './step.js'; + +function aRequest(url: string): Request { + return Request.newBuilder().url(url).build(); +} + +function aResponse(status: number): Response { + return Response.newBuilder() + .request(aRequest('https://example.com')) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(status)) + .build(); +} + +class RecordingTransport implements Transport { + readonly calls: { + request: Request; + options: RequestOptions | undefined; + signal: AbortSignal | undefined; + }[] = []; + #response: Response; + + constructor(response: Response) { + this.#response = response; + } + + send( + request: Request, + options?: RequestOptions, + signal?: AbortSignal, + ): Promise { + this.calls.push({request, options, signal}); + return Promise.resolve(this.#response); + } + + close(): Promise { + return Promise.resolve(); + } +} + +function passthroughStep(log: string[], label: string): Step { + return async (_request, ctx) => { + log.push(label); + return ctx.next(); + }; +} + +describe('Cursor terminal dispatch (PIPE-9, PIPE-13)', () => { + test('an exhausted cursor dispatches to the terminal transport, threading options and signal', async () => { + const canned = aResponse(200); + const transport = new RecordingTransport(canned); + const request = aRequest('https://example.com/a'); + const signal = new AbortController().signal; + const context = createRequestContext(request); + + const cursor = new Cursor({steps: [], transport, request, context, signal}); + const response = await cursor.advance(); + + expect(response).toBe(canned); + expect(transport.calls).toHaveLength(1); + expect(transport.calls[0]?.request).toBe(request); + expect(transport.calls[0]?.signal).toBe(signal); + }); +}); + +describe('Cursor step invocation (PIPE-12)', () => { + test('ctx.context is the exact reference passed to the constructor, visible to every step', async () => { + const request = aRequest('https://example.com'); + const context = createRequestContext(request); + const seen: ExecutionContext[] = []; + const step: Step = async (_request, ctx) => { + seen.push(ctx.context); + return ctx.next(); + }; + const descriptor: StepDescriptor = { + type: Symbol('probe'), + stage: 'PRE_LOGGING', + fn: step, + }; + + await new Cursor({ + steps: [descriptor], + transport: new RecordingTransport(aResponse(200)), + request, + context, + }).advance(); + + expect(seen[0]).toBe(context); + }); +}); + +describe('Cursor fork availability (PIPE-12, PIPE-15)', () => { + test('ctx.fork is undefined for a non-pillar-stage step', async () => { + const request = aRequest('https://example.com'); + const context = createRequestContext(request); + const seenFork: ((() => Next) | undefined)[] = []; + const step: Step = async (_request, ctx) => { + seenFork.push(ctx.fork); + return ctx.next(); + }; + const descriptor: StepDescriptor = { + type: Symbol('probe'), + stage: 'PRE_LOGGING', + fn: step, + }; + + await new Cursor({ + steps: [descriptor], + transport: new RecordingTransport(aResponse(200)), + request, + context, + }).advance(); + + expect(seenFork[0]).toBeUndefined(); + }); + + test('ctx.fork is present for a pillar-stage step', async () => { + const request = aRequest('https://example.com'); + const context = createRequestContext(request); + let sawFork: (() => Next) | undefined; + const step: Step = async (_request, ctx) => { + sawFork = ctx.fork; + invariant(sawFork !== undefined, 'pillar step must receive a fork'); + return sawFork()(); + }; + const descriptor: StepDescriptor = { + type: Symbol('retry'), + stage: 'RETRY', + fn: step, + }; + + await new Cursor({ + steps: [descriptor], + transport: new RecordingTransport(aResponse(200)), + request, + context, + }).advance(); + + expect(sawFork).toBeDefined(); + }); +}); + +describe('Cursor bidirectionality (PIPE-12)', () => { + test('a step that short-circuits never reaches the terminal transport', async () => { + const request = aRequest('https://example.com'); + const context = createRequestContext(request); + const synthetic = aResponse(204); + const transport = new RecordingTransport(aResponse(200)); + const shortCircuit: Step = () => Promise.resolve(synthetic); + const descriptor: StepDescriptor = { + type: Symbol('short-circuit'), + stage: 'PRE_LOGGING', + fn: shortCircuit, + }; + + const response = await new Cursor({ + steps: [descriptor], + transport, + request, + context, + }).advance(); + + expect(response).toBe(synthetic); + expect(transport.calls).toHaveLength(0); + }); + + test('a step may substitute the outbound response on the way back out', async () => { + const request = aRequest('https://example.com'); + const context = createRequestContext(request); + const fromTransport = aResponse(200); + const substituted = aResponse(203); + const transport = new RecordingTransport(fromTransport); + let sawFromTransport: Response | undefined; + const substituteResponse: Step = async (_request, ctx) => { + sawFromTransport = await ctx.next(); + return substituted; + }; + const descriptor: StepDescriptor = { + type: Symbol('substitute-response'), + stage: 'PRE_LOGGING', + fn: substituteResponse, + }; + + const response = await new Cursor({ + steps: [descriptor], + transport, + request, + context, + }).advance(); + + expect(sawFromTransport).toBe(fromTransport); + expect(response).toBe(substituted); + }); +}); + +describe('Cursor continuation reuse (PIPE-11, PIPE-15)', () => { + test('a second call to the same next() throws CursorAlreadyAdvancedError', async () => { + const request = aRequest('https://example.com'); + const context = createRequestContext(request); + let capturedNext: Next | undefined; + const step: Step = async (_request, ctx) => { + capturedNext = ctx.next; + return ctx.next(); + }; + const descriptor: StepDescriptor = { + type: Symbol('probe'), + stage: 'PRE_LOGGING', + fn: step, + }; + + await new Cursor({ + steps: [descriptor], + transport: new RecordingTransport(aResponse(200)), + request, + context, + }).advance(); + + invariant( + capturedNext !== undefined, + 'the step must have run and captured its next()', + ); + const rejection: unknown = await capturedNext().catch( + (error: unknown) => error, + ); + expect(rejection).toBeInstanceOf(CursorAlreadyAdvancedError); + }); + + test('a second call to the same fork()-returned continuation throws CursorAlreadyAdvancedError', async () => { + const request = aRequest('https://example.com'); + const context = createRequestContext(request); + let capturedContinuation: Next | undefined; + const step: Step = async (_request, ctx) => { + invariant(ctx.fork !== undefined, 'pillar step must receive a fork'); + capturedContinuation = ctx.fork(); + return capturedContinuation(); + }; + const descriptor: StepDescriptor = { + type: Symbol('retry'), + stage: 'RETRY', + fn: step, + }; + + await new Cursor({ + steps: [descriptor], + transport: new RecordingTransport(aResponse(200)), + request, + context, + }).advance(); + + invariant( + capturedContinuation !== undefined, + 'the step must have run and captured its fork() continuation', + ); + const rejection: unknown = await capturedContinuation().catch( + (error: unknown) => error, + ); + expect(rejection).toBeInstanceOf(CursorAlreadyAdvancedError); + }); +}); + +describe('Cursor request substitution (PIPE-14)', () => { + test('a substituted request propagates downstream and to the terminal dispatch', async () => { + const original = aRequest('https://example.com/a'); + const substituted = aRequest('https://example.com/b'); + const context = createRequestContext(original); + const seenByDownstream: Request[] = []; + const substituteStep: Step = async (_request, ctx) => ctx.next(substituted); + const downstreamStep: Step = async (request, ctx) => { + seenByDownstream.push(request); + return ctx.next(); + }; + const transport = new RecordingTransport(aResponse(200)); + const steps: StepDescriptor[] = [ + {type: Symbol('substitute'), stage: 'PRE_LOGGING', fn: substituteStep}, + {type: Symbol('downstream'), stage: 'POST_LOGGING', fn: downstreamStep}, + ]; + + await new Cursor({steps, transport, request: original, context}).advance(); + + expect(seenByDownstream[0]).toBe(substituted); + expect(transport.calls[0]?.request).toBe(substituted); + }); +}); + +describe('Cursor request substitution across forks (PIPE-14, PIPE-16)', () => { + test('a substitution made inside one fork is what the next fork dispatches', async () => { + const original = aRequest('https://example.com/original'); + const substituted = aRequest('https://example.com/substituted'); + const context = createRequestContext(original); + const transport = new RecordingTransport(aResponse(200)); + const reDriving: Step = async (_request, ctx) => { + invariant(ctx.fork !== undefined, 'a pillar step must receive a fork'); + await ctx.fork()(substituted); + return ctx.fork()(); + }; + const descriptor: StepDescriptor = { + type: Symbol('retry'), + stage: 'RETRY', + fn: reDriving, + }; + + await new Cursor({ + steps: [descriptor], + transport, + request: original, + context, + }).advance(); + + // PIPE-14's stickiness is global to the call, not scoped to the fork that substituted: PIPE-16's + // "forks advance independently" is about cursor position, not about request isolation. + expect(transport.calls.map(call => call.request)).toEqual([ + substituted, + substituted, + ]); + }); +}); + +describe('Cursor fork (PIPE-15, PIPE-16, PIPE-17)', () => { + test('a step forking twice re-visits every downstream step on both attempts', async () => { + const request = aRequest('https://example.com'); + const context = createRequestContext(request); + const options = RequestOptions.EMPTY; + const log: string[] = []; + const retryStep: Step = async (_request, ctx) => { + invariant(ctx.fork !== undefined, 'retryStep must occupy a pillar stage'); + log.push('retry:attempt-1'); + await ctx.fork()(); + log.push('retry:attempt-2'); + return ctx.fork()(); + }; + const steps: StepDescriptor[] = [ + {type: Symbol('retry'), stage: 'RETRY', fn: retryStep}, + { + type: Symbol('downstream'), + stage: 'POST_RETRY', + fn: passthroughStep(log, 'downstream'), + }, + ]; + const transport = new RecordingTransport(aResponse(200)); + + await new Cursor({steps, transport, request, context, options}).advance(); + + expect(log).toEqual([ + 'retry:attempt-1', + 'downstream', + 'retry:attempt-2', + 'downstream', + ]); + expect(transport.calls).toHaveLength(2); + // PIPE-17: the caller's per-call options are carried unchanged across every re-drive fork and threaded + // into each terminal dispatch -- shared by reference, never copied-and-diverged per fork. + expect(transport.calls.map(call => call.options)).toEqual([ + options, + options, + ]); + }); +}); diff --git a/packages/core/src/pipeline/cursor.ts b/packages/core/src/pipeline/cursor.ts new file mode 100644 index 0000000..87a5570 --- /dev/null +++ b/packages/core/src/pipeline/cursor.ts @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/cursor.ts +import type {ExecutionContext} from '../context/context.js'; +import type {Request} from '../http/request.js'; +import type {RequestOptions} from '../http/request-options.js'; +import type {Response} from '../http/response.js'; +import {invariant} from '../invariant.js'; +import type {Transport} from '../seams/transport.js'; +import {CursorAlreadyAdvancedError} from './errors.js'; +import {PILLAR_STAGES, type Stage} from './stage.js'; +import type {Next, StepContext, StepDescriptor} from './step.js'; + +/** + * Everything a `Cursor` needs, bundled into one object. Six positional parameters would fail ESLint's + * `max-params: 3`, and Phase 1 reserves the `eslint-disable` escape hatch for private builder-internal + * constructors only -- the same trap 4a's `ContextInit` and 4b's `DispatchConfig` were built to dodge. + * + * @internal + */ +export interface CursorInit { + readonly steps: readonly StepDescriptor[]; + readonly transport: Transport; + readonly request: Request; + readonly context: ExecutionContext; + readonly options?: RequestOptions | undefined; + readonly signal?: AbortSignal | undefined; +} + +/** + * Drives one call through the flattened step array (PIPE-9..PIPE-17). One instance per `Runtime.send()` + * call (PIPE-10); `advance()` is its single public entry point. Internally a private recursive dispatcher + * indexed by array position -- `next` and every `fork()` call are one-shot closures built over the same + * dispatcher (PIPE-15/16), sharing a single mutable in-flight request so a substitution sticks globally for + * the rest of the call (PIPE-14). + * + * There is deliberately no settable start position: a fork produces a fresh one-shot closure over the + * existing dispatcher, never a second `Cursor`, so every instance starts at position 0. + * + * @internal + */ +export class Cursor { + readonly #steps: readonly StepDescriptor[]; + readonly #transport: Transport; + #request: Request; + readonly #options: RequestOptions | undefined; + readonly #signal: AbortSignal | undefined; + readonly #context: ExecutionContext; + + constructor(init: CursorInit) { + this.#steps = init.steps; + this.#transport = init.transport; + this.#request = init.request; + this.#options = init.options; + this.#signal = init.signal; + this.#context = init.context; + } + + /** + * The in-flight request as of now: the one passed in, or whatever a step last substituted (PIPE-14). + * `Runtime` reads this after the drive so the exchange context describes the request actually sent. + */ + get request(): Request { + return this.#request; + } + + /** + * Drives the call from position 0 through every step and on to the terminal transport dispatch. + * Called exactly once per cursor -- `Runtime.send()` allocates a fresh cursor per call (PIPE-10). + * + * @returns the response the outermost step returned, which may be a synthetic one it short-circuited + * with, a substituted one, or the terminal transport's own (PIPE-12). + * @throws CursorAlreadyAdvancedError when a step reuses an already-invoked continuation (PIPE-15). + */ + async advance(): Promise { + return this.#dispatch(0); + } + + async #dispatch(position: number): Promise { + if (position >= this.#steps.length) { + // PIPE-13: exhausted -- dispatch the current in-flight request to the terminal transport. + return this.#transport.send(this.#request, this.#options, this.#signal); + } + const descriptor = this.#steps[position]; + invariant( + descriptor !== undefined, + `pipeline cursor position ${String(position)} is within bounds but undefined`, + ); + const next = this.#continuationAt(position + 1, descriptor.stage); + const ctx: StepContext = PILLAR_STAGES.has(descriptor.stage) + ? { + next, + context: this.#context, + fork: (): Next => + this.#continuationAt(position + 1, descriptor.stage), + } + : {next, context: this.#context}; + return descriptor.fn(this.#request, ctx); + } + + /** + * Builds a ONE-SHOT continuation targeting `targetPosition` (PIPE-11/15: a second call throws + * CursorAlreadyAdvancedError). `ctx.next` and every `ctx.fork()` call share this helper -- both always + * target `position + 1` of the requesting step; `fork()` may simply be called again to obtain a fresh + * one-shot continuation bound to that same target (PIPE-16). + */ + #continuationAt(targetPosition: number, ownerStage: Stage): Next { + let used = false; + return async (replacementRequest?: Request): Promise => { + if (used) throw new CursorAlreadyAdvancedError(ownerStage); + used = true; + if (replacementRequest !== undefined) { + this.#request = replacementRequest; // PIPE-14: sticks for every later step and the terminal dispatch. + } + return this.#dispatch(targetPosition); + }; + } +} diff --git a/packages/core/src/pipeline/errors.test.ts b/packages/core/src/pipeline/errors.test.ts new file mode 100644 index 0000000..2e5c52c --- /dev/null +++ b/packages/core/src/pipeline/errors.test.ts @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/errors.test.ts +// Exercises: PIPE-5 (PillarCollisionError), PIPE-21 (AnchorNotFoundError), PIPE-18/19 (CrossStageEditError), +// PIPE-11/15 (CursorAlreadyAdvancedError), PIPE-8 (ReservedStageError) +import {describe, expect, test} from 'bun:test'; +import {DexpaceError} from '../http/errors.js'; +import { + AnchorNotFoundError, + CrossStageEditError, + CursorAlreadyAdvancedError, + PillarCollisionError, + ReservedStageError, +} from './errors.js'; + +describe('PillarCollisionError (PIPE-5)', () => { + test('carries the stage and both colliding type symbols, extends DexpaceError', () => { + const existing = Symbol('existing'); + const incoming = Symbol('incoming'); + + const error = new PillarCollisionError('RETRY', existing, incoming); + + expect(error).toBeInstanceOf(DexpaceError); + expect(error.name).toBe('PillarCollisionError'); + expect(error.stage).toBe('RETRY'); + expect(error.existingType).toBe(existing); + expect(error.incomingType).toBe(incoming); + // PIPE-5: the message itself names both types, not just the instance fields. + expect(error.message).toContain('Symbol(existing)'); + expect(error.message).toContain('Symbol(incoming)'); + }); +}); + +describe('AnchorNotFoundError (PIPE-21)', () => { + test('carries the missing anchor type and the attempted operation', () => { + const anchorType = Symbol('missing'); + + const error = new AnchorNotFoundError(anchorType, 'insertAfter'); + + expect(error).toBeInstanceOf(DexpaceError); + expect(error.anchorType).toBe(anchorType); + expect(error.operation).toBe('insertAfter'); + expect(error.message).toContain('Symbol(missing)'); // PIPE-21: the message identifies the type + }); +}); + +describe('CrossStageEditError (PIPE-18, PIPE-19)', () => { + test('carries the anchor stage and the incoming stage', () => { + const error = new CrossStageEditError('RETRY', 'AUTH'); + + expect(error).toBeInstanceOf(DexpaceError); + expect(error.anchorStage).toBe('RETRY'); + expect(error.incomingStage).toBe('AUTH'); + }); +}); + +describe('CursorAlreadyAdvancedError (PIPE-11, PIPE-15)', () => { + test('carries the stage of the step that reused its continuation', () => { + const error = new CursorAlreadyAdvancedError('RETRY'); + + expect(error).toBeInstanceOf(DexpaceError); + expect(error.stage).toBe('RETRY'); + }); +}); + +describe('ReservedStageError (PIPE-8)', () => { + test('carries the attempted operation', () => { + const error = new ReservedStageError('append'); + + expect(error).toBeInstanceOf(DexpaceError); + expect(error.operation).toBe('append'); + }); +}); diff --git a/packages/core/src/pipeline/errors.ts b/packages/core/src/pipeline/errors.ts new file mode 100644 index 0000000..d49770c --- /dev/null +++ b/packages/core/src/pipeline/errors.ts @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/errors.ts +import {DexpaceError} from '../http/errors.js'; +import type {Stage} from './stage.js'; + +/** + * PIPE-5: installing a distinct second step onto an occupied pillar; names both types and the stage. + * + * @internal + */ +export class PillarCollisionError extends DexpaceError { + readonly stage: Stage; + readonly existingType: symbol; + readonly incomingType: symbol; + + // eslint-disable-next-line max-params -- constructor parameters fixed by error model: PIPE-5 requires the stage and BOTH colliding types, plus the taxonomy's trailing `options?: ErrorOptions`; same exemption as HttpStatusError. Revisit only if the error model drops a field. + constructor( + stage: Stage, + existingType: symbol, + incomingType: symbol, + options?: ErrorOptions, + ) { + // PIPE-5: the error names BOTH step types and points at the replace path. Symbols are rendered with + // String() (`Symbol(retry)`) -- a bare symbol field is invisible in a stack trace or log line + // (docs/knowledge/error-handling.md:40), the same reason 4a's DuplicateContextKeyError renders its key. + super( + `pillar stage '${stage}' already holds ${String(existingType)}; cannot install ${String(incomingType)} (use replace() to swap it)`, + options, + ); + this.stage = stage; + this.existingType = existingType; + this.incomingType = incomingType; + } +} + +/** + * PIPE-21: an insertAfter/insertBefore/replace whose anchor type matches nothing in the pipeline. + * + * @internal + */ +export class AnchorNotFoundError extends DexpaceError { + readonly anchorType: symbol; + readonly operation: string; + + constructor(anchorType: symbol, operation: string, options?: ErrorOptions) { + // PIPE-21: "fail with an error identifying the missing type" -- in the message, not only as a field. + super( + `${operation}: no step of type ${String(anchorType)} is present in the pipeline`, + options, + ); + this.anchorType = anchorType; + this.operation = operation; + } +} + +/** + * PIPE-18/PIPE-19: a cross-stage insert/replace -- the incoming descriptor's stage differs from the + * anchor's. + * + * @internal + */ +export class CrossStageEditError extends DexpaceError { + readonly anchorStage: Stage; + readonly incomingStage: Stage; + + constructor( + anchorStage: Stage, + incomingStage: Stage, + options?: ErrorOptions, + ) { + super( + `cannot insert/replace across stages: anchor is in '${anchorStage}', incoming step declares '${incomingStage}'`, + options, + ); + this.anchorStage = anchorStage; + this.incomingStage = incomingStage; + } +} + +/** + * PIPE-11/PIPE-15: a step reused an already-invoked next()/fork() continuation instead of forking again. + * + * @internal + */ +export class CursorAlreadyAdvancedError extends DexpaceError { + readonly stage: Stage; + + constructor(stage: Stage, options?: ErrorOptions) { + super( + `step at stage '${stage}' reused an already-invoked continuation; a re-driving step must call fork() again`, + options, + ); + this.stage = stage; + } +} + +/** + * PIPE-8: an attempt to install a user step onto the reserved, terminal SEND stage. + * + * @internal + */ +export class ReservedStageError extends DexpaceError { + readonly operation: string; + + constructor(operation: string, options?: ErrorOptions) { + super( + `${operation}: the SEND stage is reserved for the terminal transport hop and cannot hold a user step`, + options, + ); + this.operation = operation; + } +} diff --git a/packages/core/src/pipeline/runtime.test.ts b/packages/core/src/pipeline/runtime.test.ts new file mode 100644 index 0000000..1b9a346 --- /dev/null +++ b/packages/core/src/pipeline/runtime.test.ts @@ -0,0 +1,309 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/runtime.test.ts +// Exercises: PIPE-9 (an empty pipeline dispatches directly, no cursor/context allocated), PIPE-10 (each +// send() allocates its own per-call state, interleaved calls share none of it, and the built step view is +// frozen and copied), PIPE-11 (per-call mutable state lives on the cursor, never on the runtime), PIPE-14 +// (a substituted request reaches the wire, and is what the exchange context is built from), PIPE-25 +// (get steps() exposes the flattened, immutable array), PIPE-26 (Runtime itself satisfies the Transport SPI +// with one send() method, and nests inside another pipeline with the caller's options intact), PIPE-27 +// (close() never touches the wrapped transport), CTX-17's positive half (the first store entry is installed +// by the first promotion), CTX-1/2/3/6 (exchangeSource pins the call key and instrumentation when it +// rebuilds) +import {describe, expect, test} from 'bun:test'; +import { + createRequestContext, + type ExecutionContext, +} from '../context/context.js'; +import {contextStore} from '../context/store.js'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {RequestOptions} from '../http/request-options.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {invariant} from '../invariant.js'; +import type {Transport} from '../seams/transport.js'; +import {exchangeSource, Runtime} from './runtime.js'; +import type {Step, StepDescriptor} from './step.js'; + +function aRequest(url: string): Request { + return Request.newBuilder().url(url).build(); +} + +function aResponse(status: number): Response { + return Response.newBuilder() + .request(aRequest('https://example.com')) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(status)) + .build(); +} + +class RecordingTransport implements Transport { + readonly calls: { + request: Request; + options: RequestOptions | undefined; + signal: AbortSignal | undefined; + }[] = []; + closeCalls = 0; + #response: Response; + + constructor(response: Response) { + this.#response = response; + } + + send( + request: Request, + options?: RequestOptions, + signal?: AbortSignal, + ): Promise { + this.calls.push({request, options, signal}); + return Promise.resolve(this.#response); + } + + close(): Promise { + this.closeCalls += 1; + return Promise.resolve(); + } +} + +// No `afterEach(() => contextStore.clear())`: the singleton is shared by every test file in the run, so a +// blanket clear wipes entries a sibling installed (4a's plan forbids it by name; testing.md:50,52). Nothing +// here needs one -- `Runtime.send()` evicts its own entry in a `finally`, on the success and the throw path. + +describe('Runtime.send empty pipeline (PIPE-9)', () => { + test('dispatches directly to the terminal transport, threading options and signal, no context installed', async () => { + const canned = aResponse(200); + const transport = new RecordingTransport(canned); + const runtime = new Runtime([], transport); + const request = aRequest('https://example.com/a'); + const signal = new AbortController().signal; + const sizeBefore = contextStore.size; + + const response = await runtime.send(request, undefined, signal); + + expect(response).toBe(canned); + expect(transport.calls).toEqual([{request, options: undefined, signal}]); + // A delta, not an absolute size: `contextStore` is process-wide, so a sibling test file sharing the + // process must not be able to turn this assertion red (styleguide 11.7 -- tests survive any order). + expect(contextStore.size).toBe(sizeBefore); + }); +}); + +describe('Runtime.send context-store wiring (CTX-17, CTX-8)', () => { + test('installs a RequestContext before dispatch, then evicts it after the call resolves', async () => { + let observed: ExecutionContext | undefined; + const step: Step = async (_request, ctx) => { + observed = contextStore.get(ctx.context.key); + return ctx.next(); + }; + const descriptor: StepDescriptor = { + type: Symbol('probe'), + stage: 'PRE_LOGGING', + fn: step, + }; + const runtime = new Runtime( + [descriptor], + new RecordingTransport(aResponse(200)), + ); + + const response = await runtime.send(aRequest('https://example.com')); + + invariant( + observed !== undefined, + 'the step must have observed an installed context', + ); + expect(observed.kind).toBe('request'); + expect(contextStore.get(observed.key)).toBeUndefined(); // evicted in send()'s finally + expect(response.status.code).toBe(200); + }); + + test('evicts the installed context even when a step throws', async () => { + let observedKey: symbol | undefined; + + // eslint-disable-next-line @typescript-eslint/require-await -- throwing before any await IS the case under test: PIPE-29/30 hold structurally because an `async` step body that throws synchronously still surfaces as a rejected promise. `Promise.reject` would exercise something else. Revisit if a step ever throws through a real await. + const step: Step = async (_request, ctx) => { + observedKey = ctx.context.key; + throw new Error('boom'); + }; + const descriptor: StepDescriptor = { + type: Symbol('throws'), + stage: 'PRE_LOGGING', + fn: step, + }; + const runtime = new Runtime( + [descriptor], + new RecordingTransport(aResponse(200)), + ); + + const rejection: unknown = await runtime + .send(aRequest('https://example.com')) + .catch((error: unknown) => error); + expect(rejection).toBeInstanceOf(Error); + expect((rejection as Error).message).toBe('boom'); + invariant( + observedKey !== undefined, + 'the step must have run and captured its call key', + ); + expect(contextStore.get(observedKey)).toBeUndefined(); + }); +}); + +describe('exchangeSource (PIPE-14, CTX-1, CTX-2, CTX-3, CTX-6)', () => { + // Tested directly rather than by spying on `contextStore.install`: the exchange context is evicted in + // `send()`'s own `finally`, so observing it end-to-end would mean patching a method on the process-wide + // singleton -- a mock of an owned interface (styleguide 11.3) that also leaks across test files sharing + // the process if a run is ever parallelised. `exchangeSource` is a pure function; the end-to-end half that + // remains observable (the substituted request is what actually reached the wire) is asserted below. + test('returns the SAME context object when no step substituted the request', () => { + const request = aRequest('https://example.com'); + const context = createRequestContext(request, {operationName: 'GetWidget'}); + + expect(exchangeSource(context, request)).toBe(context); + }); + + test('rebuilds around the substituted request, pinning the same key and instrumentation', () => { + const original = aRequest('https://example.com/original'); + const substituted = aRequest('https://example.com/substituted'); + const context = createRequestContext(original, { + operationName: 'GetWidget', + }); + + const rebuilt = exchangeSource(context, substituted); + + expect(rebuilt.request).toBe(substituted); + expect(rebuilt.key).toBe(context.key); // CTX-3: one call key for the whole chain + expect(rebuilt.instrumentation).toBe(context.instrumentation); // CTX-2: carried forward by reference + expect(rebuilt.operationName).toBe('GetWidget'); + }); +}); + +describe('Runtime.send request substitution reaches the wire (PIPE-14)', () => { + test('the transport receives the substituted request, not the original', async () => { + const original = aRequest('https://example.com/original'); + const substituted = aRequest('https://example.com/substituted'); + const substituteStep: Step = async (_request, ctx) => ctx.next(substituted); + const descriptor: StepDescriptor = { + type: Symbol('substitute'), + stage: 'PRE_LOGGING', + fn: substituteStep, + }; + const transport = new RecordingTransport(aResponse(200)); + + await new Runtime([descriptor], transport).send(original); + + expect(transport.calls[0]?.request).toBe(substituted); + }); +}); + +describe('Runtime concurrency (PIPE-10, PIPE-11)', () => { + test("two interleaved sends never observe each other's in-flight request", async () => { + const transport = new RecordingTransport(aResponse(200)); + const rewrite: Step = async (request, ctx) => { + await Promise.resolve(); // hand the event loop over, so both drives are mid-flight at once + return ctx.next(aRequest(`${request.url.href}rewritten`)); + }; + const descriptor: StepDescriptor = { + type: Symbol('rewrite'), + stage: 'PRE_LOGGING', + fn: rewrite, + }; + const runtime = new Runtime([descriptor], transport); + + await Promise.all([ + runtime.send(aRequest('https://example.com/a/')), + runtime.send(aRequest('https://example.com/b/')), + ]); + + // PIPE-11: per-call mutable state lives on the per-call cursor, so one call's substituted request + // (PIPE-14 makes it stick for the rest of *that* call) cannot leak into the other's dispatch. + expect(transport.calls.map(call => call.request.url.href).sort()).toEqual([ + 'https://example.com/a/rewritten', + 'https://example.com/b/rewritten', + ]); + }); +}); + +describe('Runtime.steps (PIPE-25)', () => { + test('exposes the exact flattened array it was constructed with', () => { + const descriptor: StepDescriptor = { + type: Symbol('probe'), + stage: 'PRE_LOGGING', + fn: async (_r, ctx) => ctx.next(), + }; + const runtime = new Runtime( + [descriptor], + new RecordingTransport(aResponse(200)), + ); + + expect(runtime.steps).toEqual([descriptor]); + }); + + test('the exposed view is frozen', () => { + const runtime = new Runtime([], new RecordingTransport(aResponse(200))); + + expect(Object.isFrozen(runtime.steps)).toBe(true); + }); + + test("copies the caller's array, so a later mutation of it cannot reach the built runtime", () => { + const descriptor: StepDescriptor = { + type: Symbol('probe'), + stage: 'PRE_LOGGING', + fn: async (_r, ctx) => ctx.next(), + }; + const source: StepDescriptor[] = [descriptor]; + const runtime = new Runtime(source, new RecordingTransport(aResponse(200))); + + source.push({...descriptor, type: Symbol('smuggled')}); + + // PIPE-10: immutable after construction, and not by caller discipline. + expect(runtime.steps).toEqual([descriptor]); + }); +}); + +describe('Runtime as a nested transport (PIPE-26)', () => { + test("a built pipeline stands in as another pipeline's transport, options and signal surviving both hops", async () => { + const transport = new RecordingTransport(aResponse(200)); + const log: string[] = []; + const probe = + (label: string): Step => + async (_request, ctx) => { + log.push(`enter:${label}`); + const response = await ctx.next(); + log.push(`exit:${label}`); + return response; + }; + const inner = new Runtime( + [{type: Symbol('inner'), stage: 'PRE_SERDE', fn: probe('inner')}], + transport, + ); + const outer = new Runtime( + [{type: Symbol('outer'), stage: 'PRE_REDIRECT', fn: probe('outer')}], + inner, + ); + const options = RequestOptions.EMPTY; + const signal = new AbortController().signal; + + await outer.send(aRequest('https://example.com'), options, signal); + + expect(log).toEqual([ + 'enter:outer', + 'enter:inner', + 'exit:inner', + 'exit:outer', + ]); + // PIPE-26: "options survive the indirection" -- through the outer cursor, the nested runtime's own + // send(), and its cursor, reaching the terminal transport as the same references the caller passed. + expect(transport.calls[0]?.options).toBe(options); + expect(transport.calls[0]?.signal).toBe(signal); + }); +}); + +describe('Runtime.close (PIPE-27)', () => { + test('never calls the underlying transport close', async () => { + const transport = new RecordingTransport(aResponse(200)); + const runtime = new Runtime([], transport); + + await runtime.close(); + + expect(transport.closeCalls).toBe(0); + }); +}); diff --git a/packages/core/src/pipeline/runtime.ts b/packages/core/src/pipeline/runtime.ts new file mode 100644 index 0000000..06a4e91 --- /dev/null +++ b/packages/core/src/pipeline/runtime.ts @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/runtime.ts +import { + createDispatchContext, + createRequestContext, + promoteToExchange, + promoteToRequest, + type ExecutionContext, + type RequestContext, +} from '../context/context.js'; +import {contextStore} from '../context/store.js'; +import type {Request} from '../http/request.js'; +import type {RequestOptions} from '../http/request-options.js'; +import type {Response} from '../http/response.js'; +import type {Transport} from '../seams/transport.js'; +import {Cursor} from './cursor.js'; +import type {StepDescriptor} from './step.js'; + +/** + * The request context to promote from once the drive finishes: the original, unless a step substituted the + * outbound request (PIPE-14), in which case an off-chain rebuild around the request that was actually sent, + * pinned to the SAME call key (CTX-6's explicit-key path) and carrying the same instrumentation bundle by + * reference (CTX-2/CTX-3). Promoting straight off the original would pair the response with a request that + * never left the process, against CTX-1's "the exchange stage exposes the request and the response". Doing it + * here rather than widening `promoteToExchange` with a request-override keeps promotion strictly additive. + * + * Exported (still `@internal`, still absent from the package barrel) so its two branches can be asserted as + * the pure function they are. The alternative -- observing the exchange context end-to-end -- would require + * patching `install` on the process-wide `contextStore` singleton, since `send()` evicts the entry in its own + * `finally`. + * + * @internal + */ +export function exchangeSource( + context: RequestContext, + finalRequest: Request, +): RequestContext { + if (finalRequest === context.request) return context; + return createRequestContext(finalRequest, { + key: context.key, + instrumentation: context.instrumentation, + operationName: context.operationName, + }); +} + +/** + * The built, immutable pipeline (PIPE-10, PIPE-25). Implements `Transport` itself (PIPE-26) -- Phase 2's + * `Transport` SPI has one method (`send`), so there is no second `sendAsync` entry point to delegate through. + * `close()` deliberately never touches the wrapped transport (PIPE-27): the pipeline never owns it. + * + * @internal + */ +export class Runtime implements Transport { + readonly #steps: readonly StepDescriptor[]; + readonly #transport: Transport; + + constructor(steps: readonly StepDescriptor[], transport: Transport) { + // PIPE-10/PIPE-25: the built runtime is immutable, and `get steps()` hands out a read-only view. Copying + // and freezing here rather than trusting the caller makes both structural -- `PipelineBuilder` is not the + // only construction site (tests build one directly, and Phase 5+ may too), so an unfrozen array passed in + // would leave the "immutable after construction" guarantee resting on caller discipline. + this.#steps = Object.freeze([...steps]); + this.#transport = transport; + } + + /** + * Drives `request` through the flattened step array and on to the wrapped transport, installing this + * call's `ExecutionContext` in the store for the duration of the drive and evicting it again on both + * the resolve and the throw path (CTX-17, CTX-9). + * + * A pipeline with no steps skips both the cursor and the context entirely and dispatches straight to + * the wrapped transport (PIPE-9). + * + * @param request - the request to send. + * @param options - per-call operational overrides, carried unchanged across every re-drive fork and + * threaded into each terminal dispatch (PIPE-17). + * @param signal - the caller's abort signal, threaded to the terminal dispatch. Not observed between + * steps in this phase -- see the roadmap's Phase 4c open finding F9. + * @returns whatever the outermost step returned (PIPE-12). + */ + async send( + request: Request, + options?: RequestOptions, + signal?: AbortSignal, + ): Promise { + if (this.#steps.length === 0) { + // PIPE-9: an empty pipeline dispatches directly to the terminal transport, no cursor allocated. + return this.#transport.send(request, options, signal); + } + const dispatchContext = createDispatchContext(); + const requestContext = promoteToRequest(dispatchContext, request); + contextStore.install(requestContext); // CTX-17's positive half: the first store entry, at the first promotion. + let currentContext: ExecutionContext = requestContext; // tracks the latest install for the finally below. + try { + const cursor = new Cursor({ + steps: this.#steps, + transport: this.#transport, + request, + context: requestContext, + options, + signal, + }); + const response = await cursor.advance(); + // PIPE-14: a step may have substituted the outbound request -- promote from whatever was actually sent. + const exchangeContext = promoteToExchange( + exchangeSource(requestContext, cursor.request), + response, + ); + contextStore.install(exchangeContext); // install-or-replace under the same key (CTX-8). + currentContext = exchangeContext; + return response; + } finally { + contextStore.close(currentContext); // always the most recently installed context for this call. + } + } + + async close(): Promise { + // PIPE-27: the pipeline never owns its transport and MUST NOT close it. + } + + get steps(): readonly StepDescriptor[] { + return this.#steps; // PIPE-25: "exposes a read-only, ordered view of its steps." + } +} diff --git a/packages/core/src/pipeline/stage.test.ts b/packages/core/src/pipeline/stage.test.ts new file mode 100644 index 0000000..7a329b8 --- /dev/null +++ b/packages/core/src/pipeline/stage.test.ts @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/stage.test.ts +// Exercises: PIPE-2 (the mandatory chain, outermost pre-redirect slot through terminal SEND), PIPE-3 +// (pre/post extension slots around every pillar), PIPE-4 (exactly the 5 configurable pillars), PIPE-8 (SEND +// is the final, terminal stage) +import {describe, expect, test} from 'bun:test'; +import {PILLAR_STAGES, STAGE_ORDER} from './stage.js'; + +describe('STAGE_ORDER (PIPE-2, PIPE-3)', () => { + test('lists every stage exactly once, in declaration order', () => { + expect(STAGE_ORDER).toEqual([ + 'PRE_REDIRECT', + 'REDIRECT', + 'POST_REDIRECT', + 'PRE_RETRY', + 'RETRY', + 'POST_RETRY', + 'PRE_AUTH', + 'AUTH', + 'POST_AUTH', + 'PRE_LOGGING', + 'LOGGING', + 'POST_LOGGING', + 'PRE_SERDE', + 'SERDE', + 'POST_SERDE', + 'SEND', + ]); + expect(new Set(STAGE_ORDER).size).toBe(STAGE_ORDER.length); + }); + + test('PRE_REDIRECT is the outermost slot (PIPE-2)', () => { + expect(STAGE_ORDER.at(0)).toBe('PRE_REDIRECT'); + }); + + test('SEND is the terminal, final stage (PIPE-8)', () => { + expect(STAGE_ORDER.at(-1)).toBe('SEND'); + }); +}); + +describe('PILLAR_STAGES (PIPE-4)', () => { + test('is exactly REDIRECT, RETRY, AUTH, LOGGING, SERDE', () => { + expect([...PILLAR_STAGES].sort()).toEqual([ + 'AUTH', + 'LOGGING', + 'REDIRECT', + 'RETRY', + 'SERDE', + ]); + }); + + test('does not include SEND or any extension slot', () => { + expect(PILLAR_STAGES.has('SEND')).toBe(false); + expect(PILLAR_STAGES.has('PRE_REDIRECT')).toBe(false); + expect(PILLAR_STAGES.has('POST_LOGGING')).toBe(false); + }); +}); diff --git a/packages/core/src/pipeline/stage.ts b/packages/core/src/pipeline/stage.ts new file mode 100644 index 0000000..28f3272 --- /dev/null +++ b/packages/core/src/pipeline/stage.ts @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/stage.ts + +/** + * Fixed, totally-ordered pipeline stages (PIPE-1, PIPE-2). A string-literal union, not a TS `enum` -- + * `erasableSyntaxOnly` bars enums, and `Stage` has no behavior beyond ordering, which `STAGE_ORDER` alone + * provides. `PRE_REDIRECT` is the outermost slot PIPE-2 mandates; `POST_REDIRECT`..`POST_SERDE` are PIPE-3's + * SHOULD extension slots around every pillar. `SEND` is terminal and reserved -- PIPE-8, flattening skips it + * and `PipelineBuilder` rejects any attempt to install a step there. + * + * @internal + */ +export type Stage = + | 'PRE_REDIRECT' + | 'REDIRECT' + | 'POST_REDIRECT' + | 'PRE_RETRY' + | 'RETRY' + | 'POST_RETRY' + | 'PRE_AUTH' + | 'AUTH' + | 'POST_AUTH' + | 'PRE_LOGGING' + | 'LOGGING' + | 'POST_LOGGING' + | 'PRE_SERDE' + | 'SERDE' + | 'POST_SERDE' + | 'SEND'; + +/** + * Declaration order (PIPE-1, PIPE-25): `PipelineBuilder.build()` flattens by walking this array. Inserting a + * further stage later is one splice here -- no existing `Stage` value needs to change, so there is no + * numeric-gap "renumbering" concern to design around. + * + * @internal + */ +export const STAGE_ORDER: readonly Stage[] = [ + 'PRE_REDIRECT', + 'REDIRECT', + 'POST_REDIRECT', + 'PRE_RETRY', + 'RETRY', + 'POST_RETRY', + 'PRE_AUTH', + 'AUTH', + 'POST_AUTH', + 'PRE_LOGGING', + 'LOGGING', + 'POST_LOGGING', + 'PRE_SERDE', + 'SERDE', + 'POST_SERDE', + 'SEND', +]; + +/** A pillar stage admits at most one step (PIPE-4). @internal */ +export const PILLAR_STAGES: ReadonlySet = new Set([ + 'REDIRECT', + 'RETRY', + 'AUTH', + 'LOGGING', + 'SERDE', +]); diff --git a/packages/core/src/pipeline/step.ts b/packages/core/src/pipeline/step.ts new file mode 100644 index 0000000..a309f0a --- /dev/null +++ b/packages/core/src/pipeline/step.ts @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/pipeline/step.ts +import type {ExecutionContext} from '../context/context.js'; +import type {Request} from '../http/request.js'; +import type {Response} from '../http/response.js'; +import type {Stage} from './stage.js'; + +/** + * Advances the pipeline once, optionally substituting a replacement request first (PIPE-14). `Request` + * values are immutable, so "substitute" means constructing a new one and passing it downstream -- the + * substitution sticks for every remaining step and the terminal dispatch for the rest of the current call. + * Calling with no argument carries the current request through unchanged. + * + * One-shot: each handle advances the chain exactly once (PIPE-15). A step that needs to re-drive the + * chain calls `ctx.fork()` again for a fresh handle rather than reusing this one. + * + * @throws CursorAlreadyAdvancedError -- as a rejected promise -- when an already-invoked handle is + * invoked a second time (PIPE-11/PIPE-15). + * + * @internal + */ +export type Next = (request?: Request) => Promise; + +/** + * What a step receives on each invocation (PIPE-12). `fork` is present only when the invoking step occupies + * a pillar stage (PIPE-15/16); an ordinary step's `ctx.fork` is `undefined`. + * + * The call's per-call `options` and `AbortSignal` are deliberately absent here: `Cursor` carries both and + * threads them into the terminal dispatch, but PIPE-17's "readable by any step" clause has no reader until + * Phase 5a's retry engine, which adds both fields as one additive amendment (5a Task 1). + * + * @internal + */ +export interface StepContext { + readonly next: Next; + readonly fork?: (() => Next) | undefined; + readonly context: ExecutionContext; +} + +/** + * A pipeline step (PIPE-12): receives the inbound request, MAY invoke the rest of the chain via `ctx.next` + * (or `ctx.fork` to re-drive more than once), and MAY inspect or substitute the outbound response -- + * including short-circuiting by never calling `next` at all. + * + * A step that forks more than once owns closing whatever response its own prior fork produced before + * invoking `fork()` again (PIPE-40) -- that responsibility sits on the wrapping step, not on `Cursor`. + * + * @internal + */ +export type Step = (request: Request, ctx: StepContext) => Promise; + +/** + * A registered step: its function plus the identity (`type`) PIPE-6's reference-identity pillar check and + * PIPE-18/19's anchor-type matching both key off, and the `stage` it occupies. + * + * @internal + */ +export interface StepDescriptor { + readonly type: symbol; + readonly stage: Stage; + readonly fn: Step; +}