diff --git a/CHANGELOG.md b/CHANGELOG.md index ac4fd92..aeb8655 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,15 @@ All notable changes to OLP land here. Per `CLAUDE.md` release_kit overlay, this - **Authority:** ADR 0004 Amendment 6 (this commit); ADR 0004 § Decision § Chain advancement step 4; ADR 0004 Amendment 2 (soft triggers deferred — precondition); ADR 0004 Amendment 5 (per-hop attribution channel via `X-OLP-Fallback-Detail`); ALIGNMENT.md Rule 2 (No Invention rationale); GitHub issue #8 — closed by this commit. - **Test count:** 468 → 468 (no test change). +### D42 — Streaming singleflight design ADR + v1.x roadmap (issue #16) + +- **Design-only ratification of the v1.x streaming singleflight implementation.** ADR 0005 Amendment 6 (D34) had deferred this work with a "design alone warrants a dedicated ADR" note. D42 fulfils the note as ADR 0005 Amendment 8, ratifying the `cacheStore.getOrComputeStreaming(...)` API shape, per-(keyId, cacheKey) inflight Map, tee fan-out with bounded per-client backpressure queues, late-joiner replay buffer, AbortController propagation on all-disconnect, D38 `tryAcquireSpawn` coordination (only the first caller's spawn counts against the semaphore), cache TTL race handling, the new `STREAM_BACKPRESSURE` error code (NOT a hard trigger), and the new `X-OLP-Streaming-Inflight: source | attached | solo` header. Implementation acceptance criteria are enumerated in Amendment 8 §13. +- **Multi-layer safeguards to ensure the v1.x work is not forgotten.** New file `docs/v1x-roadmap.md` is a single living landing page for every Phase-1 deferral (streaming SF, multi-key auth, soft-trigger reactivation, `/health` activeSpawns, provider-level `cacheKeyFields`, streaming-path SPAWN_FAILED salvage, D40 AUTH_MISSING tuple test). Each entry names the ratifying ADR, the load-bearing code anchor, and a concrete trigger to start. Cross-references added at: `lib/cache/store.mjs#getOrCompute` JSDoc (sibling API TODO), `server.mjs` streaming-branch entry (~line 810, the peek+spawn pattern Amendment 8 replaces), `README.md § Known limitations` (user-facing surface), and `docs/adr/0005-cache-cross-provider.md` Amendment 8 § "Cross-references and safeguards". +- **Issue #16 status.** STAYS OPEN as the v1.x implementation tracker. The body of the issue is updated post-D42 to reference Amendment 8 and clarify scope ("design ratified; implementation pending"). DO NOT close the issue until Amendment 8 §13's test surface is green against an actual implementation. +- **No code-behavior change. No new tests.** Amendment 8 is design-only. The implementation will go through full Iron Rule 10 (fresh-context opus reviewer + acceptance-criteria-gated test pass) when the v1.x sprint kicks off. +- **Authority:** ADR 0005 Amendment 8 (this commit); ADR 0005 Amendment 6 (D34 — original deferral note); GitHub issue #16 (round-6 F13 — sibling TOCTOU); ADR 0002 Amendment 6 (D38 — `tryAcquireSpawn` semantics that §7 coordination builds on); ADR 0004 Amendment 5 (D40 — observability pattern §11 extends); `CLAUDE.md` release_kit_overlay phase_rolling_mode — under Unreleased; CC 开发铁律 v1.6 § 10.x (design-only amendment; fresh-context reviewer not required per the Iron Rule 10 implementation-phase scope, documented in the amendment's procedural mechanism). +- **Test count:** 468 → 468 (no test change — design-only). + ## v0.1.0 — 2026-05-24 ### Phase 1 Close — Multi-provider proxy core diff --git a/README.md b/README.md index 0cfdb73..70907c9 100644 --- a/README.md +++ b/README.md @@ -190,6 +190,15 @@ Phase 1 is in progress. This table reflects what is currently shipped vs. what i | `scripts/migrate-from-ocp.mjs` | 📋 Planned (Phase 7) | OCP → OLP migration tool | | `setup.mjs` | 📋 Planned | Setup wizard / initial config | +### Known limitations + +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. +- **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 not yet implemented.** All requests today share the cache namespace `__anonymous__`. The cache data model is keyed by `keyId` and ready to accept real identities when `lib/keys.mjs` lands. Tracked in [v1.x roadmap #2](./docs/v1x-roadmap.md). +- **Provider-level `cacheKeyFields` mask not implemented.** Cache keys include every IR field including ones individual plugins drop at spawn (e.g., Anthropic plugin drops `temperature`). Spurious cache misses possible (extra spawn cost; never spurious hits). Conservative posture documented in [ADR 0005 Amendment 7](./docs/adr/0005-cache-cross-provider.md). Tracked in [v1.x roadmap #5](./docs/v1x-roadmap.md). + --- ## Architecture diff --git a/docs/adr/0005-cache-cross-provider.md b/docs/adr/0005-cache-cross-provider.md index fe33330..2d2030d 100644 --- a/docs/adr/0005-cache-cross-provider.md +++ b/docs/adr/0005-cache-cross-provider.md @@ -7,6 +7,139 @@ ## Amendments +### Amendment 8 — 2026-05-25: Streaming singleflight — v1.x design ratification (D42, issue #16) + +**Status:** Design ratified. Implementation deferred to v1.x. + +**Context.** Amendment 6 (D34) formally deferred streaming-path D4 singleflight participation with the note "the design alone warrants a dedicated ADR." Round-6 cold-audit F13 (filed as issue #16) raised the sibling TOCTOU window: `server.mjs:782 preCheckHit = await cacheStore.peek(...)` followed by streaming-branch entry at line 811 (gated on `!preCheckHit`) creates a race where, between peek and spawn, a concurrent populator can write the cache OR a TTL can expire. The streaming branch is path-locked at the moment of the peek result. + +This amendment ratifies the v1.x design so the implementation work has a single specification to follow. + +**Design — per-(keyId, cacheKey) inflight Map + tee-streaming + bounded per-client backpressure.** + +1. **Coordination primitive.** Extend `CacheStore` with `getOrComputeStreaming(keyId, cacheKey, sourceFactory): { stream: AsyncIterator, isFirst: boolean }`. Internally maintains `_streamingInflight: Map`. Three outcomes on call: + - **Cache hit** (cached entry exists and is alive): synthesize an async iterator that yields the cached chunks. `isFirst = false`. No spawn. + - **Inflight join** (entry exists in `_streamingInflight`): attach a new `AttachedClient` to the existing entry. `isFirst = false`. No spawn. + - **Cache miss + no inflight**: create a new `StreamingInflightEntry`, invoke `sourceFactory()` to obtain the underlying spawn iterator, attach as the source. `isFirst = true`. Subsequent identical-key callers join this entry until it completes or aborts. + + The Map check + insert is synchronous (no `await` between read and write), matching the D38 `tryAcquireSpawn` atomicity invariant. Document this in the implementation header. + +2. **StreamingInflightEntry shape.** + + ``` + { + compositeKey: string, // keyId + '\0' + cacheKey + source: AsyncIterator, + sourceAbortController: AbortController, + accumulatedChunks: IRChunk[], // for late joiners (replay buffer) + attachedClients: Set, + sourceDone: boolean, // source iterator exhausted + sourceError: Error | null, // non-null if source threw + sourceAborted: boolean, // true if AbortController fired + spawnAcquiredProvider: string | null, // for D38 release coordination + } + ``` + +3. **AttachedClient shape.** + + ``` + { + id: string, // request ID (D40 fallback log correlator) + queue: IRChunk[], // per-client tee buffer + queueByteSize: number, // running sum of JSON.stringify(chunk).length for cap + yieldedAccumulated: boolean, // true after late-joiner replay drained + done: boolean, + resolveNext: ((chunk) => void) | null, // promise resolver for the next chunk + rejectNext: ((err) => void) | null, + } + ``` + +4. **Tee fan-out loop (single-reader, multi-writer).** Source iterator is drained by ONE reader (the entry's tee task), which on each chunk: + - Pushes the chunk into `accumulatedChunks` (late-joiner replay buffer; bounded — see §10). + - For each `client ∈ attachedClients`: if `client.queueByteSize + chunkSize > PER_CLIENT_QUEUE_CAP` (default 1 MB), the client is disconnected with `STREAM_BACKPRESSURE` (see §8). Otherwise push the chunk into `client.queue`, update `queueByteSize`, fire `resolveNext` if pending. + + When the source iterator returns/throws/aborts, the tee task: + - On normal completion: writes `accumulatedChunks` to cache via the standard cache-write conditions (cacheable opt-out, truncated-not-cached, size cap — all from Amendment 1/3/5). Resolves all clients' `resolveNext` with their remaining queue then sentinel-marks `done`. Releases the D38 spawn slot once. Removes the entry from `_streamingInflight`. + - On source error: rejects all clients with the error. Does NOT write cache. Releases the D38 spawn slot. Removes entry. + - On source abort (all clients disconnected): cancels the iterator via AbortController, releases the slot, removes entry. No cache write (partial response not persisted, matches D16 buffered-path SPAWN_FAILED salvage NOT applying to abort). + +5. **Late-joiner policy.** When a new client attaches mid-stream: + - Drain `accumulatedChunks` into the client's queue immediately (synchronous burst). + - If the burst exceeds `PER_CLIENT_QUEUE_CAP`, the client is rejected immediately with `STREAM_BACKPRESSURE` (the implication is that the source has produced more than 1 MB before this client attached — late joiner is too late to catch up). + - From that point on, the client receives live chunks via the tee loop. + +6. **Cache TTL race during inflight.** If a cache entry is alive at peek time but expires during the inflight period, late joiners that arrive AFTER expiry still see the inflight entry in `_streamingInflight` (Map lookup precedes cache peek per the new contract). They attach via inflight join. No fresh spawn. The expired cache entry is overwritten by the inflight completion. + +7. **D38 maxConcurrent coordination.** Only the first caller's source-spawn calls `tryAcquireSpawn`. Subsequent attached clients DO NOT call it — they share the existing spawn slot. On source completion / error / abort, `releaseSpawn` fires once. If `tryAcquireSpawn` returns false for the first caller, the request fails with `CONCURRENCY_LIMIT` per D38 (existing behavior) and the streaming branch is not entered. + +8. **Backpressure error code.** New `PROVIDER_ERROR_CODES.STREAM_BACKPRESSURE`. **NOT a hard trigger** — the source spawned successfully; only one client's queue overflowed. The affected client receives a synthetic `{ type: 'stop', finish_reason: 'length' }` followed by `[DONE]` (matching D35 #10 truncation marker pattern). Server logs `stream_backpressure_disconnect` with `{ provider, model, client_id, queue_byte_size, per_client_cap }`. Other attached clients continue receiving chunks normally. + +9. **Client mid-stream disconnect (network drop / abort).** The HTTP response stream's `close` event triggers client cleanup: remove from `attachedClients`, no fallback advancement (the source is still running for other clients). If `attachedClients.size === 0`, the tee task fires `sourceAbortController.abort()` (which propagates to the underlying CLI spawn — D38's plugin spawn loops already handle AbortController per ADR 0002 § Provider contract). + +10. **Replay buffer cap.** `accumulatedChunks` is bounded at `ACCUMULATED_REPLAY_CAP` (default 10 MB, matches the existing cache-entry size cap from D23). If the source produces more than the cap before completion, the entry is marked NOT cacheable (cache write skipped at source-complete). Late joiners attaching past the cap receive `STREAM_BACKPRESSURE` immediately (the replay burst would exceed `PER_CLIENT_QUEUE_CAP`). First caller's stream continues unaffected because they were attached before the cap was hit. + +11. **Observability.** New log events: + - `streaming_inflight_join` — fires when a request attaches to an existing inflight entry. Fields: `{ provider, model, attached_count_after, accumulated_chunk_count }`. + - `streaming_inflight_source_done` — fires when the source completes. Fields: `{ provider, model, attached_count, accumulated_chunk_count, cache_written }`. + - `stream_backpressure_disconnect` — see §8. + - `streaming_inflight_abort` — fires when all clients disconnect and source is aborted. Fields: `{ provider, model, accumulated_chunk_count }`. + + New X-OLP-* header: `X-OLP-Streaming-Inflight: source | attached | solo` distinguishing which role this client played. `solo` = first caller, no joiners during stream (functionally equivalent to today's behavior). `source` = first caller, ≥1 joiner attached. `attached` = joined an existing inflight entry. Adds one field to the X-OLP-* set (currently 5); update D18-D40 documentation when implementation lands. + +12. **Server.mjs wiring.** Replace the current streaming branch peek+spawn pattern (server.mjs:782 `preCheckHit = await cacheStore.peek(...)` and lines 811–817 conditional) with: + ```js + const { stream, isFirst } = await cacheStore.getOrComputeStreaming( + keyId, + streamCacheKey, + async () => { + // sourceFactory: invoked only on first caller; encapsulates the D38 + // tryAcquireSpawn gate and provider.spawn invocation + ... + } + ); + ``` + `isFirst` plumbed into the X-OLP-Streaming-Inflight header. The TOCTOU window collapses because Map check + insert is synchronous. + +13. **Test surface (when implementation lands).** At minimum: + - Single client streaming (`isFirst=true`, no joiners) — behavior identical to today. + - 2 concurrent identical streams — only 1 spawn (`getActiveSpawnCount` returns 1 at the spawn peak); both clients receive identical chunk sequences in order. + - 3 concurrent, mid-stream join — client 2 attaches mid-stream, receives accumulated burst + live tail; client 3 attaches after source-complete, served from cache. + - First-client disconnect mid-stream, clients 2/3 continue, source NOT aborted. + - All clients disconnect mid-stream → source aborted (AbortController fired), no cache write. + - Source errors mid-stream → all attached clients receive the error; no cache write. + - Backpressure: slow client → `PER_CLIENT_QUEUE_CAP` exceeded → `STREAM_BACKPRESSURE` disconnect with `finish_reason: 'length'`; other clients unaffected. + - D38 semaphore: 2 concurrent identical streams hitting `maxConcurrent=1` — first spawns, second JOINS (does not get CONCURRENCY_LIMIT). 3 concurrent DIFFERENT streams hitting `maxConcurrent=2` — first 2 spawn, third gets CONCURRENCY_LIMIT (existing D38 path). + - Cache TTL race: entry expires during inflight; late joiner attaches via inflight join; inflight completion overwrites the expired cache slot. + - Replay buffer cap: source produces > `ACCUMULATED_REPLAY_CAP`; entry marked not cacheable; late joiner past cap gets `STREAM_BACKPRESSURE`; first caller stream unaffected. + - X-OLP-Streaming-Inflight header values across all the above scenarios. + +14. **Defaults to ratify in implementation.** `PER_CLIENT_QUEUE_CAP = 1 MB`, `ACCUMULATED_REPLAY_CAP = 10 MB` (matches D23 cache-entry size cap), `STREAM_BACKPRESSURE` not in `HARD_TRIGGER_CODES`. These are starting points; v1.x implementation may tune based on real-world latency profiles. + +**Issue #16 status.** Stays OPEN as the v1.x implementation tracker. The issue body should be updated post-D42 to reference this amendment and adjust scope ("design ratified; implementation pending"). DO NOT close issue #16 until §13's test surface is green on the actual implementation. + +**Cross-references and safeguards (so the implementation is not forgotten):** +- `docs/v1x-roadmap.md` (new at D42) — single landing page for all v1.x deferrals, with streaming SF as item #1. Each entry cross-references the relevant ADR amendment and the GitHub issue. +- `lib/cache/store.mjs` — TODO comment near `getOrCompute` pointing at this amendment for the streaming sibling API. +- `server.mjs` — TODO comment near the streaming branch entry (line ~810) pointing at this amendment + issue #16 with the words "ADR 0005 Amendment 8 — v1.x". +- `README.md § Known limitations` — line item exposing this to users (current behavior: each concurrent identical streaming request spawns its own CLI). +- This amendment is item #1 in the next session-start handoff if the maintainer opens a v1.x sprint. + +**Why this is the right shape (rationale):** +- Mirrors the D4 buffered-path singleflight (`getOrCompute`) pattern, keeping the cache API surface coherent rather than fragmenting into two parallel coordination primitives. +- Reuses D38 `tryAcquireSpawn` semantics for the first-caller path; attached callers naturally don't consume slots. +- Late-joiner replay via `accumulatedChunks` resolves the case where a client arrives between source-spawn and source-complete without forcing it to wait for completion. +- Bounded per-client queue protects against the "one slow client stalls the source" failure mode; the slow client gets a clean `STREAM_BACKPRESSURE` disconnect instead of corrupting the tee for other clients. +- AbortController propagation ensures the source CLI process is reaped if all clients drop — no orphan processes consuming Anthropic/Codex/Mistral quota. + +**Authority:** +- Amendment 6 (D34 F1) — original deferral with "design alone warrants a dedicated ADR" note; this amendment is the dedicated ADR. +- GitHub issue #16 (round-6 F13) — sibling TOCTOU window; same root cause. +- ADR 0002 Amendment 6 (D38) — `tryAcquireSpawn` / `releaseSpawn` semantics that the §7 coordination builds on. +- D40 Amendment 5 — per-hop observability pattern that §11 extends to streaming. +- CC 开发铁律 v1.6 § 10.x — design ADR ratification; fresh-context reviewer not required for design-only amendments (no code change in D42). + +**Procedural mechanism:** CC 开发铁律 v1.6 § 10.x (design-only amendment per Iron Rule 10's implementation-phase scope — the implementation PR that lands this ADR's design will go through full Iron Rule 10 with a fresh-context opus reviewer at that time). + ### Amendment 7 — 2026-05-24: Document cache-key-vs-CLI-args discrepancy as v0.1 conservative trade-off (D34 F8) - **Finding:** Round-6 cold-audit F8 (P2) — Provider plugins (`lib/providers/anthropic.mjs`, `codex.mjs`, `mistral.mjs`) drop `temperature`, `max_tokens`, `top_p`, `stop`, `tools`, and `tool_choice` when spawning their respective CLIs. These CLIs (`claude -p`, `codex exec --json`, `vibe --prompt`) do not accept those flags. However, the cache key (per Amendment 2) includes all of them. Consequence: two requests that differ only in `temperature` produce identical CLI output (the CLI ignores it) but different cache keys → spurious miss. The caller pays the spawn cost twice for what is, at the CLI layer, the same request. diff --git a/docs/v1x-roadmap.md b/docs/v1x-roadmap.md new file mode 100644 index 0000000..90822ac --- /dev/null +++ b/docs/v1x-roadmap.md @@ -0,0 +1,105 @@ +# OLP v1.x Roadmap — Deferred Work Tracker + +**Purpose.** Single landing page for every Phase-1 deferral that an actual v1.x sprint must pick up. Each entry cross-references its ratifying ADR, its GitHub issue (if any), and the load-bearing code anchor so a future maintainer can resume without spelunking the commit history. + +**Status:** Living document. Add new entries at the top. Each item should answer: +1. **What** is deferred? +2. **Why** was it deferred (link the ratifying ADR amendment). +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. + +--- + +## #1 — Streaming-path singleflight + TOCTOU close + +- **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. + +## #2 — Multi-key auth (`lib/keys.mjs`) + +- **What.** Per-API-key identity, namespace scoping for the cache, ownership tier (owner vs guest) for header gating, and audit log of which key issued which request. +- **Why deferred.** Phase 1 ships single-tenant — the cache layer carries `keyId='__anonymous__'` (D5). No real user identity is needed for personal/family use today. +- **Design ADR (NOT YET RATIFIED).** No design ADR exists yet. v1.x sprint must produce one before implementation. +- **Tracking.** Not a GitHub issue (no governance event filed for it). Tracked here + in `AGENTS.md § Key files to know` (`lib/keys.mjs` marked 📋 Planned). +- **Blocks.** + - `X-OLP-Fallback-Detail` owner-only gating (D40 / ADR 0004 Amendment 5 — currently ungated). + - `/health` per-key visibility (currently anonymous-only). +- **Code anchors today.** + - `lib/cache/store.mjs` per-keyId namespace Map — the data model is already keyed by `keyId`; only the keyId source is hardcoded. + - `server.mjs` request handlers — the `keyId='__anonymous__'` constant needs to be replaced by a header/token lookup. +- **Trigger to start.** First multi-user deployment of OLP (e.g., maintainer + spouse + child accessing the same instance with separate identities). + +## #3 — Soft trigger reactivation (ADR 0004 Amendment 2) + +- **What.** Per-provider `quotaStatus` polling, `softThreshold` comparisons, soft-skip advancement when quota approaches limit. Currently `evaluateSoftTriggers` always returns `false` because `quotaSnapshot` is never populated. +- **Why deferred.** v0.1 hard triggers (SPAWN_FAILED / CLI_NOT_FOUND / SPAWN_TIMEOUT / CONCURRENCY_LIMIT) are sufficient for fallback advancement at personal/family scale. Soft triggers require persistent quota snapshots and a polling mechanism, which adds operational surface (timer drift, snapshot staleness, observability burden). +- **Design ADR.** [`docs/adr/0004-fallback-engine.md` Amendment 2](./adr/0004-fallback-engine.md) — explicit v1.x deferral with mitigations (startup warning if user configures soft thresholds without runtime enforcement). +- **Tracking.** Not a GitHub issue. Tracked here + via the startup warning in `server.mjs` (the `_softTriggersConfigured` warn emission). +- **Blocks.** + - Issue #8 (`X-OLP-Provider-Used` chain-origin semantics) — Option A (track `firstAttemptedProvider`) becomes preferable once soft triggers can fire. See ADR 0004 Amendment 6 § v1.x re-evaluation. + - `X-OLP-Fallback-Detail` `trigger_type: 'soft'` path — currently dead code, becomes live with this work. +- **Code anchors today.** + - `lib/fallback/engine.mjs` `evaluateSoftTriggers` (returns false unconditionally at v0.1). + - `lib/providers/base.mjs` `Provider.quotaStatus` contract (declared but unused at v0.1). +- **Trigger to start.** First quota-rate-limit event in the wild — at which point the operator would want pre-emptive advancement rather than spawn-then-fail. + +## #4 — `/health` `activeSpawns` integration + +- **What.** Surface D38 `getActiveSpawnCount(providerName)` per-provider on the `/health` endpoint at the path `providers.status..activeSpawns`. +- **Why deferred.** D38 (issue #1) shipped the runtime enforcement and exported `getActiveSpawnCount`; `/health` integration was scoped out as forward-looking polish. +- **Design ADR.** [`docs/adr/0002-plugin-architecture.md` Amendment 6](./adr/0002-plugin-architecture.md) — names the target path explicitly: "`/health` integration deferred — when surfaced there will land at `providers.status..activeSpawns`; not wired at D38." +- **Tracking.** Not a GitHub issue. Tracked here. +- **Code anchors today.** + - `lib/providers/index.mjs` exports `getActiveSpawnCount` already. + - `server.mjs handleHealth` — extension point for the new field. +- **Trigger to start.** First time the maintainer wants per-provider concurrency visibility for capacity planning. + +## #5 — Provider-level `cacheKeyFields` (per-plugin mask) + +- **What.** Per-plugin declaration of which IR fields are actually consumed by the underlying CLI invocation, used by `computeCacheKey` to skip fields that the plugin drops at spawn. Reduces spurious-miss rate from the v0.1 conservative-posture trade-off (Amendment 7). +- **Why deferred.** At personal/family scale the extra spawn cost from spurious misses is negligible. The contract extension adds complexity (per-plugin field set + plumbing through `buildDefaultChain` → `executeHopFn` → `computeCacheKey`). +- **Design ADR.** [`docs/adr/0005-cache-cross-provider.md` Amendment 7 § Forward path](./adr/0005-cache-cross-provider.md). +- **Tracking.** Not a GitHub issue. Tracked here. +- **Code anchors today.** + - Plugin file headers — each lists its "fields dropped at spawn" table for human reference; the v1.x amendment makes that table machine-readable. + - `lib/cache/keys.mjs computeCacheKey` — would accept `pluginCacheKeyMask` parameter. +- **Trigger to start.** First time spurious-miss rate becomes a measurable load factor. + +## #6 — Streaming-path SPAWN_FAILED salvage + +- **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). +- **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). + +## #7 — AUTH_MISSING tuple path test coverage (D40 follow-up) + +- **What.** Dedicated test in `test-features.mjs` Suite D40 that asserts the `fallbackDetail` tuple records the AUTH_MISSING path with `trigger_type: 'auth_missing'`. D40 reviewer flagged this as the last gap in the engine-path matrix; code is structurally correct, just lacks an explicit pin. +- **Why deferred.** Low priority — the AUTH_MISSING early-return branch has the tuple push BEFORE it (verified in D40 reviewer pass), so coverage is implicit via the other engine-path tests. A 3-line dedicated test would make the pin explicit. +- **Design.** No ADR needed. ~5-line test addition. +- **Tracking.** Not a GitHub issue. Tracked here. +- **Trigger to start.** Next routine test-suite hardening pass, OR when AUTH_MISSING handling is changed for any reason. + +--- + +## Adding a new entry + +When a future D-day defers work, the deferring commit should: + +1. **Always** update this file with a new entry at the top. +2. **Always** name the ratifying ADR amendment (or note "no ADR yet — future work needs one"). +3. **Always** name the load-bearing code anchor (`file:line` form preferred over symbolic names — the symbolic name can drift). +4. **Always** name a concrete trigger to start the work — vague triggers ("when needed") let entries rot. +5. If the deferral has a GitHub issue, keep it OPEN and reference it here. If it does NOT, leave a note explaining why (e.g., "tracked here only — no external governance event filed"). + +The maintainer's session-startup discipline should grep this file at sprint kickoff. If an entry's "trigger to start" condition is met, it leaves this page and becomes a sprint item. diff --git a/lib/cache/store.mjs b/lib/cache/store.mjs index c77f883..aefc976 100644 --- a/lib/cache/store.mjs +++ b/lib/cache/store.mjs @@ -265,6 +265,14 @@ export class CacheStore { * @param {() => Promise<*>} computeFn - async function producing the value * @param {number} [ttlMs] * @returns {Promise<*>} + * + * TODO(v1.x — ADR 0005 Amendment 8 / issue #16): add a sibling + * `getOrComputeStreaming(keyId, cacheKey, sourceFactory)` for the streaming + * path. This API handles buffered responses only; the streaming branch in + * server.mjs currently uses a peek+spawn pattern with a TOCTOU window. + * The streaming sibling will mirror this method's shape but with a tee + * fan-out and per-client backpressure queues. See docs/v1x-roadmap.md #1 + * for the design contract and acceptance criteria. */ async getOrCompute(keyId, cacheKey, computeFn, ttlMs) { // 1. Cache hit — return immediately, no singleflight overhead diff --git a/server.mjs b/server.mjs index d7447d6..e836c69 100644 --- a/server.mjs +++ b/server.mjs @@ -807,6 +807,12 @@ async function handleChatCompletions(req, res) { // Acquired here, released in the `finally` below. The gate intentionally // lives BEFORE the streaming-branch entry check so the buffered fallthrough // can re-attempt acquire from a clean slate. + // TODO(v1.x — ADR 0005 Amendment 8 / issue #16): replace this peek+spawn + // pattern with cacheStore.getOrComputeStreaming(...) to close the TOCTOU + // window between line ~782 peek and the spawn at line ~846, and to make N + // concurrent identical streaming requests share one spawn (tee-streaming). + // Design ratified in D42; see docs/v1x-roadmap.md #1 for the trigger and + // acceptance criteria. DO NOT remove this comment until the v1.x impl lands. let streamingAcquired = false; if (ir.stream && chain.length === 1 && !bypassCacheForFirstHop && !preCheckHit && cacheableForFirstHop) { const candidatePlugin = loadedProviders.get(chain[0].provider);