Skip to content

fix(cli): remove unreachable plugin modules and associated tests#982

Closed
ksapru wants to merge 0 commit intoNVIDIA:mainfrom
ksapru:fix/remove-dead-plugin-modules
Closed

fix(cli): remove unreachable plugin modules and associated tests#982
ksapru wants to merge 0 commit intoNVIDIA:mainfrom
ksapru:fix/remove-dead-plugin-modules

Conversation

@ksapru
Copy link
Copy Markdown

@ksapru ksapru commented Mar 26, 2026

Summary

Removes unreachable plugin modules that are compiled into dist/ but are not referenced from the plugin entry point, reducing package size and maintenance surface.

Related Issue

Fixes #977

Changes

  • Remove unused modules:
    • blueprint/runner.ts
    • blueprint/snapshot.ts
    • blueprint/ssrf.ts
    • commands/migration-state.ts
  • Remove associated test files
  • Verify no remaining static or dynamic import paths to removed modules

Type of Change

  • Code change for a new feature, bug fix, or refactor.
  • Code change with doc updates.
  • Doc only. Prose changes without code sample modifications.
  • Doc only. Includes code sample changes.

Testing

  • npx prek run --all-files passes (or equivalently make check).
  • npm test passes.
  • make docs builds without warnings. (for doc-only changes)

Checklist

General

Code Changes

  • Formatters applied — npx prek run --all-files auto-fixes formatting (or make format for targeted runs).
  • Tests added or updated for new or changed behavior.
  • No secrets, API keys, or credentials committed.
  • Doc pages updated for any user-facing behavior changes (new commands, changed defaults, new features, bug fixes that contradict existing docs).

Doc Changes

  • Follows the style guide.
  • New pages include SPDX license header and frontmatter, if creating a new page.
  • Cross-references and links verified.

Summary by CodeRabbit

Release Notes

  • Revert
    • Removed blueprint planning and application functionality for OpenClaw sandbox orchestration.
    • Removed snapshot creation, restoration, and rollback capabilities for host configuration management.
    • Removed host state detection and migration mechanics for snapshot bundling.
    • Removed security endpoint URL validation and private IP detection.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bb6bc7f2-9986-4fc6-9cf3-941fe885bcb9

📥 Commits

Reviewing files that changed from the base of the PR and between 5c269c1 and 2b244a6.

📒 Files selected for processing (8)
  • nemoclaw/src/blueprint/runner.test.ts
  • nemoclaw/src/blueprint/runner.ts
  • nemoclaw/src/blueprint/snapshot.test.ts
  • nemoclaw/src/blueprint/snapshot.ts
  • nemoclaw/src/blueprint/ssrf.test.ts
  • nemoclaw/src/blueprint/ssrf.ts
  • nemoclaw/src/commands/migration-state.test.ts
  • nemoclaw/src/commands/migration-state.ts
💤 Files with no reviewable changes (8)
  • nemoclaw/src/blueprint/runner.test.ts
  • nemoclaw/src/blueprint/ssrf.test.ts
  • nemoclaw/src/blueprint/snapshot.test.ts
  • nemoclaw/src/blueprint/ssrf.ts
  • nemoclaw/src/commands/migration-state.test.ts
  • nemoclaw/src/blueprint/snapshot.ts
  • nemoclaw/src/commands/migration-state.ts
  • nemoclaw/src/blueprint/runner.ts

📝 Walkthrough

Walkthrough

This PR removes 8 unreachable TypeScript modules and their test files that were compiled into the distribution but had no runtime import path from the plugin entry point. The deleted code includes blueprint runner orchestration, snapshot/restore migration logic, endpoint URL SSRF validation, and host migration state management.

Changes

Cohort / File(s) Summary
Blueprint runner module and tests
nemoclaw/src/blueprint/runner.ts, nemoclaw/src/blueprint/runner.test.ts
Removed CLI orchestration logic for OpenClaw sandbox lifecycle, including run ID emission, blueprint loading, action dispatching (plan, apply, status, rollback), and argument parsing with comprehensive Vitest coverage.
Blueprint snapshot module and tests
nemoclaw/src/blueprint/snapshot.ts, nemoclaw/src/blueprint/snapshot.test.ts
Removed snapshot/restore/cutover/rollback functionality for migrating host OpenClaw configuration into OpenShell sandboxes, including manifest management and test suite with mocked filesystem and execa.
SSRF validation module and tests
nemoclaw/src/blueprint/ssrf.ts, nemoclaw/src/blueprint/ssrf.test.ts
Removed private-IP detection and endpoint URL validation logic that prevented SSRF attacks, including IPv4/IPv6 parsing and DNS resolution tests.
Migration state module and tests
nemoclaw/src/commands/migration-state.ts, nemoclaw/src/commands/migration-state.test.ts
Removed host OpenClaw state detection and snapshot migration mechanics, including state bundling, manifest generation, security validation, and restoration logic with extensive test coverage.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • jyaunches
  • ericksoa

Poem

🐰 Hop, hop, the dust is gone at last!
Unused code removed, the package sleek and fast.
No runner, snapshot, SSRF to weigh us down,
The plugin springs ahead—less bloat, more renown!
Migration dreams are shelved; we ship what's truly blessed.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: removing unreachable plugin modules and their tests to reduce package size.
Linked Issues check ✅ Passed The PR fully addresses issue #977 by removing all four identified unreachable modules (runner.ts, snapshot.ts, ssrf.ts, migration-state.ts) and their associated test files.
Out of Scope Changes check ✅ Passed All changes are directly in scope: only the four identified unreachable modules and their tests are deleted, with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@ksapru
Copy link
Copy Markdown
Author

ksapru commented Mar 27, 2026

After rebasing and running the full test suite locally, I found that the removed modules are still part of core execution and sandbox flows (causing timeouts and test failures).

Reverting this change for now to avoid regressions. I’ll follow up with a more targeted cleanup of unused exports instead.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dead plugin modules: 4 TypeScript files ship in dist/ but are unreachable at runtime

1 participant