Skip to content

test: make workspace-load symlink tests compatible with Windows#97437

Open
aniruddhaadak80 wants to merge 2 commits into
openclaw:mainfrom
aniruddhaadak80:fix/workspace-load-symlink-skip-win32-final
Open

test: make workspace-load symlink tests compatible with Windows#97437
aniruddhaadak80 wants to merge 2 commits into
openclaw:mainfrom
aniruddhaadak80:fix/workspace-load-symlink-skip-win32-final

Conversation

@aniruddhaadak80

@aniruddhaadak80 aniruddhaadak80 commented Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Related: #97437

What Problem This Solves

Fixes an issue where several workspace-load symlink boundary tests were unconditionally skipped on Windows environments, even if the environment supports creating file and directory symlinks (like with Developer Mode enabled).

Why This Change Was Made

Replaces the hardcoded process.platform !== "win32" skips with dynamic capability checks (canCreateDirectorySymlinks and canCreateFileSymlinks). If file and directory symlinks are supported by the environment, the tests execute. Otherwise, they skip gracefully while keeping coverage active on capable hosts. Additionally, replaces literal "dir" arguments in symlink calls with the OS-appropriate directorySymlinkType to ensure tests run properly on Windows.

User Impact

No user-visible product impact. This improves test portability and preserves regression coverage on capable Windows environments.

Evidence

Tests run successfully on Windows when directory junctions are supported.
Test output from a Windows environment:
✓ src/skills/loading/workspace-load.test.ts (29) ✓ loadWorkspaceSkillEntries (29) ✓ loads direct skills under skills directory (skills/skill.md) ✓ keeps loading direct skills (skills/skill/SKILL.md) unchanged ... Test Files 1 passed (1) Tests 29 passed (29) Start at 08:04:21 Duration 2.41s
Code conforms to existing symlink capability patterns by using directorySymlinkType.

@clawsweeper

clawsweeper Bot commented Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Summary
The PR changes src/skills/loading/workspace-load.test.ts from Windows platform skips to symlink capability guards and Windows junction-aware directory links.

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

Reproducibility: yes. from source inspection: current main still guards the affected symlink boundary tests with process.platform !== "win32", so a capable Windows host skips them today. I did not run a Windows host in this read-only review.

Review metrics: 1 noteworthy metric.

  • Windows-gated tests: 9 guards changed. These tests can now run on Windows hosts with symlink or junction capability, which Linux-only CI does not exercise.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
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:

  • none.

Risk before merge

  • [P1] The Windows-specific junction branch is not exercised by the Linux CI lanes; maintainers are relying on the PR body's copied Windows output unless they request a stronger linked artifact or full transcript.

Maintainer options:

  1. Accept the focused Windows test coverage risk (recommended)
    Merge after normal review if maintainers accept the copied Windows output as enough proof for the Windows-only branch.
  2. Ask for a fuller Windows artifact
    Request a full command transcript, terminal screenshot, or linked artifact if maintainers want stronger auditability before enabling these cases.

Next step before merge

  • No ClawSweeper repair lane is needed because the concrete prior defects are fixed and the remaining action is normal maintainer and CI review.

Security
Cleared: The diff is limited to a test file and temporary symlink capability probes; I found no concrete security or supply-chain regression.

Review details

Best possible solution:

Land the focused test-only change after normal maintainer and CI review, with any broader symlink test helper extraction left for a later batch if this pattern spreads.

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

Yes from source inspection: current main still guards the affected symlink boundary tests with process.platform !== "win32", so a capable Windows host skips them today. I did not run a Windows host in this read-only review.

Is this the best way to solve the issue?

Yes. The latest head aligns the setup with the same directorySymlinkType capability pattern used by a sibling symlink test file, which is the narrow maintainable fix for this test-only portability gap.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied Windows live output showing the changed workspace-load test file passed after the patch.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied Windows live output showing the changed workspace-load test file passed after the patch.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P3: This is a low-risk test-portability cleanup with no user-visible product behavior change.
  • merge-risk: 🚨 automation: The diff changes when Windows-capable test hosts execute symlink boundary cases, and that branch is not independently covered by the observed Linux CI lanes.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied Windows live output showing the changed workspace-load test file passed after the patch.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied Windows live output showing the changed workspace-load test file passed after the patch.
Evidence reviewed

PR surface:

Tests +47. Total +47 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 1 63 16 +47
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 63 16 +47

What I checked:

Likely related people:

  • brokemac79: GitHub file history shows recent work touching src/skills/loading/workspace-load.test.ts for sandbox-readable workspace skills, which is adjacent to the workspace skill loading tests under review. (role: recent area contributor; confidence: medium; commits: 3b6bcbfb5045; files: src/skills/loading/workspace-load.test.ts)
  • shakkernerd: GitHub file history shows the skills subsystem layout refactor as an earlier central change to this test file and surrounding skills loading area. (role: subsystem refactor author; confidence: medium; commits: d9278c8efdb5; files: src/skills/loading/workspace-load.test.ts)
  • steipete: GitHub file history shows recent documentation and lint-related touches to the same test file, and this person is adjacent to current skills-loading maintenance. (role: recent adjacent contributor; confidence: low; commits: 43cee29f70c9, 304e2c83c01f; files: src/skills/loading/workspace-load.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-28T07:23:07.518Z sha 7d5f2f3 :: needs real behavior proof before merge. :: [P2] Use the junction-safe link type in this guarded test | [P2] Keep the configured-root symlink setup aligned with the guard

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. labels Jun 28, 2026
@BradGroux BradGroux mentioned this pull request Jun 28, 2026
Draft
@aniruddhaadak80

aniruddhaadak80 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 5, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 20, 2026

Copy link
Copy Markdown

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

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S stale Marked as stale due to inactivity status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aniruddhaadak80 @Patrick-Erichsen