fix: send chat model choices as model ids

[e7h4n]

Summary

• remove modelSelection and modelProvider from chat write request contracts
• send only model from frontend chat thread creation and model update flows
• normalize model to the existing model-first server flow so org policy/provider admission chooses the provider
• update API/platform helpers and coverage to assert model-only request bodies

Verification

• pnpm -F @vm0/api-contracts check-types
• NODE_OPTIONS=--max-old-space-size=4096 pnpm -F @vm0/app check-types
• pnpm -F @vm0/core check-types
• pnpm -F api exec oxlint ...changed API files
• pnpm -F api exec oxlint ...changed API files --type-aware -c ../../.oxlintrc.typeaware.json
• pnpm -F @vm0/api-contracts exec oxlint src/contracts/chat-threads.ts src/contracts/index.ts
• pnpm -F @vm0/core exec oxlint src/index.ts
• pnpm -F @vm0/app exec oxlint ...changed platform files
• git diff --check

Note: api check-types still gets killed by the local environment with exit 137 before reporting TypeScript diagnostics.

[codecov]

Codecov Report

❌ Patch coverage is 88.88889% with 8 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...pi/src/signals/routes/test-model-provider-state.ts	85.18%	4 Missing ⚠️
...ckages/api-contracts/src/contracts/chat-threads.ts	89.65%	3 Missing ⚠️
.../apps/api/src/signals/routes/zero-chat-messages.ts	88.88%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

API preview: https://pr-20639-api.vm6.ai

[e7h4n]

Changes Requested

PR_AUTO_REVIEW_COMMIT: e786d57
PR_AUTO_REVIEW_VERDICT: Changes Requested

Summary

This PR simplifies chat write contracts so clients send a model id and the API resolves the effective provider from org policy. The main API, platform, CLI, and tests are mostly aligned with the model-only shape, but the API contract removal needs rollout compatibility coverage before merge.

Findings

Critical (P0) - must fix before merge

• None found.

High Priority (P1) - should fix before merge

• turbo/packages/api-contracts/src/contracts/chat-threads.ts:395: The new chatThreadCreateBodySchema requires model, and the update/send schemas now only expose model, but there is no compatibility parser or regression test for still-loaded old frontend/CLI clients sending the previous modelSelection / modelProvider write shapes. During deployment, old browser bundles can keep posting modelSelection to the new API before refresh; create-thread and model-update requests will fail validation because model is missing, and existing-thread sends with an old run-level model selection can silently fall back to the stored thread model. The repo testing standard explicitly requires previous request/data-shape coverage for API contracts when old and new versions can overlap during rollout. Please either add a read-time/request compatibility normalizer with tests for the previous write bodies, or document and gate a deliberate breaking rollout path that prevents old clients from talking to the new API.

Testing

• Coverage: Insufficient. The PR updates model-only happy paths, but it does not cover previous request shapes for /api/zero/chat-threads, /api/zero/chat-threads/:id/model-selection, or /api/zero/chat/messages.
• Conventions: The added/updated tests otherwise follow the integration-style approach. Latest CI for this head was still in progress at review time and had test-api (3) / test-api (8) reported failing, but GitHub had not made logs available yet because the workflow run was still active.

[e7h4n]

Changes Requested

PR_AUTO_REVIEW_COMMIT: e28a60d
PR_AUTO_REVIEW_VERDICT: Changes Requested

Summary

This PR simplifies chat write contracts so clients send a model id and the API resolves the effective provider from org policy. The main API, platform, CLI, and tests are mostly aligned with the model-only shape, but the API contract removal still needs rollout compatibility coverage before merge.

Findings

Critical (P0) - must fix before merge

• None found.

High Priority (P1) - should fix before merge

• turbo/packages/api-contracts/src/contracts/chat-threads.ts:395: The new chatThreadCreateBodySchema requires model, and the update/send schemas now only expose model, but there is still no compatibility parser or regression test for still-loaded old frontend/CLI clients sending the previous modelSelection / modelProvider write shapes. During deployment, old browser bundles can keep posting modelSelection to the new API before refresh; create-thread and model-update requests will fail validation because model is missing, and existing-thread sends with an old run-level model selection can silently fall back to the stored thread model. The repo testing standard explicitly requires previous request/data-shape coverage for API contracts when old and new versions can overlap during rollout. Please either add a request compatibility normalizer with tests for the previous write bodies, or document and gate a deliberate breaking rollout path that prevents old clients from talking to the new API.

Testing

• Coverage: Insufficient. The PR updates model-only happy paths, but it does not cover previous request shapes for /api/zero/chat-threads, /api/zero/chat-threads/:id/model-selection, or /api/zero/chat/messages.
• Conventions: The added/updated tests otherwise follow the integration-style approach. Latest CI for this head was still running at review time.

[e7h4n]

LGTM

PR_AUTO_REVIEW_COMMIT: 4820478
PR_AUTO_REVIEW_VERDICT: LGTM

Summary

