fix(e2e): stabilize release candidate validation - #6840
Conversation
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR updates managed gateway recovery retries, adds a dedicated GPU patch stop timeout, strengthens E2E gateway PID and Hermes image-state handling, adjusts GPU inference test parameters, and records validation-derived cold-path performance budgets. ChangesCold-path performance calibration
Managed gateway recovery
Compatibility GPU patch timeout
E2E fixture reliability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RecoveryController
participant ManagedProbe
participant SettleWindow
RecoveryController->>ManagedProbe: Probe before settle deadline
ManagedProbe-->>RecoveryController: Return managed health result
RecoveryController->>SettleWindow: Sleep until deadline
RecoveryController->>ManagedProbe: Probe at settle deadline
ManagedProbe-->>RecoveryController: Return final managed health result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence 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: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/lib/actions/sandbox/process-recovery.ts`:
- Around line 791-800: Update the managed probe retry loop around managedProbe
so its retry count or deadline is derived from settleSeconds, not the broader
attempts budget based on timeoutSeconds. Ensure retries, including interval
sleeps, cannot extend beyond the configured settle window while preserving the
existing authenticated-result and definitive-failure behavior.
In `@test/e2e/support/e2e-recovery-helpers.test.ts`:
- Around line 196-200: Replace the literal shell-script assertions in the
resolveGatewayPid test with behavior-based cases through the public
resolveGatewayPid boundary. Provide scripted process responses that demonstrate
matching starttime acceptance and stale or mismatched PID rejection, and assert
the returned outcomes; remove the toContain/not.toContain checks for
implementation details.
🪄 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: f51ca997-1cbb-43e0-be8e-0279a89fd624
📒 Files selected for processing (16)
ci/full-e2e-cold-path-calibration.jsonci/onboard-performance-budget.jsonci/source-shape-test-budget.jsonsrc/lib/actions/sandbox/process-recovery.tssrc/lib/onboard/docker-gpu-patch-constants.tssrc/lib/onboard/docker-gpu-patch-recreate.test.tssrc/lib/onboard/docker-gpu-patch-recreate.tstest/e2e/fixtures/clients/gateway.tstest/e2e/live/gpu-e2e.test.tstest/e2e/live/rebuild-hermes-image-state.tstest/e2e/live/rebuild-hermes.test.tstest/e2e/mock-parity.jsontest/e2e/support/e2e-recovery-helpers.test.tstest/e2e/support/rebuild-hermes-image-state.test.tstest/onboard-performance-config-schema.test.tstest/process-recovery-managed-controller.test.ts
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Addressed both informational advisor warnings in
Focused verification: 28 targeted integration tests, 491 changed-scope tests, source-shape guard, Biome, diff checks, and |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Addressed both current CodeRabbit findings in 1a653e8:
Validation on this head: focused recovery suites, 830 affected tests via |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/fixtures/clients/gateway.ts`:
- Around line 185-186: Update the shell validation in expectPidStable to inspect
the process state from $1 after set -- $rest and exit before printing the
identity when it is a terminal state such as Z or X. Preserve the existing
start-time validation and only print the PID identity for non-terminal
processes.
🪄 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: 014367e9-8c20-4d21-a6dc-dc94c9c15a3b
📒 Files selected for processing (6)
src/lib/actions/sandbox/gateway-restart.test.tssrc/lib/actions/sandbox/process-recovery.test.tssrc/lib/actions/sandbox/process-recovery.tstest/e2e/fixtures/clients/gateway.tstest/e2e/support/e2e-recovery-helpers.test.tstest/process-recovery-managed-controller.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/process-recovery-managed-controller.test.ts
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
✨ Thanks for the fix, @cjagwani. Stabilizing the release candidate validation should improve daily release reliability. Ready for maintainer review. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-6840.docs.buildwithfern.com/nemoclaw |
PR #6840 is now conflict-free at head |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Maintainer pass on exact head The current primary advisor warning is still valid. Please either add a repository-backed provenance assertion for the adjustment evidence (including a negative case), or replace the adjustment with a fresh exact-head five-run calibration and remove the interim claims. Also sync the branch with current @coderabbitai resume |
|
Tip For best results, initiate chat on the files or code changes. #!/bin/bash
set -euo pipefail
fd -a 'onboard-performance-config-schema.test.ts' || true
fd -a 'full-e2e-cold-path-calibration.json' || true
echo "---calibration json---"
cat ci/full-e2e-cold-path-calibration.json 2>/dev/null | head -200
echo "---schema test---"
sed -n '1,250p' test/onboard-performance-config-schema.test.ts 2>/dev/null
</parameter>
</invoke> |
…-evidence-followups Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
@cv, please authorize the credentialed E2E plan for this exact reviewed revision.
The coordination failure is the expected authorization gate: no selected E2E job ran and no repository secret was exposed. Please run |
|
@jyaunches, could you authorize the exact credentialed E2E plan while @cv is unavailable?
All ordinary CI and exact-head Terra/Nemotron reviews are clean; CodeRabbit has no active unresolved thread; commits are Verified and DCO-signed. No credentialed job has run and no secret was exposed. Please use |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@cv @jyaunches — please review and authorize the exact credentialed E2E plan for #6840 through the E2E / PR Gate Controller
Selected jobs:
The PR is open, mergeable, approved, and bound to current Please enter PR |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Please hold further sync-only pushes while the current validation runs. The last three main-merge commits have each invalidated an active CI/E2E evidence run, and GitHub can merge a clean behind branch safely. Please sync again only if a real conflict appears or a maintainer asks for it; we are trying to land this release target promptly. |
Done :) |
|
@cv — the exact current-head E2E plan is ready for maintainer authorization. I have held further sync-only pushes as requested.
The authoritative gate-producing ordinary CI run is green, the exact-head primary advisor has 0 blockers / 0 warnings / 0 suggestions, CodeRabbit has no unresolved thread, all commits are Verified and DCO-signed, and the PR is mergeable. No selected E2E job has run and no repository secret has been exposed. Please run |
|
@cv — the fresh exact-diff E2E plan is ready after your signed empty retry commit.
The new-head authoritative CI is green, all eight CLI shards passed, the canonical primary advisor is high-confidence with 0 blockers / 0 warnings / 0 suggestions, CodeRabbit has no unresolved thread, and the PR remains approved and mergeable. No selected job has run for this exact SHA and no repository secret was exposed. The prior Please launch a fresh first-attempt |
Summary
Fix the non-Jetson failures from the v0.0.82 release-candidate run. Managed recovery now distinguishes transient supervisor races from definitive failure, the GPU and full-E2E assertions use bounded evidence-based limits, and the Hermes rebuild fixture reclaims only its exact owned images instead of accumulating them until the hosted runner is lost.
Changes
Escaped-defect evidence
The exact-candidate run exposed test assumptions that existing fast coverage did not exercise: retryable post-settle supervisor responses were treated as definitive, the live PID oracle parsed process listings instead of the supervisor identity record, a routed GPU response could spend its small token budget on reasoning, and the Hermes fixture overwrote registry image ownership before production cleanup could reclaim it. The Hermes jobs then lost separate hosted runners before logs or artifacts could upload. The new focused tests cover the stable local boundaries, while the live fixture now records and proves exact image reclamation; a fresh full default E2E run remains the authoritative integrated validation after merge.
Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm run test:changedpassed 1,041 tests in 102 files; focused Hermes ownership/progress and mock-parity tests passed; CLI typecheck, exact project membership, source-shape guard, Biome, and diff checks passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — not claimed: an exploratory localnpm testwas stopped after unrelated local concurrency/environment failures; required PR CI and the fresh exact-SHA full live run are the authoritative broad evidence.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Summary by CodeRabbit
Bug Fixes
/procstart-identity/state.Tests / Validation