fix+test+docs: D53 — tried_providers schema semantic fix (D45 P2 deferral closed) (#30)

Sixth Phase 3 D-day. Small focused fix for the D45 fresh-context opus
reviewer P2 finding that was deferred: auditCtx.tried_providers on the
key_no_provider_access 403 path was being stamped with the ORIGINAL
chain (which was filtered out, never dispatched), distorting downstream
audit queries like "which providers did key X actually call".

CHANGES:

  - server.mjs handleChatCompletions ~L815: on key_no_provider_access
    403, auditCtx.tried_providers = [] (was _originalChainProviders).
    The configured-but-blocked chain still appears in the human-
    readable error message body — the audit just doesn't claim those
    providers were "tried" when the server's filter dispatched zero.

  - docs/adr/0007-multi-key-auth.md § 8 amendment: new paragraph
    spelling out the tried_providers semantic. "The list of providers
    the server actually dispatched a spawn against. A provider that
    was configured in the chain but filtered out by providers_enabled
    gating is NOT included — the key didn't try the provider, the
    gate did. On the 403 path tried_providers is the empty array."
    Plus a forward note that audit log rotation moved to Phase 3 /
    ADR 0008 § 5.

  - test-features.mjs Suite 20h-extra-audit (+1 test — 600 → 601):
    creates guest key with providers_enabled: ['mistral']; fires
    request for Anthropic-routed model; asserts 403
    key_no_provider_access; reads audit row from audit.ndjson;
    asserts tried_providers === []. Pins the D53 semantic against
    regression.

  - CHANGELOG.md: D53 entry under Unreleased.

NOT IN D53:

  - E2E + docs polish (D54)
  - Phase 3 close → v0.3.0 (D55; maintainer-triggered)

Test count: 600 → 601 (+1). Verified locally via npm test.

AUTHORITY:

  - ADR 0007 § 8 amendment (D53, 2026-05-25).
  - D45 fresh-context opus reviewer P2 deferral note.
  - CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased.
  - Standing autopilot grant.

ALIGNMENT.md scope check: small entry-surface change (audit context
field assignment on one error path) + ADR amendment + new test. Per
ALIGNMENT.md Rule 1 the ADR amendment is the authority citation for
the server change. No provider plugin / IR / models-registry change.

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
dtzp555-max
2026-05-25 16:32:59 +10:00
committed by GitHub
co-authored by taodeng Claude Opus 4.7
parent 408d5a839a
commit 68e50da68a
4 changed files with 52 additions and 2 deletions
+11
View File
@@ -4,6 +4,17 @@ All notable changes to OLP land here. Per `CLAUDE.md` release_kit overlay, this
## Unreleased
### D53 — `tried_providers` schema semantic fix (D45 P2 deferral closed)
Sixth Phase 3 D-day. Small focused fix for the D45 fresh-context opus reviewer P2 finding that was deferred: `auditCtx.tried_providers` on the `key_no_provider_access` 403 path was being stamped with the ORIGINAL chain (which was filtered out, never dispatched), distorting downstream audit queries like "which providers did key X actually call".
- **`server.mjs` 403 path fix** (around L815): `auditCtx.tried_providers = []` (was `_originalChainProviders`). The configured-but-blocked chain still appears in the human-readable error message body — the audit just doesn't claim those providers were "tried" when the server's filter dispatched zero.
- **ADR 0007 § 8 amendment**: new paragraph spelling out the `tried_providers` semantic — "the list of providers the server actually dispatched a spawn against. A provider that was configured in the chain but filtered out by `providers_enabled` gating is NOT included — the key didn't try the provider, the gate did. On the 403 path `tried_providers` is the empty array." Plus a forward note that audit log rotation moved to Phase 3 / ADR 0008 § 5.
- **Suite 20h-extra-audit (+1 test — 600 → 601):** creates a guest key with `providers_enabled: ['mistral']`; fires a request for an Anthropic-routed model; asserts 403 `key_no_provider_access`; reads the audit row from `audit.ndjson`; asserts `tried_providers === []`. This pins the D53 semantic against regression — if a future change reverts to stamping the original chain, the test fails.
- **Documentation:** CHANGELOG D53 entry; ADR 0007 § 8 amendment.
- **Test count:** 600 → 601 (+1 D53 regression test).
- **Authority:** ADR 0007 § 8 amendment (D53, 2026-05-25); D45 fresh-context opus reviewer P2 deferral note; CLAUDE.md `release_kit overlay phase_rolling_mode` — under Unreleased; standing autopilot grant.
### D52 — Daily audit rotation (`lib/audit.mjs` extension + `bin/olp-audit-rotate.mjs`)
Fifth Phase 3 D-day. Adds daily UTC-aware rotation to `lib/audit.mjs` per ADR 0008 § 5 + ships an external cron tool. Rotation is **synchronous** at v0.3.0 (Lane 3 = B daily rotation; synchronous design eliminates the race that an async wrapper would create between date-change-detection and the append).
+3 -1
View File
@@ -263,7 +263,9 @@ Field origin:
**No PII.** Audit deliberately captures NO request body, NO response body, NO IR-message content. Hash + shape only. This is a personal/family deployment property; do not relax without a separate ADR amendment.
**Rotation.** Phase 2 does NOT rotate `audit.ndjson`. Rotation policy lands in Phase 3 alongside Dashboard / audit query work (§ 12).
**`tried_providers` semantics (clarification, D53 / 2026-05-25).** The field captures the list of providers the server **actually dispatched a spawn against** for this request. A provider that was configured in the chain but filtered out by `providers_enabled` gating (resulting in 403 `key_no_provider_access`) is NOT included — the key didn't try the provider, the gate did. On the 403 path `tried_providers` is the empty array. The configured-but-blocked chain providers appear in the human-readable error message returned to the client but are intentionally NOT surfaced in the audit event, so downstream queries like "which providers did key X actually call" stay accurate. This semantic was implicit in the D45 implementation (where the field was set to the original chain on 403, misrepresenting "tried"); D53 corrects the implementation + amends this section to spell out the intent.
**Rotation.** Phase 2 does NOT rotate `audit.ndjson`. Rotation policy ships in Phase 3 — daily rotation via `lib/audit.mjs` `_maybeRotateAudit` synchronous trigger on first append after UTC date change + optional `bin/olp-audit-rotate.mjs` external cron. See ADR 0008 § 5.
---
+9 -1
View File
@@ -813,7 +813,15 @@ async function handleChatCompletions(req, res) {
chain = chain.filter(hop => isProviderEnabled(olpIdentity, hop.provider));
if (chain.length === 0) {
auditCtx.error_code = 'key_no_provider_access';
auditCtx.tried_providers = _originalChainProviders;
// D53 (D45 P2 deferral fix): tried_providers semantic per ADR 0007 § 8 is
// "providers the server actually dispatched a spawn against". On 403 the
// filtered chain is empty — no provider was dispatched, so the audit
// value is []. The configured-but-blocked chain providers go into the
// human-readable error message + a separate _diagnostic field we don't
// surface in audit (would distort downstream queries like "which
// providers did key X actually call"). ADR 0007 § 8 amended in this
// D-day's CHANGELOG entry to spell the semantic.
auditCtx.tried_providers = [];
const allowed = olpIdentity.providers_enabled === '*' ? '*' : (olpIdentity.providers_enabled ?? []).join(', ') || '(none)';
return sendError(
res, 403,
+29
View File
@@ -10146,6 +10146,35 @@ describe('Suite 20 — server.mjs auth integration (D45, ADR 0007)', () => {
});
assert.equal(r.status, 200);
});
it('20h-extra-audit: D53 — key_no_provider_access 403 audit row has tried_providers = []', async () => {
// Per ADR 0007 § 8 tried_providers semantics clarification: the field
// captures providers the server actually dispatched. On 403 (filter
// rejected the whole chain) the server dispatched zero providers, so
// tried_providers MUST be []. The configured-but-blocked chain only
// appears in the human-readable error message, not in the audit.
// D45 P2 deferral fix — was previously stamping the original chain
// which distorted "which providers did key X actually call" queries.
const { id, plaintext_token } = createKey({ name: '20h-extra-audit', owner_tier: 'guest', providers_enabled: ['mistral'], olpHome: TMP });
const r = await fetch({
port, method: 'POST', path: '/v1/chat/completions',
headers: { Authorization: `Bearer ${plaintext_token}` },
body: { model: 'claude-sonnet-4-6', messages: [{ role: 'user', content: '20h-extra-audit' }] },
});
assert.equal(r.status, 403);
assert.equal(JSON.parse(r.body).error.type, 'key_no_provider_access');
// Wait for the res.on('finish') audit append
await new Promise(resolve => setTimeout(resolve, 25));
const auditPath = _pathJoinForSetup(TMP, 'logs', 'audit.ndjson');
const lines = fsReadFileSync(auditPath, 'utf-8').trim().split('\n').filter(Boolean);
const row = lines.map(l => JSON.parse(l)).find(r =>
r.path === '/v1/chat/completions' && r.status_code === 403 && r.key_id === id,
);
assert.ok(row, '403 audit row must be present');
assert.equal(row.error_code, 'key_no_provider_access');
assert.deepEqual(row.tried_providers, [],
'tried_providers MUST be [] on key_no_provider_access — D53 semantic fix per ADR 0007 § 8 clarification');
});
});
// ── 20i: per-key cache namespace isolation (criterion #1) ────────────────