[freeqaz]

Problem

kubernetes-compute-real, kubernetes-compute-provisioning-real and harness-workspace-released-k3d wait at their end for local-path to delete the volumes of namespaces their cases deleted. The waits exist only for lane cleanup (ac5abda): local-path writes each claim's directory into the lane's shared storage (<cluster dir>/storage, bound into every node) as root or uid 1000, and a directory left behind made the runner user's rm fail with EACCES. No product code is involved: the tests delete the namespaces, and Kubernetes and local-path delete the volumes.

kubernetes-compute-real sets the length of the k3d Fixture and Configuration lane, and its after hook added 13-17 s to its end (file wall minus test time on the last three main runs; kubernetes-compute-driver-real, which has no such hook, shows about 2 s).

Change

• scripts/ci/cleanup.mjs: after deleting a k3d cluster, if <directory>/storage is not empty, empty it as root from the cluster's own node image (docker run --rm --pull=never --network=none --user=0 --security-opt=label=disable --entrypoint=find ... /storage -mindepth 1 -delete). The cluster is gone by then, so no workload can write there again. The image is already present; it is never pulled. An empty storage directory starts no container. An image value that starts with - is refused.
• tests/helpers/kubernetes-compute-real.mjs: createNamespaceReaper() becomes a plain deleteNamespaces(); the two compute files drop the after hook, and harness-workspace-released-k3d drops its per-case volume wait.
• ci-cleanup.test.mjs: a new case leaves an unwritable volume directory in the storage and checks the exact container command, that it runs only after the cluster is deleted, and that the cluster directory is then gone. The existing k3d cleanup case gets an empty storage directory and checks that no container starts.

Verification

