Skip to content

refactor(cli): extract maintenance image helpers#2962

Merged
cv merged 60 commits intomainfrom
refactor/retry-maintenance-image-helpers
May 5, 2026
Merged

refactor(cli): extract maintenance image helpers#2962
cv merged 60 commits intomainfrom
refactor/retry-maintenance-image-helpers

Conversation

@cv
Copy link
Copy Markdown
Contributor

@cv cv commented May 4, 2026

Summary

Extract pure Docker image parsing and orphan-selection logic from the subprocess-heavy maintenance action module.

Stack Navigation

Changes

  • Added maintenance-image-helpers.ts for Docker image row parsing and registered image-tag comparison.
  • Updated garbageCollectImages to use the extracted helpers while keeping Docker/prompt orchestration in maintenance-actions.ts.
  • Added direct helper coverage for image parsing, registered tag collection, and orphan detection.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Refactor

    • Improved internal image maintenance operations with restructured logic for orphaned image detection and cleanup.
  • Tests

    • Added comprehensive test coverage for image maintenance utilities, including image parsing, tag registration, and orphan detection.

cv added 30 commits May 2, 2026 13:36
@cv cv self-assigned this May 4, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 4, 2026

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
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 4, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2b0b3582-062e-430c-8f7c-c3c38c6686ba

📥 Commits

Reviewing files that changed from the base of the PR and between e7e73c1 and 151abad.

📒 Files selected for processing (3)
  • src/lib/maintenance-actions.ts
  • src/lib/maintenance-image-helpers.test.ts
  • src/lib/maintenance-image-helpers.ts

📝 Walkthrough

Walkthrough

A new maintenance utilities module is introduced with three helper functions for parsing sandbox images and identifying orphaned images. The existing garbageCollectImages function is refactored to delegate image parsing and orphan detection to these new utilities, removing duplicated logic.

Changes

Image Maintenance Refactoring

Layer / File(s) Summary
Type & Helper Functions
src/lib/maintenance-image-helpers.ts
Introduces SandboxImageRow type and three helpers: parseSandboxImageRows() parses tab-separated image data into rows with size defaulting to "unknown"; getRegisteredImageTags() builds a Set of non-null imageTag values from sandboxes; findOrphanedSandboxImages() compares parsed images against registered tags to identify orphaned entries.
Refactoring Integration
src/lib/maintenance-actions.ts
garbageCollectImages() is refactored to use parseSandboxImageRows() for initial parsing and findOrphanedSandboxImages() for orphan detection, removing manual image row parsing and local orphan-filter logic.
Tests
src/lib/maintenance-image-helpers.test.ts
Unit tests verify parseSandboxImageRows() handles tab-separated rows with missing sizes, getRegisteredImageTags() extracts non-null imageTag values into a Set, and findOrphanedSandboxImages() correctly identifies images not in the registered tag set.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 Orphaned images once scattered and lost,
Now parsed with precision, sorted with cost,
Helpers extracted, logic made clean,
Maintenance flows clearer than ever we've seen!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/retry-maintenance-image-helpers

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

@cv cv added the v0.0.34 Release target label May 4, 2026
@wscurran wscurran added NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). refactor This is a refactor of the code and/or architecture. labels May 4, 2026
@cv cv requested a review from cjagwani May 5, 2026 00:27
@cv cv added v0.0.35 Release target and removed v0.0.34 Release target labels May 5, 2026
@prekshivyas prekshivyas self-assigned this May 5, 2026
@cv cv requested a review from prekshivyas May 5, 2026 22:40
cv added a commit that referenced this pull request May 5, 2026
## Summary
Stabilize CLI coverage runs by cleaning compiled CLI output before
coverage builds and validating that generated JavaScript sourcemaps
point at existing sources. This makes coverage use a clean `dist/`
baseline instead of stale generated files.

## Stack Navigation
- Position: 46 of 60
- Previous PR: [#2944 — test(cli): cover runtime utility
helpers](#2944)
- Next PR: [#2962 — refactor(cli): extract maintenance image
helpers](#2962)

## Changes
- Added `npm run clean:cli` for removing stale `dist/` artifacts without
changing the normal `build:cli` ordering.
- Added `scripts/check-dist-sourcemaps.ts` and `npm run
dist:sourcemaps:check` to fail fast on stale generated sourcemaps.
- Updated the CLI coverage pre-push hook to clean `dist/`, rebuild, and
validate sourcemaps before running coverage.
- Rebaselined CLI function coverage to the clean-dist value exposed by
the stabilized run.
- Added unit coverage for the sourcemap checker.

## Type of Change
- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Verification
- [x] `npx prek run --all-files` passes
- [x] `npm test` passes
- [x] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [ ] Docs updated for user-facing behavior changes
- [ ] `make docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
@cv cv marked this pull request as ready for review May 5, 2026 23:01
@cv cv changed the base branch from test/stabilize-cli-coverage-dist to main May 5, 2026 23:01
@cv cv enabled auto-merge (squash) May 5, 2026 23:01
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Copy link
Copy Markdown
Contributor

@prekshivyas prekshivyas left a comment

Choose a reason for hiding this comment

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

LGTM. Clean three-helper extraction — 3 files / +85 / -16.

Three pure functions (parseSandboxImageRows, getRegisteredImageTags, findOrphanedSandboxImages) extracted from garbageCollectImages into maintenance-image-helpers.ts. Inline parsing/filter blocks fully replaced by the helper calls. Same parsing rules (tab-split, missing-size default "unknown", empty-line drop), same orphan filter. Zero behavior change.

+43 lines of direct unit tests covering parsing edge cases (empty lines, missing size), registered-tag collection (handles null/missing imageTag), and orphan filter.

CI: pr.yaml mostly green (lint/dco/check-hash/legacy-path-guard/changes PASS); macos-e2e/checks + pr-self-hosted builds still in flight at review time. No failures.

@cv cv merged commit f8f4013 into main May 5, 2026
11 checks passed
cv added a commit that referenced this pull request May 5, 2026
## Summary
Extract pure upgrade sandbox classification helpers from the
subprocess-heavy upgrade action module.

## Stack Navigation
- Position: 48 of 60
- Previous PR: [#2962 — refactor(cli): extract maintenance image
helpers](#2962)
- Next PR: [#2964 — refactor(cli): extract sandbox destroy
helpers](#2964)

## Changes
- Added `upgrade-sandboxes-helpers.ts` for confirmation bypass,
stale/unknown classification, and rebuildable/stopped splitting.
- Updated `upgradeSandboxes` to call the extracted helpers while keeping
OpenShell and rebuild orchestration in the action module.
- Added direct helper coverage for stale, unknown, current, running, and
stopped sandbox cases.

## Type of Change
- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Verification
- [x] `npx prek run --all-files` passes
- [x] `npm test` passes
- [x] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [ ] Docs updated for user-facing behavior changes
- [ ] `make docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Refactored the sandbox upgrade workflow by introducing modular helper
functions that improve code organization, maintainability, and
readability while preserving all existing behavior and functionality.

* **Tests**
* Added comprehensive test suite covering upgrade confirmation handling,
sandbox version classification, and the intelligent grouping of
sandboxes by their rebuild state and configuration.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). refactor This is a refactor of the code and/or architecture. v0.0.35 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants