Skip to content

Refactor cmd HTTP server lifecycle into shared serveAndWait helper - #7104

Merged
lpcox merged 8 commits into
mainfrom
copilot/duplicate-code-http-server-lifecycle
Jun 6, 2026
Merged

Refactor cmd HTTP server lifecycle into shared serveAndWait helper#7104
lpcox merged 8 commits into
mainfrom
copilot/duplicate-code-http-server-lifecycle

Conversation

Copilot AI commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

run() in internal/cmd/root.go and runProxy() in internal/cmd/proxy.go duplicated the same serve/cancel/shutdown lifecycle with diverging timeout expression and shutdown handling shape. This change centralizes that lifecycle so timeout and graceful shutdown behavior are maintained in one place.

  • Lifecycle deduplication

    • Added internal/cmd/serve.go with serveAndWait(...) to encapsulate:
      • background Serve execution
      • cancellation on unexpected serve errors
      • context-driven shutdown wait
      • graceful http.Server.Shutdown with timeout
  • Root command refactor

    • Replaced inline server lifecycle block in run() with serveAndWait(...).
    • Preserved gateway-specific shutdown signal logging via callback hook.
  • Proxy command refactor

    • Replaced inline server lifecycle block in runProxy() with serveAndWait(...).
    • Preserved proxy-specific shutdown and startup logging; kept listener/TLS setup local to proxy serve function.
  • Timeout standardization

    • Introduced a shared timeout constant (httpServerShutdownTimeout = 5s) and removed proxy-local timeout duplication.
  • Focused coverage

    • Added internal/cmd/serve_test.go to validate graceful shutdown behavior of the shared helper.
if err := serveAndWait(
    ctx,
    cancel,
    httpServer,
    httpServerShutdownTimeout,
    func() { debugLog.Print("Shutdown signal received, initiating graceful shutdown") },
    func() error { return httpServer.Serve(listener) },
); err != nil {
    debugLog.Printf("Graceful shutdown completed with error: %v", err)
}

Copilot AI changed the title [WIP] Refactor duplicate HTTP server lifecycle code Refactor cmd HTTP server lifecycle into shared serveAndWait helper Jun 6, 2026
Copilot finished work on behalf of lpcox June 6, 2026 16:30
Copilot AI requested a review from lpcox June 6, 2026 16:30
@lpcox
lpcox marked this pull request as ready for review June 6, 2026 16:35
Copilot AI review requested due to automatic review settings June 6, 2026 16:35

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 centralizes the duplicated HTTP server serve/cancel/shutdown lifecycle used by the gateway root command and the proxy subcommand into a shared serveAndWait(...) helper, standardizing shutdown timeout behavior and adding focused test coverage for graceful shutdown.

Changes:

  • Added internal/cmd/serve.go with a shared serveAndWait(...) helper and a common shutdown timeout constant.
  • Refactored internal/cmd/root.go and internal/cmd/proxy.go to use serveAndWait(...) instead of inline lifecycle logic.
  • Added internal/cmd/serve_test.go to validate graceful shutdown behavior.
Show a summary per file
File Description
internal/cmd/serve.go Introduces shared serve/shutdown helper and shared shutdown timeout constant.
internal/cmd/serve_test.go Adds test coverage for the shared helper’s graceful shutdown behavior.
internal/cmd/root.go Replaces inline HTTP server lifecycle logic with serveAndWait(...) usage.
internal/cmd/proxy.go Replaces inline proxy server lifecycle logic with serveAndWait(...) usage and removes duplicated timeout constant.

Copilot's findings

Tip

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

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Comment thread internal/cmd/serve_test.go
Comment thread internal/cmd/serve.go
Comment thread internal/cmd/root.go
Comment thread internal/cmd/proxy.go
Comment thread internal/cmd/serve.go
lpcox and others added 5 commits June 6, 2026 09:45
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox

lpcox commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Copilot AI commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix failing ci checks https://github.com/github/gh-aw-mcpg/actions/runs/27068086510/job/79892367529?pr=7104 https://github.com/github/gh-aw-mcpg/actions/runs/27068086510/job/79892367530?pr=7...

Fixed in 1c89159. The failing CI jobs were due to syntax regressions in internal/cmd/serve.go and internal/cmd/proxy.go (missing/extra brace). I restored brace balance and validated with make lint, make build, go test ./internal/cmd, and make test; all pass now.

Copilot finished work on behalf of lpcox June 6, 2026 16:52
@lpcox
lpcox merged commit 3ee8033 into main Jun 6, 2026
16 checks passed
@lpcox
lpcox deleted the copilot/duplicate-code-http-server-lifecycle branch June 6, 2026 17:00
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: HTTP Server Lifecycle in cmd/root.go and cmd/proxy.go

3 participants