From d6347e33f2853ec4ab26e652eed517b152bc2350 Mon Sep 17 00:00:00 2001 From: dtzp555 Date: Sun, 24 May 2026 17:06:16 +1000 Subject: [PATCH] =?UTF-8?q?docs(governance):=20D31=20=E2=80=94=20ADR=20ame?= =?UTF-8?q?ndment=20trio=20(F5=20ADR=200003=20+=20F11=20ADR=200005=20?= =?UTF-8?q?=C2=A7=20D2=20+=20F13/F14=20ALIGNMENT=20Speculative-Candidate?= =?UTF-8?q?=20class)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cold-audit catch from 2026-05-24 (round 3) Three coordinated governance amendments per the major-decisions already made (per "继续吧 unless major decision" + my F5/F11/F13+F14 option-(b) calls earlier in the session). All P3-class, all docs-only. Single PR per IDR cleanup-batch convention given the unified theme ("spec was aspirational; reality is X; amend spec to match"). Changes (3 docs files, +52 / -2): 1. docs/adr/0003-intermediate-representation.md — Amendment 1 (NEW Amendments section, first ever for ADR 0003): - F5 closure: removes the unimplemented `__irRoundTripTest()` export-name claim from § Mitigations. The mental model (symmetric irToNative+nativeToIR pair on each plugin) does NOT fit the actual plugin shape (asymmetric spawn-wrappers: request→CLI-args+stdin, response-stream→IR-chunks). Zero plugins export this function. - Documents the substitute test strategy that IS live at v0.1: · Suite 3 covers entry-surface IR↔OpenAI round-trip · Per-plugin spawn-with-mock test blocks cover IR→CLI→IR-chunk · Lossy-translation edges documented inline in each plugin's header comment (mistral.mjs DOCS-N markers being the most thorough; anthropic.mjs + codex.mjs carry equivalent tables) - Future plugins MUST satisfy both (a) spawn-with-mock test block and (b) header lossy-field documentation — the enforcement mechanism replacing the original export-name aspiration. - Original § Mitigations bullet replaced with a pointer to Amendment 1 (preserves history). 2. docs/adr/0005-cache-cross-provider.md — Amendment 5 (after D27 Amendment 4): - F11 closure: acknowledges that the "Anthropic's own prompt cache is consulted at the provider" half of § D2 is structurally impossible at v0.1. - Root cause: Anthropic plugin uses `claude -p --output-format text` (verified at anthropic.mjs:196), a plain-text wire surface that has no Messages-API field for cache_control markers. The markers are also stripped at IR construction (openai-to-ir.mjs:45-89 whitelists fields, drops cache_control) per ADR 0003 IR design reaffirmed in D27 Amendment 4 (F10). - Clarifies v0.1 effective behavior: OLP-cache-bypass works correctly (no double-caching when Anthropic + markers); the delegate-to-Anthropic-prompt-cache half does not fire. - Forward path (v1.x): switch Anthropic plugin to --output-format json (NDJSON wire) or direct Messages API spawn — substantial parser rewrite; deferred since no user has requested delegation. - § D2 paragraph stays intact; an inline parenthetical reference to Amendment 5 is added immediately after the affected sentence. 3. ALIGNMENT.md — new "Rule 4 exception class — Speculative-Candidate Plugin" sub-section after Rule 5, before § Authorities: - F13+F14 closure: formalizes the gap between strict ALIGNMENT Rule 2 (no speculative shape assumptions) + Rule 4 (unalignable plugins are deleted, not feature-flagged) and the actual practice where Codex (D6) + Mistral (D8) plugins ship with explicit UNPINNED assumptions documented in their headers, gated as Candidate-not-Enabled. - 5 precise conditions for a Speculative-Candidate plugin: (1) STATIC_REGISTRY entry present, (2) candidate: true in models-registry, (3) NOT in any user's providers.enabled config, (4) header has explicit UNPINNED assumption section with labeled IDs and planned-pin triggers, (5) defensive multi-shape parsers tied to UNPINNED markers (greppable for future single-shape replacement). - Enablement criteria (Speculative-Candidate → Enabled): 3 explicit requirements (remove UNPINNED labels from header; replace multi-shape parsers with single-shape; file pin ADR amendment). - Currently-in-class table: · codex.mjs (D6) — A3 (auth token field name), A4 (NDJSON event schema) · mistral.mjs (D8) — A4 (JSON output event schema), A5 (model flag), A6 (exact model IDs), A7 (streaming vs json output mode), A8 (stdin prompt passing) - Anthropic explicitly NOT in this class (CLI version pinned at @anthropic-ai/claude-code v2.1.89 per Provider Authority Pins table). - CI enforcement noted as deferred (reviewer obligation today; a future PR may add grep-based gate). Pre-commit fold-in (per evidence-first checkpoint #4): - **D31 reviewer flagged wording precision** ("Provider Inventory table above" was structurally wrong — the table is BELOW the new sub-section). Folded in: 1-word fix "above" → "below" on the Speculative-Candidate condition #2. Tests: 400/400 unchanged (docs-only). Authority: - ADR 0003 § Mitigations (modified) + new Amendment 1 (self-amendment) - ADR 0005 § D2 (annotated) + new Amendment 5 (self-amendment) - ALIGNMENT.md § Rules (extended with Rule 4 exception class) - D27 F10 Amendment 4 of ADR 0005 (cited for cache_control IR-strip precedent) - CC 开发铁律 v1.6 § 10.x — Round-3 Cold Audit caught all 3 items Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent of drafter): APPROVE_WITH_MINOR. Verified: - Suite 3 + 3 plugin spawn-with-mock blocks + 3 plugin lossy-field header tables all present (F5 substitute test strategy claims match implementation) - anthropic.mjs:196 hardcodes --output-format text + openai-to-ir.mjs drops cache_control from messages (F11 wire-limitation technically accurate) - All 8 UNPINNED assumption labels (codex A3/A4 + mistral A4/A5/A6/A7/A8) match plugin header text exactly — no invention - Anthropic-not-in-class claim verified against Provider Authority Pins table Follow-up items (reviewer's non-blocking notes, NOT in this PR): 1. **mistral.mjs A5 internal disconnect** — header lines 145-156 label A5 as `UNPINNED-D-later-verifies`, but function body at 371-374 already cites DeepWiki enumeration as the pin and marks A5 `CONFIRMED-NOT-APPLICABLE` (no --model flag exists in vibe CLI). The disconnect predates D31 and is a per-plugin header cleanup (not a constitutional amendment), so it stays out of D31 scope per IDR. File as separate follow-up issue — A5 should be moved out of the Speculative-Candidate table once the header is updated. 2. **ADR 0005 Amendment ordering cosmetic** — strict reverse-chronological would put Amendment 5 above Amendment 4 (current order is 4, 5, 3, 2 because both 4 and 5 share the 2026-05-24 date and 5 was inserted after 4). Not blocking; future cleanup if maintainers want strict numeric-descending. Co-Authored-By: Claude Opus 4.7 --- ALIGNMENT.md | 28 ++++++++++++++++++++ docs/adr/0003-intermediate-representation.md | 16 ++++++++++- docs/adr/0005-cache-cross-provider.md | 10 ++++++- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/ALIGNMENT.md b/ALIGNMENT.md index ebfb446..1b52953 100644 --- a/ALIGNMENT.md +++ b/ALIGNMENT.md @@ -34,6 +34,34 @@ The five Rules below apply to every PR touching a provider plugin, the entry sur 5. **Rule 5 (Cite in Commits).** Every commit touching `lib/providers/.mjs` must cite the provider CLI authority in the commit body. Every commit touching the entry surface must cite the OpenAI spec section URL. Every commit touching the IR must cite the authorizing ADR. CI performs a soft check for the citation pattern; reviewers enforce per `CLAUDE.md`. +### Rule 4 exception class — Speculative-Candidate Plugin (D31 F13+F14) + +Rule 4 says "unalignable plugins / fields are deleted, not feature-flagged." This sub-section formalizes a narrow exception: a plugin may ship in `lib/providers/.mjs` with UNPINNED assumptions documented in its header IF AND ONLY IF all five of the following hold: + +1. The plugin's `STATIC_REGISTRY` entry is present (so the Provider contract validates at startup). +2. The plugin is `candidate: true` in `models-registry.json` (or equivalently, the plugin's row in the Provider Inventory table below is in the **Candidate Providers** section, not **Enabled Providers**). +3. The plugin is NOT in any user's `providers.enabled` config — i.e., it cannot be Enabled in any deployment. `POST /v1/chat/completions` returns 503 for this provider until a Phase E2E audit passes and the enabled flag is explicitly set. +4. The plugin header contains an explicit section enumerating each speculative shape assumption with a labeled assumption-ID (e.g., A3, A4) and a planned-pin trigger (e.g., "D7 will probe via real CLI capture", "first enable will pin"). +5. The plugin's defensive multi-shape parsers (e.g., trying multiple NDJSON event field names) are tied to the UNPINNED markers — a future implementer can grep the header assumption labels to find every speculative branch and remove all but the actually-observed shape at pin time. + +A plugin satisfying all five conditions is a **Speculative-Candidate**. It is Rule 2 / Rule 4 compliant via this exception class. + +**Enablement (Speculative-Candidate → Enabled)** requires: +- Removing the UNPINNED assumption labels from the header (replacing each with `CONFIRMED` or `CONFIRMED-NOT-APPLICABLE` as appropriate). +- Replacing defensive multi-shape parsers with single-shape implementations matching the observed (now-pinned) wire format. +- Filing a per-assumption ADR amendment or a single inclusion ADR (per ADR 0006) documenting the primary-source pin for each assumption that was UNPINNED. + +**Currently in this class (D31 ratification, 2026-05-24):** + +| Plugin file | Phase | Labelled UNPINNED assumptions | +|---|---|---| +| `lib/providers/codex.mjs` (D6) | Phase 2 | A3 (auth token field name), A4 (NDJSON event schema) | +| `lib/providers/mistral.mjs` (D8) | Phase 3 | A4 (JSON output event schema), A5 (model flag), A6 (exact model IDs), A7 (streaming vs json output mode), A8 (stdin prompt passing) | + +The Anthropic plugin (`lib/providers/anthropic.mjs`, D4–D5) is NOT in this class — its CLI authority is pinned (`@anthropic-ai/claude-code` v2.1.89 per the Provider Authority Pins table above). It conforms to the standard Rule 4 path. + +**CI enforcement note:** The `alignment.yml` workflow does not currently enforce the Speculative-Candidate annotation requirement automatically. The five-condition check is a reviewer obligation. A future PR may add a grep-based gate requiring any file in `lib/providers/` that contains the string `UNPINNED` to also contain the five-condition annotations. D31 establishes the class; automated enforcement is deferred. + --- ## Authorities diff --git a/docs/adr/0003-intermediate-representation.md b/docs/adr/0003-intermediate-representation.md index 49466b0..6597fbf 100644 --- a/docs/adr/0003-intermediate-representation.md +++ b/docs/adr/0003-intermediate-representation.md @@ -5,6 +5,20 @@ - **Authors:** project maintainer (with AI drafting assistance) - **Related:** OLP v0.1 spec §4.1, §4.2 (IR subsection); ADR 0002 (plugin architecture); ADR 0004 (fallback engine — requires shape-stable requests for safe replay) +## Amendments + +### Amendment 1 — 2026-05-24: Remove __irRoundTripTest() claim; document substitute test strategy (D31 F5) + +- **Finding:** Round-3 cold-audit F5 (P2 ADR-claim vs implementation drift) — § Mitigations names a `__irRoundTripTest()` export on every provider plugin as the structural counter-measure against silent lossy translation. ZERO plugins export this function; the export-name was an early-design aspiration that does not fit the actual plugin shape (spawn-wrappers with asymmetric request→CLI-args+stdin and response-stream→IR, not symmetric `irToNative`+`nativeToIR` functions that could be tested in isolation via a round-trip fixture). +- **Decision:** Remove the `__irRoundTripTest()` export-name claim from § Mitigations. The structural counter-measure against silent lossy translation is preserved via the existing test strategy documented below; the goal (CI-visible lossy-translation coverage) is met; only the mechanism name was wrong. +- **Substitute test strategy (live at v0.1):** + - Entry-surface IR↔OpenAI round-trip — Suite 3 (`irChunkToOpenAISSE format` + `openai-to-ir` tests in `test-features.mjs`) covers the OpenAI ↔ IR direction at the entry surface. + - Per-plugin spawn-with-mock tests in each plugin's test block (anthropic, codex, and mistral spawn unit tests) exercise the IR → CLI args → IR-chunk round-trip end-to-end. + - Lossy-translation edges are documented inline in each plugin's header comment; `mistral.mjs`'s explicit "DOCS-N" UNPINNED markers are the most thorough example; `anthropic.mjs` and `codex.mjs` carry equivalent lossy-field tables in their headers. + - Future plugins MUST include both: (a) a spawn-with-mock test block, and (b) header documentation of any lossy IR fields. These two requirements are the enforcement mechanism replacing the `__irRoundTripTest()` export-name aspiration. +- **Original § Mitigations update:** The `__irRoundTripTest()` sentence in § Mitigations below is replaced with a pointer to this amendment. See § Mitigations. +- **Procedural mechanism:** CC 开发铁律 v1.6 § 10.x (Round-3 Cold Audit caught it as F5). + ## Context OLP exposes exactly one external API surface: OpenAI-compatible `/v1/chat/completions` (per spec §4.1). Internally, OLP fans out to N providers, each with its own native protocol — Anthropic Messages API, OpenAI Chat Completions, Mistral La Plateforme, xAI Grok, Moonshot Kimi, etc. The combinatorial fact is unavoidable: a request entering as OpenAI shape must exit as Anthropic shape (or Mistral shape, or whatever the routing chain advances to), and the response on the way back must travel the inverse path. @@ -86,7 +100,7 @@ for await (const irChunk of irResponseStream) { **Mitigations** - IR v1.0 is deliberately a subset, not a superset, of what providers can do. Where providers diverge in expressive power (tool calling, structured outputs, reasoning modes), the IR encodes the *common subset* in v1.0, with caveats documenting provider extensions. Expanding the IR to cover provider extensions is an explicit amendment, not a silent generalization. -- Each provider plugin includes a `__irRoundTripTest()` export that exercises the plugin's `irToNative` + `nativeToIR` pair against a fixture set in `test-features.mjs`. Round-trip discrepancies fail CI. This is the structural counter-measure against the silent-lossy-translation failure mode. +- *(The `__irRoundTripTest()` export-name claim that originally appeared here was removed by Amendment 1 (D31 F5). The structural counter-measure against silent lossy translation is the substitute test strategy described in Amendment 1: entry-surface round-trip tests in Suite 3 of `test-features.mjs` + per-plugin spawn-with-mock test blocks + inline lossy-field header documentation. Future plugins must satisfy both requirements.)* - The IR has no external surface. If IR v1.0 turns out to be the wrong shape and IR v2.0 is incompatible, the migration is OLP-internal: provider plugins are updated, the entry adapter is updated, no client code changes. The cost of being wrong is bounded by the project's own codebase. ## Alternatives considered diff --git a/docs/adr/0005-cache-cross-provider.md b/docs/adr/0005-cache-cross-provider.md index 8e34b7e..765ea1d 100644 --- a/docs/adr/0005-cache-cross-provider.md +++ b/docs/adr/0005-cache-cross-provider.md @@ -15,6 +15,14 @@ - **No code change:** F10 is a docs-only clarification. The bypass behavior is correct; the cache key slot's nullness is intentional given v1.0 IR design. - **Procedural mechanism:** CC 开发铁律 v1.6 § 10.x (Round-3 Cold Audit). +### Amendment 5 — 2026-05-24: Acknowledge claude -p --output-format text wire limitation for D2 (D31 F11) + +- **Finding:** Round-3 cold-audit F11 (P3 ADR-claim vs implementation drift) — § D2 promises "Anthropic's own prompt cache is consulted at the provider" as part of the bypass rationale, but this is structurally impossible at v0.1. The Anthropic plugin uses `claude -p --output-format text`, a plain-text wire surface that has no Messages-API field for `cache_control` markers. The markers are also stripped at IR construction (`openai-to-ir.mjs` whitelists fields and drops `cache_control`) per the ADR 0003 IR whitelist policy documented in Amendment 4 (D27 F10). +- **Clarification:** § D2's actual effect at v0.1 is OLP-cache-bypass only — when the request contains Anthropic `cache_control` markers AND the active hop is Anthropic, OLP does NOT write to its response cache, leaving the request to spawn fresh. The "Anthropic's own prompt cache is consulted at the provider" delegation half is NOT live at v0.1 because the wire surface (`claude -p --output-format text`) does not carry `cache_control` markers to the Anthropic backend. The OLP-cache-bypass half IS correct and working. +- **Forward path (deferred to v1.x):** For full delegation (actually forwarding `cache_control` markers to Anthropic's prompt-cache infrastructure), the Anthropic plugin would need to switch from `claude -p --output-format text` to either `--output-format json` (NDJSON wire with structured event envelopes that could carry `cache_control` in the request construction) or a direct Anthropic Messages API spawn. Both options are substantial parser rewrites. No user has reported needing the delegation behavior at v0.1; the OLP-cache-bypass half (no double-caching) is the practically valuable behavior. +- **No code change:** F11 is a docs-only clarification. The bypass behavior is correct; the wire limitation is an honest-spec acknowledgment. An inline reference to this amendment is added to § D2 below. +- **Procedural mechanism:** CC 开发铁律 v1.6 § 10.x (Round-3 Cold Audit caught it as F11). + ### Amendment 3 — 2026-05-24: Implement § "Cache write conditions" items 3 and 4 (D23) - **Finding:** Cold-audit round-2 Finding 3 (P2 cache-condition drift) — § "Cache write conditions" items 3 and 4 were documented in this ADR but never wired in code. Zero matches for `cacheable` / `10485760` / any size-cap pattern in `lib/` or `server.mjs`. The invariants existed only in prose. @@ -88,7 +96,7 @@ The `` segment ensures per-key isolation; the `` is the > **Implementation note (as of 2026-05-24):** The v0.1 implementation in `lib/cache/store.mjs` uses an in-memory `Map` as the backing store — no files are written to `~/.olp/cache/`. The file-backed layout described above is the designed shape; it transitions in via a Phase 2 storage adapter. Per-key isolation and singleflight (D4) are live; file persistence is not. -**D2 — `cache_control` bypass (ported, scope-expanded).** If the IR request contains Anthropic `cache_control` markers AND the active provider in the current chain hop is Anthropic, the OLP response cache is bypassed (Anthropic's own prompt cache is consulted at the provider). If the active provider is not Anthropic, the `cache_control` markers are stripped from the IR before provider translation (so they don't get passed to providers that wouldn't understand them) and a debug log entry is emitted. Future provider-specific bypass markers extend this rule by adding their own provider-conditional bypass logic. +**D2 — `cache_control` bypass (ported, scope-expanded).** If the IR request contains Anthropic `cache_control` markers AND the active provider in the current chain hop is Anthropic, the OLP response cache is bypassed (Anthropic's own prompt cache is consulted at the provider). *(Wire-limitation note: at v0.1 the "Anthropic's own prompt cache is consulted at the provider" half is not live — the Anthropic plugin's `claude -p --output-format text` wire surface does not carry `cache_control` markers to the provider backend; the effective behavior is OLP-cache-bypass only. See Amendment 5 (D31 F11) for the full clarification and forward path.)* If the active provider is not Anthropic, the `cache_control` markers are stripped from the IR before provider translation (so they don't get passed to providers that wouldn't understand them) and a debug log entry is emitted. Future provider-specific bypass markers extend this rule by adding their own provider-conditional bypass logic. **D3 — chunked stream replay (ported).** When a cache hit occurs on a `stream: true` request, the cached response is replayed as SSE chunks with the same chunking granularity as the original spawn. This requires storing the chunk boundaries (not just the concatenated content) in the cache entry. Cache entry format includes a `chunks[]` array, each with `delta` and `timestamp` relative to spawn start; replay throttles to the original timing pattern (configurable: real-time replay vs. burst-replay; default burst-replay because real-time replay adds latency without value for cached requests).