Skip to content

Doctor: expose sandbox registry findings#84326

Merged
giodl73-repo merged 6 commits into
openclaw:mainfrom
giodl73-repo:doctor-sandbox-structured-repairs
Jun 22, 2026
Merged

Doctor: expose sandbox registry findings#84326
giodl73-repo merged 6 commits into
openclaw:mainfrom
giodl73-repo:doctor-sandbox-structured-repairs

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Exposes legacy sandbox registry files as structured doctor health findings.
  • Adds dry-run repair effects for migrate, remove-empty, and quarantine registry-file outcomes.
  • Keeps the existing doctor:sandbox legacy run path as the only real mutating sandbox registry migration path for normal doctor / doctor --fix.
  • Declares the structured check inline on the existing ordered doctor contribution, so this PR does not add another core registry entry.
  • Aligns the structured check description and fix hint with the current SQLite sandbox registry contract.

This is intentionally the small sandbox registry slice from the doctor repair stack. It does not migrate sandbox image builds, namespace probes, Docker checks, or plugin-sensitive doctor rules.

Stack

Stacked on #86627, which has landed, and rebased onto current upstream/main.

Current branch head is signed commit ba2742aa6a05f1b5626e7c44e05acebe77c7ba63 (Fix unused session accessor destructure). The last commit is a narrow rebased-base cleanup for an unused sessionStore destructure in resolveSessionTranscriptRuntimeReadTarget(...); it was needed so prod type/lint CI passes on current main.

Real behavior proof

Behavior or issue addressed:
Legacy sandbox registry files are now visible through structured doctor health checks, while the existing doctor --fix path still owns the actual migration from legacy JSON/sharded registry files into SQLite-backed sandbox registry storage. Structured dry-run effects expose the intended migrate, quarantine, and remove-empty outcomes without mutating state.

Real environment tested:
WSL Ubuntu 24.04, OpenClaw checkout /root/src/openclaw-84326-proof, prior PR head dc2a3c13eb2b6dd41c7ad63926540d4f89391a7c, source CLI via node scripts/run-node.mjs, source harness via node --import tsx --input-type=module, isolated temp OPENCLAW_STATE_DIR, temp OPENCLAW_CONFIG_PATH, temp HOME, and synthetic legacy sandbox registry fixtures. The branch has since been rebased and updated to ba2742aa6a05f1b5626e7c44e05acebe77c7ba63; the latest functional sandbox behavior is unchanged.

Exact steps or command run after this patch:

  1. Reused the isolated source CLI lint/fix proof for a valid legacy sandbox/containers.json migration into SQLite-backed sandbox_registry_entries.
  2. Ran a source harness importing legacySandboxRegistryInspectionToHealthFinding and legacySandboxRegistryInspectionToRepairEffect from src/commands/doctor-sandbox.ts.
  3. The harness emitted copied dry-run effect output for valid monolithic migrate, invalid monolithic quarantine, and empty sharded removal cases.
  4. Reproduced the failed checks-node-core-tooling benchmark test locally, updated it to assert the SQLite restart intent row, and reran that tooling test successfully.
  5. Removed the unused sessionStore destructure exposed by current upstream/main, then reran pnpm tsgo:core successfully.

Evidence after fix:
The structured finding points at SQLite, and dry-run repair effects cover all three registry-file outcomes with source-specific targets. The earlier real CLI fix proof still applies to the mutating path: doctor --fix --yes --non-interactive removed the legacy containers.json, printed the migration/doctor-complete markers, exited 0, and persisted the synthetic openclaw-sbx-proof row in SQLite sandbox_registry_entries.

Observed result after fix:
Structured sandbox registry findings and contribution metadata describe SQLite-backed registry storage. The dry-run repair-effect path reports migrate, quarantine, and remove-empty effects without mutating state, and sharded inspections report the sharded registry directory instead of the monolithic JSON file. The real mutating path remains owned by doctor --fix.

What was not tested:
The proof used synthetic legacy registry fixtures and did not run Docker, build sandbox images, probe namespaces, or exercise live sandbox container lifecycle. The prior doctor --fix source CLI run emitted an unrelated Gateway credentials warning while checking local Gateway status, but the command still exited 0 and the sandbox registry migration completed.

