Skip to content

fix(tests): make clawdbot config tests pass on Windows - #3868

Open
Irish-Joseph wants to merge 1 commit into
openclaw:mainfrom
Irish-Joseph:fix/clawdbot-config-windows-tests
Open

Irish-Joseph wants to merge 1 commit into
openclaw:mainfrom
Irish-Joseph:fix/clawdbot-config-windows-tests

Conversation

@Irish-Joseph

@Irish-Joseph Irish-Joseph commented Sep 30, 2026 •

Copy link
Copy Markdown

What Problem This Solves

Fixes: bun run --cwd packages/clawhub test:src fails 7 of 8 clawdbotConfig.test.ts tests on Windows (0 failures on Linux), which blocks Windows-based contributors from getting a green CLI test baseline before making any other change.

User Impact

No user-visible change. Contributors on Windows can run the clawhub CLI test suite and see the real pass/fail state instead of 7 spurious failures in src/cli/clawdbotConfig.test.ts.

Why This Change Was Made

Two test-fixture/platform issues, implementation code untouched:

  1. Unescaped backslashes in JSON5 fixtures. Workspace paths are interpolated into JSON5 string literals; on Windows the \ is parsed as an escape sequence, silently corrupting the value (JSON5 tolerates the invalid escapes instead of erroring). The fixtures now escape backslashes before interpolation.
  2. USERPROFILE shadowing the HOME fallback. resolveHome() intentionally prefers USERPROFILE on Windows, but the two tests documenting the HOME fallback only set HOME — on Windows the real user profile won. Those tests now clear USERPROFILE (restored by the existing afterEach) so they keep targeting the fallback they document.

