fix(tests): make ACP model selection resilient

[Josh Duffney (duffney)]

Summary

Closes #1439.

• select a non-default model from the ACP session's advertised models or model configOptions instead of hard-coding a server-controlled model ID
• resolve session-aware selectors only after newSession, while preserving fixed TEST_MODEL overrides
• report the initial and confirmed models as typed test data so the integration assertion proves the model changed
• keep the Flask coding prompt on the default model and exercise switching in a separate short authenticated probe
• distinguish missing capability/no alternate candidate from an attempted ACP model-selection failure

Follow-up commits

• 14a3e2ac — expose typed model-selection state and consolidate capability detection
• 53f6d643 — isolate model selection from the coding prompt and preserve TEST_MODEL precedence
• cd2c166c — support non-default models advertised through typed ACP configOptions

Testing

• pnpm exec vitest run apps/workers/coder-acp-copilot/src/acp-client.test.ts packages/test-utils/src/harness.test.ts packages/test-utils/src/run-test-worker.test.ts — 57 tests passed
• pnpm --filter test-utils build — passed
• pnpm --filter coder-acp-copilot build — passed
• Copilot container integration attempted without GITHUB_TOKEN; the unchanged Docker bootstrap failed before compilation because both Corepack and npm hit a registry.npmjs.org TLS ECONNRESET, so the authenticated coding/model probes could not run locally

Documentation and compatibility

No user-facing API, CLI, Portal, database, or deployment behavior changes. Inline integration-harness documentation now covers dynamic probe selection and TEST_MODEL precedence.

[github-actions]

Test Results (Node.js 22)

test: Run #116

Tests 📝	Passed ✅	Failed ❌	Skipped ⏭️	Pending ⏳	Other ❓	Flaky 🍂	Duration ⏱️
3011	3011	0	0	0	0	0	1m18s

🎉 All tests passed!

Github Test Reporter

[github-actions]

Test Results (Node.js 22)

test: Run #120

Tests 📝	Passed ✅	Failed ❌	Skipped ⏭️	Pending ⏳	Other ❓	Flaky 🍂	Duration ⏱️
3011	3011	0	0	0	0	0	1m20s

🎉 All tests passed!

Github Test Reporter

[Cedric Vidal (cedricvidal)]

Good direction. Choosing a model from the ones the session offers is the right fix for the hard-coded claude-opus-4.6. Resolving the selector after newSession also makes sense, since the model list is per session. A few things I'd like to see before merging:

1. The success-path check can't fail.
confirmedModel is only set on the same code path that logs Model set to "X" via session/set_model. So once confirmedModel is defined, checking for that log line always passes. It doesn't show that the model actually changed. runACPSession already logs Session models: current=<id>, available=[…]. Suggest reading current from that line and asserting first.confirmedModel !== current.

2. The prompt tests now run on an arbitrary model.
The selector also applies to the real prompt sessions. The Flask prompt and the session-reuse check therefore run on whatever non-default model the server lists first. That can change on the server side at any time (preview, premium or rate-limited models), which could cause flaky failures that have nothing to do with model switching. Options:

• Pick a model deterministically: prefer a known allow-list and fall back to the first non-default.
• Or test model switching in a separate short session and keep the prompt runs on the default model.

3. TEST_MODEL is silently overridden.
test-worker.ts does { ...options, model: selectFirstAvailableNonDefaultModel }. That overwrites the TEST_MODEL value the harness passes in, even though harness.ts still documents TEST_MODEL. Suggest model: options.model ?? selectFirstAvailableNonDefaultModel so it still works as a manual override.

4. The selector ignores models advertised through configOptions.
selectFirstAvailableNonDefaultModel only reads sessionResult.models. If an agent lists its models only through a config option with category: "model", the selector returns undefined and the test quietly takes the skip path. Fine for now, but worth documenting in the selector or handling there.

5. Nit: duplicated capability check, and a test-only option in production code.
The new else if (typeof model === "function") branch in runACPSession repeats the capability check and warning that selectModel already has. A small shared helper, or a single "selector returned no model" warning, would stop the two from drifting apart. ACPModelSelector is also a test-motivated option on the production ACPClientOptions. That's acceptable, but a short note saying what it's for would help.

Checked locally: the acp-client unit tests pass and coder-acp-copilot builds. I didn't run the Docker integration test.

[Josh Duffney (duffney)]

Addressed all five points in three follow-up commits:

1. Non-tautological success assertion (14a3e2ac): runACPSession now returns typed initialModel state through test-utils; the integration test asserts confirmedModel !== initialModel instead of checking the success log produced by the same branch.
2. Prompt stability (53f6d643): the Flask coding run stays on the session default. Dynamic switching runs in a separate short Reply with OK. container session used only by the model-selection assertion.
3. TEST_MODEL precedence (53f6d643): an explicit harness model now wins; the dynamic selector is used only when TEST_SELECT_NON_DEFAULT_MODEL=true and no TEST_MODEL is set.
4. configOptions support (cd2c166c): the selector now handles both flat and grouped model config options, with models-field precedence and typed unit coverage.
5. Capability cleanup/docs (14a3e2ac): capability detection is shared, selector intent is documented, and mechanism-specific assertion wording was removed.

Validation: 57 focused tests pass; test-utils and coder-acp-copilot builds pass. I retried the Docker integration, but the unchanged pnpm bootstrap still hit an external registry.npmjs.org ECONNRESET before compilation, and no GITHUB_TOKEN is available locally for the authenticated probes.

[github-actions]

Test Results (Node.js 22)

test: Run #122

Tests 📝	Passed ✅	Failed ❌	Skipped ⏭️	Pending ⏳	Other ❓	Flaky 🍂	Duration ⏱️
3020	3020	0	0	0	0	0	1m25s

🎉 All tests passed!

Github Test Reporter

[Cedric Vidal (cedricvidal)]

LGTM

[Josh Duffney (duffney)]

Copilot resolve the merge conflicts in this pull request

[Copilot]

Copilot resolve the merge conflicts in this pull request

Resolved the merge conflict and merged current main in 0e15126. The ACP model-selection probe and its failure assertion are preserved. Diff checks pass; local tests were blocked by DNS resolution of a package-feed URL in the lockfile.

[github-actions]

Test Results (Node.js 22)

test: Run #144

Tests 📝	Passed ✅	Failed ❌	Skipped ⏭️	Pending ⏳	Other ❓	Flaky 🍂	Duration ⏱️
3171	3171	0	0	0	0	0	1m26s

🎉 All tests passed!

Github Test Reporter