test: make fs-safe symlink tests compatible with Windows by aniruddhaadak80 · Pull Request #90271 · openclaw/openclaw

aniruddhaadak80 · 2026-06-04T09:20:39Z

What Problem This Solves

The fs-safe symlink-related tests previously hardcoded skips for Windows platforms (win32). This caused a loss of test coverage on Windows configurations where symlinking is actually supported (e.g. Developer Mode or elevated privileges).

Why This Change Was Made

Instead of assuming win32 lacks symlink support, we dynamically probe for file and directory symlink capabilities via canCreateFileSymlinks and canCreateDirectorySymlinks. If the capability is present, the tests run, providing test coverage when possible. Additionally, it uses directory junction on Windows for compatibility without Administrator privileges. Temporary directories are used for the probes to prevent test pollution.

User Impact

No direct end-user impact. Improves test robustness and coverage for Windows contributors.

Evidence

The tests were run locally on a Windows host. The dynamic probing successfully skipped only the file symlink tests (due to host permissions) but passed the directory junction tests without dirtying the global state:

 RUN  v4.1.8 ...

 ?  infra  src/infra/fs-safe.test.ts (34 tests | 10 skipped) 3652ms

 Test Files  1 passed (1)
      Tests  24 passed | 10 skipped (34)

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Makes symlink-related fs-safe tests conditional on actual symlink capability (rather than OS-only checks), improving cross-platform reliability (notably on Windows where symlink creation may be restricted).

Changes:

Add runtime probes to detect whether file and directory symlinks can be created.
Replace it.runIf(process.platform !== "win32") with it.skipIf(!canCreate*Symlinks) for symlink tests.
Use explicit symlink types for directory symlinks (junction on Windows, dir elsewhere).

+const canCreateFileSymlinks = (() => {
+  const probeDir = fsSync.mkdtempSync(path.join(os.tmpdir(), "openclaw-fs-file-symlink-probe-"));
+  const targetFile = path.join(probeDir, "target.txt");
+  const linkFile = path.join(probeDir, "link.txt");
+  try {
+    fsSync.writeFileSync(targetFile, "target", "utf8");
+    fsSync.symlinkSync(targetFile, linkFile, "file");
+    return true;
+  } catch {
+    return false;
+  } finally {
+    fsSync.rmSync(probeDir, { recursive: true, force: true });
+  }
+})();


+const canCreateDirectorySymlinks = (() => {
+  const probeDir = fsSync.mkdtempSync(path.join(os.tmpdir(), "openclaw-fs-dir-symlink-probe-"));
+  const targetDir = path.join(probeDir, "target");
+  const linkDir = path.join(probeDir, "link");
+  try {
+    fsSync.mkdirSync(targetDir);
+    fsSync.symlinkSync(targetDir, linkDir, process.platform === "win32" ? "junction" : "dir");
+    return true;
+  } catch {
+    return false;
+  } finally {
+    fsSync.rmSync(probeDir, { recursive: true, force: true });
+  }
+})();


clawsweeper · 2026-06-04T09:22:22Z

Codex review: needs changes before merge. Reviewed July 5, 2026, 4:37 AM ET / 08:37 UTC.

Summary
The PR changes src/infra/fs-safe.test.ts to probe symlink capability and run eligible symlink tests on Windows instead of skipping every Windows host.

PR surface: Tests +32. Total +32 across 1 file.

Reproducibility: yes. at source level. Current main still hard-skips the affected fs-safe symlink tests on Windows, and the PR body includes Windows terminal output after the change.

Review metrics: 1 noteworthy metric.

Windows symlink gates: 6 tests changed. The PR replaces blanket Windows skips with capability skips for the affected fs-safe symlink coverage.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Guard both probe cleanup calls and rerun the focused fs-safe test/lint checks.

Risk before merge

[P1] The new module-load symlink probes can still make Windows test discovery fail if temp-dir cleanup throws, even though the change is test-only.

Maintainer options:

Guard probe cleanup before merge (recommended)
Wrap both probe cleanup calls so a Windows cleanup failure cannot abort fs-safe test discovery.
Accept the test-only flake risk
Maintainers could merge the current branch as-is because the risk is limited to tests, but that leaves a known Windows import-failure path.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Guard both symlink probe cleanup paths in `src/infra/fs-safe.test.ts` by tracking `probeDir` and wrapping `fsSync.rmSync(probeDir, { recursive: true, force: true })` in its own try/catch, matching `src/infra/install-safe-path.test.ts`; keep the change test-only and run `node scripts/run-oxlint.mjs src/infra/fs-safe.test.ts` plus `node scripts/run-vitest.mjs src/infra/fs-safe.test.ts`.

Next step before merge

[P2] A narrow automated repair can guard the two cleanup calls in one test file; no product or ownership decision is required.

Security
Cleared: The diff is limited to a test file and does not change production code, dependencies, workflows, secrets handling, package resolution, or release surfaces.

Review findings

[P3] Guard symlink probe cleanup failures — src/infra/fs-safe.test.ts:20

Review details

Best possible solution:

Land the test-only portability change after guarding both probe cleanups to match the existing install-safe-path probe pattern and rerunning focused lint/test proof.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level. Current main still hard-skips the affected fs-safe symlink tests on Windows, and the PR body includes Windows terminal output after the change.

Is this the best way to solve the issue?

Yes, with a small cleanup. Capability-gated symlink tests with Windows junctions are the narrow maintainable test-only fix, but the probe cleanup should be guarded before merge.

Full review comments:

[P3] Guard symlink probe cleanup failures — src/infra/fs-safe.test.ts:20
Both new capability probes run while the test module is being declared, but rmSync() in the finally block is unguarded. On Windows, a transient cleanup failure can abort importing this test file instead of letting the capability check fall back to unsupported; mirror the guarded cleanup already used in src/infra/install-safe-path.test.ts.
Confidence: 0.82

