mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-21 21:15:10 +00:00
82d2e1cbea8908ad348738c7f52e3f2a24863e4f
10
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 
|
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> |
||
|
|
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> |
||
|
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
|
||
|
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> |
||
|
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>
|
||
|
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>
|
||
|
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>
|
||
|
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>
|
||
|
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>
|
||
|
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)
|