ClawSweeper and maintainer status

Latest visible ClawSweeper status:

  • Proof: sufficient.
  • Status label: ready for maintainer look.
  • No concrete contributor-facing blocker remains from the latest durable review.

Maintainer severity finding:

  • Keep core/doctor/sandbox/registry-files as a warning, not info.
  • The finding is actionably stale, noncanonical state with an existing corrective path through openclaw doctor --fix.
  • This matches the Doctor severity bar: warning is for state the user should correct even if OpenClaw can still run; info is for advisory state that can be left indefinitely without affecting canonical behavior, repair clarity, upgrade safety, or reliability.
  • RFC pointer: RFC: Doctor health upgrades rfcs#1, rfcs/needs_refactoring/0002-doctor-health-upgrades.md, now records the warning/info bar in the structured Doctor health contract.

Validation

Current head ba2742aa6a05f1b5626e7c44e05acebe77c7ba63:

  • node scripts\run-vitest.mjs run --config test\vitest\vitest.tooling.config.ts test\scripts\bench-gateway-restart.test.ts --reporter=verbose - 31 passed, 2 skipped
  • $env:OPENCLAW_VITEST_MAX_WORKERS='1'; node scripts\run-vitest.mjs run src\scripts\test-projects.test.ts src\commands\doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src\flows\doctor-health-contributions.test.ts --reporter=dot - 143 tests passed across 3 shards
  • pnpm exec oxlint scripts\bench-gateway-restart.ts test\scripts\bench-gateway-restart.test.ts src\scripts\test-projects.test.ts src\commands\doctor-sandbox.ts src\commands\doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src\flows\doctor-health-contributions.ts src\flows\doctor-health-contributions.test.ts src\config\sessions\session-accessor.ts
  • pnpm exec oxfmt --check scripts\bench-gateway-restart.ts test\scripts\bench-gateway-restart.test.ts src\scripts\test-projects.test.ts src\commands\doctor-sandbox.ts src\commands\doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src\flows\doctor-health-contributions.ts src\flows\doctor-health-contributions.test.ts src\config\sessions\session-accessor.ts
  • pnpm tsgo:core
  • pnpm tsgo:test:src
  • git diff --check

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Proof artifact for the sandbox structured repair stage:

Doctor sandbox structured repair proof

Run note and image source: https://gist.github.com/giodl73-repo/c6ff5e2d3d3d5ca04e5fd3c5fb0c660d

Crabbox proof: direct AWS cbx_c86ebac61277 (tidal-hermit), run run_1807262a6457, exit 0, 5 test files / 66 tests passed.

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations agents Agent runtime and tooling size: XL maintainer Maintainer-authored PR labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 21, 2026, 12:29 PM ET / 16:29 UTC.

Summary
The PR adds a structured core/doctor/sandbox/registry-files doctor check with dry-run state repair effects, focused tests, and a Windows-portable Vitest cache-path assertion.

PR surface: Source +73, Tests +88. Total +161 across 5 files.

Reproducibility: not applicable. this is a feature PR for structured doctor output, not a bug report. The changed behavior is reviewable from the PR body, current source, and the doctor lint/repair code path.

Review metrics: 1 noteworthy metric.

  • Structured lint checks: 1 added. The new check runs under doctor --lint and can change exit behavior for upgraded installs with legacy sandbox registry files.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor-sandbox.ts, migration/backfill/repair: src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts, migration/backfill/repair: src/flows/doctor-health-contributions.test.ts, migration/backfill/repair: src/flows/doctor-health-contributions.ts, unknown-data-model-change: src/commands/doctor-sandbox.ts, unknown-data-model-change: src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts. Migration or upgrade compatibility proof is recorded; maintainers should verify it before merge.

Root-cause cluster
Relationship: canonical
Canonical: #84326
Summary: This PR is the canonical sandbox-registry structured-health slice; merged framework and registry-storage PRs are prerequisites or adjacent context, not replacements.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Get maintainer acceptance of warning-level lint behavior, or lower the structured finding impact before merge.