Overall correctness: patch is correct
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 53bd4dde6c00.

Label changes

Label justifications:

P3: This is a low-risk test-only portability improvement with no production runtime, config, or user-data impact.
merge-risk: 🚨 automation: The diff adds module-load filesystem probes whose unguarded cleanup can fail test discovery on Windows hosts.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-change Windows terminal output for the focused fs-safe test file showing passed and skipped tests.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-change Windows terminal output for the focused fs-safe test file showing passed and skipped tests.

Evidence reviewed

PR surface:

Tests +32. Total +32 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	0	0	0	0
Tests	1	41	9	+32
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	41	9	+32

Acceptance criteria:

[P1] node scripts/run-oxlint.mjs src/infra/fs-safe.test.ts.
[P1] node scripts/run-vitest.mjs src/infra/fs-safe.test.ts.

What I checked:

Current main behavior: Current main still hard-skips the affected fs-safe symlink tests on Windows with it.runIf(process.platform !== "win32"). (src/infra/fs-safe.test.ts:166, 53bd4dde6c00)
PR behavior: The PR branch adds file and directory symlink capability probes, then gates the affected symlink tests with it.skipIf(!canCreate...). (src/infra/fs-safe.test.ts:9, 4b26b1d2c751)
Remaining patch defect: Both new probe finally blocks call fsSync.rmSync(...) without catching cleanup errors. (src/infra/fs-safe.test.ts:20, 4b26b1d2c751)
Sibling pattern: The existing install-safe-path symlink capability probe runs before test declaration and guards temp-dir cleanup with its own try/catch. (src/infra/install-safe-path.test.ts:19, 53bd4dde6c00)
Real behavior proof: The PR body includes after-change Windows terminal output for the focused fs-safe test file showing 24 passed and 10 skipped. (4b26b1d2c751)
Related search: Live GitHub title/body searches for the same fs-safe Windows symlink test problem did not find a narrower canonical PR or issue; the only related context is the broad Windows tracker.

Likely related people:

steipete: Recent commit history shows repeated fs-safe and install-path safety test/helper work in this area. (role: feature owner / area contributor; confidence: high; commits: 334a1dd71698, 1365b71f0285, c659590d22eb; files: src/infra/fs-safe.test.ts, src/infra/fs-safe.ts, src/infra/install-safe-path.test.ts)
shakkernerd: Recent history for src/infra/fs-safe.test.ts includes fs-safe fixture and rejection assertion hardening. (role: recent adjacent test contributor; confidence: medium; commits: 986025afe425, 896e2edd59ab; files: src/infra/fs-safe.test.ts)
aniruddhaadak80: The author also has merged history introducing the closest sibling Windows symlink capability probe pattern in install-safe-path tests. (role: sibling portability contributor; confidence: medium; commits: 11a0ad10e91a; files: src/infra/install-safe-path.test.ts, src/infra/fs-safe.test.ts)
vincentkoc: Recent adjacent history touches install-path and fs-safe infrastructure near this test surface. (role: recent adjacent contributor; confidence: medium; commits: c6611639ab9c, 5911b5bf2d4c; files: src/infra/install-safe-path.test.ts, src/infra/fs-safe.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (1 earlier review cycle)

reviewed 2026-06-28T17:59:17.233Z sha 4b26b1d :: needs changes before merge. :: [P3] Guard symlink probe cleanup failures

aniruddhaadak80 · 2026-06-04T10:03:17Z

@clawsweeper re-review

clawsweeper · 2026-06-04T10:03:20Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26945002434
Updated: 2026-06-04T10:09:35.132Z

aniruddhaadak80 · 2026-06-28T17:48:46Z

Rebased onto main. Ready for re-review.

aniruddhaadak80 · 2026-07-05T08:22:27Z

@clawsweeper re-review

clawsweeper · 2026-07-05T08:22:30Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

openclaw-barnacle · 2026-07-20T04:40:30Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

Copilot AI review requested due to automatic review settings June 4, 2026 09:20

openclaw-barnacle Bot added size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 4, 2026

Copilot AI reviewed Jun 4, 2026

View reviewed changes

openclaw-barnacle Bot removed the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label Jun 4, 2026

BradGroux mentioned this pull request Jun 4, 2026

WORKING: All Microsoft Issues and PRs (refresh) #74163

Draft

test: make fs-safe symlink tests compatible with Windows

4b26b1d

aniruddhaadak80 force-pushed the fix/fs-safe-symlink-skip-win32 branch from ddee9ca to 4b26b1d Compare June 28, 2026 17:48

openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 28, 2026

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 5, 2026

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 20, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

test: make fs-safe symlink tests compatible with Windows#90271

test: make fs-safe symlink tests compatible with Windows#90271
aniruddhaadak80 wants to merge 1 commit into
openclaw:mainfrom
aniruddhaadak80:fix/fs-safe-symlink-skip-win32

aniruddhaadak80 commented Jun 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

aniruddhaadak80 commented Jun 4, 2026

Uh oh!

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

aniruddhaadak80 commented Jun 28, 2026

Uh oh!

aniruddhaadak80 commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

openclaw-barnacle Bot commented Jul 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

aniruddhaadak80 commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

clawsweeper Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aniruddhaadak80 commented Jun 4, 2026

Uh oh!

clawsweeper Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aniruddhaadak80 commented Jun 28, 2026

Uh oh!

aniruddhaadak80 commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openclaw-barnacle Bot commented Jul 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aniruddhaadak80 commented Jun 4, 2026 •

edited

Loading

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading