[Irish-Joseph]

What Problem This Solves

Fixes: 28 of 93 packages/clawhub/src/cli/commands/skills.test.ts tests fail on Windows (0 failures on Linux), which blocks Windows-based contributors from getting a green CLI test baseline.

User Impact

No user-visible change. Contributors on Windows can run the skills install/update test suite and see the real pass/fail state instead of 28 spurious failures.

Why This Change Was Made

The tests inject the POSIX literal workdir /work (via the shared makeGlobalOpts default) and assert against POSIX literals like /work/skills/html. On Windows the implementation correctly joins onto the platform rendering of that workdir, so every path assertion mismatched (\work\skills\html vs /work/skills/html). The real CLI never passes an unresolved literal — resolveWorkdir() always returns a platform-resolved absolute path — so the test-side fix is to model that: the workdir is now derived with resolve("/work") and all expectations are built from the same platform path via a small workPath() helper. Temp/backup-directory matchers accept either separator. Linux behavior is unchanged by construction: on POSIX, resolve("/work") is /work and join("/work", …) reproduces the old literals byte-for-byte. Implementation code is untouched.

Evidence

• Before (Windows, unmodified main): skills.test.ts → 65 passed / 28 failed, e.g. expected "writeSkillOrigin" to be called with '/work/skills/html' … received '\work\skills\html'.
• After (same Windows host): 93 passed (93).
• Linux (ubuntu 24.04 container, Bun 1.4.2): same file → 93 passed (93) — byte-identical expectations on POSIX, so no Linux behavior change.
• Full packages/clawhub source suite on this host: 81 failed / 380 passed vs 109 failed / 352 passed on unmodified main — the delta is exactly the 28 repaired tests; the remaining per-file failure set is byte-identical to main (pre-existing Windows-environment failures that spawn real npm/bun, which pass on Linux CI).

Real behavior: built-CLI install / force-reinstall / update on Windows

The corrected assertions document where install/update write on a platform-resolved workdir; the same code paths run through the real built CLI (v0.24.0, this branch's head) on this Windows host against the public registry, into a scratch workdir:

> node dist\cli.js --workdir D:\skills-evidence\work install steipete/github
Installed github v1.0.0 -> D:\skills-evidence\work\skills\@steipete\github

Resulting on-disk state (real Windows paths, owner-scoped directory):

D:\skills-evidence\work\skills\@steipete\github\SKILL.md
D:\skills-evidence\work\skills\@steipete\github\_meta.json
D:\skills-evidence\work\skills\@steipete\github\skill-card.md
D:\skills-evidence\work\skills\@steipete\github\.clawhub\origin.json

.clawhub\lock.json:

{
  "version": 1,
  "skills": {
    "@steipete/github": { "version": "1.0.0", "installedAt": 1791258547733, "ownerHandle": "steipete" }
  }
}

.clawhub\origin.json:

{
  "version": 1,
  "registry": "https://clawhub.ai",
  "slug": "github",
  "ownerHandle": "steipete",
  "installedVersion": "1.0.0",
  "installedAt": 1791258547733,
  "fingerprint": "6a19a2b0…"
}

Staging-swap path (the backup/rename flow the tests assert) via --force reinstall of the existing install:

> node dist\cli.js --workdir D:\skills-evidence\work install steipete/github --force
Installed github v1.0.0 -> D:\skills-evidence\work\skills\@steipete\github

(no leftover .backup-* directory; clean swap) and the update flow:

> node dist\cli.js --workdir D:\skills-evidence\work update @steipete/github
@steipete/github up to date v1.0.0

• bun run ci:static passes; tsc -p packages/clawhub --noEmit, oxlint --type-aware, oxfmt --check clean.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[vercel]

@Irish-Joseph is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel.

A member of the Team first needs to authorize it.

[clawsweeper]

Codex review: needs changes before merge.

What this changes

This PR replaces POSIX-only skills CLI test fixtures and expectations with platform-native paths.

Example: A Windows contributor runs the skills CLI tests from drive D.

• Before: An assertion expects /work/skills/html while the command produces a Windows path, causing a false failure.
• After: The fixture and expectation both use D:\work\skills\html, preserving the exact target-directory check.

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	A focused, source-verified test repair with relevant Windows CLI output and cross-platform test results; the reported conflict affects landing readiness rather than patch quality.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (live_output): The supplied transcript exercises the built Node CLI on Windows through install and force-reinstall, showing platform-native owner-scoped files and matching lockfile state; the update command shows an up-to-date result, and the reported Windows/Linux test runs exercise the corrected assertions.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Windows contributors receive false failures from skills CLI tests despite correct platform-native command paths.
Reason: The focused change removes platform-specific false failures while retaining exact destinations, metadata, replacement ordering, and cleanup-failure coverage.

Merge readiness

⛔ Needs changes before merge - 1 item remains

This PR repairs a concrete Windows test failure that remains on current main, without changing CLI behavior. No blocking code findings; GitHub still reports a merge conflict.

Priority: P3
Reviewed head: 663c6eea1421f942f6068194e99401bcbfa1edc0

Before merge

• Complete next step - Resolve GitHub’s reported merge conflict, preserve current main’s Unreleased entries and this test repair, and validate any changed test resolution on Windows and Linux.

Findings

None.

Agent review details

How this fits together

The skills command tests pass mocked registry and filesystem state into real CLI command handlers, then check install destinations, metadata, lockfiles, and replacement ordering.

flowchart TD
  A[Test workdir fixture] --> B[Global command options]
  B --> C[Skills command handlers]
  D[Mock registry responses] --> C
  C --> E[Filesystem and metadata operations]
  E --> F[Platform-native assertions]

Loading

Technical review

Best possible solution:

Land the platform-native test repair after resolving the conflict while preserving current main’s changelog entries.

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

Yes: current main combines platform-joined command paths with POSIX literal expectations, and the contributor reports the corresponding Windows failures and passing candidate results; no tests were run during this read-only review.

Is this the best way to solve the issue?

Best fix: resolve the test fixture and derive exact expectations with platform path functions; changing production paths would violate the CLI contract, while broad separator normalization would weaken assertions unnecessarily.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 0bf64843c697.

Provenance checked

• Shared test workdir fixture keeps the original intent (a8a687e: The shared-fixture refactor centralized CLI mocks and options; its earlier blob already used join(workdir, "skills") with a /work default.)
• Install/update target and metadata assertions keeps the original intent (a9a80bb: Skills.sh and owner-qualified install coverage verifies target locations and source identity; the patch preserves those assertions and metadata.)
• Staging and rollback assertions keeps the original intent (7d69b33: Rollback-safe replacements require staging before swapping existing installs; the patch retains operation order and target checks.)
• Backup-removal failure regression keeps the original intent (fix: keep completed skill installs when backup cleanup fails #3853: A completed replacement must remain installed when deleting its backup fails; the patch preserves that regression coverage.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

• Introduced change: The pinned introduction diff changes only CHANGELOG.md and skills.test.ts; exact path assertions and backup matchers become platform-aware while command implementation remains untouched. (packages/clawhub/src/cli/commands/skills.test.ts:106, 663c6eea1421)
• Current main still needs the repair: Current main still calls makeGlobalOpts() without a resolved workdir and retains literal /work/skills paths; it does not contain this test repair. (packages/clawhub/src/cli/commands/skills.test.ts:106, 0bf64843c697)
• Production path contract: resolveWorkdir resolves explicit, environment, workspace, and cwd choices to absolute paths; skillTarget and staging operations use platform path functions. (packages/clawhub/src/cli.ts:188, 663c6eea1421)
• Contributor proof: The full live PR body reports Windows results improving from 65 passing and 28 failing to 93 passing, with 93 passing on Linux, and includes built Node CLI Windows install, force-reinstall, filesystem, lockfile, and up-to-date output. (663c6eea1421)
• Preserved backup-cleanup contract: The merged fix: keep completed skill installs when backup cleanup fails #3853 preserves completed replacements when backup removal fails; this PR retains its two-rename and lockfile assertions while changing only their path representation. (packages/clawhub/src/cli/commands/skills.test.ts:2931, d26329b44c98)
• Feature history: History across clawhub, clawdhub, and molthub paths identifies longstanding CLI coverage, shared-fixture refactoring, rollback-safe replacements, and later skills.sh work; ownership is shared rather than established by the current PR author. (packages/clawhub/src/cli/commands/skills.test.ts, 0bf64843c697)

Likely related people:

• Peter Steinberger: Raw commit d26329b adds packages/clawhub/src/cli/commands/skills.test.ts:2920 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: d26329b44c98; files: packages/clawhub/src/cli/commands/skills.test.ts)
• Patrick Erichsen: Raw commit a9a80bb adds packages/clawhub/src/cli/commands/skills.test.ts:1848 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: a9a80bbf6d9a; files: packages/clawhub/src/cli/commands/skills.test.ts)

Review metrics

Metric	Value	Why it matters
Change scope	Production +0/-0; tests +86/-71; changelog +4/-0	The change preserves runtime behavior and updates existing coverage rather than adding a parallel implementation.

Labels

Label changes:

No label changes.

Label justifications:

• P3: This repairs contributor test portability without changing installed-user behavior.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.
• proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (46 earlier review cycles; latest 8 shown)

• reviewed 2026-10-11T04:42:26.845Z sha 663c6ee :: needs changes before merge. :: none
• reviewed 2026-10-11T17:34:18.132Z sha 663c6ee :: needs changes before merge. :: none
• reviewed 2026-10-12T13:11:44.219Z sha 663c6ee :: needs changes before merge. :: none
• reviewed 2026-10-12T15:10:50.822Z sha 663c6ee :: needs changes before merge. :: none
• reviewed 2026-10-13T09:46:21.913Z sha 663c6ee :: needs changes before merge. :: none
• reviewed 2026-10-13T17:37:48.287Z sha 663c6ee :: needs changes before merge. :: none
• reviewed 2026-10-13T21:33:21.212Z sha 663c6ee :: needs changes before merge. :: none
• reviewed 2026-10-14T07:42:26.520Z sha 663c6ee :: needs changes before merge. :: none

Reviewed October 14, 2026, 7:37 AM ET / 11:37 UTC (Revision 47).

[github-actions]

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