From CI result files (before: main runs 40044177248 / 40045547653 / 40046084436 and other PR runs today; after: this PR's two runs).

	before	after
Fixture and Configuration lane test wall	main 92.4 / 97.2 / 90.6 s; other PRs 87.9 / 101.7 / 90.9 / 86.5 s	73.3 / 76.3 s
kubernetes-compute-real wall minus test time	14.3-17.3 s	2.0 / 2.0 s
Fixture and Configuration job total, same CI batch	144 s (#3391)	130 s
harness-workspace-released-k3d case	15.5-18.4 s	8.2 s (second run; the first head did not include it)
State and Plugin Status lanes	92-102 s / 88-94 s	93.5-94.5 s / 91.2-92.4 s (bound by other files)

• Mutants: without the new cleanup step, the new ci-cleanup case fails with the original EACCES: permission denied, unlink .../storage/pvc-1_tenant_workspace/state; running the step before the cluster is deleted fails it; dropping the empty-storage check fails the existing case.
• find /storage -mindepth 1 -delete (with --security-opt=label=disable) checked against the pinned k3s node image: it removes nested, hidden and unwritable entries.
• Pinned at 2 CPUs: ci-cleanup.test.mjs 20/20 on both heads.
• The k3d files could not run locally (no k3d host available here), so the PR's k3d lanes, whose Cleanup steps must remove the storage, are the proof: green on both heads.
• tsc --build tsconfig.json tests/tsconfig.json, eslint and prettier are clean on the touched files.
• Internal review: land with nits; nits applied (SELinux label, empty-storage test, the third file's stale comment and wait).

Risks

• The removed waits also failed when a namespace stayed stuck in deletion or local-path's helper failed. Neither is product behaviour: the tests delete these namespaces, and volume deletion is Kubernetes and local-path. The cluster is deleted at lane end either way.
• A namespace's background teardown now overlaps the next file in the same lane; earlier cases already overlapped later ones.
• If the node image were missing while the storage has entries, --pull=never makes the Cleanup step fail visibly instead of pulling.

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

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR removes two file-level volume waits and empties leftover shared storage after deleting the owned k3d cluster.

Example: A fixture leaves storage/pvc-1_tenant_workspace/state behind.

• Before: The test file waits for local-path to remove the volume so runner cleanup can succeed.
• After: The file finishes without that wait, and lane cleanup removes leftover storage as root after verifying cluster deletion.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	A useful, focused patch with successful head-bound integration checks and limited inspectable runtime evidence.
Proof confidence	🦐 gold shrimp (3/6)	This collaborator-authored CI maintenance PR is outside the ordinary contributor proof gate; affected k3d jobs passed at the reviewed head, while blocked log redirects prevented inspection of the actual storage-removal branch.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Performance · Worth it: Yes · Fix scope: Complete
User problem: Contributors wait for file-end volume deletion even though the lane subsequently destroys the disposable cluster.
Reason: The maintainer-authored direction is narrow, and the original wait existed to satisfy the cleanup requirement now handled by lane cleanup.

Merge readiness

✅ Ready for maintainer review

Keep this PR open for maintainer landing: it replaces a cleanup-only wait with focused lane cleanup, and no actionable correctness or security defect was found.

Priority: P3
Reviewed head: 202ca1a26980ba647d6b54039b576ba5e636f8ec

Before merge

None.

Findings

None.

Tests

• Missing end-to-end proof: Available CI metadata confirms successful affected jobs, but detailed traces of the nonempty-storage branch and measured performance improvement could not be inspected.

Agent review details

How this fits together

CI preparation records a disposable cluster, node image, and shared storage directory; lane cleanup consumes that record, deletes owned cluster resources, and removes the directory.

flowchart TB
 A[Fixture tests] --> B[Delete test namespaces]
 B --> C[Lane cleanup]
 C --> D[Delete and verify owned cluster]
 D --> E[Empty leftover shared storage]
 E --> F[Remove cluster directory]

Loading

Technical review

Best possible solution:

Keep disposable storage cleanup in the existing lane cleanup owner and remove the redundant per-file waits.

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

This is a performance improvement; source confirms the existing polling path, and the earlier merged PR records the cleanup permission failure it prevents.

Is this the best way to solve the issue?

The change addresses the cleanup requirement at its existing owner, preserves cluster-absence checks, and removes the replaced polling path.

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Per-file volume deletion waits changes intended behavior with a stated reason (ac5abda: Prevent runner cleanup from failing on root-owned local-path directories, as documented in test(kubernetes): stop waiting out cleanup grace periods in the configuration lane #1283 and later shared by test(k3d): move the namespace and provisioning cases into their own file #1452.)
• Owned cluster cleanup keeps the original intent (2ba1727: Delete owned cluster resources and verify their absence before removing the cluster directory; the inserted operation preserves that ordering and existing ownership checks.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Focused cleanup implementation: Ownership and filesystem checks precede cluster deletion; storage cleanup runs afterward using the recorded node image, no network, no image pull, and a mount limited to the storage directory. (scripts/ci/cleanup.mjs:330, 202ca1a26980)
• Resource producer: Preparation creates the private cluster directory and shared storage, records nodeImage, and mounts that storage into the fixture nodes. (scripts/ci/prepare.mjs:1352, 202ca1a26980)
• Original cleanup rationale: test(kubernetes): stop waiting out cleanup grace periods in the configuration lane #1283 documents the EACCES failure that motivated the volume wait; test(k3d): move the namespace and provisioning cases into their own file #1452 later shared that wait between files. (tests/helpers/kubernetes-compute-real.mjs:127, 456818c5bd61)
• Current main still uses the wait: Fetched main retains createNamespaceReaper and its file-level wait; its cluster cleanup does not include the new storage-emptying operation. (scripts/ci/cleanup.mjs:330, a3b7d0111095)
• Head-bound CI verification: https://github.com/openclaw/openclaw-enterprise/actions/runs/40054198241 completed successfully for the reviewed head, including both affected k3d jobs; detailed job logs redirected to a host blocked by the review network. (.github/actions/run-ci-lane/action.yml:221, 202ca1a26980)
• Cleanup boundary coverage: The added CLI test covers nonempty storage and deletion ordering with fake commands; existing cases reject ownerless resources, foreign directories, invalid inventories, and surviving cluster resources. (tests/integration/ci-cleanup.test.mjs:217, 202ca1a26980)

Likely related people:

• kevinlin-openai: Raw commit d353d52 adds scripts/ci/cleanup.mjs:112 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: d353d527feb7; files: scripts/ci/cleanup.mjs)

Review metrics

Metric	Value	Why it matters
Change scope	CI cleanup +43/-1 lines; tests and helpers +76/-47 lines	The growth adds one cleanup operation and its boundary test while removing the namespace-tracking and polling machinery.

Labels

Label changes:

• add P3: This improves fixture-lane efficiency without changing product runtime behavior.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P3: This improves fixture-lane efficiency without changing product runtime behavior.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

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.

Reviewed October 23, 2026, 10:39 PM ET / October 24, 2026, 02:39 UTC.

[freeqaz]

@clawsweeper re-review

[clawsweeper]

🦞👀
Exact review queued.

Re-review progress:

• State: Complete
• Detail: The item is closed; no stale verdict was published.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/40057060459
• Updated: 2026-10-24T03:08:31.753Z