No actionable comments were generated in the recent review. 🎉

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai review

Automated PR review summary

Reviewed PR #3080: refactor(uninstall): extract plan domain helpers

Recommendation

• Recommendation: PASS
• Highest observed severity: low
• Block merge: no
• Why: The main risks in this refactor were silent semantic drift in uninstall planning and overbroad shim deletion. Targeted runtime/source-backed probes showed strict shim classification, correct env-derived uninstall path planning, correct toggle behavior, and unchanged uninstall.sh entrypoint wiring.
• Reviewer summary: Reviewed the PR with PR-specific adversarial probes against the extracted uninstall planning helpers and the installed CLI wiring. The tested plan-layer behaviors matched the PR's stated refactor-only scope.

Installation and setup findings

• Nemoclaw installation from the local repo worked. I used the repository installer entrypoint in non-interactive mode with the local checkout as source, confirmed the created sandbox via nemoclaw list/status, SSHed into that NemoClaw-managed sandbox and got '2+2=4', then ran an in-sandbox OpenClaw probe that returned '4'. The only incomplete piece is that the original installer process hit the 600s timeout while onboarding policy setup was still running, but the installed CLI, created sandbox, and provider reachability were all proven functional.

What was validated

• The PR revision was checked out in an isolated review environment.
• The local checkout was installed using the repository installer flow as closely as the environment allowed.
• Adversarial, PR-specific probes were then run against the installed environment and relevant repository context.
• Diff summary:

 .../skills/nemoclaw-contributor-create-pr/SKILL.md |   20 +-
 .../nemoclaw-maintainer-pr-comparator/SKILL.md     |  121 --
 .../checks/tier-0-gates.md                         |   61 -
 .../checks/tier-1-correctness.md                   |   86 --
 .../checks/tier-2-quality.md                       |   64 --
 .../repo-policy.md                                 |   86 --
 .../scripts/check-coderabbit-threads.sh            |  105 --
 .../scripts/collect-gates.sh                       |   84 --
 .../scripts/find-candidates.sh                     |   86 --
 .../scripts/parse-supersession.sh                  |   68 --
 .../scripts/render-verdict.py                      |  232 ----
 .../templates/verdict.md                           |   88 --
 .../tiebreakers.md                                 |   61 -
 .../validation/backtest.md                         |   69 --
 .agents/skills/nemoclaw-skills-guide/SKILL.md      |    2 +-
 .../references/agent-skills.md                     |    4 +-
 .../nemoclaw-user-configure-inference/SKILL.md     |  329 ++----
 .../references/inference-options.md                |    4 +-
 .../references/set-up-sub-agent.md                 |  120 --
 .../references/sw
...[truncated]

Failing tests and unresolved impact

• No failing adversarial tests were captured.

Passing tests and why they mattered

Passing test 1: Shim classification is strict enough to avoid deleting user-managed wrapper-like files

• What was tested: Only exact installer/dev-managed shim layouts are removable; wrapper-like files with extra content are preserved.
• Why it mattered: If false, uninstall planning could delete user-managed executables or other foreign files.
• Observed result: Probe classified an exact 3-line wrapper as managed-wrapper/remove=true, while the same wrapper plus an extra line became preserve-foreign-file/remove=false.
• Command: node /tmp/pr3080_probe.mjs
• Recommended follow-up coverage: Keep this as unit/regression coverage because matcher broadening would create destructive false positives during uninstall.

Passing test 2: Host uninstall plan derives cleanup targets from env while preserving a foreign shim

• What was tested: buildHostUninstallPlan uses HOME/TMPDIR/XDG_BIN_HOME-derived paths and preserves a non-managed shim instead of deleting it.
• Why it mattered: If false, the refactor could change uninstall targets, miss cleanup, or remove a user-managed binary.
• Observed result: Flattened plan preserved the foreign shim, used TMPDIR-based cleanup globs, targeted state dirs under the supplied HOME, and included OpenShell binary paths /usr/local/bin/openshell plus <XDG_BIN_HOME>/openshell.
• Command: node /tmp/pr3080_probe.mjs
• Recommended follow-up coverage: Keep integration-style regression coverage around env-derived path construction and shim preservation because future wrapper migration will depend on it.

Passing test 3: Toggle handling matches claimed uninstall planning behavior

• What was tested: keepOpenShell/deleteModels/custom gateway toggles produce preserve-openshell-binary, delete-ollama-model, and custom gateway/docker-volume actions without semantic drift.
• Why it mattered: If false, the extracted planning layer would not faithfully model uninstall.sh behavior for future command-wrapper migration.
• Observed result: Observed destroy-openshell-gateway customgw, delete-docker-volume openshell-cluster-customgw, deletion of both Ollama model actions, preserve-openshell-binary, and state-path deletions in expected order.
• Command: node /tmp/pr3080_probe.mjs
• Recommended follow-up coverage: Keep as regression/integration coverage because stacked follow-up PRs will likely reuse this plan contract directly.

Passing test 4: Runtime uninstall entrypoint remains uninstall.sh

• What was tested: This PR is plan-layer only and does not switch the user-facing uninstall command away from the shell script.
• Why it mattered: If false, runtime behavior changed beyond the stated refactor scope and would need end-to-end uninstall validation.
• Observed result: Source inspection showed src/lib/commands/uninstall.ts and command registry still describe running uninstall.sh; no evidence that this PR replaced the shell entrypoint.
• Command: grep -RIn -E 'uninstall\.sh|buildHostUninstallPlan' src/lib uninstall.sh package.json
• Recommended follow-up coverage: No new automated test needed for this refactor-only PR; add an integration test when the next PR actually introduces the wrapper entrypoint.

Bottom line

• Based on the install evidence and adversarial probes, this PR looks reasonable to approve.