You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
execcommandwithoutcontext enforce-readiness: propagate context in connectStdioMCPServer (2 sites), add nolint support, then enfo
[Content truncated due to length] #38282
The new execcommandwithoutcontext analyzer (24th custom linter) landed fully documented (doc.go "All 24", README, and spec_test.godocumentedAnalyzers() all at 24 entries) but is not yet CI-enforced. A repo-wide audit shows it is close to enforceable: of ~90 prod exec.Command sites across 30 files, only 3 sit inside context-receiving functions — 2 genuine violations plus 1 false positive (tracked separately as the nil-guard precision issue, #38281). This issue covers converting the 2 genuine sites and the remaining prerequisites for enforcement.
Genuine violations (safe to convert)
Both are in connectStdioMCPServer(ctx context.Context, ...), which is always called with a real, non-nil context (context.WithTimeout(context.Background(), 30*time.Second) in connectToMCPServer), so propagating ctx is correct and binds the MCP subprocess lifetime to the 30s timeout:
The *exec.Cmd is handed to mcp.CommandTransport; binding it via exec.CommandContext(ctx, ...) ensures the spawned MCP server process is terminated when the 30s timeout (or any future cancellation) fires, instead of leaking until it exits on its own.
Add internal/nolint support — execcommandwithoutcontext does not import pkg/linters/internal/nolint (unlike the 12 enforced linters), so //nolint/redaction directives are silently ignored. Wire nolint.BuildLineIndex + per-diagnostic suppression as the other linters do, to match parity.
Convert the 2 genuine sites above.
Enforce: append -execcommandwithoutcontext to LINTER_FLAGS in .github/workflows/cgo.yml:1122 (currently 12 flags), making it the 13th enforced linter.
Recommendation
Convert the 2 connectStdioMCPServer sites in this issue. Track the FP fix in #38281 and the nolint parity + CI flag as the final enabling steps (do not enforce until both #38281 and nolint support land).
Validation checklist
mcp_inspect_mcp.go:150 and :155 use exec.CommandContext(ctx, ...).
MCP inspect still connects to stdio/docker servers (manual smoke or existing tests).
execcommandwithoutcontext imports internal/nolint and honors //nolint directives.
Summary
The new
execcommandwithoutcontextanalyzer (24th custom linter) landed fully documented (doc.go"All 24", README, andspec_test.godocumentedAnalyzers()all at 24 entries) but is not yet CI-enforced. A repo-wide audit shows it is close to enforceable: of ~90 prodexec.Commandsites across 30 files, only 3 sit inside context-receiving functions — 2 genuine violations plus 1 false positive (tracked separately as the nil-guard precision issue, #38281). This issue covers converting the 2 genuine sites and the remaining prerequisites for enforcement.Genuine violations (safe to convert)
Both are in
connectStdioMCPServer(ctx context.Context, ...), which is always called with a real, non-nil context (context.WithTimeout(context.Background(), 30*time.Second)inconnectToMCPServer), so propagatingctxis correct and binds the MCP subprocess lifetime to the 30s timeout:Why this is a real improvement
The
*exec.Cmdis handed tomcp.CommandTransport; binding it viaexec.CommandContext(ctx, ...)ensures the spawned MCP server process is terminated when the 30s timeout (or any future cancellation) fires, instead of leaking until it exits on its own.Remaining prerequisites for enforcement
Enforce-readiness checklist
pkg/workflow/github_cli.go:32is mis-flagged; enforcing now would either break CI or force an unsafe autofix. Blocker.internal/nolintsupport —execcommandwithoutcontextdoes not importpkg/linters/internal/nolint(unlike the 12 enforced linters), so//nolint/redaction directives are silently ignored. Wirenolint.BuildLineIndex+ per-diagnostic suppression as the other linters do, to match parity.-execcommandwithoutcontexttoLINTER_FLAGSin.github/workflows/cgo.yml:1122(currently 12 flags), making it the 13th enforced linter.Recommendation
Convert the 2
connectStdioMCPServersites in this issue. Track the FP fix in #38281 and the nolint parity + CI flag as the final enabling steps (do not enforce until both #38281 and nolint support land).Validation checklist
mcp_inspect_mcp.go:150and:155useexec.CommandContext(ctx, ...).execcommandwithoutcontextimportsinternal/nolintand honors//nolintdirectives.execcommandwithoutcontextis clean, then CI flag added.Effort: Small for the 2 conversions; Medium overall including nolint wiring. Gated on #38281.
References: §27254549539