Skip to content

test: make qqbot symlinked media helper test robust on Windows#90223

Merged
vincentkoc merged 3 commits into
openclaw:mainfrom
aniruddhaadak80:fix/qqbot-symlink-skip-win32
Jun 22, 2026
Merged

test: make qqbot symlinked media helper test robust on Windows#90223
vincentkoc merged 3 commits into
openclaw:mainfrom
aniruddhaadak80:fix/qqbot-symlink-skip-win32

Conversation

@aniruddhaadak80

@aniruddhaadak80 aniruddhaadak80 commented Jun 4, 2026 •
edited by vincentkoc
Loading

Copy link
Copy Markdown
Contributor

Replaces the hardcoded Windows skip in the QQ Bot file-utils test with a dynamic file-symlink capability check. If file symlinks are supported by the environment, the test executes. Otherwise, it skips gracefully while keeping coverage active on capable hosts.

What Problem This Solves

The symlinked local-media helper test should reject symlinked media paths when the runtime can create file symlinks, but it should not fail the suite on Windows or restricted environments where file symlink creation is unavailable. Gating the test on actual capability avoids false negatives while preserving the security regression coverage where the behavior can be exercised.

Evidence

  • Windows Vitest proof from the contributor: extensions/qqbot/src/engine/utils/file-utils.test.ts completed with 1 passed test file, 4 passed tests, and 1 skipped symlink test when file symlink creation was unavailable.
  • The follow-up repair commit cb7d5a162e24f7ec5be6985e97b2b74ae45b20f9 changes the probe to async fs.promises APIs and skips solely on !canCreateFileSymlinks, which addresses the stale Copilot comments about non-Windows restricted environments and synchronous import-time filesystem work.
  • Current PR CI is otherwise green; the remaining failed check was the external-PR body proof gate requiring these authored sections.

Copilot AI review requested due to automatic review settings June 4, 2026 07:35
@openclaw-barnacle openclaw-barnacle Bot added channel: qqbot 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

Copilot AI 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.

Pull request overview

Note

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

Updates the qqbot downloadFile test suite to conditionally skip the symlink-related test based on whether the environment can create file symlinks.

Changes:

  • Added a symlink capability probe (canCreateFileSymlinks) using a temp directory.
  • Adjusted the symlink test’s skip condition to be more selective on Windows.

Comment thread extensions/qqbot/src/engine/utils/file-utils.test.ts Outdated
Comment thread extensions/qqbot/src/engine/utils/file-utils.test.ts Outdated
@clawsweeper

clawsweeper Bot commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 20, 2026, 6:58 AM ET / 10:58 UTC.

Summary
The PR changes the QQ Bot file-utils test to probe file-symlink capability and skip the symlink rejection case only when the environment cannot create file symlinks.

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

Reproducibility: yes. for the test behavior: current main and v2026.6.8 unconditionally skip the symlink rejection case on Windows, while PR head uses a capability probe. I did not run tests because this was a read-only review.

Review metrics: none identified.

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.

Next step before merge

  • No ClawSweeper repair lane is needed because there are no actionable code findings; the remaining work is ordinary maintainer and required-check review.

Security
Cleared: The diff only changes a test skip gate using a temporary local filesystem probe and introduces no runtime, dependency, workflow, credential, or supply-chain surface.

Review details

Best possible solution:

Land the focused test-only capability gate after ordinary maintainer and required-check review, leaving QQ Bot runtime behavior unchanged.

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

Yes for the test behavior: current main and v2026.6.8 unconditionally skip the symlink rejection case on Windows, while PR head uses a capability probe. I did not run tests because this was a read-only review.

Is this the best way to solve the issue?

Yes. A local file-symlink capability gate is the narrow maintainable fix for this test, and it avoids the broad platform skip while keeping runtime code unchanged.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P3: This is a low-risk test robustness change with no runtime, configuration, migration, or user-facing behavior change.
  • 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 (terminal): The PR body includes after-patch Windows terminal output showing the focused QQ Bot file-utils test file passing with the symlink case skipped on a host without file-symlink permission.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-patch Windows terminal output showing the focused QQ Bot file-utils test file passing with the symlink case skipped on a host without file-symlink permission.
Evidence reviewed

PR surface:

Tests +15. Total +15 across 1 file.

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

What I checked:

Likely related people:

  • sliverp: Introduced the QQ Bot channel and later updated the QQ Bot media allowlist/file-utils area that this test covers. (role: QQ Bot feature history contributor; confidence: medium; commits: bf6f506dfae6, ddb7a8dd80b8; files: extensions/qqbot/src/utils/file-utils.ts, extensions/qqbot/src/utils/file-utils.test.ts, extensions/qqbot)
  • steipete: Local shortlog shows the heaviest history on the QQ Bot file-utils paths, including bundled extension test optimization and nearby helper refactors. (role: QQ Bot area refactor contributor; confidence: medium; commits: af954a81d13e, aec24f4599be, ba68537d9d59; files: extensions/qqbot/src/engine/utils/file-utils.ts, extensions/qqbot/src/engine/utils/file-utils.test.ts, extensions/qqbot/src/utils/file-utils.ts)
  • vincentkoc: Current main blame and recent commits carry the present engine file-utils test layout, making this a recent routing signal for the touched path. (role: recent adjacent contributor; confidence: low; commits: 22f696d010f5, 844f405ac1be; files: extensions/qqbot/src/engine/utils/file-utils.ts, extensions/qqbot/src/engine/utils/file-utils.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.

@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. 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 Jun 4, 2026
@aniruddhaadak80

aniruddhaadak80 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

@clawsweeper

clawsweeper Bot commented Jun 4, 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.

Re-review progress:

@openclaw-barnacle 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 BradGroux mentioned this pull request Jun 4, 2026
Draft
@vincentkoc vincentkoc mentioned this pull request Jun 21, 2026
Merged
@clawsweeper clawsweeper Bot mentioned this pull request Jun 21, 2026
Closed
@vincentkoc
vincentkoc force-pushed the fix/qqbot-symlink-skip-win32 branch from 0a22111 to 1a183e2 Compare June 22, 2026 23:22
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 22, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 22, 2026
@vincentkoc vincentkoc self-assigned this Jun 22, 2026
@vincentkoc
vincentkoc merged commit 82316c2 into openclaw:main Jun 22, 2026
91 of 95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

@vincentkoc vincentkoc

Labels

channel: qqbot 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: XS 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.

3 participants

@aniruddhaadak80 @vincentkoc