stack 7/7: NIM vision classification, service repair, and the qwen3.8-max rename

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)], {

devlog/_plan/260804_stack7_service_vision/000_scope.md

@@ -0,0 +1,95 @@
+# 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 |
+
+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.
+
+## 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 ~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
+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
+(`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 |
+|---|---|
+| `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 |

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.