fix(router,agent,mcp): 3 high-impact audit findings + tests

[rollroyces]

Summary

Static audit of src/ surfaced three high-impact bugs and three test-coverage gaps in the routing, agent-recovery, and MCP-startup paths. All fixes come with regression specs that were zero-coverage before this PR.

Changes

Bug fixes

1. src/router/scenario/subagentDetector.ts — main agent misclassified as subagent when its tools omit the Agent/Task tool.
   The missingAgentTool signal was OR-ed into isSubagent. A main agent that customized its tool registry (e.g. drops agent/task in favor of domain tools) was reclassified as a subagent, changing model selection, the fallback chain, and (via RouterRuntime line 700, if (decision.isSubagent && config.autoOrchestrate?.subagentMaxTokens)) potentially terminating the request with subagent_budget_exceeded. Only the explicit <pilotdeck-subagent-model> / <ccr-subagent-model> tag in a user message is strong enough to override isMainAgent.

2. src/agent/loop/AgentLoop.ts — Phase A token-doubling recovery burns its single-shot guard on a no-op bump.
   hasAttemptedOutputRetry = true was assigned at the top of the Phase A block, before checking whether resolveOutputTokenRetryBump returned a value. When the bump is impossible (already at model cap, no model cap known), the guard was still consumed, so the next recovery attempt skipped Phase A entirely. Moved the assignment inside the success branch.

3. src/mcp/runtime/McpRuntime.ts — non-Error throws produce error: undefined in the startup status entry.
   (err as Error).message returns undefined for string / null / undefined / plain-object throws. Coerce via String(err) so downstream UI never shows "unknown error" for an otherwise-recoverable MCP startup failure.

Test coverage (was zero before this PR)

• tests/router/scenario/subagentDetector.spec.ts — 6 cases pinning the contract (main agent with/without Agent tool, subagent, explicit tag, CCR tag, tool-name regex variants).
• tests/router/health/ProviderHealthTracker.spec.ts — 9 cases for the per-session circuit breaker (degrade/open thresholds, half_open probe protocol, success/failure closure, window success rate, reset, snapshot).
• tests/mcp/runtime/McpRuntime.spec.ts — 4 cases for error coercion across Error, McpClientError, string, null, and undefined throws.

CI / local verification

Rebased onto current main (2df7127) — auto-merged cleanly with zero conflicts (upstream changes since the original branch point are entirely in ui/ and docs/; my changes are in src/ + tests/). Local pnpm build is green against the rebased branch.

The upstream GitHub Actions CI matrix also runs cleanly against this branch (re-run after rebase, all green):

Check	Result	Duration
static-checks (Desktop Smoke)	✅ pass	36s
docker (Web Regression)	✅ pass	2m8s
web (Web Regression, pnpm run build:web + Vitest)	✅ pass	3m14s
windows-installer / build	✅ pass	5m52s

The Vitest Web suite that gates the merge reports 197 test files, 1675 tests, all passing against the PR head — my new specs are inside that count.

All 19 new tests pass:

ok 329 - McpRuntime.start: status entry uses the McpClientError message when it surfaces
ok 330 - McpRuntime.start: Error-subclass throws carry their message through
ok 331 - McpRuntime.start: non-Error throws fall back to String(err)
ok 332 - McpRuntime.start: non-Error throws of null/undefined surface as 'null' / 'undefined'
ok 462 - ProviderHealthTracker: defaults to healthy for unknown providers
ok 463 - ProviderHealthTracker: degrades after degradeThreshold consecutive failures
ok 464 - ProviderHealthTracker: opens after openThreshold consecutive failures
ok 465 - ProviderHealthTracker: open → half_open after openDurationMs
ok 466 - ProviderHealthTracker: a probe success from half_open closes the circuit
ok 467 - ProviderHealthTracker: a probe failure re-opens the circuit
ok 468 - ProviderHealthTracker: getSuccessRate is the trailing window ratio
ok 469 - ProviderHealthTracker: snapshot returns every observed provider
ok 470 - ProviderHealthTracker: reset clears a single provider; resetAll clears all
ok 471 - detectSubagent: main agent with custom toolset (no Agent tool) is NOT misclassified as subagent
ok 472 - detectSubagent: main agent with the Agent tool stays main
ok 473 - detectSubagent: actual subagent (isMainAgent=false) is always subagent
ok 474 - detectSubagent: explicit subagent tag in user message reclassifies main agent
ok 475 - detectSubagent: CCR-style tag is also accepted
ok 476 - detectSubagent: spawned_agent / launch-agent / spawn_agent name patterns also satisfy the heuristic

The 7 cancelled / 2 skipped tests in pnpm test are pre-existing baseline noise unrelated to this PR (the upstream CI comment in .github/workflows/web-regression.yml notes a known flaky streamSmoother spec and a Playwright E2E file that's excluded from the merge gate).

Reviewers can reproduce locally with:

pnpm install --frozen-lockfile
pnpm build
pnpm test

Risk

Low. All three changes are local: one boolean expression, one assignment re-ordering, one ternary chain. No public API changes, no behavior changes for canonical call paths, no schema/version bumps.

Diffstat

 src/agent/loop/AgentLoop.ts                       |  48 +++++----
 src/mcp/runtime/McpRuntime.ts                     |  11 +-
 src/router/scenario/subagentDetector.ts           |   8 +-
 tests/mcp/runtime/McpRuntime.spec.ts              |  97 ++++++++++++++
 tests/router/health/ProviderHealthTracker.spec.ts | 117 ++++++++++++++++++
 tests/router/scenario/subagentDetector.spec.ts    |  78 +++++++++++
 6 files changed, 336 insertions(+), 23 deletions(-)