Skip to content

Make ClaudeTextGeneration tests hermetic on Windows - #4508

Merged
juliusmarminge merged 2 commits into
pingdotgg:mainfrom
mihneaptu:fix/claude-textgen-windows-hermetic
Aug 15, 2026
Merged

Make ClaudeTextGeneration tests hermetic on Windows#4508
juliusmarminge merged 2 commits 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
Changes are confined to one test file; no production code paths are modified.

Overview
Makes ClaudeTextGeneration tests use a portable fake Claude CLI so Windows runs hit the stub instead of the real CLI on PATH (which caused failures, live API calls, and slow runs).

The test fixture’s fake claude binary is no longer a single #!/bin/sh script. Stub logic lives in claude-stub.mjs (same T3_FAKE_CLAUDE_* env checks and exit codes). Windows gets claude.cmd that forwards to Node; Unix keeps a small executable shim that execs the same stub.

withFakeClaudeEnv prepends the fake bin to PATH using ; on Windows and : elsewhere, so the fixture directory is actually discoverable on Windows.

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

Note

Make ClaudeTextGeneration tests hermetic on Windows

  • Replaces the POSIX shell-based fake claude binary with a Node stub (claude-stub.mjs) that replicates the same validation logic using process APIs.
  • On Windows, a claude.cmd batch file invokes the Node stub; on non-Windows, a shell launcher script is written and marked executable.
  • Fixes PATH assembly in withFakeClaudeEnv to use ; as the delimiter on Windows instead of :.
  • Platform detection uses isHostWindows from @t3tools/shared/hostProcess.

Macroscope summarized 68d1bb1.

@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: 405d6947-5dd2-4b3a-9c63-88359a1cd2ff

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[bot]
macroscopeapp Bot previously approved these changes Jul 25, 2026
@macroscopeapp

macroscopeapp Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved 68d1bb1

Test-only changes that make the ClaudeTextGeneration tests work on Windows by replacing shell script stubs with cross-platform Node.js implementations. 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

@CDVolvik CDVolvik 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.

5/5 still green on Linux, so the POSIX path is unaffected by the rewrite. Moving the stub into Node and keeping a thin .cmd entry point is the right shape, and the PATHEXT note in the comment is the part most people miss.

One thing to change while it is being touched. The stub ends with:

process.stdout.write(process.env.T3_FAKE_CLAUDE_OUTPUT ?? "");
process.exit(Number(process.env.T3_FAKE_CLAUDE_EXIT_CODE ?? 0));

process.stdout.write to a pipe is asynchronous, and process.exit does not flush what is still queued. The shell version did not have this problem because printf blocks until the write completes, so this is a behaviour the port introduces rather than inherits.

Measured with the same write-then-exit shape, payload generated inside the child so nothing large goes through the environment:

payload=    1024   process.exit():     1024   |   process.exitCode:     1024
payload=   65536   process.exit():    65536   |   process.exitCode:    65536
payload=  262144   process.exit():   182720  TRUNCATED   |   process.exitCode:   262144
payload= 1048576   process.exit():   146176  TRUNCATED   |   process.exitCode:  1048576
payload= 4194304   process.exit():   146176  TRUNCATED   |   process.exitCode:  4194304

It is latent today rather than broken: T3_FAKE_CLAUDE_OUTPUT arrives through the environment, so current outputs sit under the threshold. But the cutoff lands between 64KB and 256KB, and it depends on pipe buffer state rather than on anything in the test, so the first person who needs a larger fixture gets a partial read with no error and a passing-then-failing test. That is an awkward thing to debug in a file whose stated purpose is hermeticity.

process.exitCode = Number(...) instead of process.exit(...) fixes it and still yields the right exit status, since the stub already drains stdin to EOF and has nothing else keeping the loop alive. The fail() helper has the same shape, though its messages are short enough that it will not bite in practice.

Minor: the new import sits above the @t3tools/contracts line rather than in sorted position, which may or may not matter depending on whether the import ordering rule is enforced here.

Co-authored-by: codex <codex@users.noreply.github.com>
@macroscopeapp
macroscopeapp Bot dismissed their stale review August 15, 2026 11:45

Dismissing prior approval to re-evaluate 68d1bb1

