[ThinkOffApp]

Summary

The 2026-09-22 switcher audit found that bin/model-picker.mjs (PR #119) can probe the fleet, raise a CHOICE intent, and apply the tap — but nothing invoked it when the decision came from a channel other than the CLI's own blocking poll. A CodeWatch tap or a GroupMind web tap decided the intent and then nothing consumed the decision.

• src/confirmations.mjs: createIntent() takes an optional kind tag (exposed via getIntent/listIntents) and an optional offeredModels map (option → model name shown in the offer, recorded at raise time). decideIntent() — the one chokepoint every decision path already funnels through (HTTP /intent/:id/decision, the chat-reply poller, approve_intent/deny_intent) — now fires a registered per-kind handler once, there, regardless of which channel settled the intent (registerKindHandler/unregisterKindHandler/pendingKindHandler), passing offeredModels through to it.
• src/model-selection.mjs (new): the probe/offer/apply core factored out of bin/model-picker.mjs, including a new applyChoice() — re-probe the chosen entry, write the selection file if it's still usable, otherwise refuse with CHANGED_SINCE_OFFER (previous selection untouched). Usable standalone (no offer-time reading required, e.g. from the daemon) as well as from pickModel()'s own CLI flow. Also exports resolveModelRegistryPath()/resolveModelSelectionPath() — the one place mcp.confirmations.model_registry/.model_selection_path are read from config, so the daemon (apply) and the MCP tool (offer) can never silently disagree on which registry/selection file they mean.
• bin/iak-mcp-daemon.mjs: registers a "model" kind handler at startup that calls applyChoice() (passing through the offered model for the chosen option) and logs the outcome, including a [model changed since offer: X -> Y] note when the re-probed entry is UP but now names a different model than it did in the offer. model-picker.mjs's own CLI-raised intents are not kind-tagged, so running the CLI against this daemon still applies via its own in-process re-probe exactly as before — a tap is never applied twice.
• src/mcp-server.mjs: new request_model_choice MCP tool — the daemon-side way to raise a model choice without shelling out to the CLI. Probes the registry, raises a kind: "model" CHOICE intent (with offeredModels attached), follows the same daemon-forward / in-process pattern request_choice already uses. Its description states plainly that without a running daemon the decision is returned but is NOT applied — no selection file is written in that mode, by this tool or anything else.
• bin/model-picker.mjs: now a thin CLI shell over src/model-selection.mjs (arg parsing, exit codes, rendering). Same exported names, same behaviour — test/model-picker.test.mjs is unchanged and still passes.

Update (review round 2): fixed three issues from review — (1) the daemon and the MCP tool were reading the custom registry/selection path from two different config keys (mcp.confirmations.model_registry vs mcp.model_registry), so a custom path configured once could make the offer and the apply probe different files; both now go through the shared resolveModelRegistryPath/resolveModelSelectionPath. (2) applyChoice() applied any re-probed UP entry regardless of whether it still named the model shown in the offer; it now still applies (the box IS up) but reports modelChanged: {offered, applied} on the outcome, and the daemon logs it. (3) request_model_choice's description was reworded to say plainly that the no-daemon fallback does not apply the decision.

What is tested, and what is not

Tested (test/model-selection.test.mjs, new, 10 tests):

• A decided kind: "model" intent whose chosen entry still probes UP: the registered handler fires and applyChoice() writes the selection file with the expected selectedId/model/selectedFrom.
• A decided model intent whose chosen entry has gone DOWN between offer and tap: applyChoice() returns CHANGED_SINCE_OFFER, the error names what it was and is now, and no selection file is written.
• Negative control: an ordinary (non-model) CHOICE intent triggers no apply — the registered "model" handler is not called and no file is written.
• resolveModelRegistryPath()/resolveModelSelectionPath(): a custom path under mcp.confirmations is honoured; the old (wrong) one-level-up key mcp.model_registry is explicitly asserted NOT to be picked up; the default falls back to <root>/config/models.json.
• A custom registry path configured once resolves identically regardless of caller, plus a structural check that both bin/iak-mcp-daemon.mjs and src/mcp-server.mjs actually call the shared resolver (and don't read the old keys directly any more).
• applyChoice() still applies a box that is UP but now serves a different model than offered, and reports modelChanged: {offered, applied}; a matching case with no drift confirms modelChanged is undefined when nothing changed.
• End-to-end: createIntent() with offeredModels → decideIntent() → the daemon-shaped kind handler → applyChoice() reports the correct modelChanged for the specific option that was picked.
• Structural check that request_model_choice's description text states both that a live daemon applies the choice and that the no-daemon fallback does not.
• I verified every one of the above actually depends on its wiring/logic by temporarily reverting each fix in turn (the decideIntent() hook, the resolver call in src/mcp-server.mjs, the modelChanged computation in applyChoice(), the tool-description wording) and confirming the corresponding test(s) fail, then restored the fix and confirmed they pass again.
• Full suite: npm test → 736/736 passing, exit 0 (726 original + 10 new across two review rounds). Ran the new/changed test file alone first (node --test test/model-selection.test.mjs), then the full suite both times.
• node bin/model-picker.mjs --dry-run in this worktree: there is no config/models.json here (only config/models.example.json), so it correctly reports NO_REGISTRY and exits 2 — no live boxes were touched, and I did not fabricate a registry to probe hosts that aren't reachable from here.

Not tested / known gaps:

• No test runs bin/iak-mcp-daemon.mjs itself end-to-end (it needs mcp.confirmations.room/codewatch_gate_url configured and a live GroupMind room to be meaningful) — the daemon's own registerKindHandler('model', ...) callback and its [model changed since offer: ...] log line are exercised only by inspection and by a test that mirrors its shape against src/confirmations.mjs directly, not by importing/running the daemon script itself (which has top-level side effects — loadConfig, process.exit without channels configured — that make it awkward to import in a unit test).
• No test exercises request_model_choice in src/mcp-server.mjs end-to-end (the MCP tool itself, its daemon-forward path, or its in-process fallback) — only its description text is checked structurally. Existing MCP-server tests don't cover it either.
• The request_model_choice in-process fallback (no daemon running) still has no consumer of the decision at all — same limitation request_choice already has in that mode; now stated explicitly in the tool description, not silently left.
• I did not run this against a live daemon, a real config/models.json, or real boxes — everything above is unit-level with fake registries/probes. No process was restarted, no config file was touched, and /Users/petrus/ide-agent-kit (the live daemon's checkout) was never touched — all work happened in the ~/tmp/iak-picker worktree on feat/wire-model-picker.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.