Skip to content

[deep-report] Restore defer recover() convention in 2 goroutines at pkg/cli/mcp_inspect_inspector.go #33346

@github-actions

Description

@github-actions

Problem

Two production goroutines in pkg/cli/mcp_inspect_inspector.go skip the codebase's documented defer recover() panic-isolation convention:

  • pkg/cli/mcp_inspect_inspector.go:139-144 — monitors serverCmd.Wait() for each MCP server; only defer wg.Done() on the stack.
  • pkg/cli/mcp_inspect_inspector.go:201-204 — wraps wg.Wait(); close(done) cleanup signal.

Neither is panic-prone today, but both deviate from the convention modeled by pkg/cli/update_check.go:275 (and now followed by 4 of the 6 prod goroutines).

Why This Matters

Convention drift, not current crash risk. A future maintainer who adds work inside either closure may not notice the missing defer recover() guard, and a panic inside an MCP-server monitor goroutine would terminate the whole gh aw mcp-inspect process instead of being logged and contained.

Proposed Fix

Add defer recover() to both goroutine bodies. Match the existing pattern in pkg/cli/update_check.go:275.

Optional follow-up: extract a SafeGo(log, name, fn) helper to eliminate the boilerplate at all 6 sites — but that's out of scope for this quick win.

Acceptance Criteria

  • Both goroutines have defer blocks that recover from panics and log them.
  • No behavior change in the happy path.

Suggested agent / effort

Copilot — Tiny (~12 line diff in 1 file).

Source

DeepReport intelligence run, 2026-05-19. See discussion #33212 Sergo Run 13 finding [L1].

Generated by 🔬 DeepReport - Intelligence Gathering Agent · ● 23.5M ·

  • expires on May 21, 2026, 4:03 PM UTC

Metadata

Metadata

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions