Skip to content

fix(scripts): make --result_dir absolute in server-conformance.sh - #1409

Open
PerryLink wants to merge 1 commit into
modelcontextprotocol:mainfrom
PerryLink:fix/server-conformance-relative-result-dir
Open

PerryLink wants to merge 1 commit into
modelcontextprotocol:mainfrom
PerryLink:fix/server-conformance-relative-result-dir

Conversation

@PerryLink

Copy link
Copy Markdown
Contributor

scripts/server-conformance.sh resolves --result_dir only once, so a relative path — exactly the form CONTRIBUTING.md:64 documents — writes every result into a nested copy of itself.

"$CONTRIBUTING.md:64"  ->  scripts/server-conformance.sh --result_dir ./conformance-results

:63 creates the directory, :64 makes it WORKDIR, and then :95/:101 run the tests from inside that directory while passing --output-dir "$RESULT_DIR" (:99/:105). A relative $RESULT_DIR therefore resolves against itself: <dir>/<dir>/.

Measured on Windows / Git Bash:

$ bash scripts/server-conformance.sh --result_dir relout
... Total: 40 passed, 0 failed          # exit 0

$ ls relout
relout/                                  # only this
$ ls relout/relout | head -3
account-usage-configuration
...                                      # all 30 scenario dirs in here

$ bash scripts/server-conformance.sh --result_dir relout2   # with this patch
$ ls relout2 | head -3
account-usage-configuration              # 30 dirs directly, no nesting

The run still reports success, which is why nobody noticed: the results are real, just one level down from where the documented command says they are.

The change

This is the block scripts/client-conformance.sh:58-62 already has — added there by #1366 (2026-10-14) and the server script was missed:

     mkdir -p "$RESULT_DIR"
+    # Make the path absolute: the tests run from inside it (see below).
+    RESULT_DIR=$(cd "$RESULT_DIR" && pwd)
     WORKDIR="$RESULT_DIR"

Two lines in one file, taking the absolute path so the later cd "$WORKDIR" cannot change what --output-dir refers to. Cobbled from the client script verbatim, comment included, so the two stay recognisably the same fix.

Scope

  • Not touched: the missing $(go env GOEXE) on the server binary at :70. I checked before ruling it out — unlike the client path, nothing spawns the server through cmd.exe, and Git Bash executes the extensionless PE, so it is not a defect.
  • Duplicate check: commits?path=scripts/server-conformance.sh (3 commits, last 2026-07-14) and a search for result_dir (only scripts: fix client-conformance.sh on Windows and with a relative --result_dir #1366 and scripts: add conformance test runner #694) found no open PR, no open issue and no merged fix for the server side. The CONTRIBUTING.md:64 line that documents the relative form is unchanged since it was written.

A relative --result_dir — the form CONTRIBUTING.md documents — was passed to
--output-dir from inside the same directory, so every result landed in
<dir>/<dir>/. The run still exited 0, which is why it went unnoticed.

client-conformance.sh already received this fix in modelcontextprotocol#1366; take the same two
lines here.

This branch has not been deployed

No deployments
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.

1 participant