[Irish-Joseph]

What Problem This Solves

Fixes: packages/clawhub/src/config.test.ts > "writes config with restricted modes" fails on Windows, blocking Windows-based contributors from a green CLI test baseline.

User Impact

No user-visible change. The writeGlobalConfig permission tests now pass on Windows in addition to Linux.

Why This Change Was Made

The test stubs process.platform to linux (which correctly drives the chmod branch), but the fixture path was the POSIX literal /tmp/clawhub-config-test/config.json. writeGlobalConfig runs resolve() on that override, which on Windows re-renders it with a drive prefix and backslashes (D:\tmp\clawhub-config-test\config.json), so the mkdir assertion against the literal /tmp/clawhub-config-test never matched. The fixture is now built from tmpdir() with resolve(), and the mkdir expectation is derived from dirname() of the same fixture path — the assertions still verify the exact same behavior (0o700 dir, 0o600 file, explicit chmod) on every platform.

Evidence

Real behavior: built CLI writing the global config (real clawhub login --token)

The unit test documents writeGlobalConfig's permission behavior; the same code path exercised through the real built CLI:

Linux (ubuntu 24.04, Node 24, built CLI from this branch) — fresh config dir is created 0o700, the file 0o600:

$ CLAWHUB_CONFIG_PATH=/work/cfg/config.json node dist/cli.js --registry http://127.0.0.1:38700 login --token <demo-token> --no-browser
OK. Logged in as @evidence-bot.
$ stat -c '%a %n' /work/cfg /work/cfg/config.json
700 /work/cfg
600 /work/cfg/config.json

Pre-existing world-readable config file is re-chmodded to 0o600 on login (the chmod branch for existing files):

$ touch /work2/cfg/config.json && chmod 644 /work2/cfg/config.json
$ CLAWHUB_CONFIG_PATH=/work2/cfg/config.json node dist/cli.js --registry http://127.0.0.1:38700 login --token <demo-token> --no-browser
$ stat -c '%a %n' /work2/cfg/config.json
600 /work2/cfg/config.json

Windows (same built CLI, same command shape) — the config file is written; the POSIX mode assertions are Linux-only by design (process.platform !== "win32" guards the chmod):

> set CLAWHUB_CONFIG_PATH=D:\cfg-evidence\config.json
> node dist\cli.js --registry http://127.0.0.1:38700 login --token <demo-token> --no-browser
OK. Logged in as @evidence-bot.
> type D:\cfg-evidence\config.json
{
  "registry": "http://127.0.0.1:38700",
  "token": "<demo-token>"
}

(Runs against a local registry emulator; no public registry or real credentials involved.)

• Before (Windows, unmodified main): config.test.ts → 2 passed / 1 failed (mkdir expected /tmp/clawhub-config-test, received D:\tmp\clawhub-config-test).
• After (same Windows host): bunx vitest run -c vitest.config.ts src/config.test.ts → 3 passed (3).
• Full packages/clawhub source suite on the same host: 108 failed / 353 passed vs 109 failed / 352 passed on unmodified main; the delta is exactly this test.
• Linux/macOS behavior is unchanged: resolve(join(tmpdir(), …)) yields the same kind of absolute POSIX path the literal did, and the expectations are derived from the same variable.
• CI: bun run ci:static passes.
• 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.)
• tsc -p packages/clawhub --noEmit, oxlint --type-aware, oxfmt --check all clean on the touched files.

