Conversation
This reverts commit 4ebeae4.
|
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. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughCore sandbox logs utilities are extracted from ChangesSandbox Logs Extraction & Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
## Summary Extract pure sandbox destroy decision helpers from the destructive destroy action module. ## Stack Navigation - Position: 49 of 60 - Previous PR: [#2963 — refactor(cli): extract upgrade sandbox helpers](#2963) - Next PR: [#2965 — refactor(cli): extract sandbox logs helpers](#2965) ## Changes - Added `sandbox-destroy-helpers.ts` for delete outcome parsing and cleanup decision helpers. - Updated destroy and rebuild actions to import delete outcome parsing from the helper module. - Moved/added helper tests for missing-sandbox output, delete outcomes, host-service cleanup decisions, and gateway cleanup decisions. ## 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 <[email protected]> --------- Signed-off-by: Carlos Villela <[email protected]>
Signed-off-by: Carlos Villela <[email protected]>
prekshivyas
left a comment
There was a problem hiding this comment.
LGTM. Same extraction pattern as #2962/#2963/#2964 — 3 files / +116 / -105.
Six helpers moved verbatim into sandbox-logs-helpers.ts plus one new shared utility, exitCodeFromSignal, that de-duplicates the same signal-to-exit-code logic that was previously inlined in two places (exitWithSpawnResult + the streamSandboxFollowLogs::exitFromSignal lambda). sandbox-logs-action.ts shrinks -86/+15 by importing.
SpawnLikeResult → LogProbeResult (internal rename). getLogsProbeTimeoutMs now takes env as a parameter (default process.env) — testability improvement, production unchanged.
Test file renamed (73% similarity, not deleted). Restructure drops vi.mock boilerplate (no longer needed since helpers are pure) and switches the timeout test from vi.stubEnv to explicit env passing — cleaner, no global mutation. +1 new test for exitCodeFromSignal.
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.
## Summary Extract pure policy-add argument parsing helpers from the policy/channel action module. ## Stack Navigation - Position: 51 of 60 - Previous PR: [#2965 — refactor(cli): extract sandbox logs helpers](#2965) - Next PR: [#2967 — refactor(cli): group oclif core modules](#2967) ## Changes - Added `policy-channel-helpers.ts` for custom policy source parsing, confirmation bypass detection, and policy-add argument normalization. - Updated `addSandboxPolicy` to use the extracted parser helpers while keeping filesystem and policy application logic in the action module. - Added direct helper coverage for `--from-file`, `--from-dir`, mutual exclusion, missing values, confirmation aliases, and preset argument parsing. ## 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 <[email protected]> --------- Signed-off-by: Carlos Villela <[email protected]> Co-authored-by: Prekshi Vyas <[email protected]>
Summary
Extract pure sandbox log option and argv helpers from the log streaming action module.
Stack Navigation
Changes
sandbox-logs-helpers.tsfor option normalization, timeout parsing, probe descriptions, signal exit codes, and OpenShell argv builders.sandbox-logs-action.tsto import the helper logic and focus on subprocess orchestration.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela [email protected]
Summary by CodeRabbit
Refactor
Tests