Evidence

  • Real behavior (built CLI on Windows): from a neutral directory with no --workdir and no .clawhub marker, clawhub sync --all --dry-run with a Clawdbot config present resolves and scans the configured workspace — Roots with skills: D:\ws-evidence\workspace-main\skills → Dry run: would publish 1 skill(s); the control run without the config falls back to cwd — Error: No skills found (checked configured roots). Full transcript in <details> below.
  • Unit suite: before (Windows, unmodified main): 1 passed / 7 failed (e.g. expected 'D:ws…' to be 'D:\ws\…' — backslashes eaten by JSON5). After (same host): 8 passed (8).
  • CI: bun run ci:static passes. tsc -p packages/clawhub --noEmit, oxlint --type-aware, oxfmt --check clean.
  • CI workflow replication (Linux, ubuntu:24.04 + Bun 1.4.2 + Node 24, matching the CI workflow): static, packages, types-build, e2e-http all pass; unit (7,311 tests) has only 4 pre-existing timing flakes that fail identically on unmodified main in the same environment. (The PR's queued GitHub CI run is awaiting first-time-contributor approval.)
  • Linux behavior is unchanged by construction: no backslashes to escape on POSIX paths, and USERPROFILE is unset in the Linux test environment.
Full built-CLI transcript (Windows, scratch drive, local registry emulator)

Clawdbot config at D:\ws-evidence\state\clawdbot.json (no private data):

{ agents: { defaults: { workspace: "D:\ws-evidence\workspace-main" } } }

One skill at D:\ws-evidence\workspace-main\skills\demo-skill\SKILL.md. Command from D:\ws-evidence\nocwd (no --workdir, no CLAWHUB_WORKDIR, no .clawhub marker), CLAWDBOT_STATE_DIR=D:\ws-evidence\state:

┌  ClawHub sync
Roots with skills: D:\ws-evidence\workspace-main\skills
To sync: - demo-skill  NEW (publish 1.0.0)  (1 files)
│
└  Dry run: would publish 1 skill(s).

Control run, identical command minus CLAWDBOT_STATE_DIR:

┌  ClawHub sync
Error: No skills found (checked configured roots)

Registry request log for the first run (real resolution transport):

[registry] GET /api/v1/whoami
[registry] GET /api/v1/resolve slug=demo-skill

@vercel

vercel Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

@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 clawsweeper Bot added P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient Contributor real behavior proof is sufficient. 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. labels Sep 30, 2026
@clawsweeper

clawsweeper Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

What this changes

This PR escapes Windows paths in JSON5 test fixtures and clears USERPROFILE in two HOME fallback tests.

Example: A Windows contributor runs the configuration tests with a workspace at D:\ws\workspace-main.

  • Before: The fixture's unescaped backslashes corrupt the parsed workspace and fail the assertion.
  • After: The fixture preserves D:\ws\workspace-main and retains the same workspace assertion.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The focused patch appears correct and useful, but the supplied runtime evidence remains internally inconsistent.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Windows contributors encounter spurious configuration-test failures before making their own changes.
Reason: The repair addresses two source-verified platform assumptions while preserving the behavior each test checks.

Merge readiness

⛔ Blocked before merge - 1 item remains

This PR fixes concrete Windows test-fixture failures that remain on current main; no blocking code findings were found.

Priority: P3
Reviewed head: 0b3ed6b65bb4875aac33e87f944a789791aeaec5

Before merge

  • Add real behavior proof - The contributor reports eight passing Windows tests and supplies built-CLI workspace discovery through sync against a local registry emulator, but the displayed config contains single backslashes and decodes to D:ws-evidenceworkspace-main instead of the claimed absolute path; provide matching raw config bytes and CLI output, or a terminal screenshot showing both, with credentials, IPs, and non-public endpoints redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Findings

None.

Tests

  • Missing end-to-end proof: Provide matching raw config bytes and Windows built-CLI output for configured-workspace discovery; the displayed config currently cannot produce the claimed path.
Agent review details

How this fits together

The CLI resolves its working directory from flags, environment, directory markers, and Clawdbot or OpenClaw configuration; these tests exercise configuration parsing and home-directory fallback.

flowchart TD
  A[Windows test environment] --> B[JSON5 fixtures]
  B --> C[Configuration reader]
  D[Home environment variables] --> E[Home resolver]
  E --> C
  C --> F[Workspace resolver]
  F --> G[Existing assertions]
Loading

Technical review

Best possible solution:

Keep the platform correction in test fixtures while preserving production parsing, home-directory precedence, and existing assertions.

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

Current-main fixtures interpolate Windows backslashes into JSON5 strings, and both HOME tests retain USERPROFILE despite the resolver's Windows precedence; source independently supports the reported failures, without a local test run.

Is this the best way to solve the issue?

This is the best fix location: fixture escaping preserves JSON5 coverage, and clearing USERPROFILE isolates the HOME fallback; changing production parsing or Windows precedence would alter valid behavior.

AGENTS.md: found and applied where relevant.

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

Provenance checked

  • Workspace JSON5 fixtures keeps the original intent (2b552c4: Default workspace coverage was added alongside configuration-based workdir resolution; later OpenClaw and malformed-config tests reused direct path interpolation.)
  • HOME fallback fixtures keeps the original intent (fix: prefer $HOME over os.homedir() for path resolution #299: Environment-based home resolution addressed stale operating-system home paths, intentionally preferring USERPROFILE on Windows and HOME on Unix.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

  • Exact introduced patch: The pinned patch changes only seven fixture interpolations, adds a backslash-escaping helper, clears USERPROFILE in two fallback tests, and adds a changelog entry; production code and assertions are unchanged. (packages/clawhub/src/cli/clawdbotConfig.test.ts:14, 0b3ed6b65bb4)
  • Current main still needs the repair: Current main still interpolates native paths directly into JSON5 strings and leaves USERPROFILE set in both HOME tests. (packages/clawhub/src/cli/clawdbotConfig.test.ts:33, 0bf64843c697)
  • Intentional Windows precedence: resolveHome prefers USERPROFILE before HOME on Windows; merged fix: prefer $HOME over os.homedir() for path resolution #299 explicitly established that contract. Clearing USERPROFILE only in fallback fixtures respects it. (packages/clawhub/src/homedir.ts:12, 30b263c27c02)
  • Verified parser contract: The configuration reader imports JSON5, pinned to 2.2.3. Its escape parser consumes the backslash and returns unrecognized escaped characters directly, explaining both the fixture failures and the displayed proof-config discrepancy. (lib/parse.js:801, c3a75242772a)
  • Live contributor evidence inspected: The full live body and contributor comments report Windows tests improving from one pass to eight passes, but the built-CLI transcript displays a JSON5 workspace with single backslashes. That string decodes to D:ws-evidenceworkspace-main, so the claimed absolute workspace result remains unsupported by the displayed config. (0b3ed6b65bb4)
  • Feature history and local limit: History across clawhub, clawdhub, and molthub paths identifies workspace-resolution work, HOME fallback work, and malformed-config coverage. Some historical blobs are unavailable locally; GitHub commit patches independently verify the relevant earlier fixture additions. (packages/clawhub/src/cli/clawdbotConfig.test.ts, 0bf64843c697)

Likely related people:

  • Peter Steinberger: Raw commit 2b552c4 adds packages/clawdhub/src/cli/clawdbotConfig.test.ts:80 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 2b552c4803d2; files: packages/clawdhub/src/cli/clawdbotConfig.test.ts)
  • Steve: Raw commit 30b263c adds packages/clawdhub/src/cli/clawdbotConfig.test.ts:194 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 30b263c27c02; files: packages/clawdhub/src/cli/clawdbotConfig.test.ts)
  • Vincent Koc: Raw commit f179315 adds packages/clawhub/src/cli/clawdbotConfig.test.ts:160 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: f1793158043b; files: packages/clawhub/src/cli/clawdbotConfig.test.ts)

Review metrics

Metric Value Why it matters
Patch scope Production +0/-0; tests +20/-7; changelog +1/-0 The small increase repairs fixture serialization and environment isolation without changing runtime behavior.

Labels

Label changes:

No label changes.

Label justifications:

  • P3: This repairs a contributor test baseline on Windows and changes no shipped runtime behavior.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

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 (62 earlier review cycles; latest 8 shown)
  • reviewed 2026-10-13T11:16:25.922Z sha 0b3ed6b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-13T13:02:34.267Z sha 0b3ed6b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-13T16:35:48.621Z sha 0b3ed6b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-13T19:36:30.430Z sha 0b3ed6b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-13T21:16:31.885Z sha 0b3ed6b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-14T06:52:06.068Z sha 0b3ed6b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-14T10:37:09.833Z sha 0b3ed6b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-14T13:36:51.787Z sha 0b3ed6b :: needs real behavior proof before merge. :: none

Reviewed October 14, 2026, 10:47 AM ET / 14:47 UTC (Revision 63).

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Oct 2, 2026
@Irish-Joseph

Copy link
Copy Markdown
Author

Verification summary for maintainers (run against the current head):

  • clawdbotConfig.test.ts: 8/8 pass on Windows (1/7 on unmodified main here) and the assertions are unchanged in intent — same fixture values, now valid JSON5 on every platform.
  • bun run ci:static passes; tsc --noEmit, oxlint --type-aware, oxfmt --check clean.
  • Test-only change (fixture escaping + USERPROFILE cleared in the two tests that document the HOME fallback); no production code touched.

The only red badge is the Vercel preview-deploy status ("Authorization required to deploy"), which needs a team-member authorization click; no test or CI job is failing. Happy to make any further adjustments.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. 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 status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Oct 3, 2026
@Irish-Joseph

Copy link
Copy Markdown
Author

CI workflow replication (Linux)

The queued CI / CodeQL Light / Security Gate / Deploy Test runs on this PR are awaiting first-time-contributor approval. To de-risk approval, I replicated the CI workflow jobs in a clean ubuntu:24.04 container (Bun 1.4.2, Node 24.21, matching the workflow's toolchain), running the exact workflow commands (bun run ci:static, ci:unit, ci:packages, ci:types-build, ci:e2e-http) against this branch's head:

  • static, packages, types-build, e2e-http: pass.
  • unit (7,311 tests): only 4 failures in this run, all timing-sensitive tests unrelated to this diff (convex/githubImport.test.ts gzip-byte race, scripts/ui-proof-backend.test.mjs process-tree reap, scripts/clawsweeper-dispatch-workflow.test.ts, packages/clawhub-admin/.../searchInsights.test.ts spawn timeout) — the identical 4 tests fail on unmodified main in the same environment, i.e. pre-existing under-provisioned-runner flakes (the container has 7 GB RAM vs the 16-vCI runner). No new failures versus main; this branch's regression tests pass.

Could a maintainer approve the queued workflow runs on this PR (GitHub Actions → Approve and run)? Everything above should reproduce green on the provisioned runner.

@Irish-Joseph

Copy link
Copy Markdown
Author

CI replication now covers the remaining two workflow jobs as well, in the same ubuntu:24.04 container against this branch head: claws-openclaw-contract — 9/9 pass (including the real package dry-run against the pinned OpenClaw source); playwright-smoke — 18/18 pass (production build + live-site browser smoke). All seven CI jobs are now verified green (unit flakes identical to unmodified main). Still just waiting on the first-time-contributor "Approve and run" for the queued runs.

@clawsweeper clawsweeper Bot added status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. 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. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. proof: sufficient Contributor real behavior proof is sufficient. labels Oct 8, 2026
@clawsweeper clawsweeper Bot removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Oct 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot added the stale label Oct 22, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. stale status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant