Skip to content

fix(tests): keep stub runner paths shell-safe so test_judge passes on Windows - #159

Open
Yi-111-a wants to merge 1 commit into
ayghri:mainfrom
Yi-111-a:tests/judge-windows-shell-paths
Open

Yi-111-a wants to merge 1 commit into
ayghri:mainfrom
Yi-111-a:tests/judge-windows-shell-paths

Conversation

@Yi-111-a

@Yi-111-a Yi-111-a commented Sep 10, 2026

Copy link
Copy Markdown

Summary

Three end-to-end tests in tests/test_judge.py fail on Windows on an unmodified ff690b6:

FAIL: test_judging_produces_paired_rows_the_scorer_accepts
FAIL: test_a_malformed_verdict_skips_its_group_instead_of_killing_the_run
FAIL: test_runner_failure_skips_its_group_and_continues
Ran 17 tests ... FAILED (failures=3)

Those three tests build their stub judge runner as an sh -c command string and interpolate a Path into it with an f-string. On Windows str(Path) yields backslash separators, and the shell reads a backslash as an escape character:

$ sh -c 'printf payload > C:\Users\me\AppData\Local\Temp\tmpbl_g_9zt\prompt.txt'
$ echo $?
0
$ ls 'C:UsersmeAppDataLocalTemptmpbl_g_9ztprompt.txt'
C:UsersmeAppDataLocalTemptmpbl_g_9ztprompt.txt

The redirect exits 0 while writing to a mangled path, so the stub runner looks like it ran and wrote nothing. The cat C:\Users\... that follows then fails, the group is skipped, and judge.main returns 1 — the three tests fail for a reason unrelated to what they assert.

The same interpolation splits on whitespace if a path contains a space, which is reachable when %TEMP% sits under a user name with a space:

$ sh -c 'printf payload > .../dir with spaces/out.json'
cat: with: No such file or directory
cat: spaces/out.json: No such file or directory

Fix: render every interpolated path with Path.as_posix() and shlex.quote(). Both are no-ops on POSIX, so Linux and macOS behavior is unchanged.

Before → after, same Windows machine, same checkout:

command before after
python -m unittest tests.test_judge -v Ran 17 testsFAILED (failures=3) Ran 17 testsOK
python -m unittest discover -s tests -v Ran 41 testsFAILED (failures=3) Ran 41 testsOK

This is test-fixture only. scripts/judge.py, scripts/run_evals.py, the skill, the hooks, and every manifest are untouched.

Authorship and provenance — select exactly one

  • Human-authored — substantive implementation and text were produced by a human.
  • Autonomous agent-authored — an agent planned and produced most of the substantive change.
  • Hybrid — a human and one or more agents both made substantive contributions.

Agent/tool and model/version: WorkBuddy agent harness; model DeepSeek-V4.1-Flash.

Agent contribution: Located the three failing tests, diagnosed the cause, wrote the shell_path helper and its three call sites, ran the before/after comparisons on Windows, and drafted this PR.

Human verification: The submitter was given the diagnosis, the diff, and the before/after results, selected the authorship category above, and approved submission. The submitter did not personally re-run the commands; every run reported under Verification was performed by the agent that wrote the change.

Known limitations or uncertain results: Checks ran on Windows (Python 3.13.12). Path.as_posix() and shlex.quote() are no-ops for the POSIX temp paths used on Linux and macOS, so those runs should be unaffected, but no Linux or macOS run was performed here. The three tests still require a POSIX sh on PATH; a Windows machine without Git Bash errors rather than skips, and this PR does not change that.

Labels

Target label: Target:Evals

Author label: Author:AI

Workflow labels: bug

Safety and side effects

  • The change does not access or expose secrets, private files, or unrelated user/repository data.
  • Scripts, hooks, workflows, and evals are bounded and do not create surprising or irreversible side effects.
  • No destructive, privileged, production, externally visible, or persistent action occurs without explicit user intent and appropriate safeguards.
  • Network access, third-party code, permissions, and provider costs are minimized and documented.
  • Prompt text, examples, and fixtures contain no hidden instructions that weaken safety or expand agent authority.

Side effects, permissions, network access, and cost: None. Test-fixture string construction only; no new dependencies, network calls, or provider costs. The tests keep using temporary directories and delete them as before.

Compatibility

  • This is not a breaking change.
  • This is a breaking change; it was discussed, and migration/deprecation documentation is included below.
  • Canonical and mirrored skill files are synchronized when applicable.
  • Relevant platform manifests and installation documentation were reviewed.

Migration or rollback notes: None required. Path.as_posix() and shlex.quote() are identity operations for the POSIX temp paths currently used in CI, so the Ubuntu run is byte-identical.

Verification

  • python -m unittest tests.test_judge -v on Windows before the fix — Ran 17 tests, FAILED (failures=3) (clean ff690b6).
  • python -m unittest tests.test_judge -v on Windows after the fix — Ran 17 tests, OK.
  • python -m unittest discover -s tests -v on Windows after the fix — Ran 41 tests, OK.
  • python -m unittest tests.test_judge with TEMP/TMP/TMPDIR pointed at a directory containing a space — Ran 17 tests, OK.
  • python scripts/run_evals.py validateEvaluation cases are valid.
  • git diff --check — clean.

The three tests failed before the change on the same machine and pass after it; no test was skipped, deleted, or weakened.

Why this stayed red: no workflow runs this file. plugin-load-check.yml invokes tests.test_always_on_hooks on a [ubuntu-latest, windows-latest] matrix, pi-load-check.yml runs scripts/check_pi_extension.py, and cursor-skill-sync.yml watches the skill files. grep -rn "test_judge\|discover" .github/workflows/ returns nothing, so tests/test_judge.py is not executed on any runner.

Behavior evals: Not applicable; no skill-behavior, runner, or plugin-runtime change.

Final accountability

  • I reviewed the complete diff, removed unrelated generated changes, and take responsibility for the submitted content.
  • All failed, skipped, or unrun checks are disclosed above.

The three end-to-end tests in tests/test_judge.py build their stub judge
runner as an `sh -c` string and interpolate a Path into it with an
f-string. On Windows str(Path) yields backslash separators, which the
shell reads as escapes: `cat > C://Users//...` redirects to a mangled path
and still exits 0, so the stub runner looks like it ran while having
written nothing. The `cat` that follows then fails, the group is skipped,
and the test fails for a reason unrelated to what it asserts.

The same interpolation also splits on whitespace when a path contains a
space, which is reachable when %TEMP% sits under a user name with one.

Render interpolated paths with Path.as_posix() and shlex.quote(). Both
are no-ops on POSIX, so Linux and macOS are unchanged.
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