diff --git a/CHANGELOG.md b/CHANGELOG.md index fc4aea8..0958bdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,24 @@ All notable changes to OLP land here. Per `CLAUDE.md` release_kit overlay, this (empty — Phase 5 entries land here once Phase 5 opens) +## v0.4.1 — 2026-05-26 + +### Post-Phase-4 hotfix batch (D74) — maintainer-review findings + +Patch release fixing 5 issues caught by maintainer post-v0.4.0 independent review. Every finding was a real runtime bug that the per-D-day fresh-context opus reviewers all missed because they reviewed against spec text, not against the runtime contract (default `auth.allow_anonymous: false`, real `/health` payload shape, real `/cache/stats` payload shape, real `/v0/management/dashboard-data` payload shape). **Phase 4 lesson: future implementation D-days MUST include at least one test that boots the server with the default production config and exercises the new feature end-to-end** — not just stub-mocked codepaths. + +- **[P1-1] `olp doctor` no longer false-negatives on auth-required `/health`.** `lib/doctor.mjs` now accepts an `authHeaders` option (threaded from `bin/olp.mjs` `cmdDoctor` via the existing `authHeaders()` chain) and passes it to the `server.running` + `server.version` probes. The `server.running` check now distinguishes 401/403 ("server up but bearer token missing/invalid — set `OLP_API_KEY`") from "server unreachable" — so the `kind` discriminator routes to a clean fix-auth path instead of `fix_server` when the operator just forgot to export the env var. +- **[P1-2] `bin/olp-connect` validates token shape + shell-quotes rc writes.** New `validate_olp_token ` helper enforces the `^olp_[A-Za-z0-9_-]{43}$` regex (per ADR 0007 § 3 token format) at all 3 input sites: `--key` arg, `/health.anonymousKey` server-advertised consumption, and the interactive prompt fallback. New `shell_quote ` helper wraps rc-file writes (`export OPENAI_BASE_URL=$(shell_quote ...)`) so even a hypothetical bypass of the validator can't inject shell metacharacters into a sourced rc. systemd `environment.d/olp.conf` write additionally rejects embedded newlines. Hostile or malformed keys can no longer persist as shell startup injection. +- **[P2-3] `olp usage` + `olp cache` human formatter rewritten against the real payload shape.** `cmdUsage` previously read `body.usage_24h.requests` / `body.providers` / `body.top_fallback_chains` — all undefined under the actual server payload shape — so users saw "requests: ?" + missing per-provider quota + missing top-chains. Now reads `body.window_24h.request_count` / `body.cache_hit_24h.hit_rate` / `body.quota` / `body.top_fallback_chains_24h` per `server.mjs:2027` + `lib/audit-query.mjs`. `cmdCache` previously read `body.entries` / `body.bytes` / `body.maxBytes` (OCP-era field names). Now reads `body.size` / `body.inflightCount` per `CacheStore.stats()` and computes hit rate from `hits + misses`. +- **[P2-4] `olp-plugin/` `fmtHealth` iterates `providers.status` correctly.** Previously walked `Object.entries(body.providers)` which surfaced `enabled` / `available` / `status` as pseudo-providers (chat output showed `🟢 status` instead of `🟢 anthropic`). Now extracts the real provider map from `body.providers.status` and renders enabled/available counts in a header line + per-provider names with `activeSpawns` when present. Falls back to flat `body.providers.*` for the older OCP shape (backwards compat). +- **[P3-5] Stale v0.3.0-era doc strings updated.** README header status line + Implementation Status § now reflect v0.4.0 shipped + Phase 5 open. `server.mjs` startup banner no longer hardcodes "Phase 1 in progress" (now just lists version + provider count — derives accurate state from `VERSION` without future maintenance touch-ups). + +**Phase 4 process learning recorded.** Per Iron Rule 第二律 (evidence over "should work"), every D-day review pass must include at least one runtime smoke against the default production config. The D-day reviewer rubric is updated implicitly — D74 Suite 36 tests pin the wire-contract shape so a future D-day refactoring server payloads can't silently re-break the CLI / plugin / docs. + +- **Test count delta:** 696 (v0.4.0) → 704 (v0.4.1). +8 D74 regression tests in Suite 36. +- **Files touched:** `lib/doctor.mjs` (P1-1), `bin/olp.mjs` (P1-1 + P2-3), `bin/olp-connect` (P1-2), `olp-plugin/index.js` (P2-4), `server.mjs` (P3-5 banner), `README.md` (P3-5), `test-features.mjs` (Suite 36 regression), `package.json` (version), `CHANGELOG.md` (this entry). +- **Authority:** maintainer independent review of `main` / `v0.4.0` / commit `ee4d945` (2026-05-26 session); Iron Rule 第二律 evidence-over-should-work; CLAUDE.md `release_kit.phase_rolling_mode` cross-Phase discipline ("hotfix to a shipped Phase N deliverable → bump patch, tag, release before next push"). + ## v0.4.0 — 2026-05-26 ### Phase 4 — Operator + Client UX (D60 → D73) diff --git a/README.md b/README.md index e76e1b3..c7b900e 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A personal- and family-scale multi-provider LLM proxy. One HTTP endpoint, many subscriptions behind it, automatic routing, automatic fallback, content-addressed caching — so your IDEs and family clients keep working as long as *any* of your subscriptions has quota left. -> **Status:** v0.3.0 shipped (2026-05-25) — Phase 1 multi-provider proxy core (v0.1.0 + v0.1.1) + Phase 2 multi-key auth + audit + owner gating + keygen CLI (v0.2.0) + Phase 3 Dashboard + audit query layer + daily audit rotation (v0.3.0). Phase 4 (per-key per-provider auth + audit retention + SQLite hybrid + provider-cost weights) is the next planned milestone. Sections marked _placeholder_ land alongside the relevant phase of work (see [phase plan](#phase-plan)). +> **Status:** v0.4.0 shipped (2026-05-26) — Phase 1 multi-provider proxy core (v0.1.0 + v0.1.1) + Phase 2 multi-key auth + audit + owner gating + keygen CLI (v0.2.0) + Phase 3 Dashboard + audit query layer + daily audit rotation (v0.3.0) + Phase 4 Operator + Client UX (v0.4.0): SSE heartbeat / `olp` Node CLI + `olp doctor` framework / `olp-connect` zero-config LAN setup / `/health.anonymousKey` opt-in / `/olp` Telegram-Discord plugin / 6-IDE integration docs. Phase 5 scope is open — candidates per ADR 0010 § Out-of-Phase-4-scope: `/v1/messages` (gated on ADR 0009 P0 outcome + named family CC user), context-window-exceeded fallback trigger, per-(provider, model) live stats. Sections marked _placeholder_ land alongside the relevant phase of work (see [phase plan](#phase-plan)). --- @@ -252,9 +252,9 @@ Use a dedicated bot key — not the maintainer's personal owner key — so revoc --- -## Implementation status (as of 2026-05-25, post-v0.2.0) +## Implementation status (as of 2026-05-26, post-v0.4.0) -Phase 1 closed at v0.1.1 (multi-provider proxy core + pre-Phase-2 cleanup). Phase 2 closed at v0.2.0 (multi-key auth + audit + owner gating + keygen CLI; ADR 0007 § 10 all 11 acceptance criteria shipped). Phase 3 closed at v0.3.0 (Dashboard + `lib/audit-query.mjs` + daily audit rotation; ADR 0008 § 10 all 15 acceptance criteria shipped). Phase 4 (per-key per-provider auth + audit retention + SQLite hybrid + provider-cost weights) is the next planned milestone. This table reflects what is currently shipped vs. what is designed for later phases. +Phase 1 closed at v0.1.1 (multi-provider proxy core + pre-Phase-2 cleanup). Phase 2 closed at v0.2.0 (multi-key auth + audit + owner gating + keygen CLI; ADR 0007 § 10 all 11 acceptance criteria shipped). Phase 3 closed at v0.3.0 (Dashboard + `lib/audit-query.mjs` + daily audit rotation; ADR 0008 § 10 all 15 acceptance criteria shipped). Phase 4 closed at v0.4.0 (Operator + Client UX per ADR 0010: SSE heartbeat + `recentErrors[20]` + `/v0/management/status` / `olp` Node CLI + `olp doctor` framework + ADR 0002 Amendment 7 / `olp-connect` bash + `/health.anonymousKey` + ADR 0011 / `olp-plugin/` Telegram-Discord + 6-IDE integration docs). Phase 5 scope is open — candidates per ADR 0010 § Out-of-Phase-4-scope. This table reflects what is currently shipped vs. what is designed for later phases. | File / artifact | Status | Notes | |---|---|---| @@ -289,7 +289,9 @@ Behaviors that work correctly at personal/family scale but have ratified follow- - **Soft triggers configured but inert.** `routing.soft_triggers` in `~/.olp/config.json` is honored by the engine's evaluation logic but `quotaStatus()` polling is not wired (ADR 0004 Amendment 2). A startup warning fires if the field is non-empty so the inert state is visible. - **Multi-key auth + owner gating + keygen CLI shipped at v0.2.0 (D44 + D45 + D46 + D47).** `lib/keys.mjs` (core), `lib/audit.mjs` (audit), owner-vs-guest `/health` payload trimming + `X-OLP-Fallback-Detail` policy gating, `bin/olp-keys.mjs` (keygen CLI). All 11 ADR 0007 § 10 acceptance criteria covered. v0.2.0 maintainer-merged 2026-05-25. -- **Phase 3 (Dashboard + audit query layer + rotation) shipped to main (D48-D54); v0.3.0 release pending.** `docs/adr/0008-dashboard-and-audit-query.md` ratified at D48. `lib/audit-query.mjs` (D49) implements the 5-function aggregate query API (in-memory ndjson scan, PII-guarded). 4 new owner-only_block endpoints at D50 (`/dashboard`, `/v0/management/dashboard-data`, `/v0/management/quota`, `/cache/stats`). `dashboard.html` full multi-panel UI at D51 (vanilla HTML+JS+fetch, 30s poll with visibilitychange pause). Daily audit rotation at D52 (synchronous on first append after UTC midnight; `audit-YYYY-MM-DD.ndjson` naming) + optional `bin/olp-audit-rotate.mjs` cron tool. `tried_providers` schema semantic fix at D53 (D45 P2 deferral). Phase 3 close to v0.3.0 is maintainer-triggered per CLAUDE.md `release_kit.phase_close_trigger`. +- **Phase 3 (Dashboard + audit query layer + rotation) shipped at v0.3.0 (D48–D54).** `docs/adr/0008-dashboard-and-audit-query.md` + `lib/audit-query.mjs` (D49) + 4 owner-only_block endpoints (D50) + `dashboard.html` (D51) + daily audit rotation (D52) + `tried_providers` schema fix (D53). All 15 ADR 0008 § 10 acceptance criteria covered. + +- **Phase 4 (Operator + Client UX) shipped at v0.4.0 (D60 → D73).** ADR 0010 (charter) + ADR 0011 (anonymous-key trusted-LAN limits) + ADR 0002 Amendment 7 (provider `doctorChecks()` contract). Default `OLP_PORT` 3456 → 4567 so OLP and OCP can co-host. SSE heartbeat (D61) + `recentErrors[20]` + `/v0/management/status` (D62-D63). `bin/olp.mjs` Node CLI + `bin/olp-keys.mjs` + `lib/doctor.mjs` framework with `next_action.ai_executable[]` (D64-D67). `bin/olp-connect` bash zero-config IDE auto-config + opt-in `/health.anonymousKey` (D68-D70). `olp-plugin/` OpenClaw `/olp` Telegram-Discord plugin (read-only, no chat mutations) + 6 IDE integration docs at `docs/integrations/*.md` (D71-D73). Test count 623 → 696. **Bootstrap workflow (D47):** for first-run / production setup: diff --git a/bin/olp-connect b/bin/olp-connect index 9f5f75e..cfb55c5 100755 --- a/bin/olp-connect +++ b/bin/olp-connect @@ -114,6 +114,35 @@ key_display() { fi } +# D74 P1-2: validate OLP API key format. Per ADR 0007 § 3, tokens are +# `olp_` + 32 random bytes base64url-encoded (43 chars, no padding). This +# regex pins the on-the-wire shape so a malformed or hostile `--key` / +# server-advertised `anonymousKey` never gets persisted into a shell rc. +# Returns 0 on valid, 1 on invalid (with diagnostic to stderr). +validate_olp_token() { + local k="$1" source="$2" + if [[ ! "$k" =~ ^olp_[A-Za-z0-9_-]{43}$ ]]; then + log_err "Rejected $source: token format does not match ^olp_[A-Za-z0-9_-]{43}$ (ADR 0007 § 3)." + log_err " Got ${#k}-char value starting with '$(echo "$k" | cut -c1-8)...'" + log_err " Expected: olp_ followed by 43 base64url chars. Run 'npx olp-keys list' on the server" + log_err " to confirm the key format, or have the operator regenerate with 'npx olp-keys keygen'." + return 1 + fi + return 0 +} + +# D74 P1-2: POSIX shell-quote a value before interpolating into a shell rc +# write. Wraps in single quotes + escapes embedded single quotes per: +# foo'bar → 'foo'\''bar' +# Even with the validator above, this is defense-in-depth: any non-token +# string that slips through (e.g., environment.d KEY=VALUE writes) MUST be +# safe to source. Same helper pattern as lib/doctor.mjs _shellQuote. +shell_quote() { + local s="$1" + # Escape any single quotes: ' → '\'' + printf "'%s'" "${s//\'/\'\\\'\'}" +} + # Detect Claude Code and print warn-only message. Per ADR 0010 § Out of # Phase 4 scope, OLP does NOT ship /v1/messages and CC is not a supported # client. The user is steered toward Cline + OLP. @@ -297,17 +326,20 @@ append_olp_block() { if $DRY_RUN; then log_change "[dry-run] would append OLP block to $rc_file:" log_change " # OLP LAN (added by olp-connect)" - log_change " export OPENAI_BASE_URL=$base_url/v1" - [[ -n "$key" ]] && log_change " export OPENAI_API_KEY=$(key_display "$key")" + log_change " export OPENAI_BASE_URL=$(shell_quote "$base_url/v1")" + [[ -n "$key" ]] && log_change " export OPENAI_API_KEY=$(shell_quote "$(key_display "$key")")" log_change " # /OLP LAN" return 0 fi + # D74 P1-2: shell-quote values before writing to rc files. Defense-in-depth + # alongside validate_olp_token — even if a future code path bypasses the + # validator, the rc file remains safe to source. { echo "" echo "# OLP LAN (added by olp-connect)" - echo "export OPENAI_BASE_URL=$base_url/v1" + echo "export OPENAI_BASE_URL=$(shell_quote "$base_url/v1")" if [[ -n "$key" ]]; then - echo "export OPENAI_API_KEY=$key" + echo "export OPENAI_API_KEY=$(shell_quote "$key")" fi echo "# /OLP LAN" } >> "$rc_file" @@ -341,6 +373,14 @@ set_system_env() { return 0 fi mkdir -p "$env_dir" 2>/dev/null + # D74 P1-2: systemd environment.d format is KEY=VALUE per line. While + # systemd does its own parsing (no shell sourcing), reject embedded + # newlines defensively — validate_olp_token already enforces the + # restricted charset for the API key, so this is belt-and-braces. + if [[ "$base_url" == *$'\n'* || "$key" == *$'\n'* ]]; then + log_err "Refusing to write environment.d entry: value contains newline." + return 2 + fi { echo "OPENAI_BASE_URL=$base_url/v1" if [[ -n "$key" ]]; then @@ -363,8 +403,13 @@ main() { --port=*) port="${1#*=}"; shift ;; --key) key="${2:?--key requires a value}" [[ -z "$key" ]] && { log_err "--key cannot be empty (omit --key for zero-config / auto-discovery)"; exit 1; } + # D74 P1-2: reject malformed --key before it ever reaches an rc write. + validate_olp_token "$key" "--key flag" || exit 1 shift 2 ;; - --key=*) key="${1#*=}"; shift ;; + --key=*) key="${1#*=}" + [[ -z "$key" ]] && { log_err "--key cannot be empty (omit --key for zero-config / auto-discovery)"; exit 1; } + validate_olp_token "$key" "--key flag" || exit 1 + shift ;; --no-system-env) NO_SYSTEM_ENV=true; shift ;; --dry-run) DRY_RUN=true; shift ;; --version) show_version; exit 0 ;; @@ -457,6 +502,13 @@ try: print(k if isinstance(k, str) and k else '') except: print('')" 2>/dev/null || echo "") if [[ -n "$anon_key" ]]; then + # D74 P1-2: validate server-advertised token shape before consuming. + # A hostile or misconfigured server could otherwise inject arbitrary + # strings into the user's rc file via the `anonymousKey` field. + if ! validate_olp_token "$anon_key" "/health.anonymousKey from $remote_host"; then + log_err "Refusing to consume malformed advertised key. Use --key explicitly or contact the OLP operator." + exit 2 + fi key="$anon_key" log_ok "Using server-advertised anonymous key: $(key_display "$key")" log_info " (set by remote via auth.advertise_anonymous_key=true; see ADR 0011 for" @@ -479,6 +531,8 @@ except: print('')" 2>/dev/null || echo "") log_err " Re-run with: olp-connect $host --key olp_..." exit 2 fi + # D74 P1-2: also validate the interactively-prompted key. + validate_olp_token "$key" "interactive prompt" || exit 1 fi fi fi diff --git a/bin/olp.mjs b/bin/olp.mjs index 47b18c4..64604cc 100755 --- a/bin/olp.mjs +++ b/bin/olp.mjs @@ -301,29 +301,50 @@ async function cmdUsage(flags, io) { try { body = JSON.parse(res.body); } catch { io.errln('Error: server returned non-JSON body'); return 2; } if (io.wantJson) { io.emitJson(body); return 0; } + // D74 P2-3 fix: server payload shape is { generated_at, window_24h: { request_count, status_2xx, + // status_4xx, status_5xx, by_provider, by_owner_tier, by_path, median_latency_ms, p95_latency_ms }, + // cache_hit_24h: { total, hit, miss, bypass, streaming_attached, hit_rate, by_provider }, quota: [{provider, ...}], + // spend_trend_30d: [{date, request_count, by_provider}], top_fallback_chains_24h: [{chain, count, ...}], + // cache_stats: { hits, misses, size, inflightCount } } per server.mjs:2027 + lib/audit-query.mjs. io.log(colorize('OLP usage (24h)', ANSI.bold, io.useColor)); io.log('─'.repeat(60)); - const u24 = body.usage_24h ?? body.usage24h ?? body['24h'] ?? {}; - if (typeof u24 === 'object' && Object.keys(u24).length > 0) { - io.log(` requests: ${u24.requests ?? '?'}`); - io.log(` cache hits: ${u24.cache_hits ?? '?'}`); - io.log(` fallbacks: ${u24.fallbacks ?? '?'}`); + const w24 = body.window_24h ?? {}; + const cache24 = body.cache_hit_24h ?? {}; + if (typeof w24 === 'object' && (w24.request_count ?? 0) > 0) { + io.log(` requests: ${w24.request_count}`); + io.log(` 2xx / 4xx / 5xx: ${w24.status_2xx ?? 0} / ${w24.status_4xx ?? 0} / ${w24.status_5xx ?? 0}`); + if (typeof w24.median_latency_ms === 'number') { + io.log(` latency p50/p95: ${w24.median_latency_ms}ms / ${w24.p95_latency_ms ?? 0}ms`); + } + if (typeof cache24.hit_rate === 'number') { + const pct = (cache24.hit_rate * 100).toFixed(1); + io.log(` cache hit rate: ${pct}% (hit=${cache24.hit ?? 0} miss=${cache24.miss ?? 0}${cache24.streaming_attached ? ` streaming_attached=${cache24.streaming_attached}` : ''})`); + } } else { io.log(' (no 24h usage data — server may not have processed any requests yet)'); } - if (Array.isArray(body.providers)) { + if (Array.isArray(body.quota) && body.quota.length > 0) { io.log(''); io.log(colorize('Per-provider quota', ANSI.bold, io.useColor)); io.log('─'.repeat(60)); - for (const p of body.providers) { - io.log(` ${String(p.name ?? '?').padEnd(12)} ${p.percent_used != null ? `${p.percent_used}% used` : 'no quota api'}`); + for (const p of body.quota) { + const label = String(p.provider ?? '?').padEnd(12); + if (p.error) { + io.log(` ${label} error: ${p.error}`); + } else if (typeof p.percent_used === 'number') { + io.log(` ${label} ${p.percent_used}% used${p.resets_in_human ? ` (resets in ${p.resets_in_human})` : ''}`); + } else if (p.available === false) { + io.log(` ${label} unavailable`); + } else { + io.log(` ${label} no quota api`); + } } } - if (Array.isArray(body.top_fallback_chains)) { + if (Array.isArray(body.top_fallback_chains_24h) && body.top_fallback_chains_24h.length > 0) { io.log(''); - io.log(colorize('Top fallback chains', ANSI.bold, io.useColor)); + io.log(colorize('Top fallback chains (24h)', ANSI.bold, io.useColor)); io.log('─'.repeat(60)); - for (const f of body.top_fallback_chains.slice(0, 10)) { + for (const f of body.top_fallback_chains_24h.slice(0, 10)) { io.log(` ${String(f.count ?? '?').padStart(5)} ${(f.chain ?? []).join(' → ')}`); } } @@ -360,13 +381,20 @@ async function cmdCache(flags, io) { try { body = JSON.parse(res.body); } catch { io.errln('Error: server returned non-JSON body'); return 2; } if (io.wantJson) { io.emitJson(body); return 0; } - io.log(colorize('OLP cache', ANSI.bold, io.useColor)); + // D74 P2-3 fix: cacheStore.stats() returns { hits, misses, size, inflightCount } + // per lib/cache/store.mjs:320. There is no entries / evictions / bytes / maxBytes + // in the OLP cache model — those were OCP-era field names. Compute a hit rate from + // the numerator/denominator instead of fabricating bytes. + const hits = body.hits ?? 0; + const misses = body.misses ?? 0; + const denom = hits + misses; + const hitRate = denom > 0 ? ((hits / denom) * 100).toFixed(1) : '0.0'; + io.log(colorize('OLP cache (live in-memory)', ANSI.bold, io.useColor)); io.log('─'.repeat(60)); - io.log(` entries: ${body.entries ?? '?'}`); - io.log(` hits: ${body.hits ?? 0}`); - io.log(` misses: ${body.misses ?? 0}`); - io.log(` evictions:${body.evictions ?? 0}`); - io.log(` bytes: ${formatBytes(body.bytes ?? 0)} (max ${formatBytes(body.maxBytes ?? 0)})`); + io.log(` entries: ${body.size ?? 0}`); + io.log(` hits / misses: ${hits} / ${misses} (hit rate ${hitRate}%)`); + io.log(` inflight: ${body.inflightCount ?? 0}`); + if (body.generated_at) io.log(` generated_at: ${body.generated_at}`); return 0; } @@ -574,10 +602,16 @@ async function cmdDoctor(flags, io) { const proxyUrl = resolveProxyUrl({ proxyUrl: flags['proxy-url'] }); const checkFilter = typeof flags.check === 'string' ? flags.check : undefined; + // D74 P1-1: pass authHeaders so server.running / server.version checks + // succeed under the default production posture (auth.allow_anonymous: + // false). resolveBearerToken returns null when no env var is set; the + // doctor still runs but distinguishes 401 from "server down" by status + // code per the updated check. const result = await runDoctor({ olpHome, proxyUrl, checkFilter, + authHeaders: authHeaders(), }); if (io.wantJson) { diff --git a/lib/doctor.mjs b/lib/doctor.mjs index 5e91dce..c27eb5b 100644 --- a/lib/doctor.mjs +++ b/lib/doctor.mjs @@ -144,6 +144,15 @@ export function buildBuiltinChecks(opts = {}) { const olpHome = resolveOlpHome(opts); const configPath = join(olpHome, 'config.json'); const proxyUrl = resolveProxyUrl(opts); + // D74 P1-1 fix: server.running and server.version probe /health, which + // under default production posture (auth.allow_anonymous: false) requires + // an Authorization: Bearer header. Without it, the probe gets 401 and + // doctor falsely reports the server down. Caller passes the resolved + // bearer token via opts.authHeaders (a `{Authorization: 'Bearer ...'}` + // object). Empty headers means "no token configured" — the probe still + // fires but a 401 response is treated as "auth misconfigured" rather + // than "server down" (see server.running check below). + const authHeaders = opts.authHeaders ?? {}; const checks = []; @@ -298,7 +307,9 @@ export function buildBuiltinChecks(opts = {}) { id: 'server.running', category: 'server', async run() { - const r = await httpGet(`${proxyUrl}/health`, { timeoutMs: 3000 }); + // D74 P1-1: pass authHeaders so the probe works under the default + // production posture (auth.allow_anonymous: false). + const r = await httpGet(`${proxyUrl}/health`, { timeoutMs: 3000, headers: authHeaders }); if (!r.ok) { return { status: 'fail', @@ -311,6 +322,25 @@ export function buildBuiltinChecks(opts = {}) { }, }; } + // 401: server is up but the caller has no/wrong bearer token. NOT a + // "server down" condition — distinguish so the kind discriminator + // doesn't route to fix_server when the user just needs OLP_API_KEY. + if (r.status === 401 || r.status === 403) { + return { + status: 'fail', + message: `${proxyUrl}/health returned ${r.status} — server is up but the bearer token is missing or invalid. Set OLP_API_KEY env to an owner-tier token (npx olp-keys list).`, + evidence: { + fix_commands: [ + 'echo "set OLP_API_KEY= or OLP_OWNER_TOKEN=<...> then rerun olp doctor"', + ], + human_required: [ + 'Locate an owner-tier OLP API key plaintext (or run `npx olp-keys keygen --owner` to mint a new one — printed ONCE).', + 'Export it: `export OLP_API_KEY=olp_...`', + ], + reference: 'docs/adr/0007-multi-key-auth.md § 9.1 + README § Environment Variables', + }, + }; + } if (r.status !== 200) { return { status: 'fail', message: `${proxyUrl}/health returned status=${r.status}` }; } @@ -333,7 +363,8 @@ export function buildBuiltinChecks(opts = {}) { } catch { return { status: 'warn', message: 'Could not read local package.json — skipping version comparison' }; } - const r = await httpGet(`${proxyUrl}/health`, { timeoutMs: 3000 }); + // D74 P1-1: same auth-headers fix as server.running. + const r = await httpGet(`${proxyUrl}/health`, { timeoutMs: 3000, headers: authHeaders }); if (!r.ok || r.status !== 200) { return { status: 'warn', message: `Could not fetch /health to compare version (${r.error ?? `status ${r.status}`})` }; } diff --git a/olp-plugin/index.js b/olp-plugin/index.js index 2e59e75..ec81185 100644 --- a/olp-plugin/index.js +++ b/olp-plugin/index.js @@ -138,12 +138,32 @@ export function fmtHealth(body) { if (body.uptime_human || body.uptimeHuman) { out += `Uptime: ${body.uptime_human ?? body.uptimeHuman}\n`; } + // D74 P2-4 fix: server.mjs /health full payload is + // body.providers = { enabled: N, available: N, status: { : {...} } } + // The plugin previously iterated Object.entries(body.providers), which + // surfaced `enabled`, `available`, and `status` as pseudo-providers + // (typeof status === 'object' → loop body fired with name='status'). + // Walk providers.status when present; fall back to providers.* for the + // older OCP shape that lacks the .status wrapper. if (body.providers && typeof body.providers === "object") { - out += `\nProviders:\n`; - for (const [name, s] of Object.entries(body.providers)) { - if (typeof s !== "object" || s === null) continue; - const i = statusIcon(s?.ok ? "ok" : "fail"); - out += ` ${i} ${name}\n`; + const enabled = body.providers.enabled; + const available = body.providers.available; + if (typeof enabled === "number" || typeof available === "number") { + out += `Providers: ${enabled ?? "?"} enabled / ${available ?? "?"} available\n`; + } + const statusMap = body.providers.status && typeof body.providers.status === "object" + ? body.providers.status + : body.providers; + const entries = Object.entries(statusMap).filter( + ([name, s]) => typeof s === "object" && s !== null && name !== "enabled" && name !== "available" && name !== "status" + ); + if (entries.length > 0) { + out += `\nProviders:\n`; + for (const [name, s] of entries) { + const i = statusIcon(s?.ok ? "ok" : "fail"); + const spawn = typeof s?.activeSpawns === "number" ? ` spawns=${s.activeSpawns}` : ""; + out += ` ${i} ${name}${spawn}\n`; + } } } return out; diff --git a/package.json b/package.json index 2e7fd70..1c13ddd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "olp", - "version": "0.4.0", + "version": "0.4.1", "description": "Personal multi-provider LLM proxy. Successor to OCP. One HTTP endpoint, multiple subscriptions behind it, automatic routing + fallback + caching.", "type": "module", "main": "server.mjs", diff --git a/server.mjs b/server.mjs index add5040..7233dee 100644 --- a/server.mjs +++ b/server.mjs @@ -2241,8 +2241,11 @@ if (isMain) { const server = createOlpServer(); server.listen(PORT, '127.0.0.1', () => { const enabledCount = loadedProviders.size; + // D74 P3-5: banner no longer hardcodes the phase. Derives from VERSION + // (which advances at every Phase close) so banner stays accurate + // without future-maintenance touch-ups at every Phase boundary. process.stdout.write( - `OLP v${VERSION} listening on :${PORT} (${enabledCount} providers enabled — Phase 1 in progress)\n`, + `OLP v${VERSION} listening on :${PORT} (${enabledCount} providers enabled)\n`, ); }); } diff --git a/test-features.mjs b/test-features.mjs index e72b3a5..b3dcc6a 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -14791,3 +14791,196 @@ describe('Suite 35 — D71 olp-plugin/ smoke tests (ADR 0010 § Phase 4 D71-D73) assert.match(r.text, /not owner-tier/); }); }); + +// ── Suite 36: D74 v0.4.1 hotfix regression tests (maintainer-review findings) ─ +// +// These tests pin the FIVE issues maintainer caught in independent post-v0.4.0 +// review that previous D-day reviewers all missed (because they reviewed against +// spec text, not runtime contracts). Each test exercises a real codepath that, +// pre-D74, would have produced the documented wrong behavior. + +import { writeFileSync as _writeFileSyncS36, readFileSync as _readFileSyncS36, mkdtempSync as _mkdtempSyncS36, rmSync as _rmSyncS36 } from 'node:fs'; +import { tmpdir as _tmpdirS36 } from 'node:os'; +import { join as _joinS36 } from 'node:path'; +import { spawnSync as _spawnSyncS36 } from 'node:child_process'; + +describe('Suite 36 — D74 v0.4.1 hotfix regression (maintainer review findings)', () => { + it('36a (P1-1) — runDoctor accepts authHeaders + passes them to server.running probe', async () => { + // Pre-D74: lib/doctor.mjs called httpGet(`${proxyUrl}/health`) with no + // headers, so under default `auth.allow_anonymous: false` the probe got + // 401 and reported "server down" — false negative. D74 threads + // authHeaders through buildBuiltinChecks. This test confirms the wire. + const { buildBuiltinChecks } = await import('./lib/doctor.mjs'); + const tmpHome = _mkdtempSyncS36(_joinS36(_tmpdirS36(), 'olp-d74a-')); + const checks = buildBuiltinChecks({ + olpHome: tmpHome, + proxyUrl: 'http://127.0.0.1:1', // closed port — probe fails with ECONN + authHeaders: { Authorization: 'Bearer olp_FAKE' }, + }); + const serverRunning = checks.find(c => c.id === 'server.running'); + assert.ok(serverRunning, 'server.running check must exist'); + // We can't easily intercept httpGet here without DI, but the check at + // least accepts the opts and runs. Real wire-level coverage is 36b below. + _rmSyncS36(tmpHome, { recursive: true, force: true }); + }); + + it('36b (P1-1) — server.running distinguishes 401 (server up, bad token) from "server down"', async () => { + __setAuthConfig({ allow_anonymous: false, owner_only_endpoints: ['/health'] }); + const serverMod = await import('./server.mjs'); + const s = serverMod.createOlpServer(); + let port36b = 28300 + Math.floor(Math.random() * 100); + await new Promise((resolve, reject) => { + s.listen(port36b, '127.0.0.1', resolve); + s.once('error', e => { + if (e.code === 'EADDRINUSE') { port36b++; s.listen(port36b, '127.0.0.1', resolve); s.once('error', reject); } + else reject(e); + }); + }); + + const { runDoctor } = await import('./lib/doctor.mjs'); + try { + const result = await runDoctor({ + olpHome: process.env.OLP_HOME, + proxyUrl: `http://127.0.0.1:${port36b}`, + // intentionally NO authHeaders — simulates user without OLP_API_KEY + }); + const sr = result.checks.find(c => c.id === 'server.running'); + assert.ok(sr, 'server.running present'); + assert.equal(sr.status, 'fail', 'fails because no token'); + assert.match(sr.message, /401|bearer token/i, + `must mention 401 / bearer token specifically — got: ${sr.message}`); + assert.ok(!/unreachable/i.test(sr.message), + '"unreachable" would falsely imply server is down (P1-1 bug)'); + } finally { + await new Promise(r => s.close(r)); + __resetAuthConfig(); + } + }); + + it('36c (P1-2) — olp-connect rejects malformed --key', () => { + // The bash validator: ^olp_[A-Za-z0-9_-]{43}$. Anything else is rejected + // before it can touch a shell rc file or environment.d entry. + const result = _spawnSyncS36('bash', [ + _joinS36(import.meta.dirname ?? process.cwd(), 'bin/olp-connect'), + '127.0.0.1', + '--port', '1', + '--key', 'not-an-olp-token', + '--dry-run', + ], { encoding: 'utf8', timeout: 5000 }); + assert.equal(result.status, 1, `expected exit 1 for malformed --key, got ${result.status}; stderr=${result.stderr.slice(0, 300)}`); + assert.match(result.stderr + result.stdout, /token format does not match/i, + 'must surface the validator error'); + }); + + it('36d (P1-2) — olp-connect accepts a properly-formed olp_ token', () => { + // Pad to 43 base64url chars after olp_. Don't actually hit the server — + // use --port 1 (closed) which makes the connectivity probe fail at + // the connect step (exit 2), but only AFTER the --key validator passes. + const goodKey = 'olp_' + 'A'.repeat(43); + const result = _spawnSyncS36('bash', [ + _joinS36(import.meta.dirname ?? process.cwd(), 'bin/olp-connect'), + '127.0.0.1', + '--port', '1', + '--key', goodKey, + '--dry-run', + ], { encoding: 'utf8', timeout: 5000 }); + // Validator passes → reaches connectivity step → exits 2 (or whatever + // bash propagation gives) but NOT exit 1 from the validator. + assert.notEqual(result.status, 1, `expected validator to pass; got exit 1 with stderr=${result.stderr.slice(0, 300)}`); + assert.ok(!/token format does not match/i.test(result.stderr + result.stdout), + 'validator must NOT trigger for a well-formed token'); + }); + + it('36e (P2-3) — CacheStore.stats() returns {hits, misses, size, inflightCount} shape', async () => { + // Real cacheStore.stats() returns { hits, misses, size, inflightCount } per + // lib/cache/store.mjs. Pre-D74 cmdCache read body.entries / body.bytes / + // body.maxBytes — all undefined → output showed "entries: ?" and "bytes: 0". + // Pin the shape so a future server-side rename can't reintroduce the bug. + // Use the CacheStore class directly (no HTTP) since the field names ARE the + // contract — server.mjs handleCacheStats just sendJSON(s, 200, cacheStore.stats()). + const { CacheStore: CS36 } = await import('./lib/cache/store.mjs'); + const cs = new CS36(); + const stats = cs.stats(); + assert.ok('size' in stats, 'CacheStore.stats() shape: size field present'); + assert.ok('hits' in stats, 'CacheStore.stats() shape: hits field present'); + assert.ok('misses' in stats, 'CacheStore.stats() shape: misses field present'); + assert.ok('inflightCount' in stats, 'CacheStore.stats() shape: inflightCount field present'); + assert.ok(!('entries' in stats), 'must NOT have OCP-era entries field'); + assert.ok(!('bytes' in stats), 'must NOT have OCP-era bytes field'); + assert.ok(!('maxBytes' in stats), 'must NOT have OCP-era maxBytes field'); + // Also pin that cmdCache reads body.size (not body.entries) by grepping the source + const cliSrc = _readFileSyncS36(_joinS36(import.meta.dirname ?? process.cwd(), 'bin/olp.mjs'), 'utf8'); + assert.ok(/body\.size\b/.test(cliSrc), 'cmdCache must read body.size for entries display'); + assert.ok(/body\.inflightCount\b/.test(cliSrc), 'cmdCache must read body.inflightCount'); + assert.ok(!/body\.entries\b/.test(cliSrc), 'cmdCache must NOT read body.entries (OCP-era field)'); + }); + + it('36f (P2-3) — dashboard-data payload + cmdUsage source pin the wire-contract field names', () => { + // server.mjs handleManagementDashboardData (~line 2027) builds the payload + // inline. Pin by grepping the server source: the keys MUST be window_24h / + // cache_hit_24h / quota / spend_trend_30d / top_fallback_chains_24h / + // cache_stats. Pre-D74 cmdUsage read body.usage_24h.requests / body.providers + // / body.top_fallback_chains — none of which exist in the actual payload. + const serverSrc = _readFileSyncS36(_joinS36(import.meta.dirname ?? process.cwd(), 'server.mjs'), 'utf8'); + assert.ok(/window_24h:\s*auditAggregateRequests/.test(serverSrc), + 'handleManagementDashboardData must emit window_24h field'); + assert.ok(/cache_hit_24h:\s*auditCacheHitRateWindow/.test(serverSrc), + 'handleManagementDashboardData must emit cache_hit_24h field'); + assert.ok(/top_fallback_chains_24h:/.test(serverSrc), + 'handleManagementDashboardData must emit top_fallback_chains_24h field (NOT top_fallback_chains)'); + assert.ok(/cache_stats:\s*cacheStore\.stats/.test(serverSrc), + 'handleManagementDashboardData must emit cache_stats field'); + // Now pin cmdUsage reads the matching keys + const cliSrc = _readFileSyncS36(_joinS36(import.meta.dirname ?? process.cwd(), 'bin/olp.mjs'), 'utf8'); + assert.ok(/body\.window_24h\b/.test(cliSrc), 'cmdUsage must read body.window_24h'); + assert.ok(/body\.cache_hit_24h\b/.test(cliSrc), 'cmdUsage must read body.cache_hit_24h'); + assert.ok(/body\.top_fallback_chains_24h\b/.test(cliSrc), + 'cmdUsage must read body.top_fallback_chains_24h (NOT body.top_fallback_chains)'); + assert.ok(/body\.quota\b/.test(cliSrc), 'cmdUsage must read body.quota'); + // Pre-D74 stale reads must be GONE from the CLI source + assert.ok(!/body\.usage_24h\.requests\b/.test(cliSrc), + 'cmdUsage must NOT read body.usage_24h.requests (OCP-era field)'); + }); + + it('36g (P2-4) — olp-plugin fmtHealth iterates providers.status (not providers.*)', async () => { + // Real /health full payload: providers: { enabled, available, status: { anthropic: {...} } } + // Pre-D74 fmtHealth iterated Object.entries(body.providers) and the body + // contained `status: {...}` so the loop printed a pseudo-provider named "status". + const { fmtHealth } = await import('./olp-plugin/index.js'); + const realServerShape = { + ok: true, + version: '0.4.1', + uptime_human: '1m 3s', + providers: { + enabled: 2, + available: 3, + status: { + anthropic: { ok: true, activeSpawns: 0 }, + openai: { ok: false, error: 'oauth-missing', activeSpawns: 0 }, + }, + }, + }; + const out = fmtHealth(realServerShape); + assert.match(out, /anthropic/, 'must list anthropic by name'); + assert.match(out, /openai/, 'must list openai by name'); + // assert.notMatch isn't available on assert/strict in some Node versions; + // use the explicit negation pattern to stay portable. + assert.ok(!/^\s*[🟢🔴⚪]\s*status\s*$/m.test(out), + 'must NOT list "status" as a pseudo-provider (the P2-4 bug)'); + assert.ok(!/^\s*[🟢🔴⚪]\s*enabled\s*$/m.test(out), + 'must NOT list "enabled" as a pseudo-provider'); + assert.ok(!/^\s*[🟢🔴⚪]\s*available\s*$/m.test(out), + 'must NOT list "available" as a pseudo-provider'); + // Also confirm the new enabled/available header + assert.match(out, /2 enabled \/ 3 available/); + }); + + it('36h (P3-5) — server startup banner does not hardcode a stale phase', () => { + // Pre-D74 banner literal said "Phase 1 in progress" forever. + const serverSrc = _readFileSyncS36(_joinS36(import.meta.dirname ?? process.cwd(), 'server.mjs'), 'utf8'); + assert.ok(!/Phase 1 in progress/.test(serverSrc), + 'startup banner must not hardcode "Phase 1 in progress" (stale post v0.4.0)'); + assert.ok(!/Phase 2 in progress/.test(serverSrc)); + assert.ok(!/Phase 3 in progress/.test(serverSrc)); + }); +});