External Codex CLI review pass 2 surfaced 6 substantive issues that round 1 fold-in missed — the self-consistency trap recurred when fold-in was scoped only to files codex explicitly named in round 1. This commit closes round 2 in full. 1. ADR 0002 contradicted ALIGNMENT.md (P1, codex round 2 finding 1) ADR 0002 still said "three default-enabled (Anthropic, OpenAI Codex, Mistral Vibe)" while ALIGNMENT.md (post round 1) said v0.1 ships zero Enabled Providers. Accepted ADR contradicted constitution. Fix: ADR 0002 + ADR 0001 + docs/adr/README.md index rewritten to Candidate framing. 2. release.yml would publish stale v0.1.0-bootstrap notes (P1, round 2 finding 2) The "Unreleased" amendments would have been silently dropped on tag push because release.yml extracts only the matching version section. Fix: CHANGELOG restructured so the amended state IS the v0.1.0- bootstrap section. Full review history (opus + 2 codex rounds) captured inline. 3. package.json advertised non-existent entrypoints (P2, round 2 finding 3) main/scripts.test/scripts.start pointed to files that do not exist. Local npm test and npm start failed; CI masked. Fix: remove all three from package.json. They return in Phase 1 alongside the real files. test.yml bootstrap-tolerance updated to also skip when scripts.test is absent. 4. models-registry.json missing despite SPOT claim (P2, round 2 finding 4) Fix: minimal stub committed (version + empty providers map). alignment.yml validator now actually runs. 5. alignment.yml commit-citation soft check Bash subshell trap (P2, round 2 finding 5) git log ... while read ... WARN=1 — the while loop ran in a subshell because of the pipe, so WARN never propagated out. The post-loop check always reported "clean" even when warnings fired. Fix: process substitution done less than less than (git log ...). 6. Tier A "permanent" wording inconsistent across ADR 0006 + alignment. yml workflow text (P3, round 2 finding 6) Fix: unified to "Excluded by default with no routine reinstatement path; re-inclusion requires ADR 0006 supersession or amendment with new primary-source evidence." Reviewer: OpenAI Codex CLI (external, fresh-context, pass 2). Iron Rule 10 satisfied — round 2 reviewer was not the implementer of round 1 fold-in. Memory learning updated: the self-consistency trap recurs in the fold-in step. Future fold-ins must grep the entire repo for the concept, not only edit files the reviewer named. See learnings/ai_reviewer_self_ consistency_trap.md in cross-machine memory. Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
11 KiB
ADR 0002 — Plugin Architecture for Providers
- Date: 2026-05-23
- Status: Accepted (bootstrap)
- Authors: project maintainer (with AI drafting assistance)
- Related: OLP v0.1 spec §4.2; ADR 0001 (project founding); ADR 0003 (IR design); ADR 0006 (provider inclusion framework)
Context
OLP declares a curated set of candidate providers — three anticipated Tier D (Anthropic, OpenAI Codex, Mistral Vibe), two anticipated Tier C (xAI Grok, Moonshot Kimi), and three anticipated Tier B (MiniMax, Zhipu GLM, Alibaba Qwen). Per ALIGNMENT.md § Provider Inventory, all 8 ship as Candidate at v0.1 founding; transition to Enabled requires authority pin filled + plugin landed + Phase audit passed. Each provider has its own CLI binary, its own auth artifact location, its own request shape, its own response shape, its own quota-reporting endpoint (or none), and its own rate-limit posture. The maintainer's strong prior is that this set grows over the project's lifetime — provider economics will continue to shift, and "the right five providers" in 2027 will not be identical to today's five.
The naive architecture is a monolithic dispatcher inside server.mjs:
if (provider === 'anthropic') { spawn claude -p ... }
else if (provider === 'openai') { spawn codex exec --json ... }
else if (provider === 'mistral') { spawn vibe --prompt ... }
// ... and so on for every provider, every auth shape, every quirk
This shape works for two providers, becomes painful at four, and is the structural shape that produced the worst pages of OCP's server.mjs (1667 lines, ADR 0005 context paragraph). Worse, it makes the answer to "how does a contributor add a sixth provider?" be "edit eight places inside server.mjs and hope you caught them all." That is the exact failure mode models.json SPOT (OCP ADR 0003) was designed to prevent for model metadata; the provider equivalent needs the same structural answer.
The other end of the spectrum is full external plugin discovery — npm-installed plugins, runtime registration, hot-load. That is unambiguously out of scope for v1.0: the provider set is curated for security and ToS-risk reasons (see ADR 0006), and "anyone can install a third-party plugin" violates that curation by design.
The middle path is a plugin model with a fixed in-tree provider registry: each provider is a .mjs file under lib/providers/, all conforming to a single Provider contract, loaded at startup from a static enumeration in lib/providers/index.mjs. Adding a provider means writing one file and adding one line to the registry. Disabling an optional provider means a config-file toggle, not a code change.
Decision
Per spec §4.2, OLP uses a plugin-based provider architecture with the following structure:
Filesystem layout:
lib/providers/
base.mjs # abstract Provider contract + shared helpers
index.mjs # static registry (enumeration of in-tree providers)
anthropic.mjs # spawn `claude -p` — port of OCP server.mjs spawn logic
codex.mjs # spawn `codex exec --json`
vibe.mjs # spawn `vibe --prompt --output json`
grok.mjs # spawn `grok -p --output-format streaming-json` (optional)
kimi.mjs # spawn `kimi -p --output-format stream-json` (optional)
minimax.mjs # tier-2 optional, default-disabled
glm.mjs # tier-2 optional, default-disabled
qwen.mjs # tier-2 optional, default-disabled
Provider contract (v1.0 interface — exact shape per spec §4.2):
Every provider plugin exports an object conforming to:
name: string— unique key (anthropic,openai,mistral, etc.)displayName: string— human-readable name for dashboards and consent UXmodels: string[]— models this provider servesauth: { type, storage, path, refresh }— auth-artifact profilespawn: async (normalizedRequest, authContext) => AsyncIterator<ResponseChunk>— the core invocationestimateCost: (request) => { inputTokens, outputTokensEstimate, currency, usd }— best-effort, may return nullquotaStatus: async (authContext) => { available, percentUsed, resetsAt, pool }— best-effort, null if unretrievablehealthCheck: async () => { ok, latencyMs, error? }— startup and/healthendpoint use thishints: { requiresTTY, concurrentSpawnSafe, maxConcurrent }— fingerprint and concurrency hints
Loading model. lib/providers/index.mjs is a hand-maintained static enumeration. There is no filesystem scan, no require.context, no dynamic discovery. Adding a provider requires:
- Write
lib/providers/<name>.mjsconforming to the contract. - Add one import + one entry to
lib/providers/index.mjs. - Add a row to README's "Supported Providers" table.
- File an inclusion ADR per ADR 0006's framework.
Disable model. Optional providers (tier-1 and tier-2 per ADR 0006) are present in the registry but enabled: false by default. Enable is a ~/.olp/config.json toggle, plus the tier-2 consent flow described in spec §3.1. Disabling a provider does not require touching server.mjs.
Boundary with server.mjs. server.mjs knows about the registry and the contract; it does not know about specific providers. The fallback engine (ADR 0004), the cache layer (ADR 0005), and the dashboard (spec §4.6) all consume providers through the contract, not through provider-specific code paths.
Consequences
Positive
- Adding a new provider is a four-step recipe with no
server.mjsedits required. The recipe is explicit (file + registry + README + ADR), so a future contributor (including a future Claude session) cannot accidentally do steps 1–2 without 3–4. - The contract is the test surface. A provider plugin can be tested in isolation against a contract conformance suite (
test-features.mjsextended per spec §6 Phase 1), independent ofserver.mjs. server.mjsstays generic. There is no "is this Anthropic? then do special thing" path inside the core proxy loop. Provider-specific quirks live inside the provider plugin where they belong.- Disabling a misbehaving provider (e.g., a ToS change announcement triggers fast-disable per spec §9 risks) is a config flip, not a code revert. The provider quarantine path spec §9 calls for is the existing config mechanism.
Negative
- The contract surface is real governance work. Adding a field to the contract (e.g., a new
streamingModeortoolUseShape) is an ADR amendment per OLP ALIGNMENT.md, not a quick PR. This is intentional — contract drift is the path back to the monolithic-dispatcher problem the contract was built to prevent. - Provider plugins have non-trivial duplication: every provider re-implements the same SSE-chunk-translation skeleton, the same auth-env-injection, the same spawn-with-timeout.
base.mjsexists to absorb the truly-shared parts, but resisting the temptation to push provider-specific logic intobase.mjsrequires discipline. - Some providers (notably Anthropic) have much more behavior to encode than others (Mistral Vibe is comparatively spartan). The contract has to be expressive enough for the rich case without being burdensome for the spartan case. Lossy translations are documented per-provider per ADR 0003.
Mitigations
base.mjsprovides shared helpers but does not implement theProvidercontract itself. Provider plugins compose helpers; they do not inherit from a base class. This keeps the "what does this provider do?" question answerable by reading one file.- The contract is versioned. v1.0 is the subset in this ADR; future additions (e.g., a
cancel()method for in-flight request termination, or acostPerTokensnapshot) require ADR amendment plus a contract-version bump. Old provider plugins continue to declarecontractVersion: '1.0'and the loader handles the version gap. - The provider inclusion ADR per ADR 0006 doubles as the contract-conformance review gate. A new provider's inclusion ADR must show how it satisfies each field of the contract; that review is the structural counter-measure against contract drift.
Alternatives considered
(a) Monolithic dispatch inside server.mjs. A single function with if/else if per provider, each branch implementing spawn/quota/health inline. Rejected: this is the architectural shape that produced OCP's server.mjs length problem at one provider, and it does not survive contact with 8 candidate providers (anticipated 3 D + 2 C + 3 B, eight code paths once they all transition from Candidate to Enabled). Worse, it makes provider-disable a code change, which means fast-quarantine in response to a ToS announcement (spec §9) is a release event rather than a config flip.
(b) Full external plugin discovery (npm-installable, runtime-loaded, hot-discoverable). Plugins are npm packages; OLP scans node_modules/@olp-providers/* at startup; users npm install to add a provider. Rejected for v1.0 on three grounds: (1) the provider set is curated for ToS-risk reasons (ADR 0006), and "anyone can install any provider" defeats that curation; (2) the discovery layer is itself non-trivial code (manifest validation, version compatibility, security review of third-party plugin code) that does not earn its complexity at three to eight providers; (3) the contract has not stabilized enough — locking it as a stable plugin API before v1.0 ships is premature commitment.
(c) Per-provider sub-processes / microservices. Each provider runs as a separate Node.js process; server.mjs is a router that proxies to the right sub-process. Rejected as massive over-engineering for v1.0 traffic levels (family-scale, dozens of requests per hour, not hundreds per second). The spawn-per-request cost is already the dominant latency; sub-process IPC adds latency without buying anything until the maintainer is running OLP at a scale where one Node process is the bottleneck — which is not v1.0's problem.
(d) Code generation from a YAML manifest per provider. Each provider is described in YAML; a generator emits the corresponding .mjs. Rejected as a layer that does not pay for itself. The provider plugins are ~150–400 lines of hand-written code each; generating them from YAML would shift complexity from the .mjs to the YAML + generator + the round-trip when a generated file needs to be hand-tuned for a quirk. The generator-first approach also makes the cli.js-alignment equivalent (per-provider CLI behavior alignment per OLP ALIGNMENT.md) harder to enforce, because the source of truth becomes the YAML, not the spawned-binary's real behavior.
Sources
- OLP v0.1 spec §4.2 (Plugin-based provider system, including the v1.0 Provider contract definition)
- OCP ADR 0003 (
models.jsonas SPOT) — informs the "static enumeration, not filesystem scan" loading model - OCP ADR 0005 — the context paragraph references OCP's
server.mjsreaching 1667 lines at one provider; the plugin architecture is the structural response to that complexity scaling N×