test(eval): pin credential and MCP launch contracts

[kovtcharov]

Agent evals now have regression coverage for the launch configuration that previously broke authentication: API-key/OAuth combinations, bare-mode selection, MCP configuration, and working directory. A real launcher smoke test also catches missing server imports before an eval starts.

Fixes #3569.

Test plan:

• Seven new launch-contract tests passed, including a subprocess running the configured MCP launcher with --help.
• Existing scenario subprocess suite: 21 passed.
• Black, isort, whitespace checks, and independent review passed.

These checks do not make live authenticated requests or run model evaluations.

[github-actions]

Verdict: Approve

This adds regression tests for the eval launcher's credential handling — the thing that silently broke authentication for subscription-only users — plus a smoke test that actually starts the configured MCP server. Tests only, no production code touched. Nothing blocking.

The one gap worth knowing about: the smoke test launches the MCP server with the interpreter pytest is already running under, not the one the eval config actually names. So if that interpreter goes missing from the machine's search path, the eval breaks at spawn time and this test still passes green. The import chain it set out to check is covered correctly; it's the launch command itself that isn't.

Real-world evidence

N/A — tests-only change, no user-facing surface. No evidence-bundle.md was produced for this PR, and the reviewer environment has no pytest or GAIA dependencies installed, so I could not execute the suite here; the verdict rests on static review. The PR description reports 7 new tests passing plus 21 existing ones, which is the right evidence for a test-only change.

🔍 Technical details

Issues

🟢 Smoke test ignores the config's command and env fields (tests/unit/test_eval_launch_contract.py:79-89)

The test reads server["args"] out of eval/mcp-config.json but launches with sys.executable, while the config specifies "command": "python". A host where python resolves to nothing (Windows without the launcher alias, or a python3-only container) fails the real eval at spawn while this test stays green. The env block (GAIA_MEMORY_MCP_ALWAYS, GAIA_MEMORY_ADMIN) is likewise unasserted — harmless for --help, but it means the config is only half-pinned. One extra assertion closes the gap:

    launcher = runner.REPO_ROOT / server["args"][0]
    assert launcher.is_file()
    assert "--stdio" in server["args"]
    assert shutil.which(server["command"]), f"config command not on PATH: {server['command']}"
    monkeypatch.setenv("GAIA_MCP_LOG_DIR", str(tmp_path))
    result = subprocess.run(
        [sys.executable, str(launcher), *server["args"][1:], "--help"],

(requires import shutil at the top.)

🟢 File placement diverges from the existing eval test package (tests/unit/test_eval_launch_contract.py:1)

tests/unit/eval/ already holds test_runner.py and 18 sibling eval tests. This file lands one level up. Not a correctness problem, but tests/unit/eval/test_launch_contract.py is where someone grepping for eval runner coverage will look.

🟢 The oauth_token parametrize drives no distinct assertion (tests/unit/test_eval_launch_contract.py:20)

It doubles the matrix to pin an unstated contract — that OAuth presence must not alter the command. Worth one short comment saying so, otherwise a future reader is likely to delete it as redundant.

Notes (not findings)

• test_configured_mcp_launcher_imports_real_server spawns a real subprocess that imports the full gaia.mcp.servers.agent_ui_mcp chain — integration-flavoured for tests/unit/, but it stays dependency-light because the mcp.server import there is TYPE_CHECKING-only, so --help works without the mcp extra. The 30s timeout should hold on a warm runner; watch it on cold Windows.
• Subprocess inheritance of this checkout is safe here — the root conftest.py exports the pinned source roots onto PYTHONPATH (conftest.py:70-72), so the spawned interpreter resolves gaia from this branch, not an editable install pointing elsewhere.

Strengths

• assert ("--bare" in command) == bool(api_key) is the right shape for this bug: it pins empty-string ANTHROPIC_API_KEY as equivalent to unset, which is exactly the falsy-vs-present distinction runner.py:959 relies on and the kind of thing a naive is not None check would have missed.
• assert "env" not in run.call_args.kwargs with its one-line comment pins the actual regression — that the subprocess must inherit the parent's credentials — rather than just re-asserting the argv it can see.
• Redirecting GAIA_MCP_LOG_DIR to tmp_path keeps the launcher from writing eval-out/mcp-server.err.log into the repo during a unit run. Easy to miss; good catch.
• The smoke test exercises the real launcher end to end, so a broken module-level import in agent_ui_mcp surfaces in CI instead of as a CONNECTION_CLOSED three hours into an eval.

[github-actions]

Verdict: Approve

Two new tests that pin the credential-selection and MCP-launch contracts for gaia eval agent. No prior Claude review was found (GitHub API unavailable in this runner environment), so this is a first-pass full review.

The logic is correct and well-matched to the implementation:

• ("--bare" in command) == bool(api_key) correctly mirrors os.environ.get("ANTHROPIC_API_KEY") in runner.py:959 — empty-string and None are both falsy, so the three parametrize values exercise the right branches.
• "env" not in run.call_args.kwargs pins that credentials are inherited from the parent process rather than scrubbed and re-passed — the important invariant for OAuth users.
• test_configured_mcp_launcher_imports_real_server intentionally uses a real subprocess; the inline comment explains why (mocking would mask a broken spawn). Placement in tests/unit/ is a mild categorisation mismatch (tests/mcp/ would be more accurate), but the rationale is sound and not worth blocking.

No correctness bugs, no security issues, no missing tests for new logic (this PR is the tests). Clean to merge.