Repository navigation
fix(tests): make clawdbot config tests pass on Windows - #3868
Irish-Joseph wants to merge 1 commit into
Conversation
|
@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. |
|
Codex review: needs real behavior proof before merge. What this changesThis 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.
Review scores
ProductKind: Test only · Worth it: Yes · Fix scope: Complete 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 Before merge
FindingsNone. Tests
Agent review detailsHow this fits togetherThe 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]
Technical reviewBest 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
TestingProof path: unit tests only. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes: No label changes. Label justifications:
Rating scale6/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. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (62 earlier review cycles; latest 8 shown)
Reviewed October 14, 2026, 10:47 AM ET / 14:47 UTC (Revision 63). |
|
Verification summary for maintainers (run against the current head):
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. |
CI workflow replication (Linux)The queued
Could a maintainer approve the queued workflow runs on this PR (GitHub Actions → |
|
CI replication now covers the remaining two workflow jobs as well, in the same ubuntu:24.04 container against this branch head: |
|
This pull request has been automatically marked as stale due to inactivity. |
What Problem This Solves
Fixes:
bun run --cwd packages/clawhub test:srcfails 7 of 8clawdbotConfig.test.tstests 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
clawhubCLI test suite and see the real pass/fail state instead of 7 spurious failures insrc/cli/clawdbotConfig.test.ts.Why This Change Was Made
Two test-fixture/platform issues, implementation code untouched:
\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.USERPROFILEshadowing theHOMEfallback.resolveHome()intentionally prefersUSERPROFILEon Windows, but the two tests documenting theHOMEfallback only setHOME— on Windows the real user profile won. Those tests now clearUSERPROFILE(restored by the existingafterEach) so they keep targeting the fallback they document.Evidence
--workdirand no.clawhubmarker,clawhub sync --all --dry-runwith 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.main): 1 passed / 7 failed (e.g.expected 'D:ws…' to be 'D:\ws\…'— backslashes eaten by JSON5). After (same host): 8 passed (8).bun run ci:staticpasses.tsc -p packages/clawhub --noEmit,oxlint --type-aware,oxfmt --checkclean.CIworkflow):static,packages,types-build,e2e-httpall pass;unit(7,311 tests) has only 4 pre-existing timing flakes that fail identically on unmodifiedmainin the same environment. (The PR's queued GitHubCIrun is awaiting first-time-contributor approval.)USERPROFILEis 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):One skill at
D:\ws-evidence\workspace-main\skills\demo-skill\SKILL.md. Command fromD:\ws-evidence\nocwd(no--workdir, noCLAWHUB_WORKDIR, no.clawhubmarker),CLAWDBOT_STATE_DIR=D:\ws-evidence\state:Control run, identical command minus
CLAWDBOT_STATE_DIR:Registry request log for the first run (real resolution transport):