refactor: extract generic callAndUnmarshal helper in internal/adapter/

[yvonnedevlinrh]

Summary

Extracts a generic callAndUnmarshal[T any] helper in internal/adapter/ to centralize the repeated JSON-RPC Call → error-check → Unmarshal pattern.

Split from #201; closes #237.

• New unexported callAndUnmarshal[T any](ctx, client, method, params) (T, error) in internal/adapter/call.go.
• Migrated 3 hard-error batch call sites: ExternalComplexityProvider.Analyze ([protocol.ComplexityResult]), ExternalLineCoverageProvider.Coverage ([protocol.CoverageResult]), ExternalSideEffectAnalyzer.loadBatch
  ([protocol.AnalyzeResult]).
• Error-chain contract: transport/unmarshal wrap with %w (errors.Is/errors.As preserved); protocol errors format with %s (structured JSON-RPC error object, not a Go error chain). Because the method constants are literally "complexity"/"coverage"/"analyze", the helper reproduces the exact legacy error strings.

Intentional exclusions

• Session.Initialize retains inline per-branch handling — its three distinct legacy error strings (initialize handshake / initialize error / parsing initialize result) cannot be reproduced by the collapsed generic helper without fragile string translation. Preserving exact operator-facing strings was prioritized (design D2). Effective migration is 3 sites, not 4.
• fetchTestMappings (graceful-degradation p.warn() path, D3), CallStream/ streaming, and internal/protocol/ are untouched.

How to Test

go build ./... && go test -race -count=1 -short ./... && golangci-lint run

All green. New helper: CRAP 4.0 @ 100% line coverage; package CRAPload 0. Existing adapter integration tests pass unmodified (behavioral equivalence).

How to Demo

go test -race -count=1 -run TestCallAndUnmarshal ./internal/adapter/

Table-driven TestCallAndUnmarshal covers 5 subtests: Success, TransportError, ProtocolError, UnmarshalFailure, GenericInstantiation.

Key Files Changed

internal/adapter/call.go | 51 +++++ (new helper)
internal/adapter/call_test.go | 228 +++++ (new tests)
internal/adapter/complexity.go | 14 +-
internal/adapter/coverage.go | 14 +-
internal/adapter/sideeffect.go | 12 +-
AGENTS.md | 1 +
openspec/changes/adapter-call-unmarshal-helper/* (spec artifacts)
.uf/dewey/learnings/adapter-call-unmarshal-helper-*.md (3 retrospective notes)
14 files changed, 831 insertions(+), 34 deletions(-)

Known Issues

• LOW, accepted call_test.go builds its own fake-analyzer copy via sync.Once (only one TestMain allowed per test binary, and the existing one lives in package adapter_test). The os.MkdirTemp dir isn't cleaned up —
  t.Cleanup can't be used because sync.Once.Do captures the first subtest's *testing.T and would delete the shared binary mid-run. This leaves a ~4–5MB binary in OS temp space. All 5 review-council reviewers rated this non-blocking.

This PR was generated by /uf.finale (AI-assisted).

[jflowers]

PR Review: #239 — refactor: extract generic callAndUnmarshal helper in internal/adapter/

Verdict: APPROVE

Clean, well-scoped DRY refactoring that extracts a generic callAndUnmarshal[T any] helper to centralize the JSON-RPC call pattern across 3 adapter sites. Error-chain semantics are correctly preserved (%w for transport/unmarshal, %s for protocol errors), the test suite achieves full branch coverage with 5 subtests, and all CI checks pass.

Findings

[MEDIUM] Spec MUST clause inconsistency with Session.Initialize exclusion

The spec (specs/adapter/spec.md) requires Session.Initialize migration via a MUST clause, and issue #237 states "All 5 batch call sites use the helper." The implementation migrates 3 sites and intentionally excludes Initialize (design D2 documents the rationale — legacy error string divergence). The code is correct, but the spec artifact shipped in the same PR contradicts the implementation. A follow-up to relax the spec MUST clause would resolve this inconsistency.

No Issues Found

• Security: Pure internal DRY refactoring — no new inputs, subprocess paths, credential handling, or exported API surface.
• Constitution Compliance: Principle IV (Testability) met with full branch coverage. Principle I (Accuracy) met — error strings preserved via method constant identity.
• CI: All 5 checks pass (Unit+Integration Go 1.24/1.25, E2E Go 1.24/1.25, MegaLinter).

This review was generated by /uf.review-pr (AI-assisted).