@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 15, 2026
@juliusmarminge
juliusmarminge enabled auto-merge (squash) August 15, 2026 11:46
@juliusmarminge
juliusmarminge merged commit 5ffbf3c into pingdotgg:main Aug 15, 2026
18 checks passed
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Aug 15, 2026
## What's Changed
* fix(desktop): app zoom no longer zooms the preview browser by @juliusmarminge in pingdotgg/t3code#6649
* fix(server): keep provider notification consumers alive past startSession by @tsouth89 in pingdotgg/t3code#6538
* fix(server): treat removed Bitbucket permissions endpoint as unknown, not blocking by @lnieuwenhuis in pingdotgg/t3code#6525
* fix(ssh): let cold remote servers finish starting by @gbarros-dev in pingdotgg/t3code#6168
* fix(web): preserve Claude insight line breaks by @nateEc in pingdotgg/t3code#4344
* feat(web): accept file drops across the chat workspace by @dbalders in pingdotgg/t3code#6636
* fix(web): widen ordered-list marker gutter for 3+ digit item numbers by @lnieuwenhuis in pingdotgg/t3code#6527
* fix(server): bound thread activity hydration by @t3-code[bot] in pingdotgg/t3code#6153
* fix(web): restore the Archive action in the default sidebar thread menu by @lnieuwenhuis in pingdotgg/t3code#6526
* fix(web): open diff files from nested projects by @gbarros-dev in pingdotgg/t3code#6174
* fix(mobile): use tryOpenExternalUrl for markdown links in ThreadFeed by @mohamedmastouri-hue in pingdotgg/t3code#5872
* fix(web): open the file a bare filename reference names by @Brechard in pingdotgg/t3code#6297
* fix(server): stop the provider title mirror from overwriting real thread titles by @repparw in pingdotgg/t3code#5941
* fix(shared): match source-control providers by DNS label by @gbarros-dev in pingdotgg/t3code#6175
* feat(desktop): Chrome-style hold-to-quit by @Bil0000 in pingdotgg/t3code#5508
* fix(gitlab): submit review comments on context lines by @tarik02 in pingdotgg/t3code#6348
* fix(marketing): keep Grok mark clear of mobile hero copy by @NicL9923 in pingdotgg/t3code#4542
* fix(mobile): recover the QR pairing scanner when camera access is denied by @hey-jj in pingdotgg/t3code#6487
* fix(web): keep a long path from running under the folder picker button by @Sy-D in pingdotgg/t3code#4823
* fix(terminal): right-click paste works in the terminal by @StiensWout in pingdotgg/t3code#5240
* fix(mobile): explain iOS-only settings on Android by @danvernon in pingdotgg/t3code#4981
* fix(web): stop counting a workflow coordinator as a working agent by @Rishet11 in pingdotgg/t3code#6672
* fix(web): keep floating preview anchored after panel closes by @AksharP5 in pingdotgg/t3code#6547
* fix(web): unstick /connect after in-modal sign-in by redirecting to the authorize endpoint by @TorbenWetter in pingdotgg/t3code#5133
* fix(web): keep send reachable while a turn is running on mobile by @AMohamedAakhil in pingdotgg/t3code#4781
* fix(web): reject unsupported composer image types at attach time by @mdshzb04 in pingdotgg/t3code#6574
* Make ClaudeTextGeneration tests hermetic on Windows by @mihneaptu in pingdotgg/t3code#4508
* fix(web): show command output in work log by @LikoKiko in pingdotgg/t3code#4083
* fix(web): reserve sibling column width when resizing the right panel by @Lasdw6 in pingdotgg/t3code#6279
* fix(web): replace whitespace in new ref names with dashes by @jorj-pineda in pingdotgg/t3code#6270
* fix(client-runtime): branch list no longer resets while paging through refs by @a20hek in pingdotgg/t3code#5858
* fix(web): support Shift+Insert terminal paste by @aoright in pingdotgg/t3code#5982
* fix(web): keep the composer glass aligned with the context strip at any interface font size by @Williawar in pingdotgg/t3code#5703
* fix(codex): keep background memory out of chats by @AksharP5 in pingdotgg/t3code#5468
* fix(server): treat a missing Codex rollout as a recoverable resume error by @Rishet11 in pingdotgg/t3code#6671
* fix(web): hide provider Update toast action while an update is running by @mrmg in pingdotgg/t3code#6544
* fix(desktop): agent shells inherit a UTF-8 locale on macOS by @Linus-Boehm in pingdotgg/t3code#6236
* fix(server): ignore Claude command lifecycle messages by @naveed949 in pingdotgg/t3code#6606
* docs: mention Bitbucket user read scope needed by auth probe by @thamrx in pingdotgg/t3code#6291
* fix(server): return valid preview action results by @duncan-vc in pingdotgg/t3code#5966
* fix(claude): make "Always allow for session" stick, and only for the session by @kakismash in pingdotgg/t3code#5041
* fix(ssh): surface a failed remote t3 install instead of a silent 0-byte server.log by @TorbenWetter in pingdotgg/t3code#5132
* perf(server): persist the wire projection for streaming tool.updated data by @mInrOz in pingdotgg/t3code#6675
* fix(web): stop wrapping partial code block selections in markdown fences by @JoeJoeflyn in pingdotgg/t3code#5069
* fix(web): hide T3 Connect toggle in web app settings by @JoeJoeflyn in pingdotgg/t3code#5068
* fix(web): show provider account accent badge in sidebar rows and hover card by @vitalyiegorov in pingdotgg/t3code#5980
* fix(server): wait for concurrent SQLite writers instead of failing with SQLITE_BUSY by @ostapondo in pingdotgg/t3code#5134
* fix(web): reject oversized prompts before provider turn start by @naveed949 in pingdotgg/t3code#6602
* feat(web): collapse the question prompt from its header by @Jardo-51 in pingdotgg/t3code#6773
* fix(shared): degrade an unknown system time zone to UTC in usage windows by @Rishet11 in pingdotgg/t3code#6670
* fix(claude): discover repo-local .agents/skills in skill discovery by @RoshanMhatre in pingdotgg/t3code#5488
* fix(server): let slow provider CLIs raise their discovery probe budget by @CDVolvik in pingdotgg/t3code#6223
* fix(web): retain terminal PR badges after checkout switch by @sebbonit in pingdotgg/t3code#4755
* fix(web): show selected model in context window tooltip by @nqrwhal in pingdotgg/t3code#4772
* fix(web): scale command details with code font by @Serendeep in pingdotgg/t3code#6510
* fix(web): preserve XML-like tags in user messages by @0utsights in pingdotgg/t3code#4133

