test(e2e): fix reboot service and TUI status checks - #7618
Merged
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
sandl99
added
integration: openclaw
|
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
📝 WalkthroughWalkthroughThe PR adds OpenShell gateway user-service staging and cleanup to reboot lifecycle fixtures, adds coverage for service ownership and failure outcomes, and separates ChangesGateway service lifecycle
TUI status flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant LifecyclePhaseFixture
participant OpenShellInstaller
participant Docker
participant GatewayRuntime
LifecyclePhaseFixture->>OpenShellInstaller: stage gateway user service
LifecyclePhaseFixture->>Docker: stop and recover containers
LifecyclePhaseFixture->>GatewayRuntime: restart and wait for connectivity
LifecyclePhaseFixture->>OpenShellInstaller: remove staged service
LifecyclePhaseFixture->>GatewayRuntime: restore previous runtime
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 2d30fa4 in the TypeScript / code-coverage/cliThe overall coverage in commit 2d30fa4 in the Show a code coverage summary of the most impacted files.
Updated |
Collaborator
Author
VerdictPASS. The PR is safe to merge from a security perspective. It changes only live-E2E orchestration and support tests, reuses the repository installer’s guarded user-service staging, refuses foreign or symlinked units, and does not alter a supported product runtime or credential boundary. Findings TableThe review found no security findings. Detailed Analysis
Files Reviewed
Reviewed head: |
sandl99
marked this pull request as ready for review
July 27, 2026 07:04
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 4 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
cv
approved these changes
Jul 27, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/e2e/fixtures/phases/lifecycle.ts (1)
415-438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the stage-result regex from
USER_SERVICE_STAGE_RESULT_PREFIXinstead of hardcoding the literal.Line 432 hardcodes
NEMOCLAW_E2E_GATEWAY_USER_SERVICE=again even thoughUSER_SERVICE_STAGE_RESULT_PREFIX(Line 46) already defines it. If the prefix constant ever changes, this parser silently stops matching andensureOpenShellGatewayUserServicethrows "did not report its outcome" instead of failing at the source of the mismatch.♻️ Proposed fix
- const match = result.stdout.match( - /(?:^|\n)NEMOCLAW_E2E_GATEWAY_USER_SERVICE=(upstream|existing|staged)(?:\n|$)/u, - ); + const match = result.stdout.match( + new RegExp(`(?:^|\\n)${USER_SERVICE_STAGE_RESULT_PREFIX}(upstream|existing|staged)(?:\\n|$)`, "u"), + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/fixtures/phases/lifecycle.ts` around lines 415 - 438, Update ensureOpenShellGatewayUserService to construct the stage-result regex from the existing USER_SERVICE_STAGE_RESULT_PREFIX constant rather than hardcoding NEMOCLAW_E2E_GATEWAY_USER_SERVICE=. Preserve the current matching boundaries and accepted outcomes (upstream, existing, and staged).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/support/issue-6194-tui-post-idle-contract.test.ts`:
- Around line 78-82: Update the assertions in the issue-6194 test to verify that
the `Sandbox:` status expectation occurs before the connected-idle expectation,
rather than only checking that both strings are present. Preserve the existing
absence assertion for `{NemoClaw Status}` and validate the ordering through the
script’s behavioral sequence where possible.
In `@test/e2e/support/lifecycle-user-service.test.ts`:
- Around line 49-53: Update all four synchronous execFileSync invocations in
lifecycle-user-service.test.ts to include a positive timeout and killSignal set
to "SIGKILL" in their options objects, preserving the existing encoding and
environment settings.
---
Nitpick comments:
In `@test/e2e/fixtures/phases/lifecycle.ts`:
- Around line 415-438: Update ensureOpenShellGatewayUserService to construct the
stage-result regex from the existing USER_SERVICE_STAGE_RESULT_PREFIX constant
rather than hardcoding NEMOCLAW_E2E_GATEWAY_USER_SERVICE=. Preserve the current
matching boundaries and accepted outcomes (upstream, existing, and staged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ef751b7b-4da3-4845-910c-c605d06b097a
📒 Files selected for processing (5)
test/e2e/fixtures/phases/lifecycle.tstest/e2e/live/issue-6194-tui-expect.tstest/e2e/support/e2e-phase-lifecycle.test.tstest/e2e/support/issue-6194-tui-post-idle-contract.test.tstest/e2e/support/lifecycle-user-service.test.ts
cv
approved these changes
Jul 27, 2026
23 tasks
cv
pushed a commit
that referenced
this pull request
Jul 27, 2026
<!-- markdownlint-disable MD041 --> ## Summary <!-- 1-3 plain sentences: what changes and why. Describe before-and-after behavior when it applies. Follow the NemoClaw Writing Guide: https://github.com/NVIDIA/NemoClaw/blob/main/WRITING.md. Do not add unrelated prose cleanup. --> Recover three current-lifecycle E2E regressions from run [30283434843](https://github.com/NVIDIA/NemoClaw/actions/runs/30283434843). Recreated sandboxes now tolerate the exact transient OpenShell `Error` phase, current-lifecycle EXDEV coverage uses the current CLI's compatible base image, and successful post-reboot service staging no longer invokes the sourced installer's process-level cleanup trap. ## Related Issue <!-- Fixes #NNN or Closes #NNN. Remove this section if none. --> Follow-up to #7618. Related to #7273 and #6108. ## Changes <!-- List concrete changes. If this adds an abstraction, configuration, fallback, migration, or compatibility path, name its current requirement and consumer, explain why a direct change is insufficient, and identify the test that protects it. --> - Retry only the exact same-sandbox `phase: Error` readiness response inside the existing recreated-sandbox health guard and timeout; foreign sandboxes and other phases remain terminal. - Clear the E2E staging fixture's inherited `EXIT` trap after successful upstream or NemoClaw service staging, while preserving rollback on staging failures. - Keep the release EXDEV baseline pinned to its historical sandbox image and let the current-lifecycle target use the current CLI's validated base-image resolution. - Add focused regressions for each behavior. ## 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) ## Quality Gates <!-- Check one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: Internal recovery behavior and E2E fixture coverage changed without changing a user-facing command, configuration, output, or supported workflow. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: [PASS with no findings](#7650 (comment)); exact same-sandbox matching remains behind existing health and timeout guards. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review <!-- Required for code and documentation changes after the changes and applicable validation are complete. Keep one review checkbox and one instance of each visible or hidden field. For Evidence, list changed documentation paths. For documentation-only changes, also state that the writing rules and documentation style were reviewed. For other results, explain why no documentation change is needed or why the review is blocked. For Agent, use a consistent product and surface name, such as Codex Desktop, Codex CLI, Claude Code, or Cursor. After committing all review changes, put `git rev-parse --short HEAD` and `git rev-parse --short HEAD:AGENTS.md` in the hidden metadata below. Rerun the review and refresh that metadata after any new commit. This receipt is advisory during the data-collection pilot. --> - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Reviewed all seven committed files; the changes are internal recovery behavior and E2E fixtures/tests with no user-facing documentation contract change. `git diff --check origin/main...HEAD` passed. - Agent: Codex documentation-writer subagent <!-- docs-review-head-sha: 497ca2d --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence <!-- Required only when scripts/prepare-dgx-station-host.sh changes. Maintainers must review the linked evidence before approving or merging. This is human-reviewed evidence, not authenticated hardware provenance. Exceptional bypasses use existing repository governance and must be documented on the PR. --> - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run --project cli src/lib/actions/sandbox/process-recovery.test.ts` (41 passed); focused E2E-support tests (7 passed); full `e2e-support` project (1,744 passed, 17 skipped). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run 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) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved sandbox recovery when OpenShell reports a temporary error state, allowing readiness checks to retry until the sandbox becomes available. * Prevented unrelated sandbox errors and failed states from being retried incorrectly. * Preserved installer cleanup sentinel files during gateway service staging. * **Tests** * Expanded coverage for sandbox recovery, lifecycle behavior, and runtime base-image selection. * Added validation for release-baseline and current-lifecycle deployment scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: San Dang <sdang@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
2 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
Repairs two deterministic false failures from E2E run 30228379802. Source-checkout reboot recovery now supplies the managed gateway user service that the scenario requires, and the OpenClaw TUI check now correlates status output with a response-only sentinel instead of echoed input or an earlier idle redraw.
Changes
Sandbox:and then the subsequent connected/idle redraw after/nemoclaw status.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededVerification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project e2e-support test/e2e/support/lifecycle-user-service.test.ts test/e2e/support/e2e-phase-lifecycle.test.ts test/e2e/support/issue-6194-tui-post-idle-contract.test.ts(34 passed, 7 skipped)npm run test:e2e-phases:check(125 tests across 82 files)npm run docsbuilds without warnings (doc changes only)Signed-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests