From 679e3b367d4f135c6b0a73460ffdb54b843c2fa2 Mon Sep 17 00:00:00 2001 From: dtzp555-max Date: Mon, 25 May 2026 21:03:17 +1000 Subject: [PATCH] =?UTF-8?q?docs:=20D59=20=E2=80=94=20streaming=20SF=20docs?= =?UTF-8?q?=20polish=20+=20v1.x=20roadmap=20#1=20+=20#6=20status=20updates?= =?UTF-8?q?=20(#38)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third of three D-days for v1.x roadmap #1 (streaming-path singleflight + TOCTOU close). D57 (PR #36) shipped the cache layer; D58 (PR #37) shipped the server wiring + integration. D59 polishes docs and closes the roadmap entry. ## README.md § Known limitations Inverted the streaming-path-singleflight-not-implemented bullet to a ✅ shipped marker. New text documents: - D4 singleflight now wired end-to-end on streaming path via cacheStore.getOrComputeStreaming(...). - Two concurrent identical streaming requests share one CLI spawn via tee fan-out. - Late joiners receive accumulated replay + the live tail. - Per-client backpressure (PER_CLIENT_QUEUE_CAP=1MB) protects against slow consumers. - Full-disconnect aborts source CLI via AbortController propagation. - New X-OLP-Streaming-Inflight: source | attached header annotates role. - New cache_status: 'streaming_attached' audit value tracks singleflight wins. - Authority: ADR 0005 Amendment 8, v1.x roadmap #1. ## docs/v1x-roadmap.md #1 entry rewritten to closed-state with: - Three D-day breakdown (D57 cache layer + D58 server wiring + D59 docs). - Final test count delta (603 → 623). - Deferred sub-items NOT blocking #1 closure (solo wire-value, streaming_inflight_join from cache layer, isFirst unused API). #6 entry updated to note that the implementation chose NOT to bundle streaming SPAWN_FAILED salvage with #1 (D57 tee writes cache only on clean source completion; SPAWN_FAILED mid-stream rejects + does not persist). #6 now needs its own ADR amendment when triggered. Reading-order paragraph updated to reflect that #1, #2, #4, #7 are closed and only #3, #5, #6 remain — all trigger-gated. ## Issue #16 Closed via PR squash-merge of D57 (#36) + D58 (#37). D59 docs reflect the closure. ## Scope Pure docs. No code change, no test change. 623/623 still pass. ## Authority - ADR 0005 Amendment 8 (the spec D57+D58 implemented). - docs/v1x-roadmap.md (rewritten for #1 closure + #6 unbundling). - GitHub issue #16 (closed at merge of D57+D58). Co-authored-by: dtzp555 Co-authored-by: Claude Opus 4.7 --- README.md | 2 +- docs/v1x-roadmap.md | 29 ++++++++++++++++------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 9c574e9..5d3d6e4 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ Phase 1 closed at v0.1.1 (multi-provider proxy core + pre-Phase-2 cleanup). Phas Behaviors that work correctly at personal/family scale but have ratified follow-ups for a v1.x sprint. Single landing page: [`docs/v1x-roadmap.md`](./docs/v1x-roadmap.md). -- **Streaming-path singleflight not implemented.** The cache layer's D4 singleflight (one spawn per identical concurrent request) is fully wired on the buffered path but NOT on the streaming path. N concurrent identical streaming requests at v0.1 will each spawn their own CLI process. Design ratified in [ADR 0005 Amendment 8](./docs/adr/0005-cache-cross-provider.md); implementation tracked via [issue #16](https://github.com/dtzp555-max/olp/issues/16) and [v1.x roadmap #1](./docs/v1x-roadmap.md). At family scale this is observably fine — every caller still receives the correct response; the cost is N CLI processes instead of one. +- **Streaming-path singleflight ✅ shipped (D57 + D58, 2026-05-25).** `cacheStore.getOrComputeStreaming(...)` mirrors the buffered-path `getOrCompute` and resolves the TOCTOU window between peek and spawn ([issue #16](https://github.com/dtzp555-max/olp/issues/16)). Two concurrent identical streaming requests share one CLI spawn via tee fan-out; late joiners receive accumulated replay + the live tail; per-client backpressure (`PER_CLIENT_QUEUE_CAP=1MB`) protects against slow consumers; full-disconnect aborts the source CLI via AbortController. New `X-OLP-Streaming-Inflight: source | attached` header annotates the role. New `cache_status: 'streaming_attached'` audit value tracks the singleflight win. Authority: [ADR 0005 Amendment 8](./docs/adr/0005-cache-cross-provider.md), v1.x roadmap #1. - **Soft triggers configured but inert.** `routing.soft_triggers` in `~/.olp/config.json` is honored by the engine's evaluation logic but `quotaStatus()` polling is not wired (ADR 0004 Amendment 2). A startup warning fires if the field is non-empty so the inert state is visible. - **Multi-key auth + owner gating + keygen CLI shipped at v0.2.0 (D44 + D45 + D46 + D47).** `lib/keys.mjs` (core), `lib/audit.mjs` (audit), owner-vs-guest `/health` payload trimming + `X-OLP-Fallback-Detail` policy gating, `bin/olp-keys.mjs` (keygen CLI). All 11 ADR 0007 § 10 acceptance criteria covered. v0.2.0 maintainer-merged 2026-05-25. diff --git a/docs/v1x-roadmap.md b/docs/v1x-roadmap.md index 07cf6e7..fe8b000 100644 --- a/docs/v1x-roadmap.md +++ b/docs/v1x-roadmap.md @@ -8,21 +8,23 @@ 3. **Where** does the work live in the tree today (file + anchor). 4. **When** does it need to land (trigger: load profile, security event, governance amendment). -**Reading order for a v1.x sprint kickoff.** Items #1–#3 are the most architecturally consequential and should be designed in dependency order: #2 (multi-key auth) blocks header gating in #1 and observability ownership in #4. #1 (streaming SF) blocks #5 (soft trigger reactivation) only if soft triggers are wired on streaming requests. +**Reading order for a v1.x sprint kickoff.** As of 2026-05-25, #1 (streaming SF, D57+D58) and #2 (multi-key auth, Phase 2) are CLOSED, and #4 and #7 closed in D56. Remaining v1.x scope: #3 (soft trigger reactivation), #5 (provider cacheKeyFields mask), #6 (streaming SPAWN_FAILED salvage — unbundled from #1 at #1 close). All three remaining items have explicit "trigger to start" gates that have not fired. --- -## #1 — Streaming-path singleflight + TOCTOU close +## #1 — Streaming-path singleflight + TOCTOU close — ✅ **SHIPPED (D57 + D58, 2026-05-25)** -- **What.** `cacheStore.getOrComputeStreaming(keyId, cacheKey, sourceFactory)` API replacing the current `peek + spawn` pattern in `server.mjs`. Per-(keyId, cacheKey) inflight Map with tee fan-out, bounded per-client backpressure queues, late-joiner replay buffer, AbortController propagation on all-disconnect. -- **Why deferred.** Personal/family-scale single-tenant load — N concurrent identical streaming requests is an edge case that has not been reported. Each concurrent caller receives the correct response; the waste is N CLI processes instead of one. -- **Design ADR (ratified).** [`docs/adr/0005-cache-cross-provider.md` Amendment 8](./adr/0005-cache-cross-provider.md) — full design including the inflight Map shape, tee policy, late-joiner replay, backpressure cap, D38 semaphore coordination, abort policy, cache TTL race handling, observability event set, and X-OLP-Streaming-Inflight header. Implementation acceptance criteria are in Amendment 8 §13. -- **Tracking issue.** GitHub issue [#16](https://github.com/dtzp555-max/olp/issues/16) — STAYS OPEN as v1.x tracker. Sibling: the closed-but-not-implemented Amendment 6 deferral (D34 F1). -- **Code anchors today.** - - `server.mjs` lines ~782 (`preCheckHit = await cacheStore.peek(...)`) and ~811–817 (streaming branch entry) — these are the lines the new API replaces. - - `lib/cache/store.mjs` `getOrCompute` — sibling API; the new one mirrors its shape on the streaming path. -- **Trigger to start.** Any of: (a) report of N>1 concurrent identical streaming requests in the wild, (b) v1.x sprint planning kickoff with the maintainer explicitly opening this scope, (c) downstream feature requiring tee-streaming primitive (e.g., browser-side observer attaching to an existing stream). -- **Estimated effort.** Design ADR ratified (Amendment 8) = 30 min done. Implementation = 200–400 lines + 15-20 tests + fresh-context reviewer pass. ~3-4 hours of subagent runtime with full Iron Rule 10 discipline. +- **Status.** Closed. Trigger (b) fired 2026-05-25 — maintainer "go" after v0.3.1. Shipped across three D-days: + - **D57** (PR #36) — cache layer: `cacheStore.getOrComputeStreaming(keyId, cacheKey, sourceFactory, opts) → { stream, isFirst, role }` with `_streamingInflight` Map, tee fan-out, late-joiner replay buffer, per-client backpressure (`PER_CLIENT_QUEUE_CAP=1MB`), replay cap (`ACCUMULATED_REPLAY_CAP=10MB`), AbortController propagation, synchronous Map check+insert (closes TOCTOU). Suite 27 = 12 unit tests. + - **D58** (PR #37) — server.mjs wiring: streaming branch swap; `tryAcquireSpawn`/`releaseSpawn` moved inside `sourceFactory` closure (D38 §7 coordination); `CONCURRENCY_LIMIT` fallthrough preserved; `X-OLP-Streaming-Inflight: source | attached` header; `cache_status: 'streaming_attached'` audit value + audit-query gauge reconciliation; `res.on('close') → stream.return()` for client disconnect; D16 truncated-not-cached invariant preserved via `cacheStore.delete` on stop-less exhaustion. Suite 28 = 8 HTTP integration tests. + - **D59** (this commit) — docs polish: README known-limitations entry inverted; this roadmap entry closed; issue #16 closed. +- **Design authority.** [`docs/adr/0005-cache-cross-provider.md` Amendment 8](./adr/0005-cache-cross-provider.md) — implemented per spec §§1–14 across D57+D58. +- **Tracking issue.** GitHub issue [#16](https://github.com/dtzp555-max/olp/issues/16) — CLOSED at D59 with refs to D57+D58 PRs. +- **Final test count delta.** 603 (v0.3.1) → 623 (v0.3.2/v0.4.0). +20 tests across the SF arc. +- **Deferred sub-items** (left here as future-work pointers, NOT blocking #1 closure): + - `X-OLP-Streaming-Inflight: solo` value not emitted on the wire (Amendment 8 §11). It's observable only post-stream via the `streaming_inflight_source_done` log event's `attached_count: 0`. Future ADR amendment may expose via HTTP trailer. + - `streaming_inflight_join` log event from `_attachClient` cache-layer path (carries no provider/model context). D58 emits the event from the server-layer wrapper instead; cache-layer emission would need a provider/model plumb (TODO marker at `lib/cache/store.mjs:~620`). + - `isFirst` field returned by `getOrComputeStreaming` is currently unused by server.mjs (`role` supersedes). Could be removed in a future cache-layer API cleanup. ## #2 — Multi-key auth (`lib/keys.mjs`) — **PHASE 2 ACTIVE (no longer deferred)** @@ -81,9 +83,10 @@ - **What.** Currently the streaming branch does NOT participate in D16 salvage (the salvage-on-SPAWN_FAILED + chunks pattern that the buffered path uses). Streaming SPAWN_FAILED mid-stream → the truncation marker (D35 #10) fires, but no salvage logic captures partial chunks for downstream cache reuse. - **Why deferred.** Less impactful than #1 — at most one client benefits per spawn event, and the buffered path already provides salvage for the bulk of requests. Streaming is the minority path. -- **Design ADR.** Not yet ratified. Coordinated with #1 because the tee architecture changes the salvage semantics (multiple clients may want different finish_reason interpretations on source-mid-stream-failure). +- **Status update post-#1 close (2026-05-25).** #1 was originally bundled with #6 in the design ADR (Amendment 8). The tee architecture as implemented does NOT carry salvage semantics — D57's tee writes `accumulatedChunks` to cache only on normal source completion (stop chunk seen); on SPAWN_FAILED mid-stream the cache layer rejects all clients with the error and does NOT persist partial chunks. D58 preserves D16's truncated-not-cached invariant via server-layer `cacheStore.delete` on stop-less exhaustion. #6 therefore remains independently deferrable. +- **Design ADR.** Not yet ratified. The unbundling from #1 means #6 now needs its own ADR amendment when triggered. - **Tracking.** Not a GitHub issue. Tracked here. -- **Trigger to start.** Bundled with #1 implementation work (the inflight tee architecture changes the salvage semantics, so designing them together is cheaper than serializing). +- **Trigger to start.** First report of streaming-path SPAWN_FAILED mid-stream where partial-chunk salvage would have helped a downstream caller. Practically unlikely at family scale. ## #7 — AUTH_MISSING tuple path test coverage (D40 follow-up)