Skip to content

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

Merged
guglielmo-san merged 2 commits into
modelcontextprotocol:mainfrom
PerryLink:fix/server-conformance-relative-result-dir
Oct 19, 2026
Merged

guglielmo-san merged 2 commits 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.
@guglielmo-san
guglielmo-san merged commit 88006af into modelcontextprotocol:main Oct 19, 2026
9 checks passed
guglielmo-san pushed a commit that referenced this pull request Oct 19, 2026
The result-directory fix in #1409 also changed
`scripts/server-conformance.sh` from mode 100755 to 100644. The nightly
workflow invokes this script directly, so it now exits with `Permission
denied` before running conformance checks.

Restore the executable bit. The script contents, including the
result-directory fix, remain byte-for-byte identical.

Validation:
- Direct `./scripts/server-conformance.sh --help`: exit 126 before the
change, exit 0 afterward.
- `bash -n scripts/server-conformance.sh` and `git diff --check` pass.
- Independent review confirmed this is a mode-only change and repeated
the direct help invocation successfully.

The full server conformance suite was not run for this permission-only
change. The help path exits before setup or cleanup.

AI assistance: Codex helped reproduce, implement, and verify this
change.
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