From e02dd6799214e9b6dd59cb2879d751a9a3dff47d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 05:32:28 +0900 Subject: [PATCH 01/16] docs(devlog): plan the local-model capability evidence and plugin routing unit --- .../000_plan.md | 56 +++++ .../001_capability_evidence_defect.md | 126 +++++++++++ .../002_local_model_plugin_failure.md | 60 ++++++ .../010_catalog_row_shape.md | 195 +++++++++++++++++ .../020_live_capability_ingestion.md | 197 ++++++++++++++++++ .../030_local_model_plugin_routing.md | 97 +++++++++ 6 files changed, 731 insertions(+) create mode 100644 devlog/_plan/260816_local_model_capability_and_plugin_routing/000_plan.md create mode 100644 devlog/_plan/260816_local_model_capability_and_plugin_routing/001_capability_evidence_defect.md create mode 100644 devlog/_plan/260816_local_model_capability_and_plugin_routing/002_local_model_plugin_failure.md create mode 100644 devlog/_plan/260816_local_model_capability_and_plugin_routing/010_catalog_row_shape.md create mode 100644 devlog/_plan/260816_local_model_capability_and_plugin_routing/020_live_capability_ingestion.md create mode 100644 devlog/_plan/260816_local_model_capability_and_plugin_routing/030_local_model_plugin_routing.md 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..b439a10cf6 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/000_plan.md @@ -0,0 +1,56 @@ +# 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. 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..5878b1faeb --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/001_capability_evidence_defect.md @@ -0,0 +1,126 @@ +# 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: + +``` +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. 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..a8788bd8b2 --- /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: + +``` +$ 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/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..ec62a147f9 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/010_catalog_row_shape.md @@ -0,0 +1,195 @@ +# 010 — Phase 1: read the catalog in the shape it is written + +Diff-level implementation doc. Research: `001_capability_evidence_defect.md`. + +## Goal + +`cachedCatalogModels()` must project the on-disk catalog rows it actually +receives (`slug`, `context_window`, `input_modalities`) instead of a field +shape nothing writes. After this phase, a model whose only evidence source is +the catalog carries its real `contextWindow` and `image` evidence. + +## Scope boundary + +IN: `src/routing/capability.ts`, one new focused test file. +OUT: the evidence priority order (catalog stays the fourth fallback), the +"unknown is not zero" contract, the memoization strategy, the policy +evaluator, and every other module. + +## File change map + +| Path | Action | What | +|------|--------|------| +| `src/routing/capability.ts` | MODIFY | Parse `slug`/`context_window`/`input_modalities`; match rows by slug equivalence | +| `tests/routing-capability-catalog.test.ts` | NEW | Regression: catalog-only evidence survives assembly | + +## MODIFY `src/routing/capability.ts` + +### 1. Row type — carry the slug, not a split identity + +Before: + +```ts +type CatalogModelRow = { + provider: string; + id: string; + contextWindow?: number; + inputModalities?: string[]; + reasoningEfforts?: string[]; + capabilities?: string[]; +}; +``` + +After: + +```ts +type CatalogModelRow = { + /** Codex-facing routed slug exactly as written to the catalog file. */ + slug: string; + contextWindow?: number; + inputModalities?: string[]; + reasoningEfforts?: string[]; + capabilities?: string[]; +}; +``` + +Rationale: the catalog's identity field is the combined `slug`. Splitting it +back into `provider`/`id` here would have to re-implement the slug codec's +decode rules; comparing slugs with the codec's own equivalence helper cannot +drift from it. + +### 2. Projection — read the written keys + +Before (the filter that discards all 17 rows): + +```ts + 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") } + : {}), +``` + +After: + +```ts + const rows = models + .filter((model): model is Record & { slug: string } => + typeof model === "object" && model !== null && typeof model.slug === "string" && model.slug.length > 0) + .map(model => ({ + slug: model.slug, + ...(typeof model.context_window === "number" ? { contextWindow: model.context_window } : {}), + ...(Array.isArray(model.input_modalities) + ? { inputModalities: model.input_modalities.filter((value): value is string => typeof value === "string") } + : {}), +``` + +`reasoningEfforts` and `capabilities` keep their existing guarded spreads. +They are read from `supported_reasoning_levels` only if a later phase proves +the shape; this phase does NOT invent a mapping for them, because the catalog +writes them as objects (`{ effort, description }`), not strings. Leaving them +absent preserves "unknown is not zero" — it does not regress today's +behavior, since today they are absent too. + +### 3. Lookup — slug equivalence, not field equality + +Before: + +```ts + const catalogRow = cachedCatalogModels().find(model => model.provider === providerName && model.id === modelId); +``` + +After: + +```ts + const candidateSlug = routedSlug(providerName, modelId); + const catalogRow = cachedCatalogModels().find(model => slugsEquivalent(model.slug, candidateSlug)); +``` + +New import: + +```ts +import { routedSlug, slugsEquivalent } from "../providers/slug-codec"; +``` + +`slugsEquivalent` handles the raw/encoded mix, so a native id containing "/" +(`zenmux/moonshotai/kimi-k3-free` → catalog slug +`zenmux/moonshotai-kimi-k3-free`) matches without a blind string replace. + +### Import-boundary check + +`src/providers/slug-codec.ts` imports nothing (pure string functions), so it +cannot pull `src/lab/` into `src/router.ts`. `tests/core-lab-boundary.test.ts` +is the gate that proves this and must stay green. + +### Performance + +Unchanged: the same one-time parse, the same path+mtime memo, one `routedSlug` +call per candidate (a string concat). + +## NEW `tests/routing-capability-catalog.test.ts` + +Writes a temporary catalog file in the real on-disk shape, points the catalog +path at it, and asserts assembly: + +```ts +import { describe, expect, test } from "bun:test"; + +describe("candidateCapabilityEvidence catalog rows", () => { + test("carries context window and image modality from a catalog-only model", () => { + // config declares the provider but NO modelContextWindows / modelInputModalities, + // so the catalog row 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("matches a routed slug whose native id contains a slash", () => { + // catalog slug "zenmux/moonshotai-kimi-k3-free" vs native id "moonshotai/kimi-k3-free" + const evidence = candidateCapabilityEvidence(config, "zenmux", "moonshotai/kimi-k3-free"); + expect(evidence.contextWindow).toBe(200000); + }); + + test("leaves a dimension unknown when the catalog omits it", () => { + // "unknown is not zero": a row without input_modalities must not assert image:false + const evidence = candidateCapabilityEvidence(config, "lidge", "text-only-model"); + expect(evidence.image).toBeUndefined(); + }); +}); +``` + +The catalog path helper (`readCodexCatalogPath`) resolves from environment +state; the test overrides it through the same mechanism existing catalog +tests use — B confirms that mechanism against `tests/` before writing the +file, and amends this doc if it differs. + +## Accept criteria + +1. The new test FAILS on the current tree (evidence lacks `contextWindow` and + `image`) and PASSES after the projection change. Recording both runs is the + activation evidence required by C-ACTIVATION-GROUNDING-01 — the failing-first + run is what proves the test observes the defect rather than passing vacuously. +2. `bun x tsc --noEmit` clean. +3. `tests/core-lab-boundary.test.ts` green (import boundary unbroken). +4. Full suite green via `ssh lidge` (shared routing surface). +5. The "unknown" case asserts `undefined`, never `false`. + +## Verifier commands (PLAN-VERIFIER-REAL-01) + +| Command | Reads this change? | Notes | +|---------|-------------------|-------| +| `bun test tests/routing-capability-catalog.test.ts` | YES — the file under test is the direct argument | New file; verified to exist after B | +| `bun x tsc --noEmit` | YES — `tsconfig.json` `include` covers `src/**` | Confirmed: repo-wide strict pass | +| `bun test tests/core-lab-boundary.test.ts` | YES — walks the import graph from `src/router.ts`, which reaches `src/routing/capability.ts` | Guards the new import | + +## Bypass record (PLAN-BYPASS-NAMED-01) + +This phase adds no enforcement layer; it repairs a data path. Tier: N/A. +Executing surface: none. Known bypass: N/A. Residual risk: a future catalog +schema change could desynchronize reader and writer again — the new test is +the early warning, not enforcement. Final enforcement layer: none. 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..c3f185e759 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/020_live_capability_ingestion.md @@ -0,0 +1,197 @@ +# 020 — Phase 2: absorb llama.cpp capability metadata from live discovery + +Diff-level implementation doc. Depends on Phase 1 (`010_catalog_row_shape.md`): +without it, anything this phase writes into the catalog is still discarded by +the reader. + +## Goal + +A local llama.cpp server that truthfully advertises multimodality and its +trained context length should produce correct routing evidence with no manual +config. Today it does not, because two upstream spellings are unrecognized. + +## Observed payload (verbatim) + +`GET http://100.100.125.116:8081/v1/models` on the `lidge` provider returns a +dual-shape body — an Ollama-style `models[]` plus an OpenAI-style `data[]`: + +```json +{ "models": [ { "name": "qwen3.8-27b-nvfp4", "model": "qwen3.8-27b-nvfp4", + "capabilities": ["completion", "multimodal"], + "details": { "format": "gguf", "family": "" } } ], + "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, + "n_params": 27320698192, "size": 16367838528 } } ] } +``` + +## Why both signals are dropped today + +`src/codex/catalog/provider-fetch.ts`: + +1. `modelInputModalities()` recognizes explicit `input_modalities`/`modalities` + lists, an `architecture.modality` arrow form, `capabilities.vision`, and + the capability strings `vision` / `image-input` / `image_input`. The token + `"multimodal"` is in none of those sets, so the row yields `undefined`. +2. `catalogHintsFromModelsApiItem()` reads context from + `limits.max_context_length`, `metadata.context_length`, `context_length`, + `context_size`, `max_model_len`, and `max_context_length`. llama.cpp's + `meta.n_ctx` is in none of those, so context stays unknown. + +## Scope boundary + +IN: `src/codex/catalog/provider-fetch.ts` (the two readers above), one focused +test using the verbatim payload. +OUT: the closed `text|image|audio` enum (must not widen — Codex rejects the +whole catalog file on an unknown modality), the dual `models[]`/`data[]` +merge behavior, and any other provider's parsing. + +## File change map + +| Path | Action | What | +|------|--------|------| +| `src/codex/catalog/provider-fetch.ts` | MODIFY | Accept `multimodal` as an image signal; read `meta.n_ctx` as a context source | +| `tests/catalog-llamacpp-capabilities.test.ts` | NEW | Drives the verbatim payload above | + +## MODIFY 1 — `modelInputModalities()` + +Before: + +```ts + if (capabilityRecord?.vision === true || capabilities?.some(value => ( + value === "vision" || value === "image-input" || value === "image_input" + ))) { + return ["text", "image"]; + } +``` + +After: + +```ts + if (capabilityRecord?.vision === true || capabilities?.some(value => ( + value === "vision" || value === "image-input" || value === "image_input" + // llama.cpp / Ollama-compatible servers report vision as "multimodal" in + // their capability list; it is the only image signal those servers emit. + || value === "multimodal" + ))) { + return ["text", "image"]; + } +``` + +The returned value stays `["text", "image"]` — inside the closed enum, so the +catalog-rejection hazard noted in the existing comment is untouched. + +Ordering note: the explicit-list branch and the `vision === false` branch both +run BEFORE this one, so a server that says `vision: false` or lists exact +modalities still wins. `multimodal` is a last-resort inference, consistent +with how the existing capability strings are treated. + +## MODIFY 2 — `catalogHintsFromModelsApiItem()` context source + +Before: + +```ts + 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: + +```ts + const limits = plainRecord(metadata?.limits); + // llama.cpp reports the served context under `meta`: `n_ctx` is the context + // 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. + 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, + ); +``` + +`meta` entries are appended LAST so no existing provider's precedence changes: +a server that already supplies a recognized field keeps winning. + +### Type surface + +`ProviderModelsApiItem` needs a `meta?: unknown` member (read through +`plainRecord`, so no structural typing of llama.cpp internals leaks in). B +confirms the exact declaration site and amends this doc if the type is +expressed as an index signature that already permits it. + +## NEW `tests/catalog-llamacpp-capabilities.test.ts` + +```ts +test("absorbs multimodal capability and meta.n_ctx from a llama.cpp models item", () => { + const hints = catalogHintsFromModelsApiItem("lidge", { + id: "qwen3.8-27b-nvfp4", + object: "model", + owned_by: "llamacpp", + capabilities: ["completion", "multimodal"], + meta: { n_ctx: 262144, n_ctx_train: 262144 }, + }); + expect(hints.inputModalities).toEqual(["text", "image"]); + expect(hints.contextWindow).toBe(262144); +}); + +test("an explicit vision:false still beats the multimodal inference", () => { + const hints = catalogHintsFromModelsApiItem("lidge", { + id: "text-only", + capabilities: { vision: false }, + }); + expect(hints.inputModalities).toEqual(["text"]); +}); + +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); +}); +``` + +The third case is the activation scenario for the precedence comment — without +it the ordering claim is untested prose. + +## Accept criteria + +1. All three tests fail before the change and pass after (activation grounding). +2. `bun x tsc --noEmit` clean. +3. No existing catalog test regresses — this file is shared by every provider, + so the full suite runs via `ssh lidge`. +4. The modality enum stays closed to `text|image|audio`. + +## Verifier commands (PLAN-VERIFIER-REAL-01) + +| Command | Reads this change? | Notes | +|---------|-------------------|-------| +| `bun test tests/catalog-llamacpp-capabilities.test.ts` | YES — direct argument | New file | +| `bun test` (via `ssh lidge`) | YES — `provider-fetch.ts` is exercised by the existing catalog suites | Required: shared surface | +| `bun x tsc --noEmit` | YES — `src/**` in `tsconfig.json` include | | + +## Bypass record (PLAN-BYPASS-NAMED-01) + +No enforcement added. Tier: N/A. Executing surface: none. Known bypass: a +provider that reports neither a recognized modality token nor a recognized +context field still yields unknown evidence — by design, per "unknown is not +zero". Residual risk: `multimodal` is a heuristic; a server using it to mean +"audio + text" would be mislabeled as image-capable. Wording downgrade: this +is called an inference, not detection. Final enforcement layer: none. 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..587aecac45 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/030_local_model_plugin_routing.md @@ -0,0 +1,97 @@ +# 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. + +## 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: + +``` +$ 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, currently empty) | CHOSEN — resolves for every session on this host regardless of repository | + +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 | +|------|--------|------| +| `~/.codex/AGENTS.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 file exists at the documented path with the section present. +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` | YES — the changed file is the direct argument | Human-read acceptance; no automated gate observes this file | + +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. From 25d58e92e54bfe74a1a6ae3eb8d23b1d78446d75 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 05:48:57 +0900 Subject: [PATCH 02/16] docs(devlog): fold audit round 1 blockers into the capability provenance design --- .../001_capability_evidence_defect.md | 16 +- .../003_audit_synthesis_round1.md | 123 ++++++ .../010_catalog_row_shape.md | 356 +++++++++++------- .../020_live_capability_ingestion.md | 299 +++++++-------- .../030_local_model_plugin_routing.md | 9 +- 5 files changed, 502 insertions(+), 301 deletions(-) create mode 100644 devlog/_plan/260816_local_model_capability_and_plugin_routing/003_audit_synthesis_round1.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 index 5878b1faeb..11efccb64e 100644 --- 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 @@ -27,7 +27,12 @@ It also reached the on-disk Codex catalog correctly, as "input_modalities": ["text", "image"], "supports_parallel_tool_calls": true } ``` -Yet routing evidence carried neither value: +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") @@ -124,3 +129,12 @@ The chain for a custom model is: 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/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..0cfc0ba291 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/003_audit_synthesis_round1.md @@ -0,0 +1,123 @@ +# 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:** match on the explicit provenance keys from B2 first; fall back to +slug equivalence only when exactly one row matches, and leave evidence unknown +on ambiguity. + +## B5 (Medium) — ACCEPTED + +My `vision:false` precedence test passes unchanged today, so it proves nothing +about the patch. Correct construction combines `capabilities: {vision:false}` +with `capabilities:["multimodal"]` so the new branch is actually contested. + +## 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. 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 index ec62a147f9..9bf0d80045 100644 --- 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 @@ -1,195 +1,275 @@ -# 010 — Phase 1: read the catalog in the shape it is written +# 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 -`cachedCatalogModels()` must project the on-disk catalog rows it actually -receives (`slug`, `context_window`, `input_modalities`) instead of a field -shape nothing writes. After this phase, a model whose only evidence source is -the catalog carries its real `contextWindow` and `image` evidence. +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: `src/routing/capability.ts`, one new focused test file. -OUT: the evidence priority order (catalog stays the fourth fallback), the -"unknown is not zero" contract, the memoization strategy, the policy -evaluator, and every other module. +IN: the provenance stamp in `src/codex/catalog/effort.ts`, the reader in +`src/routing/capability.ts`, one new focused test. +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/routing/capability.ts` | MODIFY | Parse `slug`/`context_window`/`input_modalities`; match rows by slug equivalence | -| `tests/routing-capability-catalog.test.ts` | NEW | Regression: catalog-only evidence survives assembly | - -## MODIFY `src/routing/capability.ts` - -### 1. Row type — carry the slug, not a split identity +| `src/codex/catalog/effort.ts` | MODIFY | Stamp `opencodex_capability_provenance` when real values are applied | +| `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()` is the only place that knows a value came from a +real `CatalogModel`: it writes exclusively inside guarded blocks that test the +model's own fields. Stamp provenance there. + +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: + + // Routing evidence provenance. ensureStrictCatalogFields() later fills + // context_window/input_modalities with compatibility defaults for Codex's + // strict parser, so their presence cannot distinguish a real provider + // assertion from a synthesized placeholder. These keys record only what a + // CatalogModel actually asserted; src/routing/capability.ts reads them and + // nothing else, which is what keeps "unknown is not zero" true. + 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; + } + if (Array.isArray(model.capabilities) && model.capabilities.length > 0) { + provenance.capabilities = model.capabilities; + } + entry.opencodex_capability_provenance = provenance; + +`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. + +## MODIFY 2 — src/routing/capability.ts + +### 2a. Row type Before: -```ts -type CatalogModelRow = { - provider: string; - id: string; - contextWindow?: number; - inputModalities?: string[]; - reasoningEfforts?: string[]; - capabilities?: string[]; -}; -``` + type CatalogModelRow = { + provider: string; + id: string; + contextWindow?: number; + inputModalities?: string[]; + reasoningEfforts?: string[]; + capabilities?: string[]; + }; After: -```ts -type CatalogModelRow = { - /** Codex-facing routed slug exactly as written to the catalog file. */ - slug: string; - contextWindow?: number; - inputModalities?: string[]; - reasoningEfforts?: string[]; - capabilities?: string[]; -}; -``` + 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[]; + }; -Rationale: the catalog's identity field is the combined `slug`. Splitting it -back into `provider`/`id` here would have to re-implement the slug codec's -decode rules; comparing slugs with the codec's own equivalence helper cannot -drift from it. +`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. -### 2. Projection — read the written keys +### 2b. Projection — read the provenance block Before (the filter that discards all 17 rows): -```ts 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") } - : {}), -``` + ... After: -```ts - const rows = models - .filter((model): model is Record & { slug: string } => - typeof model === "object" && model !== null && typeof model.slug === "string" && model.slug.length > 0) - .map(model => ({ - slug: model.slug, - ...(typeof model.context_window === "number" ? { contextWindow: model.context_window } : {}), - ...(Array.isArray(model.input_modalities) - ? { inputModalities: model.input_modalities.filter((value): value is string => typeof value === "string") } + 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 } : {}), -``` - -`reasoningEfforts` and `capabilities` keep their existing guarded spreads. -They are read from `supported_reasoning_levels` only if a later phase proves -the shape; this phase does NOT invent a mapping for them, because the catalog -writes them as objects (`{ effort, description }`), not strings. Leaving them -absent preserves "unknown is not zero" — it does not regress today's -behavior, since today they are absent too. - -### 3. Lookup — slug equivalence, not field equality - -Before: - -```ts - const catalogRow = cachedCatalogModels().find(model => model.provider === providerName && model.id === modelId); -``` - -After: - -```ts - const candidateSlug = routedSlug(providerName, modelId); - const catalogRow = cachedCatalogModels().find(model => slugsEquivalent(model.slug, candidateSlug)); -``` - -New import: - -```ts -import { routedSlug, slugsEquivalent } from "../providers/slug-codec"; -``` + ...(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") } + : {}), + }]; + }); -`slugsEquivalent` handles the raw/encoded mix, so a native id containing "/" -(`zenmux/moonshotai/kimi-k3-free` → catalog slug -`zenmux/moonshotai-kimi-k3-free`) matches without a blind string replace. +A row without provenance contributes nothing — exactly today's behavior for +every row — so this can only add evidence, never remove it. -### Import-boundary check +### 2c. Lookup — unchanged -`src/providers/slug-codec.ts` imports nothing (pure string functions), so it -cannot pull `src/lab/` into `src/router.ts`. `tests/core-lab-boundary.test.ts` -is the gate that proves this and must stay green. + const catalogRow = cachedCatalogModels().find(model => model.provider === providerName && model.id === modelId); -### Performance +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. -Unchanged: the same one-time parse, the same path+mtime memo, one `routedSlug` -call per candidate (a string concat). +### 2d. Keep the adapter tool fallback unconditional (B1) -## NEW `tests/routing-capability-catalog.test.ts` +Before: -Writes a temporary catalog file in the real on-disk shape, points the catalog -path at it, and asserts assembly: + const tools = capabilities.includes("tools") + || isNative + || (catalogRow === undefined && provider !== undefined && TOOL_CAPABLE_ADAPTERS.has(provider.adapter)) + || provider?.parallelToolCalls === true + || undefined; -```ts -import { describe, expect, test } from "bun:test"; +After: -describe("candidateCapabilityEvidence catalog rows", () => { - test("carries context window and image modality from a catalog-only model", () => { - // config declares the provider but NO modelContextWindows / modelInputModalities, - // so the catalog row 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); - }); + 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; - test("matches a routed slug whose native id contains a slash", () => { - // catalog slug "zenmux/moonshotai-kimi-k3-free" vs native id "moonshotai/kimi-k3-free" - const evidence = candidateCapabilityEvidence(config, "zenmux", "moonshotai/kimi-k3-free"); - expect(evidence.contextWindow).toBe(200000); - }); +A strict widening of a positive signal. `capabilities` stays positive-only, so +nothing can turn `tools` false. - test("leaves a dimension unknown when the catalog omits it", () => { - // "unknown is not zero": a row without input_modalities must not assert image:false - const evidence = candidateCapabilityEvidence(config, "lidge", "text-only-model"); - expect(evidence.image).toBeUndefined(); - }); -}); -``` +### Import-boundary check -The catalog path helper (`readCodexCatalogPath`) resolves from environment -state; the test overrides it through the same mechanism existing catalog -tests use — B confirms that mechanism against `tests/` before writing the -file, and amends this doc if it differs. +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. The new test FAILS on the current tree (evidence lacks `contextWindow` and - `image`) and PASSES after the projection change. Recording both runs is the - activation evidence required by C-ACTIVATION-GROUNDING-01 — the failing-first - run is what proves the test observes the defect rather than passing vacuously. -2. `bun x tsc --noEmit` clean. -3. `tests/core-lab-boundary.test.ts` green (import boundary unbroken). -4. Full suite green via `ssh lidge` (shared routing surface). -5. The "unknown" case asserts `undefined`, never `false`. +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. `bun run test` green on lidge at the pushed head (B6). ## Verifier commands (PLAN-VERIFIER-REAL-01) | Command | Reads this change? | Notes | |---------|-------------------|-------| -| `bun test tests/routing-capability-catalog.test.ts` | YES — the file under test is the direct argument | New file; verified to exist after B | -| `bun x tsc --noEmit` | YES — `tsconfig.json` `include` covers `src/**` | Confirmed: repo-wide strict pass | -| `bun test tests/core-lab-boundary.test.ts` | YES — walks the import graph from `src/router.ts`, which reaches `src/routing/capability.ts` | Guards the new import | +| `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 | +| `bun run test` on lidge | YES — shared routing surface | Requires the pushed head; verify remote HEAD first | + +## 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) -This phase adds no enforcement layer; it repairs a data path. Tier: N/A. -Executing surface: none. Known bypass: N/A. Residual risk: a future catalog -schema change could desynchronize reader and writer again — the new test is -the early warning, not enforcement. Final enforcement layer: none. +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. 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 index c3f185e759..a1396e7e6b 100644 --- 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 @@ -1,197 +1,174 @@ -# 020 — Phase 2: absorb llama.cpp capability metadata from live discovery +# 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 it, anything this phase writes into the catalog is still discarded by -the reader. +without the provenance channel, anything this phase learns is still invisible to +routing. -## Goal +**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. -A local llama.cpp server that truthfully advertises multimodality and its -trained context length should produce correct routing evidence with no manual -config. Today it does not, because two upstream spellings are unrecognized. +## What the server actually returns -## Observed payload (verbatim) +`GET http://100.100.125.116:8081/v1/models` returns a dual-envelope body: -`GET http://100.100.125.116:8081/v1/models` on the `lidge` provider returns a -dual-shape body — an Ollama-style `models[]` plus an OpenAI-style `data[]`: + { "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 } } ] } -```json -{ "models": [ { "name": "qwen3.8-27b-nvfp4", "model": "qwen3.8-27b-nvfp4", - "capabilities": ["completion", "multimodal"], - "details": { "format": "gguf", "family": "" } } ], - "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, - "n_params": 27320698192, "size": 16367838528 } } ] } -``` +The image signal (`multimodal`) is in `models[]`. The context signal +(`meta.n_ctx`) is in `data[]`. -## Why both signals are dropped today +`extractProviderModelItems()` reads ONLY `data` envelopes or top-level arrays, +and says so deliberately (src/providers/model-discovery.ts:337-343): -`src/codex/catalog/provider-fetch.ts`: + // 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. -1. `modelInputModalities()` recognizes explicit `input_modalities`/`modalities` - lists, an `architecture.modality` arrow form, `capabilities.vision`, and - the capability strings `vision` / `image-input` / `image_input`. The token - `"multimodal"` is in none of those sets, so the row yields `undefined`. -2. `catalogHintsFromModelsApiItem()` reads context from - `limits.max_context_length`, `metadata.context_length`, `context_length`, - `context_size`, `max_model_len`, and `max_context_length`. llama.cpp's - `meta.n_ctx` is in none of those, so context stays unknown. +Verified by running the verbatim payload through it: one surviving item, and +`catalogHintsFromModelsApiItem` returns `{}` for it. -## Scope boundary +## Scope decision -IN: `src/codex/catalog/provider-fetch.ts` (the two readers above), one focused -test using the verbatim payload. -OUT: the closed `text|image|audio` enum (must not widen — Codex rejects the -whole catalog file on an unknown modality), the dual `models[]`/`data[]` -merge behavior, and any other provider's parsing. +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: cross-envelope merging of `models[]` into `data[]`. That would relax a +deliberately conservative discovery boundary whose comment explains why it +exists. Changing it belongs in its own audited unit, not as a rider here. It +becomes a filed issue carrying the verbatim payload (wp2). + +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 | Accept `multimodal` as an image signal; read `meta.n_ctx` as a context source | -| `tests/catalog-llamacpp-capabilities.test.ts` | NEW | Drives the verbatim payload above | - -## MODIFY 1 — `modelInputModalities()` - -Before: - -```ts - if (capabilityRecord?.vision === true || capabilities?.some(value => ( - value === "vision" || value === "image-input" || value === "image_input" - ))) { - return ["text", "image"]; - } -``` - -After: - -```ts - if (capabilityRecord?.vision === true || capabilities?.some(value => ( - value === "vision" || value === "image-input" || value === "image_input" - // llama.cpp / Ollama-compatible servers report vision as "multimodal" in - // their capability list; it is the only image signal those servers emit. - || value === "multimodal" - ))) { - return ["text", "image"]; - } -``` - -The returned value stays `["text", "image"]` — inside the closed enum, so the -catalog-rejection hazard noted in the existing comment is untouched. - -Ordering note: the explicit-list branch and the `vision === false` branch both -run BEFORE this one, so a server that says `vision: false` or lists exact -modalities still wins. `multimodal` is a last-resort inference, consistent -with how the existing capability strings are treated. +| `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 2 — `catalogHintsFromModelsApiItem()` context source +## MODIFY — catalogHintsFromModelsApiItem() Before: -```ts - 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, - ); -``` + 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: -```ts - const limits = plainRecord(metadata?.limits); - // llama.cpp reports the served context under `meta`: `n_ctx` is the context - // 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. - 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, - ); -``` - -`meta` entries are appended LAST so no existing provider's precedence changes: -a server that already supplies a recognized field keeps winning. - -### Type surface - -`ProviderModelsApiItem` needs a `meta?: unknown` member (read through -`plainRecord`, so no structural typing of llama.cpp internals leaks in). B -confirms the exact declaration site and amends this doc if the type is -expressed as an index signature that already permits it. - -## NEW `tests/catalog-llamacpp-capabilities.test.ts` - -```ts -test("absorbs multimodal capability and meta.n_ctx from a llama.cpp models item", () => { - const hints = catalogHintsFromModelsApiItem("lidge", { - id: "qwen3.8-27b-nvfp4", - object: "model", - owned_by: "llamacpp", - capabilities: ["completion", "multimodal"], - meta: { n_ctx: 262144, n_ctx_train: 262144 }, - }); - expect(hints.inputModalities).toEqual(["text", "image"]); - expect(hints.contextWindow).toBe(262144); -}); - -test("an explicit vision:false still beats the multimodal inference", () => { - const hints = catalogHintsFromModelsApiItem("lidge", { - id: "text-only", - capabilities: { vision: false }, - }); - expect(hints.inputModalities).toEqual(["text"]); -}); - -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); -}); -``` - -The third case is the activation scenario for the precedence comment — without -it the ordering claim is untested prose. + 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. All three tests fail before the change and pass after (activation grounding). -2. `bun x tsc --noEmit` clean. -3. No existing catalog test regresses — this file is shared by every provider, - so the full suite runs via `ssh lidge`. -4. The modality enum stays closed to `text|image|audio`. +1. Tests 1-3 fail before the change and pass after (activation grounding). +2. Test 4 passes before AND after; it is a characterization test for the gap + handed to the filed issue. +3. `bun x tsc --noEmit` clean. +4. `bun run test` green on lidge at the pushed head — `provider-fetch.ts` is a + shared surface touched by many catalog suites. ## Verifier commands (PLAN-VERIFIER-REAL-01) | Command | Reads this change? | Notes | |---------|-------------------|-------| -| `bun test tests/catalog-llamacpp-capabilities.test.ts` | YES — direct argument | New file | -| `bun test` (via `ssh lidge`) | YES — `provider-fetch.ts` is exercised by the existing catalog suites | Required: shared surface | -| `bun x tsc --noEmit` | YES — `src/**` in `tsconfig.json` include | | +| `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 | +| `bun run test` on lidge | YES — existing catalog suites exercise `provider-fetch.ts` | Required: shared surface; verify remote HEAD first | + +## 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 -provider that reports neither a recognized modality token nor a recognized -context field still yields unknown evidence — by design, per "unknown is not -zero". Residual risk: `multimodal` is a heuristic; a server using it to mean -"audio + text" would be mislabeled as image-capable. Wording downgrade: this -is called an inference, not detection. Final enforcement layer: none. +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. 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 index 587aecac45..2c9ff9e8ec 100644 --- 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 @@ -81,7 +81,14 @@ availability. | Command | Reads this change? | Notes | |---------|-------------------|-------| -| `cat ~/.codex/AGENTS.md` | YES — the changed file is the direct argument | Human-read acceptance; no automated gate observes this file | +| `cat ~/.codex/AGENTS.md` | Partially — proves bytes on disk, NOT that Codex loaded them | Human-read acceptance | +| Fresh Codex process reporting its loaded instructions | YES — observes actual resolution | The only check that proves the guidance is live | + +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 From ec88666a5118273aa837323144a365fef63361b7 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 05:57:46 +0900 Subject: [PATCH 03/16] docs(devlog): fold audit round 2 precision fixes into the phase docs --- .../003_audit_synthesis_round1.md | 26 ++++++++++--- .../010_catalog_row_shape.md | 23 ++++++++++- .../020_live_capability_ingestion.md | 38 +++++++++++++++++-- .../030_local_model_plugin_routing.md | 2 +- 4 files changed, 76 insertions(+), 13 deletions(-) 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 index 0cfc0ba291..0dc5ca59bd 100644 --- 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 @@ -78,15 +78,19 @@ instead. `find()` on slug equivalence can attach one model's evidence to another. Rare, but silent and wrong when it happens. -**Amendment:** match on the explicit provenance keys from B2 first; fall back to -slug equivalence only when exactly one row matches, and leave evidence unknown -on ambiguity. +**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 `vision:false` precedence test passes unchanged today, so it proves nothing -about the patch. Correct construction combines `capabilities: {vision:false}` -with `capabilities:["multimodal"]` so the new branch is actually contested. +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 @@ -121,3 +125,13 @@ that the reproduction requires a catalog-only fixture. 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/010_catalog_row_shape.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/010_catalog_row_shape.md index 9bf0d80045..72580f35a2 100644 --- 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 @@ -242,7 +242,7 @@ amends this doc if it differs. regression the audit predicted. 4. `bun x tsc --noEmit` clean. 5. `tests/core-lab-boundary.test.ts` green. -6. `bun run test` green on lidge at the pushed head (B6). +6. Remote exact-head suite green on lidge (command in the section below). ## Verifier commands (PLAN-VERIFIER-REAL-01) @@ -251,7 +251,7 @@ amends this doc if it differs. | `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 | -| `bun run test` on lidge | YES — shared routing surface | Requires the pushed head; verify remote HEAD first | +| Remote exact-head suite (command below) | YES — shared routing surface | Required: shared surface | ## Field chain (PLAN-FIELD-CHAIN-01) @@ -273,3 +273,22 @@ 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 (B6/round-2 B2) + +Pushing a branch updates a remote ref, not a remote checkout. Audit round 2 +confirmed all three lidge checkouts sat on unrelated commits. The verifier must +therefore fetch and assert the SHA before running: + + LOCAL_SHA=$(git rev-parse HEAD) + ssh lidge "cd ~/ocx-ci/opencodex \\ + && git fetch --quiet csa906 \\ + && git checkout --quiet --detach FETCH_HEAD \\ + && test \"\$(git rev-parse HEAD)\" = \"$LOCAL_SHA\" \\ + && bun install --frozen-lockfile \\ + && bun run test" + +The `test` comparison is the gate: a mismatched checkout fails the command +instead of silently reporting a green suite for different code. `~/ocx-ci/opencodex` +is the chosen checkout (verified present, `origin` = lidge-jun/opencodex, on `dev`); +the push remote `csa906` must be added there if absent. 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 index a1396e7e6b..5b92f36020 100644 --- 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 @@ -138,9 +138,20 @@ The fourth test is the honest part: it encodes what this phase does NOT fix. ## Accept criteria -1. Tests 1-3 fail before the change and pass after (activation grounding). -2. Test 4 passes before AND after; it is a characterization test for the gap - handed to the filed issue. +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. 3. `bun x tsc --noEmit` clean. 4. `bun run test` green on lidge at the pushed head — `provider-fetch.ts` is a shared surface touched by many catalog suites. @@ -151,7 +162,7 @@ The fourth test is the honest part: it encodes what this phase does NOT fix. |---------|-------------------|-------| | `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 | -| `bun run test` on lidge | YES — existing catalog suites exercise `provider-fetch.ts` | Required: shared surface; verify remote HEAD first | +| Remote exact-head suite (command below) | YES — existing catalog suites exercise `provider-fetch.ts` | Required: shared surface | ## Field chain (PLAN-FIELD-CHAIN-01) @@ -172,3 +183,22 @@ 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 (B6/round-2 B2) + +Pushing a branch updates a remote ref, not a remote checkout. Audit round 2 +confirmed all three lidge checkouts sat on unrelated commits. The verifier must +therefore fetch and assert the SHA before running: + + LOCAL_SHA=$(git rev-parse HEAD) + ssh lidge "cd ~/ocx-ci/opencodex \\ + && git fetch --quiet csa906 \\ + && git checkout --quiet --detach FETCH_HEAD \\ + && test \"\$(git rev-parse HEAD)\" = \"$LOCAL_SHA\" \\ + && bun install --frozen-lockfile \\ + && bun run test" + +The `test` comparison is the gate: a mismatched checkout fails the command +instead of silently reporting a green suite for different code. `~/ocx-ci/opencodex` +is the chosen checkout (verified present, `origin` = lidge-jun/opencodex, on `dev`); +the push remote `csa906` must be added there if absent. 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 index 2c9ff9e8ec..6951ccd612 100644 --- 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 @@ -82,7 +82,7 @@ availability. | Command | Reads this change? | Notes | |---------|-------------------|-------| | `cat ~/.codex/AGENTS.md` | Partially — proves bytes on disk, NOT that Codex loaded them | Human-read acceptance | -| Fresh Codex process reporting its loaded instructions | YES — observes actual resolution | The only check that proves the guidance is live | +| `OCX_SHIM_BYPASS=1 codex -C debug prompt-input \| rg "mcp__node_repl__js"` | YES — renders the instructions the model actually receives | 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 From ce116b93251627768f859b105e27c1b6091966eb Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 06:01:43 +0900 Subject: [PATCH 04/16] docs(devlog): make the remote exact-head verifier self-contained --- .../010_catalog_row_shape.md | 45 ++++++++++++------- .../020_live_capability_ingestion.md | 45 ++++++++++++------- 2 files changed, 60 insertions(+), 30 deletions(-) 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 index 72580f35a2..5b287a1a38 100644 --- 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 @@ -274,21 +274,36 @@ 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 (B6/round-2 B2) +## Remote exact-head suite (round-3 correction) -Pushing a branch updates a remote ref, not a remote checkout. Audit round 2 -confirmed all three lidge checkouts sat on unrelated commits. The verifier must -therefore fetch and assert the SHA before running: +Pushing a branch updates a remote ref, not a remote checkout. Audit rounds 2-3 +found every lidge checkout on an unrelated commit, `~/ocx-ci/opencodex` carrying +uncommitted work, and no `csa906` remote configured there. The verifier must +therefore be self-contained: clone the exact SHA into a scratch directory and +never touch an existing checkout. LOCAL_SHA=$(git rev-parse HEAD) - ssh lidge "cd ~/ocx-ci/opencodex \\ - && git fetch --quiet csa906 \\ - && git checkout --quiet --detach FETCH_HEAD \\ - && test \"\$(git rev-parse HEAD)\" = \"$LOCAL_SHA\" \\ - && bun install --frozen-lockfile \\ - && bun run test" - -The `test` comparison is the gate: a mismatched checkout fails the command -instead of silently reporting a green suite for different code. `~/ocx-ci/opencodex` -is the chosen checkout (verified present, `origin` = lidge-jun/opencodex, on `dev`); -the push remote `csa906` must be added there if absent. + ssh lidge "set -e + 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" + +Why a scratch clone rather than a shared checkout: + +- `~/ocx-ci/opencodex` had modified `src/bridge.ts`, + `src/server/responses/core.ts` and others at audit time. A detach there can + refuse outright, or worse, run the suite against someone else's in-progress + edits and report a green that means nothing about this change. +- `mktemp -d` plus the `trap` cleanup keeps the run leaving no residue, so it + cannot drift into the same stale state next time. +- The `test` SHA comparison is retained and still fails closed: if the checkout + is not the exact audited commit, the command aborts before `bun run test`. + +The push remote is addressed by URL, so no remote needs to be configured on the +host. 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 index 5b92f36020..bdbe6331b3 100644 --- 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 @@ -184,21 +184,36 @@ 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 (B6/round-2 B2) +## Remote exact-head suite (round-3 correction) -Pushing a branch updates a remote ref, not a remote checkout. Audit round 2 -confirmed all three lidge checkouts sat on unrelated commits. The verifier must -therefore fetch and assert the SHA before running: +Pushing a branch updates a remote ref, not a remote checkout. Audit rounds 2-3 +found every lidge checkout on an unrelated commit, `~/ocx-ci/opencodex` carrying +uncommitted work, and no `csa906` remote configured there. The verifier must +therefore be self-contained: clone the exact SHA into a scratch directory and +never touch an existing checkout. LOCAL_SHA=$(git rev-parse HEAD) - ssh lidge "cd ~/ocx-ci/opencodex \\ - && git fetch --quiet csa906 \\ - && git checkout --quiet --detach FETCH_HEAD \\ - && test \"\$(git rev-parse HEAD)\" = \"$LOCAL_SHA\" \\ - && bun install --frozen-lockfile \\ - && bun run test" - -The `test` comparison is the gate: a mismatched checkout fails the command -instead of silently reporting a green suite for different code. `~/ocx-ci/opencodex` -is the chosen checkout (verified present, `origin` = lidge-jun/opencodex, on `dev`); -the push remote `csa906` must be added there if absent. + ssh lidge "set -e + 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" + +Why a scratch clone rather than a shared checkout: + +- `~/ocx-ci/opencodex` had modified `src/bridge.ts`, + `src/server/responses/core.ts` and others at audit time. A detach there can + refuse outright, or worse, run the suite against someone else's in-progress + edits and report a green that means nothing about this change. +- `mktemp -d` plus the `trap` cleanup keeps the run leaving no residue, so it + cannot drift into the same stale state next time. +- The `test` SHA comparison is retained and still fails closed: if the checkout + is not the exact audited commit, the command aborts before `bun run test`. + +The push remote is addressed by URL, so no remote needs to be configured on the +host. From 8c74344597f9a77d37d5ae5be0b564b202d6df32 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 06:14:48 +0900 Subject: [PATCH 05/16] docs(devlog): fold audit round 4 blockers and link issues #1796/#1797 --- .../000_plan.md | 10 ++ .../010_catalog_row_shape.md | 104 +++++++++++++++--- .../020_live_capability_ingestion.md | 73 ++++++++---- 3 files changed, 148 insertions(+), 39 deletions(-) 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 index b439a10cf6..7201fc113c 100644 --- 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 @@ -54,3 +54,13 @@ one exists; if none does, the D summary recommends creating it. 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/010_catalog_row_shape.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/010_catalog_row_shape.md index 5b287a1a38..0289d1c825 100644 --- 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 @@ -92,6 +92,67 @@ 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. + ## MODIFY 2 — src/routing/capability.ts ### 2a. Row type @@ -274,16 +335,28 @@ 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-3 correction) +## Remote exact-head suite (round-4 correction) + +Three rounds of audit found three separate reasons a naive remote command lies: +the lidge checkouts sit on unrelated commits, `~/ocx-ci/opencodex` carries +uncommitted work and has no `csa906` remote, and a non-interactive SSH shell has +no `bun` on `PATH` (`command -v bun` is empty while `~/.bun/bin/bun` exists). +The block below addresses all three and fails closed on each. -Pushing a branch updates a remote ref, not a remote checkout. Audit rounds 2-3 -found every lidge checkout on an unrelated commit, `~/ocx-ci/opencodex` carrying -uncommitted work, and no `csa906` remote configured there. The verifier must -therefore be self-contained: clone the exact SHA into a scratch directory and -never touch an existing checkout. +Preconditions asserted locally BEFORE any ssh: LOCAL_SHA=$(git rev-parse HEAD) + BRANCH=$(git rev-parse --abbrev-ref HEAD) + git push --no-verify csa906 "$BRANCH" + # The remote must actually carry this commit; an unpushed SHA makes the + # remote fetch fail with 'upload-pack: not our ref'. + test "$(git ls-remote csa906 "refs/heads/$BRANCH" | cut -f1)" = "$LOCAL_SHA" + +Then the isolated remote run: + ssh lidge "set -e + 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\" @@ -294,16 +367,13 @@ never touch an existing checkout. bun install --frozen-lockfile bun run test" -Why a scratch clone rather than a shared checkout: +Each guard exists because a specific failure was observed: -- `~/ocx-ci/opencodex` had modified `src/bridge.ts`, - `src/server/responses/core.ts` and others at audit time. A detach there can - refuse outright, or worse, run the suite against someone else's in-progress - edits and report a green that means nothing about this change. -- `mktemp -d` plus the `trap` cleanup keeps the run leaving no residue, so it - cannot drift into the same stale state next time. -- The `test` SHA comparison is retained and still fails closed: if the checkout - is not the exact audited commit, the command aborts before `bun run test`. +| Guard | Observed failure it prevents | +|-------|------------------------------| +| `git ls-remote` SHA test | `fatal: remote error: upload-pack: not our ref 60fd5a7d9...` when the branch was not pushed | +| `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 suite run there proves nothing about this change | +| `git rev-parse HEAD` test | a stale checkout silently reporting a green suite for different code | -The push remote is addressed by URL, so no remote needs to be configured on the -host. +C must run this literal block and paste its output; a paraphrase is not evidence. 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 index bdbe6331b3..61ddcd53f8 100644 --- 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 @@ -36,10 +36,26 @@ 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: cross-envelope merging of `models[]` into `data[]`. That would relax a -deliberately conservative discovery boundary whose comment explains why it -exists. Changing it belongs in its own audited unit, not as a rider here. It -becomes a filed issue carrying the verbatim payload (wp2). +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 }` @@ -151,7 +167,11 @@ The fourth test is the honest part: it encodes what this phase does NOT fix. 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. + 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. 3. `bun x tsc --noEmit` clean. 4. `bun run test` green on lidge at the pushed head — `provider-fetch.ts` is a shared surface touched by many catalog suites. @@ -184,16 +204,28 @@ 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-3 correction) +## Remote exact-head suite (round-4 correction) -Pushing a branch updates a remote ref, not a remote checkout. Audit rounds 2-3 -found every lidge checkout on an unrelated commit, `~/ocx-ci/opencodex` carrying -uncommitted work, and no `csa906` remote configured there. The verifier must -therefore be self-contained: clone the exact SHA into a scratch directory and -never touch an existing checkout. +Three rounds of audit found three separate reasons a naive remote command lies: +the lidge checkouts sit on unrelated commits, `~/ocx-ci/opencodex` carries +uncommitted work and has no `csa906` remote, and a non-interactive SSH shell has +no `bun` on `PATH` (`command -v bun` is empty while `~/.bun/bin/bun` exists). +The block below addresses all three and fails closed on each. + +Preconditions asserted locally BEFORE any ssh: LOCAL_SHA=$(git rev-parse HEAD) + BRANCH=$(git rev-parse --abbrev-ref HEAD) + git push --no-verify csa906 "$BRANCH" + # The remote must actually carry this commit; an unpushed SHA makes the + # remote fetch fail with 'upload-pack: not our ref'. + test "$(git ls-remote csa906 "refs/heads/$BRANCH" | cut -f1)" = "$LOCAL_SHA" + +Then the isolated remote run: + ssh lidge "set -e + 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\" @@ -204,16 +236,13 @@ never touch an existing checkout. bun install --frozen-lockfile bun run test" -Why a scratch clone rather than a shared checkout: +Each guard exists because a specific failure was observed: -- `~/ocx-ci/opencodex` had modified `src/bridge.ts`, - `src/server/responses/core.ts` and others at audit time. A detach there can - refuse outright, or worse, run the suite against someone else's in-progress - edits and report a green that means nothing about this change. -- `mktemp -d` plus the `trap` cleanup keeps the run leaving no residue, so it - cannot drift into the same stale state next time. -- The `test` SHA comparison is retained and still fails closed: if the checkout - is not the exact audited commit, the command aborts before `bun run test`. +| Guard | Observed failure it prevents | +|-------|------------------------------| +| `git ls-remote` SHA test | `fatal: remote error: upload-pack: not our ref 60fd5a7d9...` when the branch was not pushed | +| `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 suite run there proves nothing about this change | +| `git rev-parse HEAD` test | a stale checkout silently reporting a green suite for different code | -The push remote is addressed by URL, so no remote needs to be configured on the -host. +C must run this literal block and paste its output; a paraphrase is not evidence. From 94ec4ce0239c12e2ebc336478fe5a8c90230ec3c Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 06:27:46 +0900 Subject: [PATCH 06/16] docs(devlog): fold audit round 5 blockers (fail-closed remote steps, antigravity tri-state) --- .../010_catalog_row_shape.md | 125 +++++++++++++++--- .../020_live_capability_ingestion.md | 53 +++++--- 2 files changed, 141 insertions(+), 37 deletions(-) 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 index 0289d1c825..debab2996c 100644 --- 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 @@ -41,7 +41,8 @@ evaluator, reasoning-effort ingestion, and every other module. | Path | Action | What | |------|--------|------| -| `src/codex/catalog/effort.ts` | MODIFY | Stamp `opencodex_capability_provenance` when real values are applied | +| `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/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 | @@ -153,6 +154,75 @@ 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. +### 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 @@ -335,26 +405,36 @@ 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-4 correction) +## Remote exact-head suite (round-5 correction) -Three rounds of audit found three separate reasons a naive remote command lies: -the lidge checkouts sit on unrelated commits, `~/ocx-ci/opencodex` carries -uncommitted work and has no `csa906` remote, and a non-interactive SSH shell has -no `bun` on `PATH` (`command -v bun` is empty while `~/.bun/bin/bun` exists). -The block below addresses all three and fails closed on each. +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. -Preconditions asserted locally BEFORE any ssh: +**Step 1 — publish (separate, must succeed on its own).** - LOCAL_SHA=$(git rev-parse HEAD) + set -eu BRANCH=$(git rev-parse --abbrev-ref HEAD) + LOCAL_SHA=$(git rev-parse HEAD) git push --no-verify csa906 "$BRANCH" - # The remote must actually carry this commit; an unpushed SHA makes the - # remote fetch fail with 'upload-pack: not our ref'. - test "$(git ls-remote csa906 "refs/heads/$BRANCH" | cut -f1)" = "$LOCAL_SHA" -Then the isolated remote run: +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 + REMOTE_SHA=$(git ls-remote csa906 "refs/heads/$BRANCH" | cut -f1) + test -n "$REMOTE_SHA" + test "$REMOTE_SHA" = "$LOCAL_SHA" - ssh lidge "set -e +`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.** + + ssh lidge "set -eu export PATH=\"\$HOME/.bun/bin:\$PATH\" command -v bun >/dev/null WORKDIR=\$(mktemp -d -t ocx-verify-XXXXXX) @@ -367,13 +447,20 @@ Then the isolated remote run: bun install --frozen-lockfile bun run test" -Each guard exists because a specific failure was observed: +Each guard exists because a specific failure was observed in audit: | Guard | Observed failure it prevents | |-------|------------------------------| -| `git ls-remote` SHA test | `fatal: remote error: upload-pack: not our ref 60fd5a7d9...` when the branch was not pushed | +| 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 suite run there proves nothing about this change | -| `git rev-parse HEAD` test | a stale checkout silently reporting a green suite for different code | +| `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 | + +C runs these three literal steps in order and pastes each exit code. A green +suite whose publication step failed is not evidence. -C must run this literal block and paste its output; a paraphrase 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 index 61ddcd53f8..adfc88225c 100644 --- 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 @@ -204,26 +204,36 @@ 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-4 correction) +## Remote exact-head suite (round-5 correction) -Three rounds of audit found three separate reasons a naive remote command lies: -the lidge checkouts sit on unrelated commits, `~/ocx-ci/opencodex` carries -uncommitted work and has no `csa906` remote, and a non-interactive SSH shell has -no `bun` on `PATH` (`command -v bun` is empty while `~/.bun/bin/bun` exists). -The block below addresses all three and fails closed on each. +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. -Preconditions asserted locally BEFORE any ssh: +**Step 1 — publish (separate, must succeed on its own).** - LOCAL_SHA=$(git rev-parse HEAD) + set -eu BRANCH=$(git rev-parse --abbrev-ref HEAD) + LOCAL_SHA=$(git rev-parse HEAD) git push --no-verify csa906 "$BRANCH" - # The remote must actually carry this commit; an unpushed SHA makes the - # remote fetch fail with 'upload-pack: not our ref'. - test "$(git ls-remote csa906 "refs/heads/$BRANCH" | cut -f1)" = "$LOCAL_SHA" -Then the isolated remote run: +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 + REMOTE_SHA=$(git ls-remote csa906 "refs/heads/$BRANCH" | cut -f1) + test -n "$REMOTE_SHA" + test "$REMOTE_SHA" = "$LOCAL_SHA" - ssh lidge "set -e +`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.** + + ssh lidge "set -eu export PATH=\"\$HOME/.bun/bin:\$PATH\" command -v bun >/dev/null WORKDIR=\$(mktemp -d -t ocx-verify-XXXXXX) @@ -236,13 +246,20 @@ Then the isolated remote run: bun install --frozen-lockfile bun run test" -Each guard exists because a specific failure was observed: +Each guard exists because a specific failure was observed in audit: | Guard | Observed failure it prevents | |-------|------------------------------| -| `git ls-remote` SHA test | `fatal: remote error: upload-pack: not our ref 60fd5a7d9...` when the branch was not pushed | +| 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 suite run there proves nothing about this change | -| `git rev-parse HEAD` test | a stale checkout silently reporting a green suite for different code | +| `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 | + +C runs these three literal steps in order and pastes each exit code. A green +suite whose publication step failed is not evidence. -C must run this literal block and paste its output; a paraphrase 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. From 21320bcc901a4ab9ff36f3ef355bd503c99a7398 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 06:41:53 +0900 Subject: [PATCH 07/16] docs(devlog): make each remote verifier step self-contained; widen phase 1 scope --- .../010_catalog_row_shape.md | 13 ++++++++++++- .../020_live_capability_ingestion.md | 9 +++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) 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 index debab2996c..87e4f03dfc 100644 --- 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 @@ -33,7 +33,9 @@ said anything", so it needs a separate channel. ## Scope boundary IN: the provenance stamp in `src/codex/catalog/effort.ts`, the reader in -`src/routing/capability.ts`, one new focused test. +`src/routing/capability.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. @@ -425,6 +427,8 @@ 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" @@ -434,6 +438,7 @@ 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 @@ -458,6 +463,12 @@ Each guard exists because a specific failure was observed in audit: | `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. 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 index adfc88225c..6210ca9825 100644 --- 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 @@ -224,6 +224,8 @@ 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" @@ -233,6 +235,7 @@ 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 @@ -257,6 +260,12 @@ Each guard exists because a specific failure was observed in audit: | `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. From 99b38c1ec6611a227988e44bbd2ecd06de852e57 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 06:42:40 +0900 Subject: [PATCH 08/16] docs(devlog): record the six-round audit ledger and verdict-capture caveat --- .../004_audit_rounds_2_to_6.md | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_6.md diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_6.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_6.md new file mode 100644 index 0000000000..8678bd9ea5 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_6.md @@ -0,0 +1,66 @@ +# 004 — Audit rounds 2-6: verdict ledger + +Six 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 six 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. From 57e4dd7c4dfda4147f397c35444b8f26e5680e15 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 06:53:11 +0900 Subject: [PATCH 09/16] docs(devlog): capture jawcode generated metadata in provenance (round 7) --- .../010_catalog_row_shape.md | 82 ++++++++++++++++++- .../030_local_model_plugin_routing.md | 4 +- 2 files changed, 84 insertions(+), 2 deletions(-) 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 index 87e4f03dfc..b68797a0f6 100644 --- 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 @@ -32,7 +32,8 @@ said anything", so it needs a separate channel. ## Scope boundary -IN: the provenance stamp in `src/codex/catalog/effort.ts`, the reader in +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 `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. @@ -156,6 +157,85 @@ 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. B extracts the lookup rather than +duplicating it, so the two cannot drift. + +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("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 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 index 6951ccd612..1e8ce02afa 100644 --- 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 @@ -82,7 +82,9 @@ availability. | Command | Reads this change? | Notes | |---------|-------------------|-------| | `cat ~/.codex/AGENTS.md` | Partially — proves bytes on disk, NOT that Codex loaded them | Human-read acceptance | -| `OCX_SHIM_BYPASS=1 codex -C debug prompt-input \| rg "mcp__node_repl__js"` | YES — renders the instructions the model actually receives | Proven in audit round 2: an isolated `$CODEX_HOME/AGENTS.md` carrying this guidance appeared in the prompt, exit 0. No network needed | +| `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 From adfd40f10ef164cbbb9f1c22bbd6bc4e6915145f Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 06:57:18 +0900 Subject: [PATCH 10/16] docs(devlog): own the lookup extraction, add the context-cap regression --- .../010_catalog_row_shape.md | 32 +++++++++++++++---- .../030_local_model_plugin_routing.md | 4 +-- 2 files changed, 27 insertions(+), 9 deletions(-) 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 index b68797a0f6..d4cb0394d8 100644 --- 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 @@ -34,7 +34,8 @@ said anything", so it needs a separate channel. 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 `supportsImages` tri-state restoration 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 @@ -46,14 +47,16 @@ evaluator, reasoning-effort ingestion, and every other module. |------|--------|------| | `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()` is the only place that knows a value came from a -real `CatalogModel`: it writes exclusively inside guarded blocks that test the -model's own fields. Stamp provenance there. +`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): @@ -202,8 +205,14 @@ reintroduce the B2 defect the moment `ensureStrictCatalogFields` has run. : (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. B extracts the lookup rather than -duplicating it, so the two cannot drift. +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 @@ -222,6 +231,17 @@ catch this either: 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. 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 index 1e8ce02afa..20b34c4298 100644 --- 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 @@ -82,9 +82,7 @@ availability. | 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 | +| `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 From af6bbfc2127dd4161dee9324f4175483200e1c52 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 06:58:07 +0900 Subject: [PATCH 11/16] docs(devlog): record rounds 7-8 and the writer-vs-producer lesson --- ...s_2_to_6.md => 004_audit_rounds_2_to_8.md} | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) rename devlog/_plan/260816_local_model_capability_and_plugin_routing/{004_audit_rounds_2_to_6.md => 004_audit_rounds_2_to_8.md} (51%) diff --git a/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_6.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_8.md similarity index 51% rename from devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_6.md rename to devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_8.md index 8678bd9ea5..5b17a7ab47 100644 --- a/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_6.md +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/004_audit_rounds_2_to_8.md @@ -64,3 +64,64 @@ 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. From c435340bbb5420e37b7f5e351b394271579955d4 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 07:03:29 +0900 Subject: [PATCH 12/16] fix(routing): source capability evidence from explicit catalog provenance (#1796) candidateCapabilityEvidence() read the cached Codex catalog through a filter expecting `id`/`provider`/`contextWindow`/`inputModalities`, while the catalog writer emits `slug`/`context_window`/`input_modalities`. Every row was discarded -- 17 of 17 on a live install -- so the catalog branch of the evidence chain was dead code. It stayed invisible because the catalog is the fourth fallback: any provider declaring inline capability maps is answered earlier. A model registered through `ocx models add` has no such maps, so it routed as image-blind and context-unknown. Reading the written fields directly would have been wrong. ensureStrictCatalogFields synthesizes `input_modalities: ["text"]` and `context_window: 128000` so Codex's strict parser accepts the file, so their presence cannot distinguish an assertion from a placeholder, and reading them would turn unknown into a confident `image: false` -- the opposite of this module's "unknown is not zero" contract. Instead applyCatalogModelMetadata now stamps `opencodex_capability_provenance` carrying only values a real source asserted, plus exact provider/model_id, and capability.ts reads that block and nothing else. Two real sources exist and both are consulted: the CatalogModel and the generated jawcode metadata table, whose lookup is now exported from parsing.ts so the two callers cannot drift. The generated context is capped the same way applyCatalogMetadata caps it. Three secondary defects fixed along the way: - The adapter tool fallback was gated on `catalogRow === undefined`, which was only safe while the lookup never matched. Repairing the lookup would have silently revoked `tools: true` for every openai-chat and anthropic candidate. - Synthesized combo rows (a generic 128k/text fallback) are excluded from the stamp; their values are placeholders, not assertions. - parseAntigravityAvailableModels collapsed an absent `supportsImages` into `["text"]`, making "nobody said" indistinguishable from "the provider said no". The tri-state is restored; the strict catalog still gets its default downstream. Evidence: the new test fails 5/10 without the fix and passes 10/10 with it. End to end, with the lidge provider's inline maps removed so the catalog is the only source, evidence goes from `{tools:true}` to `{contextWindow:262144, image:true, tools:true}`. Plan and eight-round audit: devlog/_plan/260816_local_model_capability_and_plugin_routing/ --- src/codex/catalog/effort.ts | 50 ++++++- src/codex/catalog/parsing.ts | 20 ++- src/providers/antigravity-models.ts | 12 +- src/routing/capability.ts | 49 +++--- tests/routing-capability-catalog.test.ts | 183 +++++++++++++++++++++++ 5 files changed, 292 insertions(+), 22 deletions(-) create mode 100644 tests/routing-capability-catalog.test.ts 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/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/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"]); + }); +}); From cc3512ed0a2d0a5a7a9a4b976f839689d020c3d6 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 07:04:15 +0900 Subject: [PATCH 13/16] fix(catalog): read llama.cpp served context from meta.n_ctx (#1797) A llama.cpp server reports its context under `meta.n_ctx` (what the server was started with) and `meta.n_ctx_train` (the model's trained maximum). Neither was in the recognized context list, so a correct local server produced no context evidence at all. Both are appended LAST in the precedence chain, so any provider already supplying a recognized field keeps its current behavior. `n_ctx` is preferred over `n_ctx_train` because routing must not promise a window the running server will refuse. The image half of #1797 is deliberately NOT fixed here. The `multimodal` token lives in the Ollama-style `models[]` array while extractProviderModelItems reads only `data[]` envelopes, and its comment is explicit that a stray `models` key must not be trusted. Even a hand-merged item would stay image-unknown, because `multimodal` is not among the recognized capability strings. Both halves need an identity-safe join and a capability mapping, which is its own audited change. The fourth test characterizes that gap so the follow-up has a live witness. Verified: tests 1, 2 and 4 fail without this change and pass with it; test 3 passes either way and guards the precedence ordering. --- src/codex/catalog/provider-fetch.ts | 7 ++ tests/catalog-llamacpp-capabilities.test.ts | 80 +++++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 tests/catalog-llamacpp-capabilities.test.ts 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/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(); + }); +}); From f9ef0a3e230abdd4adc5a78c4e87a280591e0225 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 08:28:53 +0900 Subject: [PATCH 14/16] docs(devlog): record the implementation, verification evidence, and CI baseline --- .../040_implementation_record.md | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 devlog/_plan/260816_local_model_capability_and_plugin_routing/040_implementation_record.md 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..973e2cc298 --- /dev/null +++ b/devlog/_plan/260816_local_model_capability_and_plugin_routing/040_implementation_record.md @@ -0,0 +1,93 @@ +# 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 | `~/.codex/AGENTS.md` (host, untracked) | n/a | + +## 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. From 65f269f1b44b8efb18a2119714f50dc0d9dc0b2d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 08:42:58 +0900 Subject: [PATCH 15/16] docs: address CodeRabbit review findings on #1799 Five threads, all accepted: - Move the browser/Computer Use plugin routing guidance out of the untracked host file and into the tracked AGENTS_INSTALL.md. Listing ~/.codex/AGENTS.md as shipped was wrong: it is not in the PR and cannot be reproduced from a clone. The host copy is now recorded as host-only verification. - Mark the first provenance snippet in 010 as superseded and add one authoritative "Final stamp algorithm" section carrying the combo guard, the generated-metadata fallback, and the context cap, verbatim from effort.ts. Three separate audit rounds amended that snippet; leaving the original inline invited implementing from the wrong one. - Correct the 004 ledger scope from six rounds to eight, matching the rounds it actually records. - Renumber the 020 accept criteria, which restarted at 2 and repeated 3. - Add console language tags to the two transcript fences (MD040). --- AGENTS_INSTALL.md | 32 ++++++++ .../002_local_model_plugin_failure.md | 2 +- .../004_audit_rounds_2_to_8.md | 6 +- .../010_catalog_row_shape.md | 55 +++++++++++--- .../020_live_capability_ingestion.md | 4 +- .../030_local_model_plugin_routing.md | 2 +- .../040_implementation_record.md | 17 ++++- .../260816_pr_merge_campaign/000_triage.md | 75 +++++++++++++++++++ 8 files changed, 174 insertions(+), 19 deletions(-) create mode 100644 devlog/_plan/260816_pr_merge_campaign/000_triage.md 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/002_local_model_plugin_failure.md b/devlog/_plan/260816_local_model_capability_and_plugin_routing/002_local_model_plugin_failure.md index a8788bd8b2..86fb488b2c 100644 --- 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 @@ -19,7 +19,7 @@ The tooling was available the whole time. 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 ``` 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 index 5b17a7ab47..ae09267f5b 100644 --- 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 @@ -1,6 +1,6 @@ -# 004 — Audit rounds 2-6: verdict ledger +# 004 — Audit rounds 2-8: verdict ledger -Six audit rounds ran against this plan. `003_audit_synthesis_round1.md` covers +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. @@ -15,7 +15,7 @@ how the verdicts were captured. | 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 six rounds was ACCEPTED and folded. None were rebutted. +Every finding across all eight rounds was ACCEPTED and folded. None were rebutted. ## What the audit actually prevented 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 index d4cb0394d8..006db8918b 100644 --- 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 @@ -69,14 +69,12 @@ Existing shape (unchanged): entry.input_modalities = model.inputModalities; } -Added at the end of the function: - - // Routing evidence provenance. ensureStrictCatalogFields() later fills - // context_window/input_modalities with compatibility defaults for Codex's - // strict parser, so their presence cannot distinguish a real provider - // assertion from a synthesized placeholder. These keys record only what a - // CatalogModel actually asserted; src/routing/capability.ts reads them and - // nothing else, which is what keeps "unknown is not zero" true. +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; @@ -84,11 +82,46 @@ Added at the end of the function: if (Array.isArray(model.inputModalities) && model.inputModalities.length > 0) { provenance.input_modalities = model.inputModalities; } - if (Array.isArray(model.capabilities) && model.capabilities.length > 0) { - provenance.capabilities = model.capabilities; - } 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). 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 index 6210ca9825..568203c6a3 100644 --- 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 @@ -172,8 +172,8 @@ The fourth test is the honest part: it encodes what this phase does NOT fix. 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. -3. `bun x tsc --noEmit` clean. -4. `bun run test` green on lidge at the pushed head — `provider-fetch.ts` is a +4. `bun x tsc --noEmit` clean. +5. `bun run test` green on lidge at the pushed head — `provider-fetch.ts` is a shared surface touched by many catalog suites. ## Verifier commands (PLAN-VERIFIER-REAL-01) 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 index 20b34c4298..12b7d577cf 100644 --- 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 @@ -13,7 +13,7 @@ Node script before declaring the tooling unavailable. 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 ``` 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 index 973e2cc298..d1bb4f8560 100644 --- 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 @@ -14,7 +14,22 @@ and opened as PR #1799 against `dev`. | `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 | `~/.codex/AGENTS.md` (host, untracked) | n/a | +| 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) 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..ae5f9c9d31 --- /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. From d3f63e6048ebb6bc37129eedcc22e4b52aa88ea1 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 08:53:08 +0900 Subject: [PATCH 16/16] docs: address the second CodeRabbit round on #1799 - Criterion 5 in 020 claimed the full suite was green. It was not: the recorded run has seven GUI dependency failures. Restated to the honest bar - the targeted suites pass, the full run carries only documented environment failures, and no failure is attributable to this change. - 030 still selected ~/.codex/AGENTS.md as the placement and verified it there, contradicting the correction already recorded in 040. Marked the placement superseded and pointed every row at the tracked AGENTS_INSTALL.md. - Backticked the bare PR references in the triage doc (MD018). --- .../020_live_capability_ingestion.md | 14 ++++++++++++-- .../030_local_model_plugin_routing.md | 16 +++++++++++++--- .../_plan/260816_pr_merge_campaign/000_triage.md | 4 ++-- 3 files changed, 27 insertions(+), 7 deletions(-) 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 index 568203c6a3..f4d52ec560 100644 --- 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 @@ -173,8 +173,18 @@ The fourth test is the honest part: it encodes what this phase does NOT fix. `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. `bun run test` green on lidge at the pushed head — `provider-fetch.ts` is a - shared surface touched by many catalog suites. +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) 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 index 12b7d577cf..756a3166f9 100644 --- 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 @@ -3,6 +3,15 @@ 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 @@ -38,7 +47,8 @@ Candidates considered: |-----------|---------| | 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, currently empty) | CHOSEN — resolves for every session on this host regardless of repository | +| `~/.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. @@ -47,7 +57,7 @@ is additive with no merge risk. | Path | Action | What | |------|--------|------| -| `~/.codex/AGENTS.md` | MODIFY (append) | Browser-plugin routing rule | +| `AGENTS_INSTALL.md` | MODIFY (append) | Browser-plugin routing rule | ## Content to append @@ -71,7 +81,7 @@ availability. ## Accept criteria -1. The file exists at the documented path with the section present. +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. diff --git a/devlog/_plan/260816_pr_merge_campaign/000_triage.md b/devlog/_plan/260816_pr_merge_campaign/000_triage.md index ae5f9c9d31..8936db15cb 100644 --- a/devlog/_plan/260816_pr_merge_campaign/000_triage.md +++ b/devlog/_plan/260816_pr_merge_campaign/000_triage.md @@ -23,7 +23,7 @@ that CI is red. It is not. | #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 +`#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 @@ -45,7 +45,7 @@ The `macos` job is a pre-existing Bun segfault that also fails on unmodified ## Conflict found during triage -#1793 and #1799 both modify `src/providers/model-discovery.ts`, and #1793 also +`#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