Skip to content

fix: detect ungraceful stdin close and self-terminate #2325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
D2758695161 wants to merge 2 commits into
base: main
Choose a base branch
from
+15 −0
Open

fix: detect ungraceful stdin close and self-terminate #2325

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
792dec4
fix: detect ungraceful stdin close and self-terminate
D2758695161 Apr 14, 2026
4eccd4e
Merge branch 'main' into feat/stdin-watchdog
SamMorrowDrums Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions internal/ghmcp/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,21 @@
errC <- ghServer.Run(ctx, &mcp.IOTransport{Reader: in, Writer: out})
}()

// Stdin watchdog: detect ungraceful client disconnect and self-terminate.
// Without this, orphaned containers accumulate when the MCP client (e.g. Cursor/VS Code)
// is force-killed or crashes, because `--rm` only fires on graceful exit.
Comment on lines +320 to +322
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

The PR description mentions adding a unit test for the stdin watchdog behavior, but there are no tests in this package today (internal/ghmcp/server_test.go is empty) and this change introduces non-trivial shutdown behavior. Please add a focused unit test that exercises the EOF/error path and asserts that stop()/shutdown is triggered (e.g., by injecting an io.Reader into the stdio server wiring rather than hard-coding os.Stdin).

This issue also appears on line 323 of the same file.

Copilot generated this review using guidance from repository custom instructions.
go func() {
buf := make([]byte, 1)
for {
n, err := os.Stdin.Read(buf)

Check failure on line 326 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / lint 

declared and not used: n

Check failure on line 326 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / mcp-diff 

declared and not used: n

Check failure on line 326 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / mcp-diff 

declared and not used: n

Check failure on line 326 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / mcp-diff 

declared and not used: n

Check failure on line 326 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / mcp-diff 

declared and not used: n

Check failure on line 326 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / docs-check 

declared and not used: n

Check failure on line 326 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / build (windows-latest) 

declared and not used: n

Check failure on line 326 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest) 

declared and not used: n

Check failure on line 326 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / build (macos-latest) 

declared and not used: n
if tn == 0 || err != nil {

Check failure on line 327 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / lint 

undefined: tn (typecheck)

Check failure on line 327 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / mcp-diff 

undefined: tn

Check failure on line 327 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / mcp-diff 

undefined: tn

Check failure on line 327 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / mcp-diff 

undefined: tn

Check failure on line 327 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / docs-check 

undefined: tn

Check failure on line 327 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / build (windows-latest) 

undefined: tn

Check failure on line 327 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest) 

undefined: tn

Check failure on line 327 in internal/ghmcp/server.go

View workflow job for this annotation

GitHub Actions / build (macos-latest) 

undefined: tn
Copy link
Copy Markdown
Collaborator

@SamMorrowDrums SamMorrowDrums Apr 15, 2026

Choose a reason for hiding this comment

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

Suggested change
if tn == 0 || err != nil {
if n == 0 || err != nil {

I think you have a typo here

Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

The watchdog condition uses tn but the variable declared above is n. As written this will not compile; use the correct variable name in the n == 0 check.

Suggested change
if tn == 0 || err != nil {
if n == 0 || err != nil {

Copilot uses AI. Check for mistakes.
logger.Info("stdin closed, shutting down server")
stop()
return
}
}
}()

// Output github-mcp-server string
_, _ = fmt.Fprintf(os.Stderr, "GitHub MCP Server running on stdio\n")

Expand Down
Loading