[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 maintainer review before merge.

What this changes

This PR makes the CLI config permission test use a host-native temporary path and adds a changelog entry.

Example: A Windows contributor runs the config permission tests on drive D.

• Before: The test expects /tmp/clawhub-config-test while the implementation receives D:\tmp\clawhub-config-test; one test fails.
• After: The fixture and directory expectation use native paths; the contributor reports all three tests passing.

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A correct, narrowly scoped test repair supported by Windows before-and-after results and relevant CLI terminal evidence.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The supplied Windows test comparison exercises the changed fixture, and built CLI login traces against a local registry emulator demonstrate Windows config persistence and Linux 0700/0600 creation plus existing-file chmod; runtime code is unchanged.
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 encounter a false failure in the CLI config permission test.
Reason: A small fixture correction restores cross-platform coverage without weakening token-permission checks.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: current main still contains the Windows-incompatible fixture, and this focused correction has no blocking findings.

Priority: P3
Reviewed head: 451696df88d29e73d8c1365bfd71dbbddabc8e6d

Before merge

None.

Findings

None.

Agent review details

How this fits together

The CLI config writer receives registry and token values, resolves the configured file path, and writes restricted config files; this PR changes its mocked filesystem test fixture.

flowchart TD
  A[CLI login] --> B[Config writer]
  C[Config permission test] --> B
  D[Host temporary directory] --> C
  B --> E[Path resolution]
  E --> F[Filesystem operations]

Loading

Technical review

Best possible solution:

Use the host-native fixture and derived directory expectation while retaining all existing permission assertions.

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

Source confirms that host-native resolve and dirname cannot match the literal POSIX expectations on Windows; the contributor also supplies before-and-after Windows results, which this read-only review did not rerun.

Is this the best way to solve the issue?

This is the best fix location: normalize the fixture rather than skip Windows coverage or change correct production path resolution.

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Config permission test fixture keeps the original intent (78c2757: Merged fix(cli): secure config file permissions #164 added tests for restrictive config permissions; subsequent formatting and test-harness refactors retained the literal fixture.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

• Introduced patch: The pinned introduction changes only the test fixture, its directory expectation, imports, explanatory comment, and one changelog line; permission assertions remain intact. (packages/clawhub/src/config.test.ts:33, 451696df88d2)
• Current main remains affected: Current main retains the literal /tmp/clawhub-config-test/config.json and literal directory expectation, while the config writer resolves overrides using the host path implementation. (packages/clawhub/src/config.test.ts:29, 0bf64843c697)
• Original permission contract: Merged fix(cli): secure config file permissions #164 introduced the permission tests to protect stored tokens using directory mode 0700, file mode 0600, and chmod for existing files; this PR preserves that intent. (packages/clawdhub/src/config.test.ts:20, 78c27579a334)
• Contributor verification: The full live PR body records Windows results changing from two passes and one failure to three passes, plus built CLI login output showing config persistence on Windows and restricted new and existing files on Linux. (451696df88d2)
• External deployment status: The Vercel status explicitly says Authorization required to deploy; cancelled auto-response checks do not establish a diff-caused test failure. (451696df88d2)
• History scope and limit: History was searched across the clawdhub-to-clawhub rename and the original permission-test addition was read; config.ts blame encountered an unavailable historical blob, so no additional introduction claim relies on it. (packages/clawhub/src/config.ts, 0bf64843c697)

Review metrics

Metric	Value	Why it matters
Patch scope	Production +0/-0; tests +6/-2; changelog +1/-0	The small fixture correction preserves runtime behavior and existing coverage.

Labels

Label changes:

No label changes.

Label justifications:

• P3: This fixes contributor test portability without changing shipped CLI behavior.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
• 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 (70 earlier review cycles; latest 8 shown)

• reviewed 2026-10-13T17:36:46.491Z sha 451696d :: needs maintainer review before merge. :: none
• reviewed 2026-10-13T19:22:47.169Z sha 451696d :: needs maintainer review before merge. :: none
• reviewed 2026-10-13T23:32:36.193Z sha 451696d :: needs maintainer review before merge. :: none
• reviewed 2026-10-14T02:02:45.275Z sha 451696d :: needs maintainer review before merge. :: none
• reviewed 2026-10-14T03:41:23.126Z sha 451696d :: needs maintainer review before merge. :: none
• reviewed 2026-10-14T07:55:33.550Z sha 451696d :: needs maintainer review before merge. :: none
• reviewed 2026-10-14T09:19:51.639Z sha 451696d :: needs maintainer review before merge. :: none
• reviewed 2026-10-14T10:23:16.554Z sha 451696d :: needs maintainer review before merge. :: none

Reviewed October 14, 2026, 9:34 AM ET / 13:34 UTC (Revision 71).

[Irish-Joseph]

Verification summary for maintainers (run against the current head, including the import-ordering cleanup):

• config.test.ts: 3/3 pass on Windows (2/1 on unmodified main here); the 0o700/0o600/chmod assertions are unchanged in intent — only the fixture path is now host-native.
• bun run ci:static passes; tsc --noEmit, oxlint --type-aware, oxfmt --check clean.
• Test-only change; 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.

[Irish-Joseph]

---

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]

Status note while the re-review verdict is pending delivery: the description contains the full evidence — before/after unit results for config.test.ts (2/1 → 3/3 on this Windows host), the zero-delta full-suite comparison versus unmodified main, clean static/lint/typecheck gates, and the Linux CI-workflow replication (static/packages/types-build/e2e-http green; unit flakes identical to unmodified main). This is a test-only change with no production-code diff. If the re-review identifies a specific remaining gap, please call it out explicitly and I will address it directly.

[Irish-Joseph]

@clawsweeper re-review

[clawsweeper]

🦞👀
Exact review queued.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/37172475582
• Updated: 2026-10-04T02:54:44.141Z

[Irish-Joseph]

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.

[github-actions]

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