Skip to content

Make ClaudeTextGeneration tests hermetic on Windows#4508

Open
mihneaptu wants to merge 1 commit into
pingdotgg:mainfrom
mihneaptu:fix/claude-textgen-windows-hermetic
Open

Make ClaudeTextGeneration tests hermetic on Windows#4508
mihneaptu wants to merge 1 commit into
pingdotgg:mainfrom
mihneaptu:fix/claude-textgen-windows-hermetic

Conversation

@mihneaptu

@mihneaptu mihneaptu commented Jul 25, 2026

Copy link
Copy Markdown

What Changed

The ClaudeTextGeneration test fixture now works on Windows. The fake claude binary is a Node script with a platform-appropriate entry point — claude.cmd on Windows, a #!/bin/sh shim elsewhere — and PATH is built with the platform's separator instead of a hardcoded :.

One file, test-only. No production code changes.

Why

Fixes #4507.

On Windows all 5 tests in this file fail on unmodified main. The fixture never takes effect, so the real installed Claude CLI runs instead: assertions compare fixture strings against live model output, and the run makes real billed API calls.

Two POSIX-only assumptions caused it:

  1. PATH was joined with a literal :. Windows uses ;, so the fixture's bin directory never became its own PATH entry.
  2. The stub was written as extension-less bin/claude containing a #!/bin/sh script. Windows resolves executables through PATHEXT, so an extension-less file is skipped — and even with a correct extension a shebang script is not executable there.

Fixing only the separator is not enough, since the shebang stub still would not run. So the stub logic moved into Node (claude-stub.mjs) with a thin platform-specific launcher in front of it. The T3_FAKE_CLAUDE_* env contract and every exit code are unchanged.

No production change was needed: resolveSpawnCommand already resolves through PATHEXT and spawns .cmd with shell: true.

Validation

Windows 11, this file only:

result
before 5 failed, 134.28s (live API calls)
after 5 passed, 4.09s (stub invoked)

The 134s to 4s drop is the signal that no network calls happen anymore.

vp fmt, vp lint --report-unused-disable-directives, git diff --check, and pnpm --filter t3 typecheck are all clean for this scope.

I do not have a POSIX machine, so the shell shim is verified behaviourally under sh rather than by running the suite: happy path, args-must-contain pass and fail, stdin-must-contain pass and fail, custom exit code, and config-dir mismatch all return the same exit codes as the old script (0, 0, 2, 4, 0, 7, 5). The full suite on Linux and macOS is unverified by me and worth confirming in CI.

One note so it is not a surprise: on Windows the run now emits DEP0190 (passing args with shell: true). That comes from the existing resolveSpawnCommand path and is not introduced here — it simply was not reachable from this test before, because the stub never resolved.

Not included

ClaudeTextGeneration.test.ts builds claudeConfigDir from process.cwd(), which leaves a .claude-work-test/ directory behind in the repo after the run. It is recorded in #4507 and left out here to keep this focused.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes — n/a, no UI changes
  • I included a video for animation/interaction changes — n/a

Note

Low Risk
Test fixture changes only; no production behavior, auth, or data paths are touched.

Overview
ClaudeTextGeneration tests now install a cross-platform fake claude instead of a POSIX-only shell script, so Windows no longer falls through to the real CLI on PATH.

The shared stub logic moves into claude-stub.mjs, preserving the existing T3_FAKE_CLAUDE_* env contract and exit codes. makeFakeClaudeBinary writes claude.cmd (Node launcher) on Windows and a thin #!/bin/sh shim elsewhere. withFakeClaudeEnv prepends the fixture bin to PATH using ; on Windows and : elsewhere via isHostWindows.

Test-only change; no production spawn or Claude integration code is modified.

Reviewed by Cursor Bugbot for commit ace6621. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Make ClaudeTextGeneration tests hermetic on Windows

  • Replaces the POSIX shell script fake Claude binary with a Node.js stub (claude-stub.mjs) that handles argument, stdin, stderr, exit code, and env validations portably.
  • On Windows, generates a claude.cmd launcher that forwards to node claude-stub.mjs; on non-Windows, writes an executable shell wrapper as before.
  • Fixes PATH construction in withFakeClaudeEnv to use ; as the delimiter on Windows so the fake binary is discoverable.

Macroscope summarized ace6621.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3d63d280-0ed8-4e04-ac16-876a49cfbf57

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@macroscopeapp

macroscopeapp Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Test-only changes that make the ClaudeTextGeneration test infrastructure cross-platform compatible. Converts shell script stub to Node.js and adds Windows PATH handling. No production code is affected.

You can customize Macroscope's approvability policy. Learn more.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ClaudeTextGeneration tests are not hermetic on Windows and invoke the real Claude CLI

1 participant