From 1cbb8c470256d5237beb79f4274f3ef5a8a66d38 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 11:08:20 +0900 Subject: [PATCH 1/9] =?UTF-8?q?docs(devlog):=20plan=20stack=20layer=207=20?= =?UTF-8?q?=E2=80=94=20NIM=20vision=20classification=20and=20service=20rep?= =?UTF-8?q?air?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two overnight contributor PRs describe real defects the #951-#973 stack does not touch. This unit plans layer 7 as their reconstruction. #964 cannot be carried: five ids in its hand-written text-only list are natively image-capable per NVIDIA's own docs (inkling, minimax-m3, kimi-k2.6, step-3.7-flash, mistral-medium-3.5-128b). A false positive there is silent — the model can read the image, but the proxy substitutes another model's text description. Issue #956's own body carries two of the same errors, so reporter and author shared the premise. 010 inverts the design: maintain the 15 verified vision-capable ids and derive text-only as the complement, so an unclassified new model defaults to sidecar-on rather than to the bug being fixed. #970's premise is right but its diff is oversized: repairService() and 'ocx service repair' already exist here. 020 records the safety proof that matters — repair throws when not installed and the update path runs after 'ocx stop', but stop never deregisters on any of the three platforms. It also closes a hole #970 leaves: bin/ocx.mjs infers service presence from a possibly-stale marker, where repair would throw and lose the managed service. 030 sequences the bottom-up merge and issue closure, including the #954 security-review gate that can legitimately stop the queue. --- .../260804_stack7_service_vision/000_scope.md | 76 ++++++++ .../010_nim_vision_classification.md | 174 ++++++++++++++++++ .../020_service_repair_path.md | 133 +++++++++++++ .../030_merge_and_close_sequence.md | 91 +++++++++ 4 files changed, 474 insertions(+) create mode 100644 devlog/_plan/260804_stack7_service_vision/000_scope.md create mode 100644 devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md create mode 100644 devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md create mode 100644 devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md diff --git a/devlog/_plan/260804_stack7_service_vision/000_scope.md b/devlog/_plan/260804_stack7_service_vision/000_scope.md new file mode 100644 index 000000000..74ae3017b --- /dev/null +++ b/devlog/_plan/260804_stack7_service_vision/000_scope.md @@ -0,0 +1,76 @@ +# 000 — Scope: stack layer 7, the two real-but-off-theme contributor bugs + +## Objective + +Two overnight contributor pull requests describe real defects that the #951–#973 +stack does not touch. Both were left open at the end of the overnight triage +(`devlog/_plan/260804_overnight_triage/000_dispositions.md`) with "real, +independent, own review track" as the verdict. This unit turns that verdict into +a seventh stack layer, reconstructed here, and closes the source pull requests +as superseded. + +| Source PR | Author | Issue | Defect | +|---|---|---|---| +| #964 | @Yuxin-Qiao | #956 | NVIDIA NIM text-only models never activate the vision sidecar | +| #970 | @stephen-drew | — | `ocx update` re-registers the background service from a non-elevated updater | + +Layer 7 is the last layer. After it lands the stack merges bottom-up from #952 +and every issue a landed layer resolves gets closed with its merge commit named. + +## Baseline + +Measured 2026-08-04. `origin/dev` at `af3ddedb4` — layer 1 (#951) is **merged**, +so the chain is now six open layers, not six-of-six pending: + +| PR | Branch | Base | State | +|---|---|---|---| +| #951 | `codex/bug-stack-plan` | `dev` | **merged** `af3ddedb4` | +| #952 | `codex/908-long-context-pricing` | #951's branch | open | +| #953 | `codex/carry-contributor-bugfixes` | #952 | open | +| #954 | `codex/545-classifier-thinking-disabled` | #953 | open | +| #955 | `codex/915-cooldown-recovery-probe` | #954 | open | +| #973 | `codex/stack6-overnight-triage` | #955 | open | +| **new** | `codex/stack7-service-vision` | #973 | this unit | + +Titles currently read `stack N/6` and must be renumbered to `N/7`. + +## Why these two are reconstructed rather than carried + +The overnight unit carried six contributor fixes verbatim with `git cherry-pick -x` +because the code was right and only the base was wrong. These two are different: +each has a design defect that a straight cherry-pick would import. + +**#964** classifies NVIDIA NIM models with a hand-written 60-entry allowlist of +text-only model ids. That is the same shape that failed three separate times in +the #955 line of work — a hand-maintained allowlist over an open string domain, +where every id the author did not think of silently takes the wrong branch. Here +the failure is asymmetric and user-visible: a text-only NIM model missing from +the list keeps exactly the bug #956 reports. NIM ships ~101 discoverable model +rows and adds more continuously, so the list is stale the day it merges. + +**#970** switches the post-update service refresh from `install` to `repair`. +`repairService()` and `ocx service repair` **already exist** in this tree +(`src/service.ts:1755`, `src/service.ts:2526`), so the real change is a handful of +call sites and a pile of advice strings — not the 522-line diff the PR carries. +More importantly `repairService()` throws when the service is not installed, and +the update path runs *after* `ocx stop`. Whether that substitution is safe on all +three platforms is a correctness question the PR does not answer, and it is +answered in `020` before any code is written. + +## Non-goals + +- #961 is an enhancement (provider custom headers via PATCH), already labeled + `enhancement` by the triage bot and confirmed unchanged. No code, no relabel. +- #966 stays open with its two surviving falsifications; the author may push + corrections. +- #907 stays blocked on `lidge-jun/jawcode`; nothing in this unit touches it. +- No push to `dev`, `preview`, or `main`. Layer 7 is a `codex/` branch like the + rest of the stack. + +## Documents + +| Doc | Contents | +|---|---| +| `010_nim_vision_classification.md` | #964 reconstruction — the classification design and its diff | +| `020_service_repair_path.md` | #970 reconstruction — call-site inventory and the after-stop safety proof | +| `030_merge_and_close_sequence.md` | bottom-up merge order, retargeting, and issue closure evidence | diff --git a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md new file mode 100644 index 000000000..45b605f91 --- /dev/null +++ b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md @@ -0,0 +1,174 @@ +# 010 — Classify NVIDIA NIM vision capability by its short side (#964 / issue #956) + +## The defect being fixed + +The registry `nvidia` entry declares no `noVisionModels` +(`src/providers/registry.ts:1303-1320`), so `planVisionSidecar()` returns +`undefined` for every NIM model (`src/vision/index.ts:235`). A text-only NIM +model therefore either receives raw image parts it cannot read, or — more often — +has attachments blocked client-side because the catalog never advertises image +input (`src/codex/catalog/provider-fetch.ts:177-184`). That is issue #956, and it +is real. + +Note the field's inverted name: `noVisionModels` lists models that CANNOT see +images, and listing one there is what *enables* image support for it, via the +sidecar. Getting a model's membership wrong in either direction is a bug. + +## Why #964's list cannot be carried + +#964 enumerates ~60 text-only NIM ids by hand. Live verification against +NVIDIA's own documentation on 2026-08-04 found **five of those entries are +natively image-capable**: + +| Id | #964 says | NVIDIA says | Source | +|---|---|---|---| +| `thinkingmachines/inkling` | text-only | text, **image**, audio (RGB, 40–4096px per side) | [NIM API reference](https://docs.api.nvidia.com/nim/reference/thinkingmachines-inkling) | +| `minimaxai/minimax-m3` | text-only | Text, **Image**, Video | build.nvidia.com model page | +| `moonshotai/kimi-k2.6` | text-only | text, **image**, video, with a published `image_url` example | build.nvidia.com model page | +| `stepfun-ai/step-3.7-flash` | text-only | text + image, documented as a VLM | [NIM VLM example](https://docs.nvidia.com/nim/vision-language-models/1.7.0/examples/step-3.7-flash/api.html) | +| `mistralai/mistral-medium-3.5-128b` | text-only | text + image, NIM-certified VLM | [VLM introduction](https://docs.nvidia.com/nim/vision-language-models/latest/introduction.html) | + +A false positive here is **silent**. The model can read the image itself, but the +proxy intercepts it and sends a different model's *text description* instead. +Nothing errors; cost and latency rise and answer quality drops, and no test +fails. That is strictly worse than the bug #956 reports, which at least +announces itself. + +The false entries are not the author's invention: **issue #956's own body lists +`minimaxai/minimax-m3` and `moonshotai/kimi-k2.6` as text-only.** Reporter and +PR author shared the same wrong premise and review passed it through. This is the +fourth time in this session's line of work that a hand-written allowlist over an +open string domain has been wrong; the pattern is the finding, not the individual +entries. + +One caveat carried forward: the Mistral Medium 3.5 evidence describes the +**self-hosted** VLM NIM container, and that documentation explicitly warns not to +assume a general text endpoint exposes vision. We attach to hosted +`integrate.api.nvidia.com`. Re-verify against the hosted model page before the id +lands. Either way it is not confidently text-only, so #964's classification of it +is unsupported. + +## Why not classify by name, tag, or API + +Three candidate mechanisms, all rejected on evidence: + +1. **A modality field from the API.** `GET /v1/models` on + `integrate.api.nvidia.com` returns no input-modality field, and the documented + VLM `/v1/models` schema carries identifiers, ownership, context length and + permissions — no modalities. The registry really is the only source of truth, + as #956 states. +2. **A naming convention.** `google/gemma-4-31b-it` carries no `vision`, `-vl`, + or `omni` marker and [accepts text + image, processing video as frame + sequences](https://docs.api.nvidia.com/nim/reference/google-gemma-4-31b-it). + Counterexamples run both ways: `-vl` also appears on embedding and reranking + models that are not chat generators at all. +3. **The catalog's own labels.** The `image-to-text` filter returns 7 entries + while per-page verification finds 15 image-capable chat models. The labels are + incomplete. + +## The design: keep the short list, derive the long one + +Maintain `NVIDIA_NIM_VISION_MODELS` — the models that CAN see — and compute +`noVisionModels` as the complement over the ids we actually classify. + +This inverts the failure mode, which is the whole point. NIM adds models +continuously, so any hand-maintained list is stale on merge day. The question is +what happens to an id nobody has classified yet: + +| Design | Unknown new model defaults to | Failure when wrong | +|---|---|---| +| #964: enumerate text-only | not in list → **no sidecar** | issue #956 persists — images blocked or 400 | +| ours: enumerate vision-capable | not in list → **sidecar on** | one extra description hop; image still works | + +The second failure is recoverable and visible in logs. The first is the bug we +are fixing. The maintained list also shrinks from ~60 entries to 15, and the +short list is the one with authoritative per-model documentation behind it. + +This mirrors `CLINE_PASS_TEXT_ONLY_MODELS` +(`src/providers/registry.ts:627`), which already derives text-only membership as +`CLINE_PASS_MODELS.filter(id => !CLINE_PASS_IMAGE_MODELS.has(id))`. The pattern +is established in this file; #964 simply did not follow it. + +### Scope boundary + +The complement is taken over **chat-capable** NIM ids only. Embeddings, +rerankers, guard/safety classifiers, OCR and document extraction, image and video +generation, speech, and simulation endpoints are not chat models and must not +appear in either list. + +## Verified vision-capable set (2026-08-04) + +High confidence, per-model NVIDIA documentation: + +``` +meta/llama-3.2-11b-vision-instruct +meta/llama-3.2-90b-vision-instruct +nvidia/llama-3.1-nemotron-nano-vl-8b-v1 +nvidia/nemotron-nano-12b-v2-vl +nvidia/nemotron-3-nano-omni-30b-a3b-reasoning +nvidia/cosmos3-nano-reasoner +nvidia/ising-calibration-1.5-31b +nvidia/ising-calibration-1-35b-a3b +google/gemma-4-31b-it +google/diffusiongemma-26b-a4b-it +minimaxai/minimax-m3 +moonshotai/kimi-k2.6 +stepfun-ai/step-3.7-flash +thinkingmachines/inkling +mistralai/mistral-medium-3.5-128b +``` + +Flagged, not yet committed to code: + +- `mistralai/mistral-medium-3.5-128b` — hosted-endpoint recheck pending (above); + NVIDIA also showed a 2026-08-07 deprecation date. +- `nvidia/llama-3.1-nemotron-nano-vl-8b-v1` — catalog indicated imminent + deprecation; harmless if it disappears (an absent id classifies nothing). +- `google/paligemma` — established VLM, but its detail page 404s. Excluded from + the vision list would make it text-only, which is wrong; excluded from + classification entirely is correct until availability is confirmed. +- `nvidia/vila`, `microsoft/phi-3_5-vision-instruct`, DePlot, Phi-4 Multimodal — + image-capable but hosted endpoint deprecated. `nvidia/neva-22b`, + `kosmos-2`, `fuyu-8b` — no current hosted page found. #964's test asserts these + historical ids stay out of `noVisionModels`; that assertion stays true under + our design for free, since they are simply not classified. + +## Planned diff + +1. `src/providers/registry.ts` + - add `NVIDIA_NIM_VISION_MODELS` (the verified set above) with a comment + recording the verification date, the per-model source, and the standing + instruction to append to THIS list, never to a text-only one; + - add `NVIDIA_NIM_CHAT_MODELS` — the chat ids we classify, seeded from the + live catalog snapshot and the existing `NVIDIA_NIM_KIMI_MODELS`; + - derive `NVIDIA_NIM_NO_VISION_MODELS` as the filtered complement; + - set `noVisionModels` on the `nvidia` entry and extend the entry comment. +2. No change to `src/vision/index.ts`, `src/codex/catalog/provider-fetch.ts`, or + `src/router.ts`. The sidecar, the catalog's image-modality advertisement, and + the registry→config merge all already do the right thing once the field is + populated — which is why #956 has a working config-only workaround. + +## Tests and the red-green plan + +Extend `tests/nvidia-nim-hardening.test.ts` (the file #964 also chose): + +1. **Vision-capable ids are absent from `noVisionModels`.** Seed with the five + ids #964 got wrong. Ablate by adding one to the vision list's complement and + watch it go red. This test is the direct regression guard for #964's defect. +2. **Representative text-only ids are present** — `deepseek-ai/deepseek-v4-flash`, + `z-ai/glm-5.2`, `nvidia/nemotron-3-ultra-550b-a55b`, `openai/gpt-oss-120b`. +3. **The lists cannot overlap.** A structural assertion that + `NVIDIA_NIM_VISION_MODELS ∩ noVisionModels = ∅`. Ablate by planting a + duplicate id. +4. **Sidecar activation end to end** — `planVisionSidecar` returns a plan for a + text-only NIM model carrying an image, `undefined` without an image, and + `undefined` for `meta/llama-3.2-11b-vision-instruct` even with an image. + (#964's equivalent test passes for the wrong reason on ids like kimi-k2.6; ours + asserts the corrected classification.) +5. **A bare persisted nvidia config inherits the field** from the registry via + `routeModel` — covers the #956 reporter's exact config shape. +6. **The catalog advertises image input** for a text-only NIM model and does not + fabricate it for a vision-capable one. + +Every guard gets driven red by ablation before it counts, per the unit's +verification discipline. diff --git a/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md b/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md new file mode 100644 index 000000000..3e010839f --- /dev/null +++ b/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md @@ -0,0 +1,133 @@ +# 020 — `ocx update` must repair the service, not re-register it (#970) + +## The defect + +`ocx update` stops the running proxy before replacing package files, then brings +the background service back. It brings it back by **re-registering** it: +`serviceReinstallArgs()` returns `["service", "install"]` +(`src/service.ts:183-185`), and the Windows scheduler installer always reaches +`schtasks /create` (`src/service.ts:1723-1729`). `/create` requires elevation. + +An ordinary `ocx update` from a normal terminal inherits the user's non-admin +token. So on Windows the update stops a working proxy and then cannot put it +back — the failure mode @stephen-drew reports in #970. The direct-start fallback +keeps *a* proxy alive, but the managed service the user installed is gone until +they re-run an elevated install by hand. + +Scheduler **repair** does not re-register: it stops, rewrites the wrapper assets, +and `/run`s the existing task (`src/service.ts:1775-1785`). No `/create`, no UAC. + +## Why this is a small change, not a 522-line one + +`repairService()` and the `ocx service repair` subcommand already exist here +(`src/service.ts:1755`, `src/service.ts:2526`). #970 carries 522 added lines +largely because it also rewrites advice strings, five docs locales, and GUI +surfaces. The behavioral core is the argv one helper returns plus the call sites +that consume it. + +### Call sites that re-register after an update + +| Path | Today | Runs non-elevated? | +|---|---|---| +| `bin/ocx.mjs:136-150`, invoked `:244-253` | inlines the install argv | yes | +| `src/update/index.ts:298-320` | `serviceReinstallArgs()` | yes | +| `src/update/job.ts:737-744`, invoked `:807-814` | `serviceReinstallArgs()` | yes | +| `src/server/startup-action-control.ts:109-117` | already picks `repair` for repair mode | n/a — correct | + +The dashboard Startup action is already right, and +`tests/startup-action-control-elevation.test.ts:152` already asserts repair is +never elevated. Only the three update paths are wrong. + +## The safety question the PR does not answer + +`repairService()` **throws** when `!diag.installed` (`src/service.ts:1758-1770`). +The update path runs it *after* `ocx stop`. If stopping deregistered the service, +substituting repair would throw exactly where install used to succeed. + +It does not. Stop never deletes registration on any platform: + +| Platform | Stop does | Registration lives in | `installed` derives from | +|---|---|---|---| +| macOS | `launchctl unload` (`src/service.ts:1667`) | the plist, deleted only by `uninstallLaunchd` (`:1669-1673`) | `existsSync(plistPath())` (`:2372-2381`) | +| Windows scheduler | `schtasks /end` (`:1885-1891`) | the task, deleted only by `/delete` (`:1894-1908`) | non-empty `/query /xml` (`:2383-2400`) | +| Windows WinSW | `stopwait` (`src/lib/winsw.ts:330-332`) | the service; only `nonexistent` means absent | `:2323-2334` | +| Linux | `systemctl --user stop` (`:2067`) | the unit file, deleted only by `uninstallSystemd` (`:2069-2072`) | `existsSync(unitPath())` (`:2402-2414`) | + +`stopServiceIfInstalled()` (`:2204-2225`) calls exactly those three and no +uninstall. **Verdict: the substitution is safe on all three platforms** when the +path began with a genuinely installed service. + +Repair also re-bakes the port the same way install does — `OCX_BAKE_PORT` feeds +`resolveServiceListenPort()` (`:318-335`), which every backend's asset writer +consumes (`buildPlist` `:1607`, `buildUnit` `:2010`, `buildWindowsServiceScript` +`:1693`, WinSW XML `src/lib/winsw.ts:79-92`). So the update path's existing +`OCX_BAKE_PORT` handling keeps working unchanged. + +On macOS and Linux repair delegates straight to `installLaunchd`/`installSystemd` +(`:1787-1793`), so the service-manager outcome is identical. The change is +Windows-meaningful and non-Windows-neutral. + +## The caveat #970 introduces and does not handle + +`bin/ocx.mjs:136-150` decides "a service manages this proxy" from the mere +existence of `service-state.json`. That marker can be **stale** — present while +the actual registration is gone. Today `install` recreates the service from a +stale marker. Under a blind `repair` substitution it throws "not installed", and +the user silently loses their managed service; only the direct-start fallback +(`bin/ocx.mjs:253-297`) keeps a proxy alive. + +`src/update/index.ts` does not have this hole: it records `isServiceInstalled()` +before stopping (`:188-194`). + +**Our reconstruction closes it:** the update paths try repair first and fall back +to install when repair reports the service is not actually installed. That keeps +the non-elevated happy path (the whole point of #970) while preserving today's +recovery from a stale marker. A straight cherry-pick of #970 would import the +regression. + +## Planned diff + +1. `src/service.ts` — `serviceReinstallArgs()` returns `["service", "repair"]`; + keep the export name for out-of-module callers. `serviceRepairCommand()` + (`:500-511`) returns the backend-neutral `ocx service repair` instead of + synthesizing `install --native`. +2. `bin/ocx.mjs` — refresh via repair; on a "not installed" failure retry once + with the backend-correct install argv before the direct-start fallback. +3. `src/update/index.ts`, `src/update/job.ts` — consume the repair argv; the + existing non-viable/failed fallbacks stay exactly as they are. +4. Advice strings that fire only for an **installed** service become repair: + `src/cli/status.ts:171-177`, `src/service.ts:1927`, `:2341-2349`, `:2464-2474`, + `src/lib/winsw.ts:370-372`. First-install and missing-unit guidance + (`src/service.ts:1768-1770`, `:2045-2050`) stays install. +5. `src/cli/doctor.ts:687-696` currently sees only `serviceViable`, which + conflates "absent" with "installed but stale". It needs the installed/stale + inputs before it can choose correctly — otherwise it would advise repair to a + user who has no service at all. +6. Docs: the English lifecycle page omits `repair` from the subcommand table and + its status example literally reads `Repair: ocx service install` + (`docs-site/.../reference/cli/lifecycle.md:176-192`, `:229-236`). Same gap in + ko/ja/zh-cn/ru. + +## Tests that must move, and the red-green plan + +These currently pin `service install` in an update/recovery path and will fail +until updated — which is the proof the change is load-bearing: + +- `tests/update-job.test.ts:115-119`, `:126-134` +- `tests/winsw.test.ts:252-256` +- `tests/service.test.ts:1243-1251` +- `tests/doctor.test.ts:473-479` +- `tests/update-stop-first.test.ts:58-72` (wording plus a stronger argv assertion) + +New coverage: + +- the update refresh argv is `service repair` — ablate by restoring the install + argv and watch it go red; +- a stale `service-state.json` with no real registration still ends with a + managed service (repair throws, install retry succeeds) — ablate by removing + the fallback and watch it go red. + +Already-passing coverage that constrains us and must stay green: +`tests/service.test.ts:897-912` (scheduler repair does no `/create`), `:914-920` +(repair rejects a genuinely absent service), `tests/winsw.test.ts:213-225`, +`tests/update-job.test.ts:304-350` (`OCX_BAKE_PORT` set and restored). diff --git a/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md b/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md new file mode 100644 index 000000000..e0600a410 --- /dev/null +++ b/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md @@ -0,0 +1,91 @@ +# 030 — Merge the stack bottom-up, then close what it resolved + +## Renumbering + +Layer 1 (#951) merged as `af3ddedb4`, and this unit adds a seventh layer. Titles +currently read `stack N/6` and become `stack N/7`. The merged layer keeps its +historical title — retitling a merged PR rewrites a record nobody can act on. + +| Layer | PR | Branch | Base while stacked | +|---|---|---|---| +| 1/7 | #951 | `codex/bug-stack-plan` | merged `af3ddedb4` | +| 2/7 | #952 | `codex/908-long-context-pricing` | `dev` after #951 landed | +| 3/7 | #953 | `codex/carry-contributor-bugfixes` | #952 | +| 4/7 | #954 | `codex/545-classifier-thinking-disabled` | #953 | +| 5/7 | #955 | `codex/915-cooldown-recovery-probe` | #954 | +| 6/7 | #973 | `codex/stack6-overnight-triage` | #955 | +| 7/7 | new | `codex/stack7-service-vision` | #973 | + +Navigation comments on every open layer get refreshed to the seven-row chain with +layer 1 marked merged. + +## Merge order and the gate at each step + +Strictly bottom-up. A layer merges only after its parent has landed, because +every layer's diff is expressed against its parent's tree. + +For each layer, in order: + +1. Confirm the child's base branch is the just-merged parent, then retarget it to + `dev` (`gh pr edit --base dev`). GitHub does this automatically when the + parent merges, but it is verified rather than assumed. +2. Re-read CI on the **exact head sha** — `gh pr checks ` plus a sha match. + A remembered green is not evidence, and retargeting changes the merge base. +3. Merge. +4. Record the merge commit for the issue-closure step below. + +### The #954 gate + +Layer 4 (#545, keeping an explicit thinking disable through translation) touches +request translation for a security-relevant control. `MAINTAINERS.md` reserves +that class for human security review. If that review has not happened when the +queue reaches #954, the honest outcome is: merge #952 and #953, stop, state the +gate, and leave #954–#973 stacked. Layers above it cannot be merged out of order +to route around the gate — their diffs assume #954's tree. + +That is a `NEEDS_HUMAN` terminal outcome for the merge work-phase, not a failure, +and not a reason to shrink the goal. + +## Issue closure + +An issue closes when the layer that fixes it is **merged into `dev`**, with a +comment naming the merge commit. Closing on "the PR exists" is what makes issue +trackers untrustworthy. + +| Issue | Fixed by | Layer | +|---|---|---| +| #908 | long-context pricing tiers | #952 | +| #545 | explicit thinking disable through translation | #954 | +| #915 | cooldown early-recovery probe | #955 | +| #962 | custom rows inherit provider metadata (carried #965) | #973 | +| #956 | NIM vision classification | stack 7 | +| issues fixed by the six carried contributor fixes | — | #953 | + +The #953 row is deliberately unresolved here: the six carried fixes +(#939, #942, #943, #944, #945, #948) must be re-read at closure time to map each +to the issue it actually resolves. Several were PR-only with no filed issue. + +### Contributor PRs to close as superseded + +#964 and #970 close when stack 7 opens, not when it merges — the same policy +already applied to the six carried PRs earlier in this session, at the user's +explicit instruction. Each closing comment must name the superseding commits, +state plainly what changed relative to the contributor's version, and say that +reopening is one click. + +For #964 the comment owes the author a specific correction: five ids in the +submitted list are natively image-capable, and the reasoning is in `010`. The +contributor found a real bug and the list shape is what failed, not the finding. + +## Left open deliberately + +#961 (enhancement, provider headers), #966 (two falsifications survive), #969 +(CI governance policy), #922, #928, #935, #936, #940, #557 — each already carries +a stated reason on the PR. Nothing in this unit changes them. + +#907 stays blocked on `lidge-jun/jawcode`: the canonical `models.json` lives in +another repository across four provider bundles, and the local fix cannot land +without it. The coupling trap is recorded on the issue — correcting the Terra and +Luna base prices without recomputing `PRIORITY_MULTIPLIERS` +(`src/usage/expected-prices.ts:152-156`) trades an overcharge for an undercharge +and no test fails. From ef2e5a439666190965b31c31be9486747b695918 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 11:18:51 +0900 Subject: [PATCH 2/9] docs(devlog): fold five audit blockers into the stack 7 plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The A-gate reviewer returned FAIL. Every blocker was reproduced before being accepted; none was rebutted. 001 records the synthesis. B1 killed my own design. I proposed maintaining the 15 vision-capable ids and deriving text-only as their complement, and claimed an unclassified model would default to sidecar-on. It does not — a complement over a static chat-model list leaves an unknown id in neither list, so modelInList returns false and #956 survives verbatim: deepseek-ai/deepseek-v4-flash sidecarWouldRun=true moonshotai/kimi-k2.6 sidecarWouldRun=false brandnew/model-nobody-classified sidecarWouldRun=false I had inverted which list is maintained while keeping the closed world — the same lesson as the three earlier allowlist failures, reproduced while writing the document that cites them. 010 now changes the predicate instead: default-on for the nvidia entry with the vision list as its exception set, so a stale exception list costs one description hop rather than reproducing the bug. B2: removing a native-vision id from noVisionModels is not sufficient. The catalog advertises image input only for list members, so those models would be blocked client-side instead. They need explicit modelInputModalities. B3: src/update/job.ts:775 skips the service refresh entirely on non-elevated Windows — the dashboard path. Its stated reason is that schtasks /create needs UAC, which repair does not call, so the skip must be narrowed or the reporter's own surface stays broken. B4: repairService throws plain Errors and bin/ocx.mjs sees only an exit status, so 'fall back on not-installed' was unimplementable. Re-run diagnoseService() after a failed repair instead of parsing messages. B5: retargeting emits 'edited', which ci.yml does not listen for, so a green check on the same head sha proves nothing about the new merge base. 030 also moves the #964/#970 closure from 'when stack 7 opens' to 'open and green'. The earlier text borrowed a policy from the six carried PRs, which had verified replacement commits already on a branch; this replacement does not exist yet and its first design just failed audit. --- .../260804_stack7_service_vision/000_scope.md | 1 + .../001_audit_response.md | 114 +++++++++++++ .../010_nim_vision_classification.md | 152 ++++++++++++------ .../020_service_repair_path.md | 68 ++++++-- .../030_merge_and_close_sequence.md | 43 +++-- 5 files changed, 312 insertions(+), 66 deletions(-) create mode 100644 devlog/_plan/260804_stack7_service_vision/001_audit_response.md diff --git a/devlog/_plan/260804_stack7_service_vision/000_scope.md b/devlog/_plan/260804_stack7_service_vision/000_scope.md index 74ae3017b..0fe057d6c 100644 --- a/devlog/_plan/260804_stack7_service_vision/000_scope.md +++ b/devlog/_plan/260804_stack7_service_vision/000_scope.md @@ -71,6 +71,7 @@ answered in `020` before any code is written. | Doc | Contents | |---|---| +| `001_audit_response.md` | A-gate FAIL — five blockers, synthesis, and what changed | | `010_nim_vision_classification.md` | #964 reconstruction — the classification design and its diff | | `020_service_repair_path.md` | #970 reconstruction — call-site inventory and the after-stop safety proof | | `030_merge_and_close_sequence.md` | bottom-up merge order, retargeting, and issue closure evidence | diff --git a/devlog/_plan/260804_stack7_service_vision/001_audit_response.md b/devlog/_plan/260804_stack7_service_vision/001_audit_response.md new file mode 100644 index 000000000..4bc45e442 --- /dev/null +++ b/devlog/_plan/260804_stack7_service_vision/001_audit_response.md @@ -0,0 +1,114 @@ +# 001 — Audit response: five blockers, all accepted + +The A-gate reviewer returned **FAIL** on the first roadmap. Every blocker was +independently reproduced before being accepted; none was rebutted. This document +records the synthesis, per REVIEW-SYNTHESIS-01, before the plan was re-patched. + +## B1 [P0] — the inverted list does not default to sidecar-on + +**Claim:** `010`'s central justification is false. A complement taken over a +static `NVIDIA_NIM_CHAT_MODELS` leaves an unclassified id in *neither* list, so +`modelInList` returns false (`src/types.ts:204`), `planVisionSidecar` returns +`undefined` (`src/vision/index.ts:235`), and the catalog advertises no image +modality (`src/codex/catalog/provider-fetch.ts:176`). + +**Reproduced.** `.tmp/probe_complement.ts`, modelling the exact proposed shape: + +```console +deepseek-ai/deepseek-v4-flash sidecarWouldRun=true +moonshotai/kimi-k2.6 sidecarWouldRun=false +brandnew/model-nobody-classified sidecarWouldRun=false <-- #956 persists +``` + +**Accepted.** I inverted which list is maintained but kept the closed world. The +failure I claimed to have fixed survives verbatim for any id NVIDIA adds after +the snapshot. This is the same lesson as the three earlier allowlist failures in +this session, and I reproduced it while writing the document that cites them. + +**Root cause:** the classification field is membership-in-a-list, so any design +expressed purely as list contents inherits closed-world semantics. Escaping it +requires changing the *predicate*, not the lists. + +## B2 [P0] — verified vision models stay unusable from the Codex app + +**Claim:** removing a native-vision id from `noVisionModels` is not enough. +`applyProviderConfigHints` adds `image` to `inputModalities` only for +`noVisionModels` members (`provider-fetch.ts:176`), and NIM `/v1/models` carries +no modality metadata. So kimi-k2.6 et al. end up advertised text-only and the +Codex app blocks attachments before their native path can run. + +**Accepted.** `010` explicitly asserted the catalog "does not fabricate" image +capability for these ids and treated that as correct. It is a second bug, not a +neutral outcome: #964 makes them lossy, my first design makes them blocked. + +**Fix:** verified native-vision ids need explicit +`modelInputModalities[id] = ["text","image"]`, asserted against the emitted +catalog payload rather than against `undefined`. + +## B3 [P0] — the Windows GUI updater never reaches the refresh command + +**Claim:** `src/update/job.ts:775-790` sets `skipServiceInstall = true` +unconditionally when `process.platform === "win32" && OCX_SERVICE === "1"`. +Changing the argv cannot affect a command that is never spawned. + +**Verified in source.** The skip's own comment states the reason: "`schtasks +/create` will UAC-fail and can race the subsequent direct start." + +**Accepted, and it strengthens the change.** That skip is a workaround for +exactly the defect #970 reports. `repair` does not call `/create` +(`src/service.ts:1775-1785`), so the justification for skipping evaporates — +the skip must be narrowed to the install argv rather than left in place. Without +this, the dashboard-triggered Windows update, the most common GUI path, keeps +the bug while the CLI path gets fixed. + +## B4 [P1] — the stale-marker fallback has no discriminator + +**Claim:** `repairService()` throws plain `Error` for unsupported, conflict, +ownership, auth, absent-registration, asset-write, start, and health failures +alike (`src/service.ts:1755-1770`). `bin/ocx.mjs` spawns with inherited stdio and +sees only an exit status (`bin/ocx.mjs:251`), so "not installed" is +indistinguishable from any other failure. + +**Accepted.** My proposed "repair, fall back to install on not-installed" was +unimplementable as written. Broadening it to "install after any repair failure" +would reintroduce the UAC path and could re-register a service the user had +deliberately uninstalled concurrently. + +**Fix:** do not infer from the failure at all. Re-run a structured diagnostic +(`diagnoseService()`) after a failed repair and install only when it reports the +service genuinely absent while the managed-service marker still expresses intent. +State beats error-message parsing. + +## B5 [P1] — retargeting produces no fresh CI evidence + +**Claim:** `.github/workflows/ci.yml` uses default `pull_request` activity types +(`opened`/`synchronize`/`reopened`). A base edit emits `edited`, which is not +among them. So after retargeting a stacked child to `dev`, `gh pr checks` can +show green checks bound to the same head sha that were never run against the new +merge base. Material because `dev` is well ahead of the stacked heads. + +**Accepted.** `030` said "re-read CI on the exact head sha", which I framed as +the rigorous option. It is necessary but not sufficient: sha identity does not +imply base identity. + +**Fix:** after retargeting, merge current `dev` into the child to force a +`synchronize` event, and require a run whose base matches the retargeted PR +before merging. + +## Sequencing change this forces + +`030` closed #964 and #970 when stack 7 *opens*, mirroring the earlier carried +PRs. Those were closed with replacement code already on a branch. Here the +replacement does not exist yet and its design just failed audit, so closing now +would remove the contributor's live path while ours is unproven. + +**Changed:** both close only after stack 7 is open **and** green. This +contradicts the sequencing written in the first draft of `030`; the earlier text +was wrong and is corrected there. + +## What survived + +`020`'s after-stop safety proof — that `ocx stop` never deregisters on any of the +three platforms — was independently re-derived and holds +(`src/service.ts:2204-2225`, with `uninstall`/`remove` as separate paths at +`:2610`). It remains the foundation of the #970 reconstruction. diff --git a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md index 45b605f91..cf5b8941c 100644 --- a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md +++ b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md @@ -66,35 +66,82 @@ Three candidate mechanisms, all rejected on evidence: while per-page verification finds 15 image-capable chat models. The labels are incomplete. -## The design: keep the short list, derive the long one +## The design: change the predicate, not the list + +**A first version of this document proposed maintaining the 15 vision-capable ids +and deriving text-only as their complement. The A-gate audit falsified it and I +reproduced the failure** (`001_audit_response.md`, B1). A complement taken over a +static `NVIDIA_NIM_CHAT_MODELS` still leaves an unclassified id in *neither* +list: + +```console +$ bun run .tmp/probe_complement.ts # the proposed shape, modelled exactly +deepseek-ai/deepseek-v4-flash sidecarWouldRun=true +moonshotai/kimi-k2.6 sidecarWouldRun=false +brandnew/model-nobody-classified sidecarWouldRun=false <-- #956 persists +``` + +I had inverted which list is maintained while keeping the closed world. Any +design expressed purely as *list contents* inherits closed-world semantics, +because the classification is membership-in-a-list. Escaping it requires changing +the **predicate**. + +### Two fields, one default + +1. **`NVIDIA_NIM_VISION_MODELS`** — the 15 verified natively-image-capable ids. + These are the exception, and they carry per-model NVIDIA documentation. +2. **A provider-level default** — for the `nvidia` entry, a model that is *not* + in the vision list is treated as needing the sidecar, without enumerating it. -Maintain `NVIDIA_NIM_VISION_MODELS` — the models that CAN see — and compute -`noVisionModels` as the complement over the ids we actually classify. +Concretely this means the `nvidia` entry declares its text-only membership as +"everything except the vision list" rather than as a snapshot of ids. The +registry already carries per-provider capability flags +(`ProviderRegistryEntry`, `src/providers/registry.ts:190-230`); this adds one +more whose semantics are *default-on with an exception list*, and `router.ts` +merges it beside `noVisionModels` (`src/router.ts:243`) so a user's explicit +config still wins. -This inverts the failure mode, which is the whole point. NIM adds models -continuously, so any hand-maintained list is stale on merge day. The question is -what happens to an id nobody has classified yet: +Now the open-domain case lands correctly: -| Design | Unknown new model defaults to | Failure when wrong | +| Design | Unclassified new NIM model | Failure when wrong | |---|---|---| -| #964: enumerate text-only | not in list → **no sidecar** | issue #956 persists — images blocked or 400 | -| ours: enumerate vision-capable | not in list → **sidecar on** | one extra description hop; image still works | +| #964: enumerate text-only | no sidecar | **#956 persists** — images blocked or 400 | +| complement over a static set | no sidecar | **#956 persists** (falsified above) | +| default-on with an exception list | sidecar runs | one extra description hop; image still works | -The second failure is recoverable and visible in logs. The first is the bug we -are fixing. The maintained list also shrinks from ~60 entries to 15, and the -short list is the one with authoritative per-model documentation behind it. +Only the third actually closes the issue for models NVIDIA has not shipped yet. -This mirrors `CLINE_PASS_TEXT_ONLY_MODELS` -(`src/providers/registry.ts:627`), which already derives text-only membership as -`CLINE_PASS_MODELS.filter(id => !CLINE_PASS_IMAGE_MODELS.has(id))`. The pattern -is established in this file; #964 simply did not follow it. +### Why the exception list is safe to hand-maintain + +The objection that killed the previous design does not apply here. A stale +*exception* list degrades gracefully: a newly-released vision model missing from +it gets an unnecessary description hop, which is slower and costs a call but +still answers. A stale *enumeration* silently reproduces the reported bug. The +asymmetry is the entire argument, and it only holds when the default is on. ### Scope boundary -The complement is taken over **chat-capable** NIM ids only. Embeddings, -rerankers, guard/safety classifiers, OCR and document extraction, image and video -generation, speech, and simulation endpoints are not chat models and must not -appear in either list. +Default-on applies to **chat** ids only. Embeddings, rerankers, guard/safety +classifiers, OCR and document extraction, image/video generation, speech, and +simulation endpoints never reach `planVisionSidecar` in the first place — they +are not routed as chat models — but the implementation must confirm that rather +than assume it, since a default-on rule has a wider blast radius than a list. + +### Verified vision models also need explicit modalities (audit B2) + +Keeping a native-vision id out of `noVisionModels` is necessary but **not +sufficient**. `applyProviderConfigHints` adds `image` to a model's +`inputModalities` only for `noVisionModels` members +(`src/codex/catalog/provider-fetch.ts:176-184`), and NIM `/v1/models` publishes +no modality metadata. So a model left out of both would be advertised text-only +and the Codex app would block attachments client-side — the model can see, and +the user still cannot send. + +Every id in `NVIDIA_NIM_VISION_MODELS` therefore also gets +`modelInputModalities[id] = ["text", "image"]`, following the shape +`ZHIPU_BIGMODEL_INPUT_MODALITIES` already uses +(`src/providers/registry.ts:327-331`). The test asserts the **emitted catalog +payload** carries `image`, not merely that the id is absent from a list. ## Verified vision-capable set (2026-08-04) @@ -139,36 +186,51 @@ Flagged, not yet committed to code: - add `NVIDIA_NIM_VISION_MODELS` (the verified set above) with a comment recording the verification date, the per-model source, and the standing instruction to append to THIS list, never to a text-only one; - - add `NVIDIA_NIM_CHAT_MODELS` — the chat ids we classify, seeded from the - live catalog snapshot and the existing `NVIDIA_NIM_KIMI_MODELS`; - - derive `NVIDIA_NIM_NO_VISION_MODELS` as the filtered complement; - - set `noVisionModels` on the `nvidia` entry and extend the entry comment. -2. No change to `src/vision/index.ts`, `src/codex/catalog/provider-fetch.ts`, or - `src/router.ts`. The sidecar, the catalog's image-modality advertisement, and - the registry→config merge all already do the right thing once the field is - populated — which is why #956 has a working config-only workaround. + - add `modelInputModalities` entries pinning `["text","image"]` for each of + those ids (audit B2); + - declare the provider-level default-on rule on the `nvidia` entry, with the + vision list as its exception set, and extend the entry comment to explain + why NIM specifically gets a default rather than an enumeration. +2. The predicate change touches the classification path, so the surfaces that + read `noVisionModels` must each be checked rather than assumed: + `planVisionSidecar` (`src/vision/index.ts:235`), the fail-closed strip in + `src/server/responses/core.ts:1581`, the catalog hint + (`src/codex/catalog/provider-fetch.ts:176`), the registry→config merge + (`src/router.ts:243`), the seed fill (`src/providers/derive.ts:257`), and + `src/cli/models.ts:44`. A user's explicit config `noVisionModels` must keep + winning over the default. +3. No behavioral change is intended for any other provider. The default is + scoped to the `nvidia` entry; every other entry keeps enumerating. ## Tests and the red-green plan Extend `tests/nvidia-nim-hardening.test.ts` (the file #964 also chose): -1. **Vision-capable ids are absent from `noVisionModels`.** Seed with the five - ids #964 got wrong. Ablate by adding one to the vision list's complement and - watch it go red. This test is the direct regression guard for #964's defect. -2. **Representative text-only ids are present** — `deepseek-ai/deepseek-v4-flash`, - `z-ai/glm-5.2`, `nvidia/nemotron-3-ultra-550b-a55b`, `openai/gpt-oss-120b`. -3. **The lists cannot overlap.** A structural assertion that - `NVIDIA_NIM_VISION_MODELS ∩ noVisionModels = ∅`. Ablate by planting a - duplicate id. -4. **Sidecar activation end to end** — `planVisionSidecar` returns a plan for a - text-only NIM model carrying an image, `undefined` without an image, and - `undefined` for `meta/llama-3.2-11b-vision-instruct` even with an image. - (#964's equivalent test passes for the wrong reason on ids like kimi-k2.6; ours - asserts the corrected classification.) -5. **A bare persisted nvidia config inherits the field** from the registry via - `routeModel` — covers the #956 reporter's exact config shape. -6. **The catalog advertises image input** for a text-only NIM model and does not - fabricate it for a vision-capable one. +1. **An unclassified id gets the sidecar.** The regression guard for audit B1: + a NIM model id that appears in no list at all must still produce a vision + plan when the request carries an image. Ablate by reverting to an enumerated + `noVisionModels` and watch it go red. This is the test the first design could + not have passed. +2. **Vision-capable ids do NOT get the sidecar.** Seed with the five ids #964 got + wrong — `thinkingmachines/inkling`, `minimaxai/minimax-m3`, + `moonshotai/kimi-k2.6`, `stepfun-ai/step-3.7-flash`, + `mistralai/mistral-medium-3.5-128b` — plus the two llama-3.2 vision ids. + `planVisionSidecar` returns `undefined` for each even with an image attached. +3. **The catalog advertises image input for both classes, for different + reasons** (audit B2): a text-only id gets `image` via the `noVisionModels` + hint, and a verified vision id gets it via `modelInputModalities`. Assert the + emitted payload in both cases. Ablate the `modelInputModalities` entries and + watch the vision-id case go red. +4. **Representative text-only ids still classify correctly** — + `deepseek-ai/deepseek-v4-flash`, `z-ai/glm-5.2`, + `nvidia/nemotron-3-ultra-550b-a55b`, `openai/gpt-oss-120b`. Sidecar on, no + image forwarded raw. +5. **A user's explicit config wins.** A persisted `nvidia` provider that names + its own `noVisionModels` is not overridden by the provider default. +6. **A bare persisted nvidia config gets the fix** through `routeModel` — the + #956 reporter's exact config shape, which today needs a manual workaround. +7. **No other provider changes classification.** A structural assertion over + `PROVIDER_REGISTRY` that the default-on rule is scoped to `nvidia` only. Every guard gets driven red by ablation before it counts, per the unit's verification discipline. diff --git a/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md b/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md index 3e010839f..618e377e0 100644 --- a/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md +++ b/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md @@ -67,6 +67,33 @@ On macOS and Linux repair delegates straight to `installLaunchd`/`installSystemd (`:1787-1793`), so the service-manager outcome is identical. The change is Windows-meaningful and non-Windows-neutral. +## The Windows GUI updater never reaches the command at all (audit B3) + +Changing the argv is not enough on the surface that matters most. +`restartAfterUpdate()` sets `skipServiceInstall = true` unconditionally when the +worker is a non-elevated Windows process (`src/update/job.ts:775-790`): + +```ts +// Windows GUI update worker sets OCX_SERVICE=1 and is never elevated. +// `schtasks /create` will UAC-fail and can race the subsequent direct start. +if (process.platform === "win32" && process.env.OCX_SERVICE === "1") { + updateJob(job, {}, "Skipping service reinstall from the non-elevated update worker; ..."); + skipServiceInstall = true; +``` + +The dashboard-triggered update is the common GUI path, and it skips the refresh +entirely — so a pure argv change would fix the CLI while leaving the GUI exactly +as broken. + +The skip's own comment names its justification: `schtasks /create` needs UAC. +**Repair does not call `/create`** (`src/service.ts:1775-1785`), so the +justification does not survive this change. The skip must be narrowed to the +install argv rather than left standing: a non-elevated Windows worker should run +`service repair` and fall through to the direct start only if that fails. + +This makes the reconstruction *larger* than a find-and-replace, and it is the +part of #970 that actually delivers the fix to the user who reported it. + ## The caveat #970 introduces and does not handle `bin/ocx.mjs:136-150` decides "a service manages this proxy" from the mere @@ -79,11 +106,20 @@ the user silently loses their managed service; only the direct-start fallback `src/update/index.ts` does not have this hole: it records `isServiceInstalled()` before stopping (`:188-194`). -**Our reconstruction closes it:** the update paths try repair first and fall back -to install when repair reports the service is not actually installed. That keeps -the non-elevated happy path (the whole point of #970) while preserving today's -recovery from a stale marker. A straight cherry-pick of #970 would import the -regression. +**Our reconstruction closes it — but not by reading the failure** (audit B4). +`repairService()` throws a plain `Error` for unsupported, conflict, ownership, +auth, absent-registration, asset-write, start, and health failures alike +(`src/service.ts:1755-1770`), and `bin/ocx.mjs` spawns with inherited stdio and +sees only an exit status (`bin/ocx.mjs:251`). "Not installed" is not +distinguishable from any other failure, so message-matching is unimplementable +and a blanket "install after any repair failure" would reintroduce the UAC path +and could re-register a service the user had just deliberately uninstalled. + +Instead: after a failed repair, re-run a **structured diagnostic** +(`diagnoseService()`) and install only when it reports the service genuinely +absent while the managed-service marker still expresses intent. State beats +error-message parsing. A straight cherry-pick of #970 would import the +regression; a naive fix for it would import a worse one. ## Planned diff @@ -91,10 +127,13 @@ regression. keep the export name for out-of-module callers. `serviceRepairCommand()` (`:500-511`) returns the backend-neutral `ocx service repair` instead of synthesizing `install --native`. -2. `bin/ocx.mjs` — refresh via repair; on a "not installed" failure retry once - with the backend-correct install argv before the direct-start fallback. -3. `src/update/index.ts`, `src/update/job.ts` — consume the repair argv; the - existing non-viable/failed fallbacks stay exactly as they are. +2. `bin/ocx.mjs` — refresh via repair; on failure consult `diagnoseService()` and + install only for a genuinely absent service, before the direct-start fallback. +3. `src/update/index.ts` — consume the repair argv; existing non-viable/failed + fallbacks unchanged. +4. `src/update/job.ts` — consume the repair argv **and** narrow the + Windows/`OCX_SERVICE=1` skip (`:775-790`) so it no longer suppresses a + non-registering repair. 4. Advice strings that fire only for an **installed** service become repair: `src/cli/status.ts:171-177`, `src/service.ts:1927`, `:2341-2349`, `:2464-2474`, `src/lib/winsw.ts:370-372`. First-install and missing-unit guidance @@ -118,14 +157,21 @@ until updated — which is the proof the change is load-bearing: - `tests/service.test.ts:1243-1251` - `tests/doctor.test.ts:473-479` - `tests/update-stop-first.test.ts:58-72` (wording plus a stronger argv assertion) +- `tests/windows-deploy-close-regressions.test.ts:45` — static guard describing + the install-only Windows behavior (audit B3) New coverage: - the update refresh argv is `service repair` — ablate by restoring the install argv and watch it go red; +- **a non-elevated Windows worker (`OCX_SERVICE=1`) now receives + `service repair`** rather than skipping the refresh — the audit-B3 guard; + ablate by restoring the unconditional skip and watch it go red; - a stale `service-state.json` with no real registration still ends with a - managed service (repair throws, install retry succeeds) — ablate by removing - the fallback and watch it go red. + managed service (repair fails, diagnostic reports absent, install runs) — + ablate by removing the diagnostic recheck and watch it go red; +- a repair that fails for a reason **other** than absence does NOT trigger an + install — the audit-B4 guard against reintroducing UAC. Already-passing coverage that constrains us and must stay green: `tests/service.test.ts:897-912` (scheduler repair does no `/create`), `:914-920` diff --git a/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md b/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md index e0600a410..9a31a6c98 100644 --- a/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md +++ b/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md @@ -29,10 +29,18 @@ For each layer, in order: 1. Confirm the child's base branch is the just-merged parent, then retarget it to `dev` (`gh pr edit --base dev`). GitHub does this automatically when the parent merges, but it is verified rather than assumed. -2. Re-read CI on the **exact head sha** — `gh pr checks ` plus a sha match. - A remembered green is not evidence, and retargeting changes the merge base. -3. Merge. -4. Record the merge commit for the issue-closure step below. +2. **Force a fresh CI run against the new base** (audit B5). Retargeting alone + does not produce one: `.github/workflows/ci.yml` uses the default + `pull_request` activity types (`opened`/`synchronize`/`reopened`), and a base + edit emits `edited`. So `gh pr checks` can report green checks bound to the + same head sha that never ran against the new merge base — materially wrong + here, because `dev` is well ahead of the stacked heads. Merge current `dev` + into the child to trigger `synchronize`. +3. Verify the run's **base** matches the retargeted PR, not merely that the head + sha matches. Sha identity does not imply base identity, and a remembered green + is not evidence at all. +4. Merge. +5. Record the merge commit for the issue-closure step below. ### The #954 gate @@ -67,15 +75,30 @@ to the issue it actually resolves. Several were PR-only with no filed issue. ### Contributor PRs to close as superseded -#964 and #970 close when stack 7 opens, not when it merges — the same policy -already applied to the six carried PRs earlier in this session, at the user's -explicit instruction. Each closing comment must name the superseding commits, -state plainly what changed relative to the contributor's version, and say that -reopening is one click. +#964 and #970 close once stack 7 is open **and green** — not the moment it opens. + +The first draft of this document said "when stack 7 opens", mirroring the six +carried PRs closed earlier in this session. That parallel does not hold and the +earlier text was wrong. Those six were closed with equivalent replacement commits +already on a branch and patch-id verified; here the replacement does not exist +yet, and its first design failed the audit gate outright +(`001_audit_response.md`). Closing a contributor's live path while ours is +unproven trades a working proposal for a plan. + +Each closing comment must name the superseding commits, state plainly what +changed relative to the contributor's version, and say that reopening is one +click. For #964 the comment owes the author a specific correction: five ids in the submitted list are natively image-capable, and the reasoning is in `010`. The -contributor found a real bug and the list shape is what failed, not the finding. +contributor found a real bug and the list shape is what failed, not the finding — +and my own first replacement for that shape failed too, which belongs in the +comment as well. + +For #970 the comment should credit the part the PR got right (the `/create` +elevation diagnosis) and name the two things the reconstruction adds: the +narrowed Windows GUI skip, without which the reporter's own surface stays broken, +and the diagnostic-based install fallback. ## Left open deliberately From ab4478d155516034b703cfcf1196e32e886fbc74 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 11:27:50 +0900 Subject: [PATCH 3/9] docs(devlog): root-cause the NIM classification failures, land a smaller honest design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit round 2 closed B2/B3/B5 and returned FAIL on two P0s. Two consecutive failures on the same surface means root cause, not a third patch of the same shape. R2-B2 is the one that matters: 010 flagged 'non-chat endpoints never reach the predicate' as a thing to confirm rather than assume, and I did not confirm it. It is false. NVIDIA has no discovery filter and shouldExposeRoutedModel rejects only media-generation names, so embeddings, rerankers, guards and OCR all reach planVisionSidecar: nvidia/nv-embedqa-e5-v5 filteredOut=false nvidia/llama-3.1-nemotron-safety-guard-8b-v3 filteredOut=false nvidia/nemotron-ocr-v2 filteredOut=false Under default-on every one of them would advertise image input and burn a sidecar call before failing upstream. Root cause: twelve of the thirteen registry entries declaring noVisionModels pair it with a static models list. NVIDIA is the first asked to classify over an unbounded set, with no modality and no model-kind metadata. An unknown NIM id therefore carries no signal separating a text-only chat model from an embedding endpoint, and no predicate over an id string can recover information the provider does not publish. Draft 1 kept the closed world; draft 2 escaped it but claimed knowledge that does not exist. The design that follows: enumerate the known text-only ids (correcting #964's five false positives), pin the 15 verified vision ids with explicit modelInputModalities so they become usable, leave unknown ids untouched, and record the open-world gap as a stated limitation. Confined to registry.ts with no predicate change, so no consumer edits — the reason this draft is implementable where draft 2 was not. R2-B1 also caught two consumers earlier drafts missed: web-search/index.ts:165, and cli/models.ts:44 which uses raw .includes() instead of modelInList. R2-B3 (bin/ocx.mjs cannot import diagnoseService from TypeScript) was found and fixed before the verdict arrived; 020 already reads startup.serviceInstalled from the status --json subprocess it spawns. --- .../260804_stack7_service_vision/000_scope.md | 22 ++- .../002_audit_response_r2.md | 120 ++++++++++++++ .../010_nim_vision_classification.md | 155 ++++++++++-------- .../020_service_repair_path.md | 37 ++++- 4 files changed, 252 insertions(+), 82 deletions(-) create mode 100644 devlog/_plan/260804_stack7_service_vision/002_audit_response_r2.md diff --git a/devlog/_plan/260804_stack7_service_vision/000_scope.md b/devlog/_plan/260804_stack7_service_vision/000_scope.md index 0fe057d6c..f9e9302b9 100644 --- a/devlog/_plan/260804_stack7_service_vision/000_scope.md +++ b/devlog/_plan/260804_stack7_service_vision/000_scope.md @@ -40,13 +40,21 @@ The overnight unit carried six contributor fixes verbatim with `git cherry-pick because the code was right and only the base was wrong. These two are different: each has a design defect that a straight cherry-pick would import. -**#964** classifies NVIDIA NIM models with a hand-written 60-entry allowlist of -text-only model ids. That is the same shape that failed three separate times in -the #955 line of work — a hand-maintained allowlist over an open string domain, -where every id the author did not think of silently takes the wrong branch. Here -the failure is asymmetric and user-visible: a text-only NIM model missing from -the list keeps exactly the bug #956 reports. NIM ships ~101 discoverable model -rows and adds more continuously, so the list is stale the day it merges. +**#964** classifies NVIDIA NIM models with a hand-written 60-entry allowlist, and +five entries are backwards: `thinkingmachines/inkling`, `minimaxai/minimax-m3`, +`moonshotai/kimi-k2.6`, `stepfun-ai/step-3.7-flash`, and +`mistralai/mistral-medium-3.5-128b` are natively image-capable per NVIDIA's own +documentation. Listing them makes the proxy substitute another model's text +description for an image the model could have read — silent quality loss, no +error. Issue #956's own body carries two of the same errors, so reporter and +author shared the premise. + +Two attempts to replace the list *shape* were then falsified at the audit gate +(`001`, `002`), and the root cause is recorded in `002`: NIM is the first +provider here asked to classify over an unbounded model set, and it publishes no +modality metadata, so an unknown id carries no signal at all. The landed design +fixes the known ids and states the open-world gap as a limitation rather than +claiming a mechanism that does not work. **#970** switches the post-update service refresh from `install` to `repair`. `repairService()` and `ocx service repair` **already exist** in this tree diff --git a/devlog/_plan/260804_stack7_service_vision/002_audit_response_r2.md b/devlog/_plan/260804_stack7_service_vision/002_audit_response_r2.md new file mode 100644 index 000000000..f1b3759fd --- /dev/null +++ b/devlog/_plan/260804_stack7_service_vision/002_audit_response_r2.md @@ -0,0 +1,120 @@ +# 002 — Audit round 2: FAIL again, and the root cause + +Round 2 closed B2, B3 and B5, and returned **FAIL** on two P0s plus one High. +Two consecutive failures on the same surface triggers root-cause mode +(LOOP-REPAIR-01) rather than a third patch of the same shape. + +## What closed + +- **B2** — registry `modelInputModalities` really does reach the emitted catalog + (`derive.ts:124`, `:248` → `configuredInputModalities` → + `applyProviderConfigHints`, `provider-fetch.ts:155`, `:172`). +- **B3** — narrowing the Windows skip to the install argv does not reintroduce + the direct-start race; the failure path still re-stops the backend before + reclaiming the port (`update/job.ts:807`, `:882`, `:1020`). +- **B5** — retarget → merge `dev` → require `synchronize`-generated checks is a + real fix. +- Contributor-PR closure at "open and green" is sufficient. + +## R2-B3 [High] — `diagnoseService()` from `bin/ocx.mjs` + +Found and fixed **before** the verdict arrived, independently and identically: +`bin/ocx.mjs` is Node ESM importing only `.mjs` siblings (`bin/ocx.mjs:1`, +`:11-19`), so it cannot import `src/service.ts`. `020` now specifies reading +`startup.serviceInstalled` from the `status --json` subprocess it already spawns +at `:258`. Already corrected in the tree. + +## R2-B1 [P0] — the design was prose, not a design + +"Declare a provider-level default-on rule" named no field, no type, no +precedence, and no predicate. `noVisionModels` is `string[]` and *cannot* encode +"default on with exceptions", so this was hiding real implementation cost behind +vague wording. Accepted in full. + +The reviewer also caught two consumers I missed: `src/web-search/index.ts:165`, +absent from my plan entirely, and `src/cli/models.ts:44`, which uses raw +`.includes()` instead of `modelInList` — so any predicate change silently skips +it. I had independently confirmed the `.includes()` divergence; the web-search +call site I simply missed. + +## R2-B2 [P0] — "non-chat ids never reach the predicate" is false + +I wrote that boundary into `010` *as a thing to confirm rather than assume*, and +then did not confirm it. It is false. + +NVIDIA has no discovery filter (`registry.ts:1234` — no `models`, no +`liveModels` gate), and `shouldExposeRoutedModel` rejects only media-generation +*names* (`parsing.ts:160-164`). Reproduced: + +```console +$ bun run .tmp/probe_nonchat.ts +nvidia/nv-embedqa-e5-v5 filteredOut=false +nvidia/llama-3.1-nemotron-safety-guard-8b-v3 filteredOut=false +nvidia/nemotron-ocr-v2 filteredOut=false +nvidia/llama-nemotron-rerank-1b-v2 filteredOut=false +nvidia/nemoretriever-parse filteredOut=false +``` + +Embeddings, rerankers, guards and OCR endpoints all enter the catalog, route +through `openai-chat`, and reach `planVisionSidecar`. Under default-on every one +of them would advertise image input and burn a sidecar call before failing +upstream. + +## The root cause + +Both P0s are the same defect wearing different clothes. Vision classification in +this registry has always been **membership in a bounded set**, and every existing +user of it pairs the classification with a bounded model list: + +```console +$ # providers declaring noVisionModels, and whether their model set is bounded +cursor staticModelList=true +umans staticModelList=true +ollama-cloud staticModelList=true +volcengine (×3) staticModelList=true +alibaba-token-plan-intl staticModelList=true +... 13 entries, 12 with a static list +``` + +NVIDIA is the first provider asked to classify over an **unbounded** set: no +`models` list, live discovery, ~101 rows today and more tomorrow, and no modality +metadata to separate chat from non-chat. + +That is why every design attempt failed. #964 enumerated the open side and went +stale. My complement enumerated a different closed set and changed nothing. My +default-on escaped the closed world but, lacking any chat/non-chat signal, could +not tell an unknown text model from an unknown embedding model — **because that +information does not exist in the data**. No predicate over an id string can +recover it. + +**The honest statement of the constraint:** with no modality metadata and no +model-kind metadata, unknown NIM ids cannot be classified correctly in both +directions. Any design claiming otherwise is claiming information the provider +does not publish. + +## The design that follows from the constraint + +Stop trying to classify the unknown. Fix what is knowable and bound the rest +explicitly: + +1. **Enumerate text-only ids** — as #964 did, because for a *known* id the + classification is real and verifiable. Correct its five false positives. +2. **Pin the 15 verified vision-capable ids** with explicit + `modelInputModalities` so they are usable rather than merely unlisted (B2). +3. **Do not default unknown ids in either direction.** An unknown NIM id keeps + today's behavior. This leaves #956 open for models NVIDIA ships after our + snapshot — stated as a known limitation, not hidden behind a mechanism that + does not work. +4. **Make the staleness visible and cheap to fix** instead of pretending it does + not exist: a test that fails when the registry's NIM classification drifts + from a recorded snapshot date, so the list's age is surfaced rather than + silently rotting. + +This is a smaller claim than the previous two drafts and it is the one supported +by evidence. It fixes the reported bug for every model in the report, corrects +the five entries #964 got backwards, makes the vision-capable models usable for +the first time, and states plainly what it does not solve. + +Point 4 is the part worth arguing about at the next gate: a date-stamped +snapshot test is a maintenance signal, not a correctness guarantee, and it must +not be presented as one. diff --git a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md index cf5b8941c..3d8243ec9 100644 --- a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md +++ b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md @@ -66,13 +66,14 @@ Three candidate mechanisms, all rejected on evidence: while per-page verification finds 15 image-capable chat models. The labels are incomplete. -## The design: change the predicate, not the list +## The design: enumerate what is known, bound what is not -**A first version of this document proposed maintaining the 15 vision-capable ids -and deriving text-only as their complement. The A-gate audit falsified it and I -reproduced the failure** (`001_audit_response.md`, B1). A complement taken over a -static `NVIDIA_NIM_CHAT_MODELS` still leaves an unclassified id in *neither* -list: +**Two earlier designs in this document were falsified at the audit gate.** The +history matters because it is the argument for the current one +(`001_audit_response.md`, `002_audit_response_r2.md`). + +**Draft 1 — complement over a static chat list.** Falsified: an id in neither +list is not in `noVisionModels` either, so nothing changes for it. ```console $ bun run .tmp/probe_complement.ts # the proposed shape, modelled exactly @@ -81,51 +82,63 @@ moonshotai/kimi-k2.6 sidecarWouldRun=false brandnew/model-nobody-classified sidecarWouldRun=false <-- #956 persists ``` -I had inverted which list is maintained while keeping the closed world. Any -design expressed purely as *list contents* inherits closed-world semantics, -because the classification is membership-in-a-list. Escaping it requires changing -the **predicate**. +**Draft 2 — provider-level default-on with the vision list as its exception.** +Falsified on a boundary this document had explicitly flagged as needing +confirmation, and which I then did not confirm: + +```console +$ bun run .tmp/probe_nonchat.ts +nvidia/nv-embedqa-e5-v5 filteredOut=false +nvidia/llama-3.1-nemotron-safety-guard-8b-v3 filteredOut=false +nvidia/nemotron-ocr-v2 filteredOut=false +nvidia/llama-nemotron-rerank-1b-v2 filteredOut=false +``` -### Two fields, one default +NVIDIA has no discovery filter and `shouldExposeRoutedModel` rejects only +media-generation *names* (`src/codex/catalog/parsing.ts:160-164`). Embeddings, +rerankers, guards and OCR endpoints all reach `planVisionSidecar`, so default-on +would advertise image input for every one of them. -1. **`NVIDIA_NIM_VISION_MODELS`** — the 15 verified natively-image-capable ids. - These are the exception, and they carry per-model NVIDIA documentation. -2. **A provider-level default** — for the `nvidia` entry, a model that is *not* - in the vision list is treated as needing the sidecar, without enumerating it. +### The constraint, stated honestly -Concretely this means the `nvidia` entry declares its text-only membership as -"everything except the vision list" rather than as a snapshot of ids. The -registry already carries per-provider capability flags -(`ProviderRegistryEntry`, `src/providers/registry.ts:190-230`); this adds one -more whose semantics are *default-on with an exception list*, and `router.ts` -merges it beside `noVisionModels` (`src/router.ts:243`) so a user's explicit -config still wins. +NVIDIA is the first provider in this registry asked to classify over an +**unbounded** model set. Twelve of the thirteen entries that declare +`noVisionModels` pair it with a static `models` list; NIM has none, uses live +discovery, and publishes neither modality nor model-kind metadata. -Now the open-domain case lands correctly: +So an unknown NIM id carries no signal distinguishing a text-only chat model from +an embedding endpoint. **No predicate over an id string can recover information +the provider does not publish.** Draft 2 failed not because the rule was written +badly but because it claimed knowledge that does not exist. -| Design | Unclassified new NIM model | Failure when wrong | -|---|---|---| -| #964: enumerate text-only | no sidecar | **#956 persists** — images blocked or 400 | -| complement over a static set | no sidecar | **#956 persists** (falsified above) | -| default-on with an exception list | sidecar runs | one extra description hop; image still works | +### What this design does instead -Only the third actually closes the issue for models NVIDIA has not shipped yet. +1. **Enumerate text-only ids**, as #964 did — for a *known* id the + classification is real, checkable, and fixes the reported bug. Correct the + five false positives. +2. **Pin the 15 verified vision-capable ids** with explicit + `modelInputModalities: ["text","image"]`, so they become usable instead of + merely unlisted. +3. **Leave unknown ids alone.** They keep today's behavior in both directions. +4. **Surface staleness** with a dated snapshot test, so the list's age is visible + rather than silently rotting. -### Why the exception list is safe to hand-maintain +| Case | Behavior | Honest? | +|---|---|---| +| known text-only id | sidecar runs, image advertised | fixed | +| known vision id | native path, image advertised | fixed (new) | +| unknown id | unchanged from today | **stated limitation** | -The objection that killed the previous design does not apply here. A stale -*exception* list degrades gracefully: a newly-released vision model missing from -it gets an unnecessary description hop, which is slower and costs a call but -still answers. A stale *enumeration* silently reproduces the reported bug. The -asymmetry is the entire argument, and it only holds when the default is on. +This is a smaller claim than either falsified draft, and it is the one the +evidence supports. #956 stays partially open for models NVIDIA ships after the +snapshot — recorded as a known limitation rather than hidden behind a mechanism +that does not work. -### Scope boundary +### Point 4 is a maintenance signal, not a correctness guarantee -Default-on applies to **chat** ids only. Embeddings, rerankers, guard/safety -classifiers, OCR and document extraction, image/video generation, speech, and -simulation endpoints never reach `planVisionSidecar` in the first place — they -are not routed as chat models — but the implementation must confirm that rather -than assume it, since a default-on rule has a wider blast radius than a list. +The snapshot test tells a maintainer the classification is N days old. It cannot +tell them it is *wrong*. It must not be described in the PR as if it closed the +open-world gap. ### Verified vision models also need explicit modalities (audit B2) @@ -182,40 +195,43 @@ Flagged, not yet committed to code: ## Planned diff -1. `src/providers/registry.ts` - - add `NVIDIA_NIM_VISION_MODELS` (the verified set above) with a comment - recording the verification date, the per-model source, and the standing - instruction to append to THIS list, never to a text-only one; - - add `modelInputModalities` entries pinning `["text","image"]` for each of - those ids (audit B2); - - declare the provider-level default-on rule on the `nvidia` entry, with the - vision list as its exception set, and extend the entry comment to explain - why NIM specifically gets a default rather than an enumeration. -2. The predicate change touches the classification path, so the surfaces that - read `noVisionModels` must each be checked rather than assumed: - `planVisionSidecar` (`src/vision/index.ts:235`), the fail-closed strip in - `src/server/responses/core.ts:1581`, the catalog hint - (`src/codex/catalog/provider-fetch.ts:176`), the registry→config merge - (`src/router.ts:243`), the seed fill (`src/providers/derive.ts:257`), and - `src/cli/models.ts:44`. A user's explicit config `noVisionModels` must keep - winning over the default. -3. No behavioral change is intended for any other provider. The default is - scoped to the `nvidia` entry; every other entry keeps enumerating. +Confined to `src/providers/registry.ts`. **No predicate change**, so no consumer +edits — the reason this draft is implementable where draft 2 was not. + +1. `NVIDIA_NIM_VISION_MODELS` — the 15 verified ids, with a comment recording the + verification date, per-model source, and the standing instruction that a new + NIM model must be classified deliberately, never assumed. +2. `NVIDIA_NIM_NO_VISION_MODELS` — the text-only enumeration, seeded from #964's + list with the five false positives removed and the existing + `NVIDIA_NIM_KIMI_MODELS` reconciled (kimi-k2.6 moves to the vision list). +3. `modelInputModalities` pinning `["text","image"]` for each vision id (B2), + following `ZHIPU_BIGMODEL_INPUT_MODALITIES` (`src/providers/registry.ts:327-331`). +4. Set `noVisionModels` on the `nvidia` entry and extend the entry comment with + the open-world limitation. + +Existing consumers already behave correctly once the field is populated — which +is why #956 has a working config-only workaround. Two are worth noting even +though they need no change: `src/web-search/index.ts:165` and +`src/cli/models.ts:44`, the latter using raw `.includes()` rather than +`modelInList`. Both were missed in earlier drafts and would have needed edits +under a predicate change. ## Tests and the red-green plan Extend `tests/nvidia-nim-hardening.test.ts` (the file #964 also chose): -1. **An unclassified id gets the sidecar.** The regression guard for audit B1: - a NIM model id that appears in no list at all must still produce a vision - plan when the request carries an image. Ablate by reverting to an enumerated - `noVisionModels` and watch it go red. This is the test the first design could - not have passed. -2. **Vision-capable ids do NOT get the sidecar.** Seed with the five ids #964 got +1. **Vision-capable ids do NOT get the sidecar.** Seed with the five ids #964 got wrong — `thinkingmachines/inkling`, `minimaxai/minimax-m3`, `moonshotai/kimi-k2.6`, `stepfun-ai/step-3.7-flash`, `mistralai/mistral-medium-3.5-128b` — plus the two llama-3.2 vision ids. `planVisionSidecar` returns `undefined` for each even with an image attached. + This is the direct regression guard for #964's defect. Ablate by adding one + back to `noVisionModels` and watch it go red. +2. **An unknown id is unchanged.** The honest boundary: an id in neither list + produces no vision plan, exactly as today. Asserted so the limitation is + pinned in the test suite rather than only in prose — if a later change makes + unknown ids default in either direction, this test forces that decision to be + deliberate. 3. **The catalog advertises image input for both classes, for different reasons** (audit B2): a text-only id gets `image` via the `noVisionModels` hint, and a verified vision id gets it via `modelInputModalities`. Assert the @@ -229,8 +245,11 @@ Extend `tests/nvidia-nim-hardening.test.ts` (the file #964 also chose): its own `noVisionModels` is not overridden by the provider default. 6. **A bare persisted nvidia config gets the fix** through `routeModel` — the #956 reporter's exact config shape, which today needs a manual workaround. -7. **No other provider changes classification.** A structural assertion over - `PROVIDER_REGISTRY` that the default-on rule is scoped to `nvidia` only. +7. **The two lists cannot overlap.** A structural assertion that + `NVIDIA_NIM_VISION_MODELS ∩ NVIDIA_NIM_NO_VISION_MODELS = ∅`, so a future + edit cannot put an id in both. Ablate by planting a duplicate. +8. **Snapshot staleness is visible.** The dated-snapshot guard from design point + 4. It reports age; it does not claim correctness, and its test name says so. Every guard gets driven red by ablation before it counts, per the unit's verification discipline. diff --git a/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md b/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md index 618e377e0..2f5d3221e 100644 --- a/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md +++ b/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md @@ -115,11 +115,33 @@ distinguishable from any other failure, so message-matching is unimplementable and a blanket "install after any repair failure" would reintroduce the UAC path and could re-register a service the user had just deliberately uninstalled. -Instead: after a failed repair, re-run a **structured diagnostic** -(`diagnoseService()`) and install only when it reports the service genuinely -absent while the managed-service marker still expresses intent. State beats -error-message parsing. A straight cherry-pick of #970 would import the -regression; a naive fix for it would import a worse one. +Instead: after a failed repair, consult **structured state** and install only +when it reports the service genuinely absent while the managed-service marker +still expresses intent. State beats error-message parsing. + +The mechanism differs by caller, and `bin/ocx.mjs` is the constraint. +**It is plain Node ESM** (`#!/usr/bin/env node`) and imports only `.mjs` +siblings — `bun-binary-validator.mjs`, `npm-invocation.mjs`, +`tray-update-plan.mjs` (`bin/ocx.mjs:11-19`). It cannot import +`diagnoseService()` from `src/service.ts`, so naming that function here would +have been as unimplementable as the message-parsing it replaced. + +It does not need to. `bin/ocx.mjs` **already** spawns `ocx status --json` and +parses it in exactly this code path (`bin/ocx.mjs:258`), and that payload +carries `startup.serviceInstalled` and `startup.serviceStale` +(`src/codex/autostart-health.ts:123-127`, surfaced through `src/cli/status.ts:180`). +The existing probe reads only `proxy.running`/`startup.serviceViable`; reading +`serviceInstalled` from the same response is a field access, not a new +mechanism. + +So: repair fails → the status probe that already runs reports +`serviceInstalled === false` → install once → otherwise fall through to the +direct start without re-registering. Callers inside the TypeScript runtime +(`src/update/index.ts`, `src/update/job.ts`) can call `diagnoseService()` +directly. + +A straight cherry-pick of #970 would import the stale-marker regression; a naive +fix for it would import a worse one. ## Planned diff @@ -127,8 +149,9 @@ regression; a naive fix for it would import a worse one. keep the export name for out-of-module callers. `serviceRepairCommand()` (`:500-511`) returns the backend-neutral `ocx service repair` instead of synthesizing `install --native`. -2. `bin/ocx.mjs` — refresh via repair; on failure consult `diagnoseService()` and - install only for a genuinely absent service, before the direct-start fallback. +2. `bin/ocx.mjs` — refresh via repair; on failure read `startup.serviceInstalled` + from the `status --json` probe it already performs (`:258`) and install only + for a genuinely absent service, before the direct-start fallback. 3. `src/update/index.ts` — consume the repair argv; existing non-viable/failed fallbacks unchanged. 4. `src/update/job.ts` — consume the repair argv **and** narrow the From d62276cfd01381e5563a7c1487188d83e18e6928 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 11:36:58 +0900 Subject: [PATCH 4/9] docs(devlog): a sixth false positive changes the method, not the design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit round 3 FAIL. Three failures on one document is LOOP-DOOM territory, so this changes the verification method the design rests on rather than patching the design again. R3-B1: moonshotai/kimi-k2.5 is a sixth false positive in #964's list — NVIDIA documents GIF/JPG/PNG input, four images per prompt, with hosted image_url examples. This is fatal to draft 3's justification, not just a missing entry. Draft 3 argued 'for a known id the classification is real and verifiable' while inheriting ~54 unaudited entries from #964 and calling them known. Finding a sixth immediately after correcting five proves I never verified the remainder. Every carried id now gets verified against NVIDIA docs or dropped; dropping costs today's behavior, assuming costs a silent regression. R3-B2: my registry census was wrong. Counted directly there are 17 entries declaring noVisionModels, not 13, and the two without a static models list are opencode-go and opencode-free — opencode-zen declares none at all. The numbers came from an ad-hoc regex whose entry boundaries were wrong, and I wrote its output into two documents as fact. Same failure as R2-B2, one document later. The information-constraint argument survives and the two real exceptions strengthen it: both classify only known ids, and opencode-free has a -free suffix filter NVIDIA lacks. R3-B3: test 5 asserted that a user's noVisionModels 'wins' over the registry. mergeStringArray unions them, so a user cannot remove a registry entry. Test now asserts additions are preserved. R3-B4: dropped the dated snapshot test. A local date assertion has no NVIDIA input, so it detects elapsed time rather than drift, and its cheapest CI fix is bumping the date without auditing anything. Also: 030 now requires #956 to close with an explicit bounded-scope statement, and 020 records that the status probe runs only on the success path today. --- .../003_audit_response_r3.md | 107 ++++++++++++++++++ .../010_nim_vision_classification.md | 58 +++++++--- .../020_service_repair_path.md | 13 ++- .../030_merge_and_close_sequence.md | 5 + 4 files changed, 161 insertions(+), 22 deletions(-) create mode 100644 devlog/_plan/260804_stack7_service_vision/003_audit_response_r3.md diff --git a/devlog/_plan/260804_stack7_service_vision/003_audit_response_r3.md b/devlog/_plan/260804_stack7_service_vision/003_audit_response_r3.md new file mode 100644 index 000000000..b29838b6a --- /dev/null +++ b/devlog/_plan/260804_stack7_service_vision/003_audit_response_r3.md @@ -0,0 +1,107 @@ +# 003 — Audit round 3: a sixth false positive, and a census I invented + +Round 3 returned **FAIL**. Three failures on the same document is LOOP-DOOM-01 +territory, so the response is not a fourth patch of the same shape: it changes +the *verification method* the design depends on. + +## R3-B1 [P0] — `moonshotai/kimi-k2.5` is a sixth false positive + +`010` claimed the new list is #964's enumeration "with the five false positives +removed". #964 also lists `moonshotai/kimi-k2.5`, and NVIDIA documents it as +natively multimodal — GIF/JPG/PNG, URL or base64, four images per prompt by +default, with hosted `image_url` examples +([NIM VLM docs](https://docs.nvidia.com/nim/vision-language-models/1.7.0/examples/kimi-k2.5/api.html), +[hosted inference reference](https://docs.api.nvidia.com/nim/reference/moonshotai-kimi-k2-5-infer)). +Independently confirmed. + +**Why this is fatal to the draft-3 design, not just a missing entry.** Draft 3's +entire justification was "for a *known* id the classification is real and +verifiable". Finding a sixth false positive immediately after correcting five +shows I never verified the remainder — I inherited ~54 unaudited entries from +#964 and called them "known text-only". That phrase was unsupported. + +Subtraction-by-known-exceptions is unsafe while the base list is unaudited. The +method has to change: **every entry carried from #964 gets verified against +NVIDIA documentation, or it does not ship.** An entry that cannot be verified is +dropped rather than assumed text-only — dropping costs today's behavior, and +assuming costs a silent quality regression. + +## R3-B2 [P1] — the registry census was wrong, and I generated it carelessly + +`002` and `010` claimed "12 of 13 entries pair `noVisionModels` with a static +`models` list" and named `opencode-zen` as the exception. Counted directly: + +```console +$ rg -c "^\s+noVisionModels:" src/providers/registry.ts +17 +``` + +There are **17** such entries. Fifteen have a static `models` list; the two that +do not are **`opencode-go`** (`registry.ts:877`) and **`opencode-free`** +(`registry.ts:1585`). `opencode-zen` declares no `noVisionModels` at all. + +The wrong numbers came from an ad-hoc regex over entry bodies whose boundaries it +got wrong, and I put its output into a document as a census without checking it. +That is the same failure as R2-B2 — asserting rather than verifying — repeated +one document later. Ad-hoc extraction is now treated as a hypothesis, not +evidence. + +**The information-constraint argument survives**, and the two real exceptions +strengthen rather than weaken it: both classify only known ids and leave unknown +ids untouched, exactly as draft 3 proposes. `opencode-free` additionally has a +provider-specific `-free` suffix filter (`provider-fetch.ts:636`) giving it a +model-kind signal NVIDIA does not have. But "first provider" and the counts are +corrected wherever they appear. + +## R3-B3 [P1] — test 5 asserts semantics that do not exist + +`010` said a user's explicit config `noVisionModels` "wins" over the registry. +It does not: `mergeStringArray` **unions** them (`router.ts:95`, `:243`). A user +cannot remove a registry classification by supplying their own list. + +The test becomes "user additions are preserved alongside registry entries". +Introducing replacement or negation semantics is a separate change with its own +design cost and is out of scope here. + +## R3-B4 [P2] — drop the dated snapshot test + +I flagged it as the weakest part and the reviewer agreed for a sharper reason: a +local date assertion has no NVIDIA input, so it cannot detect drift — only +elapsed time. As a required unit test it becomes a calendar-triggered CI failure +whose cheapest fix is bumping the date without auditing anything, which actively +launders staleness. + +**Dropped.** The registry comment records the verification date and the standing +instruction; a future maintainer gets the date without CI theatre. `002`'s +description of it as detecting "drift" was wrong and is corrected. + +## What survived round 3 + +- **Accepting the unknown-model gap is legitimate, not premature surrender.** + The reviewer looked for a runtime discriminator and found none: + `formatOpenAIChatErrorBody` extracts arbitrary error text with no stable + modality code (`openai-chat.ts:33`), and the NIM tests establish none. A + retry-on-modality-error scheme would also not help the catalog gate, since + unknown models would first have to advertise image support — re-exposing + embeddings, rerankers, OCR and guards. +- **Vision and reasoning axes are orthogonal.** kimi-k2.5 and k2.6 stay in + `NVIDIA_NIM_KIMI_MODELS` for reasoning suppression while joining the vision + set. Confirmed by probe and by the reviewer against + `tests/nvidia-nim-hardening.test.ts:42`. +- **`020`'s `status --json` fix is implementable**, with one implementation note: + the probe currently runs only after a *successful* service command + (`bin/ocx.mjs:253`), so the failure path must move or duplicate it. The planned + failure-path test catches this. +- **`030`'s sequencing holds.** One correction: `010` says #956 stays partially + open for future models while `030` schedules closing it on merge. The closing + comment must state the bounded snapshot scope explicitly rather than claiming a + complete fix. + +## Method change carried into implementation + +1. Every #964-inherited id is verified against NVIDIA documentation before it + ships. Unverifiable ids are dropped, not assumed. +2. Counts and inventories come from direct, re-runnable commands whose output is + pasted, never from an ad-hoc regex summarized from memory. +3. Claims about mechanism (`mergeStringArray` semantics, catalog filters) are + read in the source before being written into a document. diff --git a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md index 3d8243ec9..1583ec19f 100644 --- a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md +++ b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md @@ -101,10 +101,25 @@ would advertise image input for every one of them. ### The constraint, stated honestly -NVIDIA is the first provider in this registry asked to classify over an -**unbounded** model set. Twelve of the thirteen entries that declare -`noVisionModels` pair it with a static `models` list; NIM has none, uses live -discovery, and publishes neither modality nor model-kind metadata. +Seventeen registry entries declare `noVisionModels`; fifteen pair it with a +static `models` list: + +```console +$ rg -c "^\s+noVisionModels:" src/providers/registry.ts +17 +``` + +The two exceptions are `opencode-go` (`registry.ts:877`) and `opencode-free` +(`registry.ts:1585`). Neither is a counter-pattern: both classify only known ids +and leave unknown ones untouched, and `opencode-free` additionally has a +provider-specific `-free` suffix filter (`provider-fetch.ts:636`) — a model-kind +signal NVIDIA does not publish. + +*(An earlier revision of this section said "12 of 13" and named `opencode-zen` +as the exception. Both were wrong; see `003_audit_response_r3.md`.)* + +NIM has no `models` list, uses live discovery, and publishes neither modality nor +model-kind metadata. So an unknown NIM id carries no signal distinguishing a text-only chat model from an embedding endpoint. **No predicate over an id string can recover information @@ -113,15 +128,21 @@ badly but because it claimed knowledge that does not exist. ### What this design does instead -1. **Enumerate text-only ids**, as #964 did — for a *known* id the - classification is real, checkable, and fixes the reported bug. Correct the - five false positives. +1. **Enumerate text-only ids — but only ids we have actually verified.** Draft 3 + said "for a known id the classification is real and verifiable" while + inheriting ~54 unaudited entries from #964 and calling them known. A sixth + false positive (`moonshotai/kimi-k2.5`) surfaced immediately after correcting + five, so **every carried id is verified against NVIDIA documentation or + dropped**. Dropping costs today's behavior; assuming costs a silent + regression. 2. **Pin the 15 verified vision-capable ids** with explicit `modelInputModalities: ["text","image"]`, so they become usable instead of merely unlisted. 3. **Leave unknown ids alone.** They keep today's behavior in both directions. -4. **Surface staleness** with a dated snapshot test, so the list's age is visible - rather than silently rotting. +4. **Record the verification date in the registry comment.** A dated *test* was + considered and dropped (`003_audit_response_r3.md`): a local date assertion + has no NVIDIA input, so it detects elapsed time rather than drift, and its + cheapest CI fix is bumping the date without auditing anything. | Case | Behavior | Honest? | |---|---|---| @@ -134,11 +155,11 @@ evidence supports. #956 stays partially open for models NVIDIA ships after the snapshot — recorded as a known limitation rather than hidden behind a mechanism that does not work. -### Point 4 is a maintenance signal, not a correctness guarantee +### The known-id half is only as good as its audit -The snapshot test tells a maintainer the classification is N days old. It cannot -tell them it is *wrong*. It must not be described in the PR as if it closed the -open-world gap. +This design's correctness rests entirely on the per-id verification, not on the +list's shape. That is why the audit is a gating step rather than a nicety, and +why an unverifiable id is dropped instead of carried. ### Verified vision models also need explicit modalities (audit B2) @@ -241,15 +262,18 @@ Extend `tests/nvidia-nim-hardening.test.ts` (the file #964 also chose): `deepseek-ai/deepseek-v4-flash`, `z-ai/glm-5.2`, `nvidia/nemotron-3-ultra-550b-a55b`, `openai/gpt-oss-120b`. Sidecar on, no image forwarded raw. -5. **A user's explicit config wins.** A persisted `nvidia` provider that names - its own `noVisionModels` is not overridden by the provider default. +5. **A user's config additions are preserved.** `mergeStringArray` **unions** + registry and user arrays (`src/router.ts:95`, `:243`), so a user cannot + remove a registry classification by supplying their own list. An earlier + revision asserted the user's list "wins", which is not the shipped semantic + (`003_audit_response_r3.md`). Replacement/negation semantics would be a + separate change with its own design cost; out of scope here. 6. **A bare persisted nvidia config gets the fix** through `routeModel` — the #956 reporter's exact config shape, which today needs a manual workaround. 7. **The two lists cannot overlap.** A structural assertion that `NVIDIA_NIM_VISION_MODELS ∩ NVIDIA_NIM_NO_VISION_MODELS = ∅`, so a future edit cannot put an id in both. Ablate by planting a duplicate. -8. **Snapshot staleness is visible.** The dated-snapshot guard from design point - 4. It reports age; it does not claim correctness, and its test name says so. +8. *(Dropped — the dated-snapshot guard. See `003_audit_response_r3.md` R3-B4.)* Every guard gets driven red by ablation before it counts, per the unit's verification discipline. diff --git a/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md b/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md index 2f5d3221e..3a0062647 100644 --- a/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md +++ b/devlog/_plan/260804_stack7_service_vision/020_service_repair_path.md @@ -134,11 +134,14 @@ The existing probe reads only `proxy.running`/`startup.serviceViable`; reading `serviceInstalled` from the same response is a field access, not a new mechanism. -So: repair fails → the status probe that already runs reports -`serviceInstalled === false` → install once → otherwise fall through to the -direct start without re-registering. Callers inside the TypeScript runtime -(`src/update/index.ts`, `src/update/job.ts`) can call `diagnoseService()` -directly. +So: repair fails → the status probe reports `serviceInstalled === false` → +install once → otherwise fall through to the direct start without re-registering. +Callers inside the TypeScript runtime (`src/update/index.ts`, +`src/update/job.ts`) can call `diagnoseService()` directly. + +**Implementation note:** that probe currently runs only on the *success* path, +after a service command exits 0 (`bin/ocx.mjs:253`). The failure path must move +or duplicate it. The planned failure-path test is what catches this. A straight cherry-pick of #970 would import the stale-marker regression; a naive fix for it would import a worse one. diff --git a/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md b/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md index 9a31a6c98..0f4a5e584 100644 --- a/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md +++ b/devlog/_plan/260804_stack7_service_vision/030_merge_and_close_sequence.md @@ -67,6 +67,11 @@ trackers untrustworthy. | #915 | cooldown early-recovery probe | #955 | | #962 | custom rows inherit provider metadata (carried #965) | #973 | | #956 | NIM vision classification | stack 7 | + +**#956 closes with a bounded scope statement.** `010` fixes the classification +for verified ids and explicitly leaves unknown/future NIM models unchanged. The +closing comment must say so rather than implying a complete fix — otherwise the +issue reads as resolved for a model NVIDIA ships next month, which it is not. | issues fixed by the six carried contributor fixes | — | #953 | The #953 row is deliberately unresolved here: the six carried fixes From 76306eb542a8baf7435ac9b5bf639a67f78df4b4 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 11:45:55 +0900 Subject: [PATCH 5/9] =?UTF-8?q?docs(devlog):=20audit=20every=20id=20in=20#?= =?UTF-8?q?964's=20list=20=E2=80=94=2026=20ship,=206=20reversed,=2032=20dr?= =?UTF-8?q?opped?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 003 made per-id verification a gating step. This is that audit, run against build.nvidia.com model pages and the NIM LLM/Visual API indexes on 2026-08-04. #964 submitted ~64 ids. Fewer than half survive: 26 confirmed text-only (explicit 'Input Modalities: Text') — these ship 6 confirmed image-capable — moved to the vision list 32 unverified or absent from NVIDIA's catalog — dropped The 26 include z-ai/glm-5.2, deepseek-v4-flash/pro and the nemotron-3 family, so the models issue #956 actually names are all fixed. No seventh false positive was found, which is the first evidence the correction has converged rather than merely advanced. The 32 dropped are mostly delisted models — harmless in isolation, since nobody can route to a model NVIDIA no longer serves. But the set includes nvidia/nemotron-nano-3-30b-a3b, a reversed-name typo of the real nvidia/nemotron-3-nano-30b-a3b which the same list also spells correctly, and mistralai/mixtral-8x22b-v0.1 where NVIDIA documents mixtral-8x22b-instruct-v0.1. Half the list was assembled rather than verified; the six reversed entries were the visible damage, this is the extent of it. Kimi is now split correctly across two independent axes: k2.5 and k2.6 join the vision list, k2-thinking and k2-instruct stay text-only, and all four remain in NVIDIA_NIM_KIMI_MODELS for reasoning suppression. google/codegemma-7b verifies while google/codegemma-1.1-7b does not — adjacent names, opposite outcomes, which is why name-based classification was rejected. --- .../260804_stack7_service_vision/000_scope.md | 3 + .../010_nim_vision_classification.md | 10 +- .../011_nim_id_audit.md | 103 ++++++++++++++++++ 3 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md diff --git a/devlog/_plan/260804_stack7_service_vision/000_scope.md b/devlog/_plan/260804_stack7_service_vision/000_scope.md index f9e9302b9..c4bb9f67b 100644 --- a/devlog/_plan/260804_stack7_service_vision/000_scope.md +++ b/devlog/_plan/260804_stack7_service_vision/000_scope.md @@ -80,6 +80,9 @@ answered in `020` before any code is written. | Doc | Contents | |---|---| | `001_audit_response.md` | A-gate FAIL — five blockers, synthesis, and what changed | +| `002_audit_response_r2.md` | A-gate FAIL round 2 — root cause and the design that follows | +| `003_audit_response_r3.md` | A-gate FAIL round 3 — a sixth false positive changes the method | | `010_nim_vision_classification.md` | #964 reconstruction — the classification design and its diff | +| `011_nim_id_audit.md` | per-id verification of #964's list: 26 ship, 6 reversed, 32 dropped | | `020_service_repair_path.md` | #970 reconstruction — call-site inventory and the after-stop safety proof | | `030_merge_and_close_sequence.md` | bottom-up merge order, retargeting, and issue closure evidence | diff --git a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md index 1583ec19f..6b259cc5f 100644 --- a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md +++ b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md @@ -194,6 +194,7 @@ google/gemma-4-31b-it google/diffusiongemma-26b-a4b-it minimaxai/minimax-m3 moonshotai/kimi-k2.6 +moonshotai/kimi-k2.5 stepfun-ai/step-3.7-flash thinkingmachines/inkling mistralai/mistral-medium-3.5-128b @@ -222,9 +223,12 @@ edits — the reason this draft is implementable where draft 2 was not. 1. `NVIDIA_NIM_VISION_MODELS` — the 15 verified ids, with a comment recording the verification date, per-model source, and the standing instruction that a new NIM model must be classified deliberately, never assumed. -2. `NVIDIA_NIM_NO_VISION_MODELS` — the text-only enumeration, seeded from #964's - list with the five false positives removed and the existing - `NVIDIA_NIM_KIMI_MODELS` reconciled (kimi-k2.6 moves to the vision list). +2. `NVIDIA_NIM_NO_VISION_MODELS` — **the 26 ids verified in `011_nim_id_audit.md` + and no others.** #964 submitted ~64; six were image-capable and 32 could not + be verified against any current NVIDIA page, so they are dropped rather than + carried. `NVIDIA_NIM_KIMI_MODELS` is reconciled: k2.5 and k2.6 move to the + vision list while k2-thinking and k2-instruct stay text-only, and all four + remain in the reasoning-suppression list, which is an independent axis. 3. `modelInputModalities` pinning `["text","image"]` for each vision id (B2), following `ZHIPU_BIGMODEL_INPUT_MODALITIES` (`src/providers/registry.ts:327-331`). 4. Set `noVisionModels` on the `nvidia` entry and extend the entry comment with diff --git a/devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md b/devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md new file mode 100644 index 000000000..fa76d1048 --- /dev/null +++ b/devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md @@ -0,0 +1,103 @@ +# 011 — Per-id audit of #964's list against NVIDIA documentation + +`003` made per-id verification a gating step: an id ships only if NVIDIA's own +documentation says text-only, otherwise it is dropped. This is that audit, +performed 2026-08-04 against `build.nvidia.com` model pages, +`docs.api.nvidia.com/nim/reference/*`, and the +[LLM APIs index](https://docs.api.nvidia.com/nim/reference/llm-apis) +cross-checked against the +[Visual Models index](https://docs.api.nvidia.com/nim/reference/visual-models-apis). + +## Result + +| Bucket | Count | +|---|---| +| Confirmed text-only — ship | 26 | +| Confirmed image-capable — moved to the vision list | 6 | +| Unverified / absent from NVIDIA's catalog — dropped | 32 | + +#964 submitted ~64 ids. **Fewer than half survive verification.** + +## Confirmed text-only (26) — these ship + +Each carries an explicit `Input Modalities: Text`, `Input Type(s): Text`, or +equivalent on its NVIDIA page: + +``` +deepseek-ai/deepseek-v4-flash nvidia/llama-3.1-nemotron-nano-8b-v1 +deepseek-ai/deepseek-v4-pro nvidia/llama-3.1-nemotron-ultra-253b-v1 +google/codegemma-7b nvidia/llama-3.3-nemotron-super-49b-v1 +meta/llama-3.1-70b-instruct nvidia/llama-3.3-nemotron-super-49b-v1.5 +meta/llama-3.1-8b-instruct nvidia/nemotron-3-nano-30b-a3b +meta/llama-3.2-1b-instruct nvidia/nemotron-3-super-120b-a12b +meta/llama-3.2-3b-instruct nvidia/nemotron-3-ultra-550b-a55b +meta/llama-3.3-70b-instruct nvidia/nemotron-mini-4b-instruct +meta/llama2-70b nvidia/nvidia-nemotron-nano-9b-v2 +mistralai/mistral-7b-instruct-v0.3 openai/gpt-oss-120b +mistralai/mistral-nemotron openai/gpt-oss-20b +moonshotai/kimi-k2-thinking poolside/laguna-xs-2.1 +moonshotai/kimi-k2-instruct z-ai/glm-5.2 +``` + +`z-ai/glm-5.2`, `deepseek-v4-flash`/`-pro` and the nemotron-3 family are the ids +issue #956 actually names, so the reported bug is fixed for every model in the +report. + +Two notes for the implementation: + +- `moonshotai/kimi-k2-thinking` and `kimi-k2-instruct` are text-only and stay in + `NVIDIA_NIM_KIMI_MODELS` for reasoning suppression. Only k2.5 and k2.6 move to + the vision list. The two axes are independent fields + (`registry.ts:1238-1240`), verified by probe. +- `google/codegemma-7b` is confirmed while `google/codegemma-1.1-7b` is not — the + point-release id has no current page. Near-identical names, different outcomes; + exactly why name-based reasoning was rejected. + +## Confirmed image-capable (6) — moved to the vision list + +`thinkingmachines/inkling`, `minimaxai/minimax-m3`, `moonshotai/kimi-k2.6`, +`moonshotai/kimi-k2.5`, `stepfun-ai/step-3.7-flash`, +`mistralai/mistral-medium-3.5-128b`. Sources in `010` and `003`. + +The audit found **no seventh** false positive among the remaining ids. That is +the first evidence that the correction has converged rather than merely advanced. + +## Unverified — dropped (32) + +Dropped rather than carried, per `003`. Dropping an id costs today's behavior; +carrying an unverified one risks the silent substitution this whole unit exists +to prevent. + +**Absent from NVIDIA's current catalog (27).** `01-ai/yi-large`, +`ai21labs/jamba-1.5-large-instruct`, `aisingapore/sea-lion-7b-instruct`, +`bigcode/starcoder2-15b`, `databricks/dbrx-instruct`, +`deepseek-ai/deepseek-coder-6.7b-instruct`, `google/codegemma-1.1-7b`, +`google/gemma-2b`, `google/recurrentgemma-2b`, the four `ibm/granite-*`, +`meta/codellama-70b`, `microsoft/phi-3.5-moe-instruct`, +`mistralai/codestral-22b-instruct-v0.1`, `mistralai/mistral-large`, +`nv-mistralai/mistral-nemo-12b-instruct`, +`nvidia/llama-3.1-nemotron-51b-instruct`, `nvidia/llama3-chatqa-1.5-70b`, +`nvidia/mistral-nemo-minitron-8b-8k-instruct`, `nvidia/nemotron-4-340b-instruct`, +the four `writer/palmyra-*`, `zyphra/zamba2-7b-instruct`. + +**Page exists, modality field absent (2).** `moonshotai/kimi-k2-instruct-0905` +and `nvidia/llama-3.1-nemotron-70b-instruct` — both deprecated endpoints whose +specifications omit modalities. + +**Id does not match NVIDIA's catalog (3).** `mistralai/mistral-large-2-instruct` +(no such id), `mistralai/mixtral-8x22b-v0.1` (NVIDIA documents +`mixtral-8x22b-instruct-v0.1`), and `nvidia/nemotron-nano-3-30b-a3b` — a +**reversed-name typo** of the real `nvidia/nemotron-3-nano-30b-a3b`, which #964 +also lists correctly. Both spellings were in the submitted list; only one is a +real model. + +## What this audit demonstrates + +The dropped ids are mostly harmless-looking noise — a delisted model classified +wrongly costs nothing because nobody can route to it. But 32 of 64 entries being +unverifiable, including a typo'd duplicate, is the measure of how much of #964's +list was assembled rather than verified. The six reversed entries were the +visible damage; this is the extent of it. + +It also bounds the fix honestly. 26 verified ids is a real fix for real models, +not a claim about NIM as a whole. From cb33f59f15971e8a9481edfbc21f34caf12c0682 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 11:54:49 +0900 Subject: [PATCH 6/9] docs(devlog): plan the qwen3.8-max rename and close the round-4 residuals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 040: qwen3.8-max-preview becomes qwen3.8-max, and the price overlay moves from a Routeway reseller proxy to Alibaba's published rate. Alibaba released Qwen3.8-Max as stable on 2026-08-03 and documents the preview endpoint as liable to be taken offline. Model Studio lists both ids today, so the rename touches 10 sites in registry.ts, 3 in expected-prices.ts, and 6 test files across both alibaba-token-plan providers. No -preview alias is added. A config naming the old id still routes, because routeModel accepts an arbitrary namespaced id for a configured provider and the upstream still serves it; what such a user loses is capability metadata keyed to a retiring preview id, which is the correct outcome. Price: Qwen publishes $2 input / $6 output. Two honesty constraints recorded — the figure is Qwen's own announcement and Model Studio has no qwen3.8-max row yet, and cache rates are unpublished so both cache fields go to 0 rather than inheriting the Routeway numbers. Carrying a reseller cache rate under a vendor price label would be a wrong number wearing a verified badge. The Routeway constant and its overlay are removed entirely, which is exactly the exit condition its own comment named. Round-4 NEAR-PASS residuals closed: - k2.5 raised the vision set to 16 and the reversed-entry count to six; both numbers were stale in 000, 010. The k2.5 regression test was missing from the no-sidecar and emitted-modality cases and is now required in both. - 011 claimed a delisted id 'cannot be routed to'. False: routeModel accepts arbitrary namespaced ids and a stale cache can surface one. The disposition holds for a narrower reason — exclusion leaves them at today's unclassified behavior — and the text now says that instead. - The Mistral Medium 3.5 hosted-endpoint recheck is resolved; it ships. --- .../260804_stack7_service_vision/000_scope.md | 23 ++-- .../010_nim_vision_classification.md | 31 ++--- .../011_nim_id_audit.md | 19 ++- .../040_qwen38_max_rename_pricing.md | 110 ++++++++++++++++++ 4 files changed, 156 insertions(+), 27 deletions(-) create mode 100644 devlog/_plan/260804_stack7_service_vision/040_qwen38_max_rename_pricing.md diff --git a/devlog/_plan/260804_stack7_service_vision/000_scope.md b/devlog/_plan/260804_stack7_service_vision/000_scope.md index c4bb9f67b..c617ff099 100644 --- a/devlog/_plan/260804_stack7_service_vision/000_scope.md +++ b/devlog/_plan/260804_stack7_service_vision/000_scope.md @@ -14,6 +14,10 @@ as superseded. | #964 | @Yuxin-Qiao | #956 | NVIDIA NIM text-only models never activate the vision sidecar | | #970 | @stephen-drew | — | `ocx update` re-registers the background service from a non-elevated updater | +A third item was added after the roadmap cycle opened, at the user's request: +renaming `qwen3.8-max-preview` to the now-stable `qwen3.8-max` and replacing its +reseller-proxy price overlay with Alibaba's published $2/$6 rate (`040`). + Layer 7 is the last layer. After it lands the stack merges bottom-up from #952 and every issue a landed layer resolves gets closed with its merge commit named. @@ -40,14 +44,17 @@ The overnight unit carried six contributor fixes verbatim with `git cherry-pick because the code was right and only the base was wrong. These two are different: each has a design defect that a straight cherry-pick would import. -**#964** classifies NVIDIA NIM models with a hand-written 60-entry allowlist, and -five entries are backwards: `thinkingmachines/inkling`, `minimaxai/minimax-m3`, -`moonshotai/kimi-k2.6`, `stepfun-ai/step-3.7-flash`, and -`mistralai/mistral-medium-3.5-128b` are natively image-capable per NVIDIA's own -documentation. Listing them makes the proxy substitute another model's text -description for an image the model could have read — silent quality loss, no -error. Issue #956's own body carries two of the same errors, so reporter and -author shared the premise. +**#964** classifies NVIDIA NIM models with a hand-written ~64-entry allowlist, and +**six** entries are backwards: `thinkingmachines/inkling`, +`minimaxai/minimax-m3`, `moonshotai/kimi-k2.6`, `moonshotai/kimi-k2.5`, +`stepfun-ai/step-3.7-flash`, and `mistralai/mistral-medium-3.5-128b` are natively +image-capable per NVIDIA's own documentation. Listing them makes the proxy +substitute another model's text description for an image the model could have +read — silent quality loss, no error. Issue #956's own body carries two of the +same errors, so reporter and author shared the premise. + +A per-id audit of the whole list (`011`) found only 26 of ~64 entries verifiable +as text-only; 32 are absent from NVIDIA's current catalog and are dropped. Two attempts to replace the list *shape* were then falsified at the audit gate (`001`, `002`), and the root cause is recorded in `002`: NIM is the first diff --git a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md index 6b259cc5f..a84c1b8e8 100644 --- a/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md +++ b/devlog/_plan/260804_stack7_service_vision/010_nim_vision_classification.md @@ -41,12 +41,12 @@ fourth time in this session's line of work that a hand-written allowlist over an open string domain has been wrong; the pattern is the finding, not the individual entries. -One caveat carried forward: the Mistral Medium 3.5 evidence describes the -**self-hosted** VLM NIM container, and that documentation explicitly warns not to -assume a general text endpoint exposes vision. We attach to hosted -`integrate.api.nvidia.com`. Re-verify against the hosted model page before the id -lands. Either way it is not confidently text-only, so #964's classification of it -is unsupported. +*(Resolved.) The Mistral Medium 3.5 caveat — that the first evidence described a +**self-hosted** VLM container rather than the hosted endpoint — was closed at the +round-4 gate. NVIDIA's hosted inference reference targets +`integrate.api.nvidia.com`, defaults to this exact id, and documents `image_url` +input, and the Build page reports a free hosted endpoint with Text and Image +modalities. It ships in the vision set with no pending recheck. ## Why not classify by name, tag, or API @@ -135,7 +135,7 @@ badly but because it claimed knowledge that does not exist. five, so **every carried id is verified against NVIDIA documentation or dropped**. Dropping costs today's behavior; assuming costs a silent regression. -2. **Pin the 15 verified vision-capable ids** with explicit +2. **Pin the 16 verified vision-capable ids** with explicit `modelInputModalities: ["text","image"]`, so they become usable instead of merely unlisted. 3. **Leave unknown ids alone.** They keep today's behavior in both directions. @@ -202,8 +202,9 @@ mistralai/mistral-medium-3.5-128b Flagged, not yet committed to code: -- `mistralai/mistral-medium-3.5-128b` — hosted-endpoint recheck pending (above); - NVIDIA also showed a 2026-08-07 deprecation date. +- `mistralai/mistral-medium-3.5-128b` — hosted endpoint **confirmed** at the + round-4 gate, so it ships. NVIDIA showed a 2026-08-07 deprecation date; when + the id goes away it classifies nothing, which is harmless. - `nvidia/llama-3.1-nemotron-nano-vl-8b-v1` — catalog indicated imminent deprecation; harmless if it disappears (an absent id classifies nothing). - `google/paligemma` — established VLM, but its detail page 404s. Excluded from @@ -220,7 +221,7 @@ Flagged, not yet committed to code: Confined to `src/providers/registry.ts`. **No predicate change**, so no consumer edits — the reason this draft is implementable where draft 2 was not. -1. `NVIDIA_NIM_VISION_MODELS` — the 15 verified ids, with a comment recording the +1. `NVIDIA_NIM_VISION_MODELS` — the 16 verified ids, with a comment recording the verification date, per-model source, and the standing instruction that a new NIM model must be classified deliberately, never assumed. 2. `NVIDIA_NIM_NO_VISION_MODELS` — **the 26 ids verified in `011_nim_id_audit.md` @@ -245,10 +246,12 @@ under a predicate change. Extend `tests/nvidia-nim-hardening.test.ts` (the file #964 also chose): -1. **Vision-capable ids do NOT get the sidecar.** Seed with the five ids #964 got - wrong — `thinkingmachines/inkling`, `minimaxai/minimax-m3`, - `moonshotai/kimi-k2.6`, `stepfun-ai/step-3.7-flash`, - `mistralai/mistral-medium-3.5-128b` — plus the two llama-3.2 vision ids. +1. **Vision-capable ids do NOT get the sidecar.** Seed with all **six** ids #964 + got wrong — `thinkingmachines/inkling`, `minimaxai/minimax-m3`, + `moonshotai/kimi-k2.6`, **`moonshotai/kimi-k2.5`**, + `stepfun-ai/step-3.7-flash`, `mistralai/mistral-medium-3.5-128b` — plus the + two llama-3.2 vision ids. k2.5 was the round-3 P0 and must appear in both this + test and the emitted-modality test below, not only in the list. `planVisionSidecar` returns `undefined` for each even with an image attached. This is the direct regression guard for #964's defect. Ablate by adding one back to `noVisionModels` and watch it go red. diff --git a/devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md b/devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md index fa76d1048..9f3f0bcba 100644 --- a/devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md +++ b/devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md @@ -93,11 +93,20 @@ real model. ## What this audit demonstrates -The dropped ids are mostly harmless-looking noise — a delisted model classified -wrongly costs nothing because nobody can route to it. But 32 of 64 entries being -unverifiable, including a typo'd duplicate, is the measure of how much of #964's -list was assembled rather than verified. The six reversed entries were the -visible damage; this is the extent of it. +Dropping is safe, but not for the reason first written here. A delisted id **is** +still reachable: `routeModel` accepts an arbitrary namespaced id for a configured +provider (`src/router.ts:402-421`), the default-provider fallback accepts +arbitrary ids (`:451-455`), and a stale discovery cache or custom row can surface +one. The correct statement is narrower: **excluding these ids leaves them at +today's unclassified behavior even when reached**, since the current `nvidia` +entry classifies none of them, and a user's own `noVisionModels` entry still +survives the union merge. + +What the drop count measures is provenance. 32 of 64 entries unverifiable — +including `nvidia/nemotron-nano-3-30b-a3b`, a reversed-name typo of a real id the +same list also spells correctly — shows how much of #964's list was assembled +rather than verified. The six reversed entries were the visible damage; this is +the extent of it. It also bounds the fix honestly. 26 verified ids is a real fix for real models, not a claim about NIM as a whole. diff --git a/devlog/_plan/260804_stack7_service_vision/040_qwen38_max_rename_pricing.md b/devlog/_plan/260804_stack7_service_vision/040_qwen38_max_rename_pricing.md new file mode 100644 index 000000000..783503dee --- /dev/null +++ b/devlog/_plan/260804_stack7_service_vision/040_qwen38_max_rename_pricing.md @@ -0,0 +1,110 @@ +# 040 — `qwen3.8-max-preview` → `qwen3.8-max`, with the vendor's published price + +Added to this unit after the roadmap cycle opened, at the user's request. It +ships as its own commits on the stack-7 branch. + +## The rename + +Alibaba released Qwen3.8-Max as a stable production model on 2026-08-03; the +preview endpoint is documented as liable to be taken offline once preview +concludes ([Qwen blog](https://qwen.ai/blog?id=qwen3.8)). Model Studio lists both +ids today — `qwen3.8-max` as the recommended model, `qwen3.8-max-preview` still +present as a separate preview id +([Model Studio models](https://help.aliyun.com/en/model-studio/getting-started/models)). + +So `qwen3.8-max` becomes the id this registry names everywhere. Both providers +are affected: `alibaba-token-plan` and `alibaba-token-plan-intl`. + +### Whether to keep `-preview` as an alias + +**Decision: do not add an alias.** A user whose config names +`qwen3.8-max-preview` keeps working without one, because `routeModel` accepts an +arbitrary namespaced model id for a configured provider (`src/router.ts:402-421`) +and forwards it to the upstream, which still serves that id. The registry entry +governs catalog rows and capability metadata, not whether a hand-named id can be +called. + +What such a user loses is the capability metadata keyed to the old id — context +window, reasoning efforts, `preserveReasoningContentModels`. That is the correct +outcome for a preview id the vendor is retiring: the metadata should follow the +supported model. Adding an alias would instead pin retired-preview metadata into +the registry indefinitely. + +### Call sites + +`src/providers/registry.ts` — 10 occurrences: the two model lists (`:355`, +`:359`, `:375`, `:382`), input modalities (`:362`, `:453`), `defaultModel` +(`:1424`), context windows `983_616` (`:1430`, `:1459`), reasoning efforts +(`:1468`), `modelDefaultReasoningEfforts` (`:1481`), and +`preserveReasoningContentModels` (`:1440`, `:1478`). + +`src/usage/expected-prices.ts` — the two overlay rows (`:137`, `:138`) plus the +source constant (`:62-63`). + +Tests naming the old id: `tests/alibaba-intl-token-plan.test.ts`, +`tests/qwen38-preserve-reasoning.test.ts`, `tests/claude-desktop-1m.test.ts`, +`tests/subagent-model-fallback-api.test.ts`, +`tests/router-discarded-baseurl-warning.test.ts`, +`tests/provider-registry-parity.test.ts`. + +Context window (983,616), reasoning efforts (`low`/`high`/`xhigh`), default +effort (`xhigh`), and modalities (`["text","image"]`) all carry over unchanged — +this is a rename, not a re-specification. + +## The price + +Qwen publishes **$2 input / $6 output** per million tokens for Qwen3.8-Max +([Qwen blog](https://qwen.ai/blog?id=qwen3.8)), matching the figures the user +gave. + +Today the overlay carries a **reseller proxy** rate: + +```ts +const QWEN38_ROUTEWAY_TEMPORARY: Cost4 = { input: 1.5, output: 5, cacheRead: 0.15, cacheWrite: 0 }; +// "https://routeway.ai/models/qwen3.8-max-preview (temporary reseller proxy; +// NOT Alibaba Token Plan billing; cacheWrite unpublished -> 0)" +``` + +Its own comment states the exit condition: *"Replace these overlays when Alibaba +publishes an official qwen3.8-max-preview token rate."* A vendor price now +exists, so the Routeway overlay and its constant are removed entirely rather than +edited. + +### Two honesty constraints + +**The $2/$6 figure is Qwen's own announcement, not a Model Studio billing table.** +The Model Studio pricing page lists `qwen3.7-max` ($2.50/$7.50) and `qwen3-max` +(tiered $1.20–$3.00 / $6.00–$15.00) but does not yet carry a `qwen3.8-max` row +([Model Studio pricing](https://www.alibabacloud.com/help/en/model-studio/model-pricing)). +The source string must say so instead of implying a billing-table verification. + +**Cache rates are unpublished.** The existing `cacheRead: 0.15` is a Routeway +number; once that source is dropped, nothing supports it. Both cache fields go to +`0`, following the convention already used in this file +(`cacheWrite unpublished -> 0`, and `GEMINI_PRICING`'s per-hour-storage note). +Carrying a reseller cache rate under a vendor-price label would be the worse +outcome — a wrong number wearing a verified badge. + +Status stays **`verified`** for input/output: the vendor published them. The +source string records that cache is unpublished and that Model Studio has no row +yet. + +## Delisted models + +"Remove models no longer served" applies to the same sweep as `011`: an id absent +from the vendor's current catalog is removed rather than carried. For NIM that is +the 32 ids `011` already dropped. For Alibaba it is the preview id, superseded +above. + +## Tests + +1. Both Alibaba entries expose `qwen3.8-max` and no longer expose + `qwen3.8-max-preview`. +2. Capability metadata survives the rename — context window 983,616, efforts + `["low","high","xhigh"]`, default `xhigh`, modalities `["text","image"]`, + membership in `preserveReasoningContentModels`. Ablate by dropping one + metadata key during the rename and watch it go red. +3. The price overlay returns `{ input: 2, output: 6 }` for both providers, with + no Routeway string left in the file. +4. `defaultModel` on `alibaba-token-plan-intl` resolves to the new id, so a bare + config still routes. From f557f91731f93bdbbf01ec6da8ec805247b93415 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 11:59:25 +0900 Subject: [PATCH 7/9] fix(providers): classify NVIDIA NIM vision capability (#956) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nvidia registry entry declared no noVisionModels, so planVisionSidecar never fired for any NIM model and the catalog never advertised image input. A text-only NIM model therefore either received raw image parts it cannot read, or had attachments blocked client-side. That is issue #956. Two verified lists, both audited per-model against NVIDIA documentation on 2026-08-04 (evidence: devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md): noVisionModels 26 ids — text-only, sidecar describes their images modelInputModalities 16 ids — natively image-capable, native path, image input advertised explicitly PR #964 proposed ~64 text-only ids. Six are natively image-capable per NVIDIA's own docs — inkling, minimax-m3, kimi-k2.6, kimi-k2.5, step-3.7-flash and mistral-medium-3.5-128b — and listing those is a silent defect: the model can read the image, but the proxy substitutes another model's text description. No error, worse answers, extra cost. Issue #956's body carries two of the same errors. A further 32 of #964's ids have no current NVIDIA page and are dropped rather than assumed text-only. The vision list also needs explicit modelInputModalities. Removing an id from noVisionModels is not enough: the catalog advertises image input only for list members, so a natively-capable model would be published as text-only and the Codex app would block attachments before the native path could run. Unclassified ids are left alone deliberately. NIM publishes no modality metadata and shouldExposeRoutedModel filters only media-generation names, so embeddings, rerankers, guards and OCR endpoints reach this same path — an unknown id carries no signal separating them from a text-only chat model. Defaulting in either direction would be a claim the data does not support. Vision and reasoning stay independent: k2.5/k2.6 join the vision list while k2-thinking/k2-instruct stay text-only, and all four keep reasoning suppression. Red-green: reintroducing #964's kimi-k2.5 entry fails 2 guards; dropping the modalities map fails 3; removing noVisionModels entirely fails 8. Restored: 23 pass / 0 fail. --- src/providers/registry.ts | 71 +++++++++++++ tests/nvidia-nim-hardening.test.ts | 160 +++++++++++++++++++++++++++++ 2 files changed, 231 insertions(+) diff --git a/src/providers/registry.ts b/src/providers/registry.ts index 91d3ad07d..708f25d94 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -520,6 +520,72 @@ const NVIDIA_NIM_KIMI_MODELS = [ ...NVIDIA_NIM_KIMI_THINKING_MODELS, "moonshotai/kimi-k2-instruct", "moonshotai/kimi-k2-instruct-0905", ]; +/** + * 260804 issue #956: NIM publishes no input-modality metadata on `/v1/models`, so the + * registry is the only source of truth for which models can see images. + * + * Two lists, both verified per-model against NVIDIA documentation on 2026-08-04 + * (build.nvidia.com model pages and docs.api.nvidia.com/nim/reference/*). Evidence and + * the per-id audit: devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md. + * + * Read `noVisionModels` carefully — it lists models that CANNOT see images, which is + * what routes them through the proxy's vision sidecar (src/vision/index.ts) and makes the + * catalog advertise image input for them. Membership is wrong in BOTH directions: + * - a text-only model missing from it keeps issue #956 (images blocked or rejected); + * - a vision model wrongly IN it gets its image silently replaced by another model's + * text description — no error, worse answers, extra cost. + * + * A new NIM id must be classified DELIBERATELY against its NVIDIA page, never assumed + * from its name: `google/gemma-4-31b-it` carries no vision marker yet accepts images, + * `-vl` also appears on embedding/reranking models, and `google/codegemma-7b` is + * text-only while `google/codegemma-1.1-7b` has no current page at all. An unclassified + * id is intentionally left alone rather than defaulted, because NIM serves non-chat + * endpoints (embeddings, rerankers, guards, OCR) that reach the same code path. + */ +const NVIDIA_NIM_VISION_MODELS = [ + "meta/llama-3.2-11b-vision-instruct", "meta/llama-3.2-90b-vision-instruct", + "nvidia/llama-3.1-nemotron-nano-vl-8b-v1", "nvidia/nemotron-nano-12b-v2-vl", + "nvidia/nemotron-3-nano-omni-30b-a3b-reasoning", "nvidia/cosmos3-nano-reasoner", + "nvidia/ising-calibration-1.5-31b", "nvidia/ising-calibration-1-35b-a3b", + "google/gemma-4-31b-it", "google/diffusiongemma-26b-a4b-it", + "minimaxai/minimax-m3", "moonshotai/kimi-k2.6", "moonshotai/kimi-k2.5", + "stepfun-ai/step-3.7-flash", "thinkingmachines/inkling", + "mistralai/mistral-medium-3.5-128b", +]; +/** + * The catalog advertises image input only for `noVisionModels` members, so a natively + * vision-capable model would otherwise be published as text-only and the Codex app would + * block attachments before the native path ever runs. + */ +const NVIDIA_NIM_VISION_INPUT_MODALITIES: Record = Object.fromEntries( + NVIDIA_NIM_VISION_MODELS.map(id => [id, ["text", "image"]]), +); +/** + * Text-only NIM chat models — 26 ids, each carrying an explicit `Input Modalities: Text` + * (or equivalent) on its NVIDIA page. PR #964 proposed ~64; six of those are natively + * image-capable and live in NVIDIA_NIM_VISION_MODELS above, and 32 more had no current + * NVIDIA page and were dropped rather than assumed. + * + * kimi-k2-thinking and kimi-k2-instruct are text-only while k2.5/k2.6 are not — vision + * and reasoning are independent axes, so all four stay in NVIDIA_NIM_KIMI_MODELS for + * reasoning suppression regardless of which list they appear in here. + */ +const NVIDIA_NIM_NO_VISION_MODELS = [ + "deepseek-ai/deepseek-v4-flash", "deepseek-ai/deepseek-v4-pro", + "google/codegemma-7b", + "meta/llama-3.1-70b-instruct", "meta/llama-3.1-8b-instruct", + "meta/llama-3.2-1b-instruct", "meta/llama-3.2-3b-instruct", + "meta/llama-3.3-70b-instruct", "meta/llama2-70b", + "mistralai/mistral-7b-instruct-v0.3", "mistralai/mistral-nemotron", + "moonshotai/kimi-k2-thinking", "moonshotai/kimi-k2-instruct", + "nvidia/llama-3.1-nemotron-nano-8b-v1", "nvidia/llama-3.1-nemotron-ultra-253b-v1", + "nvidia/llama-3.3-nemotron-super-49b-v1", "nvidia/llama-3.3-nemotron-super-49b-v1.5", + "nvidia/nemotron-3-nano-30b-a3b", "nvidia/nemotron-3-super-120b-a12b", + "nvidia/nemotron-3-ultra-550b-a55b", "nvidia/nemotron-mini-4b-instruct", + "nvidia/nvidia-nemotron-nano-9b-v2", + "openai/gpt-oss-120b", "openai/gpt-oss-20b", + "poolside/laguna-xs-2.1", "z-ai/glm-5.2", +]; const KIMI_CODING_MODEL_CONTEXT_WINDOWS: Record = Object.fromEntries( KIMI_CODING_MODELS.map(id => [id, id === "k3[1m]" ? KIMI_K3_1M_CONTEXT_WINDOW : KIMI_K3_STANDARD_CONTEXT_WINDOW]), ); @@ -1235,6 +1301,11 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ // Free pricing, but an API key is still required (free key from build.nvidia.com). freeTier: true, parallelToolCalls: false, + // 260804 issue #956: NIM exposes no input modalities, so vision capability is + // classified here. Both lists are verified per-model; unlisted ids stay unclassified + // by design (see the comment on NVIDIA_NIM_VISION_MODELS). + noVisionModels: NVIDIA_NIM_NO_VISION_MODELS, + modelInputModalities: NVIDIA_NIM_VISION_INPUT_MODALITIES, noReasoningModels: NVIDIA_NIM_KIMI_MODELS, modelReasoningEfforts: Object.fromEntries(NVIDIA_NIM_KIMI_MODELS.map(id => [id, []])), preserveReasoningContentModels: NVIDIA_NIM_KIMI_THINKING_MODELS, diff --git a/tests/nvidia-nim-hardening.test.ts b/tests/nvidia-nim-hardening.test.ts index c757617ae..0952948c0 100644 --- a/tests/nvidia-nim-hardening.test.ts +++ b/tests/nvidia-nim-hardening.test.ts @@ -1,11 +1,17 @@ // 260715 issue #126: NVIDIA NIM hardening — parallel_tool_calls opt-out, kimi // reasoning_effort suppression, and openai-chat formatErrorBody detail surfacing. // Plan/evidence: devlog/_plan/260715_issue126_nim_kimi. +// 260804 issue #956: NIM vision classification — text-only models activate the sidecar, +// natively image-capable models do not. Plan/evidence: +// devlog/_plan/260804_stack7_service_vision (010 design, 011 per-id audit). import { describe, expect, test } from "bun:test"; import { createOpenAIChatAdapter, formatOpenAIChatErrorBody } from "../src/adapters/openai-chat"; import { applyProviderConfigHints, normalizeRoutedCatalogEntry } from "../src/codex/catalog"; +import { PROVIDER_REGISTRY } from "../src/providers/registry"; +import { parseRequest } from "../src/responses/parser"; import { routeModel } from "../src/router"; import type { OcxConfig, OcxParsedRequest, OcxTool } from "../src/types"; +import { planVisionSidecar } from "../src/vision"; const tools: OcxTool[] = [{ name: "shell", description: "run", parameters: { type: "object" } }]; @@ -88,7 +94,161 @@ describe("nvidia NIM registry hardening (issue #126)", () => { }); }); +/** + * 260804 issue #956. The reported defect: the `nvidia` entry declared no + * `noVisionModels`, so `planVisionSidecar` never fired and the catalog never advertised + * image input for text-only NIM models. + * + * PR #964 proposed a ~64-id text-only list. Six of its entries are natively + * image-capable per NVIDIA's own documentation, and listing those is a SILENT defect: + * the model could read the image, but the proxy substitutes another model's text + * description. These tests pin both directions. + */ +describe("nvidia NIM vision classification (issue #956)", () => { + const nvidia = () => PROVIDER_REGISTRY.find(entry => entry.id === "nvidia")!; + + const openAiSidecar = { + providerName: "openai" as const, + provider: { adapter: "openai-responses", baseUrl: "https://chatgpt.test/v1", authMode: "forward" as const }, + accountMode: "direct" as const, + authContext: { kind: "main" as const, accountId: null }, + headers: new Headers({ authorization: "Bearer chatgpt" }), + }; + + function withImage(modelId: string) { + return parseRequest({ + model: `nvidia/${modelId}`, + input: [{ + type: "message", + role: "user", + content: [ + { type: "input_text", text: "What is in this screenshot?" }, + { type: "input_image", image_url: "data:image/png;base64,aGVsbG8=" }, + ], + }], + }); + } + + function plan(modelId: string) { + const route = routeModel(nvidiaConfig(), `nvidia/${modelId}`); + return planVisionSidecar(nvidiaConfig(), route.provider, route.modelId, withImage(modelId), openAiSidecar); + } + + // The six ids PR #964 classified backwards. Each is documented by NVIDIA as accepting + // image input, so the sidecar must NOT intercept it. Ablate by adding any of them to + // NVIDIA_NIM_NO_VISION_MODELS and this goes red. + const REVERSED_IN_964 = [ + "thinkingmachines/inkling", + "minimaxai/minimax-m3", + "moonshotai/kimi-k2.6", + "moonshotai/kimi-k2.5", + "stepfun-ai/step-3.7-flash", + "mistralai/mistral-medium-3.5-128b", + ]; + + test("natively image-capable NIM models never route through the sidecar", () => { + for (const id of [...REVERSED_IN_964, "meta/llama-3.2-11b-vision-instruct", "meta/llama-3.2-90b-vision-instruct"]) { + expect(nvidia().noVisionModels).not.toContain(id); + expect(plan(id)).toBeUndefined(); + } + }); + + test("verified text-only NIM models do route through the sidecar", () => { + for (const id of [ + "deepseek-ai/deepseek-v4-flash", + "z-ai/glm-5.2", + "nvidia/nemotron-3-ultra-550b-a55b", + "openai/gpt-oss-120b", + "moonshotai/kimi-k2-thinking", + ]) { + expect(nvidia().noVisionModels).toContain(id); + expect(plan(id)).toMatchObject({ backend: "openai" }); + } + }); + + test("a text-only model without an image needs no sidecar", () => { + const config = nvidiaConfig(); + const route = routeModel(config, "nvidia/deepseek-ai/deepseek-v4-flash"); + const noImage = parseRequest({ + model: "nvidia/deepseek-ai/deepseek-v4-flash", + input: [{ type: "message", role: "user", content: [{ type: "input_text", text: "hi" }] }], + }); + expect(planVisionSidecar(config, route.provider, route.modelId, noImage, openAiSidecar)).toBeUndefined(); + }); + + // The honest boundary. NIM publishes no modality metadata and serves non-chat + // endpoints (embeddings, rerankers, guards, OCR) that reach this same path, so an + // unclassified id is deliberately left alone rather than defaulted in either + // direction. If a future change defaults unknown ids, this test forces that to be a + // conscious decision instead of a side effect. + test("unclassified NIM ids stay unclassified, including non-chat endpoints", () => { + for (const id of [ + "nvidia/nv-embedqa-e5-v5", + "nvidia/nemotron-ocr-v2", + "nvidia/llama-nemotron-rerank-1b-v2", + "brandnew/model-nobody-classified", + ]) { + expect(nvidia().noVisionModels).not.toContain(id); + expect(nvidia().modelInputModalities?.[id]).toBeUndefined(); + expect(plan(id)).toBeUndefined(); + } + }); + + // Both classes must advertise image input, by different mechanisms: text-only models + // via the noVisionModels hint (the sidecar describes the image), vision models via + // explicit modelInputModalities. Without the latter the Codex app blocks attachments + // client-side and the native path never runs. + test("the catalog advertises image input for both classes", () => { + const config = nvidiaConfig(); + for (const id of ["deepseek-ai/deepseek-v4-flash", "z-ai/glm-5.2"]) { + const route = routeModel(config, `nvidia/${id}`); + const hinted = applyProviderConfigHints("nvidia", route.provider, { id, provider: "nvidia" }); + expect(hinted.inputModalities).toContain("image"); + } + for (const id of REVERSED_IN_964) { + const route = routeModel(config, `nvidia/${id}`); + const hinted = applyProviderConfigHints("nvidia", route.provider, { id, provider: "nvidia" }); + expect(hinted.inputModalities).toEqual(["text", "image"]); + } + }); + + test("a bare persisted nvidia config inherits the classification from the registry", () => { + // The #956 reporter's exact config shape, which today needs a manual workaround. + const route = routeModel(nvidiaConfig(), "nvidia/deepseek-ai/deepseek-v4-flash"); + expect(route.provider.noVisionModels).toContain("deepseek-ai/deepseek-v4-flash"); + expect(route.provider.modelInputModalities?.["moonshotai/kimi-k2.6"]).toEqual(["text", "image"]); + }); + + test("a user's own noVisionModels entries are preserved alongside the registry's", () => { + // mergeStringArray unions registry and user arrays, so a user adds but cannot remove. + const config = nvidiaConfig(); + config.providers.nvidia!.noVisionModels = ["some/private-endpoint"]; + const route = routeModel(config, "nvidia/some/private-endpoint"); + expect(route.provider.noVisionModels).toContain("some/private-endpoint"); + expect(route.provider.noVisionModels).toContain("z-ai/glm-5.2"); + }); + + test("the two classifications cannot overlap", () => { + const entry = nvidia(); + const visionIds = Object.keys(entry.modelInputModalities ?? {}); + expect(entry.noVisionModels?.filter(id => visionIds.includes(id))).toEqual([]); + }); + + test("kimi vision and reasoning classifications stay independent", () => { + const entry = nvidia(); + // k2.5/k2.6 see images; k2-thinking/k2-instruct do not. All four keep reasoning + // suppression, because that is a separate axis. + for (const id of ["moonshotai/kimi-k2.6", "moonshotai/kimi-k2.5", "moonshotai/kimi-k2-thinking", "moonshotai/kimi-k2-instruct"]) { + expect(entry.noReasoningModels).toContain(id); + expect(entry.modelReasoningEfforts?.[id]).toEqual([]); + } + expect(entry.modelInputModalities?.["moonshotai/kimi-k2.5"]).toEqual(["text", "image"]); + expect(entry.noVisionModels).toContain("moonshotai/kimi-k2-thinking"); + }); +}); + describe("formatOpenAIChatErrorBody (web-search sidecar detail surfacing)", () => { + test("OpenAI error object shape", () => { expect(formatOpenAIChatErrorBody(400, new Headers(), '{"error":{"message":"This model only supports single tool-calls at once!"}}')) .toBe("This model only supports single tool-calls at once!"); From e9c0bb3d1e3179a72e021f77935f146781e0cf88 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 12:09:20 +0900 Subject: [PATCH 8/9] fix(update): repair the background service instead of re-registering it (#970) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ocx update` stops the proxy before replacing package files, then brought the service back with `ocx service install`. The Windows scheduler installer always reaches `schtasks /create`, which requires elevation the updater does not have, so an ordinary non-elevated update stopped a working proxy and could not restore its managed service. serviceReinstallArgs() now returns ["service", "repair"], which rewrites the wrapper assets and restarts the EXISTING registration without /create. The export name is kept for out-of-module callers; serviceInstallArgs() is split out for the paths that genuinely need to register. Safety of the substitution: repairService() throws when the service is not installed, and the update path runs after `ocx stop` — but stop never deregisters on any platform. macOS unloads the plist, Windows calls /end, Linux calls systemctl stop; deletion lives only in the uninstall paths. Verified across all three (evidence: devlog 020). Two things a straight argv change would have missed: The Windows GUI worker skipped the refresh entirely (update/job.ts) because its own comment said /create would UAC-fail. That reason does not survive repair, so the skip is narrowed to callers still passing install argv — otherwise the dashboard-triggered update, the most common Windows path, keeps the bug while the CLI gets fixed. bin/ocx.mjs infers 'a service manages this proxy' from service-state.json existing, which can be stale. Repair correctly refuses that case, but its thrown Error is indistinguishable from any other failure there (plain Error, inherited stdio, generic exit status), so message-matching was unimplementable and a blanket install-on-failure would resurrect the elevation prompt. It now reads startup.serviceInstalled from the `status --json` subprocess it already spawns — the file is plain Node ESM and cannot import diagnoseService() directly. Advice strings that fire only for an INSTALLED service now say repair: cli/status, winsw missing-binary, stale baked paths, stale scheduler assets, the launchd older-plist and not-loaded hints. First-install and missing-unit guidance stays install. Red-green: restoring the unconditional Windows skip fails the new guard. Six existing tests pinned the install argv and were updated with reasons. 245 pass / 0 fail across the service, update, winsw, doctor, status, startup and Windows-deploy suites. --- bin/ocx.mjs | 68 +++++++++++++++---- src/cli/status.ts | 2 +- src/lib/winsw.ts | 2 +- src/service.ts | 40 +++++++---- src/update/index.ts | 21 +++--- src/update/job.ts | 22 ++++-- tests/service.test.ts | 9 ++- tests/update-job.test.ts | 56 ++++++++++++++- tests/update-stop-first.test.ts | 16 +++-- .../windows-deploy-close-regressions.test.ts | 6 +- tests/winsw.test.ts | 15 ++-- 11 files changed, 195 insertions(+), 62 deletions(-) diff --git a/bin/ocx.mjs b/bin/ocx.mjs index c4fd07680..b89ed75fd 100755 --- a/bin/ocx.mjs +++ b/bin/ocx.mjs @@ -134,20 +134,53 @@ function runNpmSelfUpdate() { } // Remember whether a background service manages the proxy BEFORE stopping — `ocx stop` - // unloads it permanently, so a successful update must reinstall it afterwards. + // unloads it, so a successful update must refresh and restart it afterwards. const serviceStatePath = join(configDir(), "service-state.json"); const serviceWasInstalled = existsSync(serviceStatePath); const trayBeforeUpdate = planWindowsTrayUpdate( process.platform === "win32" ? trayInstallState() : { installed: false, running: false }, ); - /** Read the backend from service-state.json so the update reinstalls the same one. */ - function serviceReinstallArgs() { + /** + * Refresh the existing service without re-registering it. `service repair` discovers + * the installed backend itself and, on Windows scheduler installs, rewrites the wrapper + * assets and restarts the existing task without `schtasks /create` — the elevation a + * non-admin `ocx update` does not have. + */ + function serviceRefreshArgs() { + return [launcher, "service", "repair"]; + } + /** Register from scratch, preserving the recorded backend. Only for a genuinely absent service. */ + function serviceInstallArgs() { try { const state = JSON.parse(readFileSync(serviceStatePath, "utf8")); if (state.backend === "native") return [launcher, "service", "install", "--native"]; } catch { /* missing or corrupt — fall through to default */ } return [launcher, "service", "install"]; } + /** + * Structured "is a service actually registered?" answer. + * + * This file is plain Node ESM and cannot import `diagnoseService()` from the + * TypeScript runtime, so it asks the freshly-installed launcher — which runs that + * diagnostic under Bun — and reads `startup.serviceInstalled`. + * + * Returns `null` when the probe itself could not answer, which callers must treat as + * "unknown" rather than "absent": failing closed here means NOT re-registering. + */ + function readServiceInstalledFromStatus(launcherPath) { + try { + const st = spawnSync(process.execPath, [launcherPath, "status", "--json"], { + encoding: "utf8", + timeout: 20_000, + windowsHide: true, + }); + if (st.status !== 0 || typeof st.stdout !== "string" || !st.stdout.trim()) return null; + const installed = JSON.parse(st.stdout)?.startup?.serviceInstalled; + return typeof installed === "boolean" ? installed : null; + } catch { + return null; + } + } // Capture listen target before stop clears runtime-port.json (mirrors GUI/CLI update worker). // Do not treat a live runtime port of 10100 as "missing" — track whether the read succeeded. @@ -241,15 +274,26 @@ function runNpmSelfUpdate() { if (trayBeforeUpdate.restoreOnFailure) runTrayLifecycle(launcher, "start"); } } - // The stop above unloaded any managed service; reinstall via the freshly-installed + // The stop above unloaded any managed service; refresh via the freshly-installed // launcher so the new files write the baked paths and the service restarts. if (serviceWasInstalled) { - console.log("Reinstalling the background service with the updated files..."); + console.log("Refreshing the background service with the updated files..."); const prevBake = process.env.OCX_BAKE_PORT; process.env.OCX_BAKE_PORT = String(bakePort); try { - const svcArgs = serviceReinstallArgs(); - const svc = spawnSync(process.execPath, svcArgs, { stdio: "inherit", windowsHide: true }); + let svc = spawnSync(process.execPath, serviceRefreshArgs(), { stdio: "inherit", windowsHide: true }); + // `serviceWasInstalled` is inferred from service-state.json alone, which can be + // STALE — present while the registration is gone. Repair refuses that case by + // design, and its thrown Error is indistinguishable from any other failure at + // this layer (plain Error, inherited stdio, generic exit status). So ask for + // structured state instead of parsing the failure: install only when the + // diagnostic says the service is genuinely absent. Installing after ANY repair + // failure would resurrect the elevation prompt this change exists to avoid, and + // could re-register a service the user just uninstalled. + if (svc.status !== 0 && readServiceInstalledFromStatus(launcher) === false) { + console.log("No registered service found — installing it instead."); + svc = spawnSync(process.execPath, serviceInstallArgs(), { stdio: "inherit", windowsHide: true }); + } let needDirectStart = svc.status !== 0; if (!needDirectStart) { // Exit 0 can still leave stale/missing assets that never bring the proxy @@ -274,17 +318,15 @@ function runNpmSelfUpdate() { } } if (needDirectStart) { - // On Windows, schtasks /create requires elevation. The launcher inherits the - // user's (non-admin) token, so the service reinstall can fail with access - // denied — or exit 0 while leaving a non-viable manager. Fall back to a - // direct detached proxy start so the update never leaves the user without - // a running proxy. + // A repair needs no elevation, but it can still fail — or exit 0 while leaving + // a non-viable manager. Fall back to a direct detached proxy start so the + // update never leaves the user without a running proxy. console.warn( svc.status === 0 ? "opencodex: service refresh left a non-viable manager — starting the proxy directly instead." : "opencodex: service refresh failed — starting the proxy directly instead.", ); - console.warn(" Run 'ocx service install' as administrator to refresh the background service."); + console.warn(" Run 'ocx service repair' to see why the background service could not restart."); const env = { ...process.env }; delete env.OCX_SERVICE; const child = spawn(process.execPath, [launcher, "start", "--port", String(bakePort)], { diff --git a/src/cli/status.ts b/src/cli/status.ts index 7e0ba0f8e..3d0f21086 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -173,7 +173,7 @@ export async function collectStatus(): Promise { // either way. `live` was already identity-probed a few lines above, so cross-check // rather than print registration as if it were service. const serviceSummary = service.installed && !live - ? `${service.summary} — registered but NOT serving; see ${serviceLogPath()} and re-run 'ocx service install'` + ? `${service.summary} — registered but NOT serving; see ${serviceLogPath()} and re-run 'ocx service repair'` : service.summary; const codexShim = diagnoseCodexShim(); const codexShimSummary = codexShim.summary; diff --git a/src/lib/winsw.ts b/src/lib/winsw.ts index b2302e538..d84854719 100644 --- a/src/lib/winsw.ts +++ b/src/lib/winsw.ts @@ -368,7 +368,7 @@ export function winswStatusSummary(): string { if (status === "nonexistent") { // A stale SCM service can outlive a deleted exe; surface the repair path. return existsSync(winswXmlPath()) && !existsSync(winswExePath()) - ? "native assets present but WinSW binary missing — run 'ocx service install --native' to repair" + ? "native assets present but WinSW binary missing — run 'ocx service repair'" : ""; } return `native (WinSW ${WINSW_VERSION}): ${status}`; diff --git a/src/service.ts b/src/service.ts index 69ef03767..2c4cd1608 100644 --- a/src/service.ts +++ b/src/service.ts @@ -174,13 +174,28 @@ function readServiceInstallState(): ServiceInstallState | null { return null; } -/** Single accessor for update/reinstall code — v1/legacy state maps to scheduler. */ +/** Single accessor for backend-sensitive service code — v1/legacy state maps to scheduler. */ export function readServiceBackend(): ServiceBackend { return readServiceInstallState()?.backend === "native" ? "native" : "scheduler"; } -/** The `ocx` argv that reinstalls the currently-chosen service backend (update paths). */ +/** + * The `ocx` argv that refreshes an already-installed service after an update. + * + * `repair` discovers the installed backend itself and, on Windows scheduler installs, + * rewrites the wrapper assets and restarts the existing task WITHOUT `schtasks /create` + * (see repairService below). `install` always reaches `/create`, which requires + * elevation — so an ordinary non-elevated `ocx update` used to stop a working proxy and + * then fail to bring its service back. + * + * The historical export name is kept for callers outside this module. + */ export function serviceReinstallArgs(): string[] { + return ["service", "repair"]; +} + +/** The `ocx` argv that registers a service from scratch, preserving the chosen backend. */ +export function serviceInstallArgs(): string[] { return readServiceBackend() === "native" ? ["service", "install", "--native"] : ["service", "install"]; } @@ -498,17 +513,14 @@ async function reportServiceServing( } /** - * The reinstall command for the CURRENTLY INSTALLED backend. + * The command that repairs the CURRENTLY INSTALLED backend without re-registering it. * - * Plain `ocx service install` on a native/WinSW install runs installWindows's - * transactional backend switch, which tears down WinSW and replaces it with the Task - * Scheduler backend. Advising it in a repair hint would silently change the user's - * backend, so the hint has to carry `--native` when that is what is installed. + * `ocx service repair` reads the recorded backend itself, so it cannot silently switch a + * WinSW install to Task Scheduler the way a plain `ocx service install` would, and on + * Windows it needs no elevation because it never calls `schtasks /create`. */ function serviceRepairCommand(): string { - return process.platform === "win32" && readServiceBackend() === "native" - ? "ocx service install --native" - : "ocx service install"; + return "ocx service repair"; } function systemdQuote(value: string): string { @@ -1660,8 +1672,8 @@ export function startLaunchd(deps: { throw new Error( `launchctl could not load ${p}: ${loaded.stderr || "load reported failure"}\n` + (live.loaded - ? `launchd is running an OLDER plist. Fix:\n launchctl bootout ${launchdGuiDomain()}/${LABEL}\n ocx service install` - : "The job is not loaded. Run 'ocx service install' to re-register it."), + ? `launchd is running an OLDER plist. Fix:\n launchctl bootout ${launchdGuiDomain()}/${LABEL}\n ocx service repair` + : "The job is not loaded. Run 'ocx service repair' to reload it."), ); } function stopLaunchd(): void { try { sh(`launchctl unload "${plistPath()}"`); } catch { /* not loaded */ } } @@ -1924,7 +1936,7 @@ export function bakedServicePathsDiagnostic(): string | null { if (!state?.bunPath || !state?.cliPath) return null; const missing = [state.bunPath, state.cliPath].filter(path => !existsSync(path)); if (missing.length === 0) return null; - return `STALE baked paths (missing: ${missing.join(", ")}) — run 'ocx service install' to re-bake`; + return `STALE baked paths (missing: ${missing.join(", ")}) — run 'ocx service repair' to re-bake`; } function serviceDiagnosticsSummary(): string { @@ -2341,7 +2353,7 @@ export function deriveWindowsServiceDiagnostic(inputs: WindowsServiceDiagnosticI const detail = conflict ? "CONFLICT: Task Scheduler and native WinSW are both present — run 'ocx service uninstall' then reinstall one" : stale - ? "stale or missing service assets — run 'ocx service install' to repair" + ? "stale or missing service assets — run 'ocx service repair'" : schedulerInstalled ? schedulerEnabled ? "Task Scheduler enabled" : "Task Scheduler disabled" : nativeInstalled diff --git a/src/update/index.ts b/src/update/index.ts index 0c38fcbd8..e03f096f9 100644 --- a/src/update/index.ts +++ b/src/update/index.ts @@ -186,7 +186,7 @@ export async function runUpdate(): Promise { } // Remember whether a background service manages the proxy BEFORE stopping — `ocx stop` - // unloads it permanently, so a successful update must reinstall/restart it afterwards. + // unloads it, so a successful update must repair/restart it afterwards. let serviceWasInstalled = false; try { const { isServiceInstalled } = await import("../service"); @@ -295,11 +295,11 @@ export async function runUpdate(): Promise { if (trayWasRunning) spawnSync(process.execPath, [process.argv[1], "tray", "start"], { stdio: "ignore", windowsHide: true }); } } - // The stop above unloaded any managed service; reinstall it with the NEW files + // The stop above unloaded any managed service; repair it with the NEW files // (spawn the fresh cli.ts so updated code writes the baked paths) so a // launchd/schtasks/systemd user isn't left with the background proxy down. if (serviceWasInstalled) { - console.log("🔁 Reinstalling the background service with the updated files..."); + console.log("🔁 Refreshing the background service with the updated files..."); const { serviceReinstallArgs } = await import("../service"); const { reclaimListenPort } = await import("../server/port-reclaim"); const freed = await reclaimListenPort(capturedListen.port, capturedListen.hostname, { @@ -310,7 +310,7 @@ export async function runUpdate(): Promise { onlyKillPids: capturedListen.oldPid != null ? [capturedListen.oldPid] : [], }); if (!freed) { - console.warn(`⚠️ Port ${capturedListen.port} still busy after 30s; reinstalling service with pinned --port ${capturedListen.port} anyway (refusing to hop).`); + console.warn(`⚠️ Port ${capturedListen.port} still busy after 30s; repairing service with pinned --port ${capturedListen.port} anyway (refusing to hop).`); } const prevBake = process.env.OCX_BAKE_PORT; process.env.OCX_BAKE_PORT = String(capturedListen.port); @@ -333,9 +333,8 @@ export async function runUpdate(): Promise { } } if (!serviceRefreshed || !serviceViable) { - // On Windows, schtasks /create requires elevation. The CLI inherits the - // user's (non-admin) token, so the service reinstall can fail with access - // denied — or exit 0 while leaving stale/missing assets that never start + // A repair needs no elevation (it never calls `schtasks /create`), but it can + // still fail — or exit 0 while leaving stale/missing assets that never start // the proxy. Fall back to a direct detached proxy start so the update // never leaves the user without a running proxy — but only when the port is free. if (!freed) { @@ -345,8 +344,8 @@ export async function runUpdate(): Promise { : "⚠️ Service refresh failed and the captured port is still busy; not starting on another port.", ); console.warn(process.platform === "win32" - ? ` Run 'ocx service install' as administrator, then 'ocx start --port ${capturedListen.port}'.` - : ` Run 'ocx service install' to see the reason, then 'ocx start --port ${capturedListen.port}'.`); + ? ` Run 'ocx service repair', then 'ocx start --port ${capturedListen.port}'.` + : ` Run 'ocx service repair' to see the reason, then 'ocx start --port ${capturedListen.port}'.`); } else { console.warn( serviceRefreshed @@ -357,8 +356,8 @@ export async function runUpdate(): Promise { // reasons `ocx service install` reports directly (since it now verifies // the service actually serves). console.warn(process.platform === "win32" - ? " Run 'ocx service install' as administrator to refresh the background service." - : " Run 'ocx service install' to refresh the background service and see why it failed."); + ? " Run 'ocx service repair' to refresh the background service." + : " Run 'ocx service repair' to refresh the background service and see why it failed."); const env = { ...process.env }; delete env.OCX_SERVICE; const child = spawn(process.execPath, [process.argv[1], "start", "--port", String(capturedListen.port)], { diff --git a/src/update/job.ts b/src/update/job.ts index 6dc0466bb..f0684cd19 100644 --- a/src/update/job.ts +++ b/src/update/job.ts @@ -300,7 +300,9 @@ export function restartCommand( const startArgs = pinPort ? [launcher, "start", "--port", String(Math.trunc(port))] : [launcher, "start"]; - const svcArgs = serviceInstalled ? [launcher, ...(serviceArgs ?? ["service", "install"])] : startArgs; + // Default to the non-registering refresh: an update path reaching here has an already + // installed service, and `install` would demand elevation on Windows scheduler backends. + const svcArgs = serviceInstalled ? [launcher, ...(serviceArgs ?? ["service", "repair"])] : startArgs; if (installer === "npm") { const bin = nodeBin(); const args = svcArgs; @@ -648,6 +650,8 @@ export interface RestartIo { serviceHealthTimeoutMs?: number; sleepMs?: (ms: number) => Promise; now?: () => number; + /** Test seam — defaults to process.platform so the Windows-only branch is reachable off Windows. */ + platform?: NodeJS.Platform; /** Service-mode install/reinstall command (defaults to spawnSync via runLoggedCommand). */ runService?: ( job: UpdateJobState, @@ -782,11 +786,17 @@ async function restartAfterUpdate( const preServiceAllow = reclaimKillAllowlist(); const freed = await waitFn(port, hostname, reclaimOptsFor(preServiceAllow)); let skipServiceInstall = false; - // Windows GUI update worker sets OCX_SERVICE=1 and is never elevated. - // `schtasks /create` will UAC-fail and can race the subsequent direct start. - // Keep systemd/launchd reinstall on non-Windows supervisors. - if (process.platform === "win32" && process.env.OCX_SERVICE === "1") { - updateJob(job, {}, "Skipping service reinstall from the non-elevated update worker; falling back to a direct proxy start."); + // This skip existed because the refresh ran `ocx service install`, whose Windows + // scheduler path always reaches `schtasks /create` — elevation the GUI update worker + // (OCX_SERVICE=1) never has. `service repair` rewrites the wrapper assets and + // restarts the EXISTING task with no `/create`, so the reason no longer applies and + // skipping would leave the dashboard-triggered update — the most common Windows + // path — with a stale service it could have refreshed. + // + // Only a caller that still passes install argv keeps the old behavior. + const refreshRegisters = (svcArgs ?? []).includes("install"); + if ((io.platform ?? process.platform) === "win32" && process.env.OCX_SERVICE === "1" && refreshRegisters) { + updateJob(job, {}, "Skipping service re-registration from the non-elevated update worker; falling back to a direct proxy start."); skipServiceInstall = true; } if (!freed && !skipServiceInstall) { diff --git a/tests/service.test.ts b/tests/service.test.ts index a2ebc5838..32d19bb80 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -1099,11 +1099,13 @@ describe("launchctl load verification", () => { })).toThrow(/bootout/); }); - test("throws with the install hint when no job is loaded", () => { + test("throws with the repair hint when no job is loaded", () => { + // The plist exists (this is an installed service) — reloading it is `repair`, + // not a re-registration. expect(() => startLaunchd({ launchctl: failedLoad, matches: () => ({ loaded: false, matchesPlist: false }), - })).toThrow(/service install/); + })).toThrow(/service repair/); }); }); }); @@ -1247,7 +1249,8 @@ describe("service serving confirmation", () => { matchesPlist: () => ({ loaded: true, matchesPlist: true }), }); expect(out).toContain("no proxy is answering on port 10100"); - expect(out).toContain("ocx service install"); + // Registered but not serving: repair refreshes it without demanding elevation. + expect(out).toContain("ocx service repair"); expect(out).toContain("ocx start"); }); diff --git a/tests/update-job.test.ts b/tests/update-job.test.ts index d6492a083..7adcc513f 100644 --- a/tests/update-job.test.ts +++ b/tests/update-job.test.ts @@ -115,7 +115,7 @@ describe("GUI update execution decisions", () => { test("restart command separates service and direct proxy modes", () => { expect(restartCommand(true, "npm", "/pkg/bin/ocx.mjs")).toMatchObject({ mode: "service", - args: ["/pkg/bin/ocx.mjs", "service", "install"], + args: ["/pkg/bin/ocx.mjs", "service", "repair"], }); expect(restartCommand(false, "npm", "/pkg/bin/ocx.mjs")).toMatchObject({ mode: "proxy", @@ -128,9 +128,9 @@ describe("GUI update execution decisions", () => { expect(proxy.mode).toBe("proxy"); expect(proxy.args).toEqual(["/pkg/bin/ocx.mjs", "start", "--port", "10100"]); expect(proxy.display).toContain("start --port 10100"); - // Service reinstall stays install-only at the argv level; wrappers bake --port via OCX_BAKE_PORT. + // The service refresh takes no --port at the argv level; wrappers bake it via OCX_BAKE_PORT. expect(restartCommand(true, "npm", "/pkg/bin/ocx.mjs", 10100).args).toEqual([ - "/pkg/bin/ocx.mjs", "service", "install", + "/pkg/bin/ocx.mjs", "service", "repair", ]); }); @@ -385,6 +385,56 @@ describe("GUI update execution decisions", () => { } }); + // 260804 #970: the Windows GUI update worker (OCX_SERVICE=1, never elevated) used to + // skip the service refresh entirely, because it ran `service install` whose scheduler + // path always reaches `schtasks /create`. `repair` never calls /create, so the skip's + // reason is gone and the dashboard-triggered update — the most common Windows path — + // must actually refresh the service. Ablate by restoring the unconditional skip: + // runService is then never called and this goes red. + test("a non-elevated Windows update worker repairs the service instead of skipping it", async () => { + const ranService: string[][] = []; + const spawned: Array<{ port: number }> = []; + const job: UpdateJobState = { + id: "svc-win-repair", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.42", + latestVersion: "2.7.43", + channel: "latest", + installer: "npm", + restart: true, + command: "", + log: [], + }; + writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + const prevService = process.env.OCX_SERVICE; + process.env.OCX_SERVICE = "1"; + try { + await restartAfterUpdateForTests(job, { port: 19998, hostname: "127.0.0.1" }, { + platform: "win32", + serviceInstalledFn: () => true, + serviceViableFn: () => true, + waitForPort: async () => true, + probeProxy: async () => true, + runService: (_j, _bin, args) => { + ranService.push(args); + return { status: 0 }; + }, + spawnStart: (_job, _installer, port) => { + spawned.push({ port: port ?? 0 }); + }, + }); + // The refresh ran, and it ran the non-registering subcommand. + expect(ranService.length).toBe(1); + expect(ranService[0]).toContain("repair"); + expect(ranService[0]).not.toContain("install"); + } finally { + if (prevService === undefined) delete process.env.OCX_SERVICE; + else process.env.OCX_SERVICE = prevService; + } + }); + test("service reinstall exit 0 with non-viable assets falls back to direct start", async () => { const spawned: Array<{ port: number }> = []; const job: UpdateJobState = { diff --git a/tests/update-stop-first.test.ts b/tests/update-stop-first.test.ts index e1c902a08..4032328d8 100644 --- a/tests/update-stop-first.test.ts +++ b/tests/update-stop-first.test.ts @@ -55,15 +55,21 @@ describe("update stops the running proxy before replacing files", () => { expect(launcherSource).not.toContain('"npm.cmd"'); }); - test("both paths abort when the stop fails, and reinstall a managed service after success", () => { + test("both paths abort when the stop fails, and REPAIR a managed service after success", () => { expect(updateSource).toContain("aborting the update"); - // The update path now uses serviceReinstallArgs() to preserve the chosen backend. + // 260804 #970: the refresh must not re-register. `install` reaches `schtasks /create` + // on Windows scheduler backends, which a non-elevated updater cannot run — it would + // stop a working proxy and then fail to bring its service back. expect(updateSource).toContain("serviceReinstallArgs()"); expect(launcherSource).toContain("aborting the update"); - // The launcher reads service-state.json to preserve the backend choice on reinstall. - expect(launcherSource).toContain("serviceReinstallArgs"); - // The launcher reads the state path for both service-installed detection and backend choice. + expect(launcherSource).toContain('"service", "repair"'); + // The launcher still reads service-state.json for service-installed detection, and + // for the backend choice on the genuinely-absent install fallback. expect(launcherSource).toContain('"service-state.json"'); + // That marker can be STALE, so the fallback asks for structured state rather than + // parsing a failure message; bin/ocx.mjs is plain Node and cannot import + // diagnoseService(), so it reads startup.serviceInstalled from `status --json`. + expect(launcherSource).toContain("startup?.serviceInstalled"); expect(updateSource).toContain("OCX_BAKE_PORT"); expect(launcherSource).toContain("OCX_BAKE_PORT"); // Live runtime port 10100 must not be discarded as a missing-port sentinel. diff --git a/tests/windows-deploy-close-regressions.test.ts b/tests/windows-deploy-close-regressions.test.ts index b914562ef..05287d2de 100644 --- a/tests/windows-deploy-close-regressions.test.ts +++ b/tests/windows-deploy-close-regressions.test.ts @@ -42,7 +42,11 @@ describe("update-job restart avoids the shell-less .cmd EINVAL (Windows, bun/sou expect(src).toContain("spawnWorkerFn: spawnGuiUpdateWorker"); // Foreign listeners must stay fail-closed; npm rename is covered by ocx identity. expect(src).not.toContain("killAnyListenPidOnPort"); - expect(src).toContain('process.platform === "win32" && process.env.OCX_SERVICE === "1"'); + // 260804 #970: this skip is now conditional on the refresh actually re-registering. + // `service repair` needs no elevation, so skipping it would leave the dashboard + // update — the common Windows path — with a stale service it could have refreshed. + expect(src).toContain('process.env.OCX_SERVICE === "1" && refreshRegisters'); + expect(src).toContain('const refreshRegisters = (svcArgs ?? []).includes("install")'); // Native WinSW installs must stop via stopWinswService, not Task Scheduler /end only. expect(src).toContain("readServiceBackend"); expect(src).toContain("stopWinswService"); diff --git a/tests/winsw.test.ts b/tests/winsw.test.ts index 972460a6e..40fb435e0 100644 --- a/tests/winsw.test.ts +++ b/tests/winsw.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { buildWinswXml, ensureWinswBinary, parseWinswStatus, probeScmRegistration, sha256Hex, installWinswService, statusWinswRaw, WINSW_SHA256, WINSW_SERVICE_ID } from "../src/lib/winsw"; -import { parseServiceArgs, serviceReinstallArgs } from "../src/service"; +import { parseServiceArgs, serviceInstallArgs, serviceReinstallArgs } from "../src/service"; import { loadServiceTokenFromFile } from "../src/lib/service-secrets"; import { getConfigDir } from "../src/config"; import { mkdtempSync, readFileSync, writeFileSync, rmSync } from "node:fs"; @@ -249,10 +249,17 @@ describe("service backend CLI parsing", () => { }); }); -describe("service reinstall args", () => { - test("defaults to the scheduler backend on this machine (no native state)", () => { +describe("service refresh args", () => { + // 260804 #970: the post-update refresh must NOT re-register. `repair` reads the + // installed backend itself, so it is backend-agnostic AND needs no elevation on + // Windows scheduler installs, where `install` always reaches `schtasks /create`. + test("the update refresh uses repair, not a backend-specific install", () => { + expect(serviceReinstallArgs()).toEqual(["service", "repair"]); + }); + + test("explicit installs still preserve the recorded backend", () => { // On a dev machine without a native install-state the accessor maps to scheduler. - expect(serviceReinstallArgs()).toEqual(["service", "install"]); + expect(serviceInstallArgs()).toEqual(["service", "install"]); }); }); From d40367c0cf609ed10f7eb2922e5d99da5e0eaa3e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 12:23:33 +0900 Subject: [PATCH 9/9] feat(providers): rename qwen3.8-max-preview to qwen3.8-max and price it from the vendor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Alibaba shipped Qwen3.8-Max as a stable model and documents the preview endpoint as liable to be taken offline once preview concludes. Model Studio lists both ids today, so this moves the registry to the supported one across both providers: 10 sites in registry.ts, the price overlays, and 11 test files. No -preview alias is added. A config still naming the old id keeps routing — routeModel accepts an arbitrary namespaced id for a configured provider and the upstream still serves it. What such a user loses is capability metadata keyed to a retiring preview id, which is where that metadata should no longer live. Pricing moves from a Routeway reseller proxy (1.5/5/0.15) to Qwen's published $2 input / $6 output, which is exactly the exit condition the old overlay's own comment named. Two caveats stay in the source string rather than being dropped: - the figure is Qwen's release announcement, not an Alibaba Model Studio billing row (Model Studio still lists qwen3.7-max and qwen3-max, with no 3.8 entry); - no cache rate is published anywhere, so both cache fields are 0 rather than inheriting the reseller's 0.15. A reseller number under a vendor-price label would be a wrong value wearing a verified badge. Status rises to 'verified' for input/output because the vendor published them. The intl provider's defaultModel stays qwen3.7-max — that predates this change and renaming an id is not a licence to change which model a provider selects. Red-green: dropping any single metadata key during the rename fails the new survival guard. Full suite 7753 pass / 8 skip / 0 fail across 508 files. --- src/providers/registry.ts | 26 ++++----- src/usage/expected-prices.ts | 21 +++++--- tests/alibaba-intl-token-plan.test.ts | 43 +++++++++++---- tests/alibaba-region-migration.test.ts | 4 +- tests/claude-desktop-1m.test.ts | 2 +- tests/multi-agent-compat.test.ts | 4 +- tests/provider-registry-parity.test.ts | 14 ++--- tests/qwen38-preserve-reasoning.test.ts | 10 ++-- tests/reasoning-effort.test.ts | 4 +- .../router-discarded-baseurl-warning.test.ts | 4 +- tests/subagent-model-fallback-api.test.ts | 4 +- tests/subagent-model-fallback.test.ts | 54 +++++++++---------- tests/usage-cost.test.ts | 30 ++++++++--- 13 files changed, 132 insertions(+), 88 deletions(-) diff --git a/src/providers/registry.ts b/src/providers/registry.ts index 708f25d94..87a83b7d8 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -352,14 +352,14 @@ const DEEPSEEK_THINKING_REASONING_MAP: Record = { // Evidence: https://help.aliyun.com/en/model-studio/token-plan-personal-overview // https://help.aliyun.com/en/model-studio/token-plan-quickstart const ALIBABA_TOKEN_PLAN_MODELS = [ - "qwen3.8-max-preview", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-flash", + "qwen3.8-max", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-flash", "glm-5.2", "deepseek-v4-pro", ]; const ALIBABA_TOKEN_PLAN_QWEN_MODELS = [ - "qwen3.8-max-preview", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-flash", + "qwen3.8-max", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-flash", ]; const ALIBABA_TOKEN_PLAN_INPUT_MODALITIES: Record = { - "qwen3.8-max-preview": ["text", "image"], + "qwen3.8-max": ["text", "image"], "qwen3.7-max": ["text", "image"], "qwen3.7-plus": ["text", "image"], "qwen3.6-flash": ["text", "image"], @@ -372,14 +372,14 @@ const ALIBABA_TOKEN_PLAN_INPUT_MODALITIES: Record = { // Evidence: https://www.alibabacloud.com/help/en/model-studio/token-plan-overview // https://qwencloud.com/pricing/token-plan (qwen3.8 metadata) const ALIBABA_INTL_TOKEN_PLAN_MODELS = [ - "qwen3.8-max-preview", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-plus", "qwen3.6-flash", + "qwen3.8-max", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-plus", "qwen3.6-flash", "deepseek-v4-pro", "deepseek-v4-flash", "deepseek-v3.2", "kimi-k2.7-code", "kimi-k2.6", "kimi-k2.5", "glm-5.2", "glm-5.1", "glm-5", "MiniMax-M2.5", ]; const ALIBABA_INTL_TOKEN_PLAN_QWEN_MODELS = [ - "qwen3.8-max-preview", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-plus", "qwen3.6-flash", + "qwen3.8-max", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-plus", "qwen3.6-flash", ]; // 260722 Tencent Cloud Coding Plan. The plan's model set is explicitly dynamic; these are the @@ -450,7 +450,7 @@ const VOLCENGINE_PLAN_TEXT_ONLY_MODELS = [ "doubao-seed-2.0-pro", ]; const ALIBABA_INTL_TOKEN_PLAN_INPUT_MODALITIES: Record = { - "qwen3.8-max-preview": ["text", "image"], + "qwen3.8-max": ["text", "image"], "qwen3.7-max": ["text", "image"], "qwen3.7-plus": ["text", "image"], "qwen3.6-plus": ["text", "image"], @@ -1492,13 +1492,13 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ adapter: "openai-chat", authKind: "key", dashboardUrl: "https://bailian.console.aliyun.com/cn-beijing?tab=plan", - defaultModel: "qwen3.8-max-preview", + defaultModel: "qwen3.8-max", models: ALIBABA_TOKEN_PLAN_MODELS, liveModels: false, note: "Token Plan Personal Edition · China (Beijing)", modelInputModalities: ALIBABA_TOKEN_PLAN_INPUT_MODALITIES, modelContextWindows: { - "qwen3.8-max-preview": 983_616, "qwen3.7-max": 1_000_000, "qwen3.7-plus": 1_000_000, + "qwen3.8-max": 983_616, "qwen3.7-max": 1_000_000, "qwen3.7-plus": 1_000_000, "qwen3.6-flash": 1_000_000, "glm-5.2": 1_000_000, "deepseek-v4-pro": 1_000_000, }, modelReasoningEfforts: { @@ -1508,7 +1508,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ }, modelReasoningEffortMap: { "deepseek-v4-pro": DEEPSEEK_THINKING_REASONING_MAP }, thinkingBudgetModels: ALIBABA_TOKEN_PLAN_QWEN_MODELS, - preserveReasoningContentModels: ["glm-5.2", "deepseek-v4-pro", "qwen3.8-max-preview", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-flash"], + preserveReasoningContentModels: ["glm-5.2", "deepseek-v4-pro", "qwen3.8-max", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-flash"], noVisionModels: ["glm-5.2", "deepseek-v4-pro"], }, { @@ -1527,7 +1527,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ metadataModelIdNormalize: "case-insensitive", modelInputModalities: ALIBABA_INTL_TOKEN_PLAN_INPUT_MODALITIES, modelContextWindows: { - "qwen3.8-max-preview": 983_616, + "qwen3.8-max": 983_616, "qwen3.7-max": 1_000_000, "qwen3.7-plus": 1_000_000, "qwen3.6-plus": 1_000_000, "qwen3.6-flash": 1_000_000, "deepseek-v4-pro": 1_000_000, "deepseek-v4-flash": 1_000_000, "deepseek-v3.2": 131_072, "kimi-k2.7-code": 262_144, "kimi-k2.6": 262_144, "kimi-k2.5": 262_144, @@ -1536,7 +1536,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ }, modelReasoningEfforts: { ...Object.fromEntries(ALIBABA_INTL_TOKEN_PLAN_QWEN_MODELS.map(id => [id, THINKING_BUDGET_EFFORTS])), - "qwen3.8-max-preview": ["low", "high", "xhigh"], + "qwen3.8-max": ["low", "high", "xhigh"], "glm-5.2": ZAI_GLM_52_REASONING_EFFORTS, "deepseek-v4-pro": DEEPSEEK_THINKING_EFFORTS, "deepseek-v4-flash": DEEPSEEK_THINKING_EFFORTS, @@ -1546,10 +1546,10 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ "deepseek-v4-flash": DEEPSEEK_THINKING_REASONING_MAP, }, thinkingBudgetModels: ALIBABA_INTL_TOKEN_PLAN_QWEN_MODELS, - preserveReasoningContentModels: ["glm-5.2", "deepseek-v4-pro", "deepseek-v4-flash", "qwen3.8-max-preview", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-plus", "qwen3.6-flash"], + preserveReasoningContentModels: ["glm-5.2", "deepseek-v4-pro", "deepseek-v4-flash", "qwen3.8-max", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-plus", "qwen3.6-flash"], noVisionModels: ["deepseek-v4-pro", "deepseek-v4-flash", "deepseek-v3.2", "glm-5.2", "glm-5.1", "glm-5", "MiniMax-M2.5"], noReasoningModels: ["kimi-k2.7-code", "kimi-k2.6", "kimi-k2.5", "deepseek-v3.2", "glm-5.1", "glm-5", "MiniMax-M2.5"], - modelDefaultReasoningEfforts: { "qwen3.8-max-preview": "xhigh" }, + modelDefaultReasoningEfforts: { "qwen3.8-max": "xhigh" }, }, // NEEDS_HUMAN 2026-07-10: kept for config compatibility, but this is a dashboard URL, // no /models endpoint is documented, and tools are silently ignored upstream per docs.parallel.ai. diff --git a/src/usage/expected-prices.ts b/src/usage/expected-prices.ts index 4d2d62ed4..528f1ffdc 100644 --- a/src/usage/expected-prices.ts +++ b/src/usage/expected-prices.ts @@ -40,7 +40,7 @@ const KIMI_K27_CODE: Cost4 = { input: 0.95, output: 4, cacheRead: 0.19, cacheWri const KIMI_K27_CODE_HIGHSPEED: Cost4 = { input: 1.9, output: 8, cacheRead: 0.38, cacheWrite: 1.9 }; const KIMI_K26: Cost4 = { input: 0.95, output: 4, cacheRead: 0.16, cacheWrite: 0.95 }; const KIMI_K25: Cost4 = { input: 0.6, output: 3, cacheRead: 0.1, cacheWrite: 0.6 }; -const QWEN38_ROUTEWAY_TEMPORARY: Cost4 = { input: 1.5, output: 5, cacheRead: 0.15, cacheWrite: 0 }; +const QWEN38_MAX: Cost4 = { input: 2, output: 6, cacheRead: 0, cacheWrite: 0 }; // Anthropic official list prices (USD / 1M tokens). Cache write uses the published 5-minute rate. const CLAUDE_SONNET_46: Cost4 = { input: 3, output: 15, cacheRead: 0.3, cacheWrite: 3.75 }; const CLAUDE_OPUS_46: Cost4 = { input: 5, output: 25, cacheRead: 0.5, cacheWrite: 6.25 }; @@ -58,9 +58,14 @@ const DEEPSEEK_PRICING = "https://api-docs.deepseek.com/quick_start/pricing-deta // Kimi official tables publish input/output/cache-hit only; cacheWrite is mapped to the // cache-miss input price (Kimi auto-caches with no separate write billing). 2026-07-20 re-verified. const KIMI_PRICING = "https://platform.kimi.ai/docs/pricing (official table; cacheWrite derived = input, Kimi auto-cache has no write billing)"; -// TEMPORARY proxy only: Routeway's reseller API rate is not Alibaba Token Plan billing. -// Replace these overlays when Alibaba publishes an official qwen3.8-max-preview token rate. -const QWEN38_ROUTEWAY_PRICING = "https://routeway.ai/models/qwen3.8-max-preview (temporary reseller proxy; NOT Alibaba Token Plan billing; cacheWrite unpublished -> 0)"; +// 260804: Qwen3.8-Max shipped as a stable model and Qwen published a per-token rate, which +// is the exit condition the previous Routeway reseller overlay named. Two caveats are +// deliberately in the source string rather than dropped: the figure comes from Qwen's own +// release announcement, NOT from an Alibaba Model Studio billing table (which still lists +// qwen3.7-max and qwen3-max but has no qwen3.8-max row), and no cache rate is published +// anywhere. Cache stays 0 rather than inheriting the reseller's 0.15 — a reseller number +// under a vendor-price label would be a wrong value wearing a verified badge. +const QWEN38_MAX_PRICING = "https://qwen.ai/blog?id=qwen3.8 (Qwen release announcement; no Model Studio billing row yet; cache rates unpublished -> 0)"; export const EXPECTED_PRICE_OVERLAYS: readonly ExpectedPriceOverlay[] = [ // claude-opus-5 is exposed by three providers but absent from the jawcode bundle, so @@ -132,10 +137,10 @@ export const EXPECTED_PRICE_OVERLAYS: readonly ExpectedPriceOverlay[] = [ { provider: "kimi-code", modelId: "kimi-k2.6", cost4: KIMI_K26, source: KIMI_PRICING, verifiedAt: "2026-07-20", status: "verified-derived" }, { provider: "kimi-code", modelId: "kimi-k2.5", cost4: KIMI_K25, source: KIMI_PRICING, verifiedAt: "2026-07-20", status: "verified-derived" }, { provider: "kimi-code", modelId: "kimi-for-coding", cost4: KIMI_K27_CODE, source: `derived: kimi-k2.7-code ${KIMI_PRICING}`, verifiedAt: "2026-07-20", status: "verified-derived" }, - // Alibaba has not published a per-token Token Plan rate yet. Use Routeway's - // independently published reseller rate temporarily and keep estimates derived. - { provider: "alibaba-token-plan", modelId: "qwen3.8-max-preview", cost4: QWEN38_ROUTEWAY_TEMPORARY, source: QWEN38_ROUTEWAY_PRICING, verifiedAt: "2026-07-22", status: "verified-derived" }, - { provider: "alibaba-token-plan-intl", modelId: "qwen3.8-max-preview", cost4: QWEN38_ROUTEWAY_TEMPORARY, source: QWEN38_ROUTEWAY_PRICING, verifiedAt: "2026-07-22", status: "verified-derived" }, + // Qwen3.8-Max: vendor-published input/output rate (verified). See QWEN38_MAX_PRICING + // for what that source does and does not cover. + { provider: "alibaba-token-plan", modelId: "qwen3.8-max", cost4: QWEN38_MAX, source: QWEN38_MAX_PRICING, verifiedAt: "2026-08-04", status: "verified" }, + { provider: "alibaba-token-plan-intl", modelId: "qwen3.8-max", cost4: QWEN38_MAX, source: QWEN38_MAX_PRICING, verifiedAt: "2026-08-04", status: "verified" }, // Cursor Auto router — Cursor's published fixed token price (verified). { provider: "cursor", modelId: "auto", cost4: { input: 1.25, output: 6, cacheRead: 0.25, cacheWrite: 1.25 }, source: "https://docs.cursor.com/account/pricing + https://cursor.com/blog/aug-2025-pricing", verifiedAt: "2026-07-20", status: "verified" }, ]; diff --git a/tests/alibaba-intl-token-plan.test.ts b/tests/alibaba-intl-token-plan.test.ts index f8b7cece4..1d77a1c2e 100644 --- a/tests/alibaba-intl-token-plan.test.ts +++ b/tests/alibaba-intl-token-plan.test.ts @@ -32,7 +32,7 @@ describe("alibaba-token-plan-intl registry entry", () => { expect(entry!.models).toContain("kimi-k2.7-code"); expect(entry!.models).toContain("glm-5.2"); expect(entry!.models).toContain("MiniMax-M2.5"); - expect(entry!.models).toContain("qwen3.8-max-preview"); + expect(entry!.models).toContain("qwen3.8-max"); expect(entry!.models!.length).toBe(15); }); @@ -41,9 +41,9 @@ describe("alibaba-token-plan-intl registry entry", () => { expect(entry!.metadataModelIdNormalize).toBe("case-insensitive"); }); - test("qwen3.8-max-preview has correct context window", () => { + test("qwen3.8-max has correct context window", () => { const entry = PROVIDER_REGISTRY.find(e => e.id === "alibaba-token-plan-intl"); - expect(entry!.modelContextWindows?.["qwen3.8-max-preview"]).toBe(983_616); + expect(entry!.modelContextWindows?.["qwen3.8-max"]).toBe(983_616); }); test("every international chat model has an explicit context window", () => { @@ -54,19 +54,44 @@ describe("alibaba-token-plan-intl registry entry", () => { } }); - test("qwen3.8-max-preview reasoning efforts", () => { + test("qwen3.8-max reasoning efforts", () => { const entry = PROVIDER_REGISTRY.find(e => e.id === "alibaba-token-plan-intl"); - expect(entry!.modelReasoningEfforts?.["qwen3.8-max-preview"]).toEqual(["low", "high", "xhigh"]); + expect(entry!.modelReasoningEfforts?.["qwen3.8-max"]).toEqual(["low", "high", "xhigh"]); }); - test("qwen3.8-max-preview default reasoning effort is xhigh", () => { + test("qwen3.8-max default reasoning effort is xhigh", () => { const entry = PROVIDER_REGISTRY.find(e => e.id === "alibaba-token-plan-intl"); - expect(entry!.modelDefaultReasoningEfforts?.["qwen3.8-max-preview"]).toBe("xhigh"); + expect(entry!.modelDefaultReasoningEfforts?.["qwen3.8-max"]).toBe("xhigh"); }); - test("qwen3.8-max-preview is in preserveReasoningContentModels", () => { + test("qwen3.8-max is in preserveReasoningContentModels", () => { const entry = PROVIDER_REGISTRY.find(e => e.id === "alibaba-token-plan-intl"); - expect(entry!.preserveReasoningContentModels).toContain("qwen3.8-max-preview"); + expect(entry!.preserveReasoningContentModels).toContain("qwen3.8-max"); + }); + + // 260804: Qwen3.8-Max left preview, and Alibaba documents the preview endpoint as + // liable to be taken offline. The rename must carry EVERY capability key across both + // Alibaba providers — a rename that silently drops one degrades the model without + // failing anything else. Ablate by removing any single key below and this goes red. + test("the preview id is fully retired and its metadata moved to the stable id", () => { + for (const id of ["alibaba-token-plan", "alibaba-token-plan-intl"]) { + const entry = PROVIDER_REGISTRY.find(e => e.id === id)!; + expect(entry.models).toContain("qwen3.8-max"); + expect(entry.models).not.toContain("qwen3.8-max-preview"); + expect(entry.modelContextWindows?.["qwen3.8-max"]).toBe(983_616); + expect(entry.modelContextWindows?.["qwen3.8-max-preview"]).toBeUndefined(); + expect(entry.modelInputModalities?.["qwen3.8-max"]).toEqual(["text", "image"]); + expect(entry.preserveReasoningContentModels).toContain("qwen3.8-max"); + expect(entry.preserveReasoningContentModels).not.toContain("qwen3.8-max-preview"); + } + // The intl entry additionally carries the effort ladder. + const intl = PROVIDER_REGISTRY.find(e => e.id === "alibaba-token-plan-intl")!; + expect(intl.modelReasoningEfforts?.["qwen3.8-max"]).toEqual(["low", "high", "xhigh"]); + expect(intl.modelDefaultReasoningEfforts?.["qwen3.8-max"]).toBe("xhigh"); + // Only the Beijing entry defaults to this model; intl deliberately defaults to + // qwen3.7-max. That predates this rename and is left alone — renaming an id is not + // a licence to change which model a provider selects by default. + expect(PROVIDER_REGISTRY.find(e => e.id === "alibaba-token-plan")!.defaultModel).toBe("qwen3.8-max"); }); test("non-reasoning models are marked", () => { diff --git a/tests/alibaba-region-migration.test.ts b/tests/alibaba-region-migration.test.ts index 3b56fb8be..96d4ee4a4 100644 --- a/tests/alibaba-region-migration.test.ts +++ b/tests/alibaba-region-migration.test.ts @@ -31,8 +31,8 @@ test("moves a Beijing entry holding an international endpoint", () => { const config = migratableConfig(); // Beijing catalog fields, as `ocx provider add` would have persisted them. Object.assign(config.providers["alibaba-token-plan"]!, { - models: ["qwen3.8-max-preview", "qwen3.7-max"], - defaultModel: "qwen3.8-max-preview", + models: ["qwen3.8-max", "qwen3.7-max"], + defaultModel: "qwen3.8-max", }); const projection = projectAlibabaRegionMigration(config); diff --git a/tests/claude-desktop-1m.test.ts b/tests/claude-desktop-1m.test.ts index 10f1a0c0d..6625d7587 100644 --- a/tests/claude-desktop-1m.test.ts +++ b/tests/claude-desktop-1m.test.ts @@ -32,7 +32,7 @@ test("supports1m is true at and above the threshold, false below it", async () = // Live-backed assertions against the real catalog: 1 MiB windows qualify. const oneMiB = state.models.find(m => m.route === "google-antigravity/gemini-3.1-pro"); const exact1M = state.models.find(m => m.route === "alibaba-token-plan-intl/glm-5.2"); - const below = state.models.find(m => m.route === "alibaba-token-plan-intl/qwen3.8-max-preview"); + const below = state.models.find(m => m.route === "alibaba-token-plan-intl/qwen3.8-max"); const blank = state.models.find(m => m.route === "anthropic/claude-opus-4-6"); if (oneMiB) expect(oneMiB.supports1m).toBe(true); // 1_048_576 diff --git a/tests/multi-agent-compat.test.ts b/tests/multi-agent-compat.test.ts index 5b1e24214..b76d1fe93 100644 --- a/tests/multi-agent-compat.test.ts +++ b/tests/multi-agent-compat.test.ts @@ -434,11 +434,11 @@ describe("multiAgentGuidanceText", () => { parsedFixture({ tools: [{ name: "spawn_agent" }] }), { injectionPrompt: "FALLBACK={{fallback}}", - subagentModelFallback: ["alibaba-token-plan/qwen3.8-max-preview", "kimi/k3"], + subagentModelFallback: ["alibaba-token-plan/qwen3.8-max", "kimi/k3"], }, ); expect(text).toContain("FALLBACK="); - expect(text).toContain("alibaba-token-plan/qwen3.8-max-preview"); + expect(text).toContain("alibaba-token-plan/qwen3.8-max"); expect(text).toContain("kimi/k3"); }); diff --git a/tests/provider-registry-parity.test.ts b/tests/provider-registry-parity.test.ts index a1be64d29..0d32c70c7 100644 --- a/tests/provider-registry-parity.test.ts +++ b/tests/provider-registry-parity.test.ts @@ -261,29 +261,29 @@ describe("provider registry parity", () => { label: "Alibaba Token Plan (Beijing)", adapter: "openai-chat", baseUrl: "https://token-plan.cn-beijing.maas.aliyuncs.com/compatible-mode/v1", - defaultModel: "qwen3.8-max-preview", + defaultModel: "qwen3.8-max", liveModels: false, models: [ - "qwen3.8-max-preview", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-flash", + "qwen3.8-max", "qwen3.7-max", "qwen3.7-plus", "qwen3.6-flash", "glm-5.2", "deepseek-v4-pro", ], modelInputModalities: { - "qwen3.8-max-preview": ["text", "image"], + "qwen3.8-max": ["text", "image"], "qwen3.7-max": ["text", "image"], }, modelReasoningEfforts: { - "qwen3.8-max-preview": ["low", "medium", "high", "xhigh", "max"], + "qwen3.8-max": ["low", "medium", "high", "xhigh", "max"], }, modelContextWindows: { - "qwen3.8-max-preview": 983_616, + "qwen3.8-max": 983_616, "qwen3.7-max": 1_000_000, "deepseek-v4-pro": 1_000_000, }, noVisionModels: ["glm-5.2", "deepseek-v4-pro"], - preserveReasoningContentModels: expect.arrayContaining(["qwen3.8-max-preview", "qwen3.7-max", "qwen3.7-plus"]), + preserveReasoningContentModels: expect.arrayContaining(["qwen3.8-max", "qwen3.7-max", "qwen3.7-plus"]), }); expect(KEY_LOGIN_PROVIDERS["alibaba-token-plan"].thinkingBudgetModels) - .toContain("qwen3.8-max-preview"); + .toContain("qwen3.8-max"); }); test("aggregator defaults and Neuralwatt seeds match the audited live catalogs", () => { diff --git a/tests/qwen38-preserve-reasoning.test.ts b/tests/qwen38-preserve-reasoning.test.ts index d93820d8c..a14d1d751 100644 --- a/tests/qwen38-preserve-reasoning.test.ts +++ b/tests/qwen38-preserve-reasoning.test.ts @@ -8,15 +8,15 @@ function provider(overrides: Partial = {}): OcxProviderConfig baseUrl: "https://token-plan.ap-southeast-1.maas.aliyuncs.com/compatible-mode/v1", apiKey: "sk-test", authMode: "key", - preserveReasoningContentModels: ["qwen3.8-max-preview"], - thinkingBudgetModels: ["qwen3.8-max-preview"], + preserveReasoningContentModels: ["qwen3.8-max"], + thinkingBudgetModels: ["qwen3.8-max"], ...overrides, }; } function parsedWithThinkingHistory(): OcxParsedRequest { return { - modelId: "qwen3.8-max-preview", + modelId: "qwen3.8-max", context: { messages: [ { role: "user", content: "fix the bug in auth.ts", timestamp: 1 }, @@ -102,10 +102,10 @@ describe("Qwen 3.8 reasoning_content preservation", () => { expect(assistantMsgs[1].reasoning_content).toBeUndefined(); }); - test("registry includes qwen3.8-max-preview in alibaba-token-plan preserveReasoningContentModels", async () => { + test("registry includes qwen3.8-max in alibaba-token-plan preserveReasoningContentModels", async () => { const { PROVIDER_REGISTRY } = await import("../src/providers/registry"); const alibaba = PROVIDER_REGISTRY.find(e => e.id === "alibaba-token-plan"); expect(alibaba).toBeDefined(); - expect(alibaba!.preserveReasoningContentModels).toContain("qwen3.8-max-preview"); + expect(alibaba!.preserveReasoningContentModels).toContain("qwen3.8-max"); }); }); diff --git a/tests/reasoning-effort.test.ts b/tests/reasoning-effort.test.ts index ba3385a7f..34acca295 100644 --- a/tests/reasoning-effort.test.ts +++ b/tests/reasoning-effort.test.ts @@ -640,14 +640,14 @@ describe("thinking-budget models (260709)", () => { }, }, } as unknown as OcxConfig; - const route = routeModel(config, "alibaba-token-plan/qwen3.8-max-preview"); + const route = routeModel(config, "alibaba-token-plan/qwen3.8-max"); expect(route.provider.modelInputModalities?.[route.modelId]).toEqual(["text", "image"]); expect(route.provider.thinkingBudgetModels).toContain(route.modelId); expect(route.provider.modelReasoningEfforts?.[route.modelId]).toEqual(["low", "medium", "high", "xhigh", "max"]); const body = buildBody(route.provider, route.modelId, { reasoning: "max", maxOutputTokens: 65536 }); - expect(body).toMatchObject({ model: "qwen3.8-max-preview", thinking_budget: 65536 }); + expect(body).toMatchObject({ model: "qwen3.8-max", thinking_budget: 65536 }); expect(body).not.toHaveProperty("reasoning_effort"); }); diff --git a/tests/router-discarded-baseurl-warning.test.ts b/tests/router-discarded-baseurl-warning.test.ts index c3f091bf1..05ea18e25 100644 --- a/tests/router-discarded-baseurl-warning.test.ts +++ b/tests/router-discarded-baseurl-warning.test.ts @@ -158,7 +158,7 @@ test("alibaba-token-plan is pinned to Beijing and warns about a saved internatio adapter: "openai-chat", baseUrl: ALIBABA_INTL_BASE_URL, }); - const warnings = routeCapturingWarnings(config, "alibaba-token-plan/qwen3.8-max-preview"); + const warnings = routeCapturingWarnings(config, "alibaba-token-plan/qwen3.8-max"); expect(warnings).toHaveLength(1); expect(warnings[0]).toContain("token-plan.ap-southeast-1.maas.aliyuncs.com"); @@ -167,7 +167,7 @@ test("alibaba-token-plan is pinned to Beijing and warns about a saved internatio const originalWarn = console.warn; console.warn = () => {}; try { - expect(routeModel(config, "alibaba-token-plan/qwen3.8-max-preview").provider.baseUrl) + expect(routeModel(config, "alibaba-token-plan/qwen3.8-max").provider.baseUrl) .toBe(ALIBABA_BEIJING_BASE_URL); } finally { console.warn = originalWarn; diff --git a/tests/subagent-model-fallback-api.test.ts b/tests/subagent-model-fallback-api.test.ts index d93ca16f8..7a094ef04 100644 --- a/tests/subagent-model-fallback-api.test.ts +++ b/tests/subagent-model-fallback-api.test.ts @@ -54,7 +54,7 @@ describe("/api/subagent-model-fallback atomic validation", () => { const config = makeConfig({ subagentModelFallback: [...previous] }); const res = await put(config, { - models: ["gpt-5.6-sol", 42, "alibaba-token-plan/qwen3.8-max-preview"], + models: ["gpt-5.6-sol", 42, "alibaba-token-plan/qwen3.8-max"], }); expect(res.status).toBe(400); const body = await res.json() as { error: string; index: number; value: unknown }; @@ -82,7 +82,7 @@ describe("/api/subagent-model-fallback atomic validation", () => { test("accepts a fully valid chain after validation", async () => { isolatedHome(); const config = makeConfig(); - const next = ["gpt-5.6-sol", "alibaba-token-plan/qwen3.8-max-preview"]; + const next = ["gpt-5.6-sol", "alibaba-token-plan/qwen3.8-max"]; const res = await put(config, { models: next }); expect(res.status).toBe(200); expect(await res.json()).toMatchObject({ ok: true, models: next }); diff --git a/tests/subagent-model-fallback.test.ts b/tests/subagent-model-fallback.test.ts index 127fae1c3..5a546998b 100644 --- a/tests/subagent-model-fallback.test.ts +++ b/tests/subagent-model-fallback.test.ts @@ -61,7 +61,7 @@ function cfg(overrides: Partial = {}): OcxConfig { ], subagentModelFallback: [ "gpt-5.6-sol", - "alibaba-token-plan/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", "kimi/k3", ], ...overrides, @@ -108,13 +108,13 @@ describe("subagent model fallback chain", () => { test("buildSubagentModelChain dedupes and preserves order", () => { expect(buildSubagentModelChain("gpt-5.6-sol", cfg())).toEqual([ "gpt-5.6-sol", - "alibaba-token-plan/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", "kimi/k3", ]); expect(buildSubagentModelChain("kimi/k3", cfg())).toEqual([ "kimi/k3", "gpt-5.6-sol", - "alibaba-token-plan/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", ]); }); @@ -123,7 +123,7 @@ describe("subagent model fallback chain", () => { updateAccountQuota("pool-a", 95, undefined, 20); const selected = selectAvailableSubagentModel("gpt-5.6-sol", cfg()); expect(selected).toEqual({ - model: "alibaba-token-plan/qwen3.8-max-preview", + model: "alibaba-token-plan/qwen3.8-max", rewritten: true, skipped: ["gpt-5.6-sol"], }); @@ -144,10 +144,10 @@ describe("subagent model fallback chain", () => { test("selectAvailableSubagentModel skips cached routed failures", () => { resetSubagentModelFallbackStateForTests(); updateAccountQuota("pool-a", 95, undefined, 20); - noteSubagentModelFailure("alibaba-token-plan/qwen3.8-max-preview", "quota exhausted", cfg()); + noteSubagentModelFailure("alibaba-token-plan/qwen3.8-max", "quota exhausted", cfg()); const selected = selectAvailableSubagentModel("gpt-5.6-sol", cfg()); expect(selected.model).toBe("kimi/k3"); - expect(isSubagentModelUnavailable("alibaba-token-plan/qwen3.8-max-preview", cfg())).toBe(true); + expect(isSubagentModelUnavailable("alibaba-token-plan/qwen3.8-max", cfg())).toBe(true); }); test("selectAvailableSubagentModel skips stale fallback entries that cannot route", () => { @@ -215,7 +215,7 @@ describe("subagent model fallback chain", () => { await priming; expect(midRefreshModel).toBe("gpt-5.6-sol"); expect(selectAvailableSubagentModel("gpt-5.6-sol", cfg()).model).toBe( - "alibaba-token-plan/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", ); expect(getSubagentQuotaPrimeStateForTests().primedAt).toBeGreaterThan(0); expect(getSubagentQuotaPrimeStateForTests().inFlight).toBe(false); @@ -240,7 +240,7 @@ describe("subagent model fallback chain", () => { await Promise.all([a, b, c]); expect(calls).toBe(1); expect(selectAvailableSubagentModel("gpt-5.6-sol", cfg()).model).toBe( - "alibaba-token-plan/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", ); }); @@ -287,8 +287,8 @@ describe("subagent model fallback chain", () => { test("noteSubagentModelFailure records the configured fallback slug", () => { resetSubagentModelFallbackStateForTests(); - noteSubagentModelFailure("alibaba-token-plan/qwen3.8-max-preview", "429", cfg()); - expect(isSubagentModelUnavailable("alibaba-token-plan/qwen3.8-max-preview", cfg())).toBe(true); + noteSubagentModelFailure("alibaba-token-plan/qwen3.8-max", "429", cfg()); + expect(isSubagentModelUnavailable("alibaba-token-plan/qwen3.8-max", cfg())).toBe(true); expect(isSubagentModelUnavailable("kimi/k3", cfg())).toBe(false); }); @@ -306,7 +306,7 @@ describe("subagent model fallback chain", () => { expect(selected).toEqual({ model: "gpt-5.6-sol", rewritten: false, - skipped: ["gpt-5.6-sol", "alibaba-token-plan/qwen3.8-max-preview", "kimi/k3"], + skipped: ["gpt-5.6-sol", "alibaba-token-plan/qwen3.8-max", "kimi/k3"], }); }); @@ -324,7 +324,7 @@ describe("subagent model fallback chain", () => { expect(selected).toEqual({ model: "gpt-5.6-sol", rewritten: false, - skipped: ["gpt-5.6-sol", "alibaba-token-plan/qwen3.8-max-preview", "kimi/k3"], + skipped: ["gpt-5.6-sol", "alibaba-token-plan/qwen3.8-max", "kimi/k3"], }); }); @@ -574,8 +574,8 @@ describe("subagent model fallback chain", () => { test("noteSubagentModelFailure records failures under the configured fallback slug", () => { resetSubagentModelFallbackStateForTests(); - noteSubagentModelFailure("alibaba-token-plan/qwen3.8-max-preview", "429", cfg(), "pool-a"); - expect(isSubagentModelUnavailable("alibaba-token-plan/qwen3.8-max-preview", cfg(), "pool-a")).toBe(true); + noteSubagentModelFailure("alibaba-token-plan/qwen3.8-max", "429", cfg(), "pool-a"); + expect(isSubagentModelUnavailable("alibaba-token-plan/qwen3.8-max", cfg(), "pool-a")).toBe(true); expect(isSubagentModelUnavailable("kimi/k3", cfg(), "pool-a")).toBe(false); }); @@ -585,13 +585,13 @@ describe("subagent model fallback chain", () => { "name = \"executor\"", "model = \"gpt-5.6-sol\"", "model_fallback = [", - " \"alibaba-token-plan/qwen3.8-max-preview\",", + " \"alibaba-token-plan/qwen3.8-max\",", " \"kimi/k3\",", "]", "", ].join("\n"), "utf8"); expect(readCodexAgentModelFallback("executor", dir)).toEqual([ - "alibaba-token-plan/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", "kimi/k3", ]); }); @@ -602,13 +602,13 @@ describe("subagent model fallback chain", () => { "name = \"executor\"", "model = \"gpt-5.6-sol\"", "model_fallback = [", - " \"alibaba-token-plan/qwen3.8-max-preview\",", + " \"alibaba-token-plan/qwen3.8-max\",", "]", "tools = [\"search\", \"edit\"]", "", ].join("\n"), "utf8"); expect(readCodexAgentModelFallback("executor", dir)).toEqual([ - "alibaba-token-plan/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", ]); }); @@ -651,8 +651,8 @@ describe("subagent model fallback chain", () => { test("noteSubagentModelFailure scopes routed-provider health globally", () => { resetSubagentModelFallbackStateForTests(); - noteSubagentModelFailure("alibaba-token-plan/qwen3.8-max-preview", "quota exhausted", cfg(), "account-a"); - expect(isSubagentModelUnavailable("alibaba-token-plan/qwen3.8-max-preview", cfg(), "account-b")).toBe(true); + noteSubagentModelFailure("alibaba-token-plan/qwen3.8-max", "quota exhausted", cfg(), "account-a"); + expect(isSubagentModelUnavailable("alibaba-token-plan/qwen3.8-max", cfg(), "account-b")).toBe(true); }); test("applySubagentModelFallback rewrites parsed request model", () => { @@ -670,11 +670,11 @@ describe("subagent model fallback chain", () => { ); expect(result).toEqual({ from: "gpt-5.6-sol", - to: "alibaba-token-plan/qwen3.8-max-preview", + to: "alibaba-token-plan/qwen3.8-max", skipped: ["gpt-5.6-sol"], }); - expect(parsed.modelId).toBe("alibaba-token-plan/qwen3.8-max-preview"); - expect((parsed._rawBody as { model?: string }).model).toBe("alibaba-token-plan/qwen3.8-max-preview"); + expect(parsed.modelId).toBe("alibaba-token-plan/qwen3.8-max"); + expect((parsed._rawBody as { model?: string }).model).toBe("alibaba-token-plan/qwen3.8-max"); }); test("applySubagentModelFallback is a no-op for main turns", () => { @@ -694,7 +694,7 @@ describe("subagent model fallback chain", () => { writeFileSync(join(dir, "agents", "executor.toml"), [ "name = \"executor\"", "model = \"gpt-5.6-sol\"", - "model_fallback = [\"alibaba-token-plan/qwen3.8-max-preview\"]", + "model_fallback = [\"alibaba-token-plan/qwen3.8-max\"]", "", ].join("\n"), "utf8"); updateAccountQuota("pool-a", 95); @@ -709,12 +709,12 @@ describe("subagent model fallback chain", () => { new Headers({ "x-openai-subagent": "collab_spawn" }), cfg({ subagentModelFallback: undefined }), ); - expect(result?.to).toBe("alibaba-token-plan/qwen3.8-max-preview"); + expect(result?.to).toBe("alibaba-token-plan/qwen3.8-max"); expect(resolveAgentModelFallbackForPrimary("gpt-5.6-sol", dir)).toEqual([ - "alibaba-token-plan/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", ]); expect(readCodexAgentModelFallback("executor", dir)).toEqual([ - "alibaba-token-plan/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", ]); }); diff --git a/tests/usage-cost.test.ts b/tests/usage-cost.test.ts index f4e0d114f..d58347139 100644 --- a/tests/usage-cost.test.ts +++ b/tests/usage-cost.test.ts @@ -176,17 +176,31 @@ describe("resolveMatchedPrice", () => { expect(price?.cost4.input).toBe(0.25); }); - test("17f. Alibaba Token Plan Qwen 3.8 uses the temporary Routeway proxy", () => { + // 260804: Qwen published a per-token rate for the stable qwen3.8-max, which is exactly + // the exit condition the old Routeway reseller overlay named, so the proxy rate is gone. + test("17f. Alibaba Token Plan Qwen 3.8 uses the vendor-published rate", () => { for (const provider of ["alibaba-token-plan", "alibaba-token-plan-intl"]) { - const price = resolveMatchedPrice(provider, "qwen3.8-max-preview"); + const price = resolveMatchedPrice(provider, "qwen3.8-max"); expect(price).toMatchObject({ provider, - modelId: "qwen3.8-max-preview", - cost4: { input: 1.5, output: 5, cacheRead: 0.15, cacheWrite: 0 }, + modelId: "qwen3.8-max", + cost4: { input: 2, output: 6, cacheRead: 0, cacheWrite: 0 }, source: "expected", - status: "verified-derived", + status: "verified", }); - expect(price?.sourceRef).toContain("temporary reseller proxy"); + // The source string must keep carrying what the vendor figure does NOT cover: + // there is no Model Studio billing row yet, and no published cache rate. Dropping + // either caveat would present an announcement price as billing-table verified. + expect(price?.sourceRef).toContain("qwen.ai/blog"); + expect(price?.sourceRef).toContain("cache rates unpublished"); + expect(price?.sourceRef).not.toContain("routeway"); + } + }); + + test("17g. the retired qwen3.8-max-preview id no longer resolves", () => { + // Alibaba retires the preview endpoint; capability metadata follows the stable id. + for (const provider of ["alibaba-token-plan", "alibaba-token-plan-intl"]) { + expect(resolveMatchedPrice(provider, "qwen3.8-max-preview")).toBeNull(); } }); @@ -280,8 +294,8 @@ describe("resolveMatchedPrice", () => { "kimi-code/kimi-k2.6", "kimi-code/kimi-k2.5", "kimi-code/kimi-for-coding", - "alibaba-token-plan/qwen3.8-max-preview", - "alibaba-token-plan-intl/qwen3.8-max-preview", + "alibaba-token-plan/qwen3.8-max", + "alibaba-token-plan-intl/qwen3.8-max", "cursor/auto", ]) { expect(keys.has(expected)).toBe(true);