Risk before merge

  • [P1] Merging can make existing upgraded installs with legacy sandbox registry files or shard directories fail default doctor --lint until openclaw doctor --fix migrates, removes, or quarantines that residue.

Maintainer options:

  1. Accept warning-level registry lint
    Maintainers can intentionally accept that legacy sandbox registry residue fails default doctor --lint because the existing doctor fix path already owns migration and quarantine.
  2. Lower the lint impact before merge
    If upgrade residue should remain non-failing by default, change the structured finding severity or default selection while keeping the fix hint and dry-run effects.

Next step before merge

  • [P2] Protected maintainer labeling and compatibility-sensitive lint behavior require maintainer acceptance; there is no narrow automated repair to queue unless maintainers choose a lower lint impact.

Security
Cleared: The diff touches doctor diagnostics, source tests, and a test-routing assertion only; no dependency, workflow, secret-handling, package-resolution, or supply-chain concern was found.

Review details

Best possible solution:

Land the focused structured sandbox registry check after maintainer acceptance of warning-level lint behavior, or adjust severity/default selection if upgraded legacy residue should not fail lint.

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

Not applicable: this is a feature PR for structured doctor output, not a bug report. The changed behavior is reviewable from the PR body, current source, and the doctor lint/repair code path.

Is this the best way to solve the issue?

Yes, with maintainer acceptance: keeping the structured check inside doctor:sandbox while leaving real mutation on the existing sandbox doctor path matches the merged framework. The open product choice is whether warning-level lint failure is the right upgrade behavior.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded doctor CLI/state migration improvement with normal-priority upgrade impact.
  • merge-risk: 🚨 compatibility: The PR can make existing upgraded installs with legacy sandbox registry residue fail default doctor --lint until the existing migration path is run.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR body and inspected linked gist provide terminal-style CLI/harness proof and Crabbox focused-test proof for the changed sandbox doctor behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and inspected linked gist provide terminal-style CLI/harness proof and Crabbox focused-test proof for the changed sandbox doctor behavior.
Evidence reviewed

PR surface:

Source +73, Tests +88. Total +161 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 2 76 3 +73
Tests 3 94 6 +88
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 170 9 +161

What I checked:

  • Current main lacks the structured sandbox registry check: On current main, doctor:sandbox still delegates to runSandboxHealth without declaring core/doctor/sandbox/registry-files, so the PR is not obsolete on main. (src/flows/doctor-health-contributions.ts:1315, c2ee9b0be8ae)
  • PR adds a contribution-owned structured check: The PR head adds healthChecks directly to the existing doctor:sandbox contribution while keeping run: runSandboxHealth, matching the merged contribution-order framework. (src/flows/doctor-health-contributions.ts:1318, 81fe719e39b9)
  • PR maps legacy residue to warning findings and dry-run effects: The mapping emits warning-level findings and source-specific state effects for migrate, quarantine, and remove-empty outcomes. (src/commands/doctor-sandbox.ts:394, 81fe719e39b9)
  • Default lint treats visible findings as failing: doctor --lint runs ordered contribution checks and returns nonzero when findings meet the selected severity threshold, so a new warning-level check can affect upgraded installs with legacy residue. (src/commands/doctor-lint.ts:81, c2ee9b0be8ae)
  • Existing migration owner is the sandbox registry doctor path: Current main already inspects monolithic and sharded legacy sandbox registry sources and migrates them into SQLite through migrateLegacySandboxRegistryFiles. (src/agents/sandbox/registry.ts:614, c2ee9b0be8ae)
  • Latest release lacks the structured sandbox check: v2026.6.9 shows doctor:sandbox with only run: runSandboxHealth, so this structured lint behavior is not shipped in the latest release. (src/flows/doctor-health-contributions.ts:1156, c645ec4555c0)

