test: make install-safe-path symlink tests compatible with Windows by aniruddhaadak80 · Pull Request #90275 · openclaw/openclaw

aniruddhaadak80 · 2026-06-04T09:36:53Z

Summary

Run the existing install-path symlink boundary tests on Windows when directory junctions are supported.
Use Windows junctions for directory links while preserving dir symlinks elsewhere.
Keep production install-path behavior unchanged.
Treat temporary-directory or cleanup failures in the capability probe as unsupported test environments instead of failing module import.

Linked context

No linked issue. This is a test portability improvement for existing install-path boundary coverage.

Real behavior proof

Behavior addressed: Three install-safe-path symlink boundary tests were unconditionally skipped on Windows.
Real environment tested: Native Windows Azure VM (Standard_D4ads_v6) through Crabbox.
Exact steps or command run after this patch: node scripts/run-vitest.mjs src/infra/install-safe-path.test.ts
Evidence after fix: Native Windows console output from Crabbox lease cbx_be4230e2069c, run run_0fb83e164185:

RUN  v4.1.8 C:/repo/openclaw

✓ infra src/infra/install-safe-path.test.ts (24 tests) 525ms

Test Files  1 passed (1)
Tests       24 passed (24)

Observed result after fix: The directory-junction cases executed successfully on native Windows instead of being skipped by platform.
What was not tested: No end-user install flow was exercised because the patch changes tests only.
Proof limitations or environment constraints: The tests still skip when the host cannot create directory links.
Before evidence: Current main uses it.runIf(process.platform !== "win32") for all three cases.

Tests and validation

node scripts/run-vitest.mjs src/infra/install-safe-path.test.ts
node scripts/run-oxlint.mjs src/infra/install-safe-path.test.ts
Native Windows Crabbox: 24/24 tests passed
Blacksmith Testbox tbx_01kv72nvfyz4fgpny8cyn48xfr: pnpm check:changed
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

Risk checklist

Did user-visible behavior change? No
Did config, environment, or migration behavior change? No
Did security, auth, secrets, network, or tool execution behavior change? No
Highest-risk area: Windows directory-link capability detection in the test harness.
Mitigation: Capability-gated execution plus direct native-Windows proof.

Current review state

Next action: Refresh CI on the rebased head and merge when required checks pass.
Addressed review comments: Temporary directory creation and cleanup are contained by the probe; module-level probing remains intentional because Vitest evaluates skipIf during test declaration.

Copilot

Pull request overview

Note

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

Updates the assertCanonicalPathWithinBase test suite to run symlink-related cases on Windows when directory symlinks (junctions) are supported, instead of unconditionally skipping Windows.

Changes:

Add a runtime probe (canCreateDirectorySymlinks) to detect whether directory symlinks/junctions can be created in the current environment.
Replace it.runIf(process.platform !== "win32") with it.skipIf(!canCreateDirectorySymlinks) to skip only when symlinks are unavailable.
Create directory links using the appropriate fs.symlink type (junction on Windows, dir elsewhere).

clawsweeper · 2026-06-04T09:39:03Z

Codex review: needs maintainer review before merge. Reviewed June 15, 2026, 10:59 AM ET / 14:59 UTC.

Summary
The PR changes src/infra/install-safe-path.test.ts to capability-gate directory symlink tests and use Windows junction links instead of skipping those cases on every Windows host.

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

Reproducibility: yes. Current main source still unconditionally skips the three symlink boundary tests on win32, and the PR body includes Windows terminal output showing the changed test file passing 24 tests.

Review metrics: none identified.

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:

[P2] Refresh or rebase onto current main and require fresh required checks before merge.

Risk before merge

[P1] The live PR rollup still contains old failing checks on a stale merge commit, including unrelated plugin-contract and lint failures; maintainers should rely on fresh required checks after refresh/rebase before merging.

Maintainer options:

Decide the mitigation before merge
Land the narrow test portability patch after maintainer review and fresh required checks, keeping production install-path helpers unchanged.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No ClawSweeper repair lane is needed; the branch is reviewable as-is and needs maintainer review plus fresh required checks.

Security
Cleared: The diff only changes a test file and adds no dependency, workflow, secret, package, or runtime code-execution surface.

Review details

Best possible solution:

Land the narrow test portability patch after maintainer review and fresh required checks, keeping production install-path helpers unchanged.

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

Yes. Current main source still unconditionally skips the three symlink boundary tests on win32, and the PR body includes Windows terminal output showing the changed test file passing 24 tests.

Is this the best way to solve the issue?

Yes. Capability-gating these tests and using Windows junctions is the narrow test-layer fix; production install-path helpers and callers stay unchanged.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
remove rating: 🦞 diamond lobster: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

P3: This is a low-risk test-only Windows compatibility cleanup with no runtime, config, provider, or user-facing behavior change.
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-fix Windows terminal output showing src/infra/install-safe-path.test.ts passing 24 tests.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix Windows terminal output showing src/infra/install-safe-path.test.ts passing 24 tests.

Evidence reviewed

PR surface:

Tests +17. Total +17 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	0	0	0	0
Tests	1	23	6	+17
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	23	6	+17

What I checked:

Repository policy read: Root AGENTS.md was read in full; no scoped src/infra/AGENTS.md exists, so the root review policy and test/dependency guidance apply. (AGENTS.md:1, c1219d161d3e)
Current main behavior: Current main still uses it.runIf(process.platform !== "win32") for the three assertCanonicalPathWithinBase symlink boundary cases, so the PR is not obsolete on main. (src/infra/install-safe-path.test.ts:158, c1219d161d3e)
PR diff is narrow and test-only: The PR adds a directory-link capability probe, swaps the three Windows platform skips to it.skipIf(!canCreateDirectorySymlinks), and passes junction on Windows when creating directory links. (src/infra/install-safe-path.test.ts:6, 04f3e2bd062a)
Runtime boundary checked: The production install-safe-path module only imports fs-safe defaults, re-exports fs-safe install path helpers, and implements package-name helpers; the PR does not change production install behavior. (src/infra/install-safe-path.ts:1, c1219d161d3e)
Caller path checked: Install package publish validation calls assertCanonicalPathWithinBase through assertInstallBoundaryPaths, confirming the changed tests cover a real install boundary while leaving callers untouched. (src/infra/install-package-dir.ts:87, c1219d161d3e)
Sibling pattern checked: src/plugins/discovery.test.ts already uses a module-level canCreateDirectorySymlinks probe and a helper that creates Windows directory links as junction, matching the PR's test pattern. (src/plugins/discovery.test.ts:55, c1219d161d3e)

Likely related people:

steipete: Peter Steinberger authored the March 2026 commits that expanded and tightened install-safe-path coverage, and git shortlog shows the most history in the central install-safe-path files. (role: introduced behavior and feature-history owner; confidence: high; commits: 0f4855624177, 1ae216341341, 104d32bb64cd; files: src/infra/install-safe-path.test.ts, src/infra/install-safe-path.ts, src/infra/install-package-dir.ts)
vincentkoc: Vincent Koc appears in recent history for src/infra/install-safe-path.test.ts and src/infra/install-safe-path.ts, including the latest release commit that carried these files. (role: recent area contributor; confidence: medium; commits: 8c802aa68351, c6611639ab9c; files: src/infra/install-safe-path.test.ts, src/infra/install-safe-path.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.

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

@clawsweeper re-review

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

🦞🧹
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/26945004240
Updated: 2026-06-04T10:09:24.351Z

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

vincentkoc · 2026-06-16T02:12:43Z

Land-ready verification for head 53f987d3ae8cbdd9ca3c155387998997d6606967:

Best-fix verdict: best. This keeps the platform capability handling in the test layer and leaves production install-path behavior unchanged.
Local focused proof: node scripts/run-vitest.mjs src/infra/install-safe-path.test.ts — 24/24 passed; oxlint passed.
Native Windows proof: Crabbox lease cbx_be4230e2069c, run run_0fb83e164185 on Standard_D4ads_v6 — 24/24 passed with the junction cases executing.
Changed gate: Blacksmith Testbox tbx_01kv72nvfyz4fgpny8cyn48xfr — pnpm check:changed passed. Hydration: https://github.com/openclaw/openclaw/actions/runs/27589091984
Real behavior proof: https://github.com/openclaw/openclaw/actions/runs/27589285092 — passed.
CodeQL: https://github.com/openclaw/openclaw/actions/runs/27589207773 — passed.
Autoreview: local delta and full branch review both clean; no accepted/actionable findings.
Review conversations: 0 unresolved.

The broad CI matrix has one unrelated failure in src/commands/migrate/apply.test.ts: its mock omits the existing resolveGatewayPort export. This PR changes only src/infra/install-safe-path.test.ts; the identical checks-node-agentic-commands-status-tools shard passed on the exact base SHA a4e7d9a0dbbf04b4932169bece25dc29023640d6: https://github.com/openclaw/openclaw/actions/runs/27588149842/job/81563012979

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

openclaw-barnacle Bot added size: XS 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

Comment thread src/infra/install-safe-path.test.ts

Comment thread src/infra/install-safe-path.test.ts

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

clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 4, 2026

BradGroux mentioned this pull request Jun 4, 2026

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

Draft

aniruddhaadak80 requested a review from Copilot June 4, 2026 14:22

Copilot AI reviewed Jun 4, 2026

View reviewed changes

Comment thread src/infra/install-safe-path.test.ts

clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jun 15, 2026

vincentkoc self-assigned this Jun 16, 2026

github-actions Bot mentioned this pull request Jun 16, 2026

🦞 OpenClaw 生态日报 2026-06-16 96loveslife/big_model_radar#7

Open

test: make install-safe-path symlink tests compatible with Windows

53f987d

vincentkoc force-pushed the fix/install-safe-path-symlink-skip-win32 branch from 04f3e2b to 53f987d Compare June 16, 2026 02:06

vincentkoc merged commit 11a0ad1 into openclaw:main Jun 16, 2026
311 of 318 checks passed

Haderach-Ram mentioned this pull request Jun 16, 2026

Ecosystem Digest — 2026-06-16 Haderach-Ram/openclaw-radar#47

Open

Haderach-Ram mentioned this pull request Jun 16, 2026

Ecosystem Digest — 2026-06-16 Haderach-Ram/openclaw-radar#48

Closed

This was referenced Jun 28, 2026

test(browser): replace broad win32 skip with dynamic directory symlink check #90365

Merged

test: make refresh symlink tests compatible with Windows #97438

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

test: make install-safe-path symlink tests compatible with Windows#90275

test: make install-safe-path symlink tests compatible with Windows#90275
vincentkoc merged 1 commit into
openclaw:mainfrom
aniruddhaadak80:fix/install-safe-path-symlink-skip-win32

aniruddhaadak80 commented Jun 4, 2026 •

edited by vincentkoc

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

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!

Copilot AI left a comment

Uh oh!

Uh oh!

vincentkoc commented Jun 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

Linked context

Real behavior proof

Tests and validation

Risk checklist

Current review state

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

vincentkoc commented Jun 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aniruddhaadak80 commented Jun 4, 2026 •

edited by vincentkoc

Loading

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading