Skip to content

Refactor launcher connection error logging into internal/launcher#7276

Merged
lpcox merged 2 commits into
mainfrom
copilot/aw-abc123def456-fix-duplicate-log-prefix
Jun 10, 2026
Merged

Refactor launcher connection error logging into internal/launcher#7276
lpcox merged 2 commits into
mainfrom
copilot/aw-abc123def456-fix-duplicate-log-prefix

Conversation

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

[LAUNCHER]-prefixed connection failure logs were split across internal/mcp and internal/launcher, which blurred ownership and made launcher log behavior harder to find and evolve. This change consolidates launcher-specific error diagnostics in the launcher package and removes launcher-prefixed behavior from mcp internals.

  • Logging ownership realignment

    • Added internal/launcher/connection_error_log.go with:
      • ConnectionErrorContext
      • LogConnectionError
    • This moves all [LAUNCHER] connection-failure formatting and hint logic into the launcher subsystem.
  • Launcher call-site update

    • Updated internal/launcher/log_helpers.go:
      • logLaunchError now invokes launcher-local LogConnectionError(...)
      • Removed dependency on mcp.ConnectionErrorContext / mcp.LogConnectionError.
  • mcp package decoupling

    • Removed launcher-specific connection error logging implementation from internal/mcp/connection_logging.go.
    • Updated internal/mcp/connection.go connection-failure branch to emit mcp-scoped structured errors (including sanitized stderr) without [LAUNCHER] console conventions.
  • Test ownership alignment

    • Moved connection-error logging tests from internal/mcp/errors_test.go to internal/launcher/connection_error_log_test.go so behavior coverage lives with the owning package.
// before (launcher -> mcp)
mcp.LogConnectionError(mcp.ConnectionErrorContext{ ... }, err)

// after (launcher-local ownership)
LogConnectionError(ConnectionErrorContext{ ... }, err)

Copilot AI changed the title [WIP] Fix duplicate code pattern in launcher log prefix Refactor launcher connection error logging into internal/launcher Jun 9, 2026
Copilot finished work on behalf of lpcox June 9, 2026 15:40
Copilot AI requested a review from lpcox June 9, 2026 15:40
GitHub Advanced Security started work on behalf of lpcox June 9, 2026 15:43 View session
GitHub Advanced Security finished work on behalf of lpcox June 9, 2026 15:44
GitHub Advanced Security started work on behalf of lpcox June 9, 2026 15:50 View session
GitHub Advanced Security finished work on behalf of lpcox June 9, 2026 15:51
@lpcox lpcox marked this pull request as ready for review June 10, 2026 13:44
Copilot AI review requested due to automatic review settings June 10, 2026 13:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR consolidates launcher-specific connection failure diagnostics into internal/launcher, removing [LAUNCHER]-style console logging and hinting from internal/mcp so ownership and future evolution of launcher logs are localized to the launcher subsystem.

Changes:

  • Added internal/launcher/connection_error_log.go implementing ConnectionErrorContext and LogConnectionError (launcher-owned connection failure formatting/hints).
  • Updated internal/launcher/log_helpers.go to call launcher-local LogConnectionError instead of mcp.LogConnectionError.
  • Removed launcher-specific connection error logging from internal/mcp/connection_logging.go and updated internal/mcp/connection.go to emit MCP-scoped structured error logs (including sanitized stderr) without [LAUNCHER] conventions.
  • Moved/updated tests into internal/launcher/connection_error_log_test.go.
Show a summary per file
File Description
internal/mcp/connection.go Replaces launcher-style error reporting on connect failure with MCP-scoped structured error logs and sanitized stderr logging.
internal/mcp/connection_logging.go Removes launcher-owned ConnectionErrorContext / LogConnectionError from the mcp package.
internal/launcher/log_helpers.go Switches launch-failure logging to use launcher-owned connection error logging helpers.
internal/launcher/connection_error_log.go Introduces launcher-local connection error context + diagnostic logging/hints.
internal/launcher/connection_error_log_test.go Relocates/updates coverage for launcher connection error logging behavior.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

internal/launcher/connection_error_log_test.go:334

  • In the stderr-output table, the "whitespace-only stderr is empty" case doesn't actually validate the intended behavior because the test passes the untrimmed whitespace string into LogConnectionError and provides no notInLog assertions. Trimming in the test and asserting that the stderr block is omitted will make this case meaningful and prevent regressions.
  • Files reviewed: 5/5 changed files
  • Comments generated: 0

@lpcox lpcox merged commit 45c3a90 into main Jun 10, 2026
47 checks passed
@lpcox lpcox deleted the copilot/aw-abc123def456-fix-duplicate-log-prefix branch June 10, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code Pattern: [LAUNCHER] Log Prefix Spans Two Packages

3 participants