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
111 Commits 6 Branches 14 Tags
main
Commit Graph
21 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
 		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
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 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 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 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 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) 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