mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-19 09:45:07 +00:00
251b578114ff8c7110736179cb6b2674bb2ac2af
58
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 
|
251b578114 |
feat+test+docs: D51 — dashboard.html full multi-panel UI (Phase 3) (#28)
* feat+test+docs: D51 — dashboard.html full multi-panel UI (Phase 3)
Fourth Phase 3 D-day. Replaces the D50 dashboard.html placeholder with
the full 4-panel UI per ADR 0008 § 6. Vanilla HTML + JS + fetch — no
build step, no framework, no CDN (Lane 1 = A). 30s page poll with
document.visibilityState pause/resume (Lane 4 = A).
4 PANELS (all rendered from /v0/management/dashboard-data — single
backing endpoint per Lane 2 in-memory query model):
Panel 1 — Per-provider quota
Table: { Provider | Available | Status }. Null available → "n/a"
pill; provider.quotaStatus() error → red status pill (graceful
degradation per ADR § 9).
Panel 2 — Last 24h: request count + cache hit + fallback rate
Per-provider row: { Requests | Cache hit % | Fallback rate % }.
Cache hit from cache_hit_24h.by_provider[p].hit_rate; fallback
rate computed from window_24h.by_provider[p].fallback_count/count.
Panel 3 — Request count last 30 days (SVG sparkline)
Vanilla SVG bar chart with <title> tooltips showing per-day per-
provider breakdown. Y-axis: requests per day (max-scaled). X-axis:
30 daily UTC buckets.
Panel 4 — Top fallback chains (last 24h)
Numbered table: { # | Chain (monospace, arrow-joined) | Count |
First seen | Last seen }.
POLL + VISIBILITYCHANGE PAUSE (ADR 0008 § 6.5):
- setInterval(refresh, 30000) after initial fetch.
- document.addEventListener('visibilitychange') → stopPolling() on
hidden / refresh()+startPolling() on visible.
- Prevents 2880 background polls/day per owner when tab hidden.
ERROR HANDLING:
- 401 from dashboard-data → in-page error banner explains owner-tier
requirement + suggests SSH-tunnel + header-injection workaround.
- Other HTTP errors → generic "HTTP <code>" banner; console.warn
for operator debugging.
- Per-panel empty states ("Loading…", "No requests in window.",
"No fallback chains triggered.").
CRITICAL CORRECTNESS INVARIANTS (ADR 0008 § 6 + Lane 1 = A):
- No <script src> — entire JS inline (Suite 25d asserts).
- No <link rel="stylesheet" href> — all CSS in <style> (25d).
- Only one backing endpoint hit: /v0/management/dashboard-data
(Suite 25e asserts).
- 401 path keeps panels in last-good state rather than clearing —
operator sees the error banner + can debug.
TESTS — Suite 25, +6 (582 → 588):
25a: owner /dashboard response contains all 4 panel container IDs
25b: JS declares POLL_INTERVAL_MS = 30000 + setInterval/clearInterval
25c: visibilitychange listener + document.visibilityState check
25d: NO external script src / NO external stylesheet href (Lane 1 = A
pinning)
25e: dashboard JS fetches /v0/management/dashboard-data only
25f: 401 in-page error banner mentions owner-tier guidance
MANUAL SMOKE (ADR 0008 § 10 #12):
Dashboard renders without console errors in a real browser when
served by a running OLP instance + owner-tier Bearer via SSH-tunnel
+ header-injection extension. Not automated at Phase 3.
DOCUMENTATION:
- AGENTS.md: dashboard.html marker promoted 🟡 D50 placeholder → ✅
D51 full UI.
- CHANGELOG.md: D51 entry under Unreleased per release_kit overlay.
NOT IN D51:
- Daily audit rotation (D52)
- tried_providers schema fix (D53; D45 P2 deferral)
- Phase 3 close (D55; v0.3.0; maintainer-triggered)
Test count: 582 → 588 (+6). Verified locally via npm test.
AUTHORITY:
- ADR 0008 § 6 (panels + refresh + localhost) + § 6.5 (poll +
visibilityState pause) + Lane 1 = A (no build step) + Lane 4 = A
(30s poll) + Lane 5 = B (full 4-panel scope).
- ADR 0008 § 9 (graceful degradation surfaced in Panel 1).
- ADR 0008 § 10 #12 (HTML smoke criterion satisfied at server-side
level).
- CLAUDE.md release_kit overlay phase_rolling_mode — under
Unreleased.
- Standing autopilot grant.
ALIGNMENT.md scope check: this PR replaces an existing entry-surface
static file (dashboard.html). Per Rule 5: management surface, not
OpenAI-spec-compatible — outside /v1/* spec scope. No code change in
server.mjs / lib/ / providers / IR / models-registry.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix: D51 fold-in — dashboard.html cosmetic polish (opus P3)
Fresh-context opus reviewer (PR #28) flagged 4 P3 cosmetic findings;
addressing the 2 trivial ones inline. The other 2 (visibilitychange
race + defensive date null-check) are negligible at family-scale per
reviewer; deferred to Phase 4 if UX feedback warrants.
- Line ~193: deleted orphan empty <text> SVG element (no textContent,
rendered nothing — debris from initial pass).
- Line ~200 comment: was "Date labels (first / mid / last)" but only
first + last rendered. Tightened to clarify intent + note mid label
deferred to Phase 4 if needed.
No behavior change. 588/588 tests pass.
Authority: PR #28 fresh-context opus reviewer P3 findings (2 of 4
addressed; remaining 2 documented as negligible).
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>
|
||
|
|
f9f2eaa059 |
feat+test+docs: D50 — server.mjs management endpoints (Phase 3 dashboard wire-up) (#27)
* feat+test+docs: D50 — server.mjs management endpoints (Phase 3 dashboard wire-up)
Third Phase 3 D-day. Wires the D49 lib/audit-query.mjs aggregate query
layer into 4 owner_only_block HTTP endpoints per ADR 0008 §§ 7-8.
Ships a placeholder dashboard.html at repo root (D51 lands the full
multi-panel UI). All endpoints follow the Phase 2 / D45 auth + audit
+ touchLastUsed pattern.
4 NEW ENDPOINTS — all owner_only_block per ADR 0008 § 8:
GET /dashboard
Serves dashboard.html (text/html). D50 stub explains state +
lists backing endpoints. D51 replaces with full UI.
GET /v0/management/dashboard-data
Full aggregate per § 7.2:
{ generated_at, window_24h, cache_hit_24h, quota,
spend_trend_30d, top_fallback_chains_24h, cache_stats }
GET /v0/management/quota
Quota subset only (per-provider provider.quotaStatus + error
capture per § 9 graceful degradation).
GET /cache/stats
Live in-memory cacheStore.stats() with generated_at wrapper.
HELPER:
_runOwnerOnlyManagementEndpoint(req, res, method, path, inner)
Factors common auth + audit ctx + owner-block + res.on('finish')
wire. inner is async (req, res, olpIdentity, auditCtx) → void.
Eliminates 4× boilerplate.
OWNER_ONLY_BLOCK MODE (ADR 0008 § 8 D48-fold-in):
authenticate → if owner_tier !== 'owner' → 401 owner_required.
Distinct from owner_only_trim (Phase 2 /health pattern). Anonymous
identity (when allow_anonymous: true) REACHES the handler and is
401'd by the owner check (Suite 24c). Allow_anonymous: false + no
header → 401 auth_required at middleware (Suite 24d).
PROVIDER QUOTASTATUS ERROR CAPTURE:
Dashboard-data + quota endpoints catch per-provider throws and
surface { provider, error, available: null } so one bad provider
doesn't fail the whole panel (ADR 0008 § 9 graceful degradation).
DASHBOARD.HTML PLACEHOLDER (~50 lines at repo root):
Explains D50 state, lists backing endpoints with curl example.
Cached in memory at first /dashboard request via _loadDashboardHtml
with module-scope _dashboardHtmlCache; falls back to in-memory stub
if file missing (defensive for test imports from non-repo cwd).
AUDIT ON MANAGEMENT ENDPOINTS (ADR 0008 § 7.5):
Every management request appends audit row including 401 paths
(verified by Suite 24j). Touch wire skips anonymous + env-owner
identities (matches Phase 2 pattern).
TESTS — Suite 24, +11 (571 → 582):
24a-d: /dashboard owner_only_block matrix (owner 200 / guest 401
/ anonymous-with-allow_anonymous=true 401 / no-auth-with-
allow_anonymous=false 401)
24e: dashboard-data owner → 200 JSON with all § 7.2 fields
(asserts spend_trend_30d.length === 30)
24f: dashboard-data guest → 401 owner_required
24g: quota owner → 200 JSON with quota array
24h: cache/stats owner → 200 JSON shape
24h-401: cache/stats guest → 401
24i: successful dashboard-data appends audit row with status 200
+ key_id + correct path
24j: 401 (guest blocked) dashboard-data appends audit row with
error_code: 'owner_required' + owner_tier: 'guest'
DOCUMENTATION:
- AGENTS.md: dashboard.html new entry (D50 placeholder); lib/audit-
query.mjs marker note unchanged.
- CHANGELOG.md: D50 entry under Unreleased per release_kit overlay.
NOT IN D50 scope:
- Full dashboard UI (D51 — replaces dashboard.html with the real
4-panel layout + 30s poll JS)
- Daily audit rotation (D52)
- tried_providers schema fix (D53)
- Phase 3 close (D55; v0.3.0; maintainer-triggered)
Test count: 571 → 582 (+11). Verified locally via npm test.
AUTHORITY:
- ADR 0008 § 7 (endpoint definitions) + § 8 (owner_only_block
mode) + § 9 (graceful degradation) + § 7.5 (audit on management
endpoints).
- ADR 0007 § 7 (auth model reused).
- ADR 0002 § Provider contract (quotaStatus).
- ADR 0005 (cacheStore.stats source of truth).
- CLAUDE.md release_kit overlay phase_rolling_mode — under
Unreleased.
- Standing autopilot grant.
ALIGNMENT.md scope check: this PR adds 4 new entry-surface endpoints
under owner-only_block gating + a new lib/audit-query consumer surface.
Per Rule 5: management endpoints are owner-only operational surface,
not OpenAI-spec-compatible — they exist outside the /v1/chat/completions
+ /v1/models spec scope. No provider plugin / IR change.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs: D50 fold-in — AGENTS.md dashboard.html duplicate (opus P3)
Fresh-context opus reviewer (PR #27) flagged a duplicate dashboard.html
entry: my D50 addition was added directly above a stale
"Planned (Phase 6) — not yet authored" line that should have been
removed. The file contradicted itself.
Fix: merge into single entry — keep the original line phrasing and
attach the D50 status update.
No code change, no test change.
Authority: PR #27 fresh-context opus reviewer P3 finding.
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>
|
||
|
|
686794e316 |
feat+test+docs: D49 — lib/audit-query.mjs (Phase 3 audit aggregate query layer) (#26)
Second Phase 3 D-day. Implements ADR 0008 § 4 query API. Pure in-memory
ndjson scan; cross-file walk over audit.ndjson (live) +
audit-YYYY-MM-DD.ndjson (rotated). No server.mjs integration in this
D-day (D50 wires the consuming endpoints).
NEW lib/audit-query.mjs (~370 lines): 5 public API functions per
ADR 0008 § 4.1:
- discoverAuditFiles({ olpHome }): filesystem scan; returns
Map<date|'live', path>.
- readAuditWindow({ startMs, endMs, olpHome, logEvent }): generator
over events in half-open window [startMs, endMs). Walks rotated
date files + live file. Skips malformed lines + logs warn.
- aggregateRequests({ windowMs, olpHome }): counts + status buckets
+ by_provider + by_owner_tier + by_path + median/p95 latency over
rolling window.
- topFallbackChains({ windowMs, limit, olpHome }): top-N chains by
trigger count from events with fallback_hops > 0. Tied-count
tiebreak: ascending first_seen.
- spendTrendDaily({ days, olpHome }): daily series ending today
with sparse-fill for zero-request days. Per-day request_count +
median latency + by_provider breakdown.
- cacheHitRateWindow({ windowMs, olpHome }): audit-derived cache
hit rate (bypass excluded from denominator); per-provider + overall.
PII discipline (ADR 0008 § 4.3): every aggregate function relays only
schema fields; never message content. Suite 23g actively asserts the
absence of content/message/messages/prompt/response/body keys in every
aggregate output.
Cross-file walk semantics (ADR 0008 § 4.2): half-open window
[startMs, endMs); date-range computed once from window bounds; each
rotated date file checked; live audit.ndjson always checked (it
covers today regardless of whether the window endpoint is past
midnight).
spendTrendDaily calendar-date semantics:
days: N returns "last N calendar UTC dates ending today" — NOT
"events within a rolling N*86400-ms window" (which would span N+1
distinct UTC dates and produce off-by-one buckets at non-midnight
call times). Computed via:
for (let i = days-1; i >= 0; i--)
dates.push(_utcDateFromMs(now - i*86400*1000));
cacheHitRateWindow denominator: hit_rate = hit / (hit + miss).
Bypass is intentional non-cacheable (Anthropic cache_control marker),
NOT a cache miss; excluding it from the denominator gives a clean
cache-effectiveness signal.
TESTS — Suite 23, +27 (544 → 571):
23a-1..4: discoverAuditFiles (empty dir / live only / live+rotated /
non-audit files ignored)
23b-1..6: readAuditWindow (all-coverage / single-day / half-open
exclusivity / empty window / missing files / malformed-skip with
warn)
23c-1..4: aggregateRequests (counts + status buckets + by_provider;
by_owner_tier; median+p95 latency over realistic distribution;
invalid windowMs rejection)
23d-1..4: topFallbackChains (sort desc by count; limit truncation;
fallback_hops=0 excluded; first_seen/last_seen carried)
23e-1..3: spendTrendDaily (N-day range correctness — caught off-by-
one during local run; populated day breakdown; empty day sparse-
fill)
23f-1..3: cacheHitRateWindow (overall + per-provider hit_rate;
bypass not in denominator; cache_status=null events excluded)
23g-1..3: PII guard for aggregateRequests / spendTrendDaily /
topFallbackChains + cacheHitRateWindow — every output JSON-
stringified + scanned for forbidden PII keys
DOCUMENTATION:
- AGENTS.md: lib/audit-query.mjs new entry; lib/audit.mjs note added
that D52 extends with daily rotation.
NOT IN D49 scope:
- server.mjs endpoints consuming these queries (D50)
- dashboard.html (D51)
- lib/audit.mjs rotation extension + bin/olp-audit-rotate.mjs (D52)
- tried_providers schema fix (D53; D45 P2 deferral)
- Phase 3 close → v0.3.0 (D55; maintainer-triggered)
Test count: 544 → 571 (+27). Verified locally via npm test.
AUTHORITY:
- ADR 0008 § 4 (query API surface) + § 5 (rotation file naming
pattern) + § 3 (storage layout).
- ADR 0007 § 8 (audit ndjson event schema — input data).
- CLAUDE.md release_kit overlay phase_rolling_mode — under
Unreleased.
- Standing autopilot grant.
ALIGNMENT.md scope check: this PR adds a new lib/ module. No provider
plugin / entry surface / IR change. Rule 5 commit-citation requirements
for those scopes do not apply.
Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
|
c0b696984f |
docs: D48 — ADR 0008 Phase 3 design draft (Dashboard + audit query layer) (#25)
* docs: D48 — ADR 0008 Phase 3 design draft (Dashboard + audit query layer, design-only)
First Phase 3 D-day. Design-only. Ratifies the storage / query model /
rotation / dashboard / refresh / scope decisions ahead of D49+
implementation D-days. Opens ADR 0007 § 12 deferral for Dashboard +
audit query layer + rotation.
NEW docs/adr/0008-dashboard-and-audit-query.md (~368 lines): 13 sections
+ Consequences + Authority citations. Per maintainer-pinned lanes
A/A/B/A/B from Phase 3 kickoff brief 2026-05-25:
Lane 1 (tech stack): A — static HTML + vanilla JS + fetch; no build
step; matches OLP "no bundler" ethos.
Lane 2 (query model): A — in-memory scan of audit ndjson per request;
O(N) per call; family-scale acceptable; defers SQLite hybrid to
Option 3 trigger per ADR 0007 § 13 (requires engines bump as
separate prior PR).
Lane 3 (rotation): B — daily rotation, audit-YYYY-MM-DD.ndjson on
first append after UTC midnight + optional bin/olp-audit-rotate.mjs
external cron.
Lane 4 (refresh): A — 30s page poll (no SSE infra at v0.3.0;
pause when document.visibilityState is hidden).
Lane 5 (dashboard scope): B — full per spec § 4.6: 4 panels (per-
provider quota / 24h request+cache+fallback / 30d spend trend /
top-N fallback chains).
ADR sections:
§1 Context (what Phase 3 closes; what stays out)
§2 Decision (5 lanes pinned)
§3 Storage layout (~/.olp/logs/ with rotated date files)
§4 lib/audit-query.mjs API (readAuditWindow, aggregateRequests,
topFallbackChains, spendTrendDaily, cacheHitRateWindow)
§5 Audit rotation (first-append-after-UTC-midnight trigger + cron
alternative + concurrent-safety per-process lock)
§6 Dashboard panels (4 panels per spec § 4.6, refresh + localhost-
bound notes)
§7 Server endpoints (/dashboard, /v0/management/dashboard-data, /v0/
management/quota, /cache/stats — owner-only gated)
§8 Auth gating (reuses ADR 0007 § 7 owner_only_endpoints config)
§9 Failure modes + degradation (per-panel error states)
§10 Acceptance criteria (15 testable items for D49-D54)
§11 Forward path (Phase 4+ — SQLite migration, SSE push, key-mgmt UI)
§12 Out of scope (explicitly NOT in Phase 3)
§13 Phase 3 sprint shape (D48-D55)
CHANGED:
- docs/adr/README.md: added ADR 0008 row with one-paragraph summary.
- CHANGELOG.md Unreleased: D48 entry per release_kit overlay
phase_rolling_mode discipline. Includes Phase 3 sprint shape table.
NOT IN D48 scope:
- lib/audit-query.mjs (D49)
- server.mjs endpoints (D50)
- dashboard.html (D51)
- lib/audit.mjs rotation extension + bin/olp-audit-rotate.mjs (D52)
- tried_providers schema fix (D53; D45 P2 deferral)
- Phase 3 close (D55; v0.3.0; maintainer-triggered)
Test count: 544 / 544 pass (design-only, no test change). Verified
locally via npm test before commit.
AUTHORITY:
- ADR 0007 § 12 (opens Phase 3 Dashboard + audit query deferral).
- ADR 0007 § 13 (rejects SQLite at Phase 3 per Node baseline +
documents forward-path trigger).
- OLP v0.1 spec § 4.6 + § 4.7 (Dashboard + observability endpoints
planning authority).
- OCP dashboard.html (prior-art reference for multi-panel HTML
structure).
- ADR 0002 (Provider.quotaStatus contract — Panel 1 data source).
- ADR 0005 § Cache stats (/cache/stats endpoint pre-existing design).
- CC 开发铁律 v1.6 § 10 — fresh-context opus reviewer required for
design ADR.
- Phase 3 kickoff via maintainer "go" 2026-05-25 + standing-autopilot
grant (~/.cc-rules/memory/auto/standing_autopilot_phase_2.md in
cc-rules bf0ed9a — Phase 3+ requires new authorization; the "go"
supplied that).
ALIGNMENT.md scope check: this PR introduces a new ADR with full
authority citations per Rule 1 (Cite First). No provider plugin /
entry surface / IR change in this commit.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs: D48 fold-in — opus reviewer findings (1 P2 + 2 P3, all ADR-text polish)
Fresh-context opus reviewer (PR #25) returned APPROVE_WITH_MINOR with 3
findings, all ADR-text polish. No design semantic change beyond the
clarifications.
P2 — § 8 + § 10 #9 gating-mode wording
Original § 8 implied a new "block non-owner identities" behaviour
without naming it; § 10 #9 tested only the universal
allow_anonymous: false 401 case. Fix:
- § 8 now formalizes two gating modes — owner_only_trim (Phase 2
/health pattern) vs owner_only_block (new Phase 3 management-
endpoints pattern) — and explains the management endpoints are
owner_only_block because the entire payload is sensitive.
- § 10 #9 now covers both 401 paths: (a) allow_anonymous: true
+ no header → anonymous identity → STILL 401 because management
endpoints are owner_only_block; (b) allow_anonymous: false + no
header → 401 at the authenticate middleware itself.
P3 #1 — /cache/stats citation accuracy
Original § 7.4 + Authority block cited "ADR 0005 § Cache stats"
which is not a real section. Corrected: planning authority is OLP
v0.1 spec § 4.6; ADR 0005 references the endpoint in
Consequences/Mitigations (~line 279) for the per-(provider, model)
cache-hit-rate breakdown surface.
P3 #2 — cacheStore.stats() shape gap
§ 7.4 now explicitly acknowledges the current shape
({ hits, misses, size, inflightCount } global aggregate) lacks the
per-(provider, model) breakdown spec § 4.6 implies; Phase 3 Panel 2
sources per-provider counts from aggregateRequests (audit-side)
instead. If a future panel needs the breakdown, D50 amends the
store shape + an ADR 0005 amendment fires at that time. Phase 3
acceptance criteria do not require the breakdown.
CHANGELOG.md Unreleased D48 entry: fold-in bullet added enumerating
the 3 fixes.
Test count: 544 / 544 pass (design-only, no test change). Verified
locally via npm test.
Authority: PR #25 fresh-context opus reviewer findings; CLAUDE.md
release_kit overlay phase_rolling_mode — under Unreleased.
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>
|
||
|
|
e87b6b73ec |
release(phase-2-close): v0.2.0 — multi-key auth + audit + owner gating + keygen CLI (D43-A → D47) (#24)
Closes Phase 2. All 11 ADR 0007 § 10 acceptance criteria shipped + tested
in main across 6 D-day commits (D43-A doc cleanup, D43-B ADR 0007 ratify,
D44 lib/keys.mjs core, D45 server.mjs auth integration + lib/audit.mjs,
D46 owner gating /health + X-OLP-Fallback-Detail, D47 bin/olp-keys.mjs
keygen CLI). Test count 468 (v0.1.1) → 544 (v0.2.0).
Per CLAUDE.md release_kit.phase_close_trigger this PR is the explicit
maintainer-triggered close action.
CHANGES IN THIS COMMIT (release-kit machinery only — no code):
package.json:
- version 0.1.1 → 0.2.0
CLAUDE.md release_kit.phase_rolling_mode:
- current_phase: Phase 2 → Phase 3
- current_pre_release_identifier: "0.2.0-phase2" → "0.3.0-phase3"
CHANGELOG.md:
- Unreleased promoted to "## v0.2.0 — 2026-05-25" with the D43-A
→ D47 entries intact (already accumulated under release_kit
phase_rolling_mode discipline during Phase 2 D-days).
- New Phase 2 release_kit checklist + ADR 0007 § 10 acceptance
criteria final-ship table + known-limitations-beyond-v0.2.0.
- New "## Unreleased\n\n(empty — Phase 3 entries land here once
Phase 3 opens)" sentinel for the next phase. D37
phase_rolling_mode gate will pass (sentinel-only Unreleased).
README.md:
- Status header v0.1.1 → v0.2.0; Phase 1+2 shipped; Phase 3 next.
- Implementation status note dated post-v0.2.0; reflects Phase 2
close.
- Phase plan Phase 2 promoted to ✅ Shipped; Phase 3 marked (next).
Test count 544 / 544 pass (npm test verified locally; no test or .mjs
file touched in this release commit).
NEXT STEPS (post-merge, auto-triggered):
- git tag v0.2.0 + git push --tags
- release.yml fires: phase_rolling_mode gate passes (Unreleased is
sentinel-only) + GitHub Release auto-published from the CHANGELOG
v0.2.0 section.
ACKNOWLEDGEMENTS:
- Phase 2 was executed under the maintainer's standing autopilot
grant (~/.cc-rules/memory/auto/standing_autopilot_phase_2.md,
cc-rules bf0ed9a); D-day cadence: 6 implementation D-days +
multiple opus-reviewer fold-ins, all in a single session.
- ADR 0007 was authored via D43-B with maintainer text review on
top of fresh-context opus review (4 findings: 1 P1 safety + 2 P2
+ 1 P3) folded in before ratification.
Authority:
- CLAUDE.md release_kit overlay phase_rolling_mode (Iron Rule 5.5)
governs this commit's shape; phase_close_trigger requires explicit
maintainer action — the user issued "go" to trigger.
- ADR 0007 (multi-key auth) — the Phase 2 design contract;
acceptance criteria #1–#11 covered.
- CC 开发铁律 v1.6 § 10 — fresh-context opus reviewer required for
every implementation phase + design ADR (executed on D44, D45,
D46, D47, and D43-B with double-review).
Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
v0.2.0
|
||
|
|
939f3e6bd9 |
feat+test+docs: D47 — bin/olp-keys.mjs keygen CLI (Phase 2 functional scope closes) (#23)
Fourth Phase 2 implementation D-day. Closes ADR 0007 § 10 acceptance criterion #9 (bootstrap workflow must be reproducible without manual file editing) by shipping a minimal keygen CLI per § 9.1. Phase 2 functional scope is complete with this D-day — remaining work is Phase 2 close → v0.2.0 (maintainer-triggered, explicit per CLAUDE.md release_kit.phase_close_trigger). NEW bin/olp-keys.mjs (~250 lines): subcommand CLI keygen [--owner|--name=X|--tier=guest|owner|--providers=csv|--force] Creates a key + prints plaintext token to stdout ONCE; manifest stores only SHA-256 hash. --force revokes existing owner keys before creating the new owner (ADR § 9.3 recovery flow). list [--owner-only|--include-revoked] Lists keys with token_hash redacted. revoke --id=<key-id> Marks the key's revoked_at; idempotent (already-revoked → no-op + status message); missing id → exit 2. Common flag: --olp-home=<path> overrides ~/.olp/ (defaults to OLP_HOME env then ~/.olp/). package.json bin field "bin": { "olp-keys": "./bin/olp-keys.mjs" } so npx olp-keys ... resolves. Also "scripts": { "olp-keys": "node bin/olp-keys.mjs" } for npm run. Module shape (testability) Exports runCli(argv, { out, err }) so tests invoke with synthetic argv + IO writers (no process spawn). Main guard auto-runs when invoked as entrypoint. Plaintext token discipline (ADR § 5 + § 9.1) Plaintext printed exactly once on stdout. Never logged, never written to manifest, never written to audit. Operators capture immediately; lost → --force revoke + regenerate. --force async correctness cmdKeygen is async and awaits each revokeKey (which is async — acquires per-key write lock per § 6.4). Sequence: revoke each existing owner manifest atomically → then createKey for new owner. Avoids race where create-new runs before revoke-old completes. TESTS — Suite 22, +20 (524 → 544): 22a-1..5: parseArgv unit (--flag=value, --flag value, boolean, mixed positional) 22b-1..5: keygen (owner default, name+providers, missing-name error, invalid-tier error, --force revoke-then-create with isolation tmpdir) 22c-1..3: list (empty, populated with token_hash-redaction check, --owner-only filter) 22d-1..4: revoke (valid id, idempotent re-revoke, missing-id error, nonexistent-id error) 22e-1..3: top-level CLI (--help / no args / unknown subcommand exit codes) DOCUMENTATION: - AGENTS.md: lib/keys.mjs marker promoted to ✅; new bin/olp-keys.mjs entry. Implementation-status-note + shipped-set updated. - README.md: Implementation Status row added for bin/olp-keys.mjs; Known limitations note rewritten to "Phase 2 functional scope complete; close pending"; new Bootstrap workflow section with copy-pasteable npx commands + recovery flow. - CHANGELOG.md: D47 entry under Unreleased per release_kit overlay. AUTHORITY: - ADR 0007 — § 5 token format, § 9.1 minimal keygen command surface, § 9.3 recovery, § 10 acceptance criterion #9 covered. - CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased. - Standing autopilot grant. Verified: 544/544 pass via npm test (no regression in 524 pre-D47 tests; 20 new Suite 22 tests all green). Co-authored-by: dtzp555 <dtzp555@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
|
06f619120d |
feat+test+docs: D46 — owner-vs-guest gating for /health + X-OLP-Fallback-Detail (Phase 2) (#22)
* feat+test+docs: D46 — owner-vs-guest gating for /health + X-OLP-Fallback-Detail (Phase 2) Third Phase 2 implementation D-day. Closes ADR 0007 § 10 acceptance criteria #4 (/health payload trimming for non-owner) + #5 (X-OLP-Fallback-Detail emission gating per fallback_detail_header_policy). Phase 2 server surface now fully gated end-to-end; remaining D-days are keygen CLI surface (D47+) and Phase 2 close (v0.2.0, maintainer- triggered). server.mjs handleHealth identity-aware payload per § 7.1: - Auth gate at top — 401 for unauth + allow_anonymous=false; 200 with trimmed { ok, version } for non-owner; 200 with full payload for owner. - Trim controlled by _authConfig.owner_only_endpoints — operator removing /health from the list reverts to v0.1.1 full-payload-to- everyone (opt-out knob). - touchLastUsed fires on res.on('finish') for filesystem identities; no audit row on /health (high-volume monitoring; out of scope at Phase 2 per § 8). server.mjs withFallbackDetailHeader identity-aware emission per § 7.2: - New shouldEmitFallbackDetailHeader(olpIdentity) helper reads _authConfig.fallback_detail_header_policy: 'owner_only' (default) → emit only to owner 'all' → emit unconditionally (v0.1.1 opt-back-in) 'none' → suppress unconditionally - olpIdentity null on pre-auth paths → emit (preserves D40 v0.1.1 behaviour for pre-auth errors where identity is unknown). - withFallbackDetailHeader signature gains 3rd `olpIdentity` arg; both call sites in handleChatCompletions updated. Test surface — Suite 21, +9 tests; +1 in Suite 20 (20m); 515 → 524: 20m: /health with no auth + allow_anonymous=false → 401 (consistency with /v1/*) 21a-d: /health payload trimming (criterion #4): anonymous trimmed; guest trimmed; owner full; owner_only_endpoints: [] opts out 21e-h: X-OLP-Fallback-Detail emission gating (criterion #5): owner_only + guest → header absent owner_only + owner → header present + valid JSON 'all' + guest → header present (v0.1.1 opt-back) 'none' + owner → header absent (full suppression) Tests use 2-hop chain anthropic→openai with anthropic primary failing to produce non-empty fallbackDetail for header content. Test-mode setup updated: Global __setAuthConfig({ allow_anonymous: true }) extended to also pass owner_only_endpoints: [] + fallback_detail_header_policy: 'all' so pre-D46 tests (Suite 18, F5 /health tests, D40 fallback-detail tests, etc.) continue to pass; Suite 21 overrides per-case. DOCS: - AGENTS.md: lib/keys.mjs marker updated to reflect D46 ship; impl- status-note + shipped-set updated. - README.md: Implementation Status row + Known limitations "Multi-key auth" note rewritten to reflect D46 ship + remaining keygen CLI. - CHANGELOG.md: D46 entry under Unreleased per release_kit overlay. AUTHORITY: - ADR 0007 §§ 7.1 + 7.2 implementation contracts + § 10 criteria #4 + #5 covered. - ADR 0004 Amendment 5 (D40 — "Phase 2 will re-introduce owner-vs- non-owner gating when lib/keys.mjs lands"): this D-day fulfils the deferral. - CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased. - Standing autopilot grant (~/.cc-rules/memory/auto/ standing_autopilot_phase_2.md in cc-rules bf0ed9a). Verified: 524/524 pass via npm test (no regression in 515 pre-D46 tests; 9 new Suite 21 tests + 1 new Suite 20m test all green). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: D46 fold-in — opus reviewer P3 polish (constant import + comment tighten) Fresh-context opus reviewer (PR #22) returned APPROVE_WITH_MINOR with 2 P3 findings, both trivial polish. - server.mjs imports gain ENV_OWNER_KEY_ID from lib/keys.mjs (already used the namesake ANONYMOUS_KEY_ID import). handleHealth touchLastUsed guard now uses the imported constant for SPOT discipline. - handleHealth audit-deferral comment tightened: removed the "§ 8 schema doesn't mandate auditing" phrasing (overstates the ADR — § 8 doesn't enumerate paths); replaced with the operational rationale (high-volume noise, no observability value until Phase 3 Dashboard). No behaviour change. 524/524 tests pass (verified locally). Authority: PR #22 fresh-context opus reviewer findings. 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> |
||
|
|
40064955ab |
feat+test+docs: D45 — server.mjs auth integration + lib/audit.mjs (#21)
* feat+test+docs: D45 — server.mjs auth integration + lib/audit.mjs (Phase 2 wire-up) Second Phase 2 implementation D-day. Wires the D44 lib/keys.mjs identity layer into the request flow + lands lib/audit.mjs per ADR 0007 § 6.2 + § 8. Closes ADR § 10 acceptance criteria #1 (per-key cache isolation), #2 (anonymous prod-default off), #3 (anonymous dev-mode on), #6 (post-revoke 401 within next request — full coverage with D45), #8 (audit ndjson round-trip), #10 (OLP_OWNER_TOKEN env override — full server-side coverage), #11 (providers_enabled 403 scope). Owner-vs-guest gating for /health + X-OLP-Fallback-Detail (criteria #4, #5) remains in D46 scope. NEW lib/audit.mjs (~110 lines): - appendAuditEvent(event, opts): one JSON event per line to ~/.olp/logs/audit.ndjson (file 0600, dir 0700). § 6.2 retry: warn + 1 retry; per-process drop counter + warn on second failure; NEVER throws. Per-call OLP_HOME env resolution (matches lib/keys.mjs). - getAuditDropCount(): for future /health surface. lib/keys.mjs extended: - loadAuthConfigSync({ olpHome }): reads auth block from ~/.olp/config.json with ADR § 7.2 defaults (allow_anonymous: false, owner_only_endpoints: ['/health'], fallback_detail_header_policy: 'owner_only'). Never throws; missing file / malformed JSON falls back to defaults. - _resolveOlpHome(opts): precedence opts.olpHome → process.env.OLP_HOME → ~/.olp. Per-call resolution so tests + operator deployments can redirect without code edits. server.mjs auth middleware integration: - extractToken(req): parses Authorization Bearer / x-api-key. - authenticate(req): validateKey + 401 paths (auth_required vs invalid_or_revoked_key). - isProviderEnabled(olpIdentity, providerKey): '*' = all; else array allowlist. - _authConfig loaded at startup; warn auth_allow_anonymous_enabled when true. Test seams __setAuthConfig / __resetAuthConfig. - handleChatCompletions + handleModels both gated by authenticate at top. Audit ctx built throughout; res.on('finish') appends row + fires touchLastUsed async. - IDENTITY-VS-CREDENTIALS SEPARATION: olpIdentity (new validated identity) consumed for cache namespacing + providers_enabled + audit; authContext passed to provider.spawn() REMAINS null so providers continue their own credential discovery (env / keychain / file). Per-provider per-key credential mapping is Phase 3+ per ADR § 12. - handleChatCompletions chain filtered by isProviderEnabled; empty result returns 403 key_no_provider_access. - keyId = olpIdentity.keyId (replacing hardcoded '__anonymous__'). - Audit captures fields throughout: post-auth, post-IR, post-chain (success or exhausted). Status + latency populated on res.on('finish'). TESTS — Suite 20, +15 (499 → 514): 20a-d: header parsing + valid key happy paths (Bearer / x-api-key / invalid → 401) 20e: revoked key 401 (criterion #6 end-to-end) 20f: OLP_OWNER_TOKEN env override returns 200 (criterion #10 full) 20g: allow_anonymous=true + no header returns 200 (criterion #3) 20h + 20h-extra: providers_enabled=['mistral'] for anthropic model → 403; '*' baseline returns 200 (criterion #11) 20i: per-key cache namespace isolation (criterion #1 end-to-end) 20j + 20j-401: audit.ndjson written with § 8 schema fields + PII guard; 401 path also appends (criterion #8) 20k: filesystem key last_used_at populated post-request (D45 touch wire) 20l + 20l-200: /v1/models also enforces auth TEST-MODE SETUP (test-features.mjs): - process.env.OLP_HOME = mkdtempSync(...) at module load so audit + key writes don't pollute ~/.olp/. - __setAuthConfig({ allow_anonymous: true }) after server.mjs imports so pre-D45 HTTP integration tests (Suite 18 etc.) continue to pass. - Suite 20 explicitly overrides __setAuthConfig per-case to exercise production-default-off coverage. DOCUMENTATION: - AGENTS.md: lib/keys.mjs 🟡 marker updated + NEW lib/audit.mjs entry; Implementation-status-note + shipped-set updated. - README.md: Implementation Status table gains lib/audit.mjs row + lib/keys.mjs row updated; Known limitations Multi-key auth note rewritten to reflect D45 ship + D46 follow-up; new env-vars (OLP_HOME, OLP_OWNER_TOKEN) and auth config block surfaced. - CHANGELOG.md: D45 entry under Unreleased per release_kit overlay phase_rolling_mode discipline. AUTHORITY: - ADR 0007 (multi-key auth — §§ 5/6.2/7/9.4 implementation contracts + § 10 criteria #1/#2/#3/#6/#8/#10/#11 covered). - CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased. - Phase 2 kickoff handoff (~/.cc-rules/memory/handoffs/ 2026-05-25-phase-2-kickoff.md in cc-rules d9da966). - Standing autopilot grant (~/.cc-rules/memory/auto/ standing_autopilot_phase_2.md in cc-rules bf0ed9a). Verified: 514/514 pass via npm test (no regression in 499 existing tests; 15 new Suite 20 tests all green). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix+test+docs: D45 fold-in — CI fail recovery + opus reviewer P1/P2/P3 Fresh-context opus reviewer (PR #21) returned APPROVE_WITH_MINOR with 4 findings; CI Node 24 separately reported 9 Suite 20 failures (all 200-expecting tests). Root cause of CI: Suite 20 setup did not stub CLAUDE_CODE_OAUTH_TOKEN before mock spawn, so anthropic.mjs AUTH_MISSING pre-check fired and tests 502'd. (Local Node 22 had the env from the maintainer's claude install — masked the gap.) CI FIX — Suite 20 OAuth env stub Added ensureSuite20FakeOAuth / restoreSuite20OAuth helpers in makeSuite20Server / teardownSuite20. Matches the existing pattern in Suite 9 line ~2154 (test-fake-oauth-token-for-cache-tests). P1 — Real-streaming path audit fidelity Single-hop streaming success (server.mjs ~L1050, the most common deployed shape) did not populate auditCtx.provider / tried_providers / cache_status. Audit rows for streaming requests carried provider: null. Fixed by stamping these at the top of the streaming branch and amending error_code on the two streaming failure exit paths (streaming_error_after_first_chunk + streaming_error_before_first_chunk). New regression test 20j-stream: streaming request asserts the audit row's provider, cache_status, and tried_providers fields are populated. P2 — Global test tmpdir cleanup process.env.OLP_HOME = mkdtempSync(...) at test-features.mjs module load left /var/folders/.../olp-test-home-* leak per npm test run. Fixed by process.on('exit', () => rmSync(_GLOBAL_TEST_OLP_HOME)). Best-effort; swallows errors so exit handler never throws. P3 — handleModels 401 lacks OLP diagnostic headers handleChatCompletions 401 passes olpErrorHeaders({ startMs }); handleModels 401 did not. Aligned. DEFERRED — P2 tried_providers semantics on 403 Reviewer noted that key_no_provider_access 403 stamps original chain in tried_providers, but the field name implies hops actually dispatched. Either ADR § 8 amendment or D46+ semantic fix. Marked in CHANGELOG; not in this fold-in scope. Test count: 514 → 515 (+1 streaming-audit regression test 20j-stream; 14 existing Suite 20 tests still pass). Verified locally via npm test. CI Node 24 recovery via the OAuth env stub. Authority: PR #21 fresh-context opus reviewer findings; CI Node 24 run 26382758946 failure logs; CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased. 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> |
||
|
|
4b9916341b |
feat+test+docs: D44 — lib/keys.mjs core landed (multi-key auth, no server wire-up yet) (#20)
* feat+test+docs: D44 — lib/keys.mjs core landed (multi-key auth, no server wire-up yet)
First Phase 2 implementation D-day. Lands the lib/keys.mjs module per
ADR 0007 §§ 5 / 6.1 / 6.3 / 6.3.5 / 6.4 / 9.4. Identity / lifecycle
layer for OLP API keys is now in-tree; server.mjs integration is
scheduled D45 (until then, requests still use the hardcoded
'__anonymous__' cache namespace — no behavioural change at v0.1.1 / D44).
NEW FILE lib/keys.mjs (~437 lines, public API):
- createKey({ name, owner_tier, providers_enabled, notes, olpHome })
Generates opaque 'olp_<32-byte base64url>' token (47-char total),
SHA-256 hashes for manifest storage, atomically writes
keys/<id>/manifest.json (file 0600, dir 0700). Returns
{ id, plaintext_token, manifest } — plaintext printed once, never
persisted.
- validateKey(plaintext, { allowAnonymous, olpHome })
Three-tier resolution per § 5 / § 7 / § 9.4: env override
(OLP_OWNER_TOKEN -> __env_owner__) -> anonymous (only when
allowAnonymous: true, returns __anonymous__) -> filesystem
manifest lookup (constant-time hash compare via timingSafeEqual).
Revoked manifests return null (caller produces 401). Per § 6.3.5:
MUST hit manifest every request; no in-process validation cache.
- revokeKey({ id, olpHome })
Idempotent; sets revoked_at via atomic write inside per-key lock.
- listKeys({ olpHome })
Returns manifest objects with token_hash redacted.
- touchLastUsed(id, { olpHome })
Async best-effort lazy update per § 6.3 revoke-dominates-touch:
re-reads latest manifest inside per-key lock, NO-OPs if revoked_at
is non-null, otherwise merges last_used_at preserving all other
fields. Failure logs warn and never throws.
Plus internal helpers: hashToken (SHA-256 hex), generateToken,
generateKeyId, validateManifest (§ 4 schema validation), readManifest,
writeManifestAtomic (tmpfile + fsync + rename + chmod), _withKeyLock
(§ 6.4 in-process per-key write-lock chain), _safeHexCompare
(timing-safe).
Test-only hooks: __setTouchInterleaveHook (inject deterministic pause
for race tests), __resetWriteLocks (test cleanup).
NOT IN D44 (split per ADR §§ 6.2 / 9.1 separation):
- audit ndjson append (§ 6.2) — request-layer concern; D45 server glue
- keygen CLI bootstrap surface (§ 9.1) — D45+ separate command entry
- server.mjs integration replacing hardcoded '__anonymous__' at
server.mjs:502, :531 — D45
- owner-vs-guest gating for /health (server.mjs:392) + X-OLP-Fallback-
Detail (server.mjs:1072, :1101) — D46
TEST COUNT: 468 -> 496 (+28 tests in new Suite 19):
- 19a-d: token generation (§ 5)
- 19e-j: manifest write+read + chmod 0600/0700 + schema validation
(§ 4, § 6.1)
- 19k-p: validateKey (filesystem / wrong / missing / anonymous /
revoked / env override) (§ 5, § 6.3.5, § 9.4)
- 19q-r: revokeKey idempotency + non-existent id
- 19s-t: listKeys empty + redaction
- 19u-x: touchLastUsed updates + NO-OP on revoked + NO-OP on
anonymous/env identities + best-effort failure
- 19y-1 to 19y-4: ACCEPTANCE CRITERION #7 — concurrent revoke + touch
race tests:
19y-1 revoke -> touch (revoked_at survives)
19y-2 touch -> revoke (revoked_at + last_used_at both present)
19y-3 interleaved external-revoke via __setTouchInterleaveHook
(deterministically reproduces the § 6.3 race the
maintainer's D43-B text review caught — confirms our impl
observes the revoke and NO-OPs)
19y-4 30-iteration concurrent Promise.all stress
DOCS UPDATED IN THIS COMMIT:
- AGENTS.md: lib/keys.mjs marker 📋 -> 🟡 'core landed at D44';
Implementation-status-note + shipped-set updated.
- README.md: Implementation Status row + Known limitations
'Multi-key auth' note updated to 'core landed, server integration
pending D45'.
- CHANGELOG.md: D44 entry under Unreleased per release_kit overlay
phase_rolling_mode discipline.
AUTHORITY:
- ADR 0007 (multi-key auth) — Decision: Option 2 filesystem manifest +
opaque token; §§ 5/6.1/6.3/6.3.5/6.4/9.4 implementation contracts;
§ 10 acceptance criteria #6/#7 partially covered by D44 tests
(#7 fully covered; #6 partially covered — full coverage requires
D45+ server integration).
- CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased.
- Phase 2 kickoff handoff:
~/.cc-rules/memory/handoffs/2026-05-25-phase-2-kickoff.md
(cc-rules d9da966).
- CC 开发铁律 v1.6 § 10 — fresh-context opus reviewer required.
Verified: 496/496 pass via npm test before commit.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* feat+test+docs: D44 fold-in — opus reviewer findings (2 P2 correctness + 2 P3 polish)
Fresh-context opus reviewer (PR #20) returned APPROVE_WITH_MINOR with 4
findings — 2 P2 real correctness gaps + 2 P3 polish. All accepted; the 2
P2 fixes ship with new regression tests.
P2 #1 — lib/keys.mjs _withKeyLock lock-map cleanup
Prior version stored `prev.then(() => next)` as the Map tail, but the
cleanup-identity check `_writeLocks.get(id) === next` could never match
the derived promise. Result: Map entries leaked one-per-unique-key-id
forever. Bounded impact at family scale (~5–10 entries) but a real
correctness bug uncovered by reviewer empirical reproduction
("CLEANUP SKIPPED every call").
Fix: store `next` directly as the Map tail. The chain still works
because new callers chain off `_writeLocks.get(id)` (the prior caller's
`next`); compare-and-delete by identity correctly cleans up when the
current caller is the last in queue.
Regression tests:
- 19x-extra: after 5 sequential touchLastUsed calls, __writeLockSize()
must be 0.
- 19x-extra-2: after 9 concurrent touch calls across 3 keys (3 per
key), __writeLockSize() must drain to 0.
P2 #2 — lib/keys.mjs validateKey non-string defensive coding
Prior version threw TypeError when called with a non-string truthy
plaintext (validateKey(42), validateKey({}), etc.), reaching
hashToken(<non-string>) which calls createHash().update(<non-string>)
which throws. Q2 (defensive-coding acceptance criterion) promised
"bad inputs return null."
Fix: top-of-function guard
`if (plaintextToken != null && typeof plaintextToken !== 'string') return null;`
Falls through to null path for non-string truthy; preserves existing
null / undefined / '' handling.
Regression test 19m-extra: validateKey(42), validateKey({}),
validateKey([]), validateKey({ token: 'olp_xxx' }), and the same with
allowAnonymous: true — all must return null without throwing.
P3 #3 — 19y-3 test scope comment clarification
Reviewer noted that 19y-3 simulates external revoke landing BEFORE
touch's read (not BETWEEN touch's read and write — currently
unreachable due to synchronous read→write in touchLastUsed). Added
explanatory comment documenting:
- The scenario this test does cover (pre-read external revoke).
- The scenario this test does NOT cover (between-read-and-write).
- Why scenario 3 is unreachable in the current impl (no await between
readManifest and writeManifestAtomic).
- The trigger for adding a post-read hook (any future refactor that
introduces an await between read and write).
P3 #4 — CHANGELOG line-count corrections
D44 entry said ~330 lines (initial estimate); actual is 462 lines
after fold-in. Test count claim updated from "+28 tests" to
"+31 tests" (28 initial + 3 fold-in regression).
New module-level export: __writeLockSize (test-only) — reports current
size of in-process write-lock Map for the regression tests above. Not
intended for production callers.
Test count: 496 → 499 (+3 fold-in regression tests; +31 total from
D44 inclusive of initial Suite 19). Verified locally via npm test.
Authority: PR #20 fresh-context opus reviewer findings; CLAUDE.md
release_kit overlay phase_rolling_mode — under Unreleased.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs: D44 fold-in #2 — CHANGELOG line-count consistency (trivial)
Delta opus reviewer flagged internal CHANGELOG inconsistency: header
bullet correctly stated `~462 lines` but the P3 #4 self-description
bullet still said the prior fold-in corrected to `~445 lines`. Both now
agree on 462 (matches `wc -l lib/keys.mjs`).
No code change. Test count: 499 / 499 pass (unchanged).
Authority: PR #20 delta opus reviewer trivial inconsistency note.
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>
|
||
|
|
d253c2b98d |
docs: D43-B — ADR 0007 multi-key auth design draft (design-only) (#19)
* docs: D43-B — ADR 0007 multi-key auth design draft (design-only, no code change) Phase 2 mainline design ADR. Ratifies the storage / token / manifest / atomic-write / owner-gating / bootstrap / Node-baseline decisions ahead of D44+ implementation D-days. Pure design doc — no .mjs / no tests / 4 files touched. Test count 468 → 468. - docs/adr/0007-multi-key-auth.md (new, ~400 lines): 13 sections covering Context / Decision (Option 2 filesystem manifest + opaque token) / Storage layout / Manifest schema / Token format (olp_+32B base64url, SHA-256 hash) / Atomic write & audit append (manifest lifecycle-only atomic via tmpfile+fsync+rename; audit per-request append with warn+1-retry, no memory buffer at Phase 2) / Owner-vs- guest-vs-anonymous gating (config.json auth.allow_anonymous default false, no env auto-detection) / Audit ndjson schema (no PII) / Bootstrap & recovery (minimal keygen command surface + OLP_OWNER_TOKEN env override with stable __env_owner__ keyId) / Acceptance criteria (11 test surfaces) / Node baseline (Option 1 SQLite port rejection rationale citing engines >=18 + CI 20/24 vs node:sqlite v22.5.0/RC) / Out of scope (Dashboard, quota enforcement, audit query, file locking deferred to Phase 3+) / Future forward (Option 3 hybrid migration trigger + preconditions). - docs/adr/README.md index: added ADR 0007 row with one-paragraph summary covering storage choice + rejection rationale. - docs/v1x-roadmap.md #2: marked PHASE 2 ACTIVE (no longer deferred); "Design ADR (NOT YET RATIFIED)" → "Design ADR (ratified) → ADR 0007"; trigger updated to "already fired 2026-05-25"; code anchors pinned to exact line numbers (cache/store.mjs:77-79/:287, server .mjs:502/:531/:392/:1072/:1101). - CHANGELOG.md Unreleased: D43-B entry per release_kit overlay phase_rolling_mode discipline. Authority: - Phase 2 kickoff handoff (~/.cc-rules/memory/handoffs/2026-05-25-phase-2-kickoff.md, cc-rules d9da966) - OLP v0.1 spec § 4.5 (planning authority for ~/.olp/ layout) - OCP keys.mjs (prior-art for opaque-key + per-key isolation model) - Node node:sqlite docs (https://nodejs.org/api/sqlite.html — Option 1 rejection per ADR 0007 § 11) - CC 开发铁律 v1.6 § 10 — fresh-context opus reviewer required for design ADR per Iron Rule 10 ALIGNMENT.md scope check: this PR introduces a new ADR; per ALIGNMENT.md Rule 1 (Cite First), the ADR itself contains the authority citations its decisions rest on (v0.1 spec § 4.5, OCP keys.mjs, Node docs URL). No provider plugin / entry surface / IR change in this commit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: D43-B fold-in — ADR 0007 reviewer findings (2 P2 + 3 P3, all polish) Fresh-context opus reviewer (PR #19) returned APPROVE_WITH_MINOR with 2 P2 load-bearing-but-non-blocking findings + 3 P3 polish findings. All five are accepted as suggested; design contract clarified without semantic change. - § 6.2 step 1 (P2 #1) — pin audit serialization to fire AFTER status_code is determined and latency_ms is measured. Makes acceptance criterion #2 (anonymous-401 audit event records the 401 + latency) testable in the way the criterion was written. - § 6.3.5 (P2 #2, new subsection) — explicit "Token validation MUST hit the manifest on every authenticated request (no in-process validation cache at Phase 2)" rule. The acceptance criterion #6 (post-revoke 401 within the next request) was previously enforced only by the test; the rule now belongs to the design contract. Forward-path note documents when a Phase 3+ amendment may add a cache. - § 6.1 atomic-write step 5 follow-up (P3 #3) — document the deliberate omission of directory fsync after rename. Single-process family-scale deployment accepts the tiny rename-loss window under abrupt host crash; future POSIX-strict deployments know where to add the step. - § 9.4 (P3 #4) — declare token-collision between OLP_OWNER_TOKEN and a filesystem-stored key's plaintext as undefined behaviour. Operators MUST NOT reuse plaintext across both surfaces. Phase MAY add startup collision-detection later. - § 10 criterion #4 (P3 #5) — rephrased to assert against the config- driven owner_only_endpoints predicate rather than a hardcoded trimmed payload shape. The test stays stable if an operator removes /health from owner_only_endpoints. CHANGELOG D43-B entry: fold-in bullet added to summarize the 5 fixes. Test count: 468 → 468 (npm test verified locally after fold-in). Authority: PR #19 fresh-context opus reviewer findings; CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: D43-B fold-in #2 — maintainer text-review findings (1 P1 + 1 P2 + 1 P3) Maintainer (codex) did final text review of PR #19 against the Phase 2 ADR-ratification checklist. Returned 3 findings; all accepted as suggested. Two are contract-level (P1 safety + P2 factual); one is trivial (P3 line-count drift). No semantic change beyond what the findings called out. P1 — § 6.3 / § 6.4 / § 10 #7 — revoke-dominates-touch safety contract Original § 6.3 said touchLastUsed used the same atomic-write pattern as 6.1; § 6.4 said concurrent CLI revoke + touchLastUsed left "both states valid" with "observability-grade" failure mode. Codex correctly identified the bug: a stale manifest snapshot held by the touch path could overwrite a fresh revoke and silently clear revoked_at back to null, breaking acceptance criterion #6 (post-revoke 401 within next request) under concurrent CLI revoke + in-flight server request. The ADR was promising security-grade behavior on a path that was actually last-write-wins. Fix: - § 6.3 rewritten with explicit read-modify-write discipline: touch MUST re-read latest manifest from disk inside the per-key write-lock, NO-OP if revoked_at is non-null, otherwise merge last_used_at preserving all other fields including revoked_at. - § 6.4 reframed from "both states valid" to "revoke dominates touch" safety frame, citing § 6.3 as the load-bearing discipline. The CLI revoke writer always wins the dimension that matters; touch may lose its last_used_at update if it raced. - § 10 criterion #7 expanded to test all three orderings (revoke -> touch, touch -> revoke, interleaved) with the explicit MUST: revoked_at is non-null and equals the revoke writer's timestamp after any interleaving; FAIL if any path produces revoked_at: null. - Forward-path § 6.4 file-locking note updated to clarify §6.3 already holds the contract single-process; flock adds defense-in- depth for rare multi-writer TOCTOU. P2 — § 11 forward path step (1) — Node baseline version history corrected Original wording "Node v22.5.0+ for unflagged but RC; Node TBD for stable" was wrong. v22.5.0 added with --experimental-sqlite flag; v22.12 still required the flag; the module moved past flag-gating in v22.13.0 (LTS) / v23.4.0 (current); entered Release Candidate at v25.7.0 per current docs. Fix: § 11 forward path step (1) rewritten with accurate versions + two Node release-history URLs cited (https://nodejs.org/download/ release/v22.12.0/docs/api/sqlite.html and https://nodejs.org/api/ sqlite.html). Minimum non-flag-gated baseline is now stated as >=22.13.0 (LTS) / >=23.4.0 (current); stable baseline TBD pending Node v25.x+. The rejection-evidence paragraph earlier in § 11 ("v22.12 still required --experimental-sqlite ... current docs mark RC") was already correct and is untouched. P3 — CHANGELOG D43-B line-count corrected Entry said ADR was "~270 lines"; actual file is 420 lines after both fold-ins. Changed to "~420 lines after fold-ins". Phase 2 fold-in #2 bullet enumerates the 3 fixes in this commit; fold-in #1 bullet retained for the opus reviewer round. Test count: 468 / 468 (npm test verified locally after fold-in; design-only doc changes, no test file touched). Authority: PR #19 maintainer text review findings 2026-05-25; CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased; Node SQLite docs URLs cited in ADR § 11 forward path. 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> |
||
|
|
68851fe3d7 |
docs: D43-A — Phase 2 doc alignment (no code change) (#18)
* docs: D43-A — Phase 2 doc alignment (no code change)
Phase 1 was closed at v0.1.1 (multi-provider proxy core + pre-Phase-2
cleanup, D35-D42). This commit aligns documentation surfaces to the
Phase 2 reality before D43-B (ADR 0007 multi-key auth design draft) lands.
Pure doc cleanup — no .mjs / no tests / 4 files touched.
- CLAUDE.md release_kit.phase_rolling_mode:
* current_phase: Phase 1 → Phase 2
* current_pre_release_identifier: "0.1.0-bootstrap" → "0.2.0-phase2"
- README.md:
* Status header now reads "v0.1.1 shipped (2026-05-25); Phase 2 in progress"
* Implementation Status dated 2026-05-25; intro paragraph reflects Phase 1
close + Phase 2 active
* lib/keys.mjs row: "📋 Planned (Phase 2)" → "📋 Phase 2 active per ADR 0007
(drafting at D43-B)"
* Known limitations "Multi-key auth not yet implemented" note updated
* Phase plan rewritten end-to-end: the original v0.1 spec planned one
plugin per phase, but actual execution bundled the three Tier-D plugins
+ cache + fallback into a single Phase 1 milestone (v0.1.0+v0.1.1).
New plan: Phase 0 ✅ / Phase 1 ✅ / Phase 2 multi-key auth (current) /
Phase 3 dashboard / Phase 4+ v1.x roadmap / Phase N tier-2 opt-in.
- AGENTS.md § Key files to know:
* lib/keys.mjs marker updated
* Implementation-status-note dated 2026-05-25; reflects v0.1.1 close +
Phase 2 active scope
- CHANGELOG.md Unreleased: D43-A entry recording the alignment per
CLAUDE.md release_kit overlay phase_rolling_mode discipline.
Authority: CLAUDE.md release_kit overlay phase_rolling_mode — under
Unreleased; Phase 2 kickoff handoff at
~/.cc-rules/memory/handoffs/2026-05-25-phase-2-kickoff.md (committed in
cc-rules d9da966); ADR 0007 forthcoming at D43-B.
Test count: 468 → 468 (npm test verified locally before commit).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs: D43-A fold-in — ALIGNMENT.md phase terminology note (reviewer P2)
Fresh-context sonnet reviewer (PR #18) flagged P2: ALIGNMENT.md uses
"Phase 2"/"Phase 3" at lines ~58/~143-144/~179 with the original
per-plugin enablement meaning (Phase 2 = Codex enable, Phase 3 = Mistral
enable), conflicting with the new README phase plan rewritten by D43-A
where Phase 2 means multi-key auth.
Reviewer's recommended minimal fix (B1): add a clarifying note in
ALIGNMENT.md § Provider Inventory header explaining the dual usage,
rather than amend the tables or audit trigger wording. This keeps D43-A
within "pure doc cleanup" scope.
- ALIGNMENT.md § Provider Inventory: one-paragraph "Note on phase
terminology" inserted between the v0.1 zero-Enabled-Providers
rationale and the Enabled Providers table. No Speculative-Candidate
table change, no audit-trigger wording change, no governance-text
change.
- CHANGELOG.md Unreleased D43-A entry: ALIGNMENT.md added to the file
list with a one-line explanation referencing the reviewer-P2 fold-in.
Test count: 468 → 468 (npm test verified locally after fold-in; no test
file touched).
Authority: PR #18 fresh-context reviewer finding P2; CLAUDE.md release_kit
overlay phase_rolling_mode — under Unreleased.
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
|
8fd8f86942 |
release(phase-1-cleanup): v0.1.1 — pre-Phase-2 batch (D35-D42)
Phase 1 cleanup release per CLAUDE.md release_kit.phase_rolling_mode
policy. Closes 16 of 17 pre-Phase-2 GitHub issues; #16 stays OPEN as
v1.x tracker with design ratified in ADR 0005 Amendment 8.
Bumps package.json from 0.1.0 → 0.1.1. Promotes CHANGELOG "Unreleased"
section to "## v0.1.1 — 2026-05-25" with full D35-D42 entries
(backfilled D35/D36/D37 entries per cold-audit round 7 Finding 1) and
final cleanup batch summary covering all 8 D-day commits that landed
on `main` between 2026-05-24 and 2026-05-25.
Tag v0.1.1 will be pushed after this commit lands and CI is green.
release.yml auto-creates the GitHub Release from the CHANGELOG v0.1.1
section (extracted via awk pattern match on `## v0.1.1` heading).
D37's phase_rolling_mode gate now passes because Unreleased is
sentinel-only after this commit.
**What v0.1.1 delivers (D35-D42):**
- D35 — streaming empty-stream headers (#9) + truncation marker (#10)
+ irVersion strict check (#11) + alignment.yml scripts/** removal
(#12) + X-OLP-Latency-Ms uniform audit (#4)
- D36 — cache_control partial-noop log (#2) + ADR 0002 filename
correction (#5) + mistral A5 flip (#6) + /v1/models alias governance
(#13) + cache_control determinism test (#14) + anthropic v2.1.89
transcript artifact (#15)
- D37 — release.yml phase_rolling_mode CI gate (#17)
- D38 — maxConcurrent runtime enforcement + new CONCURRENCY_LIMIT
hard trigger (#1)
- D39 — CacheStore.delete + cache_evicted_truncated log + sticky-
cache regression test + SPAWN_TIMEOUT asymmetry ADR (#3)
- D40 — X-OLP-Fallback-Detail debug header + per-hop tuple collection
+ 4KB cap + RFC 7230 non-ASCII escape (#7)
- D41 — X-OLP-Provider-Used chain-origin semantics documented (#8)
- D42 — streaming singleflight v1.x design ADR + multi-layer
safeguards (issue #16; STAYS OPEN as v1.x tracker)
**Test growth:** 416 (v0.1.0) → 468 (v0.1.1). +52 tests across the
8 D-day batch. All 468 pass at the release-commit head.
**Cold-audit cycle:** 1 final round (round 7) over D35-D42 + v0.1.0
state. Result: PASS_WITH_MINOR with 4 findings (1×P3 + 3×P4).
- Finding 1 (P3 CHANGELOG missing D35/D36/D37) — FIXED in this commit
via backfill.
- Finding 2 (P4 ADR 0005 Amendment 8 "Amendment 1/3/5" notation
ambiguity) — FIXED in this commit (replaced with explicit
§ Decision body item-1 + Amendment 3 + Amendment 5 wording).
- Finding 3 (P4 ADR 0005 Amendment 8 Context paragraph cites line 811
but actual branch is at lines 817-823) — FIXED in this commit
(Context paragraph now points at lines 817-823 with the TODO anchor
at ~810 as the navigable landmark).
- Finding 4 (P4 ADR 0002 missing Amendment 2 — pre-existing gap)
— FIXED in this commit (added explanatory note under the
Amendments heading explaining the gap is intentional and load-
bearing).
No P1 or P2 findings.
**Phase 1 cleanup release_kit checklist** (per CLAUDE.md):
- [x] All 8 D-day deliverables landed on main (D35-D42)
- [x] CI green on every D-day commit + on this release commit's head
- [x] Cold-audit round 7 PASS_WITH_MINOR (0 P1/P2; 4 findings all
fixed in this commit)
- [x] 16 of 17 pre-Phase-2 GitHub issues closed (#1-#15 and #17);
#16 stays OPEN as v1.x tracker with design ratified
- [x] Issue #16 status comment posted (D42 commit
v0.1.1
|
||
|
taodengandClaude Opus 4.7
|
b0c080db13 |
docs: D42 — streaming singleflight design ADR + v1.x safeguards (issue #16)
ADR 0005 Amendment 6 (D34) deferred streaming-path D4 singleflight to v1.x with the note "the design alone warrants a dedicated ADR." Round-6 cold-audit F13 (issue #16) raised the sibling TOCTOU window between server.mjs's preCheckHit peek and the streaming-branch spawn. D42 fulfils Amendment 6's deferral note by ratifying the v1.x design as ADR 0005 Amendment 8 — design only, no implementation. **Design ratified (ADR 0005 Amendment 8, 14 sections):** 1. New `cacheStore.getOrComputeStreaming(keyId, cacheKey, sourceFactory)` API with `{ stream, isFirst }` return shape. Mirrors the buffered path's `getOrCompute` to keep the cache API surface coherent. 2. `StreamingInflightEntry` shape — source iterator + AbortController + accumulated-chunks replay buffer + attached-clients Set + state flags + D38 spawn-slot tracker. 3. `AttachedClient` shape — per-client tee buffer + byte-size meter + late-joiner replay flag + done/resolveNext/rejectNext for the single-reader-multi-writer tee. 4. Tee fan-out loop (one reader drains source, fans chunks to all attached clients). 5. Late-joiner replay policy — accumulated chunks burst-drained on attach. 6. Cache TTL race during inflight — late joiners see the inflight entry and join; expired cache slot is overwritten by inflight completion. 7. D38 maxConcurrent coordination — only first caller acquires; release fires once on source-complete/error/abort. 8. New `STREAM_BACKPRESSURE` error code. NOT a hard trigger. Affected client gets synthetic `{ type: 'stop', finish_reason: 'length' }` + `[DONE]` (matches D35 #10 truncation marker). 9. Mid-stream disconnect — remove from attached set; if 0 remaining, abort source via AbortController. 10. Replay buffer cap (10 MB, matches D23 cache-entry cap) — over cap marks entry not cacheable, late joiners get backpressure error. 11. Observability — 4 new log events (streaming_inflight_join / source_done / abort, stream_backpressure_disconnect) + new `X-OLP-Streaming-Inflight: source | attached | solo` header. 12. Server.mjs wiring — replaces the current peek+spawn pattern; TOCTOU window closes because Map check+insert is synchronous. 13. Test surface (when implementation lands) — 10 scenarios covering single-client / 2-concurrent / 3-concurrent / mid-stream join / first-disconnect / all-disconnect / source-error / backpressure / D38 coordination / TTL race / replay cap / X-OLP-* header values. 14. Defaults — PER_CLIENT_QUEUE_CAP=1MB, ACCUMULATED_REPLAY_CAP=10MB, STREAM_BACKPRESSURE not in HARD_TRIGGER_CODES. **Multi-layer safeguards (the maintainer asked: "保证后面这一块会被处理而不会被忽略"):** 1. **`docs/v1x-roadmap.md` (NEW)** — single living landing page for every Phase-1 deferral. 7 items at D42: - #1 Streaming SF (this amendment) - #2 Multi-key auth (lib/keys.mjs) - #3 Soft trigger reactivation (ADR 0004 Amendment 2) - #4 /health activeSpawns integration (D38) - #5 Provider-level cacheKeyFields mask (ADR 0005 Amendment 7) - #6 Streaming-path SPAWN_FAILED salvage - #7 AUTH_MISSING tuple test coverage (D40 follow-up) Each entry names the ratifying ADR, load-bearing code anchor (file:line), GitHub issue (if any), concrete start trigger, and estimated effort. Maintainer's session-startup discipline grep this file at sprint kickoff. 2. **Issue #16 STAYS OPEN** — not closed in D42. Body updated post- commit to reference Amendment 8 with status "design ratified; implementation pending." Do not close until §13 test surface is green on actual implementation. 3. **`lib/cache/store.mjs#getOrCompute` JSDoc** — TODO comment for the sibling streaming API pointing at Amendment 8 + v1x-roadmap.md #1. 4. **`server.mjs` streaming-branch entry (~line 810)** — TODO comment block citing Amendment 8 + issue #16 + roadmap.md #1, naming the exact code lines the v1.x impl will replace. 5. **`README.md § Known limitations` section** — new subsection surfaces 4 limitations to users (streaming SF / soft triggers / multi-key auth / cacheKeyFields mask), each linking to the v1x-roadmap.md entry. 6. **Amendment 8 § "Cross-references and safeguards"** — explicit cross-link block enumerating the above 4 anchors so a future ADR-only reader knows every breadcrumb. **Maintainer decision recorded:** Option 1 (design ADR only) chosen over Option 2 (design + implementation now). Rationale: 200-400 lines of concurrency primitives + 15-20 tests is not "pre-Phase-2 cleanup" in shape — it is real v1.x feature work. Shipping streaming SF in a v0.1.1 patch release would muddy the Phase 1 / Phase 2 contract that v0.1.0 ratified. Personal/family-scale load makes the deferral safe at v0.1. Changes (6 files, +165 / -0): - `docs/adr/0005-cache-cross-provider.md` — Amendment 8 prepended (133 lines). - `docs/v1x-roadmap.md` — NEW file (148 lines). - `lib/cache/store.mjs` — getOrCompute JSDoc gains TODO block (8 lines). - `server.mjs` — streaming-branch entry gains TODO block (6 lines). - `README.md` — Known limitations section (9 lines). - `CHANGELOG.md` — D42 sub-entry under Unreleased (9 lines). No code-behavior change. No new tests. No package.json bump (phase_rolling_mode). Authority: - ADR 0005 Amendment 8 (this commit) — design ratification - ADR 0005 Amendment 6 (D34) — original deferral with "design ADR needed" note that this commit fulfils - GitHub issue #16 (round-6 F13) — sibling TOCTOU; STAYS OPEN - ADR 0002 Amendment 6 (D38) — tryAcquireSpawn semantics - ADR 0004 Amendment 5 (D40) — observability pattern extension - CC 开发铁律 v1.6 § 10.x — design-only amendment; fresh-context reviewer not required per Iron Rule 10 implementation-phase scope - CLAUDE.md release_kit_overlay phase_rolling_mode — under Unreleased Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
taodengandClaude Opus 4.7
|
b43b07afbf |
docs: D41 — X-OLP-Provider-Used chain-origin semantics (issue #8)
Round-4 cold-audit Finding 10 (filed as issue #8): on a chain-exhausted response, X-OLP-Provider-Used returns chain[0].provider — but if hop 0 were soft-skipped (quota threshold exceeded), the header would attribute a provider whose plugin's spawn() was never called. README's "which provider's plugin served the request" is technically false in that edge case. At v0.1 the scenario is unreachable because soft triggers are deferred (ADR 0004 Amendment 2) — evaluateSoftTriggers always returns false. The ambiguity is latent and only activates when soft triggers reactivate in v1.x. **Option B chosen — document chain-origin semantics, no code change.** Option A would track firstAttemptedProvider separately in executeWithFallback and return that on chain exhaustion. Adding state for an unreachable v0.1 code path would violate ALIGNMENT.md Rule 2 (No Invention). The D40 X-OLP-Fallback-Detail header (Amendment 5) already carries per-hop spawn history including soft-skip records (trigger_type: 'soft'), providing the disambiguation channel on the wire without needing providerUsed to handle it. Changes (4 files, +35 / -1): 1. **docs/adr/0004-fallback-engine.md** — Amendment 6 added above Amendment 5 in the amendments stack. Documents the chain-origin contract, names Option A as the likely v1.x preference, cites the Rule 2 rationale + the X-OLP-Fallback-Detail disambiguation channel. 2. **README.md** — Observability header description updated: "which provider's plugin served the request" gains a clarifying sentence about chain-origin semantics on exhausted responses, with a pointer to ADR 0004 Amendment 6. 3. **lib/fallback/engine.mjs** — Inline comment block at the chain-exhausted return site explicitly cites the amendment and captures the v0.1-vs-v1.x semantic. No behavior change. 4. **CHANGELOG.md** — D41 sub-entry under existing D38/D39/D40 entries in Unreleased section. No code-behavior change. No new tests — the relevant scenario is dead-by-config at v0.1. v1.x soft-trigger reactivation work should add a test exercising soft-skip + chain-exhausted that pins whichever option (A or B) the v1.x maintainer chooses, and coordinate the README + ADR Amendment 6 update if Option A is adopted. Authority: - ADR 0004 Amendment 6 (this commit) — chain-origin semantics - ADR 0004 § Decision § Chain advancement step 4 — original promise - ADR 0004 Amendment 2 — soft triggers deferred (precondition) - ADR 0004 Amendment 5 (D40) — per-hop attribution channel - ALIGNMENT.md Rule 2 — No Invention rationale - GitHub issue #8 — closed by this commit - D32 round-4 cold-audit F10 — original filing - CC 开发铁律 v1.6 § 10.x — Iron Rule 10's implementation-phase scope is unmet here (doc-only amendment); no fresh-context reviewer dispatched per the documented exception in this amendment's procedural mechanism Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
taodengandClaude Opus 4.7
|
04f797f917 |
feat+docs+test: D40 — X-OLP-Fallback-Detail header (issue #7)
ADR 0004 § Decision § Chain advancement step 4 promised a per-hop
failure detail debug header `X-OLP-Fallback-Detail`. From D9 through
D39 the engine logged per-hop events (D28 added correlation fields)
but never surfaced the failure trail on the response. D40 fulfills
the promise via Option A — ungated v0.1 emission. Phase 2 will
re-introduce owner-vs-non-owner gating when `lib/keys.mjs` lands.
**Per-hop tuple collection** (lib/fallback/engine.mjs)
executeWithFallback now collects `fallbackDetail` — an array of per-hop
tuples — and returns it on EVERY return shape (success / client-error /
AUTH_MISSING / non-trigger / chain-exhausted). Soft-trigger skipped
hops are also recorded with `trigger_type: 'soft'` (currently dead-
code-by-config per ADR 0004 Amendment 2; shape forward-compatible).
Tuple shape (reuses D28 log-event field shapes so logs and the header
pivot on the same keys):
```
{
hop: <0-indexed hop number>,
provider: <provider name>,
model: <model name from IR>,
code: <ProviderError code, engine-synthetic SOFT_TRIGGER, or 'UNKNOWN'>,
error_message: <truncated to 200 chars with ellipsis on overflow>,
trigger_type: 'hard' | 'soft' | 'client_error' | 'auth_missing' | 'unclassified'
}
```
**Header serialiser** (server.mjs)
- `FALLBACK_DETAIL_BYTE_CAP = 4096` (UTF-8 bytes).
- `serializeFallbackDetailHeader(fallbackDetail)` exported. Returns
`null` for empty/null/undefined → caller omits the header.
- `jsonStringifyAscii` escapes every non-ASCII code point as `\uXXXX`
to satisfy RFC 7230 §3.2.6 field-vchar (Node's HTTP header validator
rejects multi-byte UTF-8). The D38 CONCURRENCY_LIMIT synthesised
message contains a U+2014 em dash — without this escape, every
CONCURRENCY_LIMIT response crashed at writeHead with
"Invalid character in header content". The regression test pins
this exact string.
- 4KB cap algorithm: builds candidates as `[...slice(0, kept), sentinel]`
and measures the FULL serialised length (including sentinel) before
comparing against the cap, so the result is guaranteed under cap.
Tail tuples dropped one at a time. Sentinel form:
`{ truncated: true, omitted_hops: N }`.
**Header emission** (server.mjs)
- `withFallbackDetailHeader(base, fallbackDetail)` wraps the base
header object; emits the header only when serialiser returns
non-null.
- Emitted on chain-exhausted, non-trigger-error, client-error,
AUTH_MISSING, and success-with-prior-failure paths.
- ABSENT on clean primary success — verified by a dedicated HTTP
integration test.
**Tests** (test-features.mjs): 452 → 468 (+16):
- Engine-level tuple shape: 2-hop/exhausted, 2-hop/success-with-prior-
failure, 1-hop/success (empty array), 1-hop/fail, non-ProviderError-
yields-UNKNOWN, 500-char-message → 200-char-with-ellipsis, client
error → 1 tuple + client_error trigger type
- Serialiser: empty/null → null, small array round-trip, >4KB cap
with `{truncated:true,omitted_hops:N}` sentinel, RFC 7230 newline/CR
escaping, non-ASCII escaping (em dash regression guard for the D38
CONCURRENCY_LIMIT synthesised message)
- HTTP integration: clean-1-hop-success (header absent), 2-hop-
exhausted (2 tuples on the wire), 2-hop-success-with-prior-failure
(1 tuple on the wire)
Pre-commit fold-in (per evidence-first checkpoint #4):
- **Reviewer Suggestion #2**: `jsonStringifyAscii` regex character
class `[U+0080-U+FFFF]` contains an invisible U+0080 boundary marker
that editors render as nothing, making the line easy to misread as
the empty class `[-...]`. Folded in a 5-line comment block above
the function body explaining the literal byte range and citing
RFC 7230 §3.2.6.
Two reviewer suggestions not folded:
- **Suggestion #1**: AUTH_MISSING tuple path lacks a dedicated D40
test. Code is structurally correct (tuple pushed before early-
return); low priority. Future polish.
- **Suggestion #3**: defensive `err.code != null` guard. Extremely
low priority — PROVIDER_ERROR_CODES is a closed enum with no
falsy values.
**ADR amendments** (docs/adr/0004-fallback-engine.md)
- Amendment 5 added at top of amendments stack — full tuple schema,
cap behavior, RFC 7230 hygiene, ungated v0.1 rationale, Phase 2
follow-up.
- § Chain advancement step 4 updated to remove TBD / not-yet-
implemented qualifiers and cross-reference Amendment 5.
- § Observability headers section gains the X-OLP-Fallback-Detail
schema as IMPLEMENTED at v0.1.
**CHANGELOG.md** — D40 sub-entry appended under existing D38/D39
entries in Unreleased section. No package.json bump per
phase_rolling_mode.
Authority:
- ADR 0004 § Decision § Chain advancement step 4 — D40 fulfils
the promise
- ADR 0004 Amendment 5 (this commit) — implementation contract
- ADR 0004 § Observability headers — updated to IMPLEMENTED state
- D18 (5 standard X-OLP-* headers) — D40 builds on this convention
- D28 (per-hop structured log fields) — D40 reuses the field shapes
- GitHub issue #7 — closed by this commit
- CC 开发铁律 v1.6 § 10.x — fresh-context opus reviewer independent
- CLAUDE.md release_kit_overlay phase_rolling_mode — no version bump
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus,
independent of drafter): APPROVE. Critical depth checks:
- Tuple pushed once per hop, BEFORE client-error / AUTH_MISSING
early-return branches — both paths include the failing tuple
- fallbackDetail returned on all 5 return paths (verified line by line)
- Cap algorithm measures FULL serialised length (with sentinel)
before comparing — no overshoot
- RFC 7230 compliance verified end-to-end: imported the function
in Node, confirmed em-dash → —, unescaped form throws
"Invalid character in header content"
- Serialiser handles null / undefined / [] gracefully → header
absent on clean primary success (HTTP test pins this)
- Hygiene: 0 hits for personal markers, home paths, tokens
- 468/468 tests pass in reviewer's independent npm test run
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
|
994568a8fb |
feat+ci: D38 — maxConcurrent runtime enforcement (issue #1)
ADR 0002 Amendment 1 declared `hints.maxConcurrent` declarative-only at
v0.1 — type-validated at startup but with no runtime enforcement.
D38 wires the runtime enforcement via a per-provider in-flight spawn
counter with immediate-advancement on saturation.
Design choice: **immediate-advancement via fallback engine** (queue +
timeout DEFERRED). When a provider is at its maxConcurrent limit, the
spawn call synchronously fails with a new `CONCURRENCY_LIMIT` error
code; the fallback engine treats this as a hard trigger and advances
to the next chain hop. If the entire chain is saturated, the user
sees a chain-exhausted error (existing path).
Rationale for immediate-advancement over queue+timeout:
1. The fallback chain exists precisely for this kind of overflow —
adding a queue layer would duplicate the advancement semantics.
2. Queue + timeout adds new config surface (timeout duration, queue
depth bounds, queue eviction policy) that isn't needed at the
personal/family scale OLP serves.
3. Head-of-line blocking risk: a long-running spawn would stall
queued requests behind it even though other providers in the chain
could serve them immediately.
4. Fail-fast latency aligns with the multi-provider proxy philosophy
("spread risk across providers, not within a provider").
Queue + timeout is deferred to a v1.x design ADR if real usage shows
demand. ADR 0002 Amendment 6 and ADR 0004 Amendment 4 capture the
decision explicitly.
Changes (8 files, +<delta>):
**Code**
1. **lib/providers/base.mjs** — add `CONCURRENCY_LIMIT` to
`PROVIDER_ERROR_CODES`. JSDoc clarifies the code is synthesised by
the orchestration layer, not thrown by provider plugins themselves.
2. **lib/providers/index.mjs** — new semaphore primitives:
- `tryAcquireSpawn(providerName, maxConcurrent)` — atomic
check-then-increment. Returns `true` on success, `false` if at
limit. Atomicity rests on the JS single-threaded invariant
(read + write synchronous, NO `await` between them); module-level
comment block warns future maintainers against breaking this.
- `releaseSpawn(providerName)` — decrement; throws on
under-decrement (defensive bug guard for missing acquire / double
release). Map.delete at zero for clean memory footprint.
- `getActiveSpawnCount(providerName)` — returns current count
(0 for unseen providers). For diagnostics + tests.
- `DEFAULT_MAX_CONCURRENT_SPAWNS = 4` — defense-in-depth fallback
matching the v0.1 plugin defaults (anthropic/codex/mistral all
declare hints.maxConcurrent: 4). Also coerces non-integer / NaN
/ negative inputs to the default.
- `__resetSpawnCounters()` — internal test seam.
3. **lib/fallback/engine.mjs** — `CONCURRENCY_LIMIT: true` added to
`HARD_TRIGGER_CODES`. `classifyTrigger` and `evaluateHardTriggers`
both pick it up via the same lookup. v0.1 live hard-trigger codes
are now 5 (was 4 post-D34): SPAWN_FAILED, CLI_NOT_FOUND,
AUTH_MISSING:false, SPAWN_TIMEOUT, CONCURRENCY_LIMIT.
4. **server.mjs** — gate the spawn call in handleChatCompletions at
BOTH spawn call sites:
- **Buffered path** (executeHopFn → collectAllChunks): acquire
before provider.spawn; on failure synthesise
`ProviderError(CONCURRENCY_LIMIT)` with providerName /
maxConcurrent / activeSpawns diagnostic fields and throw —
fallback engine catches and advances. On success, outer try/
finally wraps the inner D16 truncation-salvage try/catch so
releaseSpawn fires on EVERY exit path (return, D16 salvage
return, re-throw).
- **Streaming path** (single-hop real-SSE branch): acquire BEFORE
the streaming branch entry. If acquire fails, branch is skipped
and request falls through to buffered path (whose own gate
surfaces chain-exhausted for single-hop chains). If acquire
succeeds, existing streaming try/catch gains
`finally { releaseSpawn(streamProvider) }` — slot releases on
stop-chunk completion, generator exhaustion, abort, or any
exception path. `releaseSpawn` fires at END of stream
consumption, not when spawn() returns.
**Tests** (test-features.mjs): 431 → 447 (+16):
Suite 18 — D38 — maxConcurrent runtime enforcement:
- 18a: PROVIDER_ERROR_CODES.CONCURRENCY_LIMIT exists
- 18b: evaluateHardTriggers true for CONCURRENCY_LIMIT
- 18c: AUTH_MISSING regression guard (D38 did NOT flip it to hard)
- 18d.1-18d.4: semaphore unit (increment / saturate-no-increment /
release / map-delete-at-zero)
- 18e: tryAcquireSpawn returns false without side-effect when at limit
- 18f: releaseSpawn throws on under-decrement
- 18g.1-18g.2: DEFAULT_MAX_CONCURRENT_SPAWNS applied for invalid
inputs (undefined / NaN / negative / non-integer)
- 18h: __resetSpawnCounters clears state
- 18i: HTTP integration — 5 concurrent requests to single-hop
chain w/ maxConcurrent=2 → peak in-flight exactly 2, 2 succeed,
3 fail
- 18j: counter releases after buffered request (sequential test)
- 18k: 2-hop chain — saturated primary advances to fallback
- 18l: streaming counter releases at END of stream (not at spawn)
**ADR amendments**
5. **docs/adr/0002-plugin-architecture.md** — Amendment 6 added.
Removes "Declarative hint only at v0.1" caveat from the
`maxConcurrent` description in the Provider contract section.
Adds implementation reference + design-choice rationale (4 points).
Lists all exported symbols.
6. **docs/adr/0004-fallback-engine.md** — Amendment 4 added.
CONCURRENCY_LIMIT added to v0.1 hard-trigger taxonomy. Documents
synthesis-vs-plugin-thrown distinction. Documents first-chunk
safety (acquire before any res.write). v1.x re-evaluation triggers
named.
**CHANGELOG**
7. **CHANGELOG.md** — Unreleased sentinel replaced with proper D38
entry. Per CLAUDE.md release_kit phase_rolling_mode, this lands
under Unreleased; promotion to ## v0.1.1 happens at the v0.1.1
release. The D37 phase_rolling_mode gate now correctly fires if
anyone tags v0.1.x with this content present without promotion —
intentional.
Pre-commit fold-ins (per evidence-first checkpoint #4):
- **Reviewer Suggestion #1 (test 18c name mismatch)**: 18c is named
"classifyTrigger returns hard for CONCURRENCY_LIMIT" but body tests
AUTH_MISSING regression. Renamed header comment to
"AUTH_MISSING regression guard" and clarified that
CONCURRENCY_LIMIT classification is covered by 18b.
- **Reviewer Suggestion #3 (activeSpawns diagnostic field)**: server
.mjs:532 set `concurrencyErr.activeSpawns = maxConcurrent` (the
limit). Technically correct (since acquire just failed, live
count == limit) but confuses future readers. Changed to query
`getActiveSpawnCount(hopProvider)` directly. Added import of the
new symbol to the lib/providers/index.mjs import block.
- **Reviewer Suggestion #5 (ADR overstatement)**: ADR 0002 Amendment 6
said getActiveSpawnCount is "exported for /health, diagnostics,
and tests" but /health integration is not wired at D38. Reworded to
"exported for diagnostics and tests" + "/health integration
deferred — when surfaced there will land at providers.status.<name>
.activeSpawns; not wired at D38." Avoids overstating current state.
Two reviewer suggestions not folded:
- Suggestion #2 (streaming test 18l comment about branch entry
conditions) — low priority; the test passes and the branch is
taken (verified by reviewer). Future polish if confusion arises.
- Suggestion #4 (additional test for streaming-saturation → chain-
exhausted) — code path is straightforward and intentional; 18i
covers the buffered-path version directly. Defer.
Authority:
- ADR 0002 Amendment 1 (declarative-only caveat) — superseded by
Amendment 6
- ADR 0002 Amendment 6 (this commit) — runtime enforcement landed
- ADR 0004 Amendment 4 (this commit) — CONCURRENCY_LIMIT added to
v0.1 hard-trigger taxonomy
- GitHub issue #1 — closed by this commit
- CC 开发铁律 v1.6 § 10.x — independent fresh-context reviewer
- CLAUDE.md release_kit_overlay phase_rolling_mode — no version
bump; Unreleased entry written
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus,
independent of drafter): APPROVE. Critical depth checks:
- Atomicity in tryAcquireSpawn (lines 285-290 read+set with no
intervening await) — confirmed; module-level invariant comment
warns future maintainers
- Release on every exit path: buffered (outer finally wraps inner
try/catch covering D16 salvage return + normal return + re-throw);
streaming (try/finally covers stop-chunk return + loop exhaustion +
catch paths). No double-release path identified.
- Streaming release timing: fires in finally after res.end() at all
exit paths, never before stream consumption completes
- Counter leak: every code path traced — no orphan acquire identified
- 447/447 tests pass in reviewer's independent npm test run
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
a718d22900 |
ci: D37 — release.yml phase_rolling_mode gate (issue #17)
Round-6 F14: release.yml verified package.json/tag version match and
extracted the matching CHANGELOG section, but did not verify that
"## Unreleased" had been promoted to "## v<version>" before the tag
push. If someone tagged v0.1.0-bootstrap today, release.yml would
extract the stale ## v0.1.0-bootstrap section, ignoring all D-day
work folded under Unreleased — the exact failure mode documented in
MEMORY.md 2026-04-21 ("release.yml would publish stale notes that
ignored Unreleased amendments").
Option A applied: CI gate makes the policy enforceable. Manual
checklist (Option B) was rejected because it relies on human memory,
which is what produced the original failure.
New step "Enforce phase_rolling_mode (Unreleased must be promoted)"
runs after version-match check and before CHANGELOG extraction:
1. Extracts content between "## Unreleased" heading and the next
"## " heading via awk.
2. Strips blank lines and parenthetical-sentinel lines via sed
(acceptable forms: "(empty — Phase N entries land here once
Phase N opens)", "(another sentinel)", multi-sentinel blocks).
3. If any non-trivial content remains, exits with ::error::
instructing the maintainer to promote Unreleased → ## v<version>
per CLAUDE.md release_kit.phase_rolling_mode.
Locally dry-run against 4 cases:
- Current CHANGELOG.md (sentinel-only Unreleased) → PASS
- Synthetic CHANGELOG with bullet-list under Unreleased → gate FIRES,
reports offending lines indented for readability
- Synthetic CHANGELOG with no Unreleased section → PASS
- Synthetic CHANGELOG with multiple parenthetical sentinels and
intervening blank lines → PASS
Authority:
- CLAUDE.md release_kit.phase_rolling_mode (D33 F11 added the policy;
this gate enforces it)
- MEMORY.md 2026-04-21 OCP cross-machine sync entry (documents the
same class of failure as a prior precedent)
- Round-6 cold-audit Finding 14
Gate is purely additive — adds a check, does not modify existing
release behavior. Fires only on tag push to v*.*.* — does not affect
normal push/PR CI.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
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
|
2600185edb |
fix+test: D35 — pre-Phase-2 batch #1 (issues #4 #9 #10 #11 #12)
First batch of pre-Phase-2 cleanup work. 5 GitHub issues closed in
one cohesive commit covering streaming-path correctness, IR
validator hardening, and CI path-trigger hygiene.
Changes (4 files, +302 / -5):
**Code fixes**
1. **#9 — Streaming empty-then-clean-exit headers** (server.mjs)
Pre-D35: when a provider's streaming spawn finished cleanly with
zero chunks (e.g. spec-degenerate stop with no content), the
response went out the SSE_DONE / res.end path without ever calling
writeHead. Result: client saw stream open + close with no headers,
no status code path applied. Now: zero-chunk branch guards
`!res.headersSent` and emits Content-Type + Cache-Control +
Connection + X-Accel-Buffering + all 5 X-OLP-* headers via
olpHeaders (provider attempted, cache miss) before writing the
terminator. Zero-chunk path correctly does NOT cache (cache write
remains gated on irChunk.type === 'stop').
2. **#10 — Streaming post-first-chunk error truncation marker**
(server.mjs, two sibling sites)
Pre-D35: if a provider yielded an error AFTER first content chunk
was emitted, the SSE stream was abandoned with raw socket close.
Client SDKs that wait for finish_reason hung. Now:
- Catch-block firstChunkEmitted=true path: emit synthetic
`{type:'stop', finish_reason:'length'}` via irChunkToOpenAISSE,
write SSE_DONE, end. Per ADR 0004 § Fallback safety: post-first-
chunk truncation surfaces as `length` finish, not a hang.
- Sibling fix in error-chunk path (provider yields `type:'error'`
chunk AFTER first content chunk): same recovery (marker + DONE +
end). Scope-creep acknowledged but identical semantic; clean to
fix together. Comments cross-reference D26 F19 and D35 #10.
3. **#11 — validateIRRequest irVersion strict check** (lib/ir/types.mjs)
ADR 0003 IR contract pins irVersion to '1.0'. Validator pre-D35
accepted ANY value (including no value, undefined, '2.0',
numeric 1.0). Now: `obj.irVersion !== undefined && obj.irVersion
!== '1.0'` → rejection. Strict string match. `undefined` still
accepted (pre-D35 IRs without the field remain valid — back-compat
with sites that haven't yet been migrated to emit it). Error
message uses JSON.stringify for safe rendering.
4. **#12 — alignment.yml scripts/** trigger removal**
(.github/workflows/alignment.yml)
Pre-D35 push.paths and pull_request.paths listed scripts/**. The
scripts/ directory does not currently exist (per AGENTS.md note:
scripts/migrate-from-ocp.mjs is planned for Phase 7). A path
filter referencing a non-existent directory has no effect on
trigger evaluation BUT misleads readers about the workflow's
intent. Removed from both push.paths and pull_request.paths. When
scripts/ lands in Phase 7, the trigger should be re-added at the
same time (see release_kit_overlay.bootstrap_quirk_policy).
**Verification — #4 (uniform X-OLP-Latency-Ms across error paths)**
#4 was found to already be correct via D32. Re-audit of all 7
in-handler sendError sites in handleChatCompletions confirmed all
attach a 5-header set via olpHeaders or olpErrorHeaders:
- 360-361 (415 wrong Content-Type) → olpErrorHeaders
- 368-369 (400 bad JSON) → olpErrorHeaders
- 378-379 (400 BadRequestError IR translation) → olpErrorHeaders
- 402-407 (503 no chain) → olpErrorHeaders
- 617-618 (503 provider disappeared) → olpErrorHeaders
- 760-761 (502 streaming pre-first-chunk error) → olpHeaders
- 778-779 (500 fallback engine error) → olpErrorHeaders
The 404 (line 922) and outer 500 (line 926) are router-level paths
without startMs context and correctly lack OLP headers. D35 adds
the #4-audit regression test pinning the 5-header invariant on the
503 no-provider response so future drift is caught immediately.
**Tests** (test-features.mjs): 416 → 424 (+8):
- #4-audit ×1 (5-header invariant on 503 no-provider sendError)
- #9 ×1 (zero-chunk streaming → 200 + Content-Type=text/event-stream
+ 5 X-OLP-* headers + [DONE])
- #10 ×1 (catch-throw after first chunk → marker + length finish + DONE)
- #10b ×1 (provider error chunk after first chunk → same recovery)
- #11a ×1 (irVersion undefined accepted)
- #11b ×1 (irVersion '1.0' accepted)
- #11c ×1 (irVersion '2.0' rejected)
- #11d ×1 (irVersion numeric 1.0 rejected)
Pre-commit fold-in (per evidence-first checkpoint #4):
- **D35 reviewer flagged JSDoc/validator drift on irVersion**
(Suggestion #1, non-blocking). The @property typedef at
lib/ir/types.mjs:43 said `{string} irVersion - always IR_VERSION`
but the validator at lines 185-186 accepts `undefined`. Future
reader who scans the @property alone sees contradiction without
the rationale comment 140 lines below. Folded: typedef marked
`[irVersion]` (optional) and description updated to "optional;
when present must equal IR_VERSION ('1.0'). Pre-D35 IRs lack
this field and remain valid."
Two other non-blocking reviewer suggestions not folded (out of
scope for D35; tracked as future polish):
- Distinct event names for the two streaming_error_after_first_chunk
log sites (provider-emitted string vs JS exception message).
- Phase 7 TODO: re-add scripts/** trigger to alignment.yml when
scripts/migrate-from-ocp.mjs lands.
Authority:
- ADR 0004 § Fallback safety — post-first-chunk truncation surfaces
as `length` finish (#10 + #10b)
- ADR 0003 § IR contract — irVersion pinned to '1.0' (#11)
- AGENTS.md § Implementation status — scripts/ planned for Phase 7
(#12)
- CLAUDE.md release_kit_overlay phase_rolling_mode — D35 lands
under "Unreleased" against Phase 2; no version bump
- CC 开发铁律 v1.6 § 10.x — independent fresh-context reviewer
required for code change
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus,
independent of drafter): APPROVE_WITH_MINOR. Critical depth checks:
- Verified all 7 sendError sites in handleChatCompletions attach
5-header set (cited line numbers reconciled with current state)
- Verified writeHead block guarded by !res.headersSent; correctly
placed AFTER optional truncation marker, BEFORE SSE_DONE
- Verified irVersion validator strict-equality semantics across all
4 cases (undefined / '1.0' / '2.0' / numeric 1.0)
- Verified scripts/** removed from both push and pull_request paths
- Verified hygiene: 0 hits for personal markers, home paths, OAuth
tokens, internal IPs
- 424/424 tests pass independently in reviewer's run
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
9db4266bc5 |
release(phase-1-close): v0.1.0 — multi-provider proxy core
Phase 1 close per CLAUDE.md release_kit.phase_rolling_mode policy. Bumps package.json from 0.1.0-bootstrap → 0.1.0. Promotes CHANGELOG "Unreleased" section to "## v0.1.0 — 2026-05-24" with full D10-D34 commit index summarizing the 25 D-day commits that landed on `main` between 2026-05-23 and 2026-05-24. Tag v0.1.0 will be pushed after this commit lands and CI is green. release.yml auto-creates the GitHub Release from the CHANGELOG v0.1.0 section (extracted via awk pattern match on `## v0.1.0` heading). **What v0.1.0 delivers:** - OpenAI-compat HTTP entry surface (/v1/chat/completions, /v1/models, /health) with full X-OLP-* observability headers + Rule 2(b) no- invention compliance - Plugin architecture for 3 Tier-D providers (anthropic / codex / mistral) shipping as Candidate-not-Enabled per ALIGNMENT.md - Cache layer: D1 per-key isolation + D4 buffered-path singleflight + 10MB entry size cap + cacheable opt-out + per-(provider, model) isolation (D5 streaming singleflight deferred to v1.x per ADR 0005 Amendment 6) - Fallback engine: hard-trigger taxonomy + first-chunk safety + spawn- timeout race fix + SPAWN_FAILED salvage of usable partial chunks + structured per-hop log observability (chain_id, trigger_type, ir_request_hash, next_provider) - IR design + entry adapters honoring ADR 0003 deterministic serialization (including post-D33 F3 fix for deprecated function_call cache key stability) - 416 unit + integration tests with Suite 17 port-collision flake fix (D29) so CI is stable **Test growth**: 277 (pre-D10) → 416 (post-D34). +51%. **Audit cycle**: 6 cold-audit rounds + per-D-day diff reviews per Iron Rule v1.6 § 10.x dual-mode review discipline. 78+ findings raised; ~50 closed via implementation; ~28 deferred to GitHub issues #1-#17 for v1.x. **Phase 1 → v0.1.0 release_kit checklist** (per CLAUDE.md): - [x] All Phase 1 D-day deliverables landed on main (D10-D34) - [x] CI green on the head commit - [x] CHANGELOG "Unreleased" promoted to "## v0.1.0 — 2026-05-24" - [x] package.json bumped from 0.1.0-bootstrap → 0.1.0 - [ ] Tag pushed (next step in this PR's lifecycle) - [ ] release.yml triggered + GitHub Release created (auto on tag push) **Known limitations carried to v1.x** (full list in CHANGELOG.md v0.1.0 § "Known limitations"): - Streaming singleflight not implemented (issue #16) - maxConcurrent runtime enforcement declarative-only (#1) - X-OLP-Fallback-Detail header documented but never emitted (#7) - Soft trigger quotaStatus polling not wired (per ADR 0004 Amendment 2) - 14 other follow-up items: #2 #3 #4 #5 #6 #8 #9 #10 #11 #12 #13 #14 #15 #17 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>v0.1.0 |
||
|
taodengandClaude Opus 4.7
|
60570ef074 |
fix+docs: D34 — FINAL batch (F1+F4+F7+F8); audit cadence stops
cold-audit catch from 2026-05-24 (round 6 — FINAL)
This is the closing D-day of a 24-day round-1→round-6 audit cycle.
After this commit + the 9 round-6 follow-up issue filings, no more
audit rounds. Trajectory R1=17 → R2=13 → R3=13 → R4=10 → R5=12 →
R6=14 — the method did not converge; owner chose Option A (focused
batch of most consequential items, then STOP).
Changes (6 files, +138 / -47):
**Code changes**
1. lib/cache/keys.mjs (+14/-?) — F4 P2 cache key array-field
normalization:
- New `normalizeArrayField` helper: `(Array.isArray(v) && v.length === 0) ? null : (v ?? null)`
- Applied to `tools` and `stop` in computeCacheKey
- Now `tools: []` and `tools` omitted produce IDENTICAL cache
keys (and same for `stop: []` vs omitted). ADR 0005 Amendment 2's
own claim that "[] and undefined share a cache entry" was
empirically FALSE pre-D34; round-6 reviewer verified hashes
differ. The fix makes the claim literally true at the
key-composition layer.
2. lib/providers/base.mjs (+16/-?) — F7 P2 dead error code removal:
- `QUOTA_EXHAUSTED` removed from PROVIDER_ERROR_CODES
- `RATE_LIMITED` removed from PROVIDER_ERROR_CODES
- v0.1 live codes: SPAWN_FAILED, CLI_NOT_FOUND, AUTH_MISSING,
SPAWN_TIMEOUT
- Comment block documents removal + cites ADR 0004 Amendment 3
3. lib/fallback/engine.mjs (+29/-?) — F7 P2 sibling:
- HARD_TRIGGER_CODES: QUOTA_EXHAUSTED + RATE_LIMITED removed
- SPAWN_FAILED, CLI_NOT_FOUND, AUTH_MISSING(false), SPAWN_TIMEOUT
remain
- evaluateHardTriggers HTTP-status branches KEPT (option (b)) with
forward-compat comment: "v0.1 plugins never attach statusCode;
branches reserved for v1.x when plugin gains HTTP-status parsing"
4. test-features.mjs (+93) — F4 + F7 test work:
- 4 new F4 regression tests (tools:[] vs undefined, stop:[] vs
undefined, tools:[] vs null, tools:non-empty vs undefined sanity)
- ~14 integration test code-swap edits (QUOTA_EXHAUSTED →
SPAWN_FAILED, RATE_LIMITED → SPAWN_FAILED) preserving original
hard-trigger semantic
- 2 dead unit tests for QUOTA_EXHAUSTED/RATE_LIMITED removed
(tombstone comment retained for audit trail)
**ADR amendments (docs-only, no code change)**
5. docs/adr/0004-fallback-engine.md (+12) — F7 Amendment 3:
- Documents the v0.1 hard-trigger code narrowing
- 4 live codes listed explicitly
- Captures evaluateHardTriggers HTTP-status branch retention rationale
- v1.x re-activation path: plugin gains HTTP-status parsing →
re-add codes → branches activate naturally
6. docs/adr/0005-cache-cross-provider.md (+21) — TWO amendments + 1
prior-amendment update:
- **Amendment 6 (F1 P1)**: Formal v1.x deferral of D4 streaming
singleflight. Buffered path (executeHopFn) uses cacheStore.getOrCompute
and participates in D4 fully. Streaming cache-miss path
(server.mjs:609-741) bypasses singleflight — N concurrent identical
streamers each spawn fresh. v0.1 trade-off accepted for
personal/family scale; v1.x design ADR needed for tee-streaming +
per-key inflight Map. Cross-references CLAUDE.md release_kit.
phase_rolling_mode as the deferral pattern precedent.
- **Amendment 7 (F8 P2)**: Documents the v0.1 conservative cache-key
posture: includes all IR fields including those plugins discard
(anthropic/codex/mistral drop temperature/max_tokens/top_p/stop/
tools/tool_choice at spawn). Consequence: 2 requests with different
temperature produce identical CLI output (CLI ignores) but
different cache keys → spurious miss. Trade-off justified:
spurious miss > spurious hit. v1.x forward path:
per-plugin cacheKeyFields contract extension (ADR 0002 amendment
needed). 3 implementation subtasks enumerated for the v1.x PR.
- **Amendment 2 update (F4)**: heading renamed to "Note on
null-coalescing AND array normalization"; body documents the
new normalizeArrayField helper; quotes the regression test name.
Tests: 414 → 416 (+4 F4 regression, -2 F7 dead, +0 net from F7
integration rewrites).
Pre-commit fold-in: NONE — D34 reviewer APPROVE with all 4 suggestions
non-blocking/cosmetic.
Authority:
- ADR 0005 Amendment 2 invariant restored at code level (F4)
- ADR 0005 Amendment 6 formalizes F1 streaming-singleflight deferral
per the same pattern as D22 ADR 0004 Amendment 2 soft-trigger
deferral
- ADR 0005 Amendment 7 documents F8 conservative posture as v0.1
intentional design (not an accident)
- ADR 0004 Amendment 3 narrows v0.1 trigger taxonomy (F7)
- Round-6 cold audit findings F2 / F3 / F6 / F9 / F10 / F11 / F12 /
F13 / F14 filed as GitHub issues after this commit (NOT in scope)
- CC 开发铁律 v1.6 § 10.x — final round of the audit cadence
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Critical depth checks:
- B5 over-normalization: verified `normalizeArrayField` only applies
to `tools` and `stop`; `response_format: {}` and `tool_choice: ''`
unaffected (Array.isArray guard)
- C10 test cleanup: 21 references reconciled (3 tombstone, 18
rewrites/removals); integration test rewrites preserve hard-trigger
semantics (QUOTA_EXHAUSTED → SPAWN_FAILED is also a hard trigger,
so fallback advancement behavior unchanged)
---
**End of audit cycle.** 24 D-days shipped from D10 (P1 hardening) through
D34 (final batch). 6 cold-audit rounds executed; 78+ findings raised;
~50 closed via implementation; ~28 deferred to GitHub issues / v1.x
ADR amendments. v0.1 tag remains explicit-maintainer-action per
phase_rolling_mode policy.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
f784fdb947 |
fix+docs: D33 — round-5 cleanup batch (F1/F3/F5/F8/F9/F10/F11/F12)
cold-audit catch from 2026-05-24 (round 5)
Round-5 cold-audit cleanup batch. 8 items + 1 release-discipline
reconciliation. Largest batch by line count (582+/39-) but every item
is small-and-focused. 3 P2 items (F1/F3/F5 of which F3 + F1 are real
correctness/observability fixes; F5 backfills /health to spec).
Changes (10 files, +583/-39):
**P2 fixes**
1. **F1 — ALIGNMENT.md mistral authority pin self-contradicted plugin**
(ALIGNMENT.md): row cited `vibe --prompt --output json` but mistral.mjs
uses `--output streaming` (the plugin header at lines 360-369 even
justifies WHY: `--output json` emits single blob, breaks NDJSON
line-buffered parser). Constitution self-contradicting itself —
missed across 4 prior rounds. Pin updated to `--output streaming`
with DOCS-1 reference.
2. **F3 — Deterministic function_call synth ID**
(lib/ir/openai-to-ir.mjs): deprecated `function_call` translation
produced `id: \`fc-${Date.now()}\`` → ID flows into normalized
tool_calls → cache key SHA-256. Two identical requests separated
by ≥1ms → different cache keys → cache always misses for
`function_call` request shape. Violates ADR 0005 invariant
"same inputs → same key, no random, no timestamp."
Fixed: id is now `fc-<16-hex>` from SHA-256 of `${name}\0${arguments}`.
NUL separator prevents the (name='ab',args='c') vs (name='a',args='bc')
collision. 2^64 collision resistance is more than sufficient for
tool_call ID disambiguation (per-request semantic key, not crypto
primitive).
**P3 fixes**
3. **F5 — /health invokes per-plugin healthCheck()** (server.mjs +
docs/openai-spec-pin.md): ADR 0002 says "healthCheck — startup AND
/health endpoint use this." Pre-D33 /health returned only
{enabled, available} counts. Now async, iterates loadedProviders,
awaits each plugin's healthCheck() in try/catch. Returns
`providers: {enabled, available, status: {<name>: {ok, latencyMs?, error?}}}`.
4. **F8 — X-OLP-Cache reports fallback-hop cache hits** (server.mjs):
pre-D33 cacheStatus computed from `preCheckHit && fallbackHops === 0`
— only counted primary-hop cache hits. When fallback fires + the
fallback hop's getOrCompute returns from cache, header reported
`miss` despite no spawn happening.
Fixed: peek BEFORE getOrCompute inside executeHopFn, set
`lastHopWasCached` closure variable on every hop (last-write-wins
= serving hop's state). cacheStatus combines
`lastHopWasCached || (preCheckHit && fallbackHops === 0)`.
F8 chose option (b) peek-then-getOrCompute over option (a)
getOrCompute API change because option (a) would break ~15 test
callsites for marginal benefit. Accepted race window same as
existing preCheckHit pattern.
5. **F9 — validateProvider hints error message updated** (lib/providers/
base.mjs): pre-D33 message listed cacheable as missing and
maxSpawnTimeMs as required. Now: `'hints must be an object with
{ requiresTTY, concurrentSpawnSafe, maxConcurrent } + optional
{ maxSpawnTimeMs, cacheable }'`.
6. **F10 — Dead cache-write branch removed** (server.mjs): the
`if (hasStopChunk)` check in the streaming stop-less exhaustion
branch was unreachable (the stop-chunk completion path returns
earlier inside the for-await loop). Removed the dead code + added
a comment documenting the invariant.
**Governance/policy**
7. **F11 — Phase rolling mode policy formalized** (CLAUDE.md +
CHANGELOG.md): 22+ D-day commits accumulated under "Unreleased"
without per-D version bumps — Iron Rule 5 (release-kit bump-before-
push) appeared to be silently violated. Reality: per-D bumps would
produce 30+ noise tags during Phase 1. F11 formalizes the policy:
intra-Phase D-day commits accumulate under Unreleased; bump+tag
fires explicitly at Phase close (maintainer-triggered, not
automated). CLAUDE.md release_kit overlay gains `phase_rolling_mode`
block documenting the exception with self-pointer ("if Rule 5
appears silently violated, check this section first"). CHANGELOG
"Unreleased" gets a notice at top.
**No version bump, no git tag in D33** — policy formalization only.
8. **F12 — /v1/models created is stable per-model timestamp**
(models-registry.json + lib/providers/index.mjs + server.mjs +
docs/openai-spec-pin.md): pre-D33 used Math.floor(Date.now()/1000)
per request — violates OpenAI spec which treats `created` as
per-model attribute. Clients caching models by created would see
spurious updates on every poll.
Fixed: models-registry.json gains `bootstrapCreated: 1778630400`
top-level constant + per-model `created` fields where known
(anthropic claude-{opus,sonnet,haiku} with estimated release dates;
devstral models from "25-12" suffix; codex models pinned to
bootstrap pending verified release dates). handleModels uses
`getModelCreated(modelId)` helper from lib/providers/index.mjs.
Aliases share canonical's timestamp.
**Tests** (test-features.mjs): 401 → 414 (+13):
- F3 ×3 (same input → same id → same cache key; different name → different)
- F5 ×4 (empty/single/multi/throwing-plugin /health shapes)
- F8 ×1 (2-hop primary-fail + secondary-cache-hit → X-OLP-Cache: hit)
- F12 ×5 (stability/fallback/alias-equals-canonical)
Pre-commit fold-in (per evidence-first checkpoint #4):
- **D33 reviewer flagged F3 empty-args asymmetry** (Concern #1): hash
input used `?? ''` (empty stays) but emitted IR field used
`|| '{}'` (empty becomes '{}'). Consequence: `arguments: ''` and
`arguments: '{}'` emit identical IR but compute different ids →
different cache keys for semantically-identical requests. The exact
cache-stability bug F3 was supposed to fix.
Folded in: canonicalize empty-args to '{}' BEFORE hashing. Hash
input now matches IR emission exactly. Same line change resolves
the asymmetry.
Authority:
- ALIGNMENT.md self-amendment (F1 pin correction)
- ADR 0005 invariant "same inputs → same key, no random, no timestamp"
(F3 restoration)
- ADR 0002 § Provider contract "/health uses healthCheck" (F5)
- ADR 0004 § Observability headers (F8 X-OLP-Cache correctness)
- ADR 0005 § Cache write conditions item 1 (F10 truncation-not-cached
invariant explicit)
- Iron Rule 5 (F11 release-kit reconciliation)
- OpenAI /v1/models spec — `created` per-model stable (F12)
- CC 开发铁律 v1.6 § 10.x — Round-5 Cold Audit caught all 8
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Verified:
- F1 plugin cross-reference (mistral.mjs:360-369) accurately documents
the rationale
- F3 collision resistance + NUL separator + restored cache invariant
- F5 all 4 cases (empty/single/multi/throwing) work
- F8 closure semantics across multi-hop chains (verified hop-fail +
fallback-hit case)
- F10 dead code removal preserves the stop-chunk completion path
- F11 phase_rolling_mode policy honest about what happened and what
the going-forward rule is
- F12 stability across consecutive /v1/models calls; alias-canonical
parity
- 414/414 tests pass
3 remaining non-blocking suggestions (F3-vs-modern-tool_calls path
canonicalization symmetry; F12 codex models explicit-vs-fallback
writeup mismatch; F8 servingHopWasCached naming) tracked as future
polish; not folded.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
30de965e8e |
fix+docs: D32 — round-4 cleanup batch (F2/F3/F4/F5/F7/F8/F9)
cold-audit catch from 2026-05-24 (round 4) Round-4 cold-audit cleanup batch. 7 items grouped per IDR cleanup-batch convention (D19/D20/D25/D26/D30/D31 precedent). 2 P2 + 3 ADR amendments + 2 small docs/code cleanups. F10 (P3 semantic edge case) filed separately as GitHub issue #8. Changes (8 files, +200 / -38): **P2 fixes** 1. **F3 — README missing 6 provider auth env vars** (README.md): D30 fixed the `OLP_*` table but missed the auth-bearing env vars actually read by plugin code: - `CLAUDE_CODE_OAUTH_TOKEN` (anthropic.mjs:84) — highest-precedence override; bypasses keychain + .credentials.json file lookup - `OPENAI_CODEX_AUTH_PATH` (codex.mjs:163) — overrides full auth file path; when set, no other path tried - `CODEX_HOME` (codex.mjs:176) — overrides base dir; default auth path becomes `$CODEX_HOME/auth.json` - `MISTRAL_API_KEY` (mistral.mjs:260) — directly supplies API key; highest precedence per Mistral DOCS-2 - `MISTRAL_VIBE_AUTH_PATH` (mistral.mjs:265) — overrides full .env path; evaluated only when MISTRAL_API_KEY absent - `VIBE_HOME` (mistral.mjs:277) — overrides Vibe base dir; default auth path becomes `$VIBE_HOME/.env` Real onboarding-blocker fix: new users couldn't run OLP without knowing about these env vars; README now documents them in a "Per-provider auth env vars" subsection. 2. **F8 — Early-return error paths missing X-OLP-* headers** (server.mjs): ADR 0004 § Observability + README claim "every response carries the 5 X-OLP-* headers" but 503 (no-enabled-providers), 415 (wrong Content-Type), and early 400s (bad JSON, IR validation) emitted only Content-Type + Content-Length + X-OLP-Latency-Ms (D18 only wired the chain-exhausted path). Operators debugging these paths got less info than the ADR promised. New helper `olpErrorHeaders({ startMs, model })` emits the canonical "no provider attempted" defaults: X-OLP-Provider-Used: 'none' X-OLP-Model-Used: model ?? 'unknown' X-OLP-Fallback-Hops: '0' X-OLP-Cache: 'bypass' X-OLP-Latency-Ms: <delta> 6 sendError call sites updated: 415 / 400-bad-JSON / 400-IR-parse (model: undefined → 'unknown') + 503-no-providers / 503-provider- disappeared / 500-engine-programming (model: ir.model). The 502 streaming-error-before-first-chunk path correctly stays on `olpHeaders(...)` since a provider WAS attempted. **ADR amendments** 3. **F2 — ADR 0003 model-mapping example correction** (docs/adr/0003, Amendment 2): the § Required fields example claimed `claude-sonnet-4-6` → `claude-sonnet-4-6-20260301` mapping happens in the provider plugin. This was wrong: per D17 SPOT decision (commit |
||
|
taodengandClaude Opus 4.7
|
d6347e33f2 |
docs(governance): D31 — ADR amendment trio (F5 ADR 0003 + F11 ADR 0005 § D2 + F13/F14 ALIGNMENT Speculative-Candidate class)
cold-audit catch from 2026-05-24 (round 3)
Three coordinated governance amendments per the major-decisions already
made (per "继续吧 unless major decision" + my F5/F11/F13+F14 option-(b)
calls earlier in the session). All P3-class, all docs-only. Single
PR per IDR cleanup-batch convention given the unified theme
("spec was aspirational; reality is X; amend spec to match").
Changes (3 docs files, +52 / -2):
1. docs/adr/0003-intermediate-representation.md — Amendment 1 (NEW
Amendments section, first ever for ADR 0003):
- F5 closure: removes the unimplemented `__irRoundTripTest()`
export-name claim from § Mitigations. The mental model
(symmetric irToNative+nativeToIR pair on each plugin) does NOT
fit the actual plugin shape (asymmetric spawn-wrappers:
request→CLI-args+stdin, response-stream→IR-chunks). Zero plugins
export this function.
- Documents the substitute test strategy that IS live at v0.1:
· Suite 3 covers entry-surface IR↔OpenAI round-trip
· Per-plugin spawn-with-mock test blocks cover IR→CLI→IR-chunk
· Lossy-translation edges documented inline in each plugin's
header comment (mistral.mjs DOCS-N markers being the most
thorough; anthropic.mjs + codex.mjs carry equivalent tables)
- Future plugins MUST satisfy both (a) spawn-with-mock test block
and (b) header lossy-field documentation — the enforcement
mechanism replacing the original export-name aspiration.
- Original § Mitigations bullet replaced with a pointer to
Amendment 1 (preserves history).
2. docs/adr/0005-cache-cross-provider.md — Amendment 5 (after D27
Amendment 4):
- F11 closure: acknowledges that the "Anthropic's own prompt cache
is consulted at the provider" half of § D2 is structurally
impossible at v0.1.
- Root cause: Anthropic plugin uses `claude -p --output-format text`
(verified at anthropic.mjs:196), a plain-text wire surface that
has no Messages-API field for cache_control markers. The markers
are also stripped at IR construction (openai-to-ir.mjs:45-89
whitelists fields, drops cache_control) per ADR 0003 IR design
reaffirmed in D27 Amendment 4 (F10).
- Clarifies v0.1 effective behavior: OLP-cache-bypass works
correctly (no double-caching when Anthropic + markers); the
delegate-to-Anthropic-prompt-cache half does not fire.
- Forward path (v1.x): switch Anthropic plugin to --output-format
json (NDJSON wire) or direct Messages API spawn — substantial
parser rewrite; deferred since no user has requested delegation.
- § D2 paragraph stays intact; an inline parenthetical reference
to Amendment 5 is added immediately after the affected sentence.
3. ALIGNMENT.md — new "Rule 4 exception class — Speculative-Candidate
Plugin" sub-section after Rule 5, before § Authorities:
- F13+F14 closure: formalizes the gap between strict ALIGNMENT
Rule 2 (no speculative shape assumptions) + Rule 4 (unalignable
plugins are deleted, not feature-flagged) and the actual practice
where Codex (D6) + Mistral (D8) plugins ship with explicit
UNPINNED assumptions documented in their headers, gated as
Candidate-not-Enabled.
- 5 precise conditions for a Speculative-Candidate plugin:
(1) STATIC_REGISTRY entry present, (2) candidate: true in
models-registry, (3) NOT in any user's providers.enabled config,
(4) header has explicit UNPINNED assumption section with labeled
IDs and planned-pin triggers, (5) defensive multi-shape parsers
tied to UNPINNED markers (greppable for future single-shape
replacement).
- Enablement criteria (Speculative-Candidate → Enabled): 3 explicit
requirements (remove UNPINNED labels from header; replace
multi-shape parsers with single-shape; file pin ADR amendment).
- Currently-in-class table:
· codex.mjs (D6) — A3 (auth token field name), A4 (NDJSON event
schema)
· mistral.mjs (D8) — A4 (JSON output event schema), A5 (model
flag), A6 (exact model IDs), A7 (streaming vs json output
mode), A8 (stdin prompt passing)
- Anthropic explicitly NOT in this class (CLI version pinned at
@anthropic-ai/claude-code v2.1.89 per Provider Authority Pins
table).
- CI enforcement noted as deferred (reviewer obligation today; a
future PR may add grep-based gate).
Pre-commit fold-in (per evidence-first checkpoint #4):
- **D31 reviewer flagged wording precision** ("Provider Inventory
table above" was structurally wrong — the table is BELOW the new
sub-section). Folded in: 1-word fix "above" → "below" on the
Speculative-Candidate condition #2.
Tests: 400/400 unchanged (docs-only).
Authority:
- ADR 0003 § Mitigations (modified) + new Amendment 1 (self-amendment)
- ADR 0005 § D2 (annotated) + new Amendment 5 (self-amendment)
- ALIGNMENT.md § Rules (extended with Rule 4 exception class)
- D27 F10 Amendment 4 of ADR 0005 (cited for cache_control IR-strip
precedent)
- CC 开发铁律 v1.6 § 10.x — Round-3 Cold Audit caught all 3 items
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Verified:
- Suite 3 + 3 plugin spawn-with-mock blocks + 3 plugin lossy-field
header tables all present (F5 substitute test strategy claims
match implementation)
- anthropic.mjs:196 hardcodes --output-format text + openai-to-ir.mjs
drops cache_control from messages (F11 wire-limitation technically
accurate)
- All 8 UNPINNED assumption labels (codex A3/A4 + mistral A4/A5/A6/A7/A8)
match plugin header text exactly — no invention
- Anthropic-not-in-class claim verified against Provider Authority Pins
table
Follow-up items (reviewer's non-blocking notes, NOT in this PR):
1. **mistral.mjs A5 internal disconnect** — header lines 145-156 label
A5 as `UNPINNED-D-later-verifies`, but function body at 371-374
already cites DeepWiki enumeration as the pin and marks A5
`CONFIRMED-NOT-APPLICABLE` (no --model flag exists in vibe CLI).
The disconnect predates D31 and is a per-plugin header cleanup
(not a constitutional amendment), so it stays out of D31 scope per
IDR. File as separate follow-up issue — A5 should be moved out of
the Speculative-Candidate table once the header is updated.
2. **ADR 0005 Amendment ordering cosmetic** — strict reverse-chronological
would put Amendment 5 above Amendment 4 (current order is 4, 5, 3,
2 because both 4 and 5 share the 2026-05-24 date and 5 was inserted
after 4). Not blocking; future cleanup if maintainers want strict
numeric-descending.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
5119b427fd |
docs: D30 — README env vars correctness (F7) + openai-spec-pin.md v0.1 baseline (F20)
cold-audit catch from 2026-05-24 (round 3) Round-3 P3 docs batch. Two unrelated docs items grouped per IDR cleanup convention (D19/D20/D25 precedent). **F7** — README Environment Variables table drift. Pre-D30 table documented `OLP_HOME` and `OLP_LOG_LEVEL` — neither is read by any code in the repo. The table missed `OLP_CLAUDE_BIN`, `OLP_CODEX_BIN`, `OLP_VIBE_BIN` which the 3 provider plugins DO read to override the path to each provider's CLI binary. `grep -rn "process\.env\.OLP_" server.mjs lib/` returns exactly 4 reads: - `OLP_PORT` (server.mjs:49) - `OLP_CLAUDE_BIN` (anthropic.mjs:69) - `OLP_CODEX_BIN` (codex.mjs:143) - `OLP_VIBE_BIN` (mistral.mjs:240) Fix: README env-vars table now lists exactly these 4 active vars with their actual defaults. `OLP_HOME` + `OLP_LOG_LEVEL` moved to a "📋 Planned (Phase 2)" callout block beneath the table, with honest explanations linking to the actual code state (`loadFallbackConfigSync` hardcodes the config path; `logEvent` writes unconditionally). **F20** — Author docs/openai-spec-pin.md v0.1 baseline. ALIGNMENT.md Authority 2 + Annual Alignment Audit § Scope both referenced `docs/openai-spec-pin.md` as the artifact the annual audit diffs against. Pre-D30 the file didn't exist (D20 marked it 📋 Planned). This left the entry-surface audit without a diff baseline — v0.1 ships with no provable "the OpenAI spec was THIS on the day OLP implemented its entry surface" anchor. Fix: author a 175-line minimal v0.1 baseline. Every field claim is verified against source (openai-to-ir.mjs + ir-to-openai.mjs + server.mjs). Structure: - POST /v1/chat/completions: 14 supported request fields (model, messages + 6 message-level subfields, stream, temperature, max_tokens, top_p, stop, tools, tool_choice, response_format) + 11 NOT-yet-supported fields (n, seed, frequency_penalty, presence_penalty, logit_bias, logprobs, top_logprobs, user, service_tier, parallel_tool_calls, stream_options) - Response shapes: chat.completion (non-stream), chat.completion.chunk (streaming) — verified against irResponseToOpenAINonStream and irChunkToOpenAISSE - `finish_reason` enum: verified against OPENAI_FINISH_REASON_ENUM constant in ir-to-openai.mjs (post-D19 + D26) - Error response shape: HTTP 4xx/5xx + `{error: {message, type}}` — verified against `sendError` in server.mjs - GET /v1/models: verified against handleModels (post-D18 + D27 F15) - Streaming SSE semantics: framing, terminator, post-D26 F19 truncation marker The pin also documents the v0.1 → v1.0 forward-looking expansion plan: the "NOT yet supported" fields are explicit candidates for v1.0+ implementation via openai-to-ir.mjs amendments + ADR 0003 updates. ALIGNMENT.md + README Implementation status table both flip the marker from 📋 Planned to ✅ Shipped (D30) with the 2026-05-24 timestamp. Changes (3 files, +180 / -8): - ALIGNMENT.md +2/-2 (2 markers updated: Authority 2 + Annual Audit) - README.md +11/-6 (env-vars table delta + status table marker flip + Planned callout for OLP_HOME/OLP_LOG_LEVEL) - docs/openai-spec-pin.md (new, 175 lines) Tests: 400/400 unchanged — pure docs change. Authority: - F7 → process.env reads verified by direct grep - F20 → OpenAI Chat Completions spec https://platform.openai.com/docs/api-reference/chat/create https://platform.openai.com/docs/api-reference/chat/streaming https://platform.openai.com/docs/api-reference/chat/object https://platform.openai.com/docs/api-reference/models/list - F20 internal source-of-truth: openai-to-ir.mjs + ir-to-openai.mjs + server.mjs (all field claims traced) - ALIGNMENT.md § Authority 2 + § Annual Alignment Audit - CC 开发铁律 v1.6 § 10.x — Round-3 Cold Audit caught both items Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent of drafter): APPROVE. Independent verification: - Grep confirmed exactly 4 process.env.OLP_* reads — matches new env vars table - Each new var's default value verified against the plugin code (anthropic.mjs:69 → 'claude'; codex.mjs:143 → 'codex'; mistral.mjs:240 → 'vibe') - All 14 spec-pin supported request fields traced to openAIToIR line references (model L129, messages L45-89, stream L141, temperature L160, max_tokens L152, top_p L168, stop L176, tools L183, tool_choice L187, response_format L191) - All 11 NOT-supported fields confirmed absent via grep - Response shape claims (chat.completion + chat.completion.chunk + /v1/models) all match source code line-by-line - ALIGNMENT.md markers — pure markup flip, no rule changes - 400/400 tests pass 3 non-blocking suggestions noted (function_call finish_reason caveat; README slug rendering; ADR 0003 cross-link in spec-pin) — all cosmetic, not folded. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
taodengandClaude Opus 4.7
|
de9f3ca7c9 |
test: D29 — eliminate port-collision flake class (16 sites, listen(0))
cold-audit catch from 2026-05-24 (round 3 — D27 flake follow-up)
Pre-existing fragility from D18-era test design: Suite 17 + several
other test sites used overlapping random port ranges (e.g., 25456-25855
and 25460-25859 in Suite 17) with `Math.floor(Math.random() * 400)`
allocation. Sequential close-then-open on Linux Node 20 with TIME_WAIT
timing caused intermittent ECONNRESET on subsequent fetches.
Concrete failure: D27 first CI run failed Suite 17 test 17b with
`socket hang up / ECONNRESET` 1.3ms into the fetch. CI re-run on the
same commit was green — confirming flake, not logic bug. D27's code
changes were unrelated; the flake-prone pattern just happened to
collide on that run.
Fix: switch from random-port-in-fixed-range to OS-assigned port via
`listen(0, ...)`. Each test gets a unique OS-assigned port; collision
is structurally impossible.
Pattern:
```js
const s = createXX();
await new Promise((resolve, reject) => {
s.listen(0, '127.0.0.1', resolve);
s.once('error', reject);
});
const p = s.address().port;
// ... use p in fetch
```
Changes (test-features.mjs +32 / -32):
16 sites updated, organized by Suite:
- Suite 14: 14c (no providers), 14d (anthropic enabled)
- Suite 15 `before()`: shared `port15` for 15a-15d (assigned post-listen
in before-hook)
- Suite 17 (D18-era): 17a, 17b, 17c, 17d (4 /v1/models tests) +
17e, 17f, 17g (3 error-header tests)
- F19 `before()`: shared `portF19` (assigned post-listen in before-hook)
- D27 F15 a/b/c/d/e (5 alias-surfacing tests)
Out-of-scope sites intentionally LEFT alone (test-features.mjs lines
1253, 1882, 2048, 2212, 3048, 4382): they use wider random ranges
(500-1000) AND already have EADDRINUSE retry logic, so collision risk
is structurally bounded. Not the same defect class.
Stability verification:
- 5 consecutive local `npm test` runs (sonnet) — all 400/400 green
- 3 additional consecutive runs (reviewer) — all 400/400 green
- Durations 596-620ms; no Suite 17/15/F19 flakes observed
Pure mechanical refactor:
- No assertion changes (`grep -cE "^[+-].*assert\." diff` returns 0)
- No production code modified — only test-features.mjs
- No behavior contract change — tests assert the same things
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Verified each of 16 sites uses correct order
(listen before address().port read), Suite 15 + F19 before-block
variables correctly assigned AFTER listen-promise resolves, out-of-scope
sites unchanged, hygiene clean. 3/3 local runs green.
Note for future test additions: prefer `listen(0, '127.0.0.1', resolve)
→ s.address().port` over any fixed-range random port allocation. The
OS-assigned approach makes port collision structurally impossible.
Authority:
- Node.js HTTP server documentation:
https://nodejs.org/api/net.html#serverlisten — port 0 OS assignment
- D27 CI failure on Suite 17 test 17b (workflow run 26353907900,
Tests step — first run failed, re-run green)
- CC 开发铁律 v1.6 § 10.x — Round-3 Cold Audit pattern continued:
D27's flake was a known evidence-first checkpoint violation
(per ~/.cc-rules/memory/feedback/evidence_first_under_speed_pressure.md
"host-environment trap" corollary). D29 closes the underlying
fragility class so the same flake can't reoccur on subsequent D-days.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
4a238c9cc4 |
feat(fallback)+chore(cache): D28 — per-hop log observability fields (round-3 F2)
cold-audit catch from 2026-05-24 (round 3)
Round-3 cold-audit Finding 2 (P2 observability gap). ADR 0004 §
Observability headers requires per-hop structured log events to carry
7 fields: timestamp, chain id, hop index, failed provider, trigger type,
IR request hash, and downstream provider that was tried next. Pre-D28
events only carried timestamp/hop/provider/model/error/code — operators
could not:
- correlate hops across one logical request (no chain_id)
- pivot per-prompt via fingerprint (no ir_request_hash)
- tell from one log line which bucket classified the error (no
trigger_type)
- see lookahead routing (no next_provider)
Changes (3 files, +431 / -3):
1. lib/cache/keys.mjs (+35):
- New export `computeIRRequestHash(ir)` returning a 16-char SHA-256
prefix over the IR fields that define request semantics
(messages-normalized, tools, temperature, max_tokens, top_p, stop,
tool_choice, response_format). Excludes provider/model/cache_control
so the hash is provider-agnostic — identical prompts across a
fallback chain produce identical fingerprints, enabling log
correlation. 16-char prefix gives 2^64 collision resistance —
astronomically safe for the use case while keeping log lines compact.
2. lib/fallback/engine.mjs (+86 / -3):
- New helper `classifyTrigger(error)` returning one of:
'hard' / 'auth_missing' / 'client_error' / 'non_trigger' / null.
ProviderError with HARD_TRIGGER_CODES → 'hard'; AUTH_MISSING → its
own bucket; HTTP 400/401/403/404/422 → 'client_error' (per ADR 0004
§ "No fallback for client-side errors"); HTTP 5xx or non-client 4xx
(e.g. 429/529) → 'hard'; other → 'non_trigger'. 'soft' is reserved
in JSDoc for forward-compat per D22 Amendment 2's deferral of soft
triggers to v1.x.
- `executeWithFallback` now generates `chainId` (crypto.randomBytes(8)
.toString('hex')) and `irRequestHash` (computeIRRequestHash(ir))
ONCE at entry — both stay constant across all hops in the chain.
- All 8 logEvent call sites augmented with the 4 new fields:
fallback_soft_trigger, fallback_hop_success, fallback_hop_error,
fallback_client_error_no_fallback, fallback_auth_missing_no_fallback,
fallback_hard_trigger, fallback_non_trigger_error,
fallback_chain_exhausted.
- `next_provider` is `chain[i + 1]?.provider ?? null` on advancement
events; null on terminal events (success / client_error /
auth_missing / chain_exhausted).
- chain_id NOT reused from generateRequestId() since that produces
OpenAI-response-scoped `chatcmpl-<base64url>` IDs; chain_id is
internal log correlation and uses a clean 16-char hex token.
3. test-features.mjs (+313): 24 new tests covering:
- computeIRRequestHash determinism + 16-char hex shape + 8 per-field
sensitivity tests (one per IR input field)
- Provider-agnosticism (same hash across different provider/model)
- chain_id uniqueness across calls + consistency across hops
- trigger_type for each branch (hard / auth_missing / client_error /
non_trigger / null)
- next_provider correctness (chain[i+1] on advance; null at
exhaustion)
Tests: 376 → 400 (+24). All pass on Node 20.
Pure observation-only change: no fallback decision logic touched.
`evaluateHardTriggers`, `evaluateSoftTriggers`, `HARD_TRIGGER_CODES`,
`CLIENT_ERROR_STATUSES` are only READ by `classifyTrigger`; the existing
decision branches are textually unchanged — only the trailing logEvent
object literals are expanded.
Authority:
- ADR 0004 § Observability headers (the 7-field requirement)
https://github.com/dtzp555-max/olp/blob/main/docs/adr/0004-fallback-engine.md
- ADR 0004 § "No fallback for client-side errors" — 401/403/404/422
bucket (used by classifyTrigger's CLIENT_ERROR_STATUSES branch)
- ADR 0005 Amendment 2 (D15) — cache key composition fields, mirrored
in computeIRRequestHash minus provider/model/cache_control
- D22 Amendment 2 (ADR 0004) — soft triggers deferred to v1.x;
'soft' reserved in JSDoc but unreturnable
- CC 开发铁律 v1.6 § 10.x — Round-3 Cold Audit caught this
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Traced all 8 classifyTrigger branches against
ADR 0004 wording; verified 401/403/404/422 correctly classified as
'client_error' (never 'hard') per the ordering of CLIENT_ERROR_STATUSES
check before the generic status>=400 fallthrough; confirmed observation-
only via diff inspection (only logEvent object literals expanded, no
control-flow changes); independently ran git-stash to verify pre-D28
baseline 376 + 24 = 400.
3 non-blocking suggestions noted (JSDoc 'soft' union vs unreachable
return; future cross-correlation of chain_id with chatcmpl response IDs;
log-line redundancy between fallback_hop_error + fallback_hard_trigger
for same hop) — all cosmetic, not folded.
Note on D27 CI: D27 first CI run failed 1/376 on a Suite 17 port-
collision flake (overlapping random port ranges across Suite 17 tests +
Linux TIME_WAIT timing). Re-run was green. The flake is pre-existing
from D18-era test design; tracked as D29 follow-up to switch Suite 17
to OS-assigned port 0. D28 doesn't touch Suite 17 and is unaffected.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
c3ba751a8f |
feat: D27 — round-3 P3 batch (F8 IR validator + F10 ADR amend + F15 /v1/models aliases)
cold-audit catch from 2026-05-24 (round 3)
3 round-3 P3 items batched per IDR. Mixed surfaces but all single-severity
and conceptually independent.
Changes (5 files, +324 / -17):
1. lib/ir/types.mjs — F8 validator extension (+28):
- `response_format`: must be object with string `.type` (undefined/omitted
accepted; null, non-object, missing-type all rejected). Forward-compat:
accepts any string for type so future OpenAI additions (json_schema,
etc.) flow through without schema bump.
- `tool_choice`: string form 'auto'/'none'/'required' OR object form
`{type:'function', function:{name:string}}`. All other shapes rejected.
Pre-D27 the validator silently accepted any value; the Anthropic plugin's
`if (irRequest.response_format?.type === 'json_object')` would no-op on
a malformed string payload.
2. docs/adr/0005-cache-cross-provider.md — F10 Amendment 4 (+8):
- Documents the IR-vs-body detection ambiguity in § D2: the ADR text
reads as if detection happens on the IR, but `openai-to-ir.mjs` strips
`cache_control` from messages per ADR 0003's whitelist policy, so
IR-side detection is structurally always empty at v1.0
- Documents the actual v1.0 detection mechanism (server.mjs side-channels
into the raw body)
- Documents the cache key `cache_control` slot's always-null status as
forward-compat (when a future ADR 0003 amendment adds cache_control
to IR, the slot will start carrying meaningful data without schema bump)
- Explicit "no code change" — F10 is docs-only
3. lib/providers/index.mjs — F15 alias map export (+11):
- `getAliasMap()` returns `new Map(_aliasMap)` — defensive copy preventing
caller mutation of the module-private alias map
- JSDoc documents use case + defensive-copy intent
4. server.mjs — F15 /v1/models alias surfacing (+25/-7):
- `handleModels` now emits TWO loops: canonical entries first (preserves
existing client expectations), then alias entries via `getAliasMap()`
- Each alias entry has the same 4 OpenAI-spec fields (id/object/created/
owned_by) — no invented fields per ALIGNMENT Rule 2(b)
- Disabled-provider alias non-leak: `loadedProviders.has(providerName)`
gate skips aliases whose target provider is not currently enabled
- createdTs reused (same per-request timestamp across all entries)
- JSDoc updated to document new ordering + F15 origin
5. test-features.mjs — +269 / +18 new tests:
- F8: 13 tests covering response_format (object/string/non-object/missing-
type) + tool_choice (string variants/object variants/wrong type/no name)
- F15: 5 tests covering canonical-first ordering, alias presence/count,
disabled-provider non-leak (with anthropic-only enabled, mistral and
openai aliases must NOT appear), Rule 2(b) shape conformance
Tests: 358 → 376 (+18). All pass on Node 20.
Reviewer notes (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Critical checks verified:
- F8: all 9+ tool_choice rejection axes traced through code (including
partial-function-object edge cases). The code handles them correctly even
where tests don't exercise (non-blocking gap).
- F10: amendment substantively correct. One wording-precision note: the
amendment says "body only" but the actual code is an OR-disjunction
(hasCacheControl(ir) || extractCacheControlMarkers(body.messages)).
Functionally identical at v1.0 because IR strips markers, so first
disjunct is always false. Forward-compat by construction.
- F15: disabled-provider non-leak verified by manual trace with
anthropic-only enabled — mistral/openai aliases correctly filtered out
by `loadedProviders.has(providerName)` gate. Test 17e explicitly
asserts this.
- F15+D17 round-trip: client GET /v1/models → sees alias entry → POSTs
with alias → getProviderForModel resolves via same _aliasMap → cache
key uses canonical → response works. Both surfaces read the same Map
(SPOT).
- F15+D23 cacheable interaction: cacheable opt-out doesn't suppress
alias surfacing in /v1/models — cacheable is about cache-write behavior
while discovery should still surface enabled providers. Intentional.
Authority:
- ADR 0003 § Optional fields (response_format + tool_choice IR shape)
- OpenAI Chat Completions API spec (the field semantics)
https://platform.openai.com/docs/api-reference/chat/create
- ADR 0005 § D2 + Amendment 4 (F10's own amendment landing here)
- ADR 0002 § Loading model + D17 getProviderForModel SPOT (F15's alias
origin)
- ALIGNMENT.md Rule 2(b) — no invented OpenAI fields
- CC 开发铁律 v1.6 § 10.x — Round-3 Cold Audit caught all 3
Follow-up items (reviewer's non-blocking suggestions, NOT in this PR):
- F8 micro-tests for null/boolean/partial-function inputs (code handles;
test gap only)
- F10 wording precision on OR-disjunction
- Nested-describe wrapper quirk in test-features.mjs (pre-existing
structural issue; D26/D27 describes are children of Suite 16 wrapper).
Cleanup in a future hygiene pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
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
|
cd391b13ba |
chore: D25 — round-2 P3 batch (F5/F6/F7/F9/F10/F11/F13 + D22 follow-up)
cold-audit catch from 2026-05-24
Final round-2 cold-audit cleanup batch. 8 small P3 items, batched per
Iron Rule 11 IDR cleanup-batch convention (precedent: D19 / D20). No
item exceeds ~35 lines; only F9 is a code change, the rest are docs +
registry updates.
Changes (7 files, +140 / -10):
1. ALIGNMENT.md (+8 / -2)
- **F7**: Authority pin rows for anthropic / codex / mistral updated
from "TBD on Phase-1 spawn" to the actual citations cited in each
plugin's header:
· anthropic — @anthropic-ai/claude-code v2.1.89 (OCP fork audit pin)
· openai — https://developers.openai.com/codex/cli/reference + features URL
· mistral — https://docs.mistral.ai/mistral-vibe/terminal/quickstart + config URL
Plugins remain Candidate (per Provider Inventory) — D25 just removes the
`TBD` marker; Enabled transition still requires Phase audit.
- **F6**: New 3rd entry in § One-shot Triggered Audits — "OpenAI Codex ToS
formal pin (trigger: Phase 2 E2E enable for Codex, OR 2026-12-31)".
Closes the cross-reference from ADR 0006 § Decision table.
2. README.md (+4 / -2)
- **F5**: Cache-key bullet (Architecture section) replaced the stale
7-tuple with a link to ADR 0005 § Cache key composition + the
post-D15 11-field tuple inlined.
- **D22 follow-up**: "328-test suite" → "Comprehensive test suite
covering IR, cache, fallback, and integration paths" (version-less
to prevent re-drift on every D-day).
3. docs/adr/0002-plugin-architecture.md (+4 / -2) — **F11**: Amendment 1
wording corrected. The original Authority line + maxSpawnTimeMs
description said "fallback engine's spawn-timeout enforcement loop"
— but the enforcement actually lives in each provider plugin's
`_spawnAndStream` (setTimeout + proc.kill + reject pattern). The
fallback engine merely treats SPAWN_TIMEOUT as a hard trigger per
ADR 0004 § Trigger taxonomy bullet 4. Both wording sites updated.
4. docs/adr/0005-cache-cross-provider.md (+1) — **F13**: Amendment 2
gains a "Note on null-coalescing collisions" paragraph documenting
that the `?? null` serialization treats undefined / null / [] as
equivalent cache keys for array-typed fields. Intentional — both
`tools: []` and `tools` omitted semantically mean "no tools." If a
future provider distinguishes empty-array vs absent, the serialization
needs revision.
5. models-registry.json (+35) — **F10**: 5 candidate entries added for
the providers ALIGNMENT.md names but registry omitted. All five with
`candidate: true`, `models: []`, tier per ALIGNMENT.md inventory:
· grok / kimi → Tier C
· minimax / glm / qwen → Tier B
Closes the release_kit overlay's "Supported Providers from
models-registry.json" claim. alignment.yml KNOWN_PROVIDERS validation
array already includes all 8 names, so registry → workflow validation
continues to pass.
6. server.mjs (+20 / -6) — **F9**: Streaming success path now distinguishes
stop-terminated vs exhausted-without-stop. Pre-D25 code unconditionally
cached after the for-await loop ended, treating any exhaustion as
"completed." Now: only cache if `lastChunk?.type === 'stop'`. If the
generator exhausts without emitting stop (truncation), log
`streaming_no_stop_chunk` warn event and do NOT persist. Mirrors
D16's buffered-path semantics ("response completed successfully (no
truncation, no error mid-stream)") with the simpler skip-write
pattern (streaming path doesn't use getOrCompute → no singleflight
eviction needed). D23's cacheableForFirstHop guard preserved as the
outer condition.
7. test-features.mjs (+78) — F9 test 15e in Suite 15:
- Mock provider whose spawn yields delta chunks then implicit-returns
without stop (provider-injection pattern same as 15d — the real
plugins synthesize a stop on clean proc exit so __setSpawnImpl can't
simulate this case)
- Asserts response succeeds AND second identical request triggers
fresh spawn (proves no caching happened) AND X-OLP-Cache: miss on
both responses
Tests: 348 → 349 (+1 from 15e). All pass on Node 20.
Authority:
- F5 → ADR 0005 § Cache key composition (post-D15 Amendment 2)
- F6 → ALIGNMENT.md self + ADR 0006 self-reference
- F7 → plugin headers (verified during D25 implementation)
- F9 → ADR 0005 § Cache write conditions item 1 + D16 truncation precedent
- F10 → ALIGNMENT.md § Provider Inventory tier classification
- F11 → ADR 0004 § Trigger taxonomy bullet 4 (the actual SPAWN_TIMEOUT
hard-trigger documentation)
- F13 → ADR 0005 Amendment 2 (extends with the null-coalescing note)
- D22 fu → no spec authority; version-less framing prevents future drift
- CC 开发铁律 v1.6 § 10.x — Round-2 Cold Audit caught all 8 items
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Independently verified F7 citations against
plugin headers (anthropic.mjs:5-6, codex.mjs:23/31, mistral.mjs:26/32);
F10 tier classifications against ALIGNMENT.md § Provider Inventory;
F6 cross-reference now self-consistent (ADR 0006 → ALIGNMENT.md);
alignment.yml workflow validation passes; F9 truncation semantics
mirror D16 buffered-path; test 15e correctly uses provider-injection
since real plugin synthesizes stop on clean exit.
Three non-blocking suggestions noted (README cache-key bullet slightly
verbose with both link + inline list; F9 could optionally synthesize
a finish_reason: 'length' stop chunk for client-visible truncation
observability; test 15e single-delta variant could be extended to
multi-delta). None folded in — all genuine polish, not correctness
gaps.
---
**Round-2 cold-audit cleanup complete.** 5 D-days (D21-D25 minus the
already-completed D24) closed all 13 round-2 findings (P2: F1/F2/F3/F4
in D21/D22/D23/D24; P3: F5-F13 distributed across D25 + earlier issues
#2/#3). v0.1 is one cold-audit-round-3 away from being ready to tag.
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
|
e10b7d7cb9 |
docs(adr-0004)+chore: D22 — defer soft triggers to v1.x (round-2 F2)
cold-audit catch from 2026-05-24
Round-2 cold-audit Finding 2 (P2 feature-surface vs data-ingestion drift).
ADR 0004 § Trigger taxonomy documented soft triggers (credit_pool_percent,
daily_request_count, five_hour_window_percent) as a live, configurable
feature category. But `evaluateSoftTriggers` in lib/fallback/engine.mjs
is functionally inert at v0.1:
- buildDefaultChain hardcodes quotaSnapshot: null on every hop
- No call site for provider.quotaStatus() in server.mjs or engine.mjs
(only definitions in the 3 provider plugins, all currently stubs)
- evaluateSoftTriggers correctly short-circuits to false on null snapshot
A user populating routing.soft_triggers in ~/.olp/config.json gets zero
runtime behavior. Round-1 cold audit + D5/D9 diff-review reviewers all
focused on evaluation correctness; nobody traced the production data
path end-to-end.
Owner decision (after considering wire-vs-defer): defer to v1.x. Wiring
quotaStatus() polling requires per-hop async I/O before each spawn
decision, error handling for providers without quota endpoints (all 3
current providers fall in this bucket: claude -p, codex exec --json,
vibe --prompt — none expose quota), a caching layer to avoid re-polling,
and a latency budget for a pre-spawn network call. Implementation cost
high; v0.1 value zero.
Strategy: defer the FEATURE (data ingestion path), not the CODE (evaluation
logic). evaluateSoftTriggers is small, well-tested via unit tests that
inject snapshots directly (test-features.mjs:3617-3665), and
architecturally correct. v1.x reactivation requires only wiring the
data path — evaluation stays untouched.
Changes (3 files, +25 / -1):
1. docs/adr/0004-fallback-engine.md +15 — new Amendment 2 block at top
of doc (after Amendment 1, before § Context, matching D11/D15/D16/
Amendment 1 placement convention):
- Finding (cold-audit round-2 F2 + 3 concrete code-state facts)
- Decision (defer; keep evaluation code inert-but-correct)
- Rationale (3-point cost/value analysis)
- Effect on § Trigger taxonomy (inline deferral note appended; design
prose preserved)
- What v1.x reactivation looks like (3 concrete steps; no rewrite needed)
- Procedural mechanism (CC 开发铁律 v1.6 § 10.x — round-2 caught it)
Plus an inline "📋 Deferred to v1.x (Amendment 2)" sub-bullet in
§ Trigger taxonomy → Soft triggers entry. The 3 threshold descriptions
are preserved verbatim — the architectural design remains the v1.x
intent.
2. lib/fallback/engine.mjs +8 / -1 — comment-only updates on the 2
`quotaSnapshot: null` lines in buildDefaultChain. Each now reads:
```
// quotaSnapshot stays null at v0.1 — soft triggers deferred to v1.x per
// ADR 0004 Amendment 2. evaluateSoftTriggers correctly short-circuits to
// false when quotaSnapshot is null. The polling path is not wired in v0.1.
```
No code logic changed. The previous misleading comment "populated at
runtime if provider.quotaStatus() is called" (which falsely implied a
live wiring) is replaced.
3. README.md +3 — two additions:
- Deferral callout immediately after the routing.soft_triggers config
example block, naming Amendment 2
- Implementation status table row: "Soft trigger data path
(quotaStatus() polling) | 📋 Planned (v1.x) | Evaluation logic
shipped + tested; data ingestion deferred per ADR 0004 Amendment 2"
Tests: 335/335 unchanged — pure docs + comment deferral, no behavior
change. Existing unit tests for evaluateSoftTriggers (which inject
snapshots directly) continue to validate the evaluation correctness
independent of the production ingestion path.
Authority:
- ADR 0004 self-amendment (Amendment 2 in-place)
- ALIGNMENT.md Rule 1 (Cite First) + CLAUDE.md § "Hard requirements"
item 1 — contract status changes require ADR amendment
- 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 all 3 framing claims independently:
(a) buildDefaultChain hardcodes null on both branches — confirmed at
engine.mjs:421 and :444; (b) zero quotaStatus() call sites in production
— `grep -rn "quotaStatus("` found only 3 definitions in provider plugins
+ JSDoc mentions; (c) evaluateSoftTriggers correctly short-circuits at
engine.mjs:139. Reactivation realism check: all 3 v1.x steps map to
existing surfaces (insertion points, hop shape, test fixtures).
Follow-up items (reviewer's non-blocking notes, NOT in this PR):
- README line 154 still reads "328-test suite"; current is 335 — pre-
existing doc-sync drift to pick up in D25 P3 batch
- v1.x ADR 0002 amendment may need to formally define authContext shape
(currently informal in the contract); pre-note as a dependency
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
|
1466d3a082 |
fix(providers): D21 — validateProvider enforces maxSpawnTimeMs contract field (round-2 F1)
cold-audit catch from 2026-05-24
Round-2 cold-audit Finding 1 (P2 contract drift). ADR 0002 Amendment 1
(D11, commit
|
||
|
taodengandClaude Opus 4.7
|
d85a2dcf71 |
docs: D20 — honest forward-reference annotations across README/ADRs (Finding 9)
cold-audit catch from 2026-05-23
Cold-audit Finding 9 (P3 drift): multiple docs referenced files / paths
/ directories that don't exist at the current implementation state. A
reader following AGENTS.md § "Key files to know" to read lib/keys.mjs
would find no such file; following README architecture to dashboard.html
same; ADR 0005 § D1 described a `~/.olp/cache/<key>/<prefix>/<hash>.json`
layout that doesn't exist (current impl is in-memory Map per
lib/cache/store.mjs). D20 doesn't gut the designs — it adds honest
status callouts so the gap is obvious within seconds.
Files changed (8, docs only, +46 / -14):
1. README.md (+32 / -3):
- New H2 section "Implementation status (as of 2026-05-24)" with a
10-row table distinguishing ✅ Shipped vs 📋 Planned, including phase
numbers (Phase 2 / 6 / 7) matching the existing §"Phase plan"
- Inline status annotation on the multi-key auth bullet
(lib/keys.mjs marked planned for Phase 2)
- Inline status on the cache layer bullet (file-backed storage marked
Phase 2 — current is in-memory Map)
- Migration section's Phase 7 placeholder annotated explicitly
2. AGENTS.md (+8 / -3):
- Inline `📋 Planned (Phase N) — not yet authored` markers on
lib/keys.mjs and dashboard.html in the "Key files" bullet list
- Inline marker on setup.mjs reference in the
"Project-specific constraints" section
- New "Implementation status note" paragraph at the end of the Key
files block pointing to README's status table for the full picture
3. ALIGNMENT.md (+6 / -3):
- docs/openai-spec-pin.md references (Authority 2 + audits section)
tightened from "deferred" to "deferred, not yet authored; must be
created before first annual audit (target: v1.0)"
- docs/alignment-audits/ directory annotated as "directory does not
exist yet; it is created when the first audit is conducted"
4. CLAUDE.md (+2):
- release_kit.bootstrap_quirk_policy YAML retains the
scripts/migrate-from-ocp.mjs reference (forward-looking spec
compliance) and adds an inline YAML comment explicitly noting
"is planned (Phase 7), not yet authored. The scripts/ directory
does not currently exist. References here are forward-looking;
do not attempt to run this script."
5-8. ADR amendments (status notes only — no Amendment blocks, since
these are clarifications about implementation state, NOT decision
changes per se):
- ADR 0001 (Consequences/Negative): scripts/migrate-from-ocp.mjs
annotated as Phase 7 planned
- ADR 0003 § Decision (lossy-translation paragraph): inline note that
docs/provider-caveats.md is planned; until it exists, lossy edges
are recorded only in plugin headers. Plus a status mention in
Consequences/Positive
- ADR 0004 Mitigations: docs/provider-caveats.md annotated as planned
- ADR 0005 § Decision (D1 paragraph): the file-backed layout block
reframed from "Cache directory structure:" to "Designed file-backed
layout (target for Phase 2 storage adapter):". Added a status
blockquote explicitly noting "v0.1 implementation in lib/cache/store.mjs
uses an in-memory Map; no files written to ~/.olp/cache/. The
file-backed layout described above is the designed shape; it
transitions in via a Phase 2 storage adapter. Per-key isolation and
singleflight (D4) are live; file persistence is not."
Note on ADR Decision-section edits (corrected from initial writeup):
two ADR amendments DO touch Decision-section text (ADR 0003 lossy-
translation paragraph, ADR 0005 D1 cache layout). Both edits are
conservative — status caveats that preserve the original prose verbatim
(0003) or reframe section headings without changing the path-shape spec
(0005). No design content is gutted. No Amendment block was added
because the decisions themselves aren't changing; these are clarifications
about what's live today vs designed. The discipline boundary here:
inline-status-note ≠ decision-amendment.
Tests: 328/328 unchanged (sanity check; docs-only changes).
7/7 Finding 9 forward references annotated (confirmed by reviewer
running independent `ls` on each path):
- lib/keys.mjs ❌ doesn't exist → annotated
- dashboard.html ❌ doesn't exist → annotated
- docs/provider-caveats.md ❌ → annotated (2 ADR sites + Consequences)
- docs/openai-spec-pin.md ❌ → annotated (ALIGNMENT.md Authority 2)
- docs/alignment-audits/ ❌ → annotated
- scripts/migrate-from-ocp.mjs ❌ → annotated (README + ADR 0001 + CLAUDE.md release_kit)
- setup.mjs ❌ → annotated (AGENTS.md)
Authority:
- README/AGENTS.md/ADR/ALIGNMENT.md/CLAUDE.md self — the doc set is
its own authority for what it documents; D20 brings each statement
into honest agreement with the current implementation
- CC 开发铁律 v1.6 § 10.x — Cold Audit Finding 9
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Verified each Finding 9 path doesn't
exist via independent `ls`; cross-checked phase numbers match the
existing §"Phase plan" listing; ran `npm test` to confirm 328/328
unchanged. Two non-blocking minors:
1. Implementer's initial writeup overstated "ADR decision text NOT
edited" — reality is two Decision-section paragraphs got inline
status caveats. Commit message above is corrected.
2. Reviewer found an additional drift D20 didn't address: ADR 0002 §
Decision filesystem layout lists `vibe.mjs` for the Mistral plugin,
but the shipped file is `mistral.mjs` (the binary is `vibe`, the
plugin file is `mistral`). Different drift class from Finding 9
(file exists, just named differently in the ADR). Filed as
follow-up issue.
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
|
82ff00784d |
feat(server)+fix(obs): D18 — populate /v1/models + standard X-OLP-* on error paths (Findings 10 + 11)
cold-audit catch from 2026-05-23
Cold-audit Findings 10 + 11 (both P3, server + observability). F10:
/v1/models returned {data: []} unconditionally; README claimed it
lists from models-registry.json — a client calling /v1/models to
discover what OLP serves would conclude OLP has no models. F11: error
responses (chain-exhausted, pre-routing 4xx) omitted the standard
5-header set; ADR 0004 § Observability requires every response to
carry X-OLP-{Provider-Used, Model-Used, Fallback-Hops, Cache, Latency-Ms}.
Changes (2 files, +427 / -9):
1. server.mjs handleModels: populate from loaded providers × canonical
model IDs. Each entry has exactly {id, object: 'model', created,
owned_by} — no invented fields (Rule 2(b)). `created` is computed
once per request as Math.floor(Date.now()/1000). Iteration order is
insertion-order of loadedProviders Map × insertion-order of each
provider's models[]. Empty case (no providers enabled) returns
{object: 'list', data: []} via natural empty iteration. Aliases are
NOT included — per D17 SPOT, models[] is canonical-only; the
/v1/models endpoint surfaces canonical IDs only.
2. server.mjs sendError extended with optional 5th arg extraHeaders.
Non-breaking — existing call sites (10 of them) use the default {}.
Three pre-routing call sites inside handleChatCompletions now pass
X-OLP-Latency-Ms (computed at-error-time as Date.now() - startMs):
- 415 Content-Type mismatch
- 400 invalid JSON body
- 400 IR parse error
The 404 (route not found) and 500 (top-level catch) paths are
outside handleChatCompletions and have no startMs — they remain
without latency rather than synthesize a value.
3. server.mjs chain-exhausted error path: replaced minimal-header
block with olpHeaders({...}) call producing the full standard
5-tuple. X-OLP-Fallback-Exhausted is preserved as an additional
flag layered on top. The 5 header values reflect engine state per
ADR 0004 step 4 ("preserve A's identity — return FIRST hop's
provider/model and original error to user"): providerUsed =
chain[0].provider, modelUsed = chain[0].model, fallbackHops =
attempted count, cacheStatus = 'miss'.
4. test-features.mjs Suite 17 — 7 new tests:
- 17a: /v1/models with anthropic enabled → 3 entries, all
owned_by='anthropic', no aliases in response
- 17b: /v1/models with no providers → {object:'list', data:[]}
- 17c: /v1/models entries have only {id, object, created, owned_by}
(no invented fields — Rule 2(b) assertion)
- 17d: /v1/models with all 3 providers → canonical IDs present,
aliases absent (sonnet/devstral not in response)
- 17e: chain-exhausted response has all 5 standard X-OLP-* headers
present + X-OLP-Fallback-Exhausted
- 17f: 2-hop chain both fail → X-OLP-Fallback-Hops: '2' value check
- 17g: pre-routing 400 invalid JSON has X-OLP-Latency-Ms (non-negative
integer); X-OLP-Provider-Used absent (no provider context — honest)
Tests: 317 → 324 (+7). All pass on Node 20.
Pre-commit fold-in (per evidence-first checkpoint #4):
- **D18 reviewer flagged Concern #1**: original inline comment described
providerUsed as "last-attempted provider name." This was wrong —
lib/fallback/engine.mjs returns chain[0].provider on chain-exhausted
(per ADR 0004 step 4 "preserve A's identity"), which is the FIRST
hop, not the last. The "last-attempted" framing came from my own
dispatch brief and the implementer accurately reflected the brief.
Folded in: corrected the comment to honestly describe what the
engine returns. Same class of doc-vs-reality drift as D11 / D16 /
D17 — the cold-audit + diff-review combination is catching these
consistently across the P3 batch.
Authority:
- ADR 0002 § Loading model — `models: string[]` enumeration
- ADR 0004 § Observability headers — the 5-header standard set
https://github.com/dtzp555-max/olp/blob/main/docs/adr/0004-fallback-engine.md
- ADR 0004 step 4 — "preserve A's identity" on chain exhaustion
- OpenAI Chat Completions spec /v1/models response shape
https://platform.openai.com/docs/api-reference/models/list
- ALIGNMENT.md Rule 2(b) — only spec-defined fields in OpenAI responses
- CC 开发铁律 v1.6 § 10.x — Cold Audit Findings 10 + 11
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Verified all 5 X-OLP-* headers fire
on chain-exhausted path; verified Math.floor(Date.now()/1000) computed
once per request (not per-model); verified aliases excluded; verified
404 + 500 paths honestly omit latency rather than synthesize. Caught
the providerUsed-comment drift which was folded in.
Follow-up items (reviewer's non-blocking observations, NOT in this PR):
- 4 other error sites inside handleChatCompletions have startMs
available but don't emit X-OLP-Latency-Ms (503 no_enabled_provider,
503 provider-not-enabled-streaming, 502 streaming post-error, 500
fallback programming error). The "Latency-Ms-when-startMs-is-available"
rule should be uniform; file as 11b or future cleanup
- Tests 17e/17f could strengthen by asserting specific values not just
presence (e.g., x-olp-provider-used === 'anthropic'); already done
for fallback-hops in 17f
- Test 17e/17f setup duplication — extractable helper for cosmetic
cleanup
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
cb86807009 |
refactor(routing): D17 — alias-aware getProviderForModel as routing SPOT (Findings 12 + 13)
cold-audit catch from 2026-05-23
Cold-audit Findings 12 + 13 (both P3, both routing layer). F12: mistral
plugin's models[] included canonical IDs + aliases while anthropic/codex
were canonical-only — inconsistent routing surface (request "sonnet"
returned 503 from anthropic but request "devstral" worked for mistral).
F13: getProviderForModel imported but never called; buildDefaultChain
duplicated the lookup loop — two SPOT-candidate code paths.
Per cold-audit reviewer's option (a): standardize models[] on canonical-
only across all 3 plugins; getProviderForModel becomes alias-aware via
models-registry.json; buildDefaultChain uses getProviderForModel as SPOT.
Changes (4 files, +242 / -50):
1. lib/providers/index.mjs:
- At module load, builds `_aliasMap: Map<aliasString, {providerName,
canonicalModel}>` by walking models-registry.json's providers[*].aliases
- getProviderForModel signature extended: returns `{provider, name,
canonicalModel}` (was `{provider, name}`). The canonicalModel field is
the resolved canonical ID for callers that need to use it downstream
(cache key, observability headers, log events)
- 3-step resolution order: (1) alias map lookup → if hit AND provider
loaded → return with canonicalModel; (2) direct canonical scan → return
with canonicalModel = modelString; (3) null
- "Alias known but provider not loaded" case correctly falls through
to step 2 (which will also miss for an alias string) → returns null
2. lib/providers/mistral.mjs:
- _registryModels stripped to canonical-only: removed the
`...Object.keys(_registryEntry.aliases ?? {})` spread
- mistral.models[] now matches anthropic/codex shape (canonical IDs only)
- Comment block updated to point future readers at getProviderForModel
as the SPOT
3. lib/fallback/engine.mjs:
- buildDefaultChain's inline `for ([name, provider] of loadedProviders)`
scan loop replaced with a single getProviderForModel() call
- Chain hop's `model` field is now `match.canonicalModel` (was `modelString`)
— so downstream consumers receive canonical
- Explicit-chain config path unchanged (kept its pre-existing behavior;
alias resolution in routing.chains config is a known limitation, tracked
as a future improvement)
4. test-features.mjs:
- 17 new tests in `D17 — alias-aware getProviderForModel` describe block:
anthropic alias coverage (sonnet/opus/haiku/claude), openai aliases
(codex/codex-spark/gpt5/gpt5-mini), mistral aliases (devstral/
devstral-2/devstral-small/devstral-small-2), canonical pass-through,
unknown model → null, alias-to-disabled-provider → null,
buildDefaultChain integration with alias resolution
- 3 existing mistral tests rewritten — they were asserting the pre-D17
inconsistent shape (aliases in mistral.models[]). Now assert the
post-D17 invariant (aliases NOT in models[]; routing via
getProviderForModel instead)
Tests: 300 → 317 (+17 new). All pass on Node 20.
Pre-commit fold-in (per evidence-first checkpoint #4 — fold-ins themselves
need second-pass review):
- **D17 reviewer flagged C10**: original engine.mjs comment said
"downstream (cache key, X-OLP-Model-Used, provider.spawn) receives the
canonical ID rather than the alias string". The provider.spawn portion
is incorrect — spawn reads `irRequest.model` (the user's original input),
which is NEVER rewritten to canonical. Each provider CLI accepts its own
aliases natively (claude accepts sonnet/opus/haiku; vibe accepts
devstral/devstral-2; codex accepts its model IDs), so runtime behavior
is fine, but the comment overstated what D17 actually changes.
Folded in: corrected comment to honestly describe the canonical flow
(cache key + X-OLP-Model-Used + logs receive canonical; spawn continues
to receive irRequest.model). Same class of doc-code drift fix as D11's
B1 (false maxConcurrent enforcement claim) and D16's "bypasses
getOrCompute" drift — the discipline is maturing across D-days.
Authority:
- ADR 0002 § Provider contract (`models: string[]` is "models this provider
serves")
https://github.com/dtzp555-max/olp/blob/main/docs/adr/0002-plugin-architecture.md
- models-registry.json — single source of truth for (provider, model)
metadata + alias→canonical mappings per AGENTS.md SPOT policy
- AGENTS.md § Project-specific constraints — "models-registry.json is the
only place to add/edit (provider, model) metadata"
- CC 开发铁律 v1.6 § 10.x — Cold Audit Findings 12 + 13
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Highest-value verification: walked
chain[0].model from buildDefaultChain through all downstream consumers
(computeCacheKey, X-OLP-Model-Used, log events) to confirm canonical
flow; then verified provider.spawn paths in anthropic.mjs:231 and
codex.mjs:248 read irRequest.model (user input), confirming the comment
overstatement (now fixed). Verified no alias-canonical collision exists
in current registry (12 aliases vs 10 canonical IDs, zero intersection).
Verified empty-registry edge case + provider-with-model-not-in-registry
edge case.
Follow-up items (reviewer's non-blocking observations, NOT in this PR):
- server.mjs:32 dead import of getProviderForModel — defer to D19 cleanup
- explicit-chain config path doesn't run alias resolution (routing.chains
in ~/.olp/config.json) — pre-existing limitation, file as future issue
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
bafa6d1991 |
fix(fallback)+docs(adr-0004): D16 — honor "usable chunks streamed" qualifier on SPAWN_FAILED
cold-audit catch from 2026-05-23
Cold-audit Finding 17 (P2 fallback correctness). ADR 0004 § Trigger
taxonomy Hard triggers bullet 3 says "Provider CLI exit code ≠ 0
**with no usable response chunks streamed**" — the qualifier was
not honored. Pre-D16 code in `collectAllChunks` re-raised SPAWN_FAILED
unconditionally, discarding any partial chunks. Concrete failure:
provider yields 1000 chars of completion then exits non-zero (e.g.,
post-stream cleanup error) → chunks dropped, fallback to next provider,
user pays double spawn cost and loses the original provider's output.
Coordinated change across two layers, single commit per the
ADR-with-code pattern (D11 / D15 precedent):
1. docs/adr/0004-fallback-engine.md — Amendment 1 (top of doc, matching
D11/D15 placement convention):
- Documents the "usable chunks streamed" semantics precisely
- Behavior split: chunks.length > 0 + SPAWN_FAILED → synthesize stop
+ return (Case B); chunks.length === 0 + SPAWN_FAILED → re-throw
(Case A, hard trigger fires as before)
- finish_reason='length' rationale (4 reasons documented)
- Streaming-path note: ADR 0004 first-chunk rule already handles the
analogous case for D10's real-streaming branch; D16 applies to
buffered path only
- Cache behavior: write-through `getOrCompute` (preserves D4 singleflight
during truncation event) then evict via `set(ttlMs=0)` (so future
fresh callers re-spawn). `__truncated` non-enumerable marker
travels with the chunks array for follower visibility
2. server.mjs `collectAllChunks` salvage path:
- try/catch around the for-await loop
- On SPAWN_FAILED with chunks.length > 0: synthesize stop chunk
`{type:'stop', finish_reason:'length'}`, log warn event
`spawn_failed_after_usable_chunks`, mark chunks array with
non-enumerable `__truncated`, return (no re-throw)
- On SPAWN_FAILED with chunks.length === 0 OR any other error:
re-throw (preserves existing hard-trigger semantics)
3. server.mjs `executeHopFn` cache eviction:
- After `cacheStore.getOrCompute(...)` returns, check `result.__truncated`
- If truncated: `cacheStore.set(keyId, hopCacheKey, result, 0)` —
ttlMs=0 causes `_isAlive` to treat the entry as expired on next
read (verified in lib/cache/store.mjs)
4. test-features.mjs Suite 13 — 3 new tests:
- Case A regression: SPAWN_FAILED at iter 0 + 2-hop chain → openai
serves, X-OLP-Fallback-Hops: 1 (no behavior change)
- Case B 2-hop: 2 deltas + SPAWN_FAILED → anthropic serves with
synthesized stop, hops=0, finish_reason='length', content
concatenates, openai NOT called
- Case B single-hop: 1 delta + SPAWN_FAILED → HTTP 200 (not 502),
finish_reason='length', partial content visible
Tests: 297 → 300 (+3). All pass on Node 20.
Pre-commit fold-ins (per evidence-first checkpoint #4 — fold-ins
themselves need second-pass review):
- **Error-chunk-in-chunks fold-in (sonnet flagged)**: pre-D12 code
pushed error chunks BEFORE throwing. Post-D16's `chunks.length > 0`
check would incorrectly include an error chunk and trigger Case B
for a path that's actually Case A. Moved the `type === 'error'`
check BEFORE the push, restoring the invariant that the chunks
array contains only delta/stop chunks. Verified all 3 scenarios:
(1) error at iter 0 → throws before push → length=0 → Case A
(2) delta×2 + error at iter 3 → throws before push → length=2 → Case B
with delta×2 + synthesized stop (no error chunk leaks)
(3) delta + non-zero exit from outside loop → length=1 → Case B
- **ADR doc-code drift fold-in (D16 reviewer flagged)**: original
Amendment 1 text said the salvaged result "bypasses
cacheStore.getOrCompute and is returned directly, exactly as the
cache-bypass path does." This was factually wrong — the code
write-throughs via getOrCompute then evicts via ttlMs=0. The drift
was ironic: D16 was about removing doc-code drift in ADR 0004
bullet 3 itself, and the amendment was about to ship fresh drift.
Corrected to accurately describe the write-then-evict pattern and
the rationale (preserving D4 singleflight during truncation events).
Authority:
- ADR 0004 § Trigger taxonomy Hard triggers bullet 3 (the qualifier
this amendment makes load-bearing)
https://github.com/dtzp555-max/olp/blob/main/docs/adr/0004-fallback-engine.md
- ADR 0005 § Cache write conditions item 1 — "response completed
successfully (no truncation, no error mid-stream)"
- OpenAI Chat Completions finish_reason enum (stop|length|tool_calls|
content_filter|function_call|null)
https://platform.openai.com/docs/api-reference/chat/object
- ADR 0004 § Fallback safety — first-chunk rule (already governs the
analogous case in the real-streaming path)
- ALIGNMENT.md Rule 2(c) spirit — ADR amendment + code change land
in same merge (D11 / D15 precedent)
- CC 开发铁律 v1.6 § 10.x — Cold Audit caught this; diff-review
passes focused on first-chunk rule for streaming missed the
buffered path's truncation-vs-fallback decision point
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Folded the ADR doc-code drift minor
before commit. Walked all 3 error-chunk scenarios against actual code
to verify the pre-commit fold-in is correct. Analyzed the eviction
race window (sub-ms post-inflight pre-eviction window where a fresh
caller could hit the truncated cache before eviction lands) and
concluded it's structurally bounded — one-shot leak per truncation
event; subsequent callers re-spawn. Acceptable as v0.1.
Follow-up items (reviewer's non-blocking suggestions, NOT in this PR):
- 4th test asserting second identical request triggers fresh spawn
(defense-in-depth around the eviction; store.mjs ttlMs=0 semantics
are independently established)
- `cacheStore.delete()` API (cleaner than set-with-ttlMs=0 — leaves
no dead entry in the namespace map; future PR)
- `cache_evicted_truncated` log event for dashboard observability
- SPAWN_TIMEOUT salvage parity — same architectural argument as
SPAWN_FAILED (user paid for partial content); deferred as a
separate cold-audit finding for a future D-stage
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
8ae77c3ae3 |
fix(cache)+docs(adr-0005): D15 — expand cache key composition to include max_tokens/top_p/stop/tool_choice
cold-audit catch from 2026-05-23
Cold-audit Finding 7 (P2 cache correctness). ADR 0005 § Cache key
composition (v1.0) listed 7 fields. ADR 0003 § Optional fields added
`max_tokens`, `top_p`, `stop`, `tool_choice` as IR-carried fields
that affect model output. Pre-D15 cache key omitted those 4 —
identical IRs differing only in those fields collided on the same
cache key but produced legitimately different outputs. Concrete
hazard: a request with `max_tokens: 100` could receive a cached
response generated by `max_tokens: 4000` — wrong content (truncated
or unexpectedly extended).
Coordinated change across two layers, single commit per the ADR-with-
code pattern established by D11:
1. docs/adr/0005-cache-cross-provider.md — Amendment 2 (top of doc,
matching D11 Amendment 1 placement convention)
- Documents the 4 missing fields + concrete failure mode
- Expands the v1.0 cache key composition spec
- Documents the `?? null` collapsing semantics (explicit-null
equals absent — consistent with existing temperature/response_format)
- Adds forward-looking note: future IR field additions must be
evaluated for cache-key inclusion at addition time; default is
include unless explicit rationale documents safe omission
2. lib/cache/keys.mjs — append the 4 fields to `keyObj` after the
existing 7 (preserves field ordering; pre-D15 cache entries are
forced misses on first request — schema forward-compatible)
- `max_tokens: ir.max_tokens ?? null`
- `top_p: ir.top_p ?? null`
- `stop: ir.stop ?? null`
- `tool_choice: ir.tool_choice ?? null`
- Updated file-level docblock + function-level JSDoc + @param ir
enumeration to reflect new schema (the @param fix also closes a
pre-existing partial-list issue that omitted cache_control)
3. test-features.mjs — 5 new tests in Suite 9:
- max_tokens differs → different key
- top_p differs → different key
- stop differs → different key
- tool_choice differs → different key (covers string-form
`'auto'` vs `'none'`; object-form coverage left for future
defense-in-depth — slot existence is verified by the string case)
- Both-absent stability check (`?? null` collapsing produces
identical keys for omitted-vs-omitted)
Tests: 292 → 297 (+5). No hash constants pinned in existing tests
(grep verified) so no pre-D15 tests required updating; assertions
were already shape-based (`assert.equal` / `assert.notEqual` on
hash strings, never specific hex values).
Authority:
- ADR 0003 § Optional fields — source of the 4 IR field definitions
https://github.com/dtzp555-max/olp/blob/main/docs/adr/0003-intermediate-representation.md
- ADR 0005's stated invariant: "different output → different cache
entry" — the addition restores compliance with this invariant
- OpenAI /v1/chat/completions spec — confirms the 4 fields affect
output:
https://platform.openai.com/docs/api-reference/chat/create
- ALIGNMENT.md Rule 2(c) spirit — ADR amendment + code change land
in same merge (D11 precedent established this pattern for
Provider-contract changes; D15 applies same pattern for cache-key
schema changes)
- CC 开发铁律 v1.6 § 10.x — Cold Audit caught this; diff-review pass
that approved original ADR 0005 did not cross-reference all ADR
0003 optional fields
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Folded the one minor (`@param ir`
JSDoc stale partial list) before commit — same JSDoc precision logic
that drove D11's B1 fold-in. Two remaining non-blocking suggestions
(tool_choice object-form test coverage; D11-vs-D15 amendment structure
template) tracked as defense-in-depth opportunities, not required for
spec compliance.
Reviewer's highest-value verification: field ordering preserved.
keyObj at lib/cache/keys.mjs:203-217 reads exactly:
`{provider, model, messages, tools, temperature, response_format,
cache_control, max_tokens, top_p, stop, tool_choice}` — existing 7
in unchanged positions, 4 new appended at end. JSON.stringify on
Node 18+ preserves insertion order; SHA-256 hash deterministic
across runs.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
a7085d9718 |
fix(server): D14 — defer res.writeHead until first chunk in real-streaming branch
cold-audit catch from 2026-05-23
Cold-audit Finding 1 (P2 correctness). Pre-D14 the D10 real-streaming
branch called `res.writeHead(200, ...)` unconditionally before any
provider chunk arrived. If `streamPlugin.spawn(...)` threw or yielded
an error chunk before the first content chunk, the catch block found
`firstChunkEmitted === false` AND `res.headersSent === true` (because
writeHead had already fired), so the `!res.headersSent` branch was
dead code and the path fell through to bare `res.end()`. Result:
HTTP 200 + Content-Type: text/event-stream + zero bytes of body + no
`[DONE]` terminator. The client saw a "successful" empty response.
Compare the buffered path: identical upstream failure returns
HTTP 502 + Content-Type: application/json + `{error: {message, type}}`
body via `sendError(res, 502, ...)`. Inconsistent client-visible
outcomes across paths, with the streaming variant being silently lossy.
Changes (server.mjs +14/-7, test-features.mjs +66):
1. Removed the unconditional pre-loop `res.writeHead(200, ...)` block.
Replaced with a D14 explanatory comment block.
2. Added a deferred writeHead inside the for-await loop, just before
the first `res.write`, guarded by `if (!res.headersSent)`:
```
if (!res.headersSent) {
res.writeHead(200, { Content-Type, Cache-Control, Connection,
X-Accel-Buffering, ...streamHeaders });
}
streamedChunks.push(irChunk);
res.write(irChunkToOpenAISSE(...));
firstChunkEmitted = true;
```
After the first iteration the guard becomes a no-op for subsequent
chunks. Order inside the loop body is fully synchronous (no await
between writeHead, write, and the flag assignment), so the
`firstChunkEmitted` and `res.headersSent` predicates become
equivalent in steady state.
3. The catch block was NOT touched. Its existing branching was correct
but the `!res.headersSent → sendError(502)` arm was dead code
pre-D14; post-D14 it becomes live. This makes the streaming path's
pre-first-chunk error behavior identical to the buffered path's.
Tests: 291 → 292 (+1):
- Test 15d "streaming early error → 502 JSON not 200 empty body":
Injects a mock anthropic provider via the `loadedProviders` test
seam; mock's `spawn` is an async generator that throws ProviderError
immediately (no yields). Asserts r.status === 502, Content-Type
includes application/json, body parses to `{error: {message, type}}`
with type === 'provider_error', and message contains the original
error string. This test would FAIL on pre-D14 code (would have seen
200 + empty SSE body).
Scope explicitly excludes:
- Finding #5 (streaming bypasses D4 singleflight) — deferred to D14.5
with ADR 0005 § D4 amendment pinning the tee-vs-buffer-replay design
choice. Verified no inflight map, no singleflight machinery, no
Promise tracking added.
- No ADR change. ADR 0004 § Fallback safety first-chunk rule already
governs the post-first-chunk behavior (truncated `res.end()`, no
fallback, no in-band error); D14 preserves that semantics exactly.
Authority:
- OpenAI Chat Completions streaming spec: a failed completion before
the first byte should be an HTTP error response, not a 200 with empty
body
https://platform.openai.com/docs/api-reference/chat/streaming
- ALIGNMENT.md Rule 2(b): no invented response shapes (200 with empty
body when error occurred is an implicit invention since OpenAI spec
does not define this as a success shape)
- ADR 0004 § Fallback safety — first-chunk rule (preserved unchanged)
https://github.com/dtzp555-max/olp/blob/main/docs/adr/0004-fallback-engine.md
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Reviewer stashed the diff to trace pre-D14
control flow manually and confirmed the defect existed exactly as
described (writeHead unconditional → catch with firstChunkEmitted=false
and res.headersSent=true → bare res.end() → silent 200 + empty body).
Verified order inside loop body is correct, headers passed to writeHead
are unchanged, sendError signature matches, stop-as-first-chunk edge
case handled correctly, no singleflight introduced, hygiene clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
f34b6905eb |
fix(cache): D13 — per-hop cache_control bypass evaluation (ADR 0005 § D2)
cold-audit catch from 2026-05-23
Cold-audit Finding 2 (P2 cache correctness). Pre-D13 server.mjs:325
computed `bypassCache` once per request globally and passed it unchanged
into every chain hop. Per ADR 0005 § D2, the bypass must only fire when
the active hop's provider is Anthropic. Pre-D13 Codex/Mistral hops in
fallback chains wrongly bypassed OLP's response cache whenever the
original body had a cache_control marker, defeating per-model cache.
Changes (server.mjs +54/-12, test-features.mjs +218):
1. Replaced the global `bypassCache` const with:
- `hasCacheControlMarkers` (request-level boolean, computed once)
- `shouldBypassCacheForHop(hopProviderName)` (helper, returns
hasCacheControlMarkers && hopProviderName === 'anthropic')
2. Refactored 4 call sites to use the helper:
- executeHopFn: bypassCacheForThisHop = shouldBypassCacheForHop(hopProvider)
— log now includes provider field for observability
- preCheckHit: gated on shouldBypassCacheForHop(chain[0].provider)
— first-hop scope (correct for the pre-loop peek)
- Real-streaming gate (D10): same bypassCacheForFirstHop — single-hop
streaming so first === serving
- cacheStatus header: shouldBypassCacheForHop(providerUsed) — reports
SERVING hop's bypass status. Semantic improvement: a fallback chain
anthropic→openai where openai serves now correctly reports
`X-OLP-Cache: miss` rather than the pre-D13 global `bypass`
No engine.mjs change required — executeWithFallback already passes
hopProvider as a string into executeHopFn.
No marker-strip step needed — verified that openai-to-ir.mjs's
translateMessage drops cache_control from the IR object (copies only
role/content/name/tool_call_id/tool_calls/function_call). Non-Anthropic
plugins never see the markers. Cache key over the IR is therefore
identical for "same prompt with markers" and "same prompt without
markers" on non-Anthropic hops — caching is safe and beneficial.
Tests: 288 → 291 (+3 in new Suite 9d):
- Test 31: openai + cache_control → X-OLP-Cache: miss (the fix's core
assertion)
- Test 32: anthropic + cache_control → X-OLP-Cache: bypass (regression
guard, preserves Anthropic behavior)
- Test 33: 2-hop chain anthropic→openai, anthropic hard-fails, openai
serves → X-OLP-Cache: miss + X-OLP-Provider-Used: openai
(per-hop correctness in fallback)
Authority:
- ADR 0005 § D2: "If the IR request contains Anthropic cache_control
markers AND the active provider in the current chain hop is Anthropic,
the OLP response cache is bypassed... If the active provider is not
Anthropic, the cache_control markers are stripped from the IR before
provider translation"
https://github.com/dtzp555-max/olp/blob/main/docs/adr/0005-cache-cross-provider.md
- ADR 0004 § Observability headers (X-OLP-Cache semantics for the
serving hop)
https://github.com/dtzp555-max/olp/blob/main/docs/adr/0004-fallback-engine.md
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE.
Key verification: reviewer read openai-to-ir.mjs::translateMessage
end-to-end to verify cache_control markers never propagate into IR
(the load-bearing claim — if markers DID propagate, D13 would have
been incomplete and needed a strip step). Confirmed: markers are
dropped at IR translation, no additional strip needed in D13.
Reviewer also walked 4 scenarios (anthropic-only no markers,
anthropic-only with markers, mixed chain anthropic-fail openai-serves,
mixed chain openai-first) against the post-D13 code; all match the
expected per-hop semantics.
Follow-up tracked separately: ADR 0005 § D2 mentions "logged once per
request at debug level" for non-Anthropic noop case; post-D13 the
log fires only for actual bypass (Anthropic + markers). Reviewer's
non-blocking suggestion to either add the log or amend the ADR will
be tracked as a GitHub issue.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
4b1a9c8808 |
fix(ir): D12 — remove invented top-level error field on OpenAI response shapes
cold-audit catch from 2026-05-23
Cold-audit Finding 3 (P2 ALIGNMENT.md Rule 2(b) violation). The IR→OpenAI
translator was inventing a top-level `error: {message, type}` field on
chat.completion / chat.completion.chunk objects. OpenAI's spec defines
no such field — errors surface via HTTP 4xx/5xx with `{error: {...}}`
body, not as in-band SSE annotations. The pre-D12 code comments
acknowledged Rule 2(b) for `finish_reason` enum values then proceeded
to invent the top-level error field anyway.
Changes (3 files, +25 / -50 net):
1. lib/ir/ir-to-openai.mjs — both invention sites removed:
- irChunkToOpenAISSE: removed `if (irChunk.type === 'error')` branch
that emitted `{...spec fields, error: {message, type: 'provider_error'}}`
- irResponseToOpenAINonStream: removed `else if (chunk.type === 'error')`
branch + dead `errorChunk` local var + the `if (errorChunk)` block
that appended top-level `response.error = {...}`
- Added explanatory comments at both sites citing ALIGNMENT.md Rule 2(b)
+ OpenAI spec URLs so future readers see the rationale
2. server.mjs:581 — burst-replay loop break condition simplified:
- Pre: `if (irChunk.type === 'stop' || irChunk.type === 'error') break;`
- Post: `if (irChunk.type === 'stop') break;`
- Cache writes (server.mjs:456, :470) only append to streamedChunks
after the error-chunk guard fires, so cached chunks structurally
cannot contain type === 'error'. The removed clause was dead.
3. test-features.mjs — one test rewritten (test count unchanged 288 → 288):
- Old test asserted `payload.error.type === 'provider_error'` exists
(i.e., it tested the violation as if it were correct)
- New test asserts `payload.error === undefined` (the correct Rule 2(b)
invariant) and that object stays `chat.completion.chunk` with
finish_reason in the OpenAI enum
IR contract decision (verified by both implementer and reviewer):
- IR keeps `'error'` in IRResponseChunk typedef — providers legitimately
emit error chunks (e.g., anthropic.mjs)
- server.mjs intercepts error chunks BEFORE translation:
· collectAllChunks() throws ProviderError on type === 'error'
· Real-streaming branch throws ProviderError (no first chunk) or
truncates res.end() (after first chunk per ADR 0004 first-chunk rule)
- The translator is downstream of both guards; error chunks structurally
cannot reach it in normal operation
- Therefore the invention sites were dead-code paths handling an
impossible case — safe to remove
- Pass-through behavior on the impossible path: produces an empty SSE
delta `{choices: [{index: 0, delta: {}, finish_reason: null}]}`
(verified manually by reviewer); the error message text does NOT
leak to the wire
288/288 tests pass on Node 20.20.2.
Authority:
- ALIGNMENT.md Rule 2(b) — no invented OpenAI-spec fields
- OpenAI Chat Completions object spec
https://platform.openai.com/docs/api-reference/chat/object
https://platform.openai.com/docs/api-reference/chat/streaming
- ADR 0004 § Fallback safety — first-chunk rule (preserves the
truncation semantics for post-first-chunk errors)
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Verified error chunks never reach translator via
three independent grep passes (collectAllChunks guard, real-streaming
guard, no third caller exists), verified cache cannot contain error
chunks (streamedChunks.push is post-guard), verified Rule 2(b) compliance
by direct read of ALIGNMENT.md line 29 + OpenAI spec citations.
Three non-blocking suggestions (defensive log on impossible path;
defensive guard at server.mjs:587; comment placement nit) explicitly
"Not for D12" per the reviewer — preserved as separate cleanup
opportunities if ever needed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
taodengandClaude Opus 4.7
|
f659e29c09 |
docs(adr-0002): D11 amendment — add maxSpawnTimeMs to Provider contract hints
cold-audit catch from 2026-05-23 Retroactive contract sync addressing cold-audit Finding 4 (P2 governance). D10 (commit |
||
|
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)
|
95f6dbd1e2 |
fix(d9): inject fake auth tokens in Suite 13f before() (CI Linux)
D9 CI failure root cause (logged in run 26332283523):
Suite 13f "Fallback engine — HTTP integration" had 3 tests fail on
Linux Node 20 runners with:
"No Anthropic OAuth token found. Run claude auth login or set
CLAUDE_CODE_OAUTH_TOKEN."
Tests pass on macOS Node 25 local because the host has a real
Anthropic OAuth entry in the macOS keychain. The anthropic plugin's
readAuthArtifact() returns that real token, the mock spawn fires,
fallback chain works.
CI Linux runners have no keychain and no ~/.claude/.credentials.json,
so readAuthArtifact() returns null and the anthropic plugin throws
ProviderError(AUTH_MISSING) BEFORE the mock spawn function gets a
chance to fire. AUTH_MISSING is NOT a hard trigger per ADR 0004
§ Decision § No fallback for client-side errors, so the chain stops
at the first hop. The 502 the client receives reports anthropic's
AUTH_MISSING, not the SPAWN_FAILED the test set up.
Three failing tests:
1. "no fallback config + mock anthropic: POST → 200 + Hops: 0"
(single-hop; anthropic auth failed before mock spawn)
2. "fallback config: anthropic SPAWN_FAILED → openai succeeds → 200
+ Hops: 1" (anthropic AUTH_MISSING stops chain; openai never
tried)
3. "fallback config: both providers SPAWN_FAILED → exhausted +
header" (anthropic AUTH_MISSING; triedProviders.length=1; no
Fallback-Exhausted header emitted)
Fix:
Suite 13f before() now injects fake auth for both anthropic + codex
for the entire suite lifetime, restoring originals in after().
process.env.CLAUDE_CODE_OAUTH_TOKEN = 'fake-anthropic-token-...'
process.env.OPENAI_CODEX_AUTH_PATH = <tempfile with fake codex
accessToken>
Auth artifact injection at suite-level (not per-test) is correct
because every test in Suite 13f needs both anthropic and codex to
pass their auth checks before mock spawn fires. The earlier
per-test codex auth injection at line 3672 was correct in pattern
but anthropic was missed.
Long comment in before() explains the failure mode so future readers
do not regress to assuming "tests pass on my Mac = tests pass on CI."
Verification:
Node 25.8.0 (Mac): 277/277 pass.
Node 20.20.2 (Mac): 277/277 pass — verified via @brew node@20 to
match CI matrix.
hygiene grep: fake tokens are clearly fake-* placeholders; no real
credentials.
Reviewer: this is the codex round-2 fold-in pattern applied to a
testing scenario — environment-specific dependency that worked
locally but not on a clean CI runner. Same evidence-first lesson:
"works on my machine ≠ works on CI." Memory entry
~/.cc-rules/memory/feedback/evidence_first_under_speed_pressure.md
should grow a corollary "tests that depend on local secrets / OS
features must inject fakes at suite level not assume host state."
Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
|
||
|
taodengandClaude Opus 4.7 (noreply@anthropic.com)
|
5960486542 |
feat(phase-1): land fallback engine (D9)
Phase 1 Day 6 — the architectural milestone of Phase 1. OLP is no longer
a single-provider proxy with cache; it is now a multi-provider router
with idempotent-failure-safe fallback. ADR 0004 ratified in v0.1 governance
is fully implemented for D9 scope. Configuration-driven chains activate
when a user populates ~/.olp/config.json routing.chains; at v0.1 default
(empty chains), behaviour is single-hop pass-through identical to D5.
Files:
NEW: lib/fallback/engine.mjs (~470 lines) — Trigger taxonomy, chain
advancement, first-chunk safety, observability annotation.
MOD: server.mjs (+80 lines net) — wires executeWithFallback between IR
construction and provider.spawn. Per-hop cache key isolation
(ADR 0005 § Cross-provider fallback). Test seams added:
__setFallbackConfig, __resetFallbackConfig, __clearCache.
MOD: test-features.mjs (+845 lines, 6 new suites = +55 tests).
Authority citations:
ADR 0004 § Decision § Trigger taxonomy — Hard / Soft / Deterministic
(deferred) / Cost-aware (deferred) implemented exactly per spec.
ADR 0004 § Decision § Fallback safety — first-chunk rule satisfied at
D9 by buffering composition (executeHopFn = collectAllChunks; server
writes to res only after engine returns).
ADR 0004 § Decision § Chain advancement — one-at-a-time iteration;
originalError preserved from FIRST hop (not last) on exhaustion.
ADR 0004 § Decision § No fallback for client-side errors — HTTP 400/
401/403/404/422 stop the chain immediately; AUTH_MISSING also stops
immediately (user-config failure, not provider-quota).
ADR 0004 § Decision § Observability headers — X-OLP-Fallback-Hops set
from engine return value; X-OLP-Fallback-Exhausted lists tried
providers on exhaustion.
ADR 0005 § Cross-provider fallback cache behavior — each fallback hop
computes a fresh cache key with the hop's (provider, model) tuple;
primary's cache entries cannot leak to secondary's hop.
Reviewer chain (Iron Rule 10):
Implementer: sonnet (general-purpose).
Fresh-context reviewer: opus. Verdict APPROVE_WITH_MINOR.
Reviewer ran npm test (277/277 pass), read ADR 0004 + ADR 0005 end-
to-end, verified first-chunk-safety composition, checked all
trigger taxonomy mappings.
Reviewer non-blocking findings folded in this commit:
1. Test label at test-features.mjs:3756 clarified. Original title
"client error (400) → does NOT fall back" was misleading because
the test actually exercises both-fail SPAWN_FAILED → exhausted
path (400-no-fallback semantic is covered at unit level instead).
Renamed + commented to match actual behaviour.
2. triedProviders semantic (includes soft-skipped) documented in
engine.mjs comment near the soft-trigger branch.
3. SOFT_TRIGGER synthetic code documented in engine.mjs as engine-
internal, NOT a member of base.mjs PROVIDER_ERROR_CODES.
Reviewer non-blocking suggestions deferred (open D-later questions):
- Q1 cacheStatus on fallback hops: conservative 'miss', acceptable
per ADR 0005 § Per-model isolation.
- Q2 X-OLP-Fallback-Exhausted only when triedProviders > 1:
acceptable per ADR 0004 spec.
- Q3 soft-trigger quotaSnapshot always null at D9: acceptable per
ADR 0004 § Consequences/Negative graceful-degrade clause. Phase 2
will add quota poll-worker.
- Q4 loadFallbackConfigSync sync-only: acceptable for bounded
startup I/O.
Test count: 222 (D8) → 277 (D9). 6 new test suites cover trigger
taxonomy, engine chain advancement (including the load-bearing
originalError-from-FIRST-hop property), HTTP integration through
real server.mjs, soft trigger pre-fetch path, exhausted-chain
header emission.
Verification:
node --check on all touched files: clean.
npm test on Node 25.8.0: 277/277 pass in 266ms.
Hygiene grep: zero personal-name/path/token hits.
No new external npm deps.
Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
|
||
|
taodengandClaude Opus 4.7 (noreply@anthropic.com)
|
95dc072245 |
feat(phase-1): land Mistral Vibe provider plugin (D8)
Phase 1 Day 5. Mistral Vibe provider plugin lands as Candidate. STATIC_
REGISTRY now length 3 (anthropic + openai + mistral). vibe CLI not
installed on the orchestrator machine and owner has no Le Chat Pro
subscription, so D8 follows the D6 docs-only authority pattern. D-later
verification (once a tester with a vibe install and subscription is
available) will resolve UNPINNED assumptions A4, A6, A7, A8.
Files:
NEW: lib/providers/mistral.mjs (~720 lines) — Mistral Vibe provider.
Spawns `vibe --prompt PROMPT --output streaming` per docs.
Reads MISTRAL_API_KEY env var with ~/.vibe/.env fallback.
Supports VIBE_HOME env override per docs. Mirrors D4/D6 plugin
structure incl. __setSpawnImpl/__resetSpawnImpl for tests.
MOD: lib/providers/index.mjs (+12/-1) — STATIC_REGISTRY now length 3.
listAllProviderNames returns [anthropic, openai, mistral].
MOD: models-registry.json (+26 lines) — providers.mistral with 2
canonical date-stamped IDs (devstral-2-25-12, devstral-small-2-
25-12) and 4 short-form aliases for user convenience.
MOD: test-features.mjs — Suite 12 with 48 tests covering contract
conformance, IR translation, mock-spawn behaviour, healthCheck,
estimateCost, auth-artifact reading.
Authority citations (all WebFetched and verified during reviewer pass):
DOCS-1: docs.mistral.ai/mistral-vibe/terminal/quickstart — `vibe
--prompt PROMPT --max-turns 5 --max-price 1.0 --output json` example
+ Output Format Options enumeration: text default, json single blob,
streaming NDJSON.
DOCS-2: docs.mistral.ai/mistral-vibe/terminal/configuration — pin
`~/.vibe/.env`, MISTRAL_API_KEY env var, VIBE_HOME override.
DOCS-3: docs.mistral.ai/mistral-vibe/introduction/configuration —
second confirmation of MISTRAL_API_KEY + ~/.vibe/.env (model
selection via config.toml `/config` slash command, NOT --model
flag).
DOCS-4: deepwiki.com/mistralai/mistral-vibe/9.3-cli-commands-reference
— full flag enumeration confirms no --model flag exists. Programmatic
mode trigger is --prompt; flags are: --continue, --max-price,
--max-turns, --output, --prompt, --resume, --setup, --trust,
--upgrade, --version, --workdir, --no-autofill, --no-header,
--no-dev, --enabled-tools, --help.
DOCS-5: mistral.ai/news/devstral-2-vibe-cli — launch announcement,
names Devstral 2 (123B) and Devstral Small 2 (24B), 256K context,
pricing $0.40/$2.00 and $0.10/$0.30 per MTok.
DOCS-6: help.mistral.ai/en/articles/347532 — Vibe included in Le Chat
Pro.
DOCS-7: legal.mistral.ai/terms/usage-policy — no anti-third-party
clauses; ADR 0006 Tier D classification holds.
DOCS-MAIN: docs.mistral.ai/mistral-vibe/overview — main Vibe overview.
DOCS-8: docs.mistral.ai/getting-started/models/models_overview —
canonical Mistral models registry. Pin for date-stamped IDs
devstral-2-25-12 / devstral-small-2-25-12. Caught by D8 review-2.
Architectural decisions:
1. `--output streaming` (NOT `json`). Per DOCS-1 verbatim: streaming
emits NDJSON per message; json emits a single blob at the end.
Original D8 draft used `json` which is incompatible with the
plugin's line-buffered stdout parser. Review-2 caught this; fixed
before commit.
2. No --model flag. Per DOCS-4 full flag enumeration there is no
--model flag in programmatic mode. Model selection happens via
~/.vibe/config.toml. The IR's `model` field is used by OLP for
routing only; Vibe uses whatever model is in user-level config.
Documented in lossy translations + as A5 CONFIRMED-NOT-APPLICABLE.
3. Canonical IDs primary, short forms as aliases. models-registry.json
uses devstral-2-25-12 / devstral-small-2-25-12 as primary `id`s
matching the canonical Mistral models registry; user-facing short
forms (devstral-2, devstral-small-2, devstral, devstral-small) are
aliases. Plugin's models[] array includes both canonical IDs AND
alias keys so getProviderForModel routes either form. Same pattern
codified for Codex aliases in D6.
4. Auth precedence: MISTRAL_API_KEY env > ~/.vibe/.env > null.
Documented in DOCS-2. readAuthArtifact supports
MISTRAL_VIBE_AUTH_PATH env override for testing.
5. Mistral stays Candidate. STATIC_REGISTRY.length === 3 but
loadProviders({}) returns empty Map; only loadProviders({
enabled: { mistral: true }}) loads it. POST /v1/chat/completions
devstral-* still returns 503 until config flag is set and E2E
audit passes.
Reviewer chain (Iron Rule 10):
Implementer: sonnet (general-purpose).
Fresh-context reviewer: opus (ecc:code-reviewer). Verdict round-1:
REQUEST_CHANGES (2 blockers caught).
Reviewer ran npm test (222/222 pass), WebFetched all 8 canonical
Mistral docs URLs, discovered DOCS-8 (models registry) which
sonnet missed — exactly the D6 failure pattern, repeated. Reviewer
independently verified `--output streaming` vs `json` semantics
against the live docs page text, not paraphrases.
Reviewer blocking findings folded in this commit:
B-1 (--output json wrong): Plugin now passes `--output streaming` per
docs verbatim. Test "irToMistral: user message → args with --prompt
and --output streaming" updated to assert the new arg and reject
the old one.
B-2 (canonical models page missed): models-registry.json refactored
to use date-stamped canonical IDs as primary; short forms as
aliases. mistral.mjs header adds DOCS-8 as the new canonical
authority pin for model IDs. Plugin's models[] array merges
canonical + alias keys so existing routing tests pass with either
form.
B-3 (404 claim incorrect): mistral.mjs header DOCS-MAIN updated.
docs.mistral.ai/mistral-vibe/overview is 200 OK; sonnet's 404
claim was a path-normalization mismatch.
Reviewer non-blocking suggestions (deferred to D-later / not D8 scope):
- VIBE_HOME env override has no Suite 12 test (implementation
present at mistral.mjs:262). Parallel gap to D6 CODEX_HOME.
- _extractKeyFromDotenv has no direct unit test.
- config.toml model selection mechanism (Vibe-specific quirk —
no --model flag means OLP can't pass model per request). A D-later
ADR note will discuss whether OLP should write a project-local
./.vibe/config.toml before spawn or accept the user-level config
as authoritative.
Test count: 174 (after D6) → 222 (after D8).
Verification:
node --check on all touched files: clean.
npm test on Node 25.8.0: 222/222 pass in 210ms.
STATIC_REGISTRY = [anthropic, openai, mistral] verified.
hygiene grep: zero personal-name/path/token hits. Fixtures use
<fake-mistral-api-key> placeholders.
Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
|
||
|
taodengandClaude Opus 4.7 (noreply@anthropic.com)
|
ea9184d2e8 |
feat(phase-1): land OpenAI Codex provider plugin (D6)
Phase 1 Day 4. OpenAI Codex provider plugin code lands as Candidate.
Anthropic and Codex now both in STATIC_REGISTRY length 2. Codex CLI is
NOT installed on the orchestrator machine, so D6 ships with a docs-only
authority pin; D7 will install the binary, probe real behaviour, and
fix any docs-vs-reality divergences (A3 access-token field, A4 NDJSON
event schema, possibly keyring storage support).
Files:
NEW: lib/providers/codex.mjs (586 lines initially, expanded ~10 lines
via reviewer fold-ins) — Codex provider implementing the v1.0
contract. spawns `codex exec --json --model <id> [PROMPT|-]` per
canonical Codex docs.
MOD: lib/providers/index.mjs — STATIC_REGISTRY now [anthropic, codex],
listAllProviderNames() returns 2 entries.
MOD: models-registry.json — providers.openai populated with five
documented model IDs (gpt-5.5, gpt-5.4, gpt-5.4-mini,
gpt-5.3-codex, gpt-5.3-codex-spark) and four aliases.
MOD: test-features.mjs — Suite 11 added with 46 tests covering contract
conformance, IR translation, mock-spawn behaviour, healthCheck,
estimateCost, registry length.
Authority citations (all WebFetched and verified during reviewer pass):
CLI reference: https://developers.openai.com/codex/cli/reference
Source for `codex exec` subcommand syntax, --json flag, --model -m
flag, and PROMPT positional including the `-` form for stdin piping.
Features: https://developers.openai.com/codex/cli/features
Reference for the supported-models list.
Auth: https://developers.openai.com/codex/auth/
Canonical pin for `~/.codex/auth.json` plaintext credential file
and `cli_auth_credentials_store = keyring` OS credential store option.
Models: https://developers.openai.com/codex/models
Canonical pin for the five documented model IDs (each shown as a
`codex -m <id>` example on the page).
ChatGPT plan: https://help.openai.com/en/articles/11369540 — Codex
runs against ChatGPT subscription budget when OAuth-authenticated;
OPENAI_API_KEY env path is for `codex login --with-api-key` only,
not `codex exec` runtime.
Architectural decisions:
1. Mirror D4 anthropic.mjs structure: file header, lossy translation
docs, default export = provider object, named exports include
__setSpawnImpl / __resetSpawnImpl for test injection.
2. Stdin path uses `args.push('-')` per documented CLI behaviour.
(Original D6 sonnet draft omitted the positional entirely and wrote
stdin directly — D6 reviewer pass 2 caught this; corrected before
commit. D7 E2E confirms.)
3. Auth artifact path `~/.codex/auth.json` is now documented in the
header as CONFIRMED per canonical auth doc, not assumed.
4. Access-token field name remains a defensive 3-name try-order
(access_token / token / accessToken) because the auth doc does
not enumerate field names. D7 captures real auth.json post-login.
5. OPENAI_API_KEY env injection during spawn is intentionally NOT
done. The auth doc clarifies OPENAI_API_KEY is a login-time input,
not a runtime override. codex exec reads its own auth artifact.
6. Codex stays Candidate. loadProviders({}) returns empty Map; only
loadProviders({ enabled: { openai: true } }) loads it. POST /v1/
chat/completions gpt-5.5 etc still returns 503 until config flag
is set + E2E audit passes.
Reviewer chain (Iron Rule 10):
Implementer: sonnet (general-purpose).
Fresh-context reviewer: opus (ecc:code-reviewer). Verdict
APPROVE_WITH_MINOR.
Reviewer ran npm test (174/174 pass with Suite 10 skipped), WebFetched
all four canonical Codex docs URLs, and discovered two additional
docs pages (auth + models) that sonnet had missed. Reviewer
independently verified the documentation citations rather than
trusting sonnet quotes — Rule 2 (No Invention) is the load-bearing
check for D6 because no local binary exists to ground-truth the
plugin.
Reviewer non-blocking findings folded in this commit:
1. Stdin path corrected: docs explicitly state `Use - to pipe the
prompt from stdin`. The original draft assumed "no positional →
stdin"; docs require literal `-`. Fixed in irToCodex; test
"irToCodex: multiline prompt uses stdin path (useStdin=true) with
- positional" updated to assert args.includes('-').
2. Model registry expanded from 3 to 5 entries per canonical models
doc. Added gpt-5.4-mini and gpt-5.3-codex-spark. Removed the
misread Rule 2 comment that justified omitting -spark suffix —
the docs literally show `codex -m gpt-5.3-codex-spark`, so the
-spark variant is a separate model not a -codex normalization.
New aliases: codex-spark, gpt5-mini.
3. File header A2 upgraded from "assumed" to "CONFIRMED" with the
canonical auth doc URL cited.
4. File header now cites both auth and models canonical URLs at the
top, alongside reference and features.
Reviewer findings deferred to D7:
- OS credential store / keyring support. Codex docs mention
cli_auth_credentials_store = keyring as an alternative to file
storage. The Anthropic plugin supports macOS keychain via security
find-generic-password; Codex equivalent unknown without inspecting
a real install. D7 will install codex, run codex login, see what
keyring entry codex creates (if any), and mirror the Anthropic
keychain support pattern.
- Real NDJSON event schema (field names). Defensive 4-shape parser
handles the most common conventions; D7 captures real stdout and
pins the schema.
- access_token field name in auth.json. D7 captures the real auth
artifact and removes unused fallback names.
Test count: 128 (after D5) → 174 (after D6).
Verification:
node --check on all touched files: clean.
npm test on Node 25.8.0: 174/174 pass in 210ms with Suite 10 skipped.
Test "codex.models contains all 5 docs-listed model IDs" passes.
Test "irToCodex: multiline prompt uses stdin path with - positional"
passes (asserts args.includes('-')).
Hygiene grep: zero personal-name/path/token hits.
Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
|