## New Contributors
* @mohamedmastouri-hue made their first contribution in pingdotgg/t3code#5872
* @NicL9923 made their first contribution in pingdotgg/t3code#4542
* @hey-jj made their first contribution in pingdotgg/t3code#6487
* @danvernon made their first contribution in pingdotgg/t3code#4981
* @Rishet11 made their first contribution in pingdotgg/t3code#6672
* @AksharP5 made their first contribution in pingdotgg/t3code#6547
* @TorbenWetter made their first contribution in pingdotgg/t3code#5133
* @AMohamedAakhil made their first contribution in pingdotgg/t3code#4781
* @mdshzb04 made their first contribution in pingdotgg/t3code#6574
* @mihneaptu made their first contribution in pingdotgg/t3code#4508
* @LikoKiko made their first contribution in pingdotgg/t3code#4083
* @Lasdw6 made their first contribution in pingdotgg/t3code#6279
* @jorj-pineda made their first contribution in pingdotgg/t3code#6270
* @a20hek made their first contribution in pingdotgg/t3code#5858
* @aoright made their first contribution in pingdotgg/t3code#5982
* @Williawar made their first contribution in pingdotgg/t3code#5703
* @mrmg made their first contribution in pingdotgg/t3code#6544
* @Linus-Boehm made their first contribution in pingdotgg/t3code#6236
* @naveed949 made their first contribution in pingdotgg/t3code#6606
* @thamrx made their first contribution in pingdotgg/t3code#6291
* @duncan-vc made their first contribution in pingdotgg/t3code#5966
* @kakismash made their first contribution in pingdotgg/t3code#5041
* @mInrOz made their first contribution in pingdotgg/t3code#6675
* @JoeJoeflyn made their first contribution in pingdotgg/t3code#5069
* @vitalyiegorov made their first contribution in pingdotgg/t3code#5980
* @ostapondo made their first contribution in pingdotgg/t3code#5134
* @Jardo-51 made their first contribution in pingdotgg/t3code#6773
* @RoshanMhatre made their first contribution in pingdotgg/t3code#5488
* @CDVolvik made their first contribution in pingdotgg/t3code#6223
* @sebbonit made their first contribution in pingdotgg/t3code#4755
* @nqrwhal made their first contribution in pingdotgg/t3code#4772
* @Serendeep made their first contribution in pingdotgg/t3code#6510
* @0utsights made their first contribution in pingdotgg/t3code#4133

**Full Changelog**: pingdotgg/t3code@v0.0.34-nightly.20260815.1100...v0.0.34-nightly.20260815.1101

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.34-nightly.20260815.1101
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 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

3 participants