Dragan Spiridonov 1aac5206ba fix(adr-093): address devil's-advocate C1-C4 critical findings ...

Four merge-blocking issues surfaced by the qe-devils-advocate audit.

C1 — effort-resolver YAML loading was silently broken in ESM
  src/shared/llm/effort-resolver.ts: replaced
  const { readFileSync } = require('fs') with a top-level
  import { readFileSync } from 'fs'. The require() call threw
  ReferenceError under ESM ("type": "module" in package.json),
  caught by the bare try/catch, so the YAML priority tier in the
  resolver chain was dead — callers always fell through to the
  hardcoded xhigh default. config/fleet-defaults.yaml was a
  decorative file. Proven fixed by runtime check:
    YAML set to 'medium' → resolveEffortLevel() now returns 'medium'.

C2 — body.effort on every /v1/messages call was unverified shape
  src/shared/llm/providers/claude.ts: the previous code
  unconditionally set body.effort on every request, regardless of
  whether the target model supported it, and used a top-level shape
  that is not documented in Anthropic's public Messages API schema.
  New behavior: effort is only set when the registry marks the
  model supportsEffortXHigh=true (Opus 4.7 today) and is nested
  under body.thinking = { type: 'adaptive', effort: <level> },
  matching the Anthropic SDK's beta.messages thinking shape.
  Non-xhigh models receive no thinking/effort field at all,
  eliminating the risk of 400-ing Sonnet 4.6 / Haiku 4.5 calls.
  Rationale inline at the call site.

C3 — CYBER_PINNED_AGENTS was missing qe-security-reviewer
  qe-security-reviewer lives at .claude/agents/v3/subagents/
  (not in .claude/agents/v3/ top level) and was present in the
  submitted Cyber Verification application. Without it in the pin
  list, that agent could route to Opus 4.7 freely. Added.
  Cross-check test now parses the application file and asserts
  every CYBER_PINNED_AGENTS entry is named in it — future edits
  to either side cannot silently diverge.

C4 — cyber pin only fired in MultiModelExecutor.consult()
  Direct HybridRouter.chat() calls from security agents bypassed
  the pin. Extracted the pin helpers into src/routing/security/
  cyber-pin.ts (applyCyberPin, shouldCyberPin, isOpus47,
  CYBER_PINNED_AGENTS, CYBER_PIN_ADVISOR_FALLBACK,
  CYBER_PIN_CHAT_FALLBACK). HybridRouter.chat() now applies the
  pin post-selectProvider, before executeWithFallback, on both
  decision.model and decision.providerModelId — covering
  canonical, OpenRouter (dotted), and Bedrock ARN forms.
  MultiModelExecutor.consult() imports from the same module so
  both paths share one source of truth.

Tests moved tests/routing/advisor/cyber-pin.test.ts →
tests/routing/security/cyber-pin.test.ts and expanded: 19 tests
covering all 4 pinned agents × multiple model-id forms, the
application cross-check, env-flag semantics, non-security
passthrough, and non-4.7 model passthrough.

Verification:
* 0 tsc errors
* 128/128 targeted tests pass (effort-resolver + cyber-pin + advisor + providers)
* 2927/2927 broader regression pass (shared/llm + coordination + routing)
* Runtime proof of C1 fix: YAML 'medium' → resolveEffortLevel() returns 'medium'

Still outstanding from the devil's-advocate audit (deferred to
follow-up, not merge-blocking):
* C5 — Bedrock/OpenRouter model IDs need empirical verification
* C6 — no integration test hits real /v1/messages with 4.7
* H1–H5, M1–M5, L1–L3

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

2026-04-17 16:31:24 +00:00

..

advisor

fix(adr-093): address devil's-advocate C1-C4 critical findings

2026-04-17 16:31:24 +00:00

escalation

fix(tests): add afterEach cleanup to 104 test files missing it (#374 P2)

2026-03-21 12:51:47 +00:00

security

fix(adr-093): address devil's-advocate C1-C4 critical findings

2026-04-17 16:31:24 +00:00