Skip to content

test: isolate user state across platforms - #1215

Open
steipete wants to merge 2 commits into
mainfrom
codex/test-isolation
Open

test: isolate user state across platforms#1215
steipete wants to merge 2 commits into
mainfrom
codex/test-isolation

Conversation

@steipete

@steipete steipete commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add one shared testutil.IsolateUserDirs helper that redirects state, config, home, AppData, and local AppData into a private per-test tree on every supported OS
  • migrate the affected CLI and provider tests, with package-wide safety nets for CLI, DigitalOcean, and Hyper-V tests that could otherwise reach persistent user state
  • extend the existing Windows CI job to run the isolation helper and affected provider suites natively

The helper asserts that os.UserConfigDir() remains inside the test root and creates every synthetic user directory with private permissions. Package-wide isolation preserves the original Go build and module cache locations so nested CLI builds do not redownload dependencies.

Thanks to @paulcam206 for the detailed reports and cross-platform reproduction work.

Reproduction and proof

On unmodified main, running one reported CLI test with a controlled fallback home passed but left this persistent lock behind:

$ env -u XDG_STATE_HOME -u XDG_CONFIG_HOME HOME="$repro_root" go test -count=1 -run '^TestAutoRouteStaticLeaseIgnoresNonStaticIDs$' -v ./internal/cli
=== RUN   TestAutoRouteStaticLeaseIgnoresNonStaticIDs
--- PASS: TestAutoRouteStaticLeaseIgnoresNonStaticIDs (0.00s)
PASS
$ find "$repro_root" -path '*/crabbox/*'
/tmp/crabbox-state-repro.Z2zIbT/Library/Application Support/crabbox/state/claim-locks/cbx_abc123.json.lock

The same controlled-fallback check on this branch leaves no Crabbox path behind:

$ env -u XDG_STATE_HOME -u XDG_CONFIG_HOME HOME="$repro_root" go test -count=1 -run '^TestAutoRouteStaticLeaseIgnoresNonStaticIDs$' -v ./internal/cli
=== RUN   TestAutoRouteStaticLeaseIgnoresNonStaticIDs
--- PASS: TestAutoRouteStaticLeaseIgnoresNonStaticIDs (0.00s)
PASS
ok  github.com/openclaw/crabbox/internal/cli  0.474s
$ find "$repro_root" -path '*/crabbox/*'
$ echo "PASS: no Crabbox files were written beneath the fallback home"
PASS: no Crabbox files were written beneath the fallback home

Verification

$ gofmt -w $(git ls-files '*.go')
$ go vet ./...
$ go test -race -p 1 ./...
ok  github.com/openclaw/crabbox/internal/cli  210.560s
ok  github.com/openclaw/crabbox/internal/providers/digitalocean  2.757s
ok  github.com/openclaw/crabbox/internal/providers/hyperv  1.716s
ok  github.com/openclaw/crabbox/internal/testutil  (cached)

The full local parallel race command was also attempted, but this machine had only 2.9 GiB free and parallel linking stopped with no space left on device; the identical suite passed with -p 1. The hosted Go job passed the exact go test -race ./... command at https://github.com/openclaw/crabbox/actions/runs/30673261639/job/91295228438.

Native Windows execution passed in the expanded existing Windows CI job at https://github.com/openclaw/crabbox/actions/runs/30673261639/job/91295228445. That job exercised the shared helper plus AWS, Daytona, DigitalOcean, Hyper-V, Namespace, Scaleway, Vast, and Vultr before rerunning the existing Pond Mesh cancellation proof. Locally, the Windows implementation was checked against Go's documented os.UserConfigDir behavior: %AppData% is redirected via APPDATA, while USERPROFILE and LOCALAPPDATA cover the other standard-library user-directory resolvers.

Closes #1212
Closes #1213

@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. P1 Urgent regression or broken agent/channel workflow affecting real users now. labels Jul 31, 2026
@clawsweeper

clawsweeper Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 1:09 AM ET / 05:09 UTC.

ClawSweeper review

What this changes

This PR adds a shared cross-platform Go-test user-directory isolation helper, applies it to affected CLI and provider tests, and expands the existing Windows CI job to exercise the isolation path natively.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open: current main still lacks the shared isolation helper, so this proof-positive regression fix remains necessary for ordinary maintainer merge review. No discrete correctness or security defect was found in the unchanged PR head.

Likely related people: steipete is the high-confidence routing candidate from the current resolver history and the patch; paulcam206 supplied the linked cross-platform reproduction evidence.

Priority: P1
Reviewed head: 80c870efb573f2b411a1c2fe2f6ce0d1e4d30a9c

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) This is a focused, cleanly validated regression repair with strong cross-platform runtime proof and no actionable review finding.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The PR body provides a controlled before/after terminal reproduction and successful hosted Go plus native Windows CI evidence for the changed behavior.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body provides a controlled before/after terminal reproduction and successful hosted Go plus native Windows CI evidence for the changed behavior.
Evidence reviewed 7 items Current state/config behavior: Current main resolves Crabbox state through XDG_STATE_HOME when present and otherwise through Go's OS-specific user configuration directory; this is the path the linked reports showed could reach persistent user state.
Cross-platform isolation implementation: The proposed helper creates private temporary home, config, state, AppData, and local AppData paths, installs all relevant environment overrides, and verifies that Go's user config directory remains inside the test root.
Package-wide coverage boundary: The branch applies package-level isolation to the CLI on each supported host path and to the DigitalOcean and Hyper-V test packages, while also migrating targeted provider tests that resolve user state or configuration.
Findings None None.
Security None None.

How this fits together

Crabbox CLI and provider tests create claims, keys, and configuration through the same user-directory resolvers used by the product. Test setup must redirect those state and config paths into temporary directories so tests cannot read, modify, or collide with a developer's persistent local Crabbox state.

flowchart TD
  A[Go CLI and provider tests] --> B[Shared test user-directory isolation]
  B --> C[Home AppData and XDG overrides]
  C --> D[State and config directory resolution]
  D --> E[Private per-test directories]
  B --> F[Package safety nets]
  F --> G[Native Windows CI coverage]
Loading

Before merge

  • Complete next step (P2) - No mechanical repair is indicated; the remaining action is routine maintainer merge review of a clean, proof-sufficient patch.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 25 files affected; 297 added, 93 removed The change is broad enough to cover the reported CLI and provider leak paths while remaining limited to tests, test utility code, and one CI job.
Package safety nets 3 test packages protected CLI, DigitalOcean, and Hyper-V receive package-level isolation for tests that might otherwise reach user state indirectly.
Native Windows coverage 9 isolation-related test packages added to one Windows job The expanded job checks the platform-specific AppData path that Linux-only Go CI cannot validate.

Technical review

Best possible solution:

Merge the shared isolation helper and its focused Windows coverage so test-created state and configuration remain inside test-owned directories across supported platforms.

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

Yes. The linked reports and the PR body provide a controlled current-main reproduction that leaves a Crabbox lock under a fallback home directory; the reported Windows path is also covered by native CI proof.

Is this the best way to solve the issue?

Yes. A shared helper that redirects both XDG and Windows user-directory inputs, backed by targeted tests and package-wide guards where broad coverage is needed, is the narrowest maintainable fix for this cross-platform test isolation defect.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0171ae2ee19f.

Labels

Label justifications:

  • P1: Persistent test residue can corrupt local configuration and state, creating current cross-run failures for developer workflows.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides a controlled before/after terminal reproduction and successful hosted Go plus native Windows CI evidence for the changed behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a controlled before/after terminal reproduction and successful hosted Go plus native Windows CI evidence for the changed behavior.

Evidence

What I checked:

  • Current state/config behavior: Current main resolves Crabbox state through XDG_STATE_HOME when present and otherwise through Go's OS-specific user configuration directory; this is the path the linked reports showed could reach persistent user state. (internal/cli/claim.go:1756, 0171ae2ee19f)
  • Cross-platform isolation implementation: The proposed helper creates private temporary home, config, state, AppData, and local AppData paths, installs all relevant environment overrides, and verifies that Go's user config directory remains inside the test root. (internal/testutil/userdirs.go:20, 80c870efb573)
  • Package-wide coverage boundary: The branch applies package-level isolation to the CLI on each supported host path and to the DigitalOcean and Hyper-V test packages, while also migrating targeted provider tests that resolve user state or configuration. (internal/providers/digitalocean/backend_test.go:2912, 80c870efb573)
  • Current resolver provenance: Git blame attributes the current state and user-config resolver implementation to the same feature-history commit, making steipete the strongest routing candidate for this area. (internal/cli/claim.go:1756, 797f20736844)
  • Current main still needs this patch: The helper is absent from current main and the PR head is not an ancestor of current main; the branch is therefore neither implemented nor superseded. (internal/testutil/userdirs.go:20, 80c870efb573)
  • After-fix behavior proof: The PR body includes controlled before/after terminal output showing that the reported CLI test no longer leaves Crabbox files under the fallback home, plus successful hosted full Go and native Windows CI runs for the PR head. (internal/testutil/userdirs_test.go:10, 80c870efb573)

Likely related people:

  • steipete: Current-main blame attributes the state and user-config resolver behavior to Peter Steinberger, and steipete also authored the focused isolation repair. (role: original implementation author and recent area contributor; confidence: high; commits: 797f20736844, 3e1c4828eacd, 80c870efb573; files: internal/cli/claim.go, internal/cli/config.go, internal/testutil/userdirs.go)
  • paulcam206: The linked reports provide the concrete persistent-state and Windows AppData reproduction that this branch addresses, and the PR explicitly credits that investigation. (role: reporter and cross-platform reproduction contributor; confidence: high; files: internal/cli/claim.go, internal/providers/hyperv/provider_test.go)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (6 earlier review cycles)
  • reviewed 2026-07-31T23:38:47.498Z sha 80c870e :: needs maintainer review before merge. :: none
  • reviewed 2026-07-31T23:54:24.597Z sha 80c870e :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T09:35:34.534Z sha 80c870e :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T16:18:25.865Z sha 80c870e :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T18:45:48.093Z sha 80c870e :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T01:41:58.042Z sha 80c870e :: needs maintainer review before merge. :: none

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

Labels

P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

1 participant