This PR removes modelSelection and modelProvider from chat write request contracts and makes the supported client write shape a single model id. The API preserves deployment compatibility by normalizing legacy modelSelection bodies into model, then internally converts that model into the existing model-first pin flow so org policy/provider admission still chooses the effective provider. CLI and Platform writes now send model-only request bodies, with API contract, API route, Platform, and CLI coverage updated around that behavior.

Findings

Critical (P0)

• None.

High Priority (P1)

• None.

Testing

• Coverage: Adequate. The PR includes contract normalization tests for legacy create/update/clear/send shapes, API BDD coverage for thread create/update/send/provider-policy behavior, Platform tests asserting model-only request bodies, and CLI command coverage asserting { model } is sent.
• Conventions: Compliant. I did not find newly introduced relative internal vi.mock() usage, direct fetch stubbing, fake timers, or database mocking in the changed files. git diff --check origin/main...origin/fix/chat-message-model-only is clean. CI for the new head is still running at review time.

[e7h4n]

LGTM

PR_AUTO_REVIEW_COMMIT: 260c249
PR_AUTO_REVIEW_VERDICT: LGTM

Summary

This PR simplifies chat write contracts so thread create, thread model update, and normal chat send requests accept model instead of the previous modelSelection/modelProvider shape. The server preserves rollout compatibility by normalizing legacy modelSelection bodies to model, then converting model into the existing model-first pin flow internally so provider admission and org policies continue to own provider selection.

Findings

Critical (P0)

• None.

High Priority (P1)

• None.

Testing

• Coverage: Adequate. The diff updates API BDD coverage, CLI request-body coverage, platform chat request helper/UI coverage, and contract compatibility tests for legacy modelSelection request bodies.
• Conventions: Compliant with the vm0 testing guidance and ccstate practices reviewed for this PR. I did not find new internal mocks, direct fetch mocking, fake timers, DB mocking, or signal lifecycle violations in the changed tests/signals.
• Local verification: git diff --check origin/main...origin/fix/chat-message-model-only passed during review; full CI is still running on the new merge head.

[lancy]

Looping in @seven332 and @hulh122 for the runner e2e semantics here.

I checked the history for the affected tests:

• t30-vm0-model-provider-inject.bats: originally added by @lancy in test(e2e): add model-provider credential injection test #2233, ported/maintained in runner e2e by @seven332 in test(e2e): sync 02-parallel tests into 03-experimental-runner #2837, then changed to the model-first provider pin flow by @hulh122 in refactor: make zero model-first only #13017. Current blame is mostly @seven332 / @hulh122.
• t54-vm0-billable-firewall.bats: added by @seven332 in fix(billing): gate model-provider proxy usage to vm0 meta-provider only #10406, updated by @seven332 in test: verify mitmproxy billing e2e #14947 for mitmproxy billing verification, then changed to the model-first provider pin flow by @hulh122 in refactor: make zero model-first only #13017.

My suggested direction is:

1. Keep this PR's product contract: chat write APIs should accept model id only; frontend should not pass provider ids.
2. Do not preserve arbitrary provider-id pins on chat message bodies just to satisfy these e2e tests.
3. Update the e2e tests/helpers so provider-specific assertions are either driven by org model policy in an isolated/serialized setup, or moved to a lower-level runner/CLI path that still intentionally supports explicit provider selection.
4. For legacy modelSelection compatibility, avoid silently dropping a real modelProviderId: either only normalize MODEL_FIRST_SELECTION_PROVIDER_ID legacy payloads, or explicitly reject real provider ids with a clear 400.

Can you confirm whether that direction is acceptable for these tests? cc @e7h4n

[lancy]

Implemented the model-only e2e follow-up discussed in Slack.

Changes in the latest commit:

• Legacy modelSelection compatibility now only accepts the model-first sentinel provider id; concrete provider ids are rejected instead of being silently dropped.
• Added contract regression coverage for create/update/send rejecting concrete provider pins.
• Updated t30 credential injection to use direct vm0 run --model-provider-type claude-code-oauth-token.
• Updated t54 so BYOK non-billable is covered through direct runner provider selection, while vm0 billable stays on chat with { model } only.
• Updated codex zero BYOK smoke to route via org model policy and send only { model } to chat.

Local verification:

• git diff --check
• bash -n on changed e2e helpers/tests
• pnpm -C turbo exec vitest run packages/api-contracts/src/contracts/__tests__/chat-threads.test.ts
• pnpm -C turbo -F @vm0/api-contracts check-types
• pnpm -C turbo -F @vm0/api-contracts lint

[lancy]

Merged latest origin/main into the PR branch and resolved conflicts after the model-only e2e update.

Conflict resolution notes:

• Kept the model-only chat payload shape (model) while adopting main's realAgentInPreview chat flag.
• Kept t30 and the BYOK half of t54 on direct vm0 run --model-provider-type so chat writes no longer pin concrete providers.
• Preserved the existing OpenRouter blank-secret regression test while resolving the main merge conflict.

Post-merge local verification:

• git diff --check
• bash -n on changed e2e helpers/tests
• pnpm -C turbo exec vitest run packages/api-contracts/src/contracts/__tests__/chat-threads.test.ts
• pnpm -C turbo -F @vm0/api-contracts check-types
• pnpm -C turbo -F @vm0/api-contracts lint

[lancy]

Changes Requested

PR_AUTO_REVIEW_COMMIT: 8a04372
PR_AUTO_REVIEW_VERDICT: Changes Requested

Summary

This PR changes chat write APIs so clients send only a selected model id, while the server resolves provider admission from org policy and available credentials. The direction is clear, but the current head still has a platform compile break and a deployment-compatibility regression for legacy provider-pinned requests.

Findings

Critical (P0) - must fix before merge

• turbo/apps/platform/src/signals/chat-page/optimistic-chat-thread-page.ts:315: modelSelectionRequestFromSelection is still called when creating a new fast-tier thread, but this PR removed the helper/import from model-selection-request.ts. This leaves an unresolved identifier in @vm0/app and should fail check-types. The fast-tier follow-up update should send the new body shape, for example model: args.modelSelection.selectedModel plus codexServiceTier, or otherwise use an existing helper that still exists.

High Priority (P1) - should fix before merge

• turbo/packages/api-contracts/src/contracts/chat-threads.ts:404: legacy modelSelection bodies are only normalized when modelProviderId is the model-first sentinel; concrete provider-pinned legacy requests are intentionally rejected by the new contract tests at turbo/packages/api-contracts/src/contracts/__tests__/chat-threads.test.ts:89. That breaks old loaded frontends / older CLI clients during rollout whenever they send the previous { modelSelection: { modelProviderId, selectedModel } } shape for BYOK or explicit provider flows. The testing standards require old/new request compatibility for deployable API contracts when old clients can still talk to new backend. If provider pins must be removed, this needs either a staged compatibility path that maps legacy provider-pinned bodies to the new model-only flow or a documented rollout/rationale with coverage showing the old clients cannot hit this path.

Testing

• Coverage: Insufficient. The PR adds contract compatibility tests for legacy model-first requests, but the provider-pinned legacy path is covered as rejection rather than a compatible old-client rollout path.
• Conventions: No new testing-convention violations found in the reviewed diff; the blocking issue is correctness/type safety plus deployment compatibility.

[lancy]

LGTM

PR_AUTO_REVIEW_COMMIT: 776ef9d
PR_AUTO_REVIEW_VERDICT: LGTM

Summary

This PR removes modelSelection/modelProvider from chat write request contracts and moves public chat create/update/send requests to model ids. The server keeps legacy request compatibility by normalizing old modelSelection payloads into model, then resolving the effective provider through model policy/provider admission.

Findings

No P0/P1 blockers found.

Testing

• Coverage: Adequate. The diff includes contract compatibility tests for legacy model-first and provider-pinned bodies, API route coverage for model-first chat/thread behavior, and platform/CLI assertions that new write requests send model rather than modelSelection.
• Conventions: Compliant with the reviewed testing standards. I did not find new internal vi.mock("../..."), direct fetch stubbing, fake timers, or DB mocking in the changed tests.
• Verification observed on the latest conflict-resolution commit: pre-commit passed with prettier, Rust checks, check-types, and commitlint; NODE_OPTIONS=--max-old-space-size=4096 pnpm check-types also passed manually after the initial local hook timeout.

[lancy]

LGTM

PR_AUTO_REVIEW_COMMIT: 51f53f5
PR_AUTO_REVIEW_VERDICT: LGTM

Summary

This PR removes modelSelection and modelProvider from the public chat write request bodies in favor of a single model field, while preserving a legacy preprocess path that accepts old modelSelection payloads and normalizes them to model-first requests. The server continues resolving provider admission through org model policy, and the platform/CLI/e2e callers now send model ids instead of provider pins.

The latest head also replaces the removed --debug-no-mock-claude E2E flag with --real-agent-in-preview in t54-vm0-billable-firewall.bats. I confirmed the replacement flag exists on vm0 run and maps to the same runner behavior: suppressing USE_MOCK_CLAUDE so the real Claude runtime is used in preview/E2E.

Findings

Critical (P0)

• None.

High Priority (P1)

• None.

Testing

• Coverage: Adequate. Contract tests cover model-only request bodies and legacy modelSelection normalization, including concrete-provider legacy payloads. API/platform tests cover model-first sends, thread creation, thread model updates, invalid model rejection, provider-policy admission, and the test-only provider-secret state helper. E2E coverage now exercises BYOK/model-policy chat flows with model payloads and the corrected real-agent flag.
• Conventions: Compliant. The new model-provider state helper is exposed through a test-only route gated by the existing isTestEndpointAllowed path and used via HTTP/app routes from BDD tests rather than direct DB access in test bodies. I did not find new forbidden internal mocks, fetch stubs, fake timers, or DB service mocks in the changed test surface.
• Local execution: Not rerun during this review pass. This review is based on the current diff, prior pushed verification already recorded on the PR, and inspection of the affected contract/API/platform/CLI/e2e paths at head 51f53f55f83e0b533e3217d0d881fa44392aa27e.