Likely related people:

  • steipete: Authored and merged the SQLite sandbox registry migration that made SQLite the canonical sandbox registry store this PR reports against. (role: sandbox registry SQLite contributor and merger; confidence: high; commits: ce015cef5775, 75c8a6c9364a, ca69917153a8; files: src/agents/sandbox/registry.ts, src/commands/doctor-sandbox.ts, docs/cli/sandbox.md)
  • giodl73-repo: Authored the merged contribution-order doctor health framework that this PR uses, so they are relevant beyond authoring this branch. (role: recent doctor health framework contributor; confidence: high; commits: e56fd1dc0466; files: src/flows/doctor-health-contributions.ts, src/flows/doctor-repair-flow.ts, src/flows/health-check-registry.ts)
  • luckylhb90: Opened the merged sharded sandbox registry work that created one of the legacy source shapes this structured check reports. (role: original sharded registry contributor; confidence: medium; commits: 26ddf1054946, ca69917153a8; files: src/agents/sandbox/registry.ts, src/commands/doctor-sandbox.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@giodl73-repo
giodl73-repo force-pushed the doctor-sandbox-structured-repairs branch from 75cfed4 to 6d28631 Compare May 19, 2026 23:43
@giodl73-repo
giodl73-repo force-pushed the doctor-sandbox-structured-repairs branch 3 times, most recently from 34abca2 to 0559f8f Compare May 20, 2026 04:20
@giodl73-repo
giodl73-repo force-pushed the doctor-sandbox-structured-repairs branch from 0559f8f to 3730823 Compare May 20, 2026 22:51
@giodl73-repo
giodl73-repo force-pushed the doctor-sandbox-structured-repairs branch 4 times, most recently from 4a18342 to 452e286 Compare May 22, 2026 04:22
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels May 22, 2026
@giodl73-repo
giodl73-repo force-pushed the doctor-sandbox-structured-repairs branch 4 times, most recently from 1ef5cd4 to 050c116 Compare May 22, 2026 19:39
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Fresh exact-head re-review request for the sandbox registry proof. The PR body already includes the source CLI lint/fix proof for core/doctor/sandbox/registry-files; please refresh proof/status labels against head 5acbf868ed112a1e4a89a5469b909ac885076b2c.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated the branch to signed head a2b632a28846d7e8698e43c394a0dcec5853c9a7 with fix(doctor): name sqlite sandbox registry repairs.

This addresses the latest review finding: structured sandbox registry output now says valid legacy entries migrate to SQLite, and the doctor health contribution description says SQLite registry storage. I also updated the PR body with copied dry-run repair-effect proof for all three outcomes: would-migrate-legacy-sandbox-registry, would-quarantine-legacy-sandbox-registry, and would-remove-empty-legacy-sandbox-registry.

Validation passed locally: focused sandbox/contribution Vitest (60 tests), targeted oxlint, targeted oxfmt --check, git diff --check, and pnpm tsgo:core.

@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Pushed c3b4033 to address the sharded sandbox registry structured-output finding.

Change made:

  • Structured sandbox registry findings now use the source-specific path: sharded inspections report shardedDir, monolithic inspections report registryPath.
  • Dry-run repair effects now target the same source-specific path the real migration removes/quarantines/migrates.
  • Added sharded coverage for both the finding path/message and repair-effect target.

Validation:

  • pnpm install --frozen-lockfile
  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.test.ts --reporter=dot
  • pnpm exec oxlint src/commands/doctor-sandbox.ts src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • pnpm exec oxfmt --check src/commands/doctor-sandbox.ts src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • pnpm tsgo:core
  • git diff --check

@clawsweeper

clawsweeper Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated the PR body for current head c3b403370c45c324948cd9b902218671ef0acdd7 so the proof section no longer references the older a2b632a... head and the copied sharded dry-run evidence shows the source-specific sharded directory target (<temp>/state/sandbox/containers) instead of the old monolithic JSON path.

No code changes since the pushed fix commit. Existing validation for c3b403370c45 remains:

  • pnpm install --frozen-lockfile
  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.test.ts --reporter=dot
  • pnpm exec oxlint src/commands/doctor-sandbox.ts src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • pnpm exec oxfmt --check src/commands/doctor-sandbox.ts src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • pnpm tsgo:core
  • git diff --check

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Pushed dc2a3c13eb to address the checks-node-core-tooling failure on #84326.

Change made:

  • Updated src/scripts/test-projects.test.ts so the expected helper-import routing includes test/scripts/check-package-dist-imports.test.ts, matching the actual routing support.
  • Made the adjacent Vitest cache-path assertion portable on Windows by normalizing separators in the assertion only.

Validation on dc2a3c13eb:

  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/scripts/test-projects.test.ts --reporter=dot
  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.test.ts --reporter=dot
  • pnpm exec oxlint src/scripts/test-projects.test.ts
  • pnpm exec oxfmt --check src/scripts/test-projects.test.ts
  • pnpm tsgo:core
  • git diff --check

@clawsweeper

clawsweeper Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Follow-up body correction for #84326 at head dc2a3c13eb2b6dd41c7ad63926540d4f89391a7c: the Real behavior proof JSON now shows the sharded remove-empty-sharded repair effect target as <temp>/state/sandbox/containers, matching the source-specific sharded directory target from the code and tests. No code changes after dc2a3c13eb; this is a proof/body correction only.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Rebased #84326 onto current upstream/main and force-with-lease pushed the refreshed branch.

Current head:

  • 81fe719e39b94e6a1bd51e6d29c2910b250aa0a4

Rebase resolution:

  • Resolved the only conflict in src/scripts/test-projects.test.ts by keeping current main's expanded tooling test route list; the prior check-package-dist-imports expectation is already present on main.
  • No behavior changes to the sandbox doctor implementation after the rebase.

Validation on 81fe719e39:

  • $env:OPENCLAW_VITEST_MAX_WORKERS='1'; node scripts/run-vitest.mjs src/scripts/test-projects.test.ts src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.test.ts --reporter=dot (143 tests passed across 3 shards)
  • pnpm exec oxlint src/scripts/test-projects.test.ts src/commands/doctor-sandbox.ts src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • pnpm exec oxfmt --check src/scripts/test-projects.test.ts src/commands/doctor-sandbox.ts src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • pnpm tsgo:core
  • git diff --check

@clawsweeper

clawsweeper Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Updated #84326 at 0567ae7aaf9db7cadca80b00a155f7861003f312.

This push fixes the checks-node-core-tooling failure in test/scripts/bench-gateway-restart.test.ts: the test now verifies the gateway restart intent in the OpenClaw state database instead of the removed legacy gateway-restart-intent.json file. I also moved benchmark CLI option parsing before the platform guard so unknown args are still reported directly on Windows.

Validation at current head:

  • node scripts\run-vitest.mjs run --config test\vitest\vitest.tooling.config.ts test\scripts\bench-gateway-restart.test.ts --reporter=verbose - 31 passed, 2 skipped
  • $env:OPENCLAW_VITEST_MAX_WORKERS='1'; node scripts\run-vitest.mjs run src\scripts\test-projects.test.ts src\commands\doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src\flows\doctor-health-contributions.test.ts --reporter=dot - 143 passed across 3 shards
  • pnpm exec oxlint scripts\bench-gateway-restart.ts test\scripts\bench-gateway-restart.test.ts src\scripts\test-projects.test.ts src\commands\doctor-sandbox.ts src\commands\doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src\flows\doctor-health-contributions.ts src\flows\doctor-health-contributions.test.ts
  • pnpm exec oxfmt --check scripts\bench-gateway-restart.ts test\scripts\bench-gateway-restart.test.ts src\scripts\test-projects.test.ts src\commands\doctor-sandbox.ts src\commands\doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src\flows\doctor-health-contributions.ts src\flows\doctor-health-contributions.test.ts
  • pnpm tsgo:core
  • pnpm tsgo:test:src
  • git diff --check

@clawsweeper please re-review current head.

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper assist is taking a look at your question.

I queued a lightweight read-only assist pass. It will post a separate answer comment and will not edit the durable ClawSweeper review comment or trigger close, merge, repair, label, or branch changes.

Request: please re-review current head.

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper assist: This request needs a full correctness review of current head 0567ae7aaf9db7cadca80b00a155f7861003f312, so the right path is @clawsweeper review.

Evidence:

  • PR head is now 0567ae7aaf9db7cadca80b00a155f7861003f312 on branch doctor-sandbox-structured-repairs.
  • The latest push says it fixed checks-node-core-tooling in test/scripts/bench-gateway-restart.test.ts by checking the SQLite gateway_restart_intent row instead of legacy gateway-restart-intent.json.
  • Reported validation at that head includes the focused benchmark Vitest run passing 31 passed, 2 skipped, focused doctor/tooling tests passing 143, plus oxlint, oxfmt --check, pnpm tsgo:core, pnpm tsgo:test:src, and git diff --check.
  • Previous ClawSweeper review was for older SHA 81fe719e39b94e6a1bd51e6d29c2910b250aa0a4, not the current head.
  • GitHub reports the PR as open, not merged, and currently mergeableState: dirty.

Suggested next action: Comment @clawsweeper review on #84326 to run the full review against current head.


Source: #84326 (comment)
Assist reasoning: low.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Rebased #84326 onto current upstream/main and force-with-lease pushed 71d4eabfeaad15bf3d424dc383733e7593a18bc7.

Validation after rebase:

  • node scripts\run-vitest.mjs run --config test\vitest\vitest.tooling.config.ts test\scripts\bench-gateway-restart.test.ts --reporter=verbose - 31 passed, 2 skipped
  • $env:OPENCLAW_VITEST_MAX_WORKERS='1'; node scripts\run-vitest.mjs run src\scripts\test-projects.test.ts src\commands\doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src\flows\doctor-health-contributions.test.ts --reporter=dot - 143 passed across 3 shards
  • pnpm exec oxlint scripts\bench-gateway-restart.ts test\scripts\bench-gateway-restart.test.ts src\scripts\test-projects.test.ts src\commands\doctor-sandbox.ts src\commands\doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src\flows\doctor-health-contributions.ts src\flows\doctor-health-contributions.test.ts
  • pnpm exec oxfmt --check scripts\bench-gateway-restart.ts test\scripts\bench-gateway-restart.test.ts src\scripts\test-projects.test.ts src\commands\doctor-sandbox.ts src\commands\doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts src\flows\doctor-health-contributions.ts src\flows\doctor-health-contributions.test.ts
  • pnpm tsgo:test:src
  • git diff --check

pnpm tsgo:core is currently blocked on rebased upstream/main by untouched src/config/sessions/session-accessor.ts:1545 (sessionStore unused, TS6133), so I left that outside this sandbox doctor PR.

@clawsweeper please re-review current head.

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper assist is taking a look at your question.

I queued a lightweight read-only assist pass. It will post a separate answer comment and will not edit the durable ClawSweeper review comment or trigger close, merge, repair, label, or branch changes.

Request: please re-review current head.

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper assist: This needs a full correctness review of current head 71d4eabfeaad15bf3d424dc383733e7593a18bc7; the assist context can confirm the re-review target and recent validation, but not replace a full review.

Evidence:

  • Current PR head is 71d4eabfeaad15bf3d424dc383733e7593a18bc7 on doctor-sandbox-structured-repairs.
  • Previous ClawSweeper review was for older SHA 81fe719e39b94e6a1bd51e6d29c2910b250aa0a4.
  • Latest rebase comment reports focused benchmark validation passed: 31 passed, 2 skipped.
  • Latest rebase comment reports focused doctor/tooling validation passed: 143 passed across 3 shards.
  • pnpm tsgo:core is reported blocked by untouched upstream src/config/sessions/session-accessor.ts:1545 unused sessionStore / TS6133.
  • PR is open, not draft, not merged, and GitHub reports mergeableState: unstable.

Suggested next action: Comment @clawsweeper review on #84326 to run the full review against 71d4eabfeaad15bf3d424dc383733e7593a18bc7.


Source: #84326 (comment)
Assist reasoning: low.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Added a follow-up cleanup commit ba2742aa6adc70d5d713560f4a80f56b5cc49366 on #84326 so pnpm tsgo:core and CI prod-types/lint are not blocked by the rebased-base unused sessionStore destructure in src/config/sessions/session-accessor.ts.

Validation for the follow-up cleanup:

  • pnpm tsgo:core
  • pnpm exec oxlint src\config\sessions\session-accessor.ts
  • pnpm exec oxfmt --check src\config\sessions\session-accessor.ts
  • git diff --check

@clawsweeper please re-review current head.

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper assist is taking a look at your question.

I queued a lightweight read-only assist pass. It will post a separate answer comment and will not edit the durable ClawSweeper review comment or trigger close, merge, repair, label, or branch changes.

Request: please re-review current head.

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper assist: This request needs a full correctness review of current head ba2742aa6a05f1b5626e7c44e05acebe77c7ba63; assist can confirm the target and evidence, but cannot perform the full review.

Evidence:

  • PR is open at Doctor: expose sandbox registry findings #84326 with head ba2742aa6a05f1b5626e7c44e05acebe77c7ba63.
  • Latest commit is ba2742aa6a05f1b5626e7c44e05acebe77c7ba63 / Fix unused session accessor destructure.
  • Previous ClawSweeper review covered older SHA 81fe719e39b94e6a1bd51e6d29c2910b250aa0a4.
  • Latest comment reports pnpm tsgo:core, targeted oxlint, targeted oxfmt --check, and git diff --check passing for src/config/sessions/session-accessor.ts.
  • PR body reports current-head validation also includes focused benchmark tests, focused doctor/tooling tests, pnpm tsgo:test:src, and git diff --check.

Suggested next action: Comment @clawsweeper review on #84326 to run the full review against ba2742aa6a05f1b5626e7c44e05acebe77c7ba63.


Source: #84326 (comment)
Assist reasoning: low.

@galiniliev galiniliev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds a structured core/doctor/sandbox/registry-files lint check for legacy sandbox registry residue while keeping actual mutation in the existing sandbox doctor repair path. That matters because upgraded installs with old monolithic or sharded registry state now get machine-readable doctor findings instead of only legacy human doctor output.

LOC: +175/-14 (7 files)

Findings: No blocking findings.

Bug/behavior: expose legacy sandbox registry residue through openclaw doctor --lint and structured repair previews while preserving openclaw doctor --fix as the real migration/quarantine/removal owner.
Affected surface: src/commands/doctor-sandbox.ts, src/flows/doctor-health-contributions.ts, and focused doctor/tooling tests.
Provenance: N/A; this is a structured doctor feature slice, not a regression fix.

Best-fix verdict: acceptable mitigation / likely best within the current split-doctor migration. Keeping the check owned by doctor:sandbox and leaving non-dry-run mutation in maybeRepairSandboxRegistryFiles matches the existing pattern used by legacy-owned structured checks.

Alternatives considered:

  • Move real migration into the split repair() implementation now. Rejected for this PR because runSandboxHealth already owns the mutating doctor path and doctor --fix calls maybeRepairSandboxRegistryFiles directly; moving it here would be a broader ownership change.
  • Make the finding info instead of warning. Rejected if maintainers accept that stale, noncanonical registry residue should fail default lint until openclaw doctor --fix handles it; this is the open product choice already called out by ClawSweeper.
  • Add a separate contribution for this check. Rejected because the ordered doctor:sandbox contribution is the right owner for sandbox registry residue, and an extra registry entry would split the same invariant.

Code read: src/commands/doctor-sandbox.ts, src/agents/sandbox/registry.ts, src/flows/doctor-health-contributions.ts, src/flows/doctor-health.ts, src/flows/doctor-repair-flow.ts, src/flows/health-check-adapter.ts, src/commands/doctor-lint.ts, scripts/bench-gateway-restart.ts, src/scripts/test-projects.test.ts, test/scripts/bench-gateway-restart.test.ts, and adjacent changed tests. Also checked scoped guides scripts/AGENTS.md and test/AGENTS.md.

Remaining uncertainty: I did not rerun the PR's Vitest/tsgo matrix locally because the checkout has unrelated local edits; I verified the diff against fetched origin/main and checked git diff --check origin/main...origin/pr/84326 locally. Maintainer acceptance is still needed for warning-level doctor --lint behavior on upgraded installs with legacy sandbox registry files.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Merged via squash.

Thanks @giodl73-repo!

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

Labels

commands Command implementations maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants