taodeng/olp
1
0
Fork 0
mirror of https://github.com/dtzp555-max/olp.git synced 2026-07-21 21:15:10 +00:00
Code Issues Packages Projects Releases Wiki Activity
109 Commits 6 Branches 14 Tags
7019294c63b1b77d15064ff6f0702d95e020cc42
Commit Graph
33 Commits
Author SHA1 Message Date
dtzp555-maxandGitHub 7019294c63 feat(sandbox): Phase 7 Solution 1 implementation + opus 4.8 (#68) 
Implements ADR 0014 Amendment 1 (4-layer Solution 1) + ADR 0002 Amendment 9 (Provider ISOLATION contract) + opus 4.8 model.

Fresh-context opus reviewer APPROVE_WITH_MINOR; 2 nit fold-ins applied. 813 unit tests pass.

Known deferred coverage: Suite 44 PI231 E2E tests are placeholders under describe.skip pending Task #9 (PI231 prod-target validation). The load-bearing negative test ('in-sandbox cat ~/.olp/keys.json MUST fail') will be validated when Task #9 runs against the merged code.

PR-B outer-bwrap superseded; archive at phase-7-pr-b-outer-bwrap-snapshot branch.
 2026-05-29 10:43:53 +10:00
taodengandClaude Opus 4.7 497b2550e6 fix(anthropic): tolerate null exit code when result event was seen (PR-B sandbox) 
The /bin/sh -c wrapping that sandbox-runtime applies (bwrap argv prefix +
inner claude command) can return exit code null on cleanup even when the
underlying claude process completed cleanly and emitted the `result`
event. The previous logic treated any non-zero exit as fatal and threw
ProviderError, causing the HTTP handler to discard the already-yielded
chunks and respond with content:null.

resultEventSeen=true is the authoritative success signal — if it's set,
the model completed and the stop chunk was yielded. Abnormal exit after
that is sandbox bookkeeping noise.

Live PI231 evidence (2026-05-28 commit 2864275 deploy):
  Direct provider test (bypasses HTTP):
    chunk: {"type":"delta","content":"DIRECT_PROOF",...}
    chunk: {"type":"stop","finish_reason":"stop"}
    ERR: claude exit null      ← throw after chunks yielded
  HTTP response: choices[0].message.content == null
                 (chunks lost when consumer received throw)

Fix: only throw on non-zero exit when resultEventSeen=false. With the
guard, the smoke `reply: SANDBOX_PROOF` request now returns the proper
content through the HTTP layer.

The pre-PR-B (non-sandbox) path is unaffected: that path runs claude
directly (no /bin/sh wrap), so exit is always 0 when the model
completes, and the resultEventSeen check is a no-op for that path.

Authority:
- live PI231 2026-05-28 transcript (direct vs HTTP path divergence)
- ADR 0009 Amendment 1 § "NDJSON event handling" — result event is
  the terminal-success indicator
- ADR 0014 § PR-B — sandbox wrap introduces /bin/sh layer

Tests: unchanged at 813 (this is a pure-defensive code path; Suite 44
on PI231 will now exercise the corrected path on E2E run).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-28 17:38:27 +10:00
taodengandClaude Opus 4.7 28642756b5 fix(sandbox): PR-B fold-ins — allow read ~/.claude + skip wrap under test mock 
Live PI231 verification of d0dcd28 surfaced two real-runtime issues:

1. ~/.claude was denyRead, but claude CLI MUST read its own OAuth
   credentials file to authenticate. Result: 100% of anthropic requests
   on sandboxed PI231 failed with "Not logged in · Please run /login".
   Live transcript:
     {"event":"fallback_hop_error", "provider":"anthropic",
      "error":"Not logged in · Please run /login"}

   The cross-tenant risk for ~/.claude was a false security trade-off:
   (a) ~/.claude is the spawn's OWN auth, not cross-tenant material
       (all OLP clients share the same Anthropic OAuth — the file is
       not multi-tenant secret)
   (b) Real protection for the model accidentally exfiltrating creds
       via tool_use is Phase 6c's --system-prompt suppressing tool
       descriptions; sandbox-runtime can only blanket-deny a path, not
       distinguish "claude reads creds" from "model emits Read tool"
   Fix: remove ~/.claude from denyRead. Keep ~/.olp, ~/.ssh, ~/.config,
   ~/.codex (all genuinely cross-tenant).
   Suite 44a (cat ~/.olp/keys.json MUST fail) unchanged — still proves
   the core acceptance.
   Suite 44c added (regression guard): in-sandbox cat ~/.claude/.credentials.json
   MUST succeed when the file exists.

2. anthropic.mjs::_spawnAndStream called wrapSpawn() unconditionally,
   including when a test had set __setSpawnImpl(mockFn). The wrap
   rewrites bin to '/bin/sh -c <wrapped-string>' which breaks every HTTP
   integration test that asserts on spawn args. Result on PI231 (sandbox
   active): 16 production-shape tests failed because their mocks were
   never called with the expected bin/args.
   Fix: skip wrapSpawn when spawnImpl !== defaultSpawn (mock active).
   Mocks don't exec, so isolation is meaningless there anyway. Real-CLI
   spawns (production + Suite 44) still get wrapped.

Tests: 813 → 813 (44c is also PI231-gated, skips on Mac with file
absent; the fold-in does not change Mac test count).

Authority:
- @anthropic-ai/sandbox-runtime v0.0.52 — wrapWithSandbox() shell-string
  return shape is the proximate cause of (2)
- Live PI231 2026-05-28 transcript: /v1/chat/completions returning
  content=null + "Not logged in" from anthropic; OLP_E2E_SANDBOX=1 npm
  test showing 16 prior pass tests now fail
- ADR 0014 § 4.1 PR-B acceptance criteria — the negative test (44a)
  continues to pass; the false-positive denyRead (~/.claude) is corrected

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-28 17:34:36 +10:00
taodengandClaude Sonnet 4.6 d0dcd281ef feat(sandbox): Phase 7 PR-B — anthropic.mjs spawn wrapped in sandbox-runtime 
Wraps the claude CLI spawn in @anthropic-ai/sandbox-runtime per ADR 0014
§ PR-B. Achieves multi-tenant filesystem + network isolation: the spawned
claude subprocess can no longer read ~/.olp/keys.json, ~/.claude/.credentials.json,
~/.ssh/, or any other per-client OAuth material. Only api.anthropic.com and
statsig.anthropic.com are reachable; only /tmp/olp-spawn/<uuid>/ is writable
per spawn (ephemeral, UUID-scoped to prevent cross-request contamination).

## Authority citations

- @anthropic-ai/sandbox-runtime v0.0.52
  https://github.com/anthropic-experimental/sandbox-runtime
  dist/sandbox/sandbox-manager.js — SandboxManager.initialize(), wrapWithSandbox()
  dist/sandbox/sandbox-utils.js  — getDefaultWritePaths()
- 2026-05-28 spike report at /tmp/sandbox-spike/report.md on PI231:
  spike-anthropic.mjs (wrapWithSandbox call signature + NDJSON proof),
  spike-deny.mjs (cat ~/.olp/keys.json MUST fail)
- OLP ADR 0014 § 2.1 PR-B, § 4.1 PR-B acceptance criteria
- OLP ADR 0009 Amendment 1 § Caveats #3 (sandbox is cloud prerequisite)
- OLP ALIGNMENT.md Rule 1 — provider plugin authority citation
- cc-mem incident 2026-05-27 § 3 (multi-tenant OAuth token exposure via
  prompt injection)

## Files changed

- lib/sandbox/manager.mjs (new): bootstrap + spawn-wrap layer.
  Exports: bootstrapSandbox(), isSandboxActive(), wrapSpawn(),
  __resetSandboxManagerForTests(). Config-at-boot model (one
  SandboxManager.initialize() at server start; per-request wrapWithSandbox()
  reads from already-initialized state). Transparent pass-through when inactive.

- lib/providers/anthropic.mjs: spawn site wrapped via wrapSpawn({
  bin, args, env, allowedDomains: ['api.anthropic.com','statsig.anthropic.com']
  }). ADR 0009 Amendment 1 spawn args unchanged — only execution is wrapped.

- server.mjs: bootstrapSandbox() called before server.listen(); startup banner
  logs sandbox.active state. /health.sandbox now includes active:boolean field
  (distinguishes "deps present" from "SandboxManager initialized and wrapping").

- test-features.mjs: Suite 43 (8 tests — manager unit: bootstrap state, idempotency,
  isSandboxActive, wrapSpawn passthrough, /health.active field) + Suite 44
  (2 PI231-gated tests: 44a security negative test + 44b positive echo test,
  skipped by default, run with OLP_E2E_SANDBOX=1 npm test).

- CHANGELOG.md, docs/adr/0014-sandbox-runtime-integration.md: PR-B status
  updated; ADR table + /health JSON example updated with active field.

## Test count

805 (pre-PR-B) → 813 (+8 Suite 43; Suite 44 skipped on macOS, runs on PI231)
All 813 pass on macOS dev machine. 0 regressions.

## PI231 validation (required before merge — Suite 44)

After apt-get install bubblewrap socat (ripgrep already present) + server restart:

1. curl /health → confirm sandbox.available=true AND sandbox.active=true
2. Real anthropic request → confirm stream-json still works end-to-end
3. OLP_E2E_SANDBOX=1 npm test → confirm Suite 44a (cat ~/.olp/keys.json MUST fail)
   and Suite 44b (echo SANDBOX_PROOF succeeds)

Reviewer: must SSH PI231, run Suite 44, and confirm the ADR 0014 § 4.1 criteria.
This commit is ready to push; do NOT push before Suite 44 transcript is captured.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-28 17:26:34 +10:00
taodengandClaude Opus 4.7 e5cfc696da fix(anthropic): suppress stream-json log spam for system/* and user events 
The NDJSON event parser matched only `system/init` and treated every
other system subtype as "unknown event type" — logged via console.error,
once per event. During the 2026-05-28 OAuth-expiry 401 cascade this
produced 2 spam lines per failed request (PI231 server log was
~50% noise).

Also caught: claude echoes {type:'user'} events back in some stream-json
modes (analogous to --replay-user-messages); these were also spammed
as unknown.

Both are now consumed silently — neither maps to an IR chunk.

Authority:
- empirical PI231 v2.1.104 transcripts 2026-05-28 (server log shows
  '[anthropic] unknown stream_json event type: system' and
  '[anthropic] unknown stream_json event type: user')
- ADR 0009 Amendment 1 § "NDJSON event handling" — generic "future-proof
  for unknown events" intent; the table was incomplete

Tests: 795 → 797
- 41e-5b: regression guard for system/<non-init-subtype>
- 41e-5c: user (echo) event consumption

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-28 16:56:52 +10:00
taodengandClaude Opus 4.7 dbac5f5521 fix(anthropic): stop injecting env.CLAUDE_CODE_OAUTH_TOKEN — let CLI auto-refresh 
OLP was injecting accessToken from ~/.claude/.credentials.json into the
spawn env as CLAUDE_CODE_OAUTH_TOKEN unconditionally. This defeated
claude CLI's built-in OAuth refresh: when the env var is present, the
CLI reads it directly and never touches credentials.json, so expired
accessTokens were never swapped using the refreshToken sitting right
there in the file. Result: hard 401 cascade every ~8h (access token
TTL) requiring manual `security find-generic-password → scp → PI231`
cycle.

Empirical 2026-05-28 spike on PI231 v2.1.104:
- expiresAt = 1 min ago (simulated expiry)
- spawn claude -p WITHOUT CLAUDE_CODE_OAUTH_TOKEN env
- response succeeded, AND credentials.json.expiresAt advanced to
  ~8h in the future
- token refresh handled internally by claude CLI

Fix: remove the env injection in _spawnAndStream. buildSpawnEnv still
forwards process.env (minus ANTHROPIC_*), so if the operator explicitly
sets CLAUDE_CODE_OAUTH_TOKEN at OLP boot, it's still honored — for
users on ephemeral CI / no-file-creds setups. The readAuthArtifact()
call remains as a preflight check (verify creds exist before spawn)
but the read value is no longer forwarded.

Tests: 793 → 795
- 41h: regression guard — no CLAUDE_CODE_OAUTH_TOKEN in env when only
  file/keychain auth is available
- 41h-2: operator-set process.env passthrough still works

User-facing impact: bot-down-every-8-hours issue resolved. Hermes +
OpenClaw clients no longer require periodic keychain → PI231 copy.

Authority:
- empirical PI231 v2.1.104 spike 2026-05-28 (no public CLI doc cites
  refresh behavior; verified by simulating expiry + observing file
  update post-spawn)
- ADR 0009 Amendment 1 § "Caveats" — implementation must remain robust
  to OAuth flow changes (this fix removes a hard-coded assumption that
  was actively harmful)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-28 16:52:43 +10:00
taodengandClaude Opus 4.7 65f945c16d fix(anthropic): assistant-aggregate fallback when claude emits no content_block_delta 
Live PI231 v2.1.104 verification of 97e7d16 surfaced a real-runtime bug:
in stream-json mode WITHOUT -p (the Transport A path locked by ADR 0009
Amendment 1), claude emits ONLY the aggregate {type:"assistant"} event
for fast/short responses — NO content_block_delta events. The previous
implementation returned null for `assistant` unconditionally, so the
delta-chunk stream was empty and OpenAI-format responses came back with
content=null.

Empirical evidence (PI231 2026-05-27):
- Manual `claude --output-format stream-json --verbose --no-session-persistence
  --system-prompt "..."` < "reply: DIAG-PROOF" → NDJSON stream contains
  system/init + assistant(content:[{text:"DIAG-PROOF"}]) + rate_limit_event
  + result. NO content_block_delta events.
- OLP server response: choices[0].message.content == null
- Provider direct test: chunks = [{type:"stop"}], no delta yielded

Fix:
- anthropicStreamJsonEventToIR(event, isFirstDelta) now handles assistant:
  - isFirstDelta=true  → extract aggregate text from message.content, yield
    as single delta chunk (this is the "no streaming" path)
  - isFirstDelta=false → return null (duplicate of already-streamed deltas)
- Non-text content blocks (thinking) are filtered out
- Multi text blocks concatenate in order

4 new tests in Suite 41 covering: aggregate-when-streamed-null, aggregate-as-
fallback-delta, multi-block-concat, thinking-block-filter.

Tests: 790 → 793 (all pass).

This was a brief-implementation gap: ADR 0009 Amendment 1 § "NDJSON event
handling" mentioned "If we somehow get assistant without prior
content_block_delta events (no streaming), then yield content as single
delta" as a robustness fallback, but the original implementation treated
the case as unreachable. Live verification proved it's the COMMON case
when --include-partial-messages is not set (which we deliberately omit
per the ADR's locked flag set).

Authority:
- claude CLI v2.1.104 § --output-format stream-json (NDJSON shape)
- claude CLI v2.1.104 § --include-partial-messages (controls whether
  content_block_delta events are emitted; we omit it per ADR 0009
  Amendment 1's locked flag set, accepting aggregate-only output)
- ADR 0009 Amendment 1 § "NDJSON event handling" — `assistant` row already
  documents the fallback intent; this commit aligns implementation with intent
- OLP ALIGNMENT.md Rule 1 — provider plugin authority citation

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-27 22:38:19 +10:00
taodengandClaude Opus 4.7 97e7d16585 feat(anthropic): stream-json + --system-prompt (ADR 0009 Amendment 1) 
Replaces claude -p --output-format text spawn with claude (no -p)
--output-format stream-json --verbose --system-prompt.

Authority:
- claude CLI v2.1.104 § --output-format stream-json (verified 2026-05-27
  empirical test on PI231: NDJSON event stream emits without -p)
- claude CLI v2.1.104 § --verbose (required companion to stream-json)
- claude CLI v2.1.104 § --system-prompt (full default-prompt replacement,
  suppresses env-block + tool descriptions)
- claude CLI v2.1.104 § --no-session-persistence
- OLP ADR 0009 Amendment 1 — decision lock + value re-anchoring
- OLP ALIGNMENT.md Rule 1 — provider plugin authority citation

Four orthogonal values delivered:
1. Hallucination fix: model no longer claims server-side cwd / OS /
   tools (verified bot self-check produces "I don't have local env"
   instead of "/home/tlab/olp")
2. ~64% per-request cost reduction: empirical Sonnet 4.6
   $0.0216 → $0.0078, from ~30% input-token reduction (16,601 → 10,700)
3. NDJSON observability: rate_limit_event + usage + cache stats per
   request now available (future audit/dashboard work)
4. Possible 30-60 day bridge for Anthropic 2026-06-15 billing split
   (uncertain per P0 spike — third-party-app classification clause)

Per ADR 0009 Amendment 1 § "Caveats": this is NOT a substitute for
Phase 7 @anthropic-ai/sandbox-runtime work; sandbox remains required
for any cloud / multi-tenant deployment.

Cache key composition unchanged (ADR 0005 IR-based hash; on-the-wire
format is internal to the provider plugin).

Tests: 771 → 790 (+19 new in Suite 41; 9 existing mocks updated to
emit valid NDJSON stream_event format instead of raw text).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-27 22:29:17 +10:00
dtzp555-maxGitHubtaodengClaude Sonnet 4.6
 		bddf2cba1e release(v0.5.1): hotfix — quota probe cache/backoff/schema-drift correctness (codex review) (#58) 
* release(v0.5.1): hotfix — quota probe cache/backoff/schema-drift correctness (codex review)

Addresses three production-quality findings from codex's post-v0.5.0 review (codex review on
PR #57, reproduced with local mocks):

F1 [P1] — Doctor bypass of cache + backoff (ADR 0013 Rule 3)
  `anthropic.quota_probe_reachable` called `_probeOnce(auth)` directly, bypassing
  `quotaProbeState.backoffUntil`. Successive `olp doctor` invocations within a backoff
  window each hit upstream — violating ADR 0013 Rule 3 (backoff is mandatory for ALL
  consumers). Fix: doctor now routes through `quotaStatus()`. ADR 0013 Rule 3 clarified:
  "All consumers of `quotaStatus()`, including `olp doctor` checks, MUST route through
  `quotaStatus()` and MUST NOT call `_probeOnce()` directly."

F2 [P2] — 200 with empty ratelimit headers cached as live data (ADR 0013 Rule 5)
  A 200 OK with zero `anthropic-ratelimit-*` headers was cached as `stale: false` (live).
  Minimum-viable-schema gate added to `_probeOnce`: requires 5h-utilization + 5h-reset +
  7d-utilization + 7d-reset present; absence → `failureKind: 'schema_drift'`, backoff
  scheduled, result NOT cached. ADR 0013 Rule 5 updated with the gate specification.

F3 [P2] — Dashboard-data loses failure detail (ADR 0013 Rule 6)
  `aggregateProviderQuota()` collapsed all failure modes into `status: 'unavailable'`
  ("no public quota api or probe disabled") — same as providers with no API at all.
  Fix: `quotaStatus()` v0.5.1 contract — `null` ONLY for opt-in-off; failures return
  `{ probe_status: 'unreachable', failure: { kind, message, backoff_until? } }`.
  New `failure_kind` enum: no_credentials | auth_failed | rate_limited | schema_drift |
  network | other. Dashboard renders `unreachable` with red border + failure detail.

Authority:
  ADR 0013 Rules 3, 5, 6 (cache + backoff + schema-drift + failure transparency)
  ADR 0008 Amendment 2 (richer quota_v2 shape; new unreachable status)
  ADR 0002 Amendment 8 unchanged (constitutional permission for the probe)
  Codex review findings F1–F3 (codex on PR #57)

Changes:
  - lib/providers/anthropic.mjs: quotaProbeState gains lastError + failureKind;
    _probeOnce: min-field gate + failureKind population; quotaStatus(): v0.5.1 contract
    (null=disabled only; probe_status:live/stale/unreachable); doctorChecks routes
    through quotaStatus(); reset functions updated
  - lib/audit-query.mjs: _normalizeAnthropicQuota handles probe_status field;
    aggregateProviderQuota emits failure/failure_kind/backoff_until; unreachable status
  - dashboard.html: unreachable CSS classes + render path + footer v0.5.1
  - test-features.mjs: 38f/j/l updated for v0.5.1 shape; 38r refactored for F1;
    38g/k gain probe_status assertions; 38u/v/w new regression tests; 756→759 tests
  - docs/adr/0008: Amendment 2 (richer ProviderQuotaEntry + quotaStatus contract)
  - docs/adr/0013: Rule 3 clarification (doctor must use quotaStatus);
    Rule 5 min-viable-schema gate specification
  - package.json: 0.5.0 → 0.5.1
  - CHANGELOG.md: v0.5.1 hotfix entry promoted from Unreleased
  - README.md / AGENTS.md: Phase 5 closed at v0.5.1; Phase 6 next

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs+code: PR #58 fold-in — reviewer Nits #1 + #2 (ADR doc-drift + opt_in_off enum)

Fresh-context reviewer (PR #58) verdict: APPROVE_WITH_MINOR, 0 blocking,
4 nits. Folding in #1 + #2 (both 1-line cosmetic-but-correct fixes).
Deferring #3 (test-only edge case in module-level seam restore) and #4
(pre-existing codex F4 — bin/olp.mjs + olp-plugin still read legacy
quota shape; separate PR planned).

Nit #1 — ADR 0002 Amendment 8 documentation drift.

Amendment 8 at v0.5.0 line 20 said "the function returns `null` rather
than throwing", and line 33 said "stale-cache-on-failure (`null` is
returned only when no cache entry exists; if a stale entry exists it's
returned with a `stale: true` marker)". v0.5.1 refined this contract:
`null` is now reserved STRICTLY for opt-in-off, and all failure modes
return `{ probe_status: 'unreachable' | 'stale', failure: {...} }`.

The substantive idempotent-failure constraint (no throw to caller) is
unchanged. The operational description in Amendment 8 was stale — fixed
to cross-reference ADR 0008 Amendment 2 + ADR 0013 Rule 6 for the
v0.5.1 contract refinement. Also references Suite 38 (38u/38v/38w) as
the regression coverage producing the new shape.

Nit #2 — `failureKind: 'opt_in_off'` declared but never produced.

The enum value was listed in both the code comment (anthropic.mjs:250)
and ADR 0008 Amendment 2 (line 30) but never actually assigned —
because when opt-in is off, `quotaStatus()` returns the literal `null`
BEFORE any state mutation happens. The enum value was dead.

Fix: removed `opt_in_off` from both enum declarations + added an
inline note explaining that consumers (audit-query, doctor) distinguish
opt-in-off by checking `quotaStatus() === null`, not via failureKind.

Deferred:

- Nit #3 (test-seam restore edge case): if a caller pre-sets
  `_quotaAuthReadFnForTest` AND passes a non-default `_authReadFn`,
  the finally block restores to null clobbering pre-set value.
  Test-only impact, no production risk. Pure hygiene; defer.

- Nit #4 (codex F4): bin/olp.mjs cmdUsage and olp-plugin/index.js
  still read legacy `body.quota` field, never consume `quota_v2`.
  Reviewer confirmed neither crashes — both gracefully fall through
  to "no quota api" branch. Out of scope for this hotfix per the
  hotfix dispatch contract; separate PR will migrate them.

Tests: 759/759 still pass post-fold-in. No test changes needed.

Authority: PR #58 review thread + ADR 0013 Rule 6 (failure transparency)
+ ADR 0008 Amendment 2 (ProviderQuotaEntry v0.5.1 shape).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-27 09:33:47 +10:00
dtzp555-maxGitHubtaodengClaude Sonnet 4.6
 		2b07a3bd1b test: D83 — Suite 38 (quota probe) + Suite 39 (dashboard smoke) (Phase 5) (#55) 
* test: D83 — Suite 38 (quota probe) + Suite 39 (dashboard smoke) (Phase 5)

ADR 0012 D83 row: comprehensive test coverage for the Phase 5 quota probe
machinery (D80) and dashboard rendering (D82).

## Authorities cited

- ADR 0012 D83 (D-day specification)
- ADR 0013 Rules 2–6 (quota probe constraints being tested)
- D80 PR #52 (anthropic.quotaStatus() + _probeOnce + _parseRateLimitHeaders)
- D81 PR #53 (aggregateProviderQuota quota_v2 shape)
- D82 PR #54 (dashboard.html Claude.ai-style restructure)
- Schema pin: ~/.cc-rules/memory/learnings/anthropic_plan_usage_probe_schema_2026_05_26.md

## Test seams added to lib/providers/anthropic.mjs

Minimal test seams added — 0 production-logic changes:
- `_setQuotaUrlsForTest(apiUrl, oauthUrl)` — redirects probe HTTP to local
  mock server (auto-detects http: vs https: to switch transport module)
- `_resetQuotaProbeStateForTest()` — resets cache + backoff + URLs + auth seam
- `_resetQuotaStateOnlyForTest()` — resets cache + backoff only (URLs stay)
- `_getQuotaProbeStateForTest()` — returns direct reference to quotaProbeState
  for test assertions and controlled state mutation
- `_setQuotaAuthReadFnForTest(fn)` — injects a mock auth reader into quotaStatus()
  so tests are not affected by real ~/.claude/.credentials.json on the machine
  (fixes 38f: no-auth test was finding real keychain credentials)

## Suite 38 — 20 quota-probe unit tests (38a–38t)

38a: all 13 ratelimit-unified-* headers parsed correctly (numeric types, null defaults)
38b: missing overage-reset → overage_reset: null
38c: missing all three overage fields → all three null
38d: new 5h-status + 7d-status fields (NEW vs OCP 2026-04) parsed correctly
38e: quota_probe_enabled: false → null without HTTP call
38f: auth returns null → quotaStatus returns null without HTTP call
38g: 200 + all 13 headers → full shape with stale:false + backoff reset
38h: cache hit within 5min TTL → no second HTTP call (request count stays 1)
38i: expired cache (6min > 5min TTL) → fires fresh HTTP probe
38j: 401 with no refreshToken → null (idempotent-failure per ADR 0002 Amendment 8 §3)
38k: 429 + stale cache → returns stale cache with stale:true + last_fresh_at
38l: 429 + no cache → null + backoff scheduled (backoffUntil in the future)
38m: exponential backoff growth: 60s→120s→240s→cap at 3600s
38n: successful probe resets backoffMs to 60s + backoffUntil to 0
38o: schemaVersion from models-registry.json (falls back to constant)
38p: doctor quota_probe_reachable disabled → ok with opt-in advisory
38q: doctor probe enabled + probe succeeds → ok with utilization in message
38r: doctor probe enabled + fails + stale cache → warn
38s: doctor probe enabled + fails + no cache → fail with fix_commands
38t: doctor probe enabled + no creds → fail with human_steps only

## Suite 39 — 8 dashboard rendering smoke tests (39a–39h)

39a: /dashboard with owner token → 200 + text/html
39b: /dashboard without token → 401
39c: /dashboard with guest key → 401 (owner-only_block enforcement per ADR 0008 §8)
39d: HTML contains "Plan Usage" header (D82 panel)
39e: HTML contains ↻ Refresh button (D82 manual refresh)
39f: HTML contains QUOTA_POLL_INTERVAL_MS = 60000 (D82 1-min refresh)
39g: HTML contains visibilitychange / visibilityState guard (D82 ADR 0012)
39h: HTML contains quota_v2 consumer + legacy renderQuota fallback code

## Test count: 727 → 755 (+28)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test: D83 fold-in — fix 38j misleading name + add 38j2 positive-path + fix 38o ESM require

Fresh-context reviewer (PR #55) flagged one blocking issue + 1 nit:

BLOCKING (Q5) — 38j misleading name + uncovered 401→refresh→retry positive path.
The original 38j test description said "401 → refresh succeeds → retry probe
succeeds" but the test actually asserted the OPPOSITE behavior (401 with no
refreshToken → null, no retry). The most important non-trivial control-flow
branch in _probeOnce (anthropic.mjs:451-457 the refresh-and-retry on 401) was
uncovered, and the misleading name actively masked the gap.

Fix:
- Renamed 38j to accurately describe what it tests: "401 with no refreshToken
  → null (idempotent-failure, no refresh attempt)". The test pins valuable
  behavior — idempotent-failure when refreshToken is absent — but now its
  name matches.
- Added 38j2 (NEW) to exercise the actual positive-path: inject creds with
  refreshToken via _setQuotaAuthReadFnForTest, mock returns 401 on first API
  call + 200 with 13 headers on retry. Asserts: 2 API calls, 1 OAuth call,
  retry parsed full shape, OAuth body contains the injected refreshToken
  (Rule 1 credential reuse verification).

NIT (Q5 dead assertion) — 38o tried to read models-registry.json via
require('fs') which is undefined in ESM. The assert was silently never
exercised. Replaced with the already-imported readFileSync from node:fs
(added _readFileSync38 import). Also tightened: schemaVersion MUST equal
the registry's quota_probe.schema_version (no longer conditional on
"if expected" which was always falsy).

Other reviewer nits deferred (non-blocking per reviewer):
- N2: seam naming convention (_ vs __) — defer
- N3: commit message "zero production changes" — note for v0.5.0 close
- N4: _getQuotaProbeStateForTest returns mutable ref — defer
- N5: test seam runtime guard — defer
- N6: coverage gaps (concurrent probes, registry-missing fallback,
       network error path) — v1.x roadmap follow-ups
- N7: Suite 39 stray error listener — defer (benign)

Test count: 755 → 756 (+1 net, +2 new minus existing 38j rename).
All 756 pass; 0 fail. Local re-run: stable.

Authority: PR #55 review thread (D83 fresh-context opus reviewer)
+ ADR 0013 Rule 1 (credential reuse via refresh).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-26 17:47:15 +10:00
dtzp555-maxGitHubtaodengClaude Sonnet 4.6
 		5288493f19 feat: D81 — dashboard-data quota_v2 shape + models-registry schema_version (Phase 5) (#53) 
Authority citations (required per CLAUDE.md § Hard requirements):
  1. ADR 0012 D81 — the D-day being implemented (Phase 5 charter, audit-query
     + dashboard-data extension row in § D-day table)
  2. ADR 0013 Rule 5 — schema_version in models-registry.json mandate. D80
     used a local constant QUOTA_SCHEMA_VERSION = '2026-05-26' (reviewer nit
     #4 at PR #52). D81 folds in Rule 5 compliance: adds quota_probe.schema_version
     to models-registry.json and has anthropic.mjs read from there with the
     constant as fallback via _resolveSchemaVersion().
  3. ADR 0008 — the audit-query design being amended (Amendment 1 added at D81
     to docs/adr/0008-dashboard-and-audit-query.md documenting: quota_probe in
     registry, aggregateProviderQuota() API shape, quota_v2 key, deprecation
     timeline for legacy quota key).
  4. D80 PR #52 (commit 82d2e1c) — producer of the quotaStatus() shape that
     D81 normalizes. The 13-field anthropic-ratelimit-unified-* shape from
     _parseRateLimitHeaders() is consumed by _normalizeAnthropicQuota() here.

Changes (A–G per D81 spec):

A. models-registry.json — new top-level quota_probe key
   - quota_probe.schema_version = '2026-05-26'
   - quota_probe.anthropic.{ source, endpoint, fields_pinned[13] }
   - fields_pinned is load-bearing for drift detection (ADR 0013 Rule 5)

B. lib/providers/anthropic.mjs — _resolveSchemaVersion() helper
   - Reads quota_probe.schema_version from modelsRegistryRaw at call time
   - Falls back to QUOTA_SCHEMA_VERSION constant if registry field absent
   - quotaStatus() now uses _resolveSchemaVersion() instead of raw constant

C. lib/audit-query.mjs — aggregateProviderQuota() export
   - Normalizes per-provider quotaStatus() returns to ProviderQuotaEntry shape
   - Live → { status:'live', utilization, reset, representative_claim, … }
   - Stale → { status:'stale', … }
   - null return → { status:'unavailable', reason:'no public quota api or probe disabled' }
   - throw → { status:'unavailable', reason:<error.message> }
   - getQuotaStatus injection point for full test isolation (D81 §F)

D. server.mjs handleManagementDashboardData — quota_v2 added
   - Calls auditAggregateProviderQuota({ providers: loadedProviders })
   - Both legacy quota (backwards compat) and quota_v2 in response
   - Graceful degradation: quota_v2 failure → warn log + empty array, rest of
     payload unaffected

E. server.mjs handleManagementQuota — quota_v2 added (mirrors D)

F. test-features.mjs — Suite 37 (7 new tests)
   - 37a: live shape normalization (status=live, utilization, reset, etc.)
   - 37b: stale shape (status=stale)
   - 37c: null returns → unavailable entries
   - 37d: throw path → unavailable with reason
   - 37e: mixed providers (live + unavailable)
   - 37f: getQuotaStatus injection verified
   - 37g: models-registry.json has quota_probe.schema_version + 13 fields_pinned

G. docs/adr/0008-dashboard-and-audit-query.md — Amendment 1 added

Test delta: 720 → 727 (+7), 0 failures.

Live quota_v2 JSON sample (illustrative — probe is opt-in per ADR 0013 Rule 4;
real values require quota_probe_enabled:true + valid OAuth credentials):

  GET /v0/management/dashboard-data (owner-only)
  {
    "quota_v2": [
      {
        "provider": "anthropic",
        "status": "live",
        "schema_version": "2026-05-26",
        "last_fresh_at": 1748300000000,
        "utilization": { "5h": 0.49, "7d": 0.31 },
        "reset": { "5h": 1748290000, "7d": 1748500000, "overall": 1748300000, "overage": null },
        "representative_claim": "five_hour",
        "fallback_percentage": 0.5,
        "overage": { "status": "rejected", "disabled_reason": "org_level_disabled_until" },
        "raw_available": true
      },
      {
        "provider": "openai",
        "status": "unavailable",
        "reason": "no public quota api or probe disabled",
        "schema_version": null,
        "last_fresh_at": null,
        "utilization": null,
        "reset": null,
        "representative_claim": null,
        "fallback_percentage": null,
        "overage": null,
        "raw_available": false
      }
    ]
  }

When probe is disabled (default), anthropic entry also returns unavailable:
  { "provider": "anthropic", "status": "unavailable",
    "reason": "no public quota api or probe disabled", ... }

NOT modified: dashboard.html (D82), codex.mjs, mistral.mjs, any quotaStatus()
return shape from anthropic.mjs (D80 contract unchanged — normalization is in
the new audit-query layer only).

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-26 17:06:41 +10:00
dtzp555-maxGitHubtaodengClaude Sonnet 4.6
 		82d2e1cbea feat: D80 — anthropic plan-usage probe port (Phase 5) (#52) 
Port OCP server.mjs:842-1109 plan-usage probe to lib/providers/anthropic.mjs:quotaStatus().

## Authority citations (CLAUDE.md hard requirement #1)

1. Schema pin: ~/.cc-rules/memory/learnings/anthropic_plan_usage_probe_schema_2026_05_26.md
   — 13-field canonical schema verified live 2026-05-26; 3 fields new vs OCP
   2026-04 capture (5h-status, 7d-status, overage-reset).

2. OCP port source: OCP server.mjs:842-1109 — usageCache, oauthRefreshBackoff,
   getOAuthCredentials, refreshOAuthToken, fetchUsageFromApi, parseRateLimitHeaders.
   Claude Code CLI uses this same POST /v1/messages internally (verified 2026-05-26
   by `strings` on @anthropic-ai/claude-code v2.1.142 / v2.1.150 Mach-O binary — see
   audit memory). This is observed CLI behaviour, not invention.

3. ADR 0002 Amendment 8 — READ-ONLY exemption for quotaStatus() direct-API access.
   Three constraints satisfied: READ-ONLY (max_tokens:1, body discarded),
   subscription-scope (same readAuthArtifact() creds as spawn path),
   idempotent-failure (returns null / stale on any error, never throws).

4. ADR 0013 — OAuth READ-ONLY consumption rules. All 7 rules satisfied:
   Rule 1: credential reuse via readAuthArtifact() (env → .credentials.json → keychain).
   Rule 2: only POST /v1/messages (no other endpoints). Body discarded; headers-only.
   Rule 3: 5min TTL cache; 60s–3600s exponential backoff; stale-on-failure.
   Rule 4: opt-in via ~/.olp/config.json providers.anthropic.quota_probe_enabled (default false).
   Rule 5: schema pin committed to memory file; drift detection protocol in place.
   Rule 6: doctor check anthropic.quota_probe_reachable surfaces probe status.
   Rule 7: does not govern spawn-path refresh (separate concern).

5. ADR 0012 D80 — Phase 5 charter: this commit is the D80 deliverable.

## Live probe transcript (2026-05-26 from MacBook keychain OAuth credentials)

Path B verification per ADR 0013 Rule 5:
  curl -s -i -m 20 -X POST https://api.anthropic.com/v1/messages \
    -H "Authorization: Bearer <token>" \
    -H "anthropic-beta: oauth-2025-04-20" \
    -H "anthropic-version: 2023-06-01" \
    -H "Content-Type: application/json" \
    -d '{"model":"claude-haiku-4-5-20251001","max_tokens":1,"messages":[{"role":"user","content":"."}]}'

Response (header lines only):
  HTTP/2 200
  anthropic-ratelimit-unified-status: allowed
  anthropic-ratelimit-unified-5h-status: allowed
  anthropic-ratelimit-unified-5h-reset: 1779794400
  anthropic-ratelimit-unified-5h-utilization: 0.09
  anthropic-ratelimit-unified-7d-status: allowed
  anthropic-ratelimit-unified-7d-reset: 1780225200
  anthropic-ratelimit-unified-7d-utilization: 0.32
  anthropic-ratelimit-unified-representative-claim: five_hour
  anthropic-ratelimit-unified-fallback-percentage: 0.5
  anthropic-ratelimit-unified-reset: 1779794400
  anthropic-ratelimit-unified-overage-disabled-reason: org_level_disabled_until
  anthropic-ratelimit-unified-overage-status: rejected
  (no anthropic-ratelimit-unified-overage-reset — expected: only present on active overage)

12/13 fields present. overage-reset absent = no active overage (expected per audit memory).
All fields parsed correctly by _parseRateLimitHeaders(). Confirmed via D80 smoke test.

## Implementation

A. quotaStatus() — full probe implementation replacing D4 null stub:
   - _readProviderConfig('anthropic') gate (Rule 4 opt-in)
   - 5min module-level cache check (quotaProbeState.cache)
   - 60s–3600s exponential backoff check (quotaProbeState.backoffUntil / backoffMs)
   - readAuthArtifact() credential read (env → .credentials.json → macOS keychain)
   - _probeOnce() → POST /v1/messages with 4 required headers; body discarded
   - 401/403 → single refresh-and-retry via _refreshAccessToken()
   - On success: cache { fetchedAt, data } + reset backoff to MIN
   - On failure: _scheduleBackoff() (doubles backoffMs, caps at MAX) + return stale or null
   - Return shape: { probedAt, source, schemaVersion, stale, fields:{...13}, raw:{...} }

B. _parseRateLimitHeaders() — all 13 fields (3 new vs OCP):
   - status, representative_claim, reset, fallback_percentage (aggregate)
   - status_5h, utilization_5h, reset_5h (5h window)
   - status_7d, utilization_7d, reset_7d (7d window)
   - overage_status, overage_disabled_reason, overage_reset (overage)
   - Numeric strings → numbers; missing fields → null (not 0 or "unknown")

C. _refreshAccessToken() — uses Node.js built-in https (no fetch/3rd-party deps).
   Shared backoff state via quotaProbeState. Max one refresh per backoff window.

D. _probeOnce() — uses Node.js built-in https. 15s timeout. Drains + discards body.

E. _readProviderConfig() — reads ~/.olp/config.json providers.<name> block.
   OLP_HOME respected (same as lib/keys.mjs). Never throws; returns {} on error.

F. doctorChecks() — new anthropic.quota_probe_reachable check (ADR 0013 Rule 6):
   - status: ok when probe disabled (returns advisory message)
   - status: ok when probe succeeds (shows utilization %)
   - status: warn when stale cache exists (probe failed but cache present)
   - status: fail when no cache + probe failed (fix_commands + human_steps recipe)

G. docs/v1x-roadmap.md — #8 Dashboard enrichment entry (D79 follow-up) added.

## Tests

- All 720 existing tests pass (npm test).
- Suite 33j updated to include anthropic.quota_probe_reachable in the expected
  probe set (3 probes total, previously 2).
- D83 (Suite 38) will add quota-probe unit tests with mock HTTP server.

## What NOT changed

- dashboard.html — untouched (D82)
- lib/audit-query.mjs — untouched (D81)
- lib/providers/codex.mjs, mistral.mjs — untouched (D84 NO-GO per ADR 0012 Amendment 1)

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-26 16:51:01 +10:00
dtzp555-maxGitHubtaodengClaude Opus 4.7
 		6edf6e0b94 fix+release(v0.4.2): D75 — codex CLI v0.133.0 schema + per-hop model override (#47) 
Patch release fixing 5 bugs caught by real-machine E2E testing on PI231 +
Mac mini (2026-05-26 session). Prior D-day reviewers + the post-v0.4.0
maintainer review all missed these because they reviewed against spec text
and against the local OLP install's cached codex CLI shape, not against a
fresh `npm install -g @openai/codex` on a remote operator host getting
v0.133.0 for the first time.

F1 — codex auth.json schema pin (lib/providers/codex.mjs readAuthArtifact)
  Real codex CLI v0.133.0 nests the access token under `tokens.access_token`,
  not at top-level access_token / token / accessToken. Pre-D75 readAuthArtifact
  returned null → OLP reported "auth artifact missing" even for fully
  logged-in users. Fix: prepend creds?.tokens?.access_token to the precedence
  chain at both override + default branches. Legacy fields preserved as
  fallback. Authority: codex CLI v0.133.0 on-disk auth.json shape verified
  empirically on PI231 2026-05-26 E2E session.

F2 — codex spawn args + --skip-git-repo-check (lib/providers/codex.mjs irToCodex)
  codex CLI v0.133.0 trusted-directory sandbox refuses with "Not inside a
  trusted directory" outside git repos. OLP deploys typically outside a git
  repo. Fix: add '--skip-git-repo-check' to args before '--model'. Authority:
  codex CLI v0.133.0 reference (`codex exec --help` documents the flag).

F3 — codex NDJSON event shape pin (lib/providers/codex.mjs codexChunkToIR)
  Real v0.133.0 stream: thread.started → turn.started → item.completed
  (item.type='agent_message', item.text=<response>) → turn.completed.
  D6 defensive parser only recognised top-level content/delta/text +
  type:'stop'/done:true → every chunk silently dropped → response body had
  content: null. Fix: add three new recognisers (item.completed → delta;
  turn.completed → stop; turn.failed → error) before the legacy fallback
  chain. Legacy recognisers preserved for backward/forward compat.

F4 — `olp status` reads body.stats.cache.size, not body.cache.entries
  (bin/olp.mjs cmdStatus). Server payload nests stats under stats.cache;
  CacheStore.stats() exposes {hits, misses, size, inflightCount} — there is
  no `entries` field. D74 P2-3 fixed cmdUsage + cmdCache for the same bug
  class but missed cmdStatus.

F7 — per-hop chain `model` overrides IR model in provider.spawn()
  (server.mjs executeHopFn + streaming sourceFactory). Pre-D75 executeHopFn
  used hopModel for cache key + audit ctx but passed the original irReq
  (with irReq.model = user's request) to provider.spawn(). Chain config
  [{anthropic, claude-X}, {openai, gpt-5.5}] would always spawn BOTH plugins
  with --model claude-X — openai rejected the unknown model and the chain
  died. This broke the core OLP value prop (cross-provider fallback with
  provider-appropriate model substitution). Fix: build per-hop IR variant
  with { ...irReq, model: hopModel } and pass to spawn. Conditional skips
  clone when hopModel === irReq.model. Applied to BOTH buffered path AND
  streaming path. Authority: ADR 0004 § Chain advancement step 1 (per-hop
  config supplies provider AND model — contract always specified, code
  didn't complete it).

Out of scope (deferred to Phase 5):
- F5 (server bind / OLP_BIND env) — needs anonymous-key trust review
- F6 (doctor client-vs-server-side limit) — needs trigger-taxonomy ADR

Test count: 704 → 714 (+10 Suite 36 D75 regression tests: 36i–36r).
Files touched: lib/providers/codex.mjs, bin/olp.mjs, server.mjs,
test-features.mjs, package.json, CHANGELOG.md.

Phase 5 process learning: every provider plugin D-day must include a
real-CLI E2E on a remote operator host before merging — not on the
maintainer workstation (which may have an older CLI cached from a prior
install). D6/D7 codex E2E was deferred and that deferral compounded across
3 layers. F7 reinforces a separate lesson: when a function signature takes
(provider, model, ir), reviewers must check that `model` is consumed
everywhere downstream — not just at the call site they happened to look at.

Authority: ADR 0002 (provider contract — codex plugin), ADR 0004 (fallback
engine — per-hop model contract), lib/providers/codex.mjs D6 assumption
A2/A3/A4 docstrings (which all said "D7 will pin" and D7 never did); codex
CLI v0.133.0 on-disk schema + `codex exec --help` output verified
empirically on PI231 (2026-05-26 E2E session); Iron Rule 第二律
evidence-over-should-work; CLAUDE.md release_kit.phase_rolling_mode
cross-Phase discipline ("hotfix to a shipped Phase N deliverable → bump
patch, tag, release before next push").

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-26 12:50:41 +10:00
dtzp555-maxGitHubtaodengClaude Opus 4.7
 		e69e908dae feat+test+docs: D64-D67 — olp Node CLI + doctor framework + per-provider doctor checks + ADR 0002 Amendment 7 (#42) 
* feat+test+docs: D64+D65+D66+D67 — olp Node CLI + olp doctor framework + per-provider doctor checks + ADR 0002 Amendment 7

Second substantive Phase 4 implementation. 4 D-days bundled per Iron Rule
11 IDR — CLI dispatches to doctor; doctor calls into provider plugins via
the new contract method; ADR amendment authorizes the contract change.
Single PR is the minimum reviewable unit for "does plugin amendment +
plugin impl + doctor consumer line up?"

## D64 — bin/olp.mjs Node CLI scaffold

Operator surface for OLP. Node not bash (per ADR 0010 § Notes — bash with
python3 JSON parsing is a known fragile point; OLP standardizes on Node).

Subcommands:
- status / health / usage / models / cache — HTTP calls to existing endpoints
- providers — local: cross-references models-registry.json + config.json
- chain show [<model>] — local: prints routing.chains from ~/.olp/config.json
- logs [N] [--level X] — reads ~/.olp/logs/audit.ndjson via audit-query
- restart — launchctl (macOS) / systemctl --user (Linux), best-effort
- keys ... — delegates to bin/olp-keys.mjs runCli (no logic duplicated)
- doctor [--check <id|category>] [--json] — D65 framework
- help / --help / -h

Token / URL resolution:
- OLP_PROXY_URL env → OLP_PORT env → http://127.0.0.1:4567 (D60 default)
- OLP_API_KEY env → OLP_OWNER_TOKEN env (filesystem manifest tokens are
  one-way SHA-256 per ADR 0007 § 5 — not recoverable; CLI surfaces
  helpful 401 message pointing at olp-keys keygen)

Output:
- Default: human-readable ANSI-colored text (no chalk dep, auto-suppressed
  under --json)
- --json: raw JSON for scripting
- Exit codes: 0=ok / 1=usage / 2=network|HTTP / 3=auth

No npm deps. Built-ins only.

Installed via package.json bin entry so `npx olp <subcommand>` works.

## D65 — lib/doctor.mjs framework

Ports OCP scripts/doctor.mjs (the bedrock of AI-driven self-repair per the
OCP audit's #2 inheritance candidate). Machine-readable next_action so a
Claude Code / Cursor / etc. agent can self-repair OLP.

Check shape:
  { id, category, async run(): { status: 'ok'|'fail'|'warn', message, evidence? } }

Built-in checks: server.running, server.version, config.exists,
config.providers_enabled, config.chains_configured, auth.owner_key_exists,
system.node_version. Per-provider checks collected dynamically via
provider.doctorChecks() per D67.

--json output:
  { checks: [...], kind: noop|update|fix_oauth|fix_config|fresh_install|
    fix_server|fix_provider, next_action: { ai_executable: [],
    human_required: [], verify: 'olp doctor' }, summary }

--check <id-or-category> for tight repair-loop fast paths.

## D66 — Per-provider doctorChecks() implementations

Each shipped plugin contributes its own checks (lives in plugin file so the
provider's maintainer updates it naturally):

- anthropic.mjs: cli_available (claude --version) + oauth_token_present
  (~/.claude/.credentials.json OR ANTHROPIC_OAUTH_TOKEN env)
- codex.mjs: cli_available (codex --version) + auth_present
  (~/.codex/config.json)
- mistral.mjs: cli_available (vibe --version) + api_key_present
  (MISTRAL_API_KEY env OR ~/.vibe/.env)

Each fail returns evidence.fix_commands (for ai_executable[]) or
evidence.human_required (e.g., 'run: claude auth login').

## D67 — ADR 0002 Amendment 7

New amendment adds OPTIONAL provider.doctorChecks(): DoctorCheck[] to the
Provider contract. Backwards compatible — plugins without doctorChecks()
contribute no provider checks (default behavior). Validator extended in
lib/providers/base.mjs validateProvider.

## Test count

636 → 658 (+22 tests across Suites 32, 33).

- Suite 32 — bin/olp.mjs CLI scaffold (10 tests): parseArgv, USAGE,
  unknown-subcommand, providers local + --json, chain show, status via
  ephemeral server with owner token, ECONNREFUSED → exit 2,
  resolveBearerToken precedence
- Suite 33 — lib/doctor.mjs framework (12 tests): all kind branches
  (noop / fresh_install / fix_server / fix_oauth / fix_provider),
  collectProviderChecks reads doctorChecks(), throwing plugin captured,
  --check filter, built-in checks against temp HOME, anthropic plugin
  probe set, resolveProxyUrl precedence, deriveKind/deriveNextAction units

## Scope discipline

server.mjs UNTOUCHED. All HTTP subcommands consume EXISTING endpoints.
No new endpoints. No /health.anonymousKey. No olp-connect. No Telegram
plugin. No IDE docs bundle. No CHANGELOG / package.json version bump
(Phase 4 close handles versioning; only package.json bin entries updated).

## Known limitations (flagged for reviewer)

- olp restart not unit-tested (would require mocking child_process.spawn
  in invasive way; manual smoke-test only at this D-day)
- olp logs --level filtering matches optional level field if present in
  audit-event objects; appendAuditEvent already populates it where
  meaningful — no schema change needed in this bundle
- olp usage panel shape inferred from lib/audit-query.mjs exports; if
  /v0/management/dashboard-data wire shape differs in subtle ways,
  formatter degrades to '?' but --json always works

## Authority

- ADR 0010 § Phase 4 D-day plan D64-D67 line
- ADR 0002 Amendment 7 (this commit — new amendment)
- OCP ocp bash wrapper /Users/taodeng/ocp/ocp (subcommand reference,
  translated to Node)
- OCP scripts/doctor.mjs /Users/taodeng/ocp/scripts/doctor.mjs
  (framework reference)
- 2026-05-26 brainstorm (Top 5 OCP inheritance candidates, item 2:
  olp doctor machine-readable next_action)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix: D64-D67 reviewer P2 fold-in — shell-quote ai_executable paths + launchctl kickstart caveat

Reviewer APPROVE — 0 P0/P1, 2 P2 hardening notes folded in.

P2-1 — Shell-quote interpolated paths in fix_commands.

lib/doctor.mjs config.exists fix_commands previously interpolated
${olpHome} / ${configPath} unquoted into the printf template. A
malicious OLP_HOME env value containing shell metacharacters could
inject commands into the suggested-fix string an AI agent (or human)
pastes back into a terminal.

Added _shellQuote(s) helper (POSIX single-quote-wrap with escape for
embedded single quotes per POSIX shell rules). Risk surface is narrow
at family scale (operator local env, single-user proxy), but hardening
cost is one helper.

P2-2 — Document launchctl kickstart -k env-stale pitfall.

cmdRestart header now carries an explicit caveat that `launchctl
kickstart -k` does NOT re-read the plist EnvironmentVariables block —
launchd uses cached env from the most recent bootstrap. This is a
known OCP institutional lesson (PIT INDEX in cc-rules MEMORY.md). The
comment documents the bootout/bootstrap dance for env reloads and
notes that the Phase 4 installer (post-D73) will expose `olp restart
--full` for the safer reload path.

658/658 tests still pass; the _shellQuote change is invisible to
existing tests because the test fixtures use safe paths.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-26 08:48:20 +10:00
dtzp555-maxGitHubtaodengClaude Opus 4.7
 		1062e88e77 feat+test: D57 — cache layer streaming singleflight (ADR 0005 Amendment 8, issue #16) (#36) 
* feat+test: D57 — cache layer streaming singleflight (ADR 0005 Amendment 8, issue #16)

First of three D-days implementing the v1.x roadmap #1 streaming-path
singleflight. D57 lands the cache-layer coordination primitive only;
server.mjs wiring is D58, docs polish is D59.

## What

lib/cache/store.mjs — new method `getOrComputeStreaming(keyId, cacheKey,
sourceFactory, opts) → { stream, isFirst, role }`. Three outcomes per
Amendment 8 §1: cache_hit (no spawn), attached (joins existing inflight),
source (first caller, spawns via factory). Backed by `_streamingInflight`
Map keyed by `${keyId}\\0${cacheKey}` with synchronous check+insert per
Amendment 8 §1 + §6 atomicity invariant.

Internals (Amendment 8 §§2-10, §14):
- StreamingInflightEntry + AttachedClient typedefs
- Tee fan-out loop: single reader drains source, pushes to accumulatedChunks
  + every client's queue, fires per-client resolveNext promises
- Late-joiner replay buffer (synchronous drain on attach; reject with
  synthetic STREAM_BACKPRESSURE terminator if drain exceeds cap)
- Per-client backpressure (PER_CLIENT_QUEUE_CAP=1MB default, overridable)
- Replay buffer cap (ACCUMULATED_REPLAY_CAP=10MB default, overridable;
  cache write skipped if exceeded)
- AbortController propagation: when attachedClients.size === 0 after
  client iterator return(), source.return() + abort.signal fire
- D38 coordination via sourceFactory closure (factory wraps tryAcquireSpawn
  internally; cache layer just invokes it once)

lib/providers/base.mjs — `'STREAM_BACKPRESSURE'` added to
PROVIDER_ERROR_CODES per Amendment 8 §8. NOT in HARD_TRIGGER_CODES (engine
update lands in D58; whitelist-only map gives correct default).

test-features.mjs Suite 27 — 12 new tests (27a-27l) covering: solo stream,
2-concurrent dedup, mid-stream join + post-completion cache_hit, per-client
disconnect with other clients continuing, full disconnect → abort, source
error propagation, per-client backpressure, replay cap, TTL race during
inflight, sourceFactory throw, stats accuracy, composite key isolation.

## Scope

Strictly cache layer + base.mjs PROVIDER_ERROR_CODES entry. Untouched:
server.mjs, providers/{anthropic,codex,mistral}.mjs, fallback/engine.mjs,
IR, dashboard.html, README, CHANGELOG, package.json. D58 will wire server.

## Authority

- docs/adr/0005-cache-cross-provider.md Amendment 8 (2026-05-25, design
  ratified at D42, implementation gated on maintainer "go" — fired
  2026-05-25 post-v0.3.1)
- docs/v1x-roadmap.md #1 (streaming SF + TOCTOU close)
- GitHub issue #16 (round-6 cold-audit F13 sibling TOCTOU window)
- ADR 0002 Amendment 6 (D38 tryAcquireSpawn/releaseSpawn — invoked via
  sourceFactory closure at server layer, not directly by cache)

## Test count

603 → 615 (+12 D57 tests). Local: 615/615 pass.

## Iron Rule 11 (IDR)

D57 is the cache-layer minimum reviewable unit. D58 wires server.mjs +
adds X-OLP-Streaming-Inflight header + integration tests through HTTP
layer. D59 polishes README + closes issue #16.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix: D57 reviewer follow-ups — P2-2 (constant cleanup) + P2-3 (join-event deferral note)

Fold-in for D57 PR #36 fresh-context opus reviewer findings (APPROVE WITH
MINOR — 0 P0/P1, 3 P2). P2-1 is the D58 split (already planned); this
commit addresses P2-2 + P2-3.

P2-2 (cosmetic) — replace `Object.freeze({ value: X }).value` baroque
declaration of PER_CLIENT_QUEUE_CAP_DEFAULT + ACCUMULATED_REPLAY_CAP_DEFAULT
with a plain `export const X = 1*1024*1024`. The freeze-then-extract pattern
freezes a throwaway wrapper, which the `.value` immediately discards — does
nothing useful. Const declaration already gives binding immutability.

P2-3 (observability event parity deferral) — ADR 0005 Amendment 8 §11
lists `streaming_inflight_join` as one of four log events. The cache
layer cannot emit it correctly because provider/model identity lives in
the sourceFactory closure (server-layer concern). Added TODO note in
`_attachClient` pointing at D58 server wiring where the event will fire
on the consumer of `role: 'attached'`. The other three §11 events
(stream_backpressure_disconnect / streaming_inflight_source_done /
streaming_inflight_abort) ARE emitted from the cache layer with
{client_id, composite_key, ...} payloads; provider/model is enriched at
the server-side wrapper.

No test-surface change. 615/615 still pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-25 20:35:03 +10:00
taodengandClaude Opus 4.7 bdfea6884b feat+docs+test: D39 — D16 follow-ups (issue #3, 4 parts) 
D16 reviewer (commit `bafa6d1`) left 4 non-blocking suggestions
batched into issue #3 as a tracker. D39 closes all 4.

**Part 1 — CacheStore.delete(keyId, cacheKey) API** (lib/cache/store.mjs)

D16 originally evicted truncated entries via
`cacheStore.set(keyId, hopCacheKey, result, ttlMs=0)` — a TTL=0
tombstone purged lazily on next access. D39 introduces an explicit
delete primitive that removes the entry immediately.

- Synchronous: `delete(keyId, cacheKey) → boolean`. Returns true if
  the entry was present and removed, false if absent. Sync (not
  async) for the simplest in-memory Map contract — mirrors clear().
  Other CacheStore methods are async to leave room for a Phase 2
  file-backed adapter; delete being sync was a deliberate choice.
- Namespace cleanup: when the inner Map becomes empty after delete,
  the outer Map's per-keyId entry is also removed (memory hygiene;
  mirrors the _activeSpawns cleanup pattern from D38).
- Behavior: peek/get/getOrCompute see no trace after delete; the
  subsequent getOrCompute triggers a fresh compute.

**Part 2 — `cache_evicted_truncated` observability log** (server.mjs)

After the D16 eviction call in collectAllChunks, emit:
```js
logEvent('info', 'cache_evicted_truncated', {
  provider, model, cache_eviction_hit,
});
```

Dashboard sees salvage frequency per (provider, model). The
cache_eviction_hit boolean distinguishes "we evicted an entry" (true)
from "we tried to evict but it was already gone" (false — race with
concurrent eviction or TTL purge), preserving observability accuracy
under concurrency.

**Part 3 — Sticky-cache regression test** (test-features.mjs)

Defense-in-depth around the eviction path. Two consecutive identical
buffered requests; the first triggers SPAWN_FAILED after partial
chunks → Case B salvage returns `{ chunks..., finish_reason: 'length' }`
to the client and evicts via delete(). The second identical request
must trigger a fresh spawn (NOT serve the salvaged response from a
stale cache entry).

Asserts on BOTH invariants for defense-in-depth:
- Mock provider spawn count == 2 across the 2 identical requests
- Second request's X-OLP-Cache header is 'miss'

If eviction silently breaks in a future regression, both assertions
catch it independently.

**Part 4 — SPAWN_TIMEOUT salvage parity: DOCUMENT ASYMMETRY**
(docs/adr/0004-fallback-engine.md)

Maintainer decision: SPAWN_TIMEOUT is NOT salvaged. Document the
asymmetry rather than implementing parity. ADR 0004 Amendment 1 is
extended with a new section "Why SPAWN_TIMEOUT is excluded from
salvage" with 4-point rationale:

1. SPAWN_FAILED indicates the provider crashed mid-stream — there's
   nothing more coming; partial > nothing. Next-hop spawn has no
   advantage (same input may crash same way).
2. SPAWN_TIMEOUT indicates the provider was slow (deadline exceeded
   per `hints.maxSpawnTimeMs`). Fallback advancement to a DIFFERENT
   provider is more likely to give a complete response than salvaging
   a partial from a slow provider.
3. The "user paid for partial content" framing from D16 captures only
   SPAWN_FAILED. For SPAWN_TIMEOUT the user actually paid for "result
   within time T" — partial-at-time-T is not what was paid for;
   "full result soon after T" via fallback is closer.
4. Code-level inspection confirms the asymmetry: collectAllChunks
   catch matches ONLY `code === 'SPAWN_FAILED'` (server.mjs:563).
   SPAWN_TIMEOUT propagates via re-throw and hits evaluateHardTriggers.

v1.x re-evaluation trigger: if real usage shows users want partial-
on-timeout for very long deadlines, add a v1.x design ADR.

Stale comment fix: `lib/providers/anthropic.mjs:369` previously said
"SPAWN_TIMEOUT salvage parity is tracked in issue #3". D39 closes
that issue, so the comment is updated to point at ADR 0004 Amendment 1.

**Tests** (test-features.mjs): 447 → 452 (+5):
- 3 unit tests on CacheStore.delete (Suite 9): present-key true, absent-key
  false, namespace cleanup at empty
- 1 D16 integration test: cache_evicted_truncated log fires with
  correct fields during salvage
- 1 sticky-cache regression: spawn count 2 across 2 identical requests,
  X-OLP-Cache miss on second

Pre-commit fold-ins (per evidence-first checkpoint #4):

- **Reviewer Suggestion #1**: cacheStore.delete() return value was
  discarded at the call site → log inflated salvage metric under
  concurrent-eviction race. Folded: captured `evicted` boolean and
  added to log payload as `cache_eviction_hit`.
- **Reviewer Suggestion #2**: anthropic.mjs:369 stale comment
  pointing at now-closed issue #3. Folded: rewrote to point at
  ADR 0004 Amendment 1 § "Why SPAWN_TIMEOUT is excluded from
  salvage".
- **Reviewer Suggestion #3**: ADR 0004 attribution ambiguity —
  parenthetical "(per Amendment 3 — SPAWN_TIMEOUT is one of the 4
  live hard-trigger codes alongside SPAWN_FAILED, CLI_NOT_FOUND, and
  CONCURRENCY_LIMIT from Amendment 4)" could mis-parse as Amendment 3
  covering all four. Folded: split to
  "(per Amendment 3: SPAWN_FAILED, CLI_NOT_FOUND, SPAWN_TIMEOUT;
  per Amendment 4: CONCURRENCY_LIMIT)".

**CHANGELOG**: D39 sub-entry appended under the existing D38 entry
in Unreleased section. No package.json bump (phase_rolling_mode).

Authority:
- ADR 0005 § Cache layer — CacheStore API extension (Part 1)
- ADR 0004 Amendment 1 update — SPAWN_TIMEOUT asymmetry rationale (Part 4)
- GitHub issue #3 — closed by this commit
- D16 commit bafa6d1 non-blocking suggestions — batched here
- CC 开发铁律 v1.6 § 10.x — fresh-context opus reviewer independent
- CLAUDE.md release_kit_overlay phase_rolling_mode — under Unreleased

Reviewer (fresh-context opus, Iron Rule 10): APPROVE_WITH_MINOR.
Verified: delete() sync + call-site no-await correct; namespace
cleanup guarded on (had && ns.size === 0); SPAWN_TIMEOUT NOT in
salvage catch; ADR section 4-point rationale internally consistent
with code state; hygiene clean; 452/452 tests pass independently.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 21:13:57 +10:00
taodengandClaude Opus 4.7 994568a8fb feat+ci: D38 — maxConcurrent runtime enforcement (issue #1) 
ADR 0002 Amendment 1 declared `hints.maxConcurrent` declarative-only at
v0.1 — type-validated at startup but with no runtime enforcement.
D38 wires the runtime enforcement via a per-provider in-flight spawn
counter with immediate-advancement on saturation.

Design choice: **immediate-advancement via fallback engine** (queue +
timeout DEFERRED). When a provider is at its maxConcurrent limit, the
spawn call synchronously fails with a new `CONCURRENCY_LIMIT` error
code; the fallback engine treats this as a hard trigger and advances
to the next chain hop. If the entire chain is saturated, the user
sees a chain-exhausted error (existing path).

Rationale for immediate-advancement over queue+timeout:

1. The fallback chain exists precisely for this kind of overflow —
   adding a queue layer would duplicate the advancement semantics.
2. Queue + timeout adds new config surface (timeout duration, queue
   depth bounds, queue eviction policy) that isn't needed at the
   personal/family scale OLP serves.
3. Head-of-line blocking risk: a long-running spawn would stall
   queued requests behind it even though other providers in the chain
   could serve them immediately.
4. Fail-fast latency aligns with the multi-provider proxy philosophy
   ("spread risk across providers, not within a provider").

Queue + timeout is deferred to a v1.x design ADR if real usage shows
demand. ADR 0002 Amendment 6 and ADR 0004 Amendment 4 capture the
decision explicitly.

Changes (8 files, +<delta>):

**Code**

1. **lib/providers/base.mjs** — add `CONCURRENCY_LIMIT` to
   `PROVIDER_ERROR_CODES`. JSDoc clarifies the code is synthesised by
   the orchestration layer, not thrown by provider plugins themselves.

2. **lib/providers/index.mjs** — new semaphore primitives:
   - `tryAcquireSpawn(providerName, maxConcurrent)` — atomic
     check-then-increment. Returns `true` on success, `false` if at
     limit. Atomicity rests on the JS single-threaded invariant
     (read + write synchronous, NO `await` between them); module-level
     comment block warns future maintainers against breaking this.
   - `releaseSpawn(providerName)` — decrement; throws on
     under-decrement (defensive bug guard for missing acquire / double
     release). Map.delete at zero for clean memory footprint.
   - `getActiveSpawnCount(providerName)` — returns current count
     (0 for unseen providers). For diagnostics + tests.
   - `DEFAULT_MAX_CONCURRENT_SPAWNS = 4` — defense-in-depth fallback
     matching the v0.1 plugin defaults (anthropic/codex/mistral all
     declare hints.maxConcurrent: 4). Also coerces non-integer / NaN
     / negative inputs to the default.
   - `__resetSpawnCounters()` — internal test seam.

3. **lib/fallback/engine.mjs** — `CONCURRENCY_LIMIT: true` added to
   `HARD_TRIGGER_CODES`. `classifyTrigger` and `evaluateHardTriggers`
   both pick it up via the same lookup. v0.1 live hard-trigger codes
   are now 5 (was 4 post-D34): SPAWN_FAILED, CLI_NOT_FOUND,
   AUTH_MISSING:false, SPAWN_TIMEOUT, CONCURRENCY_LIMIT.

4. **server.mjs** — gate the spawn call in handleChatCompletions at
   BOTH spawn call sites:
   - **Buffered path** (executeHopFn → collectAllChunks): acquire
     before provider.spawn; on failure synthesise
     `ProviderError(CONCURRENCY_LIMIT)` with providerName /
     maxConcurrent / activeSpawns diagnostic fields and throw —
     fallback engine catches and advances. On success, outer try/
     finally wraps the inner D16 truncation-salvage try/catch so
     releaseSpawn fires on EVERY exit path (return, D16 salvage
     return, re-throw).
   - **Streaming path** (single-hop real-SSE branch): acquire BEFORE
     the streaming branch entry. If acquire fails, branch is skipped
     and request falls through to buffered path (whose own gate
     surfaces chain-exhausted for single-hop chains). If acquire
     succeeds, existing streaming try/catch gains
     `finally { releaseSpawn(streamProvider) }` — slot releases on
     stop-chunk completion, generator exhaustion, abort, or any
     exception path. `releaseSpawn` fires at END of stream
     consumption, not when spawn() returns.

**Tests** (test-features.mjs): 431 → 447 (+16):

Suite 18 — D38 — maxConcurrent runtime enforcement:
- 18a: PROVIDER_ERROR_CODES.CONCURRENCY_LIMIT exists
- 18b: evaluateHardTriggers true for CONCURRENCY_LIMIT
- 18c: AUTH_MISSING regression guard (D38 did NOT flip it to hard)
- 18d.1-18d.4: semaphore unit (increment / saturate-no-increment /
  release / map-delete-at-zero)
- 18e: tryAcquireSpawn returns false without side-effect when at limit
- 18f: releaseSpawn throws on under-decrement
- 18g.1-18g.2: DEFAULT_MAX_CONCURRENT_SPAWNS applied for invalid
  inputs (undefined / NaN / negative / non-integer)
- 18h: __resetSpawnCounters clears state
- 18i: HTTP integration — 5 concurrent requests to single-hop
  chain w/ maxConcurrent=2 → peak in-flight exactly 2, 2 succeed,
  3 fail
- 18j: counter releases after buffered request (sequential test)
- 18k: 2-hop chain — saturated primary advances to fallback
- 18l: streaming counter releases at END of stream (not at spawn)

**ADR amendments**

5. **docs/adr/0002-plugin-architecture.md** — Amendment 6 added.
   Removes "Declarative hint only at v0.1" caveat from the
   `maxConcurrent` description in the Provider contract section.
   Adds implementation reference + design-choice rationale (4 points).
   Lists all exported symbols.

6. **docs/adr/0004-fallback-engine.md** — Amendment 4 added.
   CONCURRENCY_LIMIT added to v0.1 hard-trigger taxonomy. Documents
   synthesis-vs-plugin-thrown distinction. Documents first-chunk
   safety (acquire before any res.write). v1.x re-evaluation triggers
   named.

**CHANGELOG**

7. **CHANGELOG.md** — Unreleased sentinel replaced with proper D38
   entry. Per CLAUDE.md release_kit phase_rolling_mode, this lands
   under Unreleased; promotion to ## v0.1.1 happens at the v0.1.1
   release. The D37 phase_rolling_mode gate now correctly fires if
   anyone tags v0.1.x with this content present without promotion —
   intentional.

Pre-commit fold-ins (per evidence-first checkpoint #4):

- **Reviewer Suggestion #1 (test 18c name mismatch)**: 18c is named
  "classifyTrigger returns hard for CONCURRENCY_LIMIT" but body tests
  AUTH_MISSING regression. Renamed header comment to
  "AUTH_MISSING regression guard" and clarified that
  CONCURRENCY_LIMIT classification is covered by 18b.

- **Reviewer Suggestion #3 (activeSpawns diagnostic field)**: server
  .mjs:532 set `concurrencyErr.activeSpawns = maxConcurrent` (the
  limit). Technically correct (since acquire just failed, live
  count == limit) but confuses future readers. Changed to query
  `getActiveSpawnCount(hopProvider)` directly. Added import of the
  new symbol to the lib/providers/index.mjs import block.

- **Reviewer Suggestion #5 (ADR overstatement)**: ADR 0002 Amendment 6
  said getActiveSpawnCount is "exported for /health, diagnostics,
  and tests" but /health integration is not wired at D38. Reworded to
  "exported for diagnostics and tests" + "/health integration
  deferred — when surfaced there will land at providers.status.<name>
  .activeSpawns; not wired at D38." Avoids overstating current state.

Two reviewer suggestions not folded:
- Suggestion #2 (streaming test 18l comment about branch entry
  conditions) — low priority; the test passes and the branch is
  taken (verified by reviewer). Future polish if confusion arises.
- Suggestion #4 (additional test for streaming-saturation → chain-
  exhausted) — code path is straightforward and intentional; 18i
  covers the buffered-path version directly. Defer.

Authority:
- ADR 0002 Amendment 1 (declarative-only caveat) — superseded by
  Amendment 6
- ADR 0002 Amendment 6 (this commit) — runtime enforcement landed
- ADR 0004 Amendment 4 (this commit) — CONCURRENCY_LIMIT added to
  v0.1 hard-trigger taxonomy
- GitHub issue #1 — closed by this commit
- CC 开发铁律 v1.6 § 10.x — independent fresh-context reviewer
- CLAUDE.md release_kit_overlay phase_rolling_mode — no version
  bump; Unreleased entry written

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus,
independent of drafter): APPROVE. Critical depth checks:
- Atomicity in tryAcquireSpawn (lines 285-290 read+set with no
  intervening await) — confirmed; module-level invariant comment
  warns future maintainers
- Release on every exit path: buffered (outer finally wraps inner
  try/catch covering D16 salvage return + normal return + re-throw);
  streaming (try/finally covers stop-chunk return + loop exhaustion +
  catch paths). No double-release path identified.
- Streaming release timing: fires in finally after res.end() at all
  exit paths, never before stream consumption completes
- Counter leak: every code path traced — no orphan acquire identified
- 447/447 tests pass in reviewer's independent npm test run

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 21:02:40 +10:00
taodengandClaude Opus 4.7 e96752a528 docs+fix+test: D36 — pre-Phase-2 batch #2 (issues #2 #5 #6 #13 #14 #15) 
Second batch of pre-Phase-2 cleanup. 6 GitHub issues closed in one
cohesive docs/governance commit with 1 small code addition (#2 debug
log line in server.mjs) and 1 transcript artifact (#15 new file).

Changes (7 files modified, 1 file created, +470 / -13):

**Code changes**

1. **#2 — cache_control partial-noop debug log** (server.mjs)

   ADR 0005 § D2 says: "for non-Anthropic targets, the bypass markers
   are noop'd (logged once per request at debug level so users can
   see they were ignored)". Pre-D36 logEvent fired only when an
   Anthropic hop actually bypassed; non-Anthropic hops with markers
   present were silently noop'd.

   New 12-line block in handleChatCompletions right after
   hasCacheControlMarkers is computed. Fires logEvent('debug',
   'cache_control_partial_noop', { chain: [<provider names>],
   marker_count: <count> }) when:
   - hasCacheControlMarkers === true AND
   - chain.some(hop => hop.provider !== 'anthropic')

   Fires at most once per request (top-level if, not in a loop). No
   log when no markers, or when every chain hop is Anthropic. The
   existing cache_bypass debug log inside shouldBypassCacheForHop is
   untouched.

   marker_count sums body-side and IR-side extractCacheControlMarkers
   results. At v0.1 the IR term is structurally 0 (openAIToIR strips
   cache_control); inline comment marks this as a revisit point for
   the future ADR 0003 amendment that activates cache_control in the
   IR whitelist.

**Documentation amendments**

2. **#5 — ADR 0002 vibe.mjs → mistral.mjs** (docs/adr/0002-plugin-architecture.md)

   § Decision filesystem layout: `vibe.mjs` corrected to
   `mistral.mjs` (file named after provider key per the convention
   anthropic.mjs/codex.mjs). The vibe.mjs entry was an early-draft
   naming choice that never landed. Amendment 5 prepended above
   Amendment 4 documenting the filename correction + explicit
   convention statement (file named after provider key, not CLI
   binary) for future contributors.

3. **#6 — mistral.mjs A5 flip + ALIGNMENT.md table update**
   (lib/providers/mistral.mjs + ALIGNMENT.md)

   Pre-D36: header A5 (model flag) status was UNPINNED-D-later-verifies
   but function body lines 376-380 said CONFIRMED-NOT-APPLICABLE
   (DeepWiki enumeration already confirmed `--model` does not exist
   on vibe CLI). Header now reflects CONFIRMED-NOT-APPLICABLE with
   DeepWiki citation (DOCS-4). ALIGNMENT.md Speculative-Candidate
   table mistral row: A5 removed from UNPINNED list; A4, A6, A7, A8
   preserved with parenthetical descriptions intact. No code change
   to function body (already correct).

4. **#13 — /v1/models alias entries — ALIGNMENT.md + spec-pin
   governance** (ALIGNMENT.md + docs/openai-spec-pin.md)

   Round-6 F10 flagged D27 F15's alias entries on /v1/models as
   borderline Rule 2(b) violation (OpenAI spec does not enumerate
   aliases as separate entries). Option C selected: keep current
   behavior, document the controlled deviation.

   - ALIGNMENT.md: new "Controlled deviations (entry-surface scope)"
     subsection under "Class-specific Exceptions". Entry 1 documents
     the /v1/models alias deviation with rationale (D27 F15
     onboarding), formal contract reference, field constraints
     (owned_by matches canonical, created matches canonical, no
     invented fields), SPOT reference (getAliasMap()), and re-
     evaluation trigger.
   - docs/openai-spec-pin.md: new alias-surfacing subsection under
     GET /v1/models with full 4-field contract table (id/object/
     created/owned_by), rationale, sourcing explanation, forward
     path.
   - server.mjs handleModels: NO CHANGE — behavior preserved.

5. **#15 — Anthropic v2.1.89 transcript artifact**
   (docs/provider-audits/anthropic.md NEW; ALIGNMENT.md +
   lib/providers/anthropic.mjs cross-references)

   Round-6 F12: ALIGNMENT.md anthropic row pin (v2.1.89, observed at
   D4) cited the plugin header; plugin header cited the observation
   date but no transcript. Circular per Rule 1 ("observed behaviour,
   transcript attached").

   New file docs/provider-audits/anthropic.md (single living artifact,
   not version-specific):
   - Date of capture: 2026-05-24
   - Observed `claude --version`: 2.1.132 (Claude Code) — captured
     today on the project maintainer's primary workstation
   - Plugin-pinned version: @anthropic-ai/claude-code v2.1.89 (from
     D4 implementation pin in ALIGNMENT.md Provider Authority Pins)
   - Version drift: honestly documented — pin is v2.1.89, live is
     v2.1.132, drift within tolerance, re-audit triggers named
   - Sample invocation: `claude -p --output-format text --no-session-
     persistence --model <model> [--debug]`
   - Flag-surface table: 5 OLP-consumed flags verbatim from
     `claude -p --help` (-p / --output-format / --no-session-
     persistence / --model / --debug)
   - Citation cross-references back to ALIGNMENT.md + plugin header

   ALIGNMENT.md anthropic row: appended "transcript artifact: docs/
   provider-audits/anthropic.md (captured 2026-05-24)" — closes the
   circular citation.

   lib/providers/anthropic.mjs header: 5-line pointer to the artifact
   with version numbers stated explicitly.

**Tests** (test-features.mjs): 424 → 431 (+7):

- #2 partial-noop log ×3:
  - Suite 9f case 1: markers + mixed chain → fires once at level=debug
  - Suite 9f case 2: no markers → suppressed
  - Suite 9f case 3: anthropic-only chain → suppressed

- #14 cache_control slot determinism ×4:
  - #14a: markers-present IR produces different key from no-markers IR
  - #14b: same IR computed twice yields identical key
  - #14c: two independently-constructed IRs with identical payloads
    yield same key
  - #14d: both top-level and content-array-nested markers affect the key

  All 4 tests call computeCacheKey directly on hand-built IRs
  (bypassing openAIToIR which strips markers at v0.1). Per ALIGNMENT.md
  Rule 2 (No Invention), no sortMarkers helper added — the slot is
  dead-code at v0.1 and shipping a helper without a caller authority
  would be invention. Test comment documents the forward-activation
  contract.

Pre-commit fold-in (per evidence-first checkpoint #4):
- **D36 reviewer flagged marker_count latent double-count risk**
  (Suggestion #1, non-blocking). The sum at server.mjs:435-437 is
  safe at v0.1 (IR term structurally 0) but will 2× when a future
  ADR 0003 amendment activates cache_control in the IR whitelist.
  Folded in a 4-line comment marking the revisit point.

Two other non-blocking reviewer suggestions not folded:
- Test count brief-vs-deliverable discrepancy (4 not 3 for #14) is
  informational — the 4-test variant is strictly better (covers
  content-array nesting which is a real extractCacheControlMarkers
  contract path).
- Recapture-procedure git-add reminder in anthropic.md is low-priority
  procedure documentation.

Authority:
- ADR 0005 § D2 — cache_control partial-noop debug log requirement (#2)
- ADR 0002 § Decision filesystem layout (Amendment 5) — plugin file
  naming convention (#5)
- DeepWiki vibe CLI flag enumeration — A5 not applicable (#6)
- ALIGNMENT.md Rule 2(b) + docs/openai-spec-pin.md GET /v1/models —
  alias controlled deviation (#13)
- ADR 0005 cache key stability invariant + ADR 0003 forward-compat —
  cache_control slot determinism contract (#14)
- ALIGNMENT.md Rule 5 (observed behaviour, transcript attached) +
  Provider Authority Pins anthropic row — transcript artifact (#15)
- CC 开发铁律 v1.6 § 10.x — independent fresh-context reviewer
- CLAUDE.md release_kit_overlay phase_rolling_mode — D36 under
  "Unreleased" against Phase 2; no version bump

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus,
independent of drafter): APPROVE. Critical depth checks:
- #2: gate condition fires only on (markers AND non-anthropic-hop);
  pre-existing cache_bypass log inside shouldBypassCacheForHop
  untouched
- #5: lib/providers/ directory verified — mistral.mjs exists,
  vibe.mjs does not exist
- #6: mistral.mjs spawn-site body comment (lines 376-380) already
  CONFIRMED-NOT-APPLICABLE pre-D36 and unchanged in this diff
- #13: server.mjs handleModels unchanged (verified via grep)
- #14: all 4 tests pass against current code; no sortMarkers helper
  shipped (Rule 2 No Invention honored)
- #15: live `claude --version` independently run by reviewer →
  matches artifact (2.1.132); all 5 OLP-consumed flags independently
  verified present in `claude -p --help`
- Hygiene: 0 hits for personal markers, home paths, OAuth tokens,
  internal IPs across all 8 files
- 431/431 tests pass in reviewer's independent npm test run

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 20:38:16 +10:00
taodengandClaude Opus 4.7 60570ef074 fix+docs: D34 — FINAL batch (F1+F4+F7+F8); audit cadence stops 
cold-audit catch from 2026-05-24 (round 6 — FINAL)

This is the closing D-day of a 24-day round-1→round-6 audit cycle.
After this commit + the 9 round-6 follow-up issue filings, no more
audit rounds. Trajectory R1=17 → R2=13 → R3=13 → R4=10 → R5=12 →
R6=14 — the method did not converge; owner chose Option A (focused
batch of most consequential items, then STOP).

Changes (6 files, +138 / -47):

**Code changes**

1. lib/cache/keys.mjs (+14/-?) — F4 P2 cache key array-field
   normalization:
   - New `normalizeArrayField` helper: `(Array.isArray(v) && v.length === 0) ? null : (v ?? null)`
   - Applied to `tools` and `stop` in computeCacheKey
   - Now `tools: []` and `tools` omitted produce IDENTICAL cache
     keys (and same for `stop: []` vs omitted). ADR 0005 Amendment 2's
     own claim that "[] and undefined share a cache entry" was
     empirically FALSE pre-D34; round-6 reviewer verified hashes
     differ. The fix makes the claim literally true at the
     key-composition layer.

2. lib/providers/base.mjs (+16/-?) — F7 P2 dead error code removal:
   - `QUOTA_EXHAUSTED` removed from PROVIDER_ERROR_CODES
   - `RATE_LIMITED` removed from PROVIDER_ERROR_CODES
   - v0.1 live codes: SPAWN_FAILED, CLI_NOT_FOUND, AUTH_MISSING,
     SPAWN_TIMEOUT
   - Comment block documents removal + cites ADR 0004 Amendment 3

3. lib/fallback/engine.mjs (+29/-?) — F7 P2 sibling:
   - HARD_TRIGGER_CODES: QUOTA_EXHAUSTED + RATE_LIMITED removed
   - SPAWN_FAILED, CLI_NOT_FOUND, AUTH_MISSING(false), SPAWN_TIMEOUT
     remain
   - evaluateHardTriggers HTTP-status branches KEPT (option (b)) with
     forward-compat comment: "v0.1 plugins never attach statusCode;
     branches reserved for v1.x when plugin gains HTTP-status parsing"

4. test-features.mjs (+93) — F4 + F7 test work:
   - 4 new F4 regression tests (tools:[] vs undefined, stop:[] vs
     undefined, tools:[] vs null, tools:non-empty vs undefined sanity)
   - ~14 integration test code-swap edits (QUOTA_EXHAUSTED →
     SPAWN_FAILED, RATE_LIMITED → SPAWN_FAILED) preserving original
     hard-trigger semantic
   - 2 dead unit tests for QUOTA_EXHAUSTED/RATE_LIMITED removed
     (tombstone comment retained for audit trail)

**ADR amendments (docs-only, no code change)**

5. docs/adr/0004-fallback-engine.md (+12) — F7 Amendment 3:
   - Documents the v0.1 hard-trigger code narrowing
   - 4 live codes listed explicitly
   - Captures evaluateHardTriggers HTTP-status branch retention rationale
   - v1.x re-activation path: plugin gains HTTP-status parsing →
     re-add codes → branches activate naturally

6. docs/adr/0005-cache-cross-provider.md (+21) — TWO amendments + 1
   prior-amendment update:
   - **Amendment 6 (F1 P1)**: Formal v1.x deferral of D4 streaming
     singleflight. Buffered path (executeHopFn) uses cacheStore.getOrCompute
     and participates in D4 fully. Streaming cache-miss path
     (server.mjs:609-741) bypasses singleflight — N concurrent identical
     streamers each spawn fresh. v0.1 trade-off accepted for
     personal/family scale; v1.x design ADR needed for tee-streaming +
     per-key inflight Map. Cross-references CLAUDE.md release_kit.
     phase_rolling_mode as the deferral pattern precedent.
   - **Amendment 7 (F8 P2)**: Documents the v0.1 conservative cache-key
     posture: includes all IR fields including those plugins discard
     (anthropic/codex/mistral drop temperature/max_tokens/top_p/stop/
     tools/tool_choice at spawn). Consequence: 2 requests with different
     temperature produce identical CLI output (CLI ignores) but
     different cache keys → spurious miss. Trade-off justified:
     spurious miss > spurious hit. v1.x forward path:
     per-plugin cacheKeyFields contract extension (ADR 0002 amendment
     needed). 3 implementation subtasks enumerated for the v1.x PR.
   - **Amendment 2 update (F4)**: heading renamed to "Note on
     null-coalescing AND array normalization"; body documents the
     new normalizeArrayField helper; quotes the regression test name.

Tests: 414 → 416 (+4 F4 regression, -2 F7 dead, +0 net from F7
integration rewrites).

Pre-commit fold-in: NONE — D34 reviewer APPROVE with all 4 suggestions
non-blocking/cosmetic.

Authority:
- ADR 0005 Amendment 2 invariant restored at code level (F4)
- ADR 0005 Amendment 6 formalizes F1 streaming-singleflight deferral
  per the same pattern as D22 ADR 0004 Amendment 2 soft-trigger
  deferral
- ADR 0005 Amendment 7 documents F8 conservative posture as v0.1
  intentional design (not an accident)
- ADR 0004 Amendment 3 narrows v0.1 trigger taxonomy (F7)
- Round-6 cold audit findings F2 / F3 / F6 / F9 / F10 / F11 / F12 /
  F13 / F14 filed as GitHub issues after this commit (NOT in scope)
- CC 开发铁律 v1.6 § 10.x — final round of the audit cadence

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Critical depth checks:
- B5 over-normalization: verified `normalizeArrayField` only applies
  to `tools` and `stop`; `response_format: {}` and `tool_choice: ''`
  unaffected (Array.isArray guard)
- C10 test cleanup: 21 references reconciled (3 tombstone, 18
  rewrites/removals); integration test rewrites preserve hard-trigger
  semantics (QUOTA_EXHAUSTED → SPAWN_FAILED is also a hard trigger,
  so fallback advancement behavior unchanged)

---

**End of audit cycle.** 24 D-days shipped from D10 (P1 hardening) through
D34 (final batch). 6 cold-audit rounds executed; 78+ findings raised;
~50 closed via implementation; ~28 deferred to GitHub issues / v1.x
ADR amendments. v0.1 tag remains explicit-maintainer-action per
phase_rolling_mode policy.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 19:33:48 +10:00
taodengandClaude Opus 4.7 f784fdb947 fix+docs: D33 — round-5 cleanup batch (F1/F3/F5/F8/F9/F10/F11/F12) 
cold-audit catch from 2026-05-24 (round 5)

Round-5 cold-audit cleanup batch. 8 items + 1 release-discipline
reconciliation. Largest batch by line count (582+/39-) but every item
is small-and-focused. 3 P2 items (F1/F3/F5 of which F3 + F1 are real
correctness/observability fixes; F5 backfills /health to spec).

Changes (10 files, +583/-39):

**P2 fixes**

1. **F1 — ALIGNMENT.md mistral authority pin self-contradicted plugin**
   (ALIGNMENT.md): row cited `vibe --prompt --output json` but mistral.mjs
   uses `--output streaming` (the plugin header at lines 360-369 even
   justifies WHY: `--output json` emits single blob, breaks NDJSON
   line-buffered parser). Constitution self-contradicting itself —
   missed across 4 prior rounds. Pin updated to `--output streaming`
   with DOCS-1 reference.

2. **F3 — Deterministic function_call synth ID**
   (lib/ir/openai-to-ir.mjs): deprecated `function_call` translation
   produced `id: \`fc-${Date.now()}\`` → ID flows into normalized
   tool_calls → cache key SHA-256. Two identical requests separated
   by ≥1ms → different cache keys → cache always misses for
   `function_call` request shape. Violates ADR 0005 invariant
   "same inputs → same key, no random, no timestamp."

   Fixed: id is now `fc-<16-hex>` from SHA-256 of `${name}\0${arguments}`.
   NUL separator prevents the (name='ab',args='c') vs (name='a',args='bc')
   collision. 2^64 collision resistance is more than sufficient for
   tool_call ID disambiguation (per-request semantic key, not crypto
   primitive).

**P3 fixes**

3. **F5 — /health invokes per-plugin healthCheck()** (server.mjs +
   docs/openai-spec-pin.md): ADR 0002 says "healthCheck — startup AND
   /health endpoint use this." Pre-D33 /health returned only
   {enabled, available} counts. Now async, iterates loadedProviders,
   awaits each plugin's healthCheck() in try/catch. Returns
   `providers: {enabled, available, status: {<name>: {ok, latencyMs?, error?}}}`.

4. **F8 — X-OLP-Cache reports fallback-hop cache hits** (server.mjs):
   pre-D33 cacheStatus computed from `preCheckHit && fallbackHops === 0`
   — only counted primary-hop cache hits. When fallback fires + the
   fallback hop's getOrCompute returns from cache, header reported
   `miss` despite no spawn happening.

   Fixed: peek BEFORE getOrCompute inside executeHopFn, set
   `lastHopWasCached` closure variable on every hop (last-write-wins
   = serving hop's state). cacheStatus combines
   `lastHopWasCached || (preCheckHit && fallbackHops === 0)`.

   F8 chose option (b) peek-then-getOrCompute over option (a)
   getOrCompute API change because option (a) would break ~15 test
   callsites for marginal benefit. Accepted race window same as
   existing preCheckHit pattern.

5. **F9 — validateProvider hints error message updated** (lib/providers/
   base.mjs): pre-D33 message listed cacheable as missing and
   maxSpawnTimeMs as required. Now: `'hints must be an object with
   { requiresTTY, concurrentSpawnSafe, maxConcurrent } + optional
   { maxSpawnTimeMs, cacheable }'`.

6. **F10 — Dead cache-write branch removed** (server.mjs): the
   `if (hasStopChunk)` check in the streaming stop-less exhaustion
   branch was unreachable (the stop-chunk completion path returns
   earlier inside the for-await loop). Removed the dead code + added
   a comment documenting the invariant.

**Governance/policy**

7. **F11 — Phase rolling mode policy formalized** (CLAUDE.md +
   CHANGELOG.md): 22+ D-day commits accumulated under "Unreleased"
   without per-D version bumps — Iron Rule 5 (release-kit bump-before-
   push) appeared to be silently violated. Reality: per-D bumps would
   produce 30+ noise tags during Phase 1. F11 formalizes the policy:
   intra-Phase D-day commits accumulate under Unreleased; bump+tag
   fires explicitly at Phase close (maintainer-triggered, not
   automated). CLAUDE.md release_kit overlay gains `phase_rolling_mode`
   block documenting the exception with self-pointer ("if Rule 5
   appears silently violated, check this section first"). CHANGELOG
   "Unreleased" gets a notice at top.

   **No version bump, no git tag in D33** — policy formalization only.

8. **F12 — /v1/models created is stable per-model timestamp**
   (models-registry.json + lib/providers/index.mjs + server.mjs +
   docs/openai-spec-pin.md): pre-D33 used Math.floor(Date.now()/1000)
   per request — violates OpenAI spec which treats `created` as
   per-model attribute. Clients caching models by created would see
   spurious updates on every poll.

   Fixed: models-registry.json gains `bootstrapCreated: 1778630400`
   top-level constant + per-model `created` fields where known
   (anthropic claude-{opus,sonnet,haiku} with estimated release dates;
   devstral models from "25-12" suffix; codex models pinned to
   bootstrap pending verified release dates). handleModels uses
   `getModelCreated(modelId)` helper from lib/providers/index.mjs.
   Aliases share canonical's timestamp.

**Tests** (test-features.mjs): 401 → 414 (+13):
- F3 ×3 (same input → same id → same cache key; different name → different)
- F5 ×4 (empty/single/multi/throwing-plugin /health shapes)
- F8 ×1 (2-hop primary-fail + secondary-cache-hit → X-OLP-Cache: hit)
- F12 ×5 (stability/fallback/alias-equals-canonical)

Pre-commit fold-in (per evidence-first checkpoint #4):
- **D33 reviewer flagged F3 empty-args asymmetry** (Concern #1): hash
  input used `?? ''` (empty stays) but emitted IR field used
  `|| '{}'` (empty becomes '{}'). Consequence: `arguments: ''` and
  `arguments: '{}'` emit identical IR but compute different ids →
  different cache keys for semantically-identical requests. The exact
  cache-stability bug F3 was supposed to fix.

  Folded in: canonicalize empty-args to '{}' BEFORE hashing. Hash
  input now matches IR emission exactly. Same line change resolves
  the asymmetry.

Authority:
- ALIGNMENT.md self-amendment (F1 pin correction)
- ADR 0005 invariant "same inputs → same key, no random, no timestamp"
  (F3 restoration)
- ADR 0002 § Provider contract "/health uses healthCheck" (F5)
- ADR 0004 § Observability headers (F8 X-OLP-Cache correctness)
- ADR 0005 § Cache write conditions item 1 (F10 truncation-not-cached
  invariant explicit)
- Iron Rule 5 (F11 release-kit reconciliation)
- OpenAI /v1/models spec — `created` per-model stable (F12)
- CC 开发铁律 v1.6 § 10.x — Round-5 Cold Audit caught all 8

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Verified:
- F1 plugin cross-reference (mistral.mjs:360-369) accurately documents
  the rationale
- F3 collision resistance + NUL separator + restored cache invariant
- F5 all 4 cases (empty/single/multi/throwing) work
- F8 closure semantics across multi-hop chains (verified hop-fail +
  fallback-hit case)
- F10 dead code removal preserves the stop-chunk completion path
- F11 phase_rolling_mode policy honest about what happened and what
  the going-forward rule is
- F12 stability across consecutive /v1/models calls; alias-canonical
  parity
- 414/414 tests pass

3 remaining non-blocking suggestions (F3-vs-modern-tool_calls path
canonicalization symmetry; F12 codex models explicit-vs-fallback
writeup mismatch; F8 servingHopWasCached naming) tracked as future
polish; not folded.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 19:12:03 +10:00
taodengandClaude Opus 4.7 30de965e8e fix+docs: D32 — round-4 cleanup batch (F2/F3/F4/F5/F7/F8/F9) 
cold-audit catch from 2026-05-24 (round 4)

Round-4 cold-audit cleanup batch. 7 items grouped per IDR cleanup-batch
convention (D19/D20/D25/D26/D30/D31 precedent). 2 P2 + 3 ADR amendments
+ 2 small docs/code cleanups. F10 (P3 semantic edge case) filed
separately as GitHub issue #8.

Changes (8 files, +200 / -38):

**P2 fixes**

1. **F3 — README missing 6 provider auth env vars** (README.md):
   D30 fixed the `OLP_*` table but missed the auth-bearing env vars
   actually read by plugin code:
   - `CLAUDE_CODE_OAUTH_TOKEN` (anthropic.mjs:84) — highest-precedence
     override; bypasses keychain + .credentials.json file lookup
   - `OPENAI_CODEX_AUTH_PATH` (codex.mjs:163) — overrides full auth
     file path; when set, no other path tried
   - `CODEX_HOME` (codex.mjs:176) — overrides base dir; default auth
     path becomes `$CODEX_HOME/auth.json`
   - `MISTRAL_API_KEY` (mistral.mjs:260) — directly supplies API key;
     highest precedence per Mistral DOCS-2
   - `MISTRAL_VIBE_AUTH_PATH` (mistral.mjs:265) — overrides full .env
     path; evaluated only when MISTRAL_API_KEY absent
   - `VIBE_HOME` (mistral.mjs:277) — overrides Vibe base dir; default
     auth path becomes `$VIBE_HOME/.env`
   Real onboarding-blocker fix: new users couldn't run OLP without
   knowing about these env vars; README now documents them in a
   "Per-provider auth env vars" subsection.

2. **F8 — Early-return error paths missing X-OLP-* headers** (server.mjs):
   ADR 0004 § Observability + README claim "every response carries the
   5 X-OLP-* headers" but 503 (no-enabled-providers), 415 (wrong
   Content-Type), and early 400s (bad JSON, IR validation) emitted only
   Content-Type + Content-Length + X-OLP-Latency-Ms (D18 only wired the
   chain-exhausted path). Operators debugging these paths got less info
   than the ADR promised.
   New helper `olpErrorHeaders({ startMs, model })` emits the canonical
   "no provider attempted" defaults:
     X-OLP-Provider-Used: 'none'
     X-OLP-Model-Used: model ?? 'unknown'
     X-OLP-Fallback-Hops: '0'
     X-OLP-Cache: 'bypass'
     X-OLP-Latency-Ms: <delta>
   6 sendError call sites updated: 415 / 400-bad-JSON / 400-IR-parse
   (model: undefined → 'unknown') + 503-no-providers / 503-provider-
   disappeared / 500-engine-programming (model: ir.model). The 502
   streaming-error-before-first-chunk path correctly stays on
   `olpHeaders(...)` since a provider WAS attempted.

**ADR amendments**

3. **F2 — ADR 0003 model-mapping example correction** (docs/adr/0003,
   Amendment 2): the § Required fields example claimed `claude-sonnet-4-6`
   → `claude-sonnet-4-6-20260301` mapping happens in the provider plugin.
   This was wrong: per D17 SPOT decision (commit cb86807), `irRequest.model`
   is passed verbatim to the CLI; each provider CLI accepts its own
   aliases natively. Both inline correction (in § Decision) AND new
   Amendment 2 (in § Amendments) added.

4. **F4 — OUTPUT_PARSE_ERROR dead code removal** (base.mjs, engine.mjs):
   `PROVIDER_ERROR_CODES.OUTPUT_PARSE_ERROR` and
   `HARD_TRIGGER_CODES.OUTPUT_PARSE_ERROR = true` were registered but
   ZERO plugins emit OUTPUT_PARSE_ERROR. ADR 0004 § Trigger taxonomy
   enumerates 4 hard-trigger categories (5xx, quota 4xx, exit-code,
   spawn-timeout) — OUTPUT_PARSE_ERROR was a 5th never authorized by
   ADR. Removed from both enums with removal-reason comments for
   auditability. Re-add via ADR 0004 amendment if a future plugin
   surfaces parse failures (cheaper than authoring an amendment for
   dead code now).

5. **F5 — ADR 0002 Amendment 4 ratifying contractVersion** (docs/adr/0002):
   `lib/providers/base.mjs:76-81` enforces `p.contractVersion === '1.0'`
   and all 3 plugins declare it, but ADR 0002 § Provider contract field
   list never named it. Same governance-violation class as D11's
   `maxSpawnTimeMs` retroactive sync (Amendment 1). Amendment 4 ratifies
   contractVersion as a required v1.0 contract field; § Provider contract
   field list updated to 10 fields (was 9).

**Small cleanups**

6. **F7 — README API Endpoints table 📋 markers** (README.md): added
   Status column with ✅ Shipped (3 rows: /v1/chat/completions, /v1/models,
   /health) and 📋 Planned (3 rows: /cache/stats Phase 5, /v0/management/
   quota Phase 6, /dashboard Phase 6). Matches D20's Implementation
   Status table convention.

7. **F9 — Codex parser inline assumption labels** (codex.mjs): added 5
   inline `// A4:` comments on each defensive branch in `codexChunkToIR`.
   Pre-D32 these branches had only the top-of-function block comment;
   ALIGNMENT.md Speculative-Candidate condition 5 promises future
   implementers can grep assumption labels to find every speculative
   branch — D32 honors that promise for codex.mjs (mistral.mjs already
   had this pattern from D8).

**Tests** (test-features.mjs):
- 17g retitled + assertion expanded to full 5-header set (was partial)
- 17h new: 415 wrong Content-Type → 5 X-OLP-* headers with 'unknown' model
- 17i new: 503 no-enabled-providers → 5 X-OLP-* headers with ir.model
- OUTPUT_PARSE_ERROR test removed (replaced with comment for audit trail)

Test count: 400 → 401 (-1 OUTPUT_PARSE_ERROR + 2 new F8 + 1 retitled
existing = net +1).

**F10 filed as issue, NOT in D32**:
https://github.com/dtzp555-max/olp/issues/8 — "Soft-skip + chain-
exhausted: X-OLP-Provider-Used semantics ambiguity". Semantic edge
case requiring soft trigger reactivation (deferred to v1.x per D22
Amendment 2); not a v0.1 user-affecting issue.

Pre-commit fold-in (per evidence-first checkpoint #4):
- **D32 reviewer caught a doc-accuracy bug**: README's
  CLAUDE_CODE_OAUTH_TOKEN row described the no-env fallback chain as
  "Searches keychain first, then `~/.claude/.credentials.json`" — but
  the actual code in anthropic.mjs:82-112 checks file FIRST, then
  keychain (darwin-only). Order was reversed. Folded in: corrected
  to "Searches `~/.claude/.credentials.json` first, then macOS
  keychain (darwin only)". Same class of doc-accuracy mistake as the
  D25 → D31 / D27 → D31 pattern: D-batch reviewer catches docs that
  contradict source.

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Independently verified:
- All 6 F3 env vars: precedence chains traced through plugin source
  (caught the anthropic reversed-order — folded in)
- F8 olpErrorHeaders helper correctly distinguishes "no provider"
  defaults from olpHeaders' "provider attempted" defaults
- F8: all 6 sendError call sites use the right model arg (undefined
  pre-IR-parse → 'unknown'; ir.model post-IR-parse)
- F8: 502 streaming-error stays on olpHeaders (provider was attempted)
- F4: OUTPUT_PARSE_ERROR removed from both enums, zero plugin emit
  references remain, test cleanup is auditable
- F5: ADR 0002 Amendment 4 matches Amendment 1's retroactive-sync
  structure; contract field list now 10 items
- F2: D17 commit cb86807 verified to match the SPOT decision claim
- F7/F9: docs/code cleanups verified
- F10 issue #8 verified OPEN with correct title
- 401/401 tests pass

3 non-blocking suggestions (anthropic precedence column folded; F2
inline mistral `--model` caveat could be added; F4 wording asymmetry
between base.mjs and engine.mjs comments) — minor polish, not folded.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 17:28:42 +10:00
taodengandClaude Opus 4.7 c3ba751a8f feat: D27 — round-3 P3 batch (F8 IR validator + F10 ADR amend + F15 /v1/models aliases) 
cold-audit catch from 2026-05-24 (round 3)

3 round-3 P3 items batched per IDR. Mixed surfaces but all single-severity
and conceptually independent.

Changes (5 files, +324 / -17):

1. lib/ir/types.mjs — F8 validator extension (+28):
   - `response_format`: must be object with string `.type` (undefined/omitted
     accepted; null, non-object, missing-type all rejected). Forward-compat:
     accepts any string for type so future OpenAI additions (json_schema,
     etc.) flow through without schema bump.
   - `tool_choice`: string form 'auto'/'none'/'required' OR object form
     `{type:'function', function:{name:string}}`. All other shapes rejected.
   Pre-D27 the validator silently accepted any value; the Anthropic plugin's
   `if (irRequest.response_format?.type === 'json_object')` would no-op on
   a malformed string payload.

2. docs/adr/0005-cache-cross-provider.md — F10 Amendment 4 (+8):
   - Documents the IR-vs-body detection ambiguity in § D2: the ADR text
     reads as if detection happens on the IR, but `openai-to-ir.mjs` strips
     `cache_control` from messages per ADR 0003's whitelist policy, so
     IR-side detection is structurally always empty at v1.0
   - Documents the actual v1.0 detection mechanism (server.mjs side-channels
     into the raw body)
   - Documents the cache key `cache_control` slot's always-null status as
     forward-compat (when a future ADR 0003 amendment adds cache_control
     to IR, the slot will start carrying meaningful data without schema bump)
   - Explicit "no code change" — F10 is docs-only

3. lib/providers/index.mjs — F15 alias map export (+11):
   - `getAliasMap()` returns `new Map(_aliasMap)` — defensive copy preventing
     caller mutation of the module-private alias map
   - JSDoc documents use case + defensive-copy intent

4. server.mjs — F15 /v1/models alias surfacing (+25/-7):
   - `handleModels` now emits TWO loops: canonical entries first (preserves
     existing client expectations), then alias entries via `getAliasMap()`
   - Each alias entry has the same 4 OpenAI-spec fields (id/object/created/
     owned_by) — no invented fields per ALIGNMENT Rule 2(b)
   - Disabled-provider alias non-leak: `loadedProviders.has(providerName)`
     gate skips aliases whose target provider is not currently enabled
   - createdTs reused (same per-request timestamp across all entries)
   - JSDoc updated to document new ordering + F15 origin

5. test-features.mjs — +269 / +18 new tests:
   - F8: 13 tests covering response_format (object/string/non-object/missing-
     type) + tool_choice (string variants/object variants/wrong type/no name)
   - F15: 5 tests covering canonical-first ordering, alias presence/count,
     disabled-provider non-leak (with anthropic-only enabled, mistral and
     openai aliases must NOT appear), Rule 2(b) shape conformance

Tests: 358 → 376 (+18). All pass on Node 20.

Reviewer notes (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Critical checks verified:
- F8: all 9+ tool_choice rejection axes traced through code (including
  partial-function-object edge cases). The code handles them correctly even
  where tests don't exercise (non-blocking gap).
- F10: amendment substantively correct. One wording-precision note: the
  amendment says "body only" but the actual code is an OR-disjunction
  (hasCacheControl(ir) || extractCacheControlMarkers(body.messages)).
  Functionally identical at v1.0 because IR strips markers, so first
  disjunct is always false. Forward-compat by construction.
- F15: disabled-provider non-leak verified by manual trace with
  anthropic-only enabled — mistral/openai aliases correctly filtered out
  by `loadedProviders.has(providerName)` gate. Test 17e explicitly
  asserts this.
- F15+D17 round-trip: client GET /v1/models → sees alias entry → POSTs
  with alias → getProviderForModel resolves via same _aliasMap → cache
  key uses canonical → response works. Both surfaces read the same Map
  (SPOT).
- F15+D23 cacheable interaction: cacheable opt-out doesn't suppress
  alias surfacing in /v1/models — cacheable is about cache-write behavior
  while discovery should still surface enabled providers. Intentional.

Authority:
- ADR 0003 § Optional fields (response_format + tool_choice IR shape)
- OpenAI Chat Completions API spec (the field semantics)
  https://platform.openai.com/docs/api-reference/chat/create
- ADR 0005 § D2 + Amendment 4 (F10's own amendment landing here)
- ADR 0002 § Loading model + D17 getProviderForModel SPOT (F15's alias
  origin)
- ALIGNMENT.md Rule 2(b) — no invented OpenAI fields
- CC 开发铁律 v1.6 § 10.x — Round-3 Cold Audit caught all 3

Follow-up items (reviewer's non-blocking suggestions, NOT in this PR):
- F8 micro-tests for null/boolean/partial-function inputs (code handles;
  test gap only)
- F10 wording precision on OR-disjunction
- Nested-describe wrapper quirk in test-features.mjs (pre-existing
  structural issue; D26/D27 describes are children of Suite 16 wrapper).
  Cleanup in a future hygiene pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 16:22:30 +10:00
taodengandClaude Opus 4.7 a281d3e424 fix: D26 — round-3 small batch (F16+F17+F18+F19) 
cold-audit catch from 2026-05-24 (round 3)

4 small fixes batched per Iron Rule 11 IDR cleanup-batch convention.
None exceeds ~15 lines; mixed surfaces (server.mjs / 2 plugins /
plugin header / tests) but all P3-class and related by being honesty
fixes to claims the ADR/spec made but code didn't honor.

Changes (5 files, +546 / -13):

1. server.mjs — TWO changes:
   - **F16 (startup warning)**: `logEvent` function moved earlier in
     the file (was at line ~124, now at line ~56) so it's defined
     before `_startupConfig` is loaded. After loading, check
     `_startupConfig.soft_triggers` — if non-empty, emit
     `logEvent('warn', 'soft_triggers_deferred_v1x', ...)` with the
     configured provider names + a descriptive message citing ADR
     0004 Amendment 2. Wires the mitigation that ADR 0004 § Mitigations
     (original pre-Amendment-2 text) requires: "Soft triggers configured
     against such providers issue a startup warning so the user knows
     the trigger will never fire." D22 deferred soft triggers but
     didn't re-implement this mitigation.
   - **F19 (streaming truncation marker)**: in the stop-less exhaustion
     branch of the real-streaming code path (post-D25 F9 — the no-cache
     fix), write a synthetic `{type:'stop', finish_reason:'length'}`
     SSE chunk via `irChunkToOpenAISSE(...)` BEFORE `res.write(SSE_DONE)`.
     Guarded on `streamedChunks.length > 0` (emitting truncation on
     a zero-content response would mislead the client). Mirrors the
     buffered D16 path which already synthesizes the same marker.
     The synthetic marker is `res.write`-only — NEVER pushed to
     `streamedChunks` — so the D25 F9 no-cache invariant
     (cache decision sees the original chunks array unchanged) is
     structurally preserved.

2. lib/providers/codex.mjs + mistral.mjs — F17 stderr propagation:
   the SPAWN_FAILED throw from the error-chunk-in-NDJSON path now
   includes `accumulatedStderr.slice(0, 200)` suffix when non-empty.
   Comment cites ADR 0004 § Chain advancement step 4 ("preserve the
   client's ability to debug — the first failure is the load-bearing
   signal").

   **anthropic.mjs intentionally NOT modified**. Re-analysis confirmed
   Anthropic's SPAWN_FAILED throws are from (a) process.on('error')
   for binary-not-found (already includes OS-level err.message),
   (b) SPAWN_TIMEOUT (separate code, D24 race fix), and (c) post-loop
   exit-code (already includes `accumulatedStderr.slice(0, 300)`).
   Anthropic uses `--output-format text`, so it has no NDJSON
   error-chunk parsing path. The cold-audit was correct that the
   error-chunk class only affects codex + mistral.

3. lib/providers/anthropic.mjs — F18 plugin header self-contained
   D4 observation note. Pre-D26: ALIGNMENT.md anthropic Authority pin
   row cited the plugin header for the OLP-side observation, and the
   plugin header cited ALIGNMENT.md back — circular citation, no
   party documented a fresh observation. Fix: plugin header now
   carries the actual observation ("@anthropic-ai/claude-code v2.1.89
   confirmed present at D4 implementation per ALIGNMENT.md Rule 5").
   ALIGNMENT.md side unchanged (still points to plugin header — but
   now points to a real artifact, not back to itself).

4. test-features.mjs — 9 new tests in 3 describe blocks:
   - F16 ×3: soft_triggers non-empty → warn fires; empty → no warn;
     undefined → no warn. Test uses inline simulation of the startup
     code path (ESM module-eval can't be re-triggered per test process
     without isolation gymnastics; inline simulation exercises the
     same `Object.keys + length > 0 + logEvent('warn', ...)` shape).
   - F17 ×3: codex with stderr → stderr appears in throw message;
     codex without stderr → no suffix; mistral with stderr → suffix
   - F19 ×3: partial-content + stop-less exhaustion → finish_reason='length'
     marker before [DONE]; zero-content + stop-less exhaustion →
     no marker (just [DONE]); D25 F9 invariant preserved (second
     identical request triggers fresh spawn, X-OLP-Cache: miss)

Tests: 349 → 358 (+9). All pass on Node 20.

Authority:
- F16 → ADR 0004 § Mitigations (original) + ADR 0004 Amendment 2 (D22)
- F17 → ADR 0004 § Chain advancement step 4
- F18 → ALIGNMENT.md Rule 1 (Cite First) + Rule 5 (Cite in Commits)
- F19 → OpenAI Chat Completions streaming spec finish_reason enum
  https://platform.openai.com/docs/api-reference/chat/streaming
- CC 开发铁律 v1.6 § 10.x — Round-3 Cold Audit caught all 4

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Verified critical concerns: (a) logEvent move is
net-zero (1 removed + 1 added; no duplicate); (b) F17 anthropic
non-application is justified by code-reading 3 SPAWN_FAILED sites in
anthropic.mjs (none is the NDJSON-error-chunk class); (c) F18 citation
chain no longer circular (verified BOTH endpoints); (d) F19 critical
no-cache invariant preserved (truncMarker is res.write-only, never
enters streamedChunks; verified both by code-path analysis and by F19
test 3 behavioral assertion).

3 non-blocking suggestions noted (F18 copy redundancy; F16 test linking
comment; F17 stderr slice-depth alignment) — all cosmetic, not folded.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 16:04:49 +10:00
taodengandClaude Opus 4.7 7ef5510837 feat(cache): D23 — implement hints.cacheable + 10MB size cap (round-2 F3) 
cold-audit catch from 2026-05-24

Round-2 cold-audit Finding 3 (P2 cache correctness). ADR 0005 § "Cache
write conditions" items 3 and 4 were documented but never wired in
code:
- Item 3: "The provider's hints.cacheable flag is not false"
- Item 4: "The response is below a size cap (default 10 MB; configurable)"

Grep verified zero matches for `cacheable` / `10485760` / size-cap
patterns in lib/ or server.mjs pre-D23.

Changes (9 files, +391 / -12):

1. docs/adr/0002-plugin-architecture.md — Amendment 3 adds `cacheable`
   to the Provider contract hints list (after D11's Amendment 1 added
   maxSpawnTimeMs). Authority chain cites ADR 0005 § Cache write
   conditions item 3 as the field's origin.

2. docs/adr/0005-cache-cross-provider.md — Amendment 3 documents the
   D23 implementation of items 3 + 4 + the D16-interaction edge case
   (truncated > 10MB → no-op eviction, structurally bounded since
   responses > 10MB are anomalous by ADR's own rationale).

3. lib/providers/base.mjs — ProviderHints typedef gains
   `[cacheable]` (optional boolean); validateProvider rejects non-
   boolean non-undefined values. Omission accepted (default = true).

4. 3 plugins (anthropic / codex / mistral) each declare
   `cacheable: true` explicitly with citation comment.

5. lib/cache/store.mjs — CacheStore constructor accepts
   `maxEntryBytes` (default 10 * 1024 * 1024 = 10_485_760) +
   injectable `_warnFn`. `set()` computes
   `Buffer.byteLength(JSON.stringify(value))`; if exceeded, warns via
   `_warnFn` and returns undefined (no persistence). `getOrCompute`
   still returns the computed value to caller — cache write skipped
   but caller gets data; subsequent identical requests re-spawn.

6. server.mjs — 4 sites coordinated for cacheable opt-out:
   - `executeHopFn`: cacheable check before D13 shouldBypassCacheForHop
     (permanent provider policy precedes per-request bypass condition)
   - `cacheStore.peek` gate at line ~504: `cacheableForFirstHop`
     short-circuit
   - Real-streaming branch entry condition at line ~522:
     `cacheableForFirstHop` added (so cacheable: false + stream falls
     through to buffered path which honors the opt-out via executeHopFn)
   - Both `cacheStore.set` sites in streaming branch wrapped in
     `if (cacheableForFirstHop)` defensive guards (post-D23
     restructure these are unreachable for cacheable: false, but the
     guards make intent explicit and survive future refactors)

7. test-features.mjs — 13 new tests:
   - 5 validator tests (Suite 4): explicit true/false, omitted, string
     rejected, number rejected
   - 5 size-cap unit tests (Suite 9): default 10MB, custom override,
     oversize skip + warn capture, within-limit normal persistence,
     getOrCompute oversize returns-but-doesn't-cache + re-spawn
   - 3 cacheable integration tests (Suite 9e): non-streaming opt-out,
     streaming opt-out (the regression case that pre-fold-in failed),
     X-OLP-Cache header consistency on both paths

Tests: 335 → 348 (+13). All pass on Node 20.

Pre-commit fold-in (per evidence-first checkpoint #4):

- **D23 reviewer flagged 2 blocking issues**: (1) the cacheable opt-out
  in initial implementation was only in `executeHopFn` (buffered path);
  the D10 real-streaming branch in server.mjs bypassed the check
  entirely — calling streamPlugin.spawn() directly and writing to
  cacheStore.set() at 2 sites without consulting cacheable. (2) Suite
  9e integration tests didn't cover stream: true so the leak wasn't
  caught.

  Both diff-review and the implementer focused on `executeHopFn`
  because that's where the cold-audit reviewer pointed for Finding 3.
  Same class of "narrow attention" miss as several earlier D-days.

  Fold-in: compute `cacheableForFirstHop` once at request entry; add
  `!cacheableForFirstHop` short-circuit to peek gate; add
  `cacheableForFirstHop` to streaming-branch entry condition (forces
  fall-through to buffered path which has the opt-out); add defensive
  guards on both `cacheStore.set` call sites. Added a 3rd Suite 9e
  test covering stream: true + cacheable: false (which pre-fold-in
  would have failed by serving the second request from cache).

  This is now the FOURTH D-day where a doc-vs-code or path-coverage
  gap was caught by the reviewer rather than the implementer. The
  v1.6 § 10.x diff-review discipline continues to pay off.

Default behavior unchanged for 3 shipped plugins (all explicitly
`cacheable: true` → cache path identical to pre-D23).

Authority:
- ADR 0002 Amendment 3 (in-place) — establishes cacheable in contract
- ADR 0005 Amendment 3 (in-place) — documents implementation of items
  3 + 4
- ADR 0005 § Cache write conditions items 3 + 4 — the original
  authority for both rules
- CC 开发铁律 v1.6 § 10.x — Round-2 Cold Audit caught the missing
  implementation; diff-review Mode A caught the streaming-path gap

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): REQUEST_CHANGES on initial, APPROVE after fold-in (implicit
— fold-in followed the exact recommendation). Verified:
- ADR amendment placement + structure
- Validator typedef + checks
- Size cap implementation in CacheStore + inflight slot release on
  oversize-skip
- All 4 interaction cases (cacheable × cache_control × D16
  × ordering) coherent post-fold-in
- 13 new tests including the regression test that would have failed
  on pre-fold-in code

Follow-up items (reviewer's non-blocking notes, NOT in this PR):
- ADR 0005 Amendment 3 could add one sentence on the prior-write-also-
  oversize case (file as docs polish)
- Consider extracting `shouldUseCacheForHop(hopProvider, ir)` helper
  combining D13 + D23 logic — reduces miss-risk for next reviewer
- Test 30 could add `assert.equal(store._inflight.size, 0)` as
  inflight-slot leak regression guard

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 14:45:08 +10:00
taodengandClaude Opus 4.7 f8348adb3b fix(providers): D24 — close spawn-timeout race when timer fires during yield-suspension (round-2 F4) 
cold-audit catch from 2026-05-24

Round-2 cold-audit Finding 4 (P2 concurrency bug). All 3 provider plugins
(anthropic / codex / mistral) had a race in spawn-timeout enforcement:

The timer's rejection branch is `if (rejectNext) { rejectNext(SPAWN_TIMEOUT) }`.
`rejectNext` is only set while the drain loop is AWAITING an empty-queue
promise. If the timer fires while the generator is suspended at a `yield`
(rejectNext === null because we're not currently awaiting), the rejection
is silently skipped. The loop then drains queued items, sees `close`
(from the SIGTERM), breaks normally. Post-loop guards on `!spawnTimedOut`
both skip: SPAWN_FAILED throw + yield-stop emit. Generator returns
normally with N partial chunks. Consumer (collectAllChunks) sees a
"successful" return. Fallback never advances. Truncated response gets
cached.

This violates BOTH:
- ADR 0004 § Trigger taxonomy bullet 4 (hard trigger never fires)
- ADR 0005 § Cache write conditions item 1 (truncated response cached)

The bug escaped FOUR reviewers: D10 diff-review, round-1 cold-audit, D11
reviewer, D14 reviewer. Round-2 cold-audit caught it because it was
unprimed and walked the timer/drain-loop race manually.

Fix (3 plugins, parallel race):

Each plugin gains an unconditional post-loop check, placed AFTER the
existing `try { drain loop } finally { clearTimeout(timer) }` block,
but BEFORE the existing `!spawnTimedOut`-gated guards:

```js
if (spawnTimedOut) {
  throw new ProviderError(
    `<cli-name> spawn timed out after ${maxSpawnTimeMs}ms`,
    'SPAWN_TIMEOUT',
  );
}
```

This closes the race surface: the inner-loop guard (unchanged, lines
~319/461/583) handles the rejectNext-set path; the new post-loop guard
handles the rejectNext-null path. Together they cover both halves —
SPAWN_TIMEOUT now reliably surfaces as a hard trigger regardless of
which path the timer fire took.

Changes (4 files, +154 / -0):
- lib/providers/anthropic.mjs: post-loop guard at lines 360-365 (after
  clearTimeout at line 348-350, before existing SPAWN_FAILED guard at 368)
- lib/providers/codex.mjs: post-loop guard at lines 521-526 (parallel)
- lib/providers/mistral.mjs: post-loop guard at lines 643-648 (parallel)
- test-features.mjs: new Suite 16 test 16e — race reproduction (109 lines)

Test 16e race reproduction (deterministic):

Exploits Node event-loop ordering. Mock spawn schedules data1 + data2 +
close via a single setImmediate. setImmediate runs before timers in the
same tick, so all 3 events queue before the timer can fire. Generator
yields data1 then suspends at yield with rejectNext null. Consumer
pauses 25ms (> 10ms timer). Timer fires during yield-suspension:
spawnTimedOut=true, rejectNext null → branch skipped, SIGTERM sent.
Consumer resumes, drain processes data2 + close, breaks normally.
Post-loop: D24 guard catches spawnTimedOut → throws SPAWN_TIMEOUT.

Pre-D24 behavior: generator returns normally (truncated, silently
cacheable). Test would FAIL.
Post-D24: SPAWN_TIMEOUT throws → test PASSES.

Reviewer empirically validated the test's diagnostic power by stripping
D24 from anthropic.mjs and observing exactly 16e fail (only 16e — 16a
through 16d still pass on the orthogonal rejectNext-set path), then
restored.

Timing flake risk: very low. Race depends only on relative ordering of
two setTimeout callbacks (10ms vs 25ms) which Node guarantees regardless
of absolute drift. Reviewer ran 7 npm test passes; 16e timing spread
26.43-27.70ms (1.3ms variance).

Tests: 334 → 335 (+1). 335/335 pass on Node 20.

Out of scope (filed in issue #3):
- SPAWN_TIMEOUT salvage parity: this fix discards partial chunks (matches
  current SPAWN_TIMEOUT semantics, doesn't introduce asymmetry vs
  SPAWN_FAILED-no-chunks). Whether to add SPAWN_TIMEOUT-with-usable-chunks
  salvage (analogous to D16's SPAWN_FAILED salvage) is a separate design
  decision tracked in issue #3.

Authority:
- ADR 0004 § Trigger taxonomy — Hard triggers bullet 4: "Provider CLI
  spawn timeout (configurable per-provider via hints.maxSpawnTimeMs)"
  https://github.com/dtzp555-max/olp/blob/main/docs/adr/0004-fallback-engine.md
- ADR 0005 § Cache write conditions item 1 (no truncation)
- engine.mjs HARD_TRIGGER_CODES['SPAWN_TIMEOUT'] = true (D10 P1.3,
  unchanged)
- CC 开发铁律 v1.6 § 10.x — Round-2 Cold Audit caught this

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Verified guard position in all 3 plugins; read
push() to confirm rejectNext-null mechanism; empirically validated test
16e's diagnostic specificity (strip-revert-restore); 7 test runs
confirmed timing stability. No blocking issues.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 14:22:53 +10:00
taodengandClaude Opus 4.7 1466d3a082 fix(providers): D21 — validateProvider enforces maxSpawnTimeMs contract field (round-2 F1) 
cold-audit catch from 2026-05-24

Round-2 cold-audit Finding 1 (P2 contract drift). ADR 0002 Amendment 1
(D11, commit f659e29) added `maxSpawnTimeMs` to the Provider contract
v1.0 hints set alongside requiresTTY/concurrentSpawnSafe/maxConcurrent.
But `lib/providers/base.mjs` validateProvider was never updated —
ProviderHints typedef listed only the original 3 fields, and the
validator's error message + per-field checks ignored maxSpawnTimeMs
entirely. A plugin that omitted the field would still pass validation;
each plugin's `?? 600_000` runtime default masked the omission at the
spawn site. Round-1 cold audit + D11 diff review both missed this.

Changes (lib/providers/base.mjs +6 / -1, test-features.mjs +36):

1. ProviderHints typedef extended:
   `@property {number} [maxSpawnTimeMs] - optional integer milliseconds, default 600000`
   The `[name]` JSDoc marker correctly indicates optional.

2. Hints existence error message updated to include the new field:
   `'hints must be an object with { requiresTTY, concurrentSpawnSafe, maxConcurrent, maxSpawnTimeMs }'`

3. Per-field validation added inside the hints-object branch:
   ```
   if (p.hints.maxSpawnTimeMs !== undefined) {
     if (typeof !== 'number' || !Number.isInteger(...) || <= 0) push error
   }
   ```
   The `!== undefined` outer guard correctly short-circuits omission to
   "valid" (the field is optional per ADR). The inner check rejects:
   negative, zero, non-integer (catches floats, NaN, Infinity since
   Number.isInteger handles all non-finite cases), and non-number types.

4. test-features.mjs Suite 4: 6 new tests covering all rejection axes
   + both positive-path cases (omitted-is-valid, positive-integer-is-valid):
   - rejects negative
   - rejects zero
   - rejects non-integer (100.5)
   - rejects non-number ('600' string)
   - accepts omitted (optional field)
   - accepts positive integer (60000)

Tests: 328 → 334 (+6). All three shipped plugins (anthropic / codex /
mistral) declare maxSpawnTimeMs: 600_000 (positive integer) and continue
to pass validation. Suite 16 spawn-timeout tests confirm the existing
positive integer flows through to the runtime enforcement loop unchanged.

Authority:
- ADR 0002 § Amendments § Amendment 1 (D11, 2026-05-23) — establishes
  maxSpawnTimeMs as part of the v1.0 hints contract
  https://github.com/dtzp555-max/olp/blob/main/docs/adr/0002-plugin-architecture.md
- ADR 0004 § Trigger taxonomy — Hard triggers bullet 4 — the contract
  field's use site (per-plugin spawn-timeout enforcement → SPAWN_TIMEOUT
  hard trigger)
- CC 开发铁律 v1.6 § 10.x — Round-2 Cold Audit caught this

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Verified ADR Amendment 1 wording matches validator
behavior; verified all 4 rejection axes plus 2 positive axes covered
by tests; verified existing plugin declarations continue to pass via
334/334 (Suite 16 spawn-timeout suite included). No blocking issues.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 14:06:21 +10:00
taodengandClaude Opus 4.7 ed82e65859 chore: D19 — cleanup batch (Findings 8 + 14 + 15 + D17 dead import) 
cold-audit catch from 2026-05-23

Batched 4 small P3 mechanical cleanups per Iron Rule 11 IDR cleanup-batch
convention (all P3, all small, no semantic feature changes beyond defensive
validation).

Changes (7 files):

1. lib/ir/ir-to-openai.mjs (+30 / -3) — Finding 8 defensive validator:
   - Added OPENAI_FINISH_REASON_ENUM Set with the 6 spec-allowed values
     (stop / length / tool_calls / content_filter / function_call / null)
   - Added normalizeFinishReason(value) helper that returns value unchanged
     if in enum, else 'stop'
   - Routed both irChunkToOpenAISSE (streaming path) and
     irResponseToOpenAINonStream (non-stream path) through the helper
   - Bonus tightening (in-scope, same finish_reason concept):
     irResponseToOpenAINonStream's gate changed from `if (chunk.finish_reason)`
     (truthy check) to `if (chunk.finish_reason !== undefined)` so an
     explicit null (valid spec value meaning "still in progress") is no
     longer silently dropped by the truthy guard
   - Note: undefined → null collapse via `?? null` is unreachable in the
     current codebase (all provider plugins explicitly set finish_reason
     on stop chunks); defensive only against a future plugin that omits
     the field — documented inline

2. .github/workflows/alignment.yml (-16) — Finding 14 dead CI cleanup:
   - Removed `setup.mjs` from path triggers (push + pull_request) — the
     file does not exist in the repo
   - Removed the dead `KNOWN_PROVIDERS=(...)` bash array from job 1 and
     its comment block — no later step iterated over it, so the array
     was abandoned
   - LEFT untouched: the Node.js inline KNOWN_PROVIDERS array in the
     models-registry validation job — that one is actively consumed by
     the schema validation script

3. lib/providers/anthropic.mjs / codex.mjs / mistral.mjs (3 × 1 line) —
   Finding 15: removed unused `PROVIDER_ERROR_CODES` from import lines.
   Each line went from `import { ProviderError, PROVIDER_ERROR_CODES } from
   './base.mjs';` to `import { ProviderError } from './base.mjs';`. The
   constant remains exported from base.mjs (its declaration site, where
   it IS used for validation).

4. server.mjs (1 line) — D17 reviewer's observation: removed unused
   `getProviderForModel` from the import line. The function is only
   called by lib/fallback/engine.mjs which imports it directly from
   lib/providers/index.mjs. server.mjs's import was dead (the routing
   SPOT lives in engine.mjs after D17 — server.mjs uses buildDefaultChain
   exclusively).

5. test-features.mjs (+44) — Suite 3 (irChunkToOpenAISSE format) extended
   with 4 new finish_reason normalization tests:
   - Test 1: non-spec streaming finish_reason ('timeout', 'overloaded',
     'cancelled') → mapped to 'stop'
   - Test 2: spec-enum streaming finish_reason (all 6 incl. null) preserved
   - Test 3: non-spec non-stream finish_reason → mapped to 'stop'
   - Test 4: spec-enum non-stream finish_reason preserved (null
     intentionally omitted — documented inline)

Tests: 324 → 328 (+4). All pass on Node 20.

Pre-commit fold-ins (per evidence-first checkpoint #4):

- **D19 reviewer suggestion #1**: added inline comment to
  normalizeFinishReason explaining the unreachable `undefined → null`
  branch (defensive only, no current plugin omits the field). Cheap
  future-reader clarity.
- **D19 reviewer suggestion #2**: added inline comment to Test 4
  explaining why null is intentionally omitted from the spec-enum list
  (non-stream `!== undefined` gate enters with null and overwrites
  default 'stop' to null — semantically odd but spec-valid).

Reviewer suggestion #3 (consider stricter `undefined → 'stop'` on
streaming-stop path vs `null → null` on delta path) explicitly marked
out of scope by reviewer — would require call-site context awareness;
filed mentally as potential future work, not tracked as an issue
since no current path triggers it.

Authority:
- ALIGNMENT.md Rule 2(b) — only spec-defined fields in OpenAI responses
- OpenAI Chat Completions spec finish_reason enum
  https://platform.openai.com/docs/api-reference/chat/object#finish_reason
- CC 开发铁律 v1.6 § 10.x — Cold Audit Findings 8 / 14 / 15

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Verified the unreachable `undefined → null` branch
claim by grep-checking all 3 provider plugins (none emit undefined);
verified the two KNOWN_PROVIDERS arrays were correctly distinguished
(only the dead bash one removed); ran npm test independently to confirm
328/328.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 12:16:19 +10:00
taodengandClaude Opus 4.7 cb86807009 refactor(routing): D17 — alias-aware getProviderForModel as routing SPOT (Findings 12 + 13) 
cold-audit catch from 2026-05-23

Cold-audit Findings 12 + 13 (both P3, both routing layer). F12: mistral
plugin's models[] included canonical IDs + aliases while anthropic/codex
were canonical-only — inconsistent routing surface (request "sonnet"
returned 503 from anthropic but request "devstral" worked for mistral).
F13: getProviderForModel imported but never called; buildDefaultChain
duplicated the lookup loop — two SPOT-candidate code paths.

Per cold-audit reviewer's option (a): standardize models[] on canonical-
only across all 3 plugins; getProviderForModel becomes alias-aware via
models-registry.json; buildDefaultChain uses getProviderForModel as SPOT.

Changes (4 files, +242 / -50):

1. lib/providers/index.mjs:
   - At module load, builds `_aliasMap: Map<aliasString, {providerName,
     canonicalModel}>` by walking models-registry.json's providers[*].aliases
   - getProviderForModel signature extended: returns `{provider, name,
     canonicalModel}` (was `{provider, name}`). The canonicalModel field is
     the resolved canonical ID for callers that need to use it downstream
     (cache key, observability headers, log events)
   - 3-step resolution order: (1) alias map lookup → if hit AND provider
     loaded → return with canonicalModel; (2) direct canonical scan → return
     with canonicalModel = modelString; (3) null
   - "Alias known but provider not loaded" case correctly falls through
     to step 2 (which will also miss for an alias string) → returns null

2. lib/providers/mistral.mjs:
   - _registryModels stripped to canonical-only: removed the
     `...Object.keys(_registryEntry.aliases ?? {})` spread
   - mistral.models[] now matches anthropic/codex shape (canonical IDs only)
   - Comment block updated to point future readers at getProviderForModel
     as the SPOT

3. lib/fallback/engine.mjs:
   - buildDefaultChain's inline `for ([name, provider] of loadedProviders)`
     scan loop replaced with a single getProviderForModel() call
   - Chain hop's `model` field is now `match.canonicalModel` (was `modelString`)
     — so downstream consumers receive canonical
   - Explicit-chain config path unchanged (kept its pre-existing behavior;
     alias resolution in routing.chains config is a known limitation, tracked
     as a future improvement)

4. test-features.mjs:
   - 17 new tests in `D17 — alias-aware getProviderForModel` describe block:
     anthropic alias coverage (sonnet/opus/haiku/claude), openai aliases
     (codex/codex-spark/gpt5/gpt5-mini), mistral aliases (devstral/
     devstral-2/devstral-small/devstral-small-2), canonical pass-through,
     unknown model → null, alias-to-disabled-provider → null,
     buildDefaultChain integration with alias resolution
   - 3 existing mistral tests rewritten — they were asserting the pre-D17
     inconsistent shape (aliases in mistral.models[]). Now assert the
     post-D17 invariant (aliases NOT in models[]; routing via
     getProviderForModel instead)

Tests: 300 → 317 (+17 new). All pass on Node 20.

Pre-commit fold-in (per evidence-first checkpoint #4 — fold-ins themselves
need second-pass review):

- **D17 reviewer flagged C10**: original engine.mjs comment said
  "downstream (cache key, X-OLP-Model-Used, provider.spawn) receives the
  canonical ID rather than the alias string". The provider.spawn portion
  is incorrect — spawn reads `irRequest.model` (the user's original input),
  which is NEVER rewritten to canonical. Each provider CLI accepts its own
  aliases natively (claude accepts sonnet/opus/haiku; vibe accepts
  devstral/devstral-2; codex accepts its model IDs), so runtime behavior
  is fine, but the comment overstated what D17 actually changes.
  Folded in: corrected comment to honestly describe the canonical flow
  (cache key + X-OLP-Model-Used + logs receive canonical; spawn continues
  to receive irRequest.model). Same class of doc-code drift fix as D11's
  B1 (false maxConcurrent enforcement claim) and D16's "bypasses
  getOrCompute" drift — the discipline is maturing across D-days.

Authority:
- ADR 0002 § Provider contract (`models: string[]` is "models this provider
  serves")
  https://github.com/dtzp555-max/olp/blob/main/docs/adr/0002-plugin-architecture.md
- models-registry.json — single source of truth for (provider, model)
  metadata + alias→canonical mappings per AGENTS.md SPOT policy
- AGENTS.md § Project-specific constraints — "models-registry.json is the
  only place to add/edit (provider, model) metadata"
- CC 开发铁律 v1.6 § 10.x — Cold Audit Findings 12 + 13

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Highest-value verification: walked
chain[0].model from buildDefaultChain through all downstream consumers
(computeCacheKey, X-OLP-Model-Used, log events) to confirm canonical
flow; then verified provider.spawn paths in anthropic.mjs:231 and
codex.mjs:248 read irRequest.model (user input), confirming the comment
overstatement (now fixed). Verified no alias-canonical collision exists
in current registry (12 aliases vs 10 canonical IDs, zero intersection).
Verified empty-registry edge case + provider-with-model-not-in-registry
edge case.

Follow-up items (reviewer's non-blocking observations, NOT in this PR):
- server.mjs:32 dead import of getProviderForModel — defer to D19 cleanup
- explicit-chain config path doesn't run alias resolution (routing.chains
  in ~/.olp/config.json) — pre-existing limitation, file as future issue

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 11:55:43 +10:00
taodengandClaude Opus 4.7 2cfd0b194e feat(phase-1): D10 P1 hardening — providers.enabled wiring + real streaming + spawn timeout 
Folds in three production-blocking defects from external Codex review
round-3 of Phase 1 (D5+D6+D8+D9 already merged). Without these, OLP
returns 503 on every request even when `npm start` succeeds, streams
arrive as a single buffered burst after the spawn completes, and hung
CLIs block the engine forever despite ADR 0004 declaring spawn-timeout
a hard trigger.

P1.1 providers.enabled config wiring (ADR 0002 § Disable model):
- loadFallbackConfigSync() returns tri-field {chains, soft_triggers,
  providersEnabled}; server.mjs reads _startupConfig.providersEnabled
  and passes to loadProviders() at startup
- Empty / missing config → 0 enabled providers → 503 no_enabled_provider
  (matches v0.1 0-Enabled posture per ALIGNMENT.md § Provider Inventory)
- __setProvidersEnabled / __resetProvidersEnabled test seams; in-place
  Map mutation preserves existing direct-mutation patterns in Suite 13

P1.2 Real SSE streaming on single-hop cache-miss (ADR 0003 entry adapter
pattern, OpenAI /v1/chat/completions stream=true contract):
- New handleChatCompletions branch when ir.stream === true && chain.length
  === 1 && !bypassCache && !preCheckHit
- for await (const irChunk of provider.spawn(...)) writes SSE per chunk
  via res.write(irChunkToOpenAISSE(...)); accumulates streamedChunks for
  cacheStore.set on stop
- First-chunk rule preserved: error-before-first-chunk → sendError(502);
  error-after-first-chunk → truncated res.end(), no fallback
- Multi-hop chains (chain.length > 1) continue to use buffered
  executeWithFallback to keep fallback safety semantics

P1.3 Spawn timeout hard trigger (ADR 0004 § Trigger taxonomy bullet 4):
- SPAWN_TIMEOUT added to PROVIDER_ERROR_CODES (lib/providers/base.mjs)
  and HARD_TRIGGER_CODES (lib/fallback/engine.mjs)
- All three plugins (anthropic.mjs / codex.mjs / mistral.mjs) wrap drain
  loop with setTimeout (default 600_000ms, configurable via
  hints.maxSpawnTimeMs); on fire: proc.kill('SIGTERM') + reject pending
  drain promise with ProviderError(..., 'SPAWN_TIMEOUT')
- Timer cleared in finally; resolveNext/rejectNext atomically nulled in
  push() + timer-fire path to prevent late-fire double-settle

Tests 277 → 288 (+11). Suite 14 (4 providers.enabled), Suite 15
(3 streaming cache-miss real-time, including arrival-count >= 2
assertion that architecturally proves real streaming), Suite 16
(4 spawn timeout, including 2-hop chain advancement from timed-out
primary). 288/288 pass on Node 20.20.2 + Node 25.8.0.

Authorities:
- ADR 0002 § Disable model — config toggle, not plugin-removal
  https://github.com/dtzp555-max/olp/blob/main/docs/adr/0002-plugin-architecture.md
- ADR 0003 § Translation direction model — entry adapter for await pattern
  https://github.com/dtzp555-max/olp/blob/main/docs/adr/0003-intermediate-representation.md
- ADR 0004 § Trigger taxonomy + § Fallback safety (first-chunk rule)
  https://github.com/dtzp555-max/olp/blob/main/docs/adr/0004-fallback-engine.md
- OpenAI /v1/chat/completions stream=true (server-sent events,
  data: {chunk} per delta, terminator data: [DONE])
  https://platform.openai.com/docs/api-reference/chat/streaming

Reviewer (Iron Rule 10): fresh-context opus, independent of drafter.
Verdict: APPROVE_WITH_MINOR. Folded the one cheap minor before commit
(Suite 15a arrival-count assertion strengthened from chunks.length > 0
to arrivalTimestamps.length >= 2 — the prior assertion would have
admitted a buffered impl). Two remaining non-blocking notes deferred:
optional writeHead deferral (low value; single-hop guard makes pre-
content 200 + empty body safe), and version bump (Phase 1 ships as
v0.1.0 aggregate when D11–D16 land).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-24 06:57:11 +10:00
taodengandClaude Opus 4.7 (noreply@anthropic.com) 95dc072245 feat(phase-1): land Mistral Vibe provider plugin (D8) 
Phase 1 Day 5. Mistral Vibe provider plugin lands as Candidate. STATIC_
REGISTRY now length 3 (anthropic + openai + mistral). vibe CLI not
installed on the orchestrator machine and owner has no Le Chat Pro
subscription, so D8 follows the D6 docs-only authority pattern. D-later
verification (once a tester with a vibe install and subscription is
available) will resolve UNPINNED assumptions A4, A6, A7, A8.

Files:
  NEW:  lib/providers/mistral.mjs (~720 lines) — Mistral Vibe provider.
        Spawns `vibe --prompt PROMPT --output streaming` per docs.
        Reads MISTRAL_API_KEY env var with ~/.vibe/.env fallback.
        Supports VIBE_HOME env override per docs. Mirrors D4/D6 plugin
        structure incl. __setSpawnImpl/__resetSpawnImpl for tests.
  MOD:  lib/providers/index.mjs (+12/-1) — STATIC_REGISTRY now length 3.
        listAllProviderNames returns [anthropic, openai, mistral].
  MOD:  models-registry.json (+26 lines) — providers.mistral with 2
        canonical date-stamped IDs (devstral-2-25-12, devstral-small-2-
        25-12) and 4 short-form aliases for user convenience.
  MOD:  test-features.mjs — Suite 12 with 48 tests covering contract
        conformance, IR translation, mock-spawn behaviour, healthCheck,
        estimateCost, auth-artifact reading.

Authority citations (all WebFetched and verified during reviewer pass):
  DOCS-1: docs.mistral.ai/mistral-vibe/terminal/quickstart — `vibe
    --prompt PROMPT --max-turns 5 --max-price 1.0 --output json` example
    + Output Format Options enumeration: text default, json single blob,
    streaming NDJSON.
  DOCS-2: docs.mistral.ai/mistral-vibe/terminal/configuration — pin
    `~/.vibe/.env`, MISTRAL_API_KEY env var, VIBE_HOME override.
  DOCS-3: docs.mistral.ai/mistral-vibe/introduction/configuration —
    second confirmation of MISTRAL_API_KEY + ~/.vibe/.env (model
    selection via config.toml `/config` slash command, NOT --model
    flag).
  DOCS-4: deepwiki.com/mistralai/mistral-vibe/9.3-cli-commands-reference
    — full flag enumeration confirms no --model flag exists. Programmatic
    mode trigger is --prompt; flags are: --continue, --max-price,
    --max-turns, --output, --prompt, --resume, --setup, --trust,
    --upgrade, --version, --workdir, --no-autofill, --no-header,
    --no-dev, --enabled-tools, --help.
  DOCS-5: mistral.ai/news/devstral-2-vibe-cli — launch announcement,
    names Devstral 2 (123B) and Devstral Small 2 (24B), 256K context,
    pricing $0.40/$2.00 and $0.10/$0.30 per MTok.
  DOCS-6: help.mistral.ai/en/articles/347532 — Vibe included in Le Chat
    Pro.
  DOCS-7: legal.mistral.ai/terms/usage-policy — no anti-third-party
    clauses; ADR 0006 Tier D classification holds.
  DOCS-MAIN: docs.mistral.ai/mistral-vibe/overview — main Vibe overview.
  DOCS-8: docs.mistral.ai/getting-started/models/models_overview —
    canonical Mistral models registry. Pin for date-stamped IDs
    devstral-2-25-12 / devstral-small-2-25-12. Caught by D8 review-2.

Architectural decisions:
  1. `--output streaming` (NOT `json`). Per DOCS-1 verbatim: streaming
     emits NDJSON per message; json emits a single blob at the end.
     Original D8 draft used `json` which is incompatible with the
     plugin's line-buffered stdout parser. Review-2 caught this; fixed
     before commit.
  2. No --model flag. Per DOCS-4 full flag enumeration there is no
     --model flag in programmatic mode. Model selection happens via
     ~/.vibe/config.toml. The IR's `model` field is used by OLP for
     routing only; Vibe uses whatever model is in user-level config.
     Documented in lossy translations + as A5 CONFIRMED-NOT-APPLICABLE.
  3. Canonical IDs primary, short forms as aliases. models-registry.json
     uses devstral-2-25-12 / devstral-small-2-25-12 as primary `id`s
     matching the canonical Mistral models registry; user-facing short
     forms (devstral-2, devstral-small-2, devstral, devstral-small) are
     aliases. Plugin's models[] array includes both canonical IDs AND
     alias keys so getProviderForModel routes either form. Same pattern
     codified for Codex aliases in D6.
  4. Auth precedence: MISTRAL_API_KEY env > ~/.vibe/.env > null.
     Documented in DOCS-2. readAuthArtifact supports
     MISTRAL_VIBE_AUTH_PATH env override for testing.
  5. Mistral stays Candidate. STATIC_REGISTRY.length === 3 but
     loadProviders({}) returns empty Map; only loadProviders({
     enabled: { mistral: true }}) loads it. POST /v1/chat/completions
     devstral-* still returns 503 until config flag is set and E2E
     audit passes.

Reviewer chain (Iron Rule 10):
  Implementer: sonnet (general-purpose).
  Fresh-context reviewer: opus (ecc:code-reviewer). Verdict round-1:
    REQUEST_CHANGES (2 blockers caught).
  Reviewer ran npm test (222/222 pass), WebFetched all 8 canonical
    Mistral docs URLs, discovered DOCS-8 (models registry) which
    sonnet missed — exactly the D6 failure pattern, repeated. Reviewer
    independently verified `--output streaming` vs `json` semantics
    against the live docs page text, not paraphrases.

Reviewer blocking findings folded in this commit:
  B-1 (--output json wrong): Plugin now passes `--output streaming` per
    docs verbatim. Test "irToMistral: user message → args with --prompt
    and --output streaming" updated to assert the new arg and reject
    the old one.
  B-2 (canonical models page missed): models-registry.json refactored
    to use date-stamped canonical IDs as primary; short forms as
    aliases. mistral.mjs header adds DOCS-8 as the new canonical
    authority pin for model IDs. Plugin's models[] array merges
    canonical + alias keys so existing routing tests pass with either
    form.
  B-3 (404 claim incorrect): mistral.mjs header DOCS-MAIN updated.
    docs.mistral.ai/mistral-vibe/overview is 200 OK; sonnet's 404
    claim was a path-normalization mismatch.

Reviewer non-blocking suggestions (deferred to D-later / not D8 scope):
  - VIBE_HOME env override has no Suite 12 test (implementation
    present at mistral.mjs:262). Parallel gap to D6 CODEX_HOME.
  - _extractKeyFromDotenv has no direct unit test.
  - config.toml model selection mechanism (Vibe-specific quirk —
    no --model flag means OLP can't pass model per request). A D-later
    ADR note will discuss whether OLP should write a project-local
    ./.vibe/config.toml before spawn or accept the user-level config
    as authoritative.

Test count: 174 (after D6) → 222 (after D8).

Verification:
  node --check on all touched files: clean.
  npm test on Node 25.8.0: 222/222 pass in 210ms.
  STATIC_REGISTRY = [anthropic, openai, mistral] verified.
  hygiene grep: zero personal-name/path/token hits. Fixtures use
    <fake-mistral-api-key> placeholders.

Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
 2026-05-23 21:48:29 +10:00
taodengandClaude Opus 4.7 (noreply@anthropic.com) ea9184d2e8 feat(phase-1): land OpenAI Codex provider plugin (D6) 
Phase 1 Day 4. OpenAI Codex provider plugin code lands as Candidate.
Anthropic and Codex now both in STATIC_REGISTRY length 2. Codex CLI is
NOT installed on the orchestrator machine, so D6 ships with a docs-only
authority pin; D7 will install the binary, probe real behaviour, and
fix any docs-vs-reality divergences (A3 access-token field, A4 NDJSON
event schema, possibly keyring storage support).

Files:
  NEW:  lib/providers/codex.mjs (586 lines initially, expanded ~10 lines
        via reviewer fold-ins) — Codex provider implementing the v1.0
        contract. spawns `codex exec --json --model <id> [PROMPT|-]` per
        canonical Codex docs.
  MOD:  lib/providers/index.mjs — STATIC_REGISTRY now [anthropic, codex],
        listAllProviderNames() returns 2 entries.
  MOD:  models-registry.json — providers.openai populated with five
        documented model IDs (gpt-5.5, gpt-5.4, gpt-5.4-mini,
        gpt-5.3-codex, gpt-5.3-codex-spark) and four aliases.
  MOD:  test-features.mjs — Suite 11 added with 46 tests covering contract
        conformance, IR translation, mock-spawn behaviour, healthCheck,
        estimateCost, registry length.

Authority citations (all WebFetched and verified during reviewer pass):
  CLI reference: https://developers.openai.com/codex/cli/reference
    Source for `codex exec` subcommand syntax, --json flag, --model -m
    flag, and PROMPT positional including the `-` form for stdin piping.
  Features:      https://developers.openai.com/codex/cli/features
    Reference for the supported-models list.
  Auth:          https://developers.openai.com/codex/auth/
    Canonical pin for `~/.codex/auth.json` plaintext credential file
    and `cli_auth_credentials_store = keyring` OS credential store option.
  Models:        https://developers.openai.com/codex/models
    Canonical pin for the five documented model IDs (each shown as a
    `codex -m <id>` example on the page).
  ChatGPT plan:  https://help.openai.com/en/articles/11369540 — Codex
    runs against ChatGPT subscription budget when OAuth-authenticated;
    OPENAI_API_KEY env path is for `codex login --with-api-key` only,
    not `codex exec` runtime.

Architectural decisions:
  1. Mirror D4 anthropic.mjs structure: file header, lossy translation
     docs, default export = provider object, named exports include
     __setSpawnImpl / __resetSpawnImpl for test injection.
  2. Stdin path uses `args.push('-')` per documented CLI behaviour.
     (Original D6 sonnet draft omitted the positional entirely and wrote
     stdin directly — D6 reviewer pass 2 caught this; corrected before
     commit. D7 E2E confirms.)
  3. Auth artifact path `~/.codex/auth.json` is now documented in the
     header as CONFIRMED per canonical auth doc, not assumed.
  4. Access-token field name remains a defensive 3-name try-order
     (access_token / token / accessToken) because the auth doc does
     not enumerate field names. D7 captures real auth.json post-login.
  5. OPENAI_API_KEY env injection during spawn is intentionally NOT
     done. The auth doc clarifies OPENAI_API_KEY is a login-time input,
     not a runtime override. codex exec reads its own auth artifact.
  6. Codex stays Candidate. loadProviders({}) returns empty Map; only
     loadProviders({ enabled: { openai: true } }) loads it. POST /v1/
     chat/completions gpt-5.5 etc still returns 503 until config flag
     is set + E2E audit passes.

Reviewer chain (Iron Rule 10):
  Implementer: sonnet (general-purpose).
  Fresh-context reviewer: opus (ecc:code-reviewer). Verdict
    APPROVE_WITH_MINOR.
  Reviewer ran npm test (174/174 pass with Suite 10 skipped), WebFetched
    all four canonical Codex docs URLs, and discovered two additional
    docs pages (auth + models) that sonnet had missed. Reviewer
    independently verified the documentation citations rather than
    trusting sonnet quotes — Rule 2 (No Invention) is the load-bearing
    check for D6 because no local binary exists to ground-truth the
    plugin.

Reviewer non-blocking findings folded in this commit:
  1. Stdin path corrected: docs explicitly state `Use - to pipe the
     prompt from stdin`. The original draft assumed "no positional →
     stdin"; docs require literal `-`. Fixed in irToCodex; test
     "irToCodex: multiline prompt uses stdin path (useStdin=true) with
     - positional" updated to assert args.includes('-').
  2. Model registry expanded from 3 to 5 entries per canonical models
     doc. Added gpt-5.4-mini and gpt-5.3-codex-spark. Removed the
     misread Rule 2 comment that justified omitting -spark suffix —
     the docs literally show `codex -m gpt-5.3-codex-spark`, so the
     -spark variant is a separate model not a -codex normalization.
     New aliases: codex-spark, gpt5-mini.
  3. File header A2 upgraded from "assumed" to "CONFIRMED" with the
     canonical auth doc URL cited.
  4. File header now cites both auth and models canonical URLs at the
     top, alongside reference and features.

Reviewer findings deferred to D7:
  - OS credential store / keyring support. Codex docs mention
    cli_auth_credentials_store = keyring as an alternative to file
    storage. The Anthropic plugin supports macOS keychain via security
    find-generic-password; Codex equivalent unknown without inspecting
    a real install. D7 will install codex, run codex login, see what
    keyring entry codex creates (if any), and mirror the Anthropic
    keychain support pattern.
  - Real NDJSON event schema (field names). Defensive 4-shape parser
    handles the most common conventions; D7 captures real stdout and
    pins the schema.
  - access_token field name in auth.json. D7 captures the real auth
    artifact and removes unused fallback names.

Test count: 128 (after D5) → 174 (after D6).

Verification:
  node --check on all touched files: clean.
  npm test on Node 25.8.0: 174/174 pass in 210ms with Suite 10 skipped.
  Test "codex.models contains all 5 docs-listed model IDs" passes.
  Test "irToCodex: multiline prompt uses stdin path with - positional"
    passes (asserts args.includes('-')).
  Hygiene grep: zero personal-name/path/token hits.

Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
 2026-05-23 21:20:13 +10:00
taodengandClaude Opus 4.7 (noreply@anthropic.com) c175e8994c feat(phase-1): land Anthropic provider plugin (D4) 
Phase 1 Day 2. Anthropic provider plugin code lands, plus contractVersion
field added across base.mjs and validated strictly. Anthropic stays
CANDIDATE per ALIGNMENT.md Provider Inventory — D5 flips to Enabled after
the real spawn E2E audit passes. POST /v1/chat/completions claude-* still
returns 503 until then.

Files:
  NEW:  lib/providers/anthropic.mjs (445 lines)
  MOD:  lib/providers/base.mjs (+8 lines — contractVersion enforcement)
  MOD:  lib/providers/index.mjs (+37 lines — STATIC_REGISTRY adds anthropic
        + getProviderByName helper)
  MOD:  models-registry.json — populates providers.anthropic with 3 models
        opus-4-7 / sonnet-4-6 / haiku-4-5, alias map, candidate marker
  MOD:  test-features.mjs (+481 lines — Suite 6: 37 new tests covering
        contract conformance, contractVersion enforcement, IR translation,
        mock-spawn behaviour, healthCheck, estimateCost)

Authority citations (all verified by independent reviewer against actual
OCP byte offsets):
  Spawn pattern: OCP server.mjs:542 stdio shape, port verbatim.
  CLI args: OCP server.mjs:384-414 buildCliArgs pattern — -p, --model X,
    --output-format text, --no-session-persistence (session-resume and
    permissions branches stripped per OLP no-state architecture).
  stdin write: OCP server.mjs:586-587 verbatim.
  Stdout text handling: OCP server.mjs:735-748 raw d.toString per chunk
    no JSON envelope, matches --output-format text.
  Auth chain: OCP server.mjs:864-888 (env CLAUDE_CODE_OAUTH_TOKEN ->
    ~/.claude/.credentials.json -> macOS keychain with both label formats
    "claude-code-credentials" and "Claude Code-credentials") ported in
    same priority order. One delta vs OCP: OLP guards keychain branch on
    process.platform === darwin, OCP relies on try/catch on Linux. Both
    behave identically; OLP avoids an unnecessary shell-out.
  Env cleanup: OCP server.mjs:530-534 — delete CLAUDECODE, ANTHROPIC_
    API_KEY, ANTHROPIC_BASE_URL, ANTHROPIC_AUTH_TOKEN. CLAUDECODE
    clobbering pitfall inherited per memory.

Architectural decisions:
  1. Anthropic stays Candidate at D4. STATIC_REGISTRY.length === 1 but
     loadProviders({}) returns empty Map. Suite 7 HTTP integration tests
     continue to verify 503 with no_enabled_provider for any claude-*
     model. D5 changes the config default to enable: { anthropic: true }
     and adds the real E2E spawn test.
  2. contractVersion === 1.0 strictly enforced (F3 fold-in from D3
     review). validateProvider in base.mjs rejects providers missing or
     having any other version string. Suite 6 includes 4 tests covering
     missing / 0.9 / 1.0 / undefined cases.
  3. quotaStatus returns null at D4 with a TODO comment pointing at the
     ALIGNMENT.md 2026-06-16 one-shot audit. Anthropic Agent SDK Credit
     pool balance API has not been pinned; verification scheduled for
     2026-06-16 per OLP one-shot audits.
  4. estimateCost returns shape but usd: null. Per-million-token rates
     not pinned at D4. Lands when models-registry.json gains a pricing
     field in a later phase.
  5. Lossy translations explicitly documented in anthropic.mjs file
     header per ADR 0003 § Lossy-translation documentation requirement.
     Includes response_format json_object (system-prompt augmented),
     top_p (no --top-p flag), tool_choice required (no flag), and
     request-level tools[] + assistant tool_calls + tool_call_id
     (text-in/text-out CLI cannot consume structured tool wire format).
     The tools[] documentation gap was a reviewer non-blocking finding;
     folded in this commit.

Mocking discipline: no real claude -p spawn in any D4 test. spawn-path
coverage uses __setSpawnImpl injection of fake child_process. No real
OAuth tokens or API keys in fixtures — all use placeholder strings
fake-oauth-token / fake-token. Auth path computed via
path.join(homedir(), .claude, .credentials.json), no hardcoded
/Users/<name> literal.

Reviewer chain (Iron Rule 10):
  Implementer: sonnet (general-purpose).
  Fresh-context reviewer: opus (ecc:code-reviewer). Verdict
    APPROVE_WITH_MINOR.
  Reviewer ran npm test (98/98 pass) and verified all five OCP citations
    at the actual byte offsets in /Users/taodeng/ocp/server.mjs. All
    citations confirmed accurate.

Reviewer non-blocking findings:
  1. tools[] and tool_calls lossy-translation undocumented — FOLDED IN
     this commit (anthropic.mjs header rewritten with full lossy list).
  2. with type json import attribute Node 20 compat — DEFERRED to CI
     verification. The syntax is stable on Node 20.10+ and the CI
     setup-node@v4 with node-version 20 resolves to latest 20.x. If
     CI Node 20 leg fails, mitigation is bump engines.node to >=20.10
     or swap both import-attribute lines for readFileSync + JSON.parse.
  3. CLI_NOT_FOUND error code declared but never thrown — DEFERRED to
     a future commit. Pure cosmetic; could distinguish ENOENT from
     generic spawn errors but no functional impact.

Test count: 61 -> 98 (+37 D4 tests).

Verification:
  node --check on all touched files: clean.
  npm test on Node 25.8.0: 98/98 pass in 209ms.
  Reviewer-run npm test independently: 98/98 pass.
  loadProviders({}) returns empty Map (verified by orchestrator and
    reviewer): Anthropic Candidate gate holds.
  hygiene grep: no personal names, no /Users/<name>/ literals, no real
    OAuth tokens or API keys.

Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
 2026-05-23 17:59:48 +10:00
taodengandClaude Opus 4.7 (noreply@anthropic.com) e2e67de23a feat(phase-1): land IR + plugin loader + server skeleton (D3) 
Phase 1 Day 1. First executable code lands. Zero providers wired yet
(per ALIGNMENT.md "v0.1 ships 0 Enabled Providers"); the server starts
clean and POST /v1/chat/completions returns 503 with no_enabled_provider.

Files added:
  lib/ir/types.mjs            - IR v1.0 schema + validators (ADR 0003)
  lib/ir/openai-to-ir.mjs     - OpenAI Chat Completions to IR
  lib/ir/ir-to-openai.mjs     - IR chunks to OpenAI SSE / non-stream
  lib/providers/base.mjs      - Provider contract + validateProvider + ProviderError
  lib/providers/index.mjs     - Static empty registry stub (ADR 0002)
  server.mjs                  - HTTP listener with createOlpServer factory + main guard
  test-features.mjs           - 61 tests across 7 suites (IR / provider / HTTP)

Files modified:
  package.json - main and scripts.start/test added back; targets now exist.

Authority citations:
  IR fields and translation direction: ADR 0003 sections Decision and
    Translation direction model.
  Provider contract (9 fields): ADR 0002 section Provider contract v1.0
    interface.
  Entry surface routes (health, v1/models, v1/chat/completions): OLP v0.1
    spec section 4.1 single-protocol entry; ALIGNMENT.md Authority 2.
  Zero-Enabled-Providers behaviour: ALIGNMENT.md Provider Inventory.

Architectural decisions worth recording:
  1. server.mjs uses a createOlpServer factory plus an import.meta.url
     main guard. The factory returns an unbound http.Server; only the
     main-script invocation calls .listen(). Tests import the real
     server.mjs and exercise the real router. No parallel implementation
     in the test file.

     This pattern was a fold-in from the orchestration step. The initial
     sonnet draft put a top-level server.listen call in server.mjs, which
     forced test-features.mjs to reimplement the router inline (a false-
     confidence trap because the real server logic would never be tested).
     Refactored before reviewer dispatch.

  2. lib/providers/index.mjs ships an empty STATIC_REGISTRY array, not a
     placeholder with dummy entries. ALIGNMENT.md Provider Inventory says
     v0.1 ships zero Enabled Providers; the registry honors that exactly.
     Phase 1 Day 2 adds the first import (Anthropic) when its plugin lands.

  3. BadRequestError lives in openai-to-ir.mjs and ProviderError in
     base.mjs. Reviewer suggested relocating to a shared lib/errors.mjs
     once the count exceeds two; deferred to Phase 1 Day 2 to ship with
     the third typed error class.

  4. contractVersion: '1.0' on each provider plugin: not enforced at D3
     because no providers exist yet. Reviewer flagged for Phase 1 Day 2
     tightening when the first provider lands.

Reviewer chain (Iron Rule 10):
  Initial implementer: sonnet (general-purpose).
  Refactor (createOlpServer + main guard) by the orchestrator after
    catching the inline-router parallel-implementation issue.
  Fresh-context reviewer: opus (ecc:code-reviewer). Verdict
    APPROVE_WITH_MINOR.

Reviewer's two non-blocking findings folded in:
  F1: removed unused createServer import from test-features.mjs line 12,
      left over from the refactor.
  F2: replaced finish_reason value 'error' with 'stop' in both the
      streaming error chunk path (lib/ir/ir-to-openai.mjs line 72) and
      the non-streaming error aggregation path (lib/ir/ir-to-openai.mjs
      line 153). The 'error' value is not in OpenAI's documented
      finish_reason enum (stop / length / tool_calls / content_filter /
      function_call / null), so emitting it would violate ALIGNMENT.md
      Rule 2 (b). Provider errors are now surfaced via a top-level
      response.error object plus an inline content marker. The matching
      test assertion at test-features.mjs line 325 was updated to verify
      finish_reason stays within the OpenAI enum.

Note on the F2 fold-in:
  Reviewer pointed only at the streaming path (line 72). After applying
  that fix I ran grep across lib/ and test-features.mjs for the same
  invention pattern and caught a second hit at line 153 (non-streaming
  aggregation). This is the "fold-in must grep the full repo, not only
  the file the reviewer named" discipline from
  ~/.cc-rules/memory/feedback/evidence_first_under_speed_pressure.md.
  Both hits are fixed in this commit.

Verification:
  node --check on all 7 new files plus modified package.json plus
    server.mjs plus lib/ir/ir-to-openai.mjs - all clean.
  npm test - 61/61 pass in 209ms, no flakes, no skipped.
  OLP_PORT=14001 node server.mjs followed by curl /health returns
    proper JSON; curl /v1/models returns 200 empty list; server shuts
    down cleanly on signal.
  grep "finish_reason.*error" returns zero hits across lib/ and tests.

Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
 2026-05-23 17:06:30 +10:00