fix(e2e): align double-onboard stale-registry test with non-destructive status (#4578)#4635
Closed
hunglp6d wants to merge 1 commit into
Closed
Conversation
…ve status (NVIDIA#4578) Commit 4f7ae09 made `nemoclaw status` non-destructive: it no longer removes stale local registry entries, keeping that cleanup in active-use paths like `connect`. The double-onboard E2E Phase 5 still expected `nemoclaw status` to reconcile (remove) the stale entry. Update the test to: 1. Verify that `status` preserves the registry entry and emits the new "No local registry entry was removed" guidance. 2. Trigger actual reconciliation via `connect --probe-only`, which calls ensureLiveSandboxOrExit and removes the stale entry. Signed-off-by: Hung Le <hple@nvidia.com>
|
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. |
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
2 tasks
9 tasks
Contributor
|
Closing in favor of #4672, which carries the same diff on an upstream NVIDIA/NemoClaw branch so maintainers can dispatch E2E from the replacement PR. |
cv
added a commit
that referenced
this pull request
Jun 3, 2026
…ve status (#4578) (#4672) <!-- markdownlint-disable MD041 --> ## Summary Replaces #4635 with the same diff on an upstream `NVIDIA/NemoClaw` branch so maintainers can dispatch E2E from the replacement PR. The `double-onboard-e2e` nightly job failed at Phase 5 because the test expected `nemoclaw status` to reconcile stale registry entries, but #4578 intentionally made `status` non-destructive. This PR aligns the test with that behavior by verifying the non-destructive `status` output first, then triggering actual reconciliation via `connect --probe-only`. ## Related Issue Fixes #4639 ## Changes - `test/e2e/test-double-onboard.sh`: replace the single `nemoclaw status` reconciliation assertion with a two-step sequence. - Verify `status` exits 1 and preserves the registry entry with the "No local registry entry was removed" guidance. - Trigger reconciliation via `connect --probe-only` and verify "Removed stale local registry entry" plus final registry cleanup. ## 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 - [ ] `npx prek run --all-files` passes - [ ] `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 Note: local checks were not rerun for this replacement branch; it copies #4635 unchanged. #4635 reported focused validation for `double-onboard-e2e` on the same fix commit: https://github.com/hunglp6d/NemoClaw/actions/runs/26792577152. --- Signed-off-by: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Clarified that the status command is non-destructive and explicitly reports when it does not remove local registry entries. * Status and connect now intentionally preserve stale registry entries unless destroyed. * Cleanup now relies on the explicit destroy action to remove registry entries. * Tests updated to verify the non-destructive status message and to avoid forcing reconciliation after teardown. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Hung Le <hple@nvidia.com> Signed-off-by: San Dang <sdang@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Hung Le <hple@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
✨ [AI-generated PR]
The
double-onboard-e2enightly job failed at Phase 5 (Stale registry reconciliation) because the test expectednemoclaw statusto reconcile stale registry entries, but commit 4f7ae09 (fix(status): preserve registry on missing live sandbox (#4578)) intentionally madestatusnon-destructive — it now prints guidance instead of removing entries. This PR aligns the test with the new behavior: it first verifies the non-destructivestatusoutput, then triggers actual reconciliation viaconnect --probe-only(which still callsensureLiveSandboxOrExit).Related Issue
Fixes #4639
Changes
test/e2e/test-double-onboard.sh: Replace the singlenemoclaw statusreconciliation assertion with a two-step sequence:statusexits 1 and preserves the registry entry ("No local registry entry was removed")connect --probe-onlyand verify "Removed stale local registry entry"Validation
A focused
custom-e2e.yamlworkflow was run on a sibling branch to confirm this fix repairs the regression. The workflow re-runs only the jobs from the original nightly that this PR targets, onubuntu-latest, off the same fix commit as this PR.fix/nightly-e2e-status-stale-reconcile-451f26f-custom-e2eonhunglp6d/NemoClawdouble-onboard-e2e (#78975727168)451f26f6a9e56d2bdc05cff47985545bb79c77a2The validation branch is intentionally not the head of this PR — it carries an extra
.github/workflows/custom-e2e.yamlcommit that is scaffolding, not part of the fix. Re-run the validation by pushing any commit to the validation branch.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesAI Disclosure
Signed-off-by: Hung Le hple@nvidia.com