From d2493a147d5286f54be34735a13f5d13f8f19597 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 05:23:05 +0900 Subject: [PATCH] fix(providers): allow a baseUrl override for Anthropic and Antigravity Both providers were pinned to their registry endpoint, so a user behind a corporate gateway or self-hosted relay could not point them anywhere else. Opt both into allowBaseUrlOverride, and require https for the destination because these are the two OAuth entries in that set: the override receives a subscription bearer. Carries @drakonkat's #2109 and #2110, combined because they share one destination-policy change and a single registry parity list. Merging them separately conflicts on that list; neither depends on the other's runtime. Also closes the gap review raised on both PRs. providerSecureTransportConfigError returned before classifying the URL whenever allowPrivateNetwork was set, so http://attacker.example with that opt-in passed the gate and adapters attached the bearer in cleartext to a public host. allowPrivateNetwork states that a destination is intentionally local; it is not a waiver of transport security. The address is now classified first, and the opt-in only excuses http for a destination that really is loopback or private. Closes #2109 Closes #2110 --- .../000_research_inventory.md | 289 ++++++++++++++++++ .../010_layer1_bearer_admission_2132.md | 70 +++++ .../020_layer2_responses_id_backfill_2131.md | 43 +++ .../030_sibling_prompt_cache_retention.md | 38 +++ .../040_sibling_routing_capability.md | 31 ++ .../050_sibling_k12_short_window.md | 25 ++ .../060_supersede_and_close_operations.md | 37 +++ .../070_execution_log.md | 180 +++++++++++ src/lib/destination-policy.ts | 47 +++ src/providers/registry.ts | 3 +- tests/anthropic-baseurl-override.test.ts | 144 +++++++++ tests/antigravity-baseurl-override.test.ts | 117 +++++++ .../provider-model-discovery-contract.test.ts | 13 +- tests/provider-registry-parity.test.ts | 5 +- .../router-discarded-baseurl-warning.test.ts | 15 +- tests/router-template-baseurl.test.ts | 3 +- 16 files changed, 1044 insertions(+), 16 deletions(-) create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/020_layer2_responses_id_backfill_2131.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/030_sibling_prompt_cache_retention.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/040_sibling_routing_capability.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/050_sibling_k12_short_window.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/060_supersede_and_close_operations.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md create mode 100644 tests/anthropic-baseurl-override.test.ts create mode 100644 tests/antigravity-baseurl-override.test.ts diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md new file mode 100644 index 0000000000..1b68a3fe29 --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md @@ -0,0 +1,289 @@ +# 000 — Research: open bug-PR backlog inventory, rubric, and disposition + +Unit: 260820_bug_pr_backlog_consolidation +Work-phase: wp1 (docs-only roadmap cycle, LOOP-DOCS-FIRST-01) +Baseline: origin/dev = ceac592d7. Worktree branch codex/fix-subagent-roster-truncation (PR #2134). + +Evidence for every claim below came from six read-only xai/grok-4.6 investigation lanes that +read the actual PR diffs with `gh pr diff` and cross-read the runtime in this worktree. Code +edits stay in the main agent. + +## 1. Inventory + +27 open bug-labeled PRs; 25 authored by someone other than lidge-jun. 17 open bug issues. + +| PR | Author | Draft | Subsystem | Files | +|---|---|---|---|---| +| 2131 | bet4it | no | responses id backfill | server/responses | +| 2127 | agentHits | yes | antigravity thought_signature | adapters/google | +| 2115 | louis-tepe | no | adapter prompt nudge | adapters/* | +| 2110 | drakonkat | no | antigravity baseUrl override | providers/registry, lib/destination-policy | +| 2109 | drakonkat | no | anthropic baseUrl override | providers/registry, lib/destination-policy | +| 2105 | lilinxiong | no | claude shell hook | cli/index, server/system-env | +| 2104 | olddonkey | no | xai OAuth responses streaming | adapters/xai | +| 2102 | lilinxiong | no | gpt-5.6 prompt_cache_retention | adapters/openai-responses | +| 2101 | Ingwannu | no | account entitlement gating | codex/catalog | +| 2100 | ntdatt812 | no | routing capability evidence | routing/capability | +| 2099 | yzxcj797 | yes | gpt-5.6 prompt_cache_retention | adapters/openai-responses | +| 2091 | luvs01 | no | prompt_cache_retention (all forward) | adapters/openai-responses | +| 2082 | yzxcj797 | yes | AgentRouter language preamble | adapters | +| 2077 | ntdatt812 | no | lab behavior overrides | routing/compatibility/behavior | +| 2075 | olddonkey | no | Fast gate native chat (CONFLICTING) | adapters/openai-chat | +| 2067 | waw4303 | yes | opencode-free headers | providers/registry | +| 2063 | yzxcj797 | yes | K12 detail.code denials (CONFLICTING) | codex/quota-rejection | +| 2062 | yzxcj797 | yes | K12 short-window quota | codex/quota, codex/routing | +| 2056 | Ingwannu | no | K12 short-window quota | codex/quota, codex/routing | +| 2054 | keepitmello | yes | cursor checkpoints (CONFLICTING) | adapters/cursor | +| 2053 | Ingwannu | no | superseded OAuth commits | oauth/* | +| 2040 | Ingwannu | no | routed tool_search passthrough | server/responses | +| 2032 | yzxcj797 | yes | claude root bypass | cli/claude | +| 2029 | yzxcj797 | yes | probe session bus absent | service-manager-probe | +| 2027 | yzxcj797 | yes | opencode-go quota gating | providers/quota | + +## 2. Scoring rubric + +Score = severity (0-35) + blast radius (0-25) + evidence quality (0-20) + fix tractability (0-20). +Threshold for this campaign: **>= 60**. + +- severity: does it break a core path (routing, auth, streaming, config persistence) for a + default configuration, or is it peripheral/cosmetic? +- blast radius: how many users/configurations does the defect reach? +- evidence quality: deterministic reproduction with logs/curl, or assertion? +- fix tractability: is a correct, testable fix small and self-contained? + +## 3. Scores and disposition + +| Item | Score | Disposition | +|---|---|---| +| Issue #2132 bearer admission forces ChatGPT credential | 96 | ABSORB — no PR exists; highest-value gap in the backlog | +| Issue #2092 / PRs #2102,#2099,#2091 prompt_cache_retention | 86 | ABSORB #2102 as base; supersede #2099, #2091 | +| Issue #2114/#1939 / PR #2029 probe bus | 80 | SUPERSEDED by maintainer PR #2130 (already open) | +| PR #2131 responses output id backfill | 80 | ABSORB | +| PR #2100 routing capability evidence | 80 | ABSORB | +| PR #2047 / #2056 + #2062 K12 short-window quota | 72 | ABSORB #2056; supersede #2062 | +| PR #2053 superseded OAuth credential commits | 72 | KEEP — C4 auth, needs human security review (MAINTAINERS.md) | +| PRs #2109 + #2110 baseUrl override | 68 | HOLD — unresolved security gap, see §6 | +| PR #2101 account entitlement gating | 64 | KEEP — large (20 files), needs its own cycle | +| PR #2077 lab behavior overrides | 62 | ABSORB | +| PR #2040 routed tool_search passthrough | 62 | KEEP — 14 files, own cycle | +| PR #2105 claude shell hook | 60 | ABSORB | +| PR #2063 K12 detail.code | — | SUPERSEDED by already-merged #2055 | +| PR #2115 code mode nudge | 54 | BELOW THRESHOLD — contracts native-OpenAI detection; needs human adapter pass | +| PR #2082 AgentRouter language | 54 | BELOW THRESHOLD | +| PR #2027 opencode-go quota | 56 | BELOW THRESHOLD | +| PR #2067 opencode-free headers | 50 | BELOW THRESHOLD | +| PR #2054 cursor checkpoints | 46 | BELOW THRESHOLD — hypothesis pending wire trace | +| PR #2032 claude root bypass | 46 | BELOW THRESHOLD — maintainer already rejected the default | +| PR #2104, #2075, #2127 | n/a | Deferred: #2075 and #2054 are CONFLICTING; #2127 is an active draft by its author | + +## 4. Duplicate clusters (evidence-backed) + +**prompt_cache_retention (issue #2092).** #2102 gates on `forward && isCanonicalOpenAiForwardProvider` +and matches `gpt-5.6` / `gpt-5.6-*`. #2099 uses a looser `startsWith("gpt-5.6")` on ANY forward +provider and carries a stray package.json 2.24.2 -> 2.25.0 bump. #2091 strips the field for every +forward request and every model, which inverts the existing gpt-5.5 preserve pin at +tests/openai-responses-passthrough.test.ts:807 — the issue reporter explicitly withdrew the +global claim. #2102 is the correct contract. + +**K12 short-window quota (issue #2047).** #2056 is a strict superset of #2062: it adds +`snapshotHasShort`, partial-snapshot preservation, `updateAccountQuota` carry, and the +parse -> cache -> DTO path the issue requires. Both rewrite the same two functions and WOULD +conflict. #2062 also carries the same stray version bump. + +**Probe bus (issues #2114/#1939).** #2130's `busUnreachable()` is a superset of #2029's two +strings and adds the on-disk unit check that #2029's reviewer demanded. Landing #2029 on top of +#2130 would REGRESS the disk check back to unconditional `absent`. + +## 5. Structural finding: this backlog is not one stack + +DEV-STACK-01 permits stacking only when later parts consume earlier parts' output. Measured file +overlap across the absorb set: + +| Cluster | Files | +|---|---| +| PCR consolidation | src/adapters/openai-responses.ts | +| #2132 + #2131 | src/server/responses/core.ts (**shared**) | +| #2100 | src/routing/capability.ts | +| #2077 | src/routing/compatibility/behavior.ts | +| K12 | src/codex/quota.ts, src/codex/routing.ts | +| #2105 | src/cli/index.ts, src/server/system-env.ts | + +Exactly one real dependency edge exists: **#2132 and #2131 both modify +`src/server/responses/core.ts`**, so they must be ordered. Everything else is disjoint. + +Forcing 12 disjoint fixes into one 12-layer chain would violate DEV-STACK-01's independence +clause and the 2-4 depth guidance, and would impose a false merge order in which an unrelated +layer blocks every layer above it. The honest shape is therefore **one bounded stack rooted on +#2134 for the genuinely dependent Responses work, plus sibling PRs off dev for the disjoint +fixes**. That is recorded here rather than silently reshaped. + +## 6. Security holds (detail deliberately not recorded here) + +The baseUrl-override pair (#2109/#2110) has an unresolved gap already raised publicly in the +CodeRabbit thread on those PRs. Per AGENTS.md, pre-disclosure security reasoning does not go in +this public directory: the analysis lives in scratch only, and these PRs are HOLD, not absorb, +until a human security pass. #2053 is C4 OAuth and requires the security review MAINTAINERS.md +mandates; it is KEEP, not absorb. + +## 7. Attribution contract + +Every superseded PR gets (a) its author credited by @login in the superseding PR body, +(b) a courteous closing comment naming the replacement PR and what was carried over, +(c) no force-push and no edit to the contributor's own branch. + + +--- + +# P-phase amendment (A-gate self-audit, 2026-08-20): the stack premise in §5 was WRONG + +The A-phase auditor lane produced nothing across three wait cycles, so it was retired +(DISPATCH-RETIRE-01) and the load-bearing claims were verified directly. Two of them failed. + +## Correction 1 — #2131 does NOT touch `src/server/responses/core.ts` + +`gh pr diff 2131 --name-only` returns `src/server/responses/responses-field-backfill.ts`, +its test, and eight docs locales. `core.ts` already imports that module on `dev` +(`src/server/responses/core.ts:6-7`, called at :3098-3099); #2131 only changes the module's +internals and signature. It never edits `core.ts`. + +#2132's fix lives in `resolveResponsesCodexAuth` (`core.ts:1082-1114`), a different region of +a file #2131 does not modify at all. + +**Therefore the single dependency edge claimed in §5 does not exist.** The corrected file map: + +| Item | Files | Overlap | +|---|---|---| +| #2132 | src/server/responses/core.ts (auth resolution) | none | +| #2131 | src/server/responses/responses-field-backfill.ts | none | +| #2102 | src/adapters/openai-responses.ts | none | +| #2100 | src/routing/capability.ts | none | +| #2077 | src/routing/compatibility/behavior.ts | none | +| #2056 | src/codex/quota.ts, src/codex/routing.ts | none | +| #2105 | src/cli/index.ts, src/server/system-env.ts | none | + +Every absorbed item is disjoint. **There is no dependency-ordered chain in this backlog at all.** + +## Consequence: this work must NOT be stacked + +DEV-STACK-01 forbids stacking independent parts: "the parts are independent — open parallel PRs +off trunk instead, since a stack imposes a false merge order." Building the requested chain +would mean any layer's review blocking every layer above it, for zero dependency benefit, and +would violate the same rule the request asked to follow. + +Docs 010 and 020 are therefore **superseded**: both become siblings based on `dev`, not layers. +PR #2134 remains its own independent PR. The stack rooted on #2134 is cancelled and the reason +is recorded here rather than the plan being quietly reshaped. + +**One exception preserved:** if two absorbed items ever do touch one file, they stack. None do. + +## Correction 2 — issue #2132 is confirmed present, with a sharper mechanism than 010 assumed + +Verified in this worktree: +- `core.ts:1088`: `const substituteMainCredential = options.admission?.source === "bearer";` + keys on HOW the caller authenticated, never on WHERE the request routes. +- `auth-context.ts:542-548`: with `ctx.kind === "main"` and that flag, a missing/dead stored + main token throws `CodexMainSubstitutionUnavailableError`. +- `core.ts:1148-1153`: that becomes the reported 401. +- The `authCtx = { kind: "main" }` fallback at `core.ts:1105` is taken whenever + `route.codexAccountMode` is unset — which is every non-`openai` provider. + +So a key-auth routed provider reaches `kind: "main"` + `substituteMainCredential: true` and +fails, exactly as reported. The defect is real and 010's fix direction stands; only its stack +position changes. + +## Correction 3 — supersede claims re-verified + +`gh pr view 2055`: `MERGED` at 2026-08-19T00:11:27Z, merge commit `2648ffa879edf93e`. #2063's +supersede stands. + +## Revised work-phase map + +| WP | Doc | Branch | Base | Content | +|---|---|---|---|---| +| wp2 | 010 | codex/fix-bearer-admission-2132 | dev | issue #2132 (score 96) | +| wp3 | 030 | codex/consolidate-prompt-cache-retention | dev | absorb #2102; supersede #2099, #2091 | +| wp4 | 040 | codex/absorb-capability-evidence | dev | absorb #2100, #2077 | +| wp5 | 050 | codex/absorb-k12-short-window | dev | absorb #2056; supersede #2062, #2063 | +| wp6 | 020 | codex/absorb-responses-id-backfill | dev | absorb #2131 + unique-id correction | +| wp7 | 060 | — | — | close-outs with attribution | + +Ordered by score, not by dependency, because no dependency exists. Each is independently +reviewable and independently mergeable, which is what DEV-STACK-01 actually asks for. + + +## Correction 4 — #2130 merged mid-cycle; #2029 is now superseded in fact, not in prospect + +`gh pr view 2130`: `MERGED` at 2026-08-19T17:25:26Z. The probe lane's verdict was conditional +("SUPERSEDED, once #2130 merges"); that condition is now satisfied. + +Consequence for `060`: **#2029 (@yzxcj797)** moves from a prospective close to an immediate one. +`dev` now carries `busUnreachable()` — a superset of #2029's two stderr strings — plus the +on-disk unit check that #2029's own reviewer demanded. Landing #2029 on top would REGRESS that +disk check back to an unconditional `absent`. Nothing from #2029 needs to be carried over; its +one unique behavior (keeping `DBUS_SESSION_BUS_ADDRESS not set` as `unknown`) is precisely what +the merged disk check replaces. + +This also removes #2130 from the open-bug-PR set: the fresh count at wp1 close is 26 open bug +PRs, of which exactly one (#2134) is lidge-jun's and 25 are not. + +## wp1 close-out evidence + +`gh pr list --repo lidge-jun/opencodex --state open --label bug --limit 100` at close: +26 total, mine = [2134], non-mine = 25. Every one of those 25 numbers appears in §1/§3 of this +document (verified by a grep loop over the list, exit 0). No open bug PR is left without a +disposition. + + +--- + +# A-gate amendment 2 — retired auditor returned late with VERDICT: FAIL; findings adjudicated + +The adversarial lane retired under DISPATCH-RETIRE-01 (three empty wait cycles) delivered after +retirement. Its verdict is FAIL. It is adjudicated here rather than discarded, because a late +reviewer is still a reviewer. + +**Findings 1 and 2 — CONFIRMED, and already corrected.** It independently measured the same +`gh pr diff --name-only` evidence and reached the same conclusion as amendment 1: #2131 does not +touch `core.ts`, no dependency edge exists, and rooting a stack on #2134 (which only touches +`agent-settings-routes.ts`) is a second DEV-STACK-01 violation. Two independent measurements now +agree. Recorded as settled. + +**Finding 5 — CONFIRMED, and it is the sharpest catch.** Doc 010 said to gate substitution on +"the native ChatGPT **pool**". That would exclude `codexAccountMode: "direct"` and re-break +#1686, whose whole point is that Direct bearer admission is only safe *because* substitution +still runs. The implemented fix uses `route.codexAccountMode !== undefined`, which covers both +`pool` and `direct` and matches the issue reporter's own suggested gate. 010's prose is +superseded by this line; the code is correct. + +**Finding 3 — CONFIRMED and material.** Overlap was measured only inside the absorb set. Three +OTHER open PRs edit `src/server/responses/core.ts`: + +| PR | Overlap with the #2132 fix | +|---|---| +| #2104 (@olddonkey) | `src/server/responses/core.ts` — review-ready, MERGEABLE | +| #2101 (@Ingwannu) | `core.ts` + `compact.ts` + `auth-context.ts` — all three files this fix touches | +| #2040 (@Ingwannu) | `core.ts` | + +Verified by `gh pr diff --name-only`. The #2132 change is 5 lines across two files and does not +restructure either function, so a textual conflict is possible but small. This is a merge-order +hazard to state on the PR, not a reason to withhold the fix. #2104 is reclassified from `n/a` +to KEEP (review-ready, not a conflicting draft — the auditor is right that grouping it with +CONFLICTING #2075 and draft #2127 was an error, and its inventory row's "adapters/xai" file +attribution was wrong). + +**Finding 4 — CONFIRMED. #2105 would have been lost.** It is scored 60 ABSORB in §3, has no +decade doc, and appears in no row of 060. An above-threshold item with no execution path is +exactly how a contributor's work disappears without a close comment. Disposition corrected to +**KEEP — remains open**, because no replacement exists. It is not closed. + +**Nits accepted:** "strict superset" overstates #2056 vs #2062 (#2062 uniquely adds +`tests/rate-limit-reset-credits.test.ts`); #2130 has empty `closingIssuesReferences` so +#1939/#2114/#2108 will not auto-close; the rubric is recorded as a single integer, so the +component arithmetic is not independently auditable. + +## Net effect on the plan + +No absorbed item is dropped and no new one is added. Two dispositions change (#2104 n/a -> KEEP, +#2105 ABSORB -> KEEP), one prose invariant in 010 is superseded by the implemented predicate, +and one merge hazard is now stated. The sibling shape from amendment 1 stands, reinforced. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md new file mode 100644 index 0000000000..991c3bb62a --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md @@ -0,0 +1,70 @@ +> **SUPERSEDED IN PART — read `000_research_inventory.md` amendments 1 and 2 first.** +> +> Two things in this document are wrong and were corrected after it was written: +> +> 1. **It is NOT a stack layer and does NOT root on #2134.** No dependency edge exists; +> the shipped PR (#2137) is based on `dev` as a sibling. +> 2. **The substitution predicate is NOT "native ChatGPT pool".** Pool-only would exclude +> `codexAccountMode: "direct"` and re-break #1686, whose Direct admission is only safe +> BECAUSE substitution still runs. The shipped predicate is +> `route.codexAccountMode !== undefined`, covering pool AND direct. Do not "correct" it back. + + +# 010 — Layer 1 (stack bottom): fix issue #2132, bearer admission must not force a ChatGPT credential + +Work-phase: wp2. Branch: `codex/fix-bearer-admission-2132`. Base: `codex/fix-subagent-roster-truncation` (PR #2134). +Absorbs: nothing (no PR exists). Closes: #2132. + +## Why this is the stack bottom + +It is the highest-scoring item in the backlog (96) and it shares `src/server/responses/core.ts` +with layer 2 (#2131). Layer 2 must be based on this, or the two edits to that file collide. + +## Defect + +Reported in #2132: after v2.23.0, a key-auth provider (Cloudflare/etc.) returns 401 +`No usable Codex main credential` when `~/.codex/auth.json` holds no ChatGPT token. Bearer +admission sets `substituteMainCredential` unconditionally, so a route that needs no ChatGPT +identity is still gated on one. + +## P-phase re-verification required (stale check) + +Before editing, confirm against the CURRENT tree — the lane read `dev`, not this branch: +1. `rg -n "substituteMainCredential" src/` — enumerate every producer and consumer. +2. Read `src/server/responses/core.ts`, `src/server/responses/compact.ts`, + `src/codex/auth-context.ts` and establish where the flag is set and where it is read. +3. Reproduce the admission decision in a unit context with a key-auth provider and an + auth.json containing no ChatGPT token. If the current code does NOT reproduce, stop and + amend this doc rather than writing a fix for a defect that is not there. + +## Intended change + +Make the substitution conditional on the resolved route actually requiring a native/ChatGPT +credential. A key-auth routed provider carries its own credential and must be admitted +without one. Exact call sites are fixed during the stale check above; the invariant is: +`substituteMainCredential` is set only when the route's credential source is the native +ChatGPT pool. + +Out of scope: changing what happens once a native route legitimately lacks a credential, +and any change to the pool/account selection itself. + +## Test plan (must fail RED first) + +New `tests/bearer-admission-key-auth.test.ts`: +1. key-auth routed provider + auth.json with NO ChatGPT token -> request is admitted (no 401). +2. native gpt route + no ChatGPT token -> still fails closed with the existing error. +3. key-auth provider + ChatGPT token present -> unchanged behavior (no regression). + +Drive the file against the unpatched tree first and record the failure output; a test that +passes before the fix does not prove anything. + +## Verification + +`bun run typecheck`; `bun test --isolate` on the new file plus the existing responses/auth +suites; full `bun test --isolate tests` before marking review-ready; `bun run privacy:scan`. + +## Standalone thesis (DEV-STACK-03) + +"A provider that carries its own key must not be gated on a ChatGPT credential." Builds and +passes its own tests at its own tip, independent of layer 2. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/020_layer2_responses_id_backfill_2131.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/020_layer2_responses_id_backfill_2131.md new file mode 100644 index 0000000000..bb5c088378 --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/020_layer2_responses_id_backfill_2131.md @@ -0,0 +1,43 @@ +# 020 — Layer 2: absorb PR #2131, backfill missing Responses output ids + +Work-phase: wp3. Branch: `codex/absorb-responses-id-backfill`. Base: `codex/fix-bearer-admission-2132` (layer 1). +Absorbs: **PR #2131 by @bet4it**. Closes: PR #2131 as superseded, with attribution. + +## Dependency edge (the only real one in this backlog) + +#2131 adds `src/server/responses/responses-field-backfill.ts` and calls it from +`src/server/responses/core.ts` — the same file layer 1 edits. This is why it stacks rather +than sitting beside layer 1. + +## Defect + +Strict decoders (grok-build) reject Responses output items that omit `id` on +`message` / `reasoning` / `function_call`. #1941 landed earlier but some relays still omit it. + +## Change to carry over + +@bet4it's implementation, preserved in substance: synthesize stable `msg_ocx_N` / `rs_ocx_N` / +`fc_ocx_N` ids keyed on `output_index`, never overwriting an id that is already present. + +## Correction to apply on top (audit finding, lane: quality) + +An invalid or missing `output_index` collapses to `0`, so two unindexed items can both become +`msg_ocx_0` — duplicate ids, which is the exact class of bug this fixes. Replace the +collapse-to-zero fallback with a monotonic per-response counter so synthesized ids are unique +even when `output_index` is absent or malformed. Add the regression test that pins it. + +Docs: the locale files in #2131 are uneven (EN/FR rewritten, JA/KO/ZH/TR only first sentence). +Carry only the EN change in this layer; locale parity is not this layer's thesis. + +## Test plan (must fail RED first) + +Carry @bet4it's tests (SSE `response.completed`, `output_item.done` via `output_index`, JSON +passthrough, preserve-existing-id, inherited `toString` type) and ADD: +- two items with missing `output_index` receive DISTINCT ids (fails on #2131 as written). + +## Verification + +Same gate as layer 1, plus explicit confirmation that layer 2's branch contains layer 1's +commit (`git log --oneline ..` shows only layer-2 commits) and that the PR +base ref names layer 1's branch. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/030_sibling_prompt_cache_retention.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/030_sibling_prompt_cache_retention.md new file mode 100644 index 0000000000..fc66d105c9 --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/030_sibling_prompt_cache_retention.md @@ -0,0 +1,38 @@ +# 030 — Sibling A: consolidate prompt_cache_retention (issue #2092) + +Work-phase: wp4. Branch: `codex/consolidate-prompt-cache-retention`. Base: **dev** (sibling, not a stack layer). +Absorbs: **PR #2102 by @lilinxiong** (base implementation). Supersedes: **#2099 by @yzxcj797**, **#2091 by @luvs01**. Closes #2092. + +## Why a sibling and not a layer + +It touches only `src/adapters/openai-responses.ts`, which no other absorbed item touches. It has +no dependency on layers 1-2, so stacking it would impose a false merge order (DEV-STACK-01). + +## Chosen contract + +@lilinxiong's #2102: strip `prompt_cache_retention` only when +`forward && isCanonicalOpenAiForwardProvider(provider)` AND the model is `gpt-5.6` or +`gpt-5.6-*`. This matches the issue's own correction — the reporter withdrew the "strip +everywhere" claim, and some non-5.6 deployments still honor the field. + +Rejected: #2091's blanket strip for every forward provider and every model (it inverts the +existing gpt-5.5 preserve pin at tests/openai-responses-passthrough.test.ts:807). +Rejected: #2099's `startsWith("gpt-5.6")`, which also matches `gpt-5.60`, and its stray +package.json 2.24.2 -> 2.25.0 bump. + +## Carried from the superseded PRs + +From @yzxcj797's #2099: the `Fixes #2092` issue link and the repro-shaped fixture +(`store:false`, streamed input array). From @luvs01's #2091: nothing — its key-auth preserve +case is already covered by #2102. + +## Tightening to apply + +Replace the string-prefix family match with the catalog/native-slug predicate if one exists +in the current tree (`rg -n "isGpt56NativeSlug|NATIVE_OPENAI_MODELS" src/`); otherwise keep +the exact `gpt-5.6` / `gpt-5.6-*` match and pin `gpt-5.60` as a NON-match in tests. + +## Test plan (must fail RED first) + +Carry #2102's tests; add `gpt-5.60` non-match; keep the gpt-5.5 preserve pin intact. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/040_sibling_routing_capability.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/040_sibling_routing_capability.md new file mode 100644 index 0000000000..62f0016582 --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/040_sibling_routing_capability.md @@ -0,0 +1,31 @@ +# 040 — Sibling B: routing capability + lab behavior evidence + +Work-phase: wp5. Branch: `codex/absorb-capability-evidence`. Base: **dev**. +Absorbs: **PR #2100 and PR #2077, both by @ntdatt812**. Closes both as superseded. + +## Why these two together, and why a sibling + +#2100 touches `src/routing/capability.ts`; #2077 touches +`src/routing/compatibility/behavior.ts`. Disjoint files, one author, one thesis: *model-keyed +lookups must use the same resolution rules the runtime uses*. Neither depends on layers 1-2. + +Note: #2077 is Lab-adjacent. Verify `tests/core-lab-boundary.test.ts` stays green — the file +already imports Lab types, so this must not newly puncture the boundary. + +## Defects + +#2100: bare map lookups made `gpt-oss:120b` inherit the provider-wide 8k window instead of the +`gpt-oss` family's 131072, and `noVisionModels` was ignored. +#2077: `map[modelId]` missed family/case overrides, and `constructor` resolved to +`Object.prototype.constructor`, making `jcsStringify` throw and silently dropping Lab subjects. + +## Change + +Route both through `modelRecordValue` / `isModelTextOnly` as @ntdatt812 wrote them. Prototype-id +safety (`constructor`, `toString`) is the load-bearing part; keep those tests verbatim. + +## Test plan + +Carry both test files. Confirm the exact-own maps (`modelPreferHostedTools`, +`modelOpenRouterRouting`) still do NOT family-spread. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/050_sibling_k12_short_window.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/050_sibling_k12_short_window.md new file mode 100644 index 0000000000..524d543b5c --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/050_sibling_k12_short_window.md @@ -0,0 +1,25 @@ +# 050 — Sibling C: K12 short-window quota (issue #2047) + +Work-phase: wp6. Branch: `codex/absorb-k12-short-window`. Base: **dev**. +Absorbs: **PR #2056 by @Ingwannu**. Supersedes: **#2062 by @yzxcj797**. Closes #2047. + +## Chosen base + +#2056 is a strict superset of #2062: `snapshotHasShort`, partial-snapshot preservation, +`updateAccountQuota` carry, and the parse -> cache -> DTO path #2047 actually requires. #2062 +drops short on a later weekly/monthly partial snapshot and carries a stray version bump. + +## Blocker to fix before this can land (raised by the maintainer on both PRs) + +A short-only snapshot with `shortPercent: 0` scores `0` instead of `CODEX_UNKNOWN_USAGE_SCORE`, +so `pickLowestUsageAmong` prefers an account whose long windows are unverified. Fix: +include `shortPercent` in `computeCodexUsageScore` only when the plan's governing long window +is finite; otherwise return `CODEX_UNKNOWN_USAGE_SCORE`. Add the short-only regression. + +This blocker is why #2056 is absorbed-and-corrected rather than simply approved. + +## Also close + +**#2063 by @yzxcj797** — superseded by ALREADY-MERGED #2055 (`2648ffa87`), which classifies +`detail.code` with a stricter own-property lookup. Close with attribution; fold nothing. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/060_supersede_and_close_operations.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/060_supersede_and_close_operations.md new file mode 100644 index 0000000000..ec55998dfc --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/060_supersede_and_close_operations.md @@ -0,0 +1,37 @@ +# 060 — Close-out: supersede operations and attribution + +Work-phase: wp7. No code. GitHub state only. + +## Ordering rule + +A PR is closed ONLY after its replacement exists and is pushed. Never close first. + +## Operations + +| Close | Author | Replaced by | Carried over | +|---|---|---|---| +| #2131 | @bet4it | layer 2 (020) | full implementation + tests, plus unique-id correction | +| #2099 | @yzxcj797 | sibling A (030) | issue link, repro fixture | +| #2091 | @luvs01 | sibling A (030) | nothing; contract deliberately narrower | +| #2100 | @ntdatt812 | sibling B (040) | full implementation + tests | +| #2077 | @ntdatt812 | sibling B (040) | full implementation + tests | +| #2102 | @lilinxiong | sibling A (030) | full implementation + tests (base) | +| #2062 | @yzxcj797 | sibling C (050) | nothing; #2056 supersedes | +| #2063 | @yzxcj797 | merged #2055 | nothing | +| #2056 | @Ingwannu | sibling C (050) | full implementation + scorer correction | +| #2029 | @yzxcj797 | maintainer PR #2130 | nothing; #2130 is a superset | + +## Comment template + +> Thanks for this, @ — closing as superseded by #, which carries from your +> patch. Your work is credited in that PR's description. + +## NOT closed, with reasons stated publicly + +- **#2109 / #2110** (@drakonkat): unresolved security gap in the override gate; needs a human + security pass (AGENTS.md security boundary). +- **#2053** (@Ingwannu): C4 OAuth surface; MAINTAINERS.md mandates security review. +- **#2101, #2040**: large (20 and 14 files); each needs its own PABCD cycle. +- **#2115, #2082, #2027, #2067, #2054, #2032**: below the 60 threshold. +- **#2104, #2075, #2127**: #2075/#2054 CONFLICTING; #2127 is an active draft by its author. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md new file mode 100644 index 0000000000..3d8506c5fd --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md @@ -0,0 +1,180 @@ +# 070 — Execution log: what actually shipped + +Appended as work-phases close. This is the record of landed state, distinct from the plan. + +## wp2 — issue #2132 (score 96) + +**PR #2137**, branch `codex/fix-bearer-admission-2132`, base `dev`. + +`substituteMainCredential` was computed from how the caller authenticated and never from where +the request routes, so a key-authenticated provider was gated on a ChatGPT credential it cannot +use. The predicate is now +`options.admission?.source === "bearer" && route.codexAccountMode !== undefined` +at both `core.ts:1088` and `compact.ts:325`. + +It covers `pool` AND `direct`. Doc 010's "native ChatGPT pool" wording would have excluded +`direct` and re-broken #1686, whose Direct admission is only safe because substitution still +runs. 010 now carries a banner saying so. + +Evidence: `tests/bearer-admission-routed-provider.test.ts` driven RED (it reproduced the exact +reported 401), full suite 13516 pass / 0 fail, typecheck and privacy scan clean. Re-audit round 2 +by the same adversarial reviewer returned **VERDICT: PASS**. + +## wp3 — issue #2092 (score 86) + +**PR #2138**, branch `codex/consolidate-prompt-cache-retention`, base `dev`. + +Absorbs @lilinxiong's #2102 contract: strip `prompt_cache_retention` on canonical ChatGPT +forward for the `gpt-5.6` family only, with an exact-or-dashed-prefix match so a future +`gpt-5.60` is not swept up. The retired value is not translated into `prompt_cache_options`. + +Evidence: 5 of the new tests fail when only the adapter change is reverted; the two narrowness +guards stay green in both directions, which is what makes them guards rather than restatements. +Full suite 13537 pass / 0 fail. + +### Closed with attribution + +| PR | Author | Superseded by | Carried | +|---|---|---|---| +| #2102 | @lilinxiong | #2138 | the implementation itself | +| #2099 | @yzxcj797 | #2138 | issue link + repro fixture | +| #2091 | @luvs01 | #2138 | nothing; contract deliberately narrower | +| #2029 | @yzxcj797 | merged #2130 | nothing; #2130 adds the disk check review demanded | +| #2063 | @yzxcj797 | merged #2055 | nothing; #2055 is the stricter own-property lookup | + +Each carries a comment naming the replacement and the specific reason, so no contributor has to +guess why their work closed. + +## Still open by decision, not omission + +- #2109 / #2110 (@drakonkat) — unresolved security gap in the override gate; needs a human pass. +- #2053 (@Ingwannu) — C4 OAuth; MAINTAINERS.md mandates security review. +- #2105 (@lilinxiong) — above threshold but no replacement exists yet; closing it now would lose work. +- #2101, #2040 — 20 and 14 files; each needs its own cycle. +- #2104 (@olddonkey) — review-ready and MERGEABLE; reclassified out of the deferred bucket, it is a + KEEP that deserves review rather than supersession. + +## Remaining work-phases + +wp4 (#2100 + #2077 capability evidence), wp5 (#2056 K12 with the scorer correction), wp6 (#2131 +responses id backfill with the duplicate-id fix). Each is a sibling off `dev`; none depends on +another. + + +## wp6 — PR #2131 (@bet4it) + +**PR #2142**, branch `codex/absorb-responses-id-backfill`, base `dev`. + +Carries @bet4it's implementation and tests, plus one correction: an absent or malformed +`output_index` collapsed to 0, so two such items both synthesized `msg_ocx_0` — duplicate ids, +the exact defect the backfill prevents. Unusable indices now take a monotonic ordinal based far +above any plausible real index. + +Evidence worth naming: applying ONLY @bet4it's original source and running the new suite gives +15 pass / 1 fail, and the single failure is the duplicate-id guard. That is what makes it a guard +rather than a restatement of behavior. + +The inherited assertion `expect(parsed.item.id).toBe("msg_ocx_0")` was replaced, not deleted +quietly, and the replacement is disclosed in the PR body. + +# Campaign state at wp6 close + +Superseded and closed with attribution: #2102, #2099, #2091, #2029, #2063, #2100, #2077, #2056, +#2062, #2131 — ten PRs, each with a comment naming its replacement and the specific reason. + +Opened: #2137 (#2132), #2138 (#2092), #2140 (#2100+#2077), #2141 (#2047), #2142 (#2131), plus +the pre-existing #2134. + +Deliberately still open: #2109/#2110 (security gap), #2053 (C4 OAuth review), #2105 (no +replacement written yet), #2101/#2040 (each needs its own cycle), #2104 (review-ready, deserves +review not supersession), and the below-threshold set (#2115, #2082, #2027, #2067, #2054, #2032, +#2075, #2127). + + +## wp7 — PR #2105 (@lilinxiong) + +**PR #2144**, branch `codex/absorb-claude-shell-hook-gate`, base `dev`. + +Implementation and tests carried unchanged. The one addition is a comment on +`reconcileShellHook` recording that "installed" is answered from the calling process's PATH, so +a service context with a stripped PATH can remove a hook an interactive shell would keep — the +reversible direction, and the one this reconcile wants. + +This closes the finding the auditor raised at #2105: it was scored ABSORB with no execution path +and would have been lost. It now has one. + + +# Campaign close — CI state and honest end state + +All six shipped PRs are green on exact head and MERGEABLE: + +| PR | Fixes | Checks | +|---|---|---| +| #2137 | issue #2132 | 25 pass / 0 fail | +| #2138 | issue #2092 (absorbs #2102) | 25 pass / 0 fail | +| #2140 | absorbs #2100 + #2077 | 25 pass / 0 fail | +| #2141 | issue #2047 (absorbs #2056) | 25 pass / 0 fail | +| #2142 | absorbs #2131 | 23 pass / 0 fail | +| #2144 | absorbs #2105 | 29 pass / 0 fail | + +#2140 first showed `npm-global-smoke` failing on windows-latest with +`EBUSY: resource busy or locked, unlink ...bun.exe` during dependency install — a Windows file +lock during Bun installation, not a defect in the routing change. Rerunning the failed jobs +turned it green, which is the evidence that it was infrastructure rather than the patch. + +## Eleven PRs closed with attribution + +#2102, #2099, #2091, #2029, #2063, #2100, #2077, #2056, #2062, #2131, #2105. + +Each carries a comment naming its replacement, what was carried over, and what was deliberately +not. Where a contributor's own assertion had to be replaced (#2056's `shortPercent: 0` scorer +case, #2131's `msg_ocx_0` collapse case), the replacement is disclosed in both the closing +comment and the superseding PR body rather than done silently. + +## Fourteen PRs deliberately still open + +- **Security holds:** #2109, #2110 (override gate), #2053 (C4 OAuth, MAINTAINERS.md review). +- **Own-cycle scale:** #2101 (20 files), #2040 (14 files). +- **Deserves review, not supersession:** #2104 — review-ready, MERGEABLE, and touching + `core.ts` alongside #2137. +- **Below the 60 threshold:** #2115, #2082, #2027, #2067, #2054, #2032, #2075, #2127. + +Nothing here is an omission. Every one is a recorded decision with a reason. + +## Merging + +Not done. DEV-STACK-04 and DEV-GIT-PUSH-01 both put merge authorization with the user, and +nothing in this campaign changes that. + + +## wp9 — PR #2101 (@Ingwannu): the ONE real stack layer + +**PR #2146**, branch `codex/absorb-account-entitlement-stacked`, base **`codex/fix-bearer-admission-2132`** (the #2137 branch), not `dev`. + +This is the single genuine dependency edge in the entire backlog. #2101 passes +`substituteMainCredentialForDirect: substituteMainCredential` into `resolveCodexAuthContext` — +the exact value #2137 corrects. Landing it on `dev` alone would silently reintroduce #2132 for +every routed provider. Everything else absorbed in this campaign was disjoint and shipped as a +sibling; this one is stacked because the code says so, not because a plan said so. + +Three corrections on top of @Ingwannu's work: + +1. **Selector compact bypassed the wire rewrite** — `accountGatedCompactWireModel` came from + `raw.model`, which never matches the gated map for `side/gpt-daybreak-blue-latest`, so a + selector-form compact still hit the native endpoint. Now derived from `route.modelId`. +2. **Direct callers evicted catalog evidence** — one 64-entry LRU shared between per-credential + Direct keys and the main/Pool keys the catalog projects from. Split into two eviction classes; + pinned by a test verified to fail against the shared LRU. +3. **Comment rot** — `native-models.ts` claimed routing never collapses Daybreak into + `gpt-5.6-sol`, which the wire normalization does. + +Evidence: full suite 13554 pass / 0 fail at the stacked tip; the composition check +(`codex-model-entitlements` + `bearer-admission-routed-provider` + `codex-auth-context` + +`server-auth`) is 146 pass / 0 fail, which is what proves the two layers agree. +Stack integrity: `git log parent..layer` shows exactly 1 commit, and a stack map was added to +#2137 so a reviewer arriving at the parent sees the chain. + +Two gaps named in the PR rather than carried silently: Direct `/v1/models` can still advertise a +Pool-only grant (advertisement only; dispatch still checks the caller credential), and +same-account gated-400 retry stays Pool-only. + diff --git a/src/lib/destination-policy.ts b/src/lib/destination-policy.ts index 75818af311..46b2aa91ba 100644 --- a/src/lib/destination-policy.ts +++ b/src/lib/destination-policy.ts @@ -130,6 +130,50 @@ function registryAllowsPrivateNetwork(name: string): boolean { return getProviderRegistryEntry(name)?.allowPrivateNetworkByDefault === true; } +/** + * OAuth registry entries that opt into `allowBaseUrlOverride` send bearer credentials to a + * user-configured endpoint (review findings, PR #2109 / PR #2110): a cleartext `http:` + * override would expose the OAuth token on the wire. `https:` is therefore required for + * every non-local destination. Loopback/localhost/private relays keep working over + * `http:` because they already sit behind the explicit `allowPrivateNetwork` opt-in + * enforced by {@link providerDestinationConfigError}. Keyed/local providers (Ollama, + * vLLM, LM Studio, LiteLLM, Moonshot, Qwen, Alibaba) are untouched: they are not + * `authKind: "oauth"`, so this check never fires for them. + */ +function registrySendsOAuthToOverriddenBaseUrl(name: string): boolean { + const entry = getProviderRegistryEntry(name); + return entry?.authKind === "oauth" && entry.allowBaseUrlOverride === true; +} + +export function providerSecureTransportConfigError( + name: string, + provider: Pick, +): string | null { + if (!registrySendsOAuthToOverriddenBaseUrl(name)) return null; + let parsed: URL; + try { + parsed = new URL(provider.baseUrl.trim()); + } catch { + return null; // invalid URLs are providerBaseUrlConfigError's concern + } + if (parsed.protocol !== "http:") return null; + const assessment = assessDestination(provider.baseUrl); + // Classify FIRST, then consult the opt-in. `allowPrivateNetwork` says "this destination is + // intentionally local", which is a statement about the address, not a waiver of transport + // security — reading it before classification let `http://attacker.example` with the opt-in + // set carry an OAuth bearer in cleartext to a public host. + if (!assessment) return null; + const local = assessment.kind === "localhost" + || assessment.kind === "loopback" + || assessment.kind === "private"; + if (local && providerAllowsPrivateNetwork(name, provider)) { + // A genuinely local relay over http stays reachable through the explicit opt-in; the + // private-network gate still governs whether it may be reached at all. + return null; + } + return "baseUrl must use https: this provider sends OAuth credentials to its endpoint, and http is allowed only for loopback/private relays"; +} + /** * Whether a provider may reach loopback/private addresses. * @@ -150,6 +194,8 @@ export function providerAllowsPrivateNetwork( } export function providerDestinationConfigError(name: string, provider: Pick): string | null { + const secureTransportError = providerSecureTransportConfigError(name, provider); + if (secureTransportError) return secureTransportError; const assessment = assessDestination(provider.baseUrl); if (!assessment) return null; if (assessment.kind === "public" || assessment.kind === "hostname") return null; @@ -331,3 +377,4 @@ export async function resolvePublicAddresses( export async function assertUrlResolvesPublic(url: string): Promise { await resolvePublicAddresses(url); } + diff --git a/src/providers/registry.ts b/src/providers/registry.ts index 53922e75ad..562f8e0c7d 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -1083,6 +1083,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ adapter: "anthropic", baseUrl: "https://api.anthropic.com", authKind: "oauth", + allowBaseUrlOverride: true, featured: true, oauthId: "anthropic", jawcodeBundle: "anthropic", @@ -1496,7 +1497,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ // 2026-07-10: defaultModel is frozen pending Vertex-specific Tier-2 evidence; Gemini API // evidence from ai.google.dev does not establish Vertex publisher availability. { id: "google-vertex", label: "Google Vertex AI", adapter: "google", baseUrl: "https://aiplatform.googleapis.com", authKind: "key", dashboardUrl: "https://console.cloud.google.com/vertex-ai", defaultModel: "gemini-3-pro", googleMode: "vertex", jawcodeBundle: "google", extraMetadataAliases: ["gemini-vertex"] }, - { id: "google-antigravity", label: "Google Antigravity", adapter: "google", baseUrl: "https://daily-cloudcode-pa.googleapis.com", authKind: "oauth", dashboardUrl: "https://antigravity.google", models: ANTIGRAVITY_MODELS, liveModels: true, defaultModel: "gemini-3.7-flash", modelContextWindows: ANTIGRAVITY_MODEL_CONTEXT_WINDOWS, modelInputModalities: ANTIGRAVITY_MODEL_INPUT_MODALITIES, modelReasoningEfforts: ANTIGRAVITY_MODEL_EFFORTS, googleMode: "cloud-code-assist", jawcodeBundle: "google", extraMetadataAliases: ["antigravity", "gemini-antigravity"] }, + { id: "google-antigravity", label: "Google Antigravity", adapter: "google", baseUrl: "https://daily-cloudcode-pa.googleapis.com", authKind: "oauth", allowBaseUrlOverride: true, dashboardUrl: "https://antigravity.google", models: ANTIGRAVITY_MODELS, liveModels: true, defaultModel: "gemini-3.7-flash", modelContextWindows: ANTIGRAVITY_MODEL_CONTEXT_WINDOWS, modelInputModalities: ANTIGRAVITY_MODEL_INPUT_MODALITIES, modelReasoningEfforts: ANTIGRAVITY_MODEL_EFFORTS, googleMode: "cloud-code-assist", jawcodeBundle: "google", extraMetadataAliases: ["antigravity", "gemini-antigravity"] }, { id: "azure-openai", label: "Azure OpenAI", adapter: "azure-openai", baseUrl: "https://{resource}.openai.azure.com/openai", authKind: "key", featured: true, dashboardUrl: "https://portal.azure.com" }, { id: "ollama", label: "Ollama (local)", adapter: "openai-chat", baseUrl: "http://localhost:11434/v1", authKind: "local", allowPrivateNetworkByDefault: true, allowBaseUrlOverride: true, featured: true, note: "Local — key usually blank" }, { id: "vllm", label: "vLLM (local)", adapter: "openai-chat", baseUrl: "http://localhost:8000/v1", authKind: "local", allowPrivateNetworkByDefault: true, allowBaseUrlOverride: true, featured: true, note: "Local — key usually blank" }, diff --git a/tests/anthropic-baseurl-override.test.ts b/tests/anthropic-baseurl-override.test.ts new file mode 100644 index 0000000000..b64e0feaab --- /dev/null +++ b/tests/anthropic-baseurl-override.test.ts @@ -0,0 +1,144 @@ +import { expect, test } from "bun:test"; +import { routeModel } from "../src/router"; +import { providerDestinationConfigError } from "../src/lib/destination-policy"; +import type { OcxConfig, OcxProviderConfig } from "../src/types"; + +/** + * Regression coverage for the allowBaseUrlOverride opt-in on the anthropic + * registry entry. + * + * Before the opt-in, the pinned registry endpoint silently outranked a saved + * baseUrl and the router emitted the discarded-baseUrl diagnostic (see + * tests/router-discarded-baseurl-warning.test.ts, which now pins google as + * its fixture). Users routing Claude traffic through a local relay or an + * enterprise gateway therefore could not redirect the provider at all. These + * tests pin the new contract: a resolved user baseUrl wins, no warning fires, + * and the registry endpoint remains the default seeded value. + */ +const PROVIDER = "anthropic"; +const REGISTRY_BASE_URL = "https://api.anthropic.com"; +const MODEL = PROVIDER + "/claude-sonnet-5"; + +function configFor(provider: OcxProviderConfig): OcxConfig { + return { + port: 10100, + defaultProvider: PROVIDER, + providers: { [PROVIDER]: provider }, + }; +} + +function routeCapturingWarnings(config: OcxConfig): { baseUrl: string; warnings: string[] } { + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { warnings.push(args.map(String).join(" ")); }; + try { + const route = routeModel(config, MODEL); + return { baseUrl: route.provider.baseUrl, warnings }; + } finally { + console.warn = originalWarn; + } +} + +test("anthropic honors a configured baseUrl override", () => { + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: "https://claude-relay.example.test", + } as OcxProviderConfig)); + + expect(baseUrl).toBe("https://claude-relay.example.test"); + // The override is applied, so the discarded-baseUrl diagnostic must not fire. + expect(warnings).toHaveLength(0); +}); + +test("anthropic keeps the registry endpoint when the seeded baseUrl is unchanged", () => { + // providerConfigSeed copies the registry baseUrl into every saved config, so the + // no-override case reaches the router as a config whose baseUrl equals the registry URL. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: REGISTRY_BASE_URL, + } as OcxProviderConfig)); + + expect(baseUrl).toBe(REGISTRY_BASE_URL); + expect(warnings).toHaveLength(0); +}); + +test("anthropic requires a resolved baseUrl once override is enabled", () => { + // allowBaseUrlOverride providers fail closed on a missing baseUrl instead of silently + // re-pinning the registry endpoint; the seed guarantees real configs always carry one. + expect(() => routeModel(configFor({ + adapter: "anthropic", + } as OcxProviderConfig), MODEL)).toThrow(/Invalid baseUrl/); +}); + +test("anthropic rejects an unresolved template baseUrl override", () => { + expect(() => routeModel(configFor({ + adapter: "anthropic", + baseUrl: "https://{region}.example.test", + } as OcxProviderConfig), MODEL)).toThrow(/Invalid baseUrl/); +}); + +/** + * Security regression (CodeRabbit, PR #2109): anthropic is an OAuth provider, so an + * allowBaseUrlOverride endpoint receives bearer credentials. A cleartext http override to a + * non-local destination must be rejected on BOTH enforcement paths: routing (normal requests, + * via assertProviderDestinationAllowed) and providerDestinationConfigError, the shared gate + * that config validation and the model-discovery outbound layer (providerGet/providerPost in + * src/lib/provider-outbound.ts) consult before any fetch. + */ +test("anthropic rejects a cleartext http override on the routing path", () => { + expect(() => routeModel(configFor({ + adapter: "anthropic", + baseUrl: "http://claude-relay.example.test", + } as OcxProviderConfig), MODEL)).toThrow(/https/); +}); + +test("anthropic rejects a cleartext http override on the discovery/config gate", () => { + expect(providerDestinationConfigError(PROVIDER, { + baseUrl: "http://claude-relay.example.test", + } as OcxProviderConfig)).toMatch(/https/); + // The https form of the same destination stays accepted. + expect(providerDestinationConfigError(PROVIDER, { + baseUrl: "https://claude-relay.example.test", + } as OcxProviderConfig)).toBeNull(); +}); + +test("anthropic keeps http for an explicitly local relay", () => { + // Loopback and allowPrivateNetwork opt-ins are the documented local-transport escape + // hatch; the https requirement must not break a localhost proxy. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: "http://127.0.0.1:8787", + allowPrivateNetwork: true, + } as OcxProviderConfig)); + + expect(baseUrl).toBe("http://127.0.0.1:8787"); + expect(warnings).toHaveLength(0); +}); + + +test("a public http override cannot buy transport security with allowPrivateNetwork", () => { + // allowPrivateNetwork states that a destination is intentionally LOCAL. It is not a waiver of + // transport security. Reading it before classifying the address let http://attacker.example + // carry this provider's OAuth bearer in cleartext to a public host. + // + // Routing REFUSES rather than downgrading: a request must not reach an endpoint that would + // receive the token in the clear, so this fails closed at the route boundary. + expect(() => routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: "http://attacker.example/v1", + allowPrivateNetwork: true, + } as OcxProviderConfig))).toThrow(/must use https/); +}); + +test("the seeded https endpoint is still reachable with the opt-in set", () => { + // Guard against over-correcting: the fix must refuse cleartext to a public host without + // refusing an ordinary https override that happens to carry the flag. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "anthropic", + baseUrl: "https://gateway.example/v1", + allowPrivateNetwork: true, + } as OcxProviderConfig)); + + expect(baseUrl).toBe("https://gateway.example/v1"); + expect(warnings).toHaveLength(0); +}); diff --git a/tests/antigravity-baseurl-override.test.ts b/tests/antigravity-baseurl-override.test.ts new file mode 100644 index 0000000000..5bc190c715 --- /dev/null +++ b/tests/antigravity-baseurl-override.test.ts @@ -0,0 +1,117 @@ +import { expect, test } from "bun:test"; +import { routeModel } from "../src/router"; +import { providerDestinationConfigError } from "../src/lib/destination-policy"; +import type { OcxConfig, OcxProviderConfig } from "../src/types"; + +/** + * Regression coverage for the allowBaseUrlOverride opt-in on the + * google-antigravity registry entry. + * + * Before the opt-in, the pinned registry endpoint silently outranked a saved + * baseUrl and the router emitted the discarded-baseUrl diagnostic (see + * tests/router-discarded-baseurl-warning.test.ts). Users routing Antigravity + * traffic through a local relay or region-specific proxy therefore could not + * redirect the provider at all. These tests pin the new contract: a resolved + * user baseUrl wins, no warning fires, and the registry endpoint remains the + * default when nothing is configured. + */ +const PROVIDER = "google-antigravity"; +const REGISTRY_BASE_URL = "https://daily-cloudcode-pa.googleapis.com"; +const MODEL = PROVIDER + "/gemini-3.7-flash"; + +function configFor(provider: OcxProviderConfig): OcxConfig { + return { + port: 10100, + defaultProvider: PROVIDER, + providers: { [PROVIDER]: provider }, + }; +} + +function routeCapturingWarnings(config: OcxConfig): { baseUrl: string; warnings: string[] } { + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { warnings.push(args.map(String).join(" ")); }; + try { + const route = routeModel(config, MODEL); + return { baseUrl: route.provider.baseUrl, warnings }; + } finally { + console.warn = originalWarn; + } +} + +test("google-antigravity honors a configured baseUrl override", () => { + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "google", + baseUrl: "https://antigravity-relay.example.test", + } as OcxProviderConfig)); + + expect(baseUrl).toBe("https://antigravity-relay.example.test"); + // The override is applied, so the discarded-baseUrl diagnostic must not fire. + expect(warnings).toHaveLength(0); +}); + +test("google-antigravity keeps the registry endpoint when the seeded baseUrl is unchanged", () => { + // providerConfigSeed copies the registry baseUrl into every saved config, so the + // no-override case reaches the router as a config whose baseUrl equals the registry URL. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "google", + baseUrl: REGISTRY_BASE_URL, + } as OcxProviderConfig)); + + expect(baseUrl).toBe(REGISTRY_BASE_URL); + expect(warnings).toHaveLength(0); +}); + +test("google-antigravity requires a resolved baseUrl once override is enabled", () => { + // allowBaseUrlOverride providers fail closed on a missing baseUrl instead of silently + // re-pinning the registry endpoint; the seed guarantees real configs always carry one. + expect(() => routeModel(configFor({ + adapter: "google", + } as OcxProviderConfig), MODEL)).toThrow(/Invalid baseUrl/); +}); + +test("google-antigravity rejects an unresolved template baseUrl override", () => { + expect(() => routeModel(configFor({ + adapter: "google", + baseUrl: "https://{region}.example.test", + } as OcxProviderConfig), MODEL)).toThrow(/Invalid baseUrl/); +}); + +/** + * Security regression (CodeRabbit, PR #2110): google-antigravity is an OAuth provider, so an + * allowBaseUrlOverride endpoint receives bearer credentials. A cleartext http override to a + * non-local destination must be rejected on BOTH enforcement paths: routing (normal requests, + * via assertProviderDestinationAllowed) and providerDestinationConfigError, the shared gate + * that config validation and the outbound layer (providerGet/providerPost in + * src/lib/provider-outbound.ts) consult before any fetch. + */ +test("google-antigravity rejects a cleartext http override on the routing path", () => { + expect(() => routeModel(configFor({ + adapter: "google", + baseUrl: "http://antigravity-relay.example.test", + } as OcxProviderConfig), MODEL)).toThrow(/https/); +}); + +test("google-antigravity rejects a cleartext http override on the discovery/config gate", () => { + expect(providerDestinationConfigError(PROVIDER, { + baseUrl: "http://antigravity-relay.example.test", + } as OcxProviderConfig)).toMatch(/https/); + // The https form of the same destination stays accepted. + expect(providerDestinationConfigError(PROVIDER, { + baseUrl: "https://antigravity-relay.example.test", + } as OcxProviderConfig)).toBeNull(); +}); + +test("google-antigravity keeps http for an explicitly local relay", () => { + // The local proxy (127.0.0.1) is the motivating use case for this override; the https + // requirement must not break it. allowPrivateNetwork is the documented local opt-in. + const { baseUrl, warnings } = routeCapturingWarnings(configFor({ + adapter: "google", + baseUrl: "http://127.0.0.1:47821", + allowPrivateNetwork: true, + } as OcxProviderConfig)); + + expect(baseUrl).toBe("http://127.0.0.1:47821"); + expect(warnings).toHaveLength(0); +}); + diff --git a/tests/provider-model-discovery-contract.test.ts b/tests/provider-model-discovery-contract.test.ts index 07c8c0e6ac..c49d4062dd 100644 --- a/tests/provider-model-discovery-contract.test.ts +++ b/tests/provider-model-discovery-contract.test.ts @@ -172,15 +172,15 @@ describe("registry-owned provider model discovery", () => { authMode: "oauth", }; - await withRegistryDiscovery("anthropic", { path: "catalog" }, () => { - const relative = buildModelsRequest(staleConfig, "oauth-token", "anthropic"); - expect(relative.url).toBe("https://api.anthropic.com/catalog"); + await withRegistryDiscovery("kimi", { path: "catalog" }, () => { + const relative = buildModelsRequest(staleConfig, "oauth-token", "kimi"); + expect(relative.url).toBe("https://api.kimi.com/coding/v1/catalog"); expect(relative.headers.Authorization).toBe("Bearer oauth-token"); }); - await withRegistryDiscovery("anthropic", { maxModels: 25 }, () => { - const defaultEndpoint = buildModelsRequest(staleConfig, "oauth-token", "anthropic"); - expect(defaultEndpoint.url).toBe("https://api.anthropic.com/v1/models?limit=1000"); + await withRegistryDiscovery("kimi", { maxModels: 25 }, () => { + const defaultEndpoint = buildModelsRequest(staleConfig, "oauth-token", "kimi"); + expect(defaultEndpoint.url).toBe("https://api.kimi.com/coding/v1/models"); }); }); @@ -604,3 +604,4 @@ describe("same-named custom provider preservation", () => { }); }); }); + diff --git a/tests/provider-registry-parity.test.ts b/tests/provider-registry-parity.test.ts index f06a8953c3..d2858f6f44 100644 --- a/tests/provider-registry-parity.test.ts +++ b/tests/provider-registry-parity.test.ts @@ -563,7 +563,10 @@ describe("provider registry parity", () => { test("base URL override permission is registry-only and limited to opted-in providers", () => { const optedIn = PROVIDER_REGISTRY.filter(entry => entry.allowBaseUrlOverride); - expect(optedIn.map(entry => entry.id)).toEqual(["ollama", "vllm", "lm-studio", "moonshot", "qwen-cloud", "alibaba", "alibaba-token-plan-intl", "litellm"]); + // Registry order. Both OAuth entries (anthropic, google-antigravity) are gated by + // providerSecureTransportConfigError; the rest are key/local providers that never send a + // subscription bearer to the override. + expect(optedIn.map(entry => entry.id)).toEqual(["anthropic", "google-antigravity", "ollama", "vllm", "lm-studio", "moonshot", "qwen-cloud", "alibaba", "alibaba-token-plan-intl", "litellm"]); for (const entry of optedIn) { expect(providerConfigSeed(entry)).not.toHaveProperty("allowBaseUrlOverride"); } diff --git a/tests/router-discarded-baseurl-warning.test.ts b/tests/router-discarded-baseurl-warning.test.ts index 05ea18e250..dc31220a3a 100644 --- a/tests/router-discarded-baseurl-warning.test.ts +++ b/tests/router-discarded-baseurl-warning.test.ts @@ -7,12 +7,12 @@ import type { OcxConfig, OcxProviderConfig } from "../src/types"; * asserted in tests/router-template-baseurl.test.ts; these tests cover the diagnostic that * tells the user it happened, so a wrong-region URL stops surfacing as a bare 401. * - * `anthropic` is the pinned fixture: a fixed remote registry endpoint, no `allowBaseUrlOverride`. + * `google` is the pinned fixture: a fixed remote registry endpoint, no `allowBaseUrlOverride`. * Warnings dedupe per (provider, discarded URL, effective URL), so each test uses a distinct * discarded URL and the suite stays order-independent. */ -const PINNED_PROVIDER = "anthropic"; -const PINNED_REGISTRY_BASE_URL = "https://api.anthropic.com"; +const PINNED_PROVIDER = "google"; +const PINNED_REGISTRY_BASE_URL = "https://generativelanguage.googleapis.com"; function configFor(providerName: string, provider: OcxProviderConfig): OcxConfig { return { @@ -37,8 +37,8 @@ function routeCapturingWarnings(config: OcxConfig, model: string, times = 1): st function routePinned(baseUrl: unknown, times = 1): string[] { return routeCapturingWarnings( - configFor(PINNED_PROVIDER, { adapter: "anthropic", baseUrl } as OcxProviderConfig), - `${PINNED_PROVIDER}/claude-sonnet-5`, + configFor(PINNED_PROVIDER, { adapter: "google", baseUrl } as OcxProviderConfig), + `${PINNED_PROVIDER}/gemini-3-pro`, times, ); } @@ -57,13 +57,13 @@ test("warns when a pinned provider discards a configured baseUrl", () => { test("routing is unchanged by the warning", () => { const config = configFor(PINNED_PROVIDER, { - adapter: "anthropic", + adapter: "google", baseUrl: "https://routing-unchanged.example.test/v1", }); const originalWarn = console.warn; console.warn = () => {}; try { - expect(routeModel(config, `${PINNED_PROVIDER}/claude-sonnet-5`).provider.baseUrl) + expect(routeModel(config, `${PINNED_PROVIDER}/gemini-3-pro`).provider.baseUrl) .toBe(PINNED_REGISTRY_BASE_URL); } finally { console.warn = originalWarn; @@ -221,3 +221,4 @@ for (const { label, id, adapter, baseUrl } of [ expect(routeModel(config, `${id}/model`).provider.baseUrl).toBe(baseUrl); }); } + diff --git a/tests/router-template-baseurl.test.ts b/tests/router-template-baseurl.test.ts index e3880a45d4..053f2ff7b7 100644 --- a/tests/router-template-baseurl.test.ts +++ b/tests/router-template-baseurl.test.ts @@ -56,7 +56,7 @@ for (const { id, registryBaseUrl } of OVERRIDE_PROVIDERS) { for (const { id, registryBaseUrl, adapter } of [ { id: "ollama-cloud", registryBaseUrl: "https://ollama.com/v1", adapter: "openai-chat" }, - { id: "anthropic", registryBaseUrl: "https://api.anthropic.com", adapter: "anthropic" }, + { id: "google", registryBaseUrl: "https://generativelanguage.googleapis.com", adapter: "google" }, ] as const) { test(`${id} keeps its fixed remote registry endpoint authoritative`, () => { const config = configFor(id, { @@ -100,3 +100,4 @@ for (const { id, adapter, registryTemplate, resolvedBaseUrl } of [ expect(routeModel(config, `${id}/model`).provider.baseUrl).toBe(registryTemplate); }); } +