Skip to content

ci(mcp-diff): prebuild HTTP server and fail when a side is unreachable - #3440

Merged
SamMorrowDrums merged 1 commit into
mainfrom
sammorrowdrums-fix-http-mcp-diff-workflow
Oct 6, 2026
Merged

SamMorrowDrums merged 1 commit into
mainfrom
sammorrowdrums-fix-http-mcp-diff-workflow

Conversation

@SamMorrowDrums

Copy link
Copy Markdown
Collaborator

Summary

The mcp-diff-http job has been green without ever reaching the baseline server. This PR prebuilds the server binary on both sides so it is listening before the action probes. It also adds a guard step that fails the job if either the base or the head server could not be reached.

Why

  • The job started the server with go run ./cmd/github-mcp-server http --port 8082. mcp-server-diff v3.0.0 (40d992e) sleeps for http_startup_wait_ms (5000) and then probes, with no readiness check.
  • The baseline is checked out into a fresh .mcp-diff-base worktree with nothing cached. Since the typed-tool refactors (feat(issues): add typed metadata, comment, and dependency outputs #3391–refactor(actions): type consolidated MCP tools #3398), compiling and linking there takes longer than 5s. Locally, go run in a fresh worktree took 7.5s before it was listening; a prebuilt binary took 0.2s.
  • When it broke:
    • Main run 37539363721 (867131b) was the first partial failure: 11 of 25 baseline configs failed, because the server came up partway through probing.
    • From 37540294159 onward, all 25 fail, including 37541499885 (50 fetch failed).
    • Earlier runs, such as 37539304919 and 37473192233, had 0 failures.
  • Why fail_on_error: true didn't fail the job (action bug): compareConfigResults treats a startup failure on only one side as configMissing ("may not exist on that version"). That is non-fatal, and the working side is diffed against an empty baseline. fail_on_error only checks diffs.has("error"), which is set only when both sides fail. With a single shared HTTP server, an ECONNREFUSED on every config therefore shows up as "25 configuration(s) have API changes" with exit 0.

What changed

  • In the HTTP job, install_command now also runs go build -o bin/github-mcp-server ./cmd/github-mcp-server. The action runs this to completion on both the PR checkout and the baseline worktree before starting the server, so the 5s wait only covers process startup. http_start_command is now ./bin/github-mcp-server http --port 8082 (bin/ is gitignored).
  • New step Verify both sides were probed:
    • Reads the action's JSON report and fails with ::error::<config>: server unreachable on <base|branch> side: … if any config has configMissing on either side.
    • Otherwise prints the negotiated protocol version and tool counts for base and head for every config, as evidence that both sides connected.
  • The stdio job is unchanged.

Recommended upstream fixes for SamMorrowDrums/mcp-server-diff, to be filed separately:

  • Poll the HTTP endpoint until it is ready instead of using a fixed sleep, and surface the server's stderr on failure (today it only goes to core.debug).
  • When a shared server is used, treat connection failures as probe errors, or have fail_on_error also cover one-sided failures.
  • Kill the shared server when the "exited prematurely" error is thrown. Locally, the server process was left running in that case.

MCP impact

  • No tool or API changes. This PR only changes CI.
  • Tool schema or behavior changed
  • New tool added

Prompts tested (tool changes only)

  • N/A

Security / limits

  • No security or limits impact. The workflow uses the same placeholder token and contents: read permissions.
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

Tool renaming

  • I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
    • I have added the new tool aliases in deprecated_tool_aliases.go
  • I am not renaming tools as part of this PR

Lint & tests

  • Linted locally with ./script/lint: not run, since no Go code changed. actionlint .github/workflows/mcp-diff.yml passed.
  • Tested locally with ./script/test: not run, since no Go code changed.

I ran the pinned action's dist/index.js locally against 2d5ad6e, the same commit as the failing main run:

Setup fetch failed discover OK Action exit Guard step
Old go run 50 0 (all 25 base configs "did not start") 0 (bug reproduced) exit 1: ::error::default: server unreachable on base side: TypeError: fetch failed
Prebuilt binary 0 50 (25 head + 25 base) 0, "no API changes" exit 0, e.g. default: base=2026-07-28 head=2026-07-28 tools base=46 head=46

I also sent a manual initialize request with curl to ./bin/github-mcp-server http, using the generated Authorization header, and it succeeded.

Docs

  • Not needed. The rationale is in workflow comments.
  • Updated (README / docs / examples)

The streamable-http job started the server with `go run` and relied on
mcp-server-diff's fixed http_startup_wait_ms sleep. On the baseline
worktree (.mcp-diff-base) nothing is cached, and since the typed tool
refactors compile+link exceeds 5s, so every baseline probe hit
ECONNREFUSED ("fetch failed"). The action treats a one-sided startup
failure as a non-fatal "config-missing" and diffs against an empty
baseline, so fail_on_error never triggered and the job stayed green.

Build the binary in install_command (which completes on both sides
before the server starts) and start it directly, and add a step that
fails the job if any configuration did not start on either side.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SamMorrowDrums
SamMorrowDrums marked this pull request as ready for review October 6, 2026 23:40
@SamMorrowDrums
SamMorrowDrums requested a review from a team as a code owner October 6, 2026 23:40
Copilot AI balanced review requested due to automatic review settings October 6, 2026 23:40
@SamMorrowDrums
SamMorrowDrums merged commit 6330b02 into main Oct 6, 2026
19 checks passed
@SamMorrowDrums
SamMorrowDrums deleted the sammorrowdrums-fix-http-mcp-diff-workflow branch October 6, 2026 23:40

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.

Copilot review overview

🟢 Approval recommended

The workflow changes correctly address delayed startup and detect previously non-fatal unreachable servers.

Review effort: Balanced
Findings: None

What changed in this PR

Prebuilds the HTTP server during MCP-diff setup and verifies both comparison sides were successfully probed.

Changes:

  • Builds and launches a compiled server binary.
  • Fails CI on one-sided probe failures.
  • Logs protocol versions and tool counts.
File Description
.github/​workflows/​mcp-diff.yml Improves HTTP diff startup reliability and probe validation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

2 participants