Skip to content

test(server): keep provider fixture directories host-portable - #9572

Merged
juliusmarminge merged 1 commit into
windows-tests/mode-bitsfrom
windows-tests/temp-cleanup
Sep 4, 2026
Merged

test(server): keep provider fixture directories host-portable#9572
juliusmarminge merged 1 commit into
windows-tests/mode-bitsfrom
windows-tests/temp-cleanup

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 4, 2026

Copy link
Copy Markdown
Member

The Claude capabilities probe can abort before its child has fully exited. On Windows, keeping that child’s cwd inside the scoped fixture makes cleanup fail with EBUSY. The Codex collaboration fixture also hard-coded /tmp, which does not exist on Windows.

Give the Claude child a separate temporary cwd with retrying final cleanup, and use the host temporary directory for Codex collaboration sessions. The provider behavior and assertions are unchanged.

Part of the Windows test-suite stack rooted at #9564; the manual Windows lane comes from #9538.

Originally prepared with Claude Fable 5 in Claude Code; finalized with GPT-5.6 Sol in T3 Code.


Note

Low Risk
Test-only fixture and cleanup changes; no production code paths or assertion changes.

Overview
Makes two provider integration tests reliable on Windows by fixing how temporary working directories are created and torn down. Production provider behavior and test assertions are unchanged.

Claude capabilities probe SDK test: The fake child’s workspace cwd is no longer nested inside the scoped fixture directory. It is created as its own temp folder and removed in a finalizer via node:fs/promises rm with retries (instead of Effect scheduling, which would stall under TestClock). That avoids EBUSY when the probe aborts the SDK before the child exits and still holds the directory as cwd.

Codex collab runtime integration tests: Every makeCodexSessionRuntime call that used a hard-coded /tmp cwd now uses os.tmpdir(), so sessions run against the host temp path on Windows and Unix alike.

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

@github-actions github-actions Bot added size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 15.1 KiB
Codex Thread snapshot wire 7.0 KiB 7.3 KiB
Codex Live turn WebSocket wire 6.5 KiB 7.8 KiB
Codex Live turn WebSocket decoded 57.0 KiB 66.4 KiB
Codex Live turn messages 8 21
Claude Total thread wire 13.6 KiB 15.1 KiB
Claude Thread snapshot wire 7.0 KiB 7.3 KiB
Claude Live turn WebSocket wire 6.5 KiB 7.8 KiB
Claude Live turn WebSocket decoded 57.8 KiB 66.4 KiB
Claude Live turn messages 9 21

Baseline: unavailable · PR result: 8170118 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.8 KiB
  • Claude decoded thread snapshot: 114.5 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch from 89092d7 to 0a640be Compare September 4, 2026 03:20
@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:XS 0-9 changed lines (additions + deletions). labels Sep 4, 2026
@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch from 0a640be to ad8d0ea Compare September 4, 2026 03:22
@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch 2 times, most recently from e1ede66 to 680f112 Compare September 4, 2026 04:54
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Sep 4, 2026
@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch from 680f112 to f534ae7 Compare September 4, 2026 07:20
@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch 3 times, most recently from efff454 to cf19766 Compare September 4, 2026 07:32
@juliusmarminge
juliusmarminge marked this pull request as ready for review September 4, 2026 07:35
@macroscopeapp

macroscopeapp Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The runtime changes are confined to test cleanup and host-temporary-directory portability, with no product request-path impact. However, the PR adds a file-level nodeBuiltinImport:off static-analysis suppression, so human review is required.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

No code changes detected at 8170118. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch from cf19766 to ff82da2 Compare September 4, 2026 17:42
@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch from ff82da2 to 679a18f Compare September 4, 2026 17:48
@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch from 679a18f to bc8fff5 Compare September 4, 2026 17:50
@juliusmarminge juliusmarminge changed the title test(server): let the fake Claude probe exit when its stdin closes test(server): keep provider fixture directories host-portable Sep 4, 2026

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit bc8fff5. Configure here.

Comment thread apps/server/src/provider/Layers/ClaudeCapabilitiesProbe.test.ts
@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch from bc8fff5 to 52e69f8 Compare September 4, 2026 18:07
… dir

The capabilities probe aborts the SDK query without waiting for the child
to exit, and on Windows a directory that is still some process's cwd cannot
be removed, so the scoped temp directory's rmdir hit EBUSY. Give the fake
CLI's workspace its own directory whose removal retries until the child has
gone.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@juliusmarminge
juliusmarminge force-pushed the windows-tests/temp-cleanup branch from 52e69f8 to 8170118 Compare September 4, 2026 20:26
@juliusmarminge
juliusmarminge merged commit 9af5139 into main Sep 4, 2026
35 of 43 checks passed
@juliusmarminge
juliusmarminge deleted the windows-tests/temp-cleanup branch September 4, 2026 20:33
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:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant