diff --git a/AGENTS_INSTALL.md b/AGENTS_INSTALL.md index 445da2e4b9..36cb3b383a 100644 --- a/AGENTS_INSTALL.md +++ b/AGENTS_INSTALL.md @@ -75,3 +75,35 @@ Regression coverage: `tests/startup-prompt.test.ts`, If a future action spends the user's identity, credits, or reputation, gate it the same way rather than relying on a prompt an agent can answer, and document it here. + +## Browser and Computer Use plugins: entry point + +This is not a consent boundary — it is a routing fact that costs agents real +time when they do not know it, and it belongs here because it applies to an +agent *operating* opencodex rather than changing it. + +The Chrome, Browser, and Computer Use plugins run **only** through the +privileged Node REPL tool (`mcp__node_repl__js`). Call it directly. If it is not +in the current tool set, search available tools for `node_repl js` before +concluding anything is unavailable. + +These do not work and are not worth attempting: + +- `node` / `node -e` importing the plugin's `scripts/browser-client.mjs`. It + refuses with `Browser use requires privileged node_repl capabilities` — the + bundle reads `globalThis.nodeRepl` and ships its own `process` shim, both + injected by the privileged REPL host and by nothing else. +- Filesystem searches for `@oai/sky`. Computer Use injects it at runtime; there + is no package on disk, so `find` and `mdfind` can only ever come back empty. +- `osascript` / AppleScript / JXA as a substitute for the plugin API. + +A failed shell attempt is evidence about the shell, not about plugin +availability. + +Smaller local models misroute here for a specific reason worth naming: the +bundled Chrome skill asks an agent not to *mention* the REPL tool in +user-facing prose while simultaneously requiring it to *use* that tool. Both +hold at once. The naming restriction governs what you say to the user; it never +means the tool is off-limits. A model that resolves the tension by avoiding the +tool will exhaust every shell path and then report the plugins as unavailable, +which is what prompted writing this down. diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/000_plan.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/000_plan.md new file mode 100644 index 0000000000..7201fc113c --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/000_plan.md @@ -0,0 +1,66 @@ +# 260816 — Local-model capability evidence and browser-plugin routing + +## Objective + +A local Qwen model (`lidge/qwen3.8-27b-nvfp4`, llama.cpp behind an +`openai-chat` adapter) could not drive the Chrome or Computer Use browser +plugins. Investigating that failure surfaced a defect in +`src/routing/capability.ts` that is **not local-model specific**: the cached +Codex catalog is read on every policy-routed request and then discarded in +full, because the reader expects a field shape the catalog writer never +produces. + +This unit fixes the capability-evidence defect, records the live-catalog +ingestion gap, and writes durable routing guidance for weaker local models +that must reach the browser plugins through the privileged Node REPL tool. + +## Constraints + +- `src/routing/capability.ts` is on the request path for every policy-routed + request. The memoized catalog read exists precisely so the parse cost is not + paid per candidate; a fix must not turn it into a per-candidate parse. +- "Unknown is not zero" is the module's stated contract (file header): a + dimension without canonical evidence must stay `undefined`, never `false`. + The fix must not convert a missing catalog field into a negative assertion. +- `src/routing/capability.ts` is reachable from `src/router.ts`, so the + core/lab boundary in `tests/core-lab-boundary.test.ts` applies: no import + may reach `src/lab/`. +- Out of scope: promotion to `main`, npm publish, GUI redesign, unrelated + provider adapters. + +## Dependency-ordered work-phase map + +The order is build-order, not effort order: the catalog reader is the +foundation both later phases depend on. + +| Phase | Doc | Depends on | Independently verifiable by | +|-------|-----|------------|------------------------------| +| 1 | `010_catalog_row_shape.md` | — | New focused test: catalog-sourced evidence survives for a routed row | +| 2 | `020_live_capability_ingestion.md` | Phase 1 | Focused test driving the observed llama.cpp `/v1/models` payload | +| 3 | `030_local_model_plugin_routing.md` | — (docs surface) | The written guidance resolves on the documented path | + +Phase 3 has no code dependency on 1 or 2 and could land in any order; it is +listed last because it is documentation, not because it is smaller. + +## Source-of-truth sync target (SOT-SYNC-01) + +`structure/` holds maintainer invariants. Phase 1 changes how routing evidence +is sourced, so C patches the structure note that describes routing evidence if +one exists; if none does, the D summary recommends creating it. + +## Research documents + +- `001_capability_evidence_defect.md` — the reproduction, the field-shape + mismatch, and why every existing test passes over it. +- `002_local_model_plugin_failure.md` — why the local model could not reach + the Chrome and Computer Use plugins. + +## Filed issues + +| Issue | Covers | Fixed by | +|-------|--------|----------| +| [#1796](https://github.com/lidge-jun/opencodex/issues/1796) | Routing discards every catalog row (field-shape mismatch) | Phase 1 | +| [#1797](https://github.com/lidge-jun/opencodex/issues/1797) | llama.cpp `multimodal` token + dual-envelope join | Deferred; Phase 2 ships the context half only | + +Phase 3 files no issue: it is host-side guidance with no opencodex defect +behind it (see `002_local_model_plugin_failure.md`). diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/001_capability_evidence_defect.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/001_capability_evidence_defect.md new file mode 100644 index 0000000000..11efccb64e --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/001_capability_evidence_defect.md @@ -0,0 +1,140 @@ +# 001 — Catalog capability evidence never reaches routing + +Research document. No diffs here; the fix design is `010_catalog_row_shape.md`. + +## Symptom that started this + +`lidge/qwen3.8-27b-nvfp4` was registered through the supported CLI: + +``` +$ ocx models add lidge qwen3.8-27b-nvfp4 --context-window 262144 --modalities text,image +Error: custom model "lidge/qwen3.8-27b-nvfp4" already exists +``` + +The row was already present and complete in `~/.opencodex/config.json`: + +```json +{ "id": "83ca0b4c-06bb-475d-b585-6c47b9d6be71", "provider": "lidge", + "modelId": "qwen3.8-27b-nvfp4", "displayName": "Qwen3.8 27B NVFP4 (lidge 5090)", + "contextWindow": 262144, "inputModalities": ["text", "image"] } +``` + +It also reached the on-disk Codex catalog correctly, as +`/Users/jun/.codex/opencodex-catalog.json`: + +```json +{ "slug": "lidge/qwen3.8-27b-nvfp4", "context_window": 262144, + "input_modalities": ["text", "image"], "supports_parallel_tool_calls": true } +``` + +Yet routing evidence carried neither value. NOTE (audit round 1, B9): this +one-liner reproduces only with the provider's `modelContextWindows` and +`modelInputModalities` ABSENT. They were added by hand later while +diagnosing, so on today's live config the earlier branches win and the +symptom is masked. The 17-to-0 catalog proof below is independent of that +and still reproduces exactly. Use a catalog-only fixture to reproduce: + +``` +candidateCapabilityEvidence(config, "lidge", "qwen3.8-27b-nvfp4") +=> { tools: true, serviceTier: "unsupported", encryptedCodexTasks: false } +``` + +No `image`. No `contextWindow`. A model registered through the documented +path is image-blind to the router. + +## Root cause: reader and writer disagree on field shape + +`src/routing/capability.ts` `cachedCatalogModels()` filters rows with: + +```ts +typeof model.id === "string" && typeof model.provider === "string" +``` + +and then reads `model.contextWindow` / `model.inputModalities`. + +The catalog file has none of those four fields. Its actual keys are +`slug` (a combined `provider/id`), `context_window`, and `input_modalities`. +Verified against the live file: + +``` +KEYS: slug,display_name,description,default_reasoning_level, +supported_reasoning_levels,...,input_modalities,...,context_window, +max_context_window,auto_compact_token_limit,... +provider? undefined id? undefined context_window? 262144 +``` + +So the filter rejects every row: + +``` +TOTAL: 17 | SURVIVING capability.ts filter: 0 +``` + +**This is not a local-model bug.** All 17 rows are discarded — native OpenAI +rows, `anthropic/claude-opus-5`, `xai/grok-4.6`, everything. The whole +`catalogRow` branch of the evidence chain is dead code in practice. + +## Why nobody noticed + +The catalog is the *fourth* fallback. For a provider with populated config +maps the earlier branches answer first: + +``` +provider.modelContextWindows[id] ?? provider.contextWindow + ?? registryEntry.modelContextWindows[id] ?? catalogRow.contextWindow ?? ... +``` + +`kimi`, `anthropic`, `xai`, and `alibaba-token-plan-intl` all declare +`modelContextWindows` and `modelInputModalities` inline, so their evidence +looks correct and the dead branch stays invisible. Only a provider that +relies on the catalog — exactly what `ocx models add` produces — is exposed. + +Confirmed by contrast on the same tree: + +``` +xai/grok-4.6 => contextWindow 500000, image true (provider maps) +anthropic/claude-opus-5 => contextWindow 1000000, no image (provider maps) +lidge/qwen3.8-27b-nvfp4 => nothing (catalog only) +``` + +`claude-opus-5` is itself a smaller instance of the same hole: the provider +block declares `modelContextWindows` but no `modelInputModalities`, and the +catalog that could have supplied `image` is discarded. + +## Why the test suite is green + +`tests/routing-profile.test.ts` and `tests/routing-compatibility.test.ts` +construct `capability` objects inline: + +```ts +{ provider: "a", model: "m1", capability: { contextWindow: 200000, tools: true } } +``` + +They exercise the *policy evaluator* with pre-made evidence and never call +`candidateCapabilityEvidence`, so no test ever reads a real catalog file. +The assembly step between the catalog on disk and the evaluator is untested. +That is the coverage gap this unit closes, and it is why "all tests green" +said nothing about this defect. + +## Field-chain note (PLAN-FIELD-CHAIN-01) + +The chain for a custom model is: + +| Stage | Path | State | +|-------|------|-------| +| creation | `src/cli/models.ts` `ocx models add` | works | +| serialization | `config.customModels[]` -> `src/codex/catalog/provider-fetch.ts:1758` | works | +| on-disk form | `~/.codex/opencodex-catalog.json` (`slug`, `context_window`, `input_modalities`) | works | +| consumer | `src/routing/capability.ts` `cachedCatalogModels()` | **broken — expects `provider`/`id`/`contextWindow`/`inputModalities`** | + +Only the last stage is wrong, which is why the value is visible everywhere a +human looks (config, CLI, catalog file) and absent exactly where routing +decides. + +## Post-audit correction (round 1) + +The fix originally proposed here — read `context_window` and +`input_modalities` straight off the row — is WRONG and was rejected in +audit. `ensureStrictCatalogFields()` synthesizes both fields for Codex's +strict parser, so reading them would convert unknown into `image:false` +and a fabricated `128000`. See `003_audit_synthesis_round1.md` B2 and the +provenance design in `010_catalog_row_shape.md`. diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/002_local_model_plugin_failure.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/002_local_model_plugin_failure.md new file mode 100644 index 0000000000..86fb488b2c --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/002_local_model_plugin_failure.md @@ -0,0 +1,60 @@ +# 002 — Why a local model could not reach the browser plugins + +Research document. Environment observation, not an opencodex code defect; +the durable-guidance design is `030_local_model_plugin_routing.md`. + +## What was observed + +A local Qwen model asked to browse with the Chrome and Computer Use plugins +tried, in order: `node -e "import('@oai/sky')"`, `find` and `mdfind` sweeps +for a `sky` package, `osascript` against Google Chrome, and finally a +hand-written `/tmp/chrome_probe.mjs` importing the plugin's +`browser-client.mjs` directly. It then reported the tooling unavailable. + +The tooling was available the whole time. + +## Why every shell attempt fails by construction + +`scripts/browser-client.mjs` in the Chrome plugin is a ~1.15 MB bundle that +expects a privileged host. Importing it from an ordinary Node process +resolves its exports and then refuses at runtime: + +```console +$ node -e "import('.../scripts/browser-client.mjs').then(m => m.setupBrowserRuntime())" +RUNTIME FAIL: Browser use requires privileged node_repl capabilities +``` + +The bundle carries its own `process` shim and reads `globalThis.nodeRepl`; +those are injected by the privileged REPL host, not by Node. Computer Use is +stricter still — `@oai/sky` has no on-disk package at all, so filesystem +searches for it can only ever come back empty. + +The single working entry point is the `mcp__node_repl__js` tool. Driving the +same plugin through it succeeded immediately in this session: browser bound, +`chrome.user.openTabs()` returned the live tab list, navigation and a +screenshot both worked. + +## Why a weaker model misroutes here + +The Chrome skill deliberately obscures its own mechanism for user-facing +reasons: + +> "Never mention `Node REPL`, `node_repl`, `REPL`, JavaScript sessions ... +> unless a user is asking for that exact information." + +while simultaneously requiring it: + +> "Run browser setup code through the Node REPL `js` tool ... If it is not +> already available, use tool discovery for `node_repl js`." + +A model with strong instruction-following holds both. A weaker one resolves +the conflict by treating the named tool as off-limits and substitutes a +shell, which is exactly the observed failure. The skill text plus its +`documentation()` payload is also ~60 KB before any work begins, which +compounds the problem for a small local context. + +## Scope boundary + +This is host/skill routing behavior, not opencodex runtime behavior. Nothing +in `src/` can fix it. The actionable output is durable guidance on a surface +a local model actually reads, and that is all Phase 3 does. diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/003_audit_synthesis_round1.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/003_audit_synthesis_round1.md new file mode 100644 index 0000000000..0dc5ca59bd --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/003_audit_synthesis_round1.md @@ -0,0 +1,137 @@ +# 003 — Audit synthesis, round 1 (REVIEW-SYNTHESIS-01) + +Reviewer: independent `explorer` on `gpt-5.6-sol` (medium effort). +Verdict: **FAIL**, 9 blockers (3 High, 4 Medium, 2 Low). + +The reviewer confirmed the baseline defect — 17 catalog rows, 0 surviving the +current filter — and then found that the proposed repair would have shipped two +regressions. Every High blocker was re-verified independently against the code +before acceptance; none are taken on the reviewer's word. + +## B1 (High) — ACCEPTED. Matching a catalog row silently removes `tools:true` + +`src/routing/capability.ts:178`: + +```ts + || (catalogRow === undefined && provider !== undefined && TOOL_CAPABLE_ADAPTERS.has(provider.adapter)) +``` + +The adapter fallback is gated on the catalog row being **absent**. That gate is +harmless today only because the lookup never matches anything. Repairing the +lookup arms it: every routed model would suddenly find its row, lose the adapter +fallback, and — because generated rows do not serialize `capabilities` — end up +with `tools: undefined`. + +Root cause: the condition encodes "no catalog row" as a proxy for "no catalog +opinion about tools". Those are different statements, and the difference was +invisible while the branch was dead. + +**Amendment:** drop the `catalogRow === undefined` guard. The adapter signal is +positive evidence about the protocol and does not become false when a row +exists. The catalog's `capabilities` list stays a positive-only signal exactly +as its comment already states. + +## B2 (High) — ACCEPTED. Synthesized catalog defaults are not evidence + +`ensureStrictCatalogFields()` manufactures values for Codex's strict parser: + +- `src/codex/catalog/parsing.ts:315-317` — absent modalities become `["text"]`. +- `src/codex/catalog/parsing.ts:328` — absent context becomes `128000`. + +```ts + const contextWindow = typeof entry.context_window === "number" && entry.context_window > 0 ? entry.context_window : 128000; +``` + +Reading those back as routing evidence converts *unknown* into a confident +negative (`image: false`) and a fabricated `128000`. That is a direct violation +of the module's own header contract at `src/routing/capability.ts:8-10`, and it +would change eligibility decisions in `src/routing/evaluator.ts`. + +This is the most valuable finding of the round: the naive fix would have +produced *wrong* evidence, which is worse than the current *missing* evidence. + +**Amendment:** do not infer from the compatibility-shaped fields. Serialize +explicit provenance when the catalog is written and read only that. The +repository already carries `opencodex_*` extension keys through the same +writer (`opencodex_catalog_kind`, `src/codex/catalog/sync.ts:371`), so this +follows an established pattern rather than inventing one. + +## B3 (High) — ACCEPTED. Phase 020 could not ingest the real payload + +`extractProviderModelItems()` reads only `data` envelopes or top-level arrays +(`src/providers/model-discovery.ts:337-343`), and the comment is explicit that a +stray `models` key must not be trusted. The observed llama.cpp body splits the +evidence: `capabilities:["completion","multimodal"]` lives in `models[]`, while +`meta.n_ctx` lives in `data[]`. So the surviving item carries context but no +modality, and my proposed test invented a merged item the parser never builds. + +**Amendment:** Phase 020 is re-scoped. Ingesting `meta.n_ctx` from the `data[]` +item is kept — it is correct and independently useful. Cross-envelope merging of +`models[]` into `data[]` is NOT adopted in this unit: it changes a deliberately +conservative discovery boundary, and the existing comment shows that +conservatism is intentional. It becomes a filed issue with the verbatim payload +instead. + +## B4 (Medium) — ACCEPTED with a narrower fix + +`routedSlug("p","a/b")` and `routedSlug("p","a-b")` both yield `p/a-b`, so a +`find()` on slug equivalence can attach one model's evidence to another. Rare, +but silent and wrong when it happens. + +**Amendment (final form):** the provenance block from B2 carries the exact +native `provider`/`model_id`, so the existing equality lookup is KEPT and no +slug fallback exists at all. The collision path is removed structurally +rather than mitigated, and no `slug-codec` import is added. See +`010_catalog_row_shape.md` section 2c. + +## B5 (Medium) — ACCEPTED + +My precedence tests passed unchanged today, so they proved nothing about the +patch. **Amendment (final form):** because B3 removed multimodal ingestion +from this unit, the rewritten tests are context-only. The contested case is +now `context_length: 32768` against `meta.n_ctx: 8192`, and audit round 2 +measured the real before/after matrix into `020_live_capability_ingestion.md`. + +## B6 (Medium) — ACCEPTED + +The canonical suite command is `bun run test` (`package.json:41`); bare +`bun test` fails `tests/test-home-guard.test.ts` because it bypasses the +wrapper's `OPENCODEX_HOME`. The reviewer also verified no `lidge` checkout holds +this head. **Amendment:** the C phase pushes the branch first and verifies the +remote `HEAD` matches before running `bun run test` there. + +## B7 (Medium) — ACCEPTED + +`cat` proves bytes, not instruction loading. **Amendment:** Phase 030's +acceptance is qualified to the default Codex home, notes `AGENTS.override.md` +precedence and `$CODEX_HOME`, and is honestly labeled human-verified. + +## B8 (Low) — ACCEPTED + +`ProviderModelsApiItem = Record & { id: string }` +(`src/providers/model-discovery.ts:33`) already permits `item.meta`. The +proposed type edit is removed from the change map. + +## B9 (Low) — ACCEPTED + +The live `lidge` provider now carries `modelContextWindows` and +`modelInputModalities` (added by hand while diagnosing), so the earlier +branches win and the original one-line reproduction no longer reproduces. The +17-to-0 catalog proof is independent and stands. **Amendment:** `001` states +that the reproduction requires a catalog-only fixture. + +## Nothing rebutted + +All nine findings are accepted. Two — B1 and B2 — would have shipped a +regression affecting every provider, not just the local model that started this +investigation. + +## Round 2 outcome + +VERDICT: GO-WITH-FIXES (blockers=4), all Medium/Low plan-precision items, +folded above and into the phase docs. The three High blockers are cleared. The +strict-parser risk flagged as the largest remaining unknown was cleared +empirically: an object-valued unknown key returned EXIT=0 on both Codex CLI +0.146.0 and the plugin app-server 0.148.0-alpha.9, while an invalid known +modality returned EXIT=1 — so the parser rejects bad enum values, not unknown +keys. diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_8.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_8.md new file mode 100644 index 0000000000..ae09267f5b --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_8.md @@ -0,0 +1,127 @@ +# 004 — Audit rounds 2-8: verdict ledger + +Eight audit rounds ran against this plan. `003_audit_synthesis_round1.md` covers +round 1 in detail; this file records the rest and the mechanical caveat about +how the verdicts were captured. + +## Round ledger + +| Round | Reviewer | Verdict | Outcome | +|-------|----------|---------|---------| +| 1 | explorer A (gpt-5.6-sol, medium) | FAIL, 9 blockers (3 High) | Design pivoted to a provenance channel | +| 2 | explorer A | GO-WITH-FIXES, 4 (Medium/Low) | Strict-parser risk cleared empirically | +| 3 | explorer A | NEAR-PASS, 1 Medium | Remote verifier rewritten as a scratch clone | +| 4 | explorer B (fresh, gpt-5.6-sol, medium) | FAIL, 1 High + 2 Medium | Combo synthesizer + missing issues found | +| 5 | explorer B | FAIL, 2 High | Antigravity synthesizer found; remote steps not fail-closed | +| 6 | explorer B | NEAR-PASS, 1 Medium + 1 Low | Step self-containment; scope boundary | + +Every finding across all eight rounds was ACCEPTED and folded. None were rebutted. + +## What the audit actually prevented + +Three defects would have shipped without it, each invisible to the test suite: + +1. **Tool support silently revoked** (round 1). Repairing the catalog lookup + would have armed the `catalogRow === undefined` guard at + `src/routing/capability.ts:178`, removing `tools: true` from every + openai-chat and anthropic candidate. +2. **Synthesized defaults presented as fact** (round 1). Reading + `context_window`/`input_modalities` directly would have converted unknown + into `image: false` and a fabricated `128000`. +3. **Two synthesizers labeled as provenance** (rounds 4-5). The combo path + (`provider-fetch.ts:697/793` -> `aggregation.ts:164`) and the Antigravity + producer (`antigravity-models.ts:334`) both manufacture values from + defaults. The second carries a real provider name, so the first guard alone + was insufficient. + +Round 6 traced eight CatalogModel-producing paths and found no third +synthesizer: configured metadata (`provider-fetch.ts:1100`), Antigravity (1338), +live discovery (1372), native combo injection (1698), custom models (1786), +trusted OpenAI rows (1904), jawcode metadata (1963), and combo derivation +(`aggregation.ts:164`). Cursor's defaults stay inside its static catalog +(`src/adapters/cursor/discovery.ts:172`, `src/providers/registry.ts:961`). + +## Pre-existing remote failures (round 6) + +A full `bun run test` at this unit's head on the remote host produced: + + 12299 pass, 11 skip, 7 fail, 7 errors + +All seven are missing `react` / `react/jsx-dev-runtime` in the scratch clone, +and the same seven files reproduce on a fresh `dev` checkout. No failing file +touches this unit's catalog, routing, or Antigravity surfaces. C must still +reach its own verified run rather than inheriting this one. + +## Verdict-capture caveat (honest record) + +`cxc review-round` records a verdict through a `SubagentStop` hook matching +`^explorer$`, which fires for plugin thread-spawned children. The reviewers here +were dispatched through the host's `multi_agent_v1` surface, so the hook never +observed their exit and rounds r1-r4 stayed `in_flight` despite real reviewer +exits carrying the required `LAUNCH:`/`VERDICT:` lines. + +This is a transport mismatch, not a missing audit. The verbatim verdict lines, +the blockers, and the path:line evidence are recorded in this file and in `003`, +and the A->B attestation carries the pasted reviewer tail. Anyone re-verifying +should read the reviewer output quoted here rather than the round status. +## Rounds 7-8 (fresh reviewer C) + +| Round | Verdict | Finding | +|-------|---------|---------| +| 7 | FAIL, 1 High + 1 Medium | `applyCatalogMetadata` is a SECOND writer of real values that the provenance stamp could not see | +| 8 | NEAR-PASS, 2 Medium + 1 Low | Extraction scope, missing context-cap regression, malformed table row | + +Round 7 is the most important finding of the whole audit after round 1's B2. +Rounds 4-6 audited **producers** — paths that manufacture a `CatalogModel` — and +correctly concluded there were exactly two synthesizers. Reviewer C inverted the +lens and audited **writers** — anything that writes `context_window` or +`input_modalities` onto an entry — and immediately found +`applyCatalogMetadata` (`src/codex/catalog/parsing.ts:458`), which writes REAL +values from the generated jawcode metadata table without ever touching a +`CatalogModel`. A stamp reading `model.*` alone would have carried identity only +for every provider that depends on that table: + + catalogModel: { "provider": "opencode-go", "id": "grok-4.6" } + serialized: { "context_window": 500000, "input_modalities": ["text","image"] } + +Auditing one direction exhaustively is not the same as auditing the other. That +is the transferable lesson from this unit. + +Round 8's writer sweep then closed the question: the complete set of assignments +is `effort.ts:126,134` (CatalogModel), `parsing.ts:466,471` (generated metadata), +`parsing.ts:277,290` (native-only overrides), and `parsing.ts:316,326,329` +(strict-parser defaults). No third real-value writer exists. An independent +sweep from the main agent reproduced exactly that list. + +Round 8 also caught that the two new regressions omitted `contextCap`, so an +implementation stamping an uncapped `500000` against a capped `350000` entry +would have passed. A capped-metadata regression was added, carried through +`candidateCapabilityEvidence` so the value is asserted end to end. + +## Final tally + +Eight rounds, three independent reviewers, 22 findings. Every one accepted and +folded; none rebutted. Four would have shipped real defects: + +1. Tool support silently revoked for openai-chat and anthropic (round 1). +2. Synthesized strict-parser defaults read as routing evidence (round 1). +3. Two synthesizers stamped as provenance — combo and Antigravity (rounds 4-5). +4. Real generated metadata invisible to provenance (round 7). + +## Verdict-capture caveat, updated + +The `SubagentStop` review observer requires `payload.agent_type === "explorer"` +(`components/pabcd-state/dist/review-observer.js:32`). Two things were checked +here: + +- `cxc doctor` reported the observer hook UNTRUSTED, so it could not have run at + all. `cxc hooks retrust` fixed that — 22 hooks now trusted. +- Even after retrust, the rounds stayed `in_flight`. The session rollout shows + `SubagentStop` events firing (15 occurrences) but carries no `agent_type` + field, so the observer's first guard returns early. + +The audit itself is unaffected: the reviewers really ran, really produced +`LAUNCH:`/`VERDICT:` lines bound to the issued launch ids, and their verbatim +output is recorded here and in the A->B attestation. What is missing is the +machine-recorded verdict, not the audit. This is worth reporting upstream as a +host-surface gap in the observer's payload contract. diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/010_catalog_row_shape.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/010_catalog_row_shape.md new file mode 100644 index 0000000000..006db8918b --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/010_catalog_row_shape.md @@ -0,0 +1,610 @@ +# 010 — Phase 1: give the catalog explicit capability provenance + +Diff-level implementation doc. Research: `001_capability_evidence_defect.md`. +**Revised after audit round 1** — see `003_audit_synthesis_round1.md`. The first +draft proposed reading the catalog's `context_window`/`input_modalities` +directly. That was rejected: those fields are synthesized for Codex's strict +parser, so reading them turns unknown into a false negative (B2), and matching +a row disarms the tool-capability fallback (B1). + +## Goal + +A model whose only evidence source is the catalog must carry its REAL +contextWindow and image evidence — and only when that evidence is real. A +synthesized compatibility default must stay unknown, and no dimension that is +correct today may regress. + +## Why not read context_window / input_modalities + +`ensureStrictCatalogFields()` fills those fields so Codex's parser accepts the +file, whether or not any provider asserted them: + + // src/codex/catalog/parsing.ts:315 + if (!Array.isArray(entry.input_modalities) && !options.preserveExactInputModalities) { + entry.input_modalities = ["text"]; + } + // src/codex/catalog/parsing.ts:328 + const contextWindow = typeof entry.context_window === "number" && entry.context_window > 0 ? entry.context_window : 128000; + +Every row therefore has both fields, and their presence says nothing about what +is known. Routing must distinguish "the provider said text-only" from "nobody +said anything", so it needs a separate channel. + +## Scope boundary + +IN: the provenance stamp in `src/codex/catalog/effort.ts` (sourcing both the +CatalogModel and the jawcode generated-metadata lookup), the reader in +`src/routing/capability.ts`, the shared generated-metadata lookup export in +`src/codex/catalog/parsing.ts`, the `supportsImages` tri-state restoration in +`src/providers/antigravity-models.ts` (added after audit round 5), and the +focused tests including the two writer-through-normalizer regressions. +OUT: the evidence priority order, the memoization strategy, the policy +evaluator, reasoning-effort ingestion, and every other module. + +## File change map + +| Path | Action | What | +|------|--------|------| +| `src/codex/catalog/effort.ts` | MODIFY | Stamp `opencodex_capability_provenance` for non-combo rows only | +| `src/providers/antigravity-models.ts` | MODIFY | Restore the supportsImages tri-state (absent != false) | +| `src/codex/catalog/parsing.ts` | MODIFY | Export the generated-metadata lookup so `effort.ts` shares it instead of duplicating | +| `src/routing/capability.ts` | MODIFY | Read the provenance block; make the adapter tool fallback unconditional | +| `tests/routing-capability-catalog.test.ts` | NEW | Real evidence survives; synthesized defaults stay unknown; tools never regresses | + +## MODIFY 1 — src/codex/catalog/effort.ts + +`applyCatalogModelMetadata()` writes exclusively inside guarded blocks that test +the `CatalogModel`'s own fields, which makes it the right place to stamp. It is +NOT the only writer of real values, though — see "Capturing jawcode generated +metadata too" below, which is why the stamp reads two sources rather than one. + +Existing shape (unchanged): + + if (typeof model.contextWindow === "number" && model.contextWindow > 0) { + entry.context_window = model.contextWindow; + entry.max_context_window = model.contextWindow; + ... + } + if (Array.isArray(model.inputModalities) && model.inputModalities.length > 0) { + entry.input_modalities = model.inputModalities; + } + +Added at the end of the function — **SUPERSEDED, kept for provenance of the +design's evolution.** This first form stamped only `CatalogModel` fields, with no +combo guard, no generated-metadata fallback, and no context cap. Audit rounds 4, +5 and 7 each proved it insufficient. The single authoritative algorithm is +"Final stamp algorithm" below; do not implement from this snippet. + + const provenance: Record = { provider: model.provider, model_id: model.id }; + if (typeof model.contextWindow === "number" && model.contextWindow > 0) { + provenance.context_window = model.contextWindow; + } + if (Array.isArray(model.inputModalities) && model.inputModalities.length > 0) { + provenance.input_modalities = model.inputModalities; + } + entry.opencodex_capability_provenance = provenance; + +### Final stamp algorithm (authoritative — this is what shipped) + +One algorithm, covering all three audit corrections: skip synthesized combo rows +(round 5), read both real evidence sources (round 7), and apply the context cap +(round 8). Verbatim from `src/codex/catalog/effort.ts`: + + function stampCapabilityProvenance(entry: RawEntry, model: CatalogModel): void { + // Virtual combo rows are synthesized from last-resort defaults (a generic + // 128k context and a ["text"] modality), so their values are placeholders + // rather than assertions. Stamping them would reintroduce the exact + // false-evidence defect this block exists to prevent. + if (model.provider === COMBO_NAMESPACE) return; + + const meta = generatedModelMetadata(model.provider, model.id); + const metaContext = typeof meta?.contextWindow === "number" && meta.contextWindow > 0 + // The generated context is capped before it reaches the entry, so + // provenance must apply the same cap or routing would advertise a + // window the cap refused. + ? applyProviderContextCap(meta.contextWindow, model.contextCap) ?? meta.contextWindow + : undefined; + const contextWindow = typeof model.contextWindow === "number" && model.contextWindow > 0 + ? model.contextWindow + : metaContext; + const inputModalities = Array.isArray(model.inputModalities) && model.inputModalities.length > 0 + ? model.inputModalities + : (Array.isArray(meta?.input) && meta.input.length > 0 ? meta.input : undefined); + + entry.opencodex_capability_provenance = { + provider: model.provider, + model_id: model.id, + ...(contextWindow !== undefined ? { context_window: contextWindow } : {}), + ...(inputModalities !== undefined ? { input_modalities: [...inputModalities] } : {}), + ...(Array.isArray(model.capabilities) && model.capabilities.length > 0 + ? { capabilities: [...model.capabilities] } + : {}), + }; + } + +`provider`/`model_id` are always stamped: they are the exact-identity match +that closes the slug-collision hole (B4). + +B must confirm the key survives `ensureStrictCatalogFields` and +`normalizeServiceTiers` (neither strips unknown keys today — +`opencodex_catalog_kind` already depends on this, src/codex/catalog/sync.ts:371) +and that Codex's strict parse accepts an extra object-valued key. If it does +not, the fallback is a flat JSON string under the same prefix; B records which +was used. + +### Excluding synthesized combo rows (round-4 B2) + +The claim that `applyCatalogModelMetadata()` only ever sees real assertions is +FALSE as written, and audit round 4 proved it. The combo synthesis path builds a +`CatalogModel` out of last-resort defaults and feeds it through the same +function: + +- `src/codex/catalog/provider-fetch.ts:697` — a synthetic `128000` context is the + documented final fallback for combo member synthesis. +- `src/codex/catalog/provider-fetch.ts:793` — that fallback plus a synthesized + `["text"]` modality is applied. +- `src/codex/catalog/aggregation.ts:164` — the result becomes an ordinary + `CatalogModel` with `provider: COMBO_NAMESPACE`. + +Reviewer reproduction: + + MEMBER={"id":"unknown","provider":"demo",...,"inputModalities":["text"],"contextWindow":128000} + DERIVED={"provider":"combo","id":"synthetic",...,"contextWindow":128000,"inputModalities":["text"]} + +Stamping that as provenance would reintroduce exactly the B2 defect the whole +redesign exists to avoid — a synthesized `128000` and a synthesized `["text"]` +presented to routing as asserted fact. + +**Amendment.** Do not stamp synthesized combo rows. The function already tests +this namespace on its first line (`src/codex/catalog/effort.ts:117`), so the +guard is a one-line reuse of an existing check: + + // Virtual combo rows are synthesized from last-resort defaults + // (provider-fetch.ts:697/793 -> aggregation.ts:164), so their context and + // modality values are placeholders, not provider assertions. Stamping them + // would recreate the exact false-evidence defect this block exists to + // prevent. Combos are not ordinary routing candidates, so skipping them + // costs nothing. + if (model.provider !== COMBO_NAMESPACE) { + const provenance: Record = { provider: model.provider, model_id: model.id }; + ... + entry.opencodex_capability_provenance = provenance; + } + +**Required regression (writer-through-normalizer).** The consumer-only tests +proposed earlier cannot catch this, because they hand-write catalog rows. B adds +a test that drives the real combo synthesis path end to end and asserts the +emitted entry carries NO `opencodex_capability_provenance`: + + test("synthesized combo rows are not stamped with capability provenance", () => { + // Drives provider-fetch synthesis -> aggregation -> applyCatalogModelMetadata, + // rather than hand-writing a row, so the writer itself is under test. + const entry = buildComboCatalogEntry(/* member with no asserted context/modalities */); + expect(entry.opencodex_capability_provenance).toBeUndefined(); + expect(entry.context_window).toBe(128000); // the synthesized default still ships to Codex + }); + +The second assertion matters: the synthesized value must keep reaching Codex's +catalog (it is what makes the row parse), while staying invisible to routing. +That split is the whole point of a separate provenance channel. + +**Residual, stated honestly.** This guard covers the one synthesized producer +found by audit. Any future path that manufactures a `CatalogModel` from defaults +would need the same treatment; the regression test is an early warning for that +class, not a proof that no other producer exists. + +### Capturing jawcode generated metadata too (round-7 B1) + +`applyCatalogModelMetadata()` is NOT the only writer of real capability values, +and a stamp built from `model.*` alone silently drops a whole class of correct +evidence. Audit round 7 found the second writer: + + // src/codex/catalog/sync.ts:321-322 — order matters + if (model) applyCatalogMetadata(e, model.provider, model.id, model.contextCap); + applyCatalogModelMetadata(e, model); + +`applyCatalogMetadata()` (`src/codex/catalog/parsing.ts:458`) looks the model up +in the generated jawcode metadata and writes `context_window` / +`input_modalities` from it. Those values are REAL assertions — they come from a +curated metadata table, not from `ensureStrictCatalogFields`. But they never +touch the `CatalogModel`, so a stamp reading only `model.*` cannot see them. + +Reviewer reproduction — a live-discovered row carrying identity only, whose +serialized entry nonetheless has full metadata: + + catalogModel: { "provider": "opencode-go", "id": "grok-4.6" } + serialized: { "context_window": 500000, "input_modalities": ["text","image"] } + +Under the previous design the provenance block would carry identity and nothing +else, and routing would still lose valid evidence — defeating the phase goal for +exactly the providers that rely on generated metadata. + +**Amendment.** Stamp provenance AFTER both writers have run, and source each +field from the model when it asserted one, otherwise from the metadata lookup. +Never from the entry itself: reading `entry.context_window` back would +reintroduce the B2 defect the moment `ensureStrictCatalogFields` has run. + + // Both real-assertion writers must have run before this point: + // applyCatalogMetadata() — jawcode generated metadata (parsing.ts:458) + // applyCatalogModelMetadata() — the CatalogModel's own fields + // Read from those two SOURCES, never from `entry`: the entry also carries + // ensureStrictCatalogFields' compatibility defaults, which are not evidence. + const meta = lookupGeneratedMetadata(model.provider, model.id); // same lookup parsing.ts:458 uses + const assertedContext = (typeof model.contextWindow === "number" && model.contextWindow > 0) + ? model.contextWindow + : (typeof meta?.contextWindow === "number" && meta.contextWindow > 0 ? meta.contextWindow : undefined); + const assertedModalities = (Array.isArray(model.inputModalities) && model.inputModalities.length > 0) + ? model.inputModalities + : (Array.isArray(meta?.input) && meta.input.length > 0 ? meta.input : undefined); + +Precedence matches the existing write order: the `CatalogModel` wins where it +asserts, generated metadata fills the rest. + +The lookup currently lives inside `applyCatalogMetadata` and is not exported +(`src/codex/catalog/parsing.ts:458-463`); `effort.ts` imports only `readCatalog` +and types from that module (`src/codex/catalog/effort.ts:34-35`). So "extract, +not duplicate" is a real edit to `parsing.ts`, not a free choice: export the +resolve-and-fetch step as a named helper and have both callers use it. That is +why `parsing.ts` is in the file map and the scope boundary above. + +Context-cap note: `applyCatalogMetadata` passes the metadata context through +`applyProviderContextCap(meta.contextWindow, contextCap)`. Provenance must apply +the same cap, or routing would advertise a window the cap already refused. B +confirms the cap argument reaching this point. + +**Required regression (writer-to-reader).** The consumer-only fixtures cannot +catch this either: + + test("provenance captures jawcode generated metadata for an identity-only model", () => { + // The CatalogModel carries provider/id ONLY; context and modalities come + // from the generated metadata table via applyCatalogMetadata. + const entry = buildRoutedEntry({ provider: "opencode-go", id: "grok-4.6" }); + expect(entry.context_window).toBe(500000); + expect(entry.opencodex_capability_provenance.context_window).toBe(500000); + expect(entry.opencodex_capability_provenance.input_modalities).toEqual(["text", "image"]); + }); + + test("provenance carries the CAPPED metadata context, not the raw table value", () => { + // applyCatalogMetadata pipes generated context through applyProviderContextCap + // (parsing.ts:464-466). An implementation that stamps the uncapped table value + // would still pass the test above while advertising a window the cap refused. + const entry = buildRoutedEntry({ provider: "opencode-go", id: "grok-4.6", contextCap: 350000 }); + expect(entry.context_window).toBe(350000); + expect(entry.opencodex_capability_provenance.context_window).toBe(350000); + // And the same value must survive all the way to routing evidence. + expect(candidateCapabilityEvidence(config, "opencode-go", "grok-4.6").contextWindow).toBe(350000); + }); + + test("a model with no assertion anywhere stamps identity only", () => { + // Neither the CatalogModel nor generated metadata asserts anything, so the + // strict default still ships to Codex while provenance stays silent. + const entry = buildRoutedEntry({ provider: "demo", id: "unknown-model" }); + expect(entry.context_window).toBe(128000); + expect(entry.opencodex_capability_provenance.context_window).toBeUndefined(); + }); + +**Why six rounds missed it.** Rounds 4-6 audited *producers* — code paths that +manufacture a `CatalogModel` — and correctly found two synthesizers. This defect +is the mirror image: a writer that supplies real values without going through a +`CatalogModel` at all. Auditing one direction thoroughly is not the same as +auditing the other. + +### Restoring the tri-state in the Antigravity producer (round-5 B2) + +The combo guard above is necessary but NOT sufficient. Audit round 5 found a +second synthesizer that carries a real provider name, so it walks straight past +a `COMBO_NAMESPACE` check: + + // src/providers/antigravity-models.ts:334 + inputModalities: info.supportsImages === true ? ["text", "image"] : ["text"], + +That ternary collapses two different facts into one value. `supportsImages: +false` (the provider said no) and `supportsImages` absent (nobody said anything) +both become `["text"]`. `src/codex/catalog/provider-fetch.ts:1338` then turns the +row into an ordinary `CatalogModel` with `provider: name`, and +`src/routing/capability.ts:163` reads `["text"]` as `image: false`. + +Reviewer reproduction, with no `supportsImages` assertion present: + + [ { "id": "future-agent-model", "contextWindow": 333333, + "inputModalities": ["text"] } ] + +Such rows are not hypothetical: `tests/google-antigravity-wire.test.ts:131` +already constructs models without the field. + +**Amendment.** Preserve the tri-state at the producer, which is the only place +the distinction still exists: + + // Tri-state, deliberately not a ternary: `true` is an assertion of image + // support, `false` is an assertion against it, and ABSENT is unknown. + // Collapsing absent into ["text"] would let routing read it as a confident + // image:false (src/routing/capability.ts:163). The strict catalog still + // receives its ["text"] compatibility default downstream via + // ensureStrictCatalogFields; only the routing-evidence channel stays honest. + ...(info.supportsImages === true + ? { inputModalities: ["text", "image"] } + : info.supportsImages === false + ? { inputModalities: ["text"] } + : {}), + +**Required regression (writer-through-normalizer).** Same class as the combo +test, and equally uncatchable by consumer-only fixtures: + + test("an Antigravity model with no supportsImages leaves modality unknown", () => { + const rows = parseAntigravityAvailableModels(/* wire payload without supportsImages */); + expect(rows[0].inputModalities).toBeUndefined(); + // The strict catalog still gets its compatibility default... + const entry = buildCatalogEntry(rows[0]); + expect(entry.input_modalities).toEqual(["text"]); + // ...but provenance carries no modality claim, so routing stays unknown. + expect(entry.opencodex_capability_provenance.input_modalities).toBeUndefined(); + }); + + test("an explicit supportsImages:false still asserts text-only", () => { + const rows = parseAntigravityAvailableModels(/* payload with supportsImages: false */); + expect(rows[0].inputModalities).toEqual(["text"]); + }); + +The second test is what keeps this a tri-state restoration rather than a +silent weakening: a provider that genuinely says "no images" must keep saying it. + +**Scope addition.** `src/providers/antigravity-models.ts` joins Phase 1's file +change map for this reason. + +**Residual, restated.** Audit found two synthesizers (combo, Antigravity). The +guard plus the tri-state cover both. Any future producer that manufactures a +`CatalogModel` field from a default would need the same treatment; the two +writer-through-normalizer tests are the early warning for that class, not proof +that no third producer exists. B greps for other `inputModalities:` and +`contextWindow:` literal assignments in producer paths before closing. + +## MODIFY 2 — src/routing/capability.ts + +### 2a. Row type + +Before: + + type CatalogModelRow = { + provider: string; + id: string; + contextWindow?: number; + inputModalities?: string[]; + reasoningEfforts?: string[]; + capabilities?: string[]; + }; + +After: + + type CatalogModelRow = { + /** Exact provider/native-id identity, from the provenance block. */ + provider: string; + id: string; + /** Only values a CatalogModel asserted; never a strict-parser default. */ + contextWindow?: number; + inputModalities?: string[]; + capabilities?: string[]; + }; + +`reasoningEfforts` is dropped: the catalog writes `supported_reasoning_levels` +as `{effort, description}` objects and this unit adds no mapping. Its absence +is today's behavior, so nothing regresses. + +### 2b. Projection — read the provenance block + +Before (the filter that discards all 17 rows): + + const rows = models + .filter((model): model is Record & { id: string; provider: string } => + typeof model === "object" && model !== null && typeof model.id === "string" && typeof model.provider === "string") + .map(model => ({ + provider: model.provider, + id: model.id, + ... + +After: + + const rows = models.flatMap(model => { + if (typeof model !== "object" || model === null) return []; + const provenance = (model as Record).opencodex_capability_provenance; + if (typeof provenance !== "object" || provenance === null) return []; + const p = provenance as Record; + if (typeof p.provider !== "string" || typeof p.model_id !== "string") return []; + return [{ + provider: p.provider, + id: p.model_id, + ...(typeof p.context_window === "number" && p.context_window > 0 + ? { contextWindow: p.context_window } + : {}), + ...(Array.isArray(p.input_modalities) + ? { inputModalities: p.input_modalities.filter((value): value is string => typeof value === "string") } + : {}), + ...(Array.isArray(p.capabilities) + ? { capabilities: p.capabilities.filter((value): value is string => typeof value === "string") } + : {}), + }]; + }); + +A row without provenance contributes nothing — exactly today's behavior for +every row — so this can only add evidence, never remove it. + +### 2c. Lookup — unchanged + + const catalogRow = cachedCatalogModels().find(model => model.provider === providerName && model.id === modelId); + +The provenance block stores the exact native provider/model_id, so the existing +equality lookup is already correct. No slug decoding, no new import, and the B4 +collision risk disappears rather than being mitigated. + +### 2d. Keep the adapter tool fallback unconditional (B1) + +Before: + + const tools = capabilities.includes("tools") + || isNative + || (catalogRow === undefined && provider !== undefined && TOOL_CAPABLE_ADAPTERS.has(provider.adapter)) + || provider?.parallelToolCalls === true + || undefined; + +After: + + const tools = capabilities.includes("tools") + || isNative + // The adapter protocol is positive evidence on its own. This was gated on + // `catalogRow === undefined`, which was safe only while the catalog lookup + // never matched: once it matches, a row that simply does not enumerate + // "tools" would silently revoke tool support for every openai-chat and + // anthropic candidate. + || (provider !== undefined && TOOL_CAPABLE_ADAPTERS.has(provider.adapter)) + || provider?.parallelToolCalls === true + || undefined; + +A strict widening of a positive signal. `capabilities` stays positive-only, so +nothing can turn `tools` false. + +### Import-boundary check + +No new import is added, so the import graph is unchanged. +`tests/core-lab-boundary.test.ts` (13 pass pre-change) must still be re-run. + +## NEW tests/routing-capability-catalog.test.ts + +Writes a temporary catalog file and points the catalog path at it. B confirms +the path-override mechanism used by existing catalog tests before writing, and +amends this doc if it differs. + + test("carries asserted context window and image modality from a catalog-only model", () => { + // Provider config declares NO modelContextWindows / modelInputModalities, + // so the provenance block is the only possible evidence source. + const evidence = candidateCapabilityEvidence(config, "lidge", "qwen3.8-27b-nvfp4"); + expect(evidence.contextWindow).toBe(262144); + expect(evidence.image).toBe(true); + }); + + test("a synthesized strict-parser default stays unknown (B2)", () => { + // Row written with context_window 128000 and input_modalities ["text"] + // by ensureStrictCatalogFields, but no provenance for either field. + const evidence = candidateCapabilityEvidence(config, "demo", "unknown-model"); + expect(evidence.contextWindow).toBeUndefined(); + expect(evidence.image).toBeUndefined(); // never false + }); + + test("matching a catalog row does not revoke adapter tool support (B1)", () => { + const evidence = candidateCapabilityEvidence(config, "lidge", "qwen3.8-27b-nvfp4"); + expect(evidence.tools).toBe(true); + }); + + test("exact identity is not confused by slug collision (B4)", () => { + // Native ids "a/b" and "a-b" both encode to the slug "p/a-b". + expect(candidateCapabilityEvidence(config, "p", "a/b").contextWindow).toBe(111000); + expect(candidateCapabilityEvidence(config, "p", "a-b").contextWindow).toBe(222000); + }); + +## Accept criteria + +1. Test 1 FAILS on the current tree and PASSES after the change; both runs + recorded (C-ACTIVATION-GROUNDING-01). +2. Test 2 asserts undefined, never false — the B2 contract. +3. Test 3 passes before AND after: it proves the fix does not introduce the + regression the audit predicted. +4. `bun x tsc --noEmit` clean. +5. `tests/core-lab-boundary.test.ts` green. +6. Remote exact-head suite green on lidge (command in the section below). + +## Verifier commands (PLAN-VERIFIER-REAL-01) + +| Command | Reads this change? | Notes | +|---------|-------------------|-------| +| `bun run test tests/routing-capability-catalog.test.ts` | YES — the file under test is the direct argument | Bare `bun test` bypasses the wrapper and fails test-home-guard (B6) | +| `bun x tsc --noEmit` | YES — tsconfig include covers `src/**` | Verified exit 0 pre-change | +| `bun run test tests/core-lab-boundary.test.ts` | YES — walks the import graph from `src/router.ts` into `src/routing/capability.ts` | Verified 13 pass pre-change | +| Remote exact-head suite (command below) | YES — shared routing surface | Required: shared surface | + +## Field chain (PLAN-FIELD-CHAIN-01) + +| Stage | Path | State after this phase | +|-------|------|------------------------| +| creation | `src/cli/models.ts` (`ocx models add`) / provider discovery | unchanged | +| serialization | `applyCatalogModelMetadata`, src/codex/catalog/effort.ts:113 | NEW provenance key | +| deserialization | `cachedCatalogModels`, src/routing/capability.ts:45 | reads provenance only | +| consumers | `candidateCapabilityEvidence` -> `src/routing/evaluator.ts` | receives real evidence; unknown stays unknown | + +No other consumer reads `CatalogModelRow`: it is a module-local type +(src/routing/capability.ts:28) with no export. + +## Bypass record (PLAN-BYPASS-NAMED-01) + +No enforcement added; this repairs a data path. Tier: N/A. Executing surface: +none. Known bypass: a CatalogModel carrying no context/modality still yields +unknown evidence — by design. Residual risk: provenance is written by exactly +one function, so a future writer bypassing `applyCatalogModelMetadata` would +produce rows routing cannot read. The new tests are the early warning, not +enforcement. Final enforcement layer: none. + +## Remote exact-head suite (round-5 correction) + +Five rounds of audit produced five distinct ways a remote command can lie. The +final form separates PUBLISH from VERIFY, and every step fails closed. + +**Step 1 — publish (separate, must succeed on its own).** + + set -eu + BRANCH=$(git rev-parse --abbrev-ref HEAD) + LOCAL_SHA=$(git rev-parse HEAD) + git push --no-verify csa906 "$BRANCH" + +Round 5 observed this step fail with `! [remote rejected] ... (permission denied)` +while the verification that followed still ran and could have reported success. +Publication is therefore its own command whose exit code is checked before +anything else happens. + +**Step 2 — assert the remote actually has this commit.** + + set -eu + BRANCH=$(git rev-parse --abbrev-ref HEAD) + LOCAL_SHA=$(git rev-parse HEAD) + REMOTE_SHA=$(git ls-remote csa906 "refs/heads/$BRANCH" | cut -f1) + test -n "$REMOTE_SHA" + test "$REMOTE_SHA" = "$LOCAL_SHA" + +`test -n` matters independently: a failed push leaves `REMOTE_SHA` EMPTY, and an +empty-vs-empty comparison would otherwise pass. + +**Step 3 — verify in an isolated scratch clone.** + + LOCAL_SHA=$(git rev-parse HEAD) + ssh lidge "set -eu + export PATH=\"\$HOME/.bun/bin:\$PATH\" + command -v bun >/dev/null + WORKDIR=\$(mktemp -d -t ocx-verify-XXXXXX) + trap 'rm -rf \"\$WORKDIR\"' EXIT + git clone --quiet --no-checkout https://github.com/csa906/opencodex.git \"\$WORKDIR\" + cd \"\$WORKDIR\" + git fetch --quiet origin $LOCAL_SHA + git checkout --quiet --detach $LOCAL_SHA + test \"\$(git rev-parse HEAD)\" = \"$LOCAL_SHA\" + bun install --frozen-lockfile + bun run test" + +Each guard exists because a specific failure was observed in audit: + +| Guard | Observed failure it prevents | +|-------|------------------------------| +| Steps split + `set -eu` | round 5: push was rejected and `ls-remote` returned empty, yet the suite still ran and could have reported success | +| `test -n "$REMOTE_SHA"` | an empty remote SHA comparing equal to an empty string | +| `git ls-remote` SHA equality | `fatal: remote error: upload-pack: not our ref 60fd5a7d9...` on an unpushed commit | +| `export PATH` + `command -v bun` | `command -v bun` empty over non-interactive ssh while `/home/lidgeai/.bun/bin/bun` exists | +| `mktemp -d` + `trap` cleanup | `~/ocx-ci/opencodex` had modified `src/bridge.ts`, `src/server/responses/core.ts`; a run there proves nothing about this change | +| `git rev-parse HEAD` equality | a stale checkout reporting a green suite for different code | + +Each step recomputes `BRANCH`/`LOCAL_SHA` so it is independently runnable in a +fresh shell: audit round 6 showed Step 2 aborting with `BRANCH: parameter not +set` when the variables only existed in Step 1. That failed closed, but a +verifier that only works when pasted in one session is not the verifier the +doc describes. + +C runs these three literal steps in order and pastes each exit code. A green +suite whose publication step failed is not evidence. + +Note: round 5 ran step 3 successfully and recorded `12299 pass, 11 skip, 7 fail` +on an unrelated tree state. C must reach a green run at THIS unit's head, or +triage each failure against `dev` before claiming the phase verified. diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/020_live_capability_ingestion.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/020_live_capability_ingestion.md new file mode 100644 index 0000000000..f4d52ec560 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/020_live_capability_ingestion.md @@ -0,0 +1,284 @@ +# 020 — Phase 2: absorb llama.cpp served context from live discovery + +Diff-level implementation doc. Depends on Phase 1 (`010_catalog_row_shape.md`): +without the provenance channel, anything this phase learns is still invisible to +routing. + +**Re-scoped after audit round 1** — see `003_audit_synthesis_round1.md` (B3). +The first draft assumed one merged model item. The real parser never builds +one, so half the original goal moves to a filed issue. + +## What the server actually returns + +`GET http://100.100.125.116:8081/v1/models` returns a dual-envelope body: + + { "models": [ { "name": "qwen3.8-27b-nvfp4", + "capabilities": ["completion", "multimodal"] } ], + "object": "list", + "data": [ { "id": "qwen3.8-27b-nvfp4", "owned_by": "llamacpp", + "meta": { "n_ctx": 262144, "n_ctx_train": 262144 } } ] } + +The image signal (`multimodal`) is in `models[]`. The context signal +(`meta.n_ctx`) is in `data[]`. + +`extractProviderModelItems()` reads ONLY `data` envelopes or top-level arrays, +and says so deliberately (src/providers/model-discovery.ts:337-343): + + // Together-style top-level /models arrays. Catalog discovery must not treat a stray + // `models` key on openai-chat responses as valid — only `data` envelopes or top-level arrays. + +Verified by running the verbatim payload through it: one surviving item, and +`catalogHintsFromModelsApiItem` returns `{}` for it. + +## Scope decision + +IN: `meta.n_ctx` / `meta.n_ctx_train` as context sources. This is a pure +addition to an existing precedence list, affects only rows that reach the +parser, and is independently useful for every llama.cpp deployment. + +OUT: the two harder halves of the image gap, now tracked as **issue #1797**: + +1. Cross-envelope merging of `models[]` into `data[]`. That relaxes a + deliberately conservative discovery boundary (src/providers/model-discovery.ts:337) + whose comment explains why it refuses a stray `models` key. It needs + identity-safe joining by model id and belongs in its own audited unit. +2. Mapping the `multimodal` capability token to image input. Audit round 4 + showed the first draft of this doc was WRONG to imply the merge alone + would restore image evidence: even a hand-merged item stays image-unknown, + because modelInputModalities (src/codex/catalog/provider-fetch.ts:991) + recognizes only vision / image-input / image_input. + + Reviewer proof: + + catalogHintsFromModelsApiItem("lidge", { + meta: { n_ctx: 262144 }, capabilities: ["completion", "multimodal"] }) + => { "capabilities": ["completion", "multimodal"] } // no inputModalities + +So this phase fixes ONLY the context source. The image gap is fully deferred, +both halves of it, to #1797. + +Also OUT (B8): the `ProviderModelsApiItem` type edit. The declaration is +already `Record & { id: string }` +(src/providers/model-discovery.ts:33), so `item.meta` is permitted with no +change. + +## File change map + +| Path | Action | What | +|------|--------|------| +| `src/codex/catalog/provider-fetch.ts` | MODIFY | Read `meta.n_ctx` / `meta.n_ctx_train` as context sources | +| `tests/catalog-llamacpp-capabilities.test.ts` | NEW | Verbatim-payload and precedence coverage | + +## MODIFY — catalogHintsFromModelsApiItem() + +Before: + + const limits = plainRecord(metadata?.limits); + const contextWindow = + positiveSafeInteger( + limits?.max_context_length, + metadata?.context_length, + item.context_length, + item.context_size, + item.max_model_len, + item.max_context_length, + ); + +After: + + const limits = plainRecord(metadata?.limits); + // llama.cpp reports the served context under `meta`: `n_ctx` is what the + // server was actually started with, `n_ctx_train` the model's trained + // maximum. Prefer the served value — routing must not promise a window the + // running server will refuse. Both come LAST so no provider that already + // supplies a recognized field changes behavior. + const meta = plainRecord(item.meta); + const contextWindow = + positiveSafeInteger( + limits?.max_context_length, + metadata?.context_length, + item.context_length, + item.context_size, + item.max_model_len, + item.max_context_length, + meta?.n_ctx, + meta?.n_ctx_train, + ); + +## NEW tests/catalog-llamacpp-capabilities.test.ts + +Rewritten after B5: the original precedence tests passed unchanged today and +so proved nothing. + + test("absorbs meta.n_ctx from the verbatim llama.cpp data[] item", () => { + // This is the item extractProviderModelItems actually produces from the + // observed dual-envelope body — not a hand-merged one. + const hints = catalogHintsFromModelsApiItem("lidge", { + id: "qwen3.8-27b-nvfp4", + object: "model", + owned_by: "llamacpp", + meta: { n_ctx: 262144, n_ctx_train: 262144 }, + }); + expect(hints.contextWindow).toBe(262144); + }); + + test("prefers the served n_ctx over the trained maximum", () => { + const hints = catalogHintsFromModelsApiItem("lidge", { + id: "short-ctx", + meta: { n_ctx: 8192, n_ctx_train: 262144 }, + }); + expect(hints.contextWindow).toBe(8192); + }); + + test("a recognized context field still wins over meta (precedence)", () => { + // Contested: without the ordering guarantee this could return 8192. + const hints = catalogHintsFromModelsApiItem("lidge", { + id: "both", + context_length: 32768, + meta: { n_ctx: 8192 }, + }); + expect(hints.contextWindow).toBe(32768); + }); + + test("the dual-envelope body still yields no image evidence (documents the gap)", () => { + // models[] carries "multimodal" but discovery reads only data[]. This + // asserts the KNOWN limitation so the filed issue has a live witness and + // a future fix has a test to flip. + const extracted = extractProviderModelItems(VERBATIM_LLAMACPP_BODY, discovery); + const hints = catalogHintsFromModelsApiItem("lidge", extracted.items[0]); + expect(hints.contextWindow).toBe(262144); + expect(hints.inputModalities).toBeUndefined(); + }); + +The fourth test is the honest part: it encodes what this phase does NOT fix. + +## Accept criteria + +1. Measured pre-change matrix (audit round 2 ran these): + + | Test | Before | After | + |------|--------|-------| + | 1 verbatim meta.n_ctx | FAIL (hints `{}`) | PASS | + | 2 served n_ctx over trained | FAIL (hints `{}`) | PASS | + | 3 recognized field wins over meta | PASS already | PASS | + | 4 dual-envelope gap characterization | FAIL (asserts the new context value too) | PASS | + + Test 3 passes today because recognized fields already win while `meta` + is ignored; it guards the ordering against a future reshuffle rather + than proving this change. Tests 1, 2 and 4 are the activation evidence. +2. Test 4 keeps characterizing the surviving image gap after the change: + contextWindow present, inputModalities still undefined. It is the live + witness for #1797 and the test a future fix flips. +3. Issue #1797 is filed and linked before this phase closes (verified with + `gh issue view 1797`). A deferral with no tracking issue is not a + deferral, it is a silent drop. +4. `bun x tsc --noEmit` clean. +5. The targeted suites pass on lidge at the pushed head, and the full suite + runs with only the documented environment failures — `provider-fetch.ts` + is a shared surface touched by many catalog suites, so the full run is + required even though it is not fully green. + + Recorded result (see `040_implementation_record.md`): 12337 tests across + 792 files, 7 fail. All seven are `Cannot find package 'react'` / + `react/jsx-dev-runtime` from `gui/` sources, because the isolated clone + installs no GUI dependencies; they reproduce identically on an unmodified + `dev` checkout. Claiming this criterion as "green" would have been false — + the honest bar is: both new suites pass, and no failure is attributable to + this change. + +## Verifier commands (PLAN-VERIFIER-REAL-01) + +| Command | Reads this change? | Notes | +|---------|-------------------|-------| +| `bun run test tests/catalog-llamacpp-capabilities.test.ts` | YES — direct argument | New file | +| `bun x tsc --noEmit` | YES — tsconfig include covers `src/**` | Verified exit 0 pre-change | +| Remote exact-head suite (command below) | YES — existing catalog suites exercise `provider-fetch.ts` | Required: shared surface | + +## Field chain (PLAN-FIELD-CHAIN-01) + +| Stage | Path | State | +|-------|------|-------| +| creation | upstream server `/v1/models` response | unchanged | +| extraction | `extractProviderModelItems`, src/providers/model-discovery.ts:329 | unchanged (data[] only) | +| hint mapping | `catalogHintsFromModelsApiItem` | NEW: meta.n_ctx read | +| serialization | `applyCatalogModelMetadata` (Phase 1 provenance) | carries the value to routing | +| consumer | `candidateCapabilityEvidence` | receives contextWindow | + +N/A: no new enum value and no new type member (B8). + +## Bypass record (PLAN-BYPASS-NAMED-01) + +No enforcement added. Tier: N/A. Executing surface: none. Known bypass: a +server reporting context nowhere in the recognized list still yields unknown — +by design. Residual risk: `n_ctx` is trusted as reported; a server misreporting +it would mislead routing exactly as any other context field would. Wording +downgrade: N/A. Final enforcement layer: none. + +## Remote exact-head suite (round-5 correction) + +Five rounds of audit produced five distinct ways a remote command can lie. The +final form separates PUBLISH from VERIFY, and every step fails closed. + +**Step 1 — publish (separate, must succeed on its own).** + + set -eu + BRANCH=$(git rev-parse --abbrev-ref HEAD) + LOCAL_SHA=$(git rev-parse HEAD) + git push --no-verify csa906 "$BRANCH" + +Round 5 observed this step fail with `! [remote rejected] ... (permission denied)` +while the verification that followed still ran and could have reported success. +Publication is therefore its own command whose exit code is checked before +anything else happens. + +**Step 2 — assert the remote actually has this commit.** + + set -eu + BRANCH=$(git rev-parse --abbrev-ref HEAD) + LOCAL_SHA=$(git rev-parse HEAD) + REMOTE_SHA=$(git ls-remote csa906 "refs/heads/$BRANCH" | cut -f1) + test -n "$REMOTE_SHA" + test "$REMOTE_SHA" = "$LOCAL_SHA" + +`test -n` matters independently: a failed push leaves `REMOTE_SHA` EMPTY, and an +empty-vs-empty comparison would otherwise pass. + +**Step 3 — verify in an isolated scratch clone.** + + LOCAL_SHA=$(git rev-parse HEAD) + ssh lidge "set -eu + export PATH=\"\$HOME/.bun/bin:\$PATH\" + command -v bun >/dev/null + WORKDIR=\$(mktemp -d -t ocx-verify-XXXXXX) + trap 'rm -rf \"\$WORKDIR\"' EXIT + git clone --quiet --no-checkout https://github.com/csa906/opencodex.git \"\$WORKDIR\" + cd \"\$WORKDIR\" + git fetch --quiet origin $LOCAL_SHA + git checkout --quiet --detach $LOCAL_SHA + test \"\$(git rev-parse HEAD)\" = \"$LOCAL_SHA\" + bun install --frozen-lockfile + bun run test" + +Each guard exists because a specific failure was observed in audit: + +| Guard | Observed failure it prevents | +|-------|------------------------------| +| Steps split + `set -eu` | round 5: push was rejected and `ls-remote` returned empty, yet the suite still ran and could have reported success | +| `test -n "$REMOTE_SHA"` | an empty remote SHA comparing equal to an empty string | +| `git ls-remote` SHA equality | `fatal: remote error: upload-pack: not our ref 60fd5a7d9...` on an unpushed commit | +| `export PATH` + `command -v bun` | `command -v bun` empty over non-interactive ssh while `/home/lidgeai/.bun/bin/bun` exists | +| `mktemp -d` + `trap` cleanup | `~/ocx-ci/opencodex` had modified `src/bridge.ts`, `src/server/responses/core.ts`; a run there proves nothing about this change | +| `git rev-parse HEAD` equality | a stale checkout reporting a green suite for different code | + +Each step recomputes `BRANCH`/`LOCAL_SHA` so it is independently runnable in a +fresh shell: audit round 6 showed Step 2 aborting with `BRANCH: parameter not +set` when the variables only existed in Step 1. That failed closed, but a +verifier that only works when pasted in one session is not the verifier the +doc describes. + +C runs these three literal steps in order and pastes each exit code. A green +suite whose publication step failed is not evidence. + +Note: round 5 ran step 3 successfully and recorded `12299 pass, 11 skip, 7 fail` +on an unrelated tree state. C must reach a green run at THIS unit's head, or +triage each failure against `dev` before claiming the phase verified. diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/030_local_model_plugin_routing.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/030_local_model_plugin_routing.md new file mode 100644 index 0000000000..756a3166f9 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/030_local_model_plugin_routing.md @@ -0,0 +1,114 @@ +# 030 — Phase 3: durable browser-plugin routing guidance for local models + +Diff-level implementation doc. Research: `002_local_model_plugin_failure.md`. +No dependency on Phases 1-2; it may land in any order. + +> **Superseded placement (review correction).** This document originally +> selected `~/.codex/AGENTS.md`. That was wrong: an untracked host file cannot +> ship with the package or be reproduced from a clone, so it is not a +> deliverable. The guidance now lives in the tracked `AGENTS_INSTALL.md`, +> which `package.json` publishes and which `AGENTS.md` and `README.md` both +> point an installing or operating agent at. Every mention of the host path +> below is historical; treat `AGENTS_INSTALL.md` as the placement decision. +> The host copy remains only as local convenience and host-only verification. + +## Goal + +A weaker local model asked to browse should reach `mcp__node_repl__js` on its +first attempt instead of exhausting `osascript`, `find`, and a hand-written +Node script before declaring the tooling unavailable. + +## The constraint that makes this a docs problem + +Nothing in `src/` participates. The failure is host-side tool routing: + +```console +$ node -e "import('.../chrome/scripts/browser-client.mjs').then(m => m.setupBrowserRuntime())" +RUNTIME FAIL: Browser use requires privileged node_repl capabilities +``` + +The bundle reads `globalThis.nodeRepl` and ships its own `process` shim, so +the privileged REPL tool is the only working host. Computer Use's `@oai/sky` +has no on-disk package at all. There is no code change that can make a shell +path work, and inventing one would be a fork of the plugin. + +## Scope boundary + +IN: a durable guidance surface a local model reads before acting. +OUT: editing the bundled plugin skills under +`~/.codex/plugins/cache/openai-bundled/` (vendor-owned, replaced on update), +any change to `src/`, and any claim that this is enforced. + +## Placement decision + +Candidates considered: + +| Candidate | Verdict | +|-----------|---------| +| Bundled plugin `SKILL.md` | REJECTED — vendor-owned, overwritten on plugin update | +| Repository `AGENTS.md` | REJECTED — this is host tooling, not opencodex development guidance; loaded for every code change where it is noise | +| `~/.codex/AGENTS.md` (global) | SUPERSEDED — host-only, untracked, cannot ship or be reproduced from a clone | +| `AGENTS_INSTALL.md` (tracked, published) | **CHOSEN** — the file an installing or operating agent reads; shipped in the package | + +Verified: `/Users/jun/.codex/AGENTS.md` exists and is 0 bytes, so the guidance +is additive with no merge risk. + +## File change map + +| Path | Action | What | +|------|--------|------| +| `AGENTS_INSTALL.md` | MODIFY (append) | Browser-plugin routing rule | + +## Content to append + +```markdown +## Browser and Computer Use plugins: entry point + +The Chrome, Browser, and Computer Use plugins run ONLY through the +`mcp__node_repl__js` tool. Call it directly; if it is not listed, search the +available tools for `node_repl js` before concluding anything is unavailable. + +These do not work and are not worth attempting: + +- `node` / `node -e` importing `scripts/browser-client.mjs` — refuses with + "Browser use requires privileged node_repl capabilities". +- Filesystem searches for `@oai/sky` — it is injected at runtime, never on disk. +- `osascript` / AppleScript / JXA as a substitute for the plugin API. + +A failed shell attempt is evidence about the shell, not about plugin +availability. +``` + +## Accept criteria + +1. The section is present in the tracked `AGENTS_INSTALL.md`. +2. The wording names the tool explicitly — the bundled skill's own instruction + to avoid naming `node_repl` in user-facing prose is what confuses a weaker + model, so this internal-guidance surface deliberately names it. +3. No claim of enforcement appears in the text. + +## Verifier commands (PLAN-VERIFIER-REAL-01) + +| Command | Reads this change? | Notes | +|---------|-------------------|-------| +| `cat ~/.codex/AGENTS.md` | Partially — proves bytes on disk, NOT that Codex loaded them | Human-read acceptance | +| `REPO=$(git rev-parse --show-toplevel); OCX_SHIM_BYPASS=1 codex -C "$REPO" debug prompt-input \| rg "mcp__node_repl__js"` | YES — renders the instructions the model actually receives | Round 7 note: the earlier form wrote ``, which a shell reads as input redirection (`no such file or directory: repo`), so it was never runnable as printed. Proven in audit round 2: an isolated `$CODEX_HOME/AGENTS.md` carrying this guidance appeared in the prompt, exit 0. No network needed | + +Discovery caveats verified during audit (B7): global guidance resolves from +`$CODEX_HOME` when set (currently unset, so `~/.codex` applies), and +`AGENTS.override.md` takes precedence over `AGENTS.md` (currently absent). +The acceptance claim is therefore scoped to the default Codex home on this +host, and is human-verified rather than gated. + +There is no repository gate for this change: `tsc`, `bun test`, and +`privacy:scan` never read `~/.codex/AGENTS.md`. This acceptance row is human +review, stated per PLAN-VERIFIER-REAL-01 rather than dressed up as a gate. + +## Bypass record (PLAN-BYPASS-NAMED-01) + +Tier: E1 (guidance). Executing surface: the model reading its instruction +file. Known bypass: a model may ignore instructions entirely — this is the +exact failure being addressed, so the mechanism cannot be self-guaranteeing. +Residual risk: guidance reduces but does not eliminate misrouting. Wording +downgrade: YES — this is an early warning, never enforcement. Final +enforcement layer: none. diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/040_implementation_record.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/040_implementation_record.md new file mode 100644 index 0000000000..d1bb4f8560 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/040_implementation_record.md @@ -0,0 +1,108 @@ +# 040 — Implementation record and verification evidence + +Terminal outcome: **DONE**. All three objectives implemented, verified, pushed, +and opened as PR #1799 against `dev`. + +## What shipped + +| Change | File | Commit | +|--------|------|--------| +| Provenance stamp sourced from CatalogModel + generated metadata, context-cap aware, combo-excluded | `src/codex/catalog/effort.ts` | c435340bb | +| Shared generated-metadata lookup export | `src/codex/catalog/parsing.ts` | c435340bb | +| Provenance reader; unconditional adapter tool fallback | `src/routing/capability.ts` | c435340bb | +| `supportsImages` tri-state restored | `src/providers/antigravity-models.ts` | c435340bb | +| `meta.n_ctx` / `n_ctx_train` as context sources | `src/codex/catalog/provider-fetch.ts` | cc3512ed0 | +| 10 regressions | `tests/routing-capability-catalog.test.ts` | c435340bb | +| 4 regressions | `tests/catalog-llamacpp-capabilities.test.ts` | cc3512ed0 | +| Plugin routing guidance | `AGENTS_INSTALL.md` | (this commit) | + +### Where the plugin guidance lives (review correction) + +The first draft of this record listed `~/.codex/AGENTS.md` as a shipped +change. That was wrong on its own terms: the file is untracked host state, so +it is not part of the PR and cannot be reproduced from a clone. CodeRabbit +caught it, and the fix is the honest one — the guidance now lives in the +tracked `AGENTS_INSTALL.md`, which is precisely the file an agent installing +or operating opencodex reads. + +The host file remains as a local convenience and is recorded here as +host-only verification, not as a deliverable. The live check +(`codex debug prompt-input | rg mcp__node_repl__js`, exit 0) proved the +guidance reaches a model through the host path; the tracked copy is what +makes it reproducible for everyone else. + +## Activation grounding (C-ACTIVATION-GROUNDING-01) + +Both suites were run against the tree with the source changes stashed, proving +they observe the defect rather than passing vacuously: + +| Suite | Without fix | With fix | +|-------|-------------|----------| +| `routing-capability-catalog` | 5 pass / 5 fail | 10 pass / 0 fail | +| `catalog-llamacpp-capabilities` | 1 pass / 3 fail | 4 pass / 0 fail | + +The llama.cpp matrix matches what audit round 6 predicted exactly: tests 1, 2 +and 4 red before, test 3 (the precedence guard) green either way. + +## End-to-end proof + +With the `lidge` provider's inline capability maps deleted — the exact state of a +user who only ran `ocx models add` — the catalog is the sole evidence source: + + before: {"tools":true,"serviceTier":"unsupported","encryptedCodexTasks":false} + after: {"contextWindow":262144,"image":true,"tools":true,...} + +The provenance block written for that row: + + {"provider":"lidge","model_id":"qwen3.8-27b-nvfp4", + "context_window":262144,"input_modalities":["text","image"]} + +## Remote exact-head suite + +Run in an isolated scratch clone at `cc3512ed0`, per the three-step block in +`010`/`020`: + + 12337 tests across 792 files — 11 skip, 7 fail, 461.65s + +All 7 failures are `Cannot find package 'react'` / `react/jsx-dev-runtime` from +`gui/` sources, because the scratch clone installs no GUI dependencies. The +failing files are `gui-management-session`, `provider-workspace-data`, +`tencent-siliconflow-providers`, `usage-surfaces`, `vision-sidecar-timeout-bounds`, +and `volcengine-providers` — none touch routing, catalog, or Antigravity. Audit +round 6 independently reproduced the same seven on an unmodified `dev` checkout. +Both new suites passed on the remote host. + +## CI (PR #1799) + +21 checks pass, including `enforce-target`, `gates`, `hygiene`, `react-doctor`, +`api usage`, all four `test` shards, and every `keyring`/`npm-global` matrix job. + +Two fail: `macos` and `ci`. The macOS job is a **Bun runtime segfault**, not a +test failure — the workflow classifies it explicitly: + + RSS: 3.40GB | Peak: 3.63GB | Machine: 7.52GB + panic: Segmentation fault at address 0xFFFFFFFFFFFFFFE8 + oh no: Bun has crashed. This indicates a bug in Bun, not your code. + ::error::Bun runtime crash repeated on the macOS suite; failing after one retry. + +The latest `dev` run (31902897010) fails the same two jobs for the same reason, +so this is pre-existing and not introduced here. + +## Scope honesty + +The image half of #1797 is NOT fixed. `extractProviderModelItems` reads only +`data[]` envelopes by explicit design, so the `multimodal` token in `models[]` is +discarded, and even a merged item would stay image-unknown because `multimodal` +is not a recognized capability string. Test 4 of the llama.cpp suite +characterizes that gap so the follow-up has a live witness rather than a prose +claim. + +## Deferred + +`cxc orchestrate` could not record the A>B transition: REVIEW-BINDING-01 accepts +a verdict only from a `SubagentStop` hook requiring `agent_type: "explorer"`, and +this host's rollout emits `SubagentStop` without that field (see `004`). The hook +was untrusted as well and was fixed with `cxc hooks retrust`, but the payload gap +remains. Fabricating the payload would defeat the exact self-attestation boundary +the rule exists to enforce, so the FSM stayed at A while the work proceeded with +its evidence recorded here. diff --git a/devlog/_plan/260816_pr_merge_campaign/000_triage.md b/devlog/_plan/260816_pr_merge_campaign/000_triage.md new file mode 100644 index 0000000000..8936db15cb --- /dev/null +++ b/devlog/_plan/260816_pr_merge_campaign/000_triage.md @@ -0,0 +1,75 @@ +# 000 — Nine-PR merge campaign: triage and merge order + +## Scope + +Nine open PRs the user marked ready for final review and merge, plus a direct +implementation of #1797 pushed to `dev`. + +## Why every PR reports BLOCKED + +This was the first thing to establish, because "BLOCKED" invites the assumption +that CI is red. It is not. + +| PR | reviewDecision | Failing checks | Real blocker | +|----|----------------|----------------|--------------| +| #1727 | REVIEW_REQUIRED | none | awaiting approval | +| #1728 | REVIEW_REQUIRED | `label` CANCELLED | awaiting approval | +| #1729 | REVIEW_REQUIRED | none | awaiting approval + stacked base | +| #1732 | REVIEW_REQUIRED | none | awaiting approval + stacked base | +| #1740 | REVIEW_REQUIRED | none | awaiting approval | +| #1750 | REVIEW_REQUIRED | none | awaiting approval | +| #1764 | REVIEW_REQUIRED | none | awaiting approval | +| #1793 | **CHANGES_REQUESTED** | `label`, `enforce-target` CANCELLED | requested changes must be resolved | +| #1799 | REVIEW_REQUIRED | `macos` pending | **5 unresolved CodeRabbit threads** | + +Eight of nine are gated purely on approval. Only two carry real work: +`#1793` has a CHANGES_REQUESTED review, and `#1799` (my own unit) has five +unresolved review threads including one Major. + +The `macos` job is a pre-existing Bun segfault that also fails on unmodified +`dev` (run 31902897010), so it is not a per-PR blocker. + +## Surface and risk + +| PR | Size | Surface | Risk | +|----|------|---------|------| +| #1727 | +1750/-7, 18 files | new `src/codex/log-guard/`, CLI, GUI, docs | additive; new subsystem | +| #1729 | +2753/-124, 28 files | extends log-guard; touches `src/codex/app-server-processes.ts` | larger, touches existing runtime | +| #1732 | +1636/-15, 13 files | log-guard maintenance + management context | additive on the chain | +| #1728 | +649/-29, 25 files | subagent surface, i18n across 5 locales, docs | behavioral: model-version routing | +| #1793 | +575/-26, 14 files | `slug-codec.ts`, `model-discovery.ts`, `router.ts`, `registry.ts` | **overlaps #1799** | +| #1740 | +1030/-240, 4 files | `release.yml`, changelog builder | release automation — security-review surface | +| #1750 | +116/-16, 12 files | `.github/scripts/*`, `src/codex/*` | CI scripts + small runtime fixes | +| #1764 | +700/-15, 6 files | issue-triage workflows and scripts | CI-only | +| #1799 | +1957/-22, 16 files | `capability.ts`, `effort.ts`, `parsing.ts`, `provider-fetch.ts` | **overlaps #1793** | + +## Conflict found during triage + +`#1793` and `#1799` both modify `src/providers/model-discovery.ts`, and `#1793` also +rewrites `src/providers/slug-codec.ts` — the module #1799's design deliberately +avoided importing. Whichever lands second must rebase and re-verify rather than +trusting a clean auto-merge. This is the single ordering constraint that is not +visible from the PR list alone. + +## Merge order (dependency-first, not effort-first) + +1. **#1799** — my own unit; its five threads are mine to close, and landing it + first fixes the shared `model-discovery.ts` baseline that #1793 must rebase on. +2. **#1727 -> #1729 -> #1732** — forced stack. `#1729` targets + `feat/codex-log-guard-inspect` (head of #1727) and `#1732` targets + `feat/codex-log-guard-protect` (head of #1729). Each child is retargeted to + `dev` after its parent lands, per AGENTS.md. +3. **#1793** — after #1799, rebased onto the new `model-discovery.ts`, with its + CHANGES_REQUESTED review resolved. +4. **#1728** — independent surface (subagent routing + i18n). +5. **#1750**, **#1764** — CI/script surface, mutually independent. +6. **#1740** — release automation LAST: it edits `.github/workflows/release.yml`, + which AGENTS.md flags as a highest-priority security-review surface. Landing it + after everything else keeps the release path stable while the rest merge. + +## Method per PR + +One PABCD cycle each: read the real diff, run the checks that cover it, dispatch +an independent `gpt-5.6-sol` explorer, fold every finding, then merge. Bypass of +the approval gate is authorized by the user; each bypass is recorded with what +was bypassed and why it was safe. diff --git a/src/codex/catalog/effort.ts b/src/codex/catalog/effort.ts index e8e334511f..a2c462d046 100644 --- a/src/codex/catalog/effort.ts +++ b/src/codex/catalog/effort.ts @@ -31,7 +31,7 @@ import { redactSecretString, redactUserPath } from "../../lib/redact"; import upstreamModelsSnapshot from "../data/upstream-models.json"; -import { readCatalog, readCodexCatalogPath } from "./parsing"; +import { generatedModelMetadata, readCatalog, readCodexCatalogPath } from "./parsing"; import type { CatalogModel, RawEntry } from "./parsing"; import { UPSTREAM_NATIVE_ENTRIES } from "./metadata"; import { nativeOpenAiCapabilitySourceSlug } from "./native-models"; @@ -148,6 +148,54 @@ export function applyCatalogModelMetadata(entry: RawEntry, model?: CatalogModel) }]; entry.additional_speed_tiers = ["fast"]; } + stampCapabilityProvenance(entry, model); +} + +/** + * Record which capability values a real source actually asserted (#1796). + * + * `ensureStrictCatalogFields` fills `context_window` and `input_modalities` with + * compatibility defaults so Codex's strict parser accepts the file, which means + * an entry ALWAYS carries both and their presence proves nothing. Routing has to + * tell "the provider said text-only" apart from "nobody said anything", so it + * reads this block and never the entry itself ("unknown is not zero", + * src/routing/capability.ts). + * + * Two real sources exist and both are consulted here, in the same precedence the + * writers use (`applyCatalogMetadata` runs first, the model's own fields + * overwrite it): the `CatalogModel` and the generated jawcode metadata table. + * Reading only the model would silently drop every provider whose capabilities + * live in that table. + */ +function stampCapabilityProvenance(entry: RawEntry, model: CatalogModel): void { + // Virtual combo rows are synthesized from last-resort defaults (a generic 128k + // context and a `["text"]` modality), so their values are placeholders rather + // than assertions. Stamping them would reintroduce the exact false-evidence + // defect this block exists to prevent. + if (model.provider === COMBO_NAMESPACE) return; + + const meta = generatedModelMetadata(model.provider, model.id); + const metaContext = typeof meta?.contextWindow === "number" && meta.contextWindow > 0 + // The generated context is capped before it reaches the entry, so provenance + // must apply the same cap or routing would advertise a window the cap refused. + ? applyProviderContextCap(meta.contextWindow, model.contextCap) ?? meta.contextWindow + : undefined; + const contextWindow = typeof model.contextWindow === "number" && model.contextWindow > 0 + ? model.contextWindow + : metaContext; + const inputModalities = Array.isArray(model.inputModalities) && model.inputModalities.length > 0 + ? model.inputModalities + : (Array.isArray(meta?.input) && meta.input.length > 0 ? meta.input : undefined); + + entry.opencodex_capability_provenance = { + provider: model.provider, + model_id: model.id, + ...(contextWindow !== undefined ? { context_window: contextWindow } : {}), + ...(inputModalities !== undefined ? { input_modalities: [...inputModalities] } : {}), + ...(Array.isArray(model.capabilities) && model.capabilities.length > 0 + ? { capabilities: [...model.capabilities] } + : {}), + }; } export function applyReasoningLevels( diff --git a/src/codex/catalog/parsing.ts b/src/codex/catalog/parsing.ts index 705a0d2dbe..df63f0eb93 100644 --- a/src/codex/catalog/parsing.ts +++ b/src/codex/catalog/parsing.ts @@ -455,11 +455,25 @@ export function catalogModelSupportsReasoningSummaries(modelId: string): boolean return values.size === 1 ? values.values().next().value : undefined; } -export function applyCatalogMetadata(entry: RawEntry, provider: string, modelId: string, contextCap?: number): void { +/** + * Resolve the generated jawcode metadata row for a provider/model pair. + * + * Exported because it is the SECOND source of real capability assertions: + * `applyCatalogMetadata` writes context/modalities from it without ever + * touching a `CatalogModel`, so the routing-evidence provenance stamp in + * `applyCatalogModelMetadata` has to consult the same table. Both callers share + * this one lookup rather than duplicating the resolve/case-fold rules, which is + * what keeps the serialized entry and its provenance from drifting apart. + */ +export function generatedModelMetadata(provider: string, modelId: string) { const jawcodeProvider = resolveMetadataProvider(provider); - if (!jawcodeProvider) return; - const meta = getModelMetadata(jawcodeProvider, modelId) + if (!jawcodeProvider) return undefined; + return getModelMetadata(jawcodeProvider, modelId) ?? (shouldCaseFoldMetadataModelId(provider) ? getModelMetadataCaseInsensitive(jawcodeProvider, modelId) : undefined); +} + +export function applyCatalogMetadata(entry: RawEntry, provider: string, modelId: string, contextCap?: number): void { + const meta = generatedModelMetadata(provider, modelId); if (!meta) return; if (typeof meta.contextWindow === "number" && meta.contextWindow > 0) { const contextWindow = applyProviderContextCap(meta.contextWindow, contextCap) ?? meta.contextWindow; diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 1f119ef303..899429440e 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -1008,6 +1008,13 @@ export function catalogHintsFromModelsApiItem(providerName: string, item: Provid item.context_size, item.max_model_len, item.max_context_length, + // llama.cpp reports the served context under `meta`: `n_ctx` is what the + // server was actually started with, `n_ctx_train` the model's trained + // maximum. Prefer the served value — routing must not promise a window the + // running server will refuse. Both come LAST so no provider already + // supplying a recognized field changes behavior (#1797). + plainRecord(item.meta)?.n_ctx, + plainRecord(item.meta)?.n_ctx_train, ); const maxInputTokens = positiveSafeInteger(limits?.max_input_tokens, item.max_input_tokens); // Some OpenAI-compatible catalogs expose the selectable ladder under diff --git a/src/providers/antigravity-models.ts b/src/providers/antigravity-models.ts index c6b0ea283d..15adece9a1 100644 --- a/src/providers/antigravity-models.ts +++ b/src/providers/antigravity-models.ts @@ -331,7 +331,17 @@ export function parseAntigravityAvailableModels( out.push({ id, ...(antigravityPositiveInteger(info.maxTokens) ? { contextWindow: antigravityPositiveInteger(info.maxTokens) } : {}), - inputModalities: info.supportsImages === true ? ["text", "image"] : ["text"], + // Tri-state, deliberately not a ternary: `true` asserts image support, + // `false` asserts against it, and ABSENT is unknown. Collapsing absent into + // `["text"]` let routing read it as a confident `image: false` (#1796). The + // strict catalog still receives its `["text"]` compatibility default + // downstream via ensureStrictCatalogFields; only the routing-evidence + // channel stays honest about what was never asserted. + ...(info.supportsImages === true + ? { inputModalities: ["text", "image"] as string[] } + : info.supportsImages === false + ? { inputModalities: ["text"] as string[] } + : {}), }); } return out; diff --git a/src/routing/capability.ts b/src/routing/capability.ts index 0d7d5cde5a..312bb846fc 100644 --- a/src/routing/capability.ts +++ b/src/routing/capability.ts @@ -26,11 +26,12 @@ import { statSync } from "node:fs"; import type { RouteCapabilityEvidence } from "./trace"; type CatalogModelRow = { + /** Exact provider/native-id identity, from the provenance block. */ provider: string; id: string; + /** Only values a real source asserted; never a strict-parser default. */ contextWindow?: number; inputModalities?: string[]; - reasoningEfforts?: string[]; capabilities?: string[]; }; @@ -52,23 +53,33 @@ function cachedCatalogModels(): CatalogModelRow[] { const catalog = readCatalog(path); const models = catalog?.models; if (!Array.isArray(models)) return []; - const rows = models - .filter((model): model is Record & { id: string; provider: string } => - typeof model === "object" && model !== null && typeof model.id === "string" && typeof model.provider === "string") - .map(model => ({ - provider: model.provider, - id: model.id, - ...(typeof model.contextWindow === "number" ? { contextWindow: model.contextWindow } : {}), - ...(Array.isArray(model.inputModalities) - ? { inputModalities: model.inputModalities.filter((value): value is string => typeof value === "string") } + // Read ONLY `opencodex_capability_provenance` (written by + // applyCatalogModelMetadata). The row's own `context_window` and + // `input_modalities` always exist because ensureStrictCatalogFields fills them + // with compatibility defaults for Codex's strict parser, so reading them would + // turn "nobody asserted anything" into a confident `image: false` and a + // fabricated 128000 — the opposite of this module's contract. A row without + // provenance contributes nothing. + const rows = models.flatMap((model): CatalogModelRow[] => { + if (typeof model !== "object" || model === null) return []; + const provenance = (model as Record).opencodex_capability_provenance; + if (typeof provenance !== "object" || provenance === null) return []; + const source = provenance as Record; + if (typeof source.provider !== "string" || typeof source.model_id !== "string") return []; + return [{ + provider: source.provider, + id: source.model_id, + ...(typeof source.context_window === "number" && source.context_window > 0 + ? { contextWindow: source.context_window } : {}), - ...(Array.isArray(model.reasoningEfforts) - ? { reasoningEfforts: model.reasoningEfforts.filter((value): value is string => typeof value === "string") } + ...(Array.isArray(source.input_modalities) + ? { inputModalities: source.input_modalities.filter((value): value is string => typeof value === "string") } : {}), - ...(Array.isArray(model.capabilities) - ? { capabilities: model.capabilities.filter((value): value is string => typeof value === "string") } + ...(Array.isArray(source.capabilities) + ? { capabilities: source.capabilities.filter((value): value is string => typeof value === "string") } : {}), - })); + }]; + }); catalogCache = { path, mtimeMs, rows }; return rows; } catch { @@ -177,13 +188,17 @@ export function candidateCapabilityEvidence( // override. const tools = capabilities.includes("tools") || isNative - || (catalogRow === undefined && provider !== undefined && TOOL_CAPABLE_ADAPTERS.has(provider.adapter)) + // The adapter protocol is positive evidence on its own. This was once gated + // on `catalogRow === undefined`, which was only safe while the catalog lookup + // never matched anything: once it matches, a row that simply does not + // enumerate "tools" would silently revoke tool support for every openai-chat + // and anthropic candidate. + || (provider !== undefined && TOOL_CAPABLE_ADAPTERS.has(provider.adapter)) || provider?.parallelToolCalls === true || undefined; const reasoningEfforts = provider?.modelReasoningEfforts?.[modelId] ?? registryEntry?.modelReasoningEfforts?.[modelId] - ?? catalogRow?.reasoningEfforts ?? (isNative ? nativeReasoningEfforts(modelId) : undefined); const tierSupport = provider diff --git a/tests/catalog-llamacpp-capabilities.test.ts b/tests/catalog-llamacpp-capabilities.test.ts new file mode 100644 index 0000000000..153999128e --- /dev/null +++ b/tests/catalog-llamacpp-capabilities.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, test } from "bun:test"; +import { catalogHintsFromModelsApiItem } from "../src/codex/catalog/provider-fetch"; +import { extractProviderModelItems } from "../src/providers/model-discovery"; + +/** + * Regression coverage for the context half of #1797. + * + * A llama.cpp server reports its served context under `meta.n_ctx`, which was in + * none of the recognized context fields, so a correct local server produced no + * context evidence at all. + * + * The image half of #1797 is NOT fixed here and is characterized below: the + * `multimodal` token lives in the Ollama-style `models[]` array while discovery + * deliberately reads only `data[]`, and even a merged item would stay + * image-unknown because `multimodal` is not a recognized capability string. + */ + +const VERBATIM_LLAMACPP_BODY = { + models: [{ + name: "qwen3.8-27b-nvfp4", + model: "qwen3.8-27b-nvfp4", + capabilities: ["completion", "multimodal"], + details: { format: "gguf" }, + }], + object: "list", + data: [{ + id: "qwen3.8-27b-nvfp4", + object: "model", + owned_by: "llamacpp", + meta: { n_ctx: 262144, n_ctx_train: 262144, n_vocab: 248320, n_embd: 5120 }, + }], +}; + +describe("llama.cpp served context ingestion (#1797)", () => { + test("absorbs meta.n_ctx from the verbatim data[] item", () => { + const hints = catalogHintsFromModelsApiItem("lidge", { + id: "qwen3.8-27b-nvfp4", + object: "model", + owned_by: "llamacpp", + meta: { n_ctx: 262144, n_ctx_train: 262144 }, + }); + expect(hints.contextWindow).toBe(262144); + }); + + test("prefers the served n_ctx over the trained maximum", () => { + // Routing must not promise a window the running server will refuse. + const hints = catalogHintsFromModelsApiItem("lidge", { + id: "short-ctx", + meta: { n_ctx: 8192, n_ctx_train: 262144 }, + }); + expect(hints.contextWindow).toBe(8192); + }); + + test("a recognized context field still wins over meta", () => { + // Contested on purpose: meta entries are appended last so no provider + // already supplying a recognized field changes behavior. + const hints = catalogHintsFromModelsApiItem("lidge", { + id: "both", + context_length: 32768, + meta: { n_ctx: 8192 }, + }); + expect(hints.contextWindow).toBe(32768); + }); + + test("the dual-envelope body yields context but still no image evidence", () => { + // Characterization of the KNOWN remaining gap in #1797, so the follow-up fix + // has a live witness and a test to flip rather than a prose claim. + const extracted = extractProviderModelItems(VERBATIM_LLAMACPP_BODY, { + maxModels: 100, + } as never); + expect(extracted.ok).toBe(true); + const items = (extracted as { ok: true; items: Array> }).items; + expect(items.length).toBe(1); + + const hints = catalogHintsFromModelsApiItem("lidge", items[0] as never); + expect(hints.contextWindow).toBe(262144); + // The "multimodal" token was discarded with models[]; unknown, never false. + expect(hints.inputModalities).toBeUndefined(); + }); +}); diff --git a/tests/routing-capability-catalog.test.ts b/tests/routing-capability-catalog.test.ts new file mode 100644 index 0000000000..5e7fd89097 --- /dev/null +++ b/tests/routing-capability-catalog.test.ts @@ -0,0 +1,183 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { candidateCapabilityEvidence } from "../src/routing/capability"; +import { applyCatalogModelMetadata } from "../src/codex/catalog/effort"; +import { applyCatalogMetadata, ensureStrictCatalogFields } from "../src/codex/catalog/parsing"; +import type { CatalogModel, RawEntry } from "../src/codex/catalog/parsing"; +import { parseAntigravityAvailableModels } from "../src/providers/antigravity-models"; +import type { OcxConfig } from "../src/types"; + +/** + * Regression coverage for #1796. + * + * The catalog branch of `candidateCapabilityEvidence` was dead code: the reader + * filtered on `id`/`provider` while the writer emits `slug`/`context_window`/ + * `input_modalities`, so every row was discarded. The repair reads an explicit + * `opencodex_capability_provenance` block instead of the compatibility-shaped + * fields, because `ensureStrictCatalogFields` synthesizes those for Codex's + * strict parser and they therefore cannot distinguish an assertion from a + * placeholder. + * + * These are writer-through-reader tests on purpose: hand-written catalog rows + * would pass while the real writer emitted nothing. + */ + +let codexHome = ""; +let previousCodexHome: string | undefined; + +beforeEach(() => { + previousCodexHome = process.env.CODEX_HOME; + codexHome = mkdtempSync(join(tmpdir(), "ocx-capability-catalog-")); + process.env.CODEX_HOME = codexHome; +}); + +afterEach(() => { + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + if (codexHome) rmSync(codexHome, { recursive: true, force: true }); +}); + +function serialize(model: CatalogModel): RawEntry { + // Mirrors the real write order in src/codex/catalog/sync.ts:321-322 — + // generated metadata first, then the model own fields, then strict + // normalization fills the compatibility defaults Codex requires. + const entry: RawEntry = { slug: `${model.provider}/${model.id}` } as RawEntry; + applyCatalogMetadata(entry, model.provider, model.id, model.contextCap); + applyCatalogModelMetadata(entry, model); + return ensureStrictCatalogFields(entry, { isRouted: true }); +} + +function writeCatalog(models: CatalogModel[]): void { + const entries = models.map(serialize); + writeFileSync(join(codexHome, "opencodex-catalog.json"), JSON.stringify({ models: entries })); +} + +function configFor(providers: string[]): OcxConfig { + // Providers declare NO modelContextWindows / modelInputModalities, so the + // catalog is the only possible evidence source for those dimensions. + const entries = providers.map(name => [name, { + adapter: "openai-chat" as const, + baseUrl: `https://${name}.example/v1`, + }]); + return { providers: Object.fromEntries(entries) } as unknown as OcxConfig; +} + +describe("catalog-sourced capability evidence (#1796)", () => { + test("carries asserted context window and image modality from a catalog-only model", () => { + writeCatalog([{ + provider: "lidge", + id: "qwen3.8-27b-nvfp4", + contextWindow: 262144, + inputModalities: ["text", "image"], + } as CatalogModel]); + + const evidence = candidateCapabilityEvidence(configFor(["lidge"]), "lidge", "qwen3.8-27b-nvfp4"); + expect(evidence.contextWindow).toBe(262144); + expect(evidence.image).toBe(true); + }); + + test("a synthesized strict-parser default stays unknown, never false", () => { + // The model asserts nothing, so ensureStrictCatalogFields writes + // context_window 128000 and input_modalities ["text"] for Codex. Neither is + // evidence, and reading them back would make unknown look like a decision. + writeCatalog([{ provider: "demo", id: "unknown-model" } as CatalogModel]); + + const evidence = candidateCapabilityEvidence(configFor(["demo"]), "demo", "unknown-model"); + expect(evidence.contextWindow).toBeUndefined(); + expect(evidence.image).toBeUndefined(); + }); + + test("matching a catalog row does not revoke adapter tool support", () => { + // The adapter fallback used to be gated on `catalogRow === undefined`, which + // was only safe while the lookup never matched anything. + writeCatalog([{ + provider: "lidge", + id: "qwen3.8-27b-nvfp4", + contextWindow: 262144, + inputModalities: ["text", "image"], + } as CatalogModel]); + + const evidence = candidateCapabilityEvidence(configFor(["lidge"]), "lidge", "qwen3.8-27b-nvfp4"); + expect(evidence.tools).toBe(true); + }); + + test("exact identity is not confused by a slug collision", () => { + // Native ids "a/b" and "a-b" both encode to the Codex-facing slug "p/a-b". + writeCatalog([ + { provider: "p", id: "a/b", contextWindow: 111000 } as CatalogModel, + { provider: "p", id: "a-b", contextWindow: 222000 } as CatalogModel, + ]); + + const config = configFor(["p"]); + expect(candidateCapabilityEvidence(config, "p", "a/b").contextWindow).toBe(111000); + expect(candidateCapabilityEvidence(config, "p", "a-b").contextWindow).toBe(222000); + }); + + test("synthesized combo rows are not stamped with capability provenance", () => { + // Combo members fall back to a generic 128k/text synthesis, so those values + // are placeholders rather than assertions. + const entry = serialize({ + provider: "combo", + id: "synthetic", + contextWindow: 128000, + inputModalities: ["text"], + } as CatalogModel); + + expect(entry.opencodex_capability_provenance).toBeUndefined(); + // The synthesized value still ships to Codex, which is what makes it parse. + expect(entry.context_window).toBe(128000); + }); +}); + +describe("provenance sources beyond the CatalogModel (#1796)", () => { + test("captures generated metadata for an identity-only model", () => { + // `applyCatalogMetadata` writes real values from the generated table without + // ever touching a CatalogModel, so a stamp reading `model.*` alone would + // drop every provider that depends on it. + const entry = serialize({ provider: "opencode-go", id: "grok-4.6" } as CatalogModel); + const provenance = entry.opencodex_capability_provenance as Record | undefined; + + expect(provenance).toBeDefined(); + expect(provenance?.context_window).toBe(entry.context_window); + expect(provenance?.input_modalities).toEqual(entry.input_modalities as string[]); + }); + + test("applies the provider context cap to generated metadata", () => { + // The entry is capped before serialization; provenance must carry the same + // value or routing would advertise a window the cap already refused. + const entry = serialize({ provider: "opencode-go", id: "grok-4.6", contextCap: 350000 } as CatalogModel); + const provenance = entry.opencodex_capability_provenance as Record | undefined; + + expect(entry.context_window).toBe(350000); + expect(provenance?.context_window).toBe(350000); + }); +}); + +describe("Antigravity supportsImages tri-state (#1796)", () => { + function ccaPayload(info: Record): unknown { + return { + models: { "wire-model": info }, + agentModelSorts: [{ groups: [{ modelIds: ["wire-model"] }] }], + }; + } + + test("an absent supportsImages leaves modality unknown", () => { + // Absent must not collapse into ["text"]: routing would read that as a + // confident image:false for a model nobody made a claim about. + const rows = parseAntigravityAvailableModels(ccaPayload({ maxTokens: 333333 })); + expect(rows?.[0]?.inputModalities).toBeUndefined(); + expect(rows?.[0]?.contextWindow).toBe(333333); + }); + + test("an explicit supportsImages:false still asserts text-only", () => { + const rows = parseAntigravityAvailableModels(ccaPayload({ maxTokens: 333333, supportsImages: false })); + expect(rows?.[0]?.inputModalities).toEqual(["text"]); + }); + + test("an explicit supportsImages:true asserts image support", () => { + const rows = parseAntigravityAvailableModels(ccaPayload({ maxTokens: 333333, supportsImages: true })); + expect(rows?.[0]?.inputModalities).toEqual(["text", "image"]); + }); +});