Skip to content

fix(cli): reconcile provisional container exits - #3204

Merged
johntmyers merged 1 commit into
NVIDIA:mainfrom
shiju-nv:fix/canonical-main-exit-race
Sep 9, 2026
Merged

fix(cli): reconcile provisional container exits#3204
johntmyers merged 1 commit into
NVIDIA:mainfrom
shiju-nv:fix/canonical-main-exit-race

Conversation

@shiju-nv

@shiju-nv shiju-nv commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Treat a driver-reported ContainerExited without a canonical main-process result as provisional for a bounded five-second reconciliation window. This prevents sandbox create --no-keep from deleting a fast finite-command sandbox before the supervisor can publish its authoritative exit status, while preserving the concrete driver error when no supervisor result arrives.

Related Issue

No issue required: this is an obvious localized bug fix for the intermittent Release Dev Rust Docker E2E failure and completes the CLI side of the lifecycle reconciliation introduced by PR #3101.

Changes

  • Recognize Error sandboxes with Ready=False, reason ContainerExited, and no exit code as provisional during sandbox create.
  • Continue watching until the canonical main-process result arrives, the existing provisioning deadline expires, or a fixed five-second reconciliation deadline expires.
  • Keep the first reconciliation deadline fixed so repeated status events and logs cannot extend the wait.
  • Preserve immediate failure for other infrastructure errors and retain the concrete ContainerExited diagnostic when reconciliation expires.
  • Add deterministic integration coverage for both a later successful canonical result with --no-keep cleanup and a missing-result deadline.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (the existing Docker lifecycle test is the CI validation target)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (not applicable; no architecture or public API change)

Keep watching briefly when a compute driver reports ContainerExited without a canonical main-process result. This prevents --no-keep cleanup from deleting the sandbox before the supervisor publishes the authoritative exit status.

Signed-off-by: Shiju <shiju@nvidia.com>
@shiju-nv
shiju-nv force-pushed the fix/canonical-main-exit-race branch from ecb539f to 223c245 Compare September 5, 2026 22:22
@johntmyers johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage labels Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

Label test:e2e applied for 223c245. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers johntmyers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

This focused lifecycle fix is project-valid, and the initial code review found no blocking defects. The change preserves the existing provisioning deadline while adding a bounded reconciliation opportunity for a later canonical main-process result.

Blocking findings:

  • No blocking findings remain

Carried findings:

  • None

Non-blocking suggestions:

  • None
Gator metadata
  • Validation: Localized CLI lifecycle bug fix with clear failure context and deterministic integration coverage
  • Docs: Not needed because this restores the existing authoritative exit-result and cleanup behavior without changing a published command or configuration contract
  • Checks: Current-head branch checks are green; required E2E rerun is queued
  • E2E: test:e2e applied; current-head Branch E2E Checks run 33995733434 queued after the label-help instruction
  • Head SHA: 223c245cd18ee426736f4c1cb8d0c885230a44ef
  • Base SHA: 320d4ef79dd572c642133f175f12bafc20d89fd9
  • Merge base SHA: 320d4ef79dd572c642133f175f12bafc20d89fd9
  • Patch ID: 39fa5507a8e5bf726feb90335c83519ea66d9aa3
  • Gator payload: 8
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:watch-pipeline

@johntmyers johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates gator:approval-needed Gator completed review; maintainer approval needed and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates gator:approval-needed Gator completed review; maintainer approval needed labels Sep 9, 2026
@johntmyers
johntmyers added this pull request to the merge queue Sep 9, 2026
@johntmyers johntmyers added gator:merge-ready and removed gator:approval-needed Gator completed review; maintainer approval needed labels Sep 9, 2026
Merged via the queue into NVIDIA:main with commit 3ea0ce8 Sep 9, 2026
120 of 127 checks passed
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: Gator found no blocking review defects, the required E2E checks completed successfully, maintainer approval was present, and the PR merged from gator:merge-ready.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

Gator metadata
  • Head SHA: 223c245cd18ee426736f4c1cb8d0c885230a44ef
  • Gator payload: 8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants