Context
During the /review-council for PR #722 (doctor-grouped-output), the Divisor agents flagged several pre-existing issues in internal/doctor/ and cmd/complyctl/cli/doctor.go that were not introduced by the PR but surfaced during review.
Findings
1. registryErrors cache bypassed by resolvePinnedFallback (SRE HIGH)
File: internal/doctor/doctor.go
The registryErrors cache (line 370) prevents redundant ResolveLatestVersion calls to unreachable registries. However, when a cached error is found, resolvePinnedFallback still calls resolver.ResolveVersion() -- making a new network call to the same unreachable registry.
With N pinned policies from one unreachable registry: the first policy triggers 2 timeouts (latest + pinned), each subsequent triggers 1 timeout (pinned fallback). For 5 policies at 5s timeout = 30s of wasted waits.
Fix: Extend the registryErrors cache to be consulted inside resolvePinnedFallback, or skip the pinned resolution attempt when the error is a known network failure (not a 404).
2. CheckVariables complexity 38, 211 lines (Architect MEDIUM)
File: internal/doctor/doctor.go:580-791
CheckVariables handles four responsibilities: evaluator-target resolution, global variable validation, target variable validation, and verbose detail generation. Gaze flags it as Q4 Dangerous (complexity 38, GazeCRAP 38).
Fix: Extract resolveEvaluatorTargets() and validateProviderVariables() helpers. Each would be under 50 lines (CS-010).
3. Run() has 8 parameters (Architect MEDIUM)
File: internal/doctor/doctor.go:117
The signature exceeds the 99-character line limit and violates AP-001 (Options struct pattern).
Fix: Introduce a RunOptions struct to bundle the parameters.
4. printDiagnostics writes to stdout, not io.Writer (Architect LOW)
File: cmd/complyctl/cli/doctor.go:108-167
Uses fmt.Println/fmt.Printf directly instead of accepting io.Writer per AP-003 (testable CLI pattern). This will be naturally addressed when #611 (machine-readable output) is implemented.
Fix: Accept io.Writer parameter, use fmt.Fprintln/fmt.Fprintf. Likely addressed alongside #611.
Priority
Medium -- the registry cache bypass (#1) has real user impact for offline/air-gapped environments. The complexity items (#2-#4) are structural improvements.
Related
LLM-assisted: yes
Context
During the
/review-councilfor PR #722 (doctor-grouped-output), the Divisor agents flagged several pre-existing issues ininternal/doctor/andcmd/complyctl/cli/doctor.gothat were not introduced by the PR but surfaced during review.Findings
1.
registryErrorscache bypassed byresolvePinnedFallback(SRE HIGH)File:
internal/doctor/doctor.goThe
registryErrorscache (line 370) prevents redundantResolveLatestVersioncalls to unreachable registries. However, when a cached error is found,resolvePinnedFallbackstill callsresolver.ResolveVersion()-- making a new network call to the same unreachable registry.With N pinned policies from one unreachable registry: the first policy triggers 2 timeouts (latest + pinned), each subsequent triggers 1 timeout (pinned fallback). For 5 policies at 5s timeout = 30s of wasted waits.
Fix: Extend the
registryErrorscache to be consulted insideresolvePinnedFallback, or skip the pinned resolution attempt when the error is a known network failure (not a 404).2.
CheckVariablescomplexity 38, 211 lines (Architect MEDIUM)File:
internal/doctor/doctor.go:580-791CheckVariableshandles four responsibilities: evaluator-target resolution, global variable validation, target variable validation, and verbose detail generation. Gaze flags it as Q4 Dangerous (complexity 38, GazeCRAP 38).Fix: Extract
resolveEvaluatorTargets()andvalidateProviderVariables()helpers. Each would be under 50 lines (CS-010).3.
Run()has 8 parameters (Architect MEDIUM)File:
internal/doctor/doctor.go:117The signature exceeds the 99-character line limit and violates AP-001 (Options struct pattern).
Fix: Introduce a
RunOptionsstruct to bundle the parameters.4.
printDiagnosticswrites to stdout, notio.Writer(Architect LOW)File:
cmd/complyctl/cli/doctor.go:108-167Uses
fmt.Println/fmt.Printfdirectly instead of acceptingio.Writerper AP-003 (testable CLI pattern). This will be naturally addressed when #611 (machine-readable output) is implemented.Fix: Accept
io.Writerparameter, usefmt.Fprintln/fmt.Fprintf. Likely addressed alongside #611.Priority
Medium -- the registry cache bypass (#1) has real user impact for offline/air-gapped environments. The complexity items (#2-#4) are structural improvements.
Related
doctorhas no machine-readable output mode for CI #611: machine-readable doctor output (would address Add the codeowners file #4)LLM-assisted: yes