Skip to content

fix(installer): retain valid DGX Station prerequisites - #7090

Merged
apurvvkumaria merged 14 commits into
mainfrom
fix/station-version-drift-warning
Jul 18, 2026
Merged

fix(installer): retain valid DGX Station prerequisites#7090
apurvvkumaria merged 14 commits into
mainfrom
fix/station-version-drift-warning

Conversation

@sandl99

@sandl99 sandl99 commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

DGX Station host preparation now follows the host-profile boundary established by merged PR #7108: generic Ubuntu requires the reviewed exact prerequisite tuple, while qualifying stock DGX OS validates its factory runtime without package or runtime mutation. The installer also accepts an exact verified Docker .asc source, selects GB300 driver and ECC rows by PCI identity on mixed-GPU systems, and keeps express output concise while retaining the full audit log.

Changes

  • Reconcile the branch with current main and preserve the fail-closed generic Ubuntu and validation-only stock DGX OS profiles from fix(installer): enable stock DGX OS on Station GB300 #7108. Unknown package, driver, and CUDA-keyring drift is rejected; the reviewed factory DKMS transition remains the only generic profile exception.
  • Pass the complete pinned prerequisite tuple unchanged through APT availability, simulation, installation, and exact post-install verification. This avoids a missing-only solve that could mutate retained prerequisites.
  • Reuse an existing Docker repository only when its source bytes exactly match the reviewed .gpg or .asc form and the referenced key is byte-identical to the freshly downloaded SHA-256 and fingerprint-verified key. New repository configuration continues to use the dearmored .gpg form.
  • Match nvidia-smi rows to the GB300 through exact PCI vendor, device, and display-class identity before applying driver and ECC qualification. Auxiliary NVIDIA GPUs are reported and skipped.
  • Show the Station audit-log path plus all warnings and errors in express install, while keeping verbose preparation details in the private log.
  • Update prerequisites documentation and split focused Docker-repository and package-transaction tests out of the host-preparation suite.

Type of Change

  • 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

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: The current revision implements the fail-closed boundaries requested in maintainer feedback and aligns profile handling with merged fix(installer): enable stock DGX OS on Station GB300 #7108. Exact-profile, full-tuple APT, PCI-identity, CUDA-keyring, repository-file, rollback, and file-safety contracts have focused regression coverage. The current head awaits human re-review.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — full npx vitest run --project installer-integration --testTimeout=15000 (295 passed, 2 skipped); Station platform identity (22 passed); npm run test:projects:check; npm run test-size:check; Bash syntax and ShellCheck.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — not applicable: this is a focused installer revision, and the complete installer-integration project plus diff-scoped repository gate passed.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only) — passed with 0 errors; Fern continues to report 2 existing warnings.
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: San Dang sdang@nvidia.com
Signed-off-by: Julie Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Improved DGX Station GPU detection and driver/ECC validation by matching devices to their PCI identities.
    • Added stricter Docker repository and signing-key compatibility checks.
    • Prevented unsafe or unexpected package changes during host preparation.
    • Improved installer output by preserving useful logs, warnings, and errors while filtering irrelevant details.
  • Improvements

    • Updated DGX Station host preparation version and strengthened installation reliability.

@sandl99 sandl99 added area: docs Documentation, examples, guides, or docs build area: install Install, setup, prerequisites, or uninstall flow bug-fix PR fixes a bug or regression platform: dgx-station Affects DGX Station hardware or workflows labels Jul 17, 2026
@sandl99 sandl99 self-assigned this Jul 17, 2026
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

DGX Station preparation now validates Docker APT sources, maps NVIDIA telemetry to GB300 PCI devices, enforces guarded package transactions, and filters preparation output. New and updated tests cover these behaviors and register the Docker repository suite with Vitest.

Changes

DGX Station preparation validation

Layer / File(s) Summary
Docker repository source validation
scripts/prepare-dgx-station-host.sh, test/install-station-docker-repository.test.ts, vitest.config.ts, scripts/checks/vitest-project-overlap.ts
Docker repository source files and key material are validated against approved GPG and ASCII variants, with coverage for compatible, divergent, and symlinked states.
GB300 PCI selection and GPU validation
scripts/prepare-dgx-station-host.sh, test/install-station-platform-identity.test.ts, test/install-station-dgx-os.test.ts
GPU checks normalize PCI IDs, identify GB300 devices through sysfs, select matching nvidia-smi rows, and enforce driver and ECC requirements.
Guarded package transaction flow
scripts/prepare-dgx-station-host.sh, test/install-station-package-transaction.test.ts, test/install-station-host-preparation.test.ts
APT plans and simulations are validated before guarded installation, with transaction cleanup and coverage for accepted and rejected actions.
Station preparation output and express integration
scripts/install.sh, test/install-express-prompt.test.ts, test/install-station-host-preparation.test.ts
Preparation logs, warnings, and errors are reformatted while exit status is preserved; express-flow assertions verify output ordering.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Installer
  participant StationPreparation
  participant GPUAndAPTValidation
  participant OutputFilter
  Installer->>StationPreparation: run host preparation
  StationPreparation->>GPUAndAPTValidation: validate GB300 identity and APT transaction
  GPUAndAPTValidation-->>StationPreparation: validation result
  StationPreparation-->>OutputFilter: emit preparation output and status
  OutputFilter-->>Installer: formatted output and preserved status
Loading

Possibly related PRs

Suggested labels: area: security

Suggested reviewers: jyaunches, senthilr-nv, cv, ericksoa

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately reflects the main change: preserving valid DGX Station prerequisites during installer preparation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/station-version-drift-warning

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

@github-code-quality

github-code-quality Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the fix/station-version-... branch remains at 80%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 980e348 fix/station-version-... e2cc24a +/-
bin/lib/credentials.js 100% 0% -100%
src/lib/inferen...lama/process.ts 100% 50% -50%
src/lib/domain/.../connect-env.ts 97% 89% -8%
src/lib/inferen...er-lifecycle.ts 71% 65% -6%
src/lib/actions...rget-runtime.ts 95% 90% -5%
src/lib/onboard...der-metadata.ts 100% 100% 0%
src/lib/sandbox...rce-identity.ts 91% 91% 0%
src/lib/state/m...-acquisition.ts 84% 84% 0%
src/lib/inference/local.ts 80% 82% +2%
src/lib/actions...flight-error.ts 80% 100% +20%

Updated July 18, 2026 20:44 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

Signed-off-by: San Dang <sdang@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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/install-station-host-preparation.test.ts`:
- Around line 449-461: Update the test assertions around the APT command checks
to filter the output into the relevant availability, simulation, and
installation lines, then compare each complete argument list exactly. Ensure the
assertions reject any appended dkms package while preserving the existing
expected docker-ce version and command behavior.
🪄 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: b1632f24-11ff-4fc6-bd68-5534eb645778

📥 Commits

Reviewing files that changed from the base of the PR and between 027632a and 84f5bb4.

📒 Files selected for processing (9)
  • docs/get-started/prerequisites.mdx
  • docs/get-started/quickstart.mdx
  • scripts/checks/vitest-project-overlap.ts
  • scripts/install.sh
  • scripts/prepare-dgx-station-host.sh
  • test/install-station-docker-repository.test.ts
  • test/install-station-host-preparation.test.ts
  • test/test-boundary-guards.test.ts
  • vitest.config.ts

Comment thread test/install-station-host-preparation.test.ts Outdated
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 2 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 2 more warnings, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-onboard, credential-sanitization, security-posture

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

sandl99 added 2 commits July 17, 2026 15:15
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
@senthilr-nv

Copy link
Copy Markdown
Collaborator

Reviewed current head 45719e0ca72957763c56fc32d93b6eeab2a45d27.

The goal is right: preserve a valid preinstalled Station stack instead of replacing it, and the Docker .asc compatibility checks plus compact installer output are useful. I think the version-retention boundary needs tightening before merge:

  1. Arbitrary package and driver drift becomes accepted. package_is_installed(), all_packages_installed(), and driver_is_loaded() make presence sufficient during check, apply, and verify. That accepts unqualified older, newer, or mixed driver/Docker/Toolkit/DKMS combinations. Please retain only an explicit qualified factory tuple or profile; unknown drift should fail closed with remediation.

  2. The missing-only APT solve can mutate retained packages. simulate_install() rejects removals, but does not reject upgrades or downgrades of already-installed prerequisites pulled in as dependencies. Post-install verification checks only the packages that were initially missing. Please compare the complete simulated transaction against the pre-state and reject any unapproved change to retained packages.

  3. The shipping OOTB topology cannot assume GPU index 0. The customer image includes an auxiliary RTX GPU, while verify_gpu() consumes the first nvidia-smi row. Locate the GB300 by PCI identity and apply driver/ECC qualification to that device; an auxiliary GPU should neither fail the host nor be mistaken for the GB300.

  4. CUDA keyring drift needs an exact trust boundary. dpkg -V plus finding the expected fingerprint does not prove that the keyring contains only the approved trust set or came from an approved package artifact. Please require an explicitly approved keyring package/hash or compare the complete fingerprint set.

Recommended shape after aligning with #7108: keep separate profiles such as generic-ubuntu-24.04-clean, shipping-ai-developer-tools-24.04, and stock-dgx-os. The shipping profile can preserve its qualified package tuple without making arbitrary drift acceptable for every generic Ubuntu host.

Please address or clarify these boundaries in the next revision.

@apurvvkumaria
apurvvkumaria requested a review from jyaunches July 17, 2026 20:41

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
scripts/prepare-dgx-station-host.sh (2)

875-903: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Enforce the documented missing-only APT transaction. The helper currently submits every package specification and rejects only removals, so retained state and dependencies remain mutable.

  • scripts/prepare-dgx-station-host.sh#L875-L903: construct an allowed delta from missing packages and the approved DKMS transition, then reject every other simulated change.
  • docs/get-started/prerequisites.mdx#L49-L53: retain the missing-only claim only after the helper enforces that boundary.
🤖 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 `@scripts/prepare-dgx-station-host.sh` around lines 875 - 903, Update
validate_package_availability, simulate_install, and install_packages in
scripts/prepare-dgx-station-host.sh to derive the allowed APT delta from
currently missing packages plus the explicitly approved DKMS transition, install
only that delta, and reject any other simulated package changes rather than
checking removals alone. In docs/get-started/prerequisites.mdx lines 49-53,
retain the missing-only claim after the helper enforces this boundary; no other
documentation changes are required.

781-791: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Reject extra CUDA signing keys. verify_key_fingerprint only checks that the expected fingerprint is present, so a same-version keyring with additional signing keys would still pass. Require the exact fingerprint set before trusting /usr/share/keyrings/cuda-archive-keyring.gpg.

🤖 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 `@scripts/prepare-dgx-station-host.sh` around lines 781 - 791, Update the CUDA
keyring validation around verify_key_fingerprint so it requires the keyring’s
complete fingerprint set to exactly match CUDA_KEY_FINGERPRINT, rejecting any
additional or missing signing keys. Preserve the existing file-safety and
expected-fingerprint checks while adding exact-set validation before the keyring
is trusted.
test/install-station-host-preparation.test.ts (1)

1241-1281: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Reset the module cache after restoring HOME.

onboard-session is imported while HOME points to the temporary directory, so its cached paths survive after that directory is removed. Add vi.resetModules() in the finally block after restoring the environment.

As per coding guidelines, deterministic tests must undo environment and global stubs and reset mock implementations when needed.

🤖 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/install-station-host-preparation.test.ts` around lines 1241 - 1281,
Update the finally cleanup in the test around the imported onboard-session
module to restore the environment and then call vi.resetModules() before the
temporary HOME directory is removed or before cleanup completes, ensuring cached
session paths do not leak into subsequent tests while preserving the existing
session and lock cleanup.

Source: Coding guidelines

🤖 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 `@docs/get-started/prerequisites.mdx`:
- Around line 49-53: Update the generic Ubuntu express-install documentation to
claim missing-only installation only after the preparation helper enforces that
behavior: it must install only absent pinned packages and reject unrelated
dependency changes rather than simulating or applying the full package
specification set. Ensure the documented behavior matches the helper’s actual
APT transaction semantics.

In `@scripts/prepare-dgx-station-host.sh`:
- Around line 1186-1190: Update the driver validation near the existing
STATION_HOST_PROFILE check to enforce an explicit allowlist of reviewed DGX OS
release/driver tuples, rejecting unknown driver drift while preserving the
generic-ubuntu validation. In test/install-station-dgx-os.test.ts lines 321-345,
replace generic non-610 acceptance with coverage for approved tuples and
rejection of an arbitrary driver version.

In `@test/install-station-package-transaction.test.ts`:
- Around line 48-76: The test currently marks every package exact while
expecting the full tuple to be installed, so it does not exercise missing-only
installation. Update the existing fixture’s package_is_exact behavior to return
nonzero for missing packages, then add a mixed-state test where an exact
retained spec is excluded from availability, simulation, and install APT
commands while missing specs are still processed.

In `@test/install-station-platform-identity.test.ts`:
- Around line 114-137: Update the `it("selects the GB300 by PCI identity when an
auxiliary RTX is listed first")` fixture so the auxiliary BDF `00000000:02:00.0`
is also labeled `NVIDIA GB300` but reports bad ECC values, while the target
`00000000:01:00.0` retains valid ECC. Adjust the expected auxiliary output to
match the misleading name and bad-ECC handling, ensuring name-based selection
fails and only PCI identity selection passes.

---

Outside diff comments:
In `@scripts/prepare-dgx-station-host.sh`:
- Around line 875-903: Update validate_package_availability, simulate_install,
and install_packages in scripts/prepare-dgx-station-host.sh to derive the
allowed APT delta from currently missing packages plus the explicitly approved
DKMS transition, install only that delta, and reject any other simulated package
changes rather than checking removals alone. In
docs/get-started/prerequisites.mdx lines 49-53, retain the missing-only claim
after the helper enforces this boundary; no other documentation changes are
required.
- Around line 781-791: Update the CUDA keyring validation around
verify_key_fingerprint so it requires the keyring’s complete fingerprint set to
exactly match CUDA_KEY_FINGERPRINT, rejecting any additional or missing signing
keys. Preserve the existing file-safety and expected-fingerprint checks while
adding exact-set validation before the keyring is trusted.

In `@test/install-station-host-preparation.test.ts`:
- Around line 1241-1281: Update the finally cleanup in the test around the
imported onboard-session module to restore the environment and then call
vi.resetModules() before the temporary HOME directory is removed or before
cleanup completes, ensuring cached session paths do not leak into subsequent
tests while preserving the existing session and lock cleanup.
🪄 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: 7003fa48-3923-41ad-b89a-f5ffca5b4059

📥 Commits

Reviewing files that changed from the base of the PR and between 45719e0 and e4b768b.

📒 Files selected for processing (11)
  • docs/get-started/prerequisites.mdx
  • scripts/checks/vitest-project-overlap.ts
  • scripts/install.sh
  • scripts/prepare-dgx-station-host.sh
  • test/install-express-prompt.test.ts
  • test/install-station-dgx-os.test.ts
  • test/install-station-host-preparation.test.ts
  • test/install-station-package-transaction.test.ts
  • test/install-station-platform-identity.test.ts
  • test/test-boundary-guards.test.ts
  • vitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • scripts/checks/vitest-project-overlap.ts
  • vitest.config.ts
  • test/test-boundary-guards.test.ts

Comment thread docs/get-started/prerequisites.mdx Outdated
Comment thread scripts/prepare-dgx-station-host.sh Outdated
Comment thread test/install-station-package-transaction.test.ts Outdated
Comment thread test/install-station-platform-identity.test.ts Outdated
apurvvkumaria and others added 2 commits July 17, 2026 17:25
Co-authored-by: San Dang <sdang@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: San Dang <sdang@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@test/install-station-package-transaction.test.ts`:
- Line 404: Update the test around the EXIT trap to invoke the production
cleanup-registration or entry-path symbol instead of installing its own trap
directly. Ensure the test still exits through the real production registration
path, so it fails if failure-path cleanup registration is removed.
- Line 384: Update the generated guard verification in the install transaction
test to execute $HOME/generated-guard/verify-plan directly instead of invoking
it through bash. Preserve the existing $APT_PLAN input and assertions so the
test verifies the generated hook’s executable permission as observable behavior.
🪄 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: 99b4ddf6-8859-42cd-82c2-d14a6aaee335

📥 Commits

Reviewing files that changed from the base of the PR and between e4b768b and 4d775b3.

📒 Files selected for processing (3)
  • scripts/prepare-dgx-station-host.sh
  • test/install-station-package-transaction.test.ts
  • test/install-station-platform-identity.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/install-station-platform-identity.test.ts

Comment thread test/install-station-package-transaction.test.ts Outdated
Comment thread test/install-station-package-transaction.test.ts Outdated
apurvvkumaria and others added 2 commits July 17, 2026 19:05
Co-authored-by: San Dang <sdang@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: San Dang <sdang@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

@cv cv 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.

Requesting changes on 10b8a5927. The current branch conflicts with main, and two regression tests bypass the exact privileged behavior they claim to protect: the generated APT pre-install hook is invoked with bash, so the test does not prove the production hook is executable, and the EXIT-cleanup test installs its own trap instead of exercising the production registration path at main. Please resolve the Station-script conflicts against current main, execute the generated hook directly, and drive cleanup through the production entry/registration function so removal of either chmod or trap wiring fails the suite. The exact package tuple, repository-key validation, PCI identity selection, and fail-closed transaction design otherwise align with the accepted host-profile boundary.

cv added 2 commits July 18, 2026 12:32

@cv cv 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.

Approved. Security review: PASS. The Station prerequisite transaction uses fixed package sources and commands, retains valid prerequisites, and fails with bounded rollback/diagnostics without consuming or logging credentials. The full focused Station suite passes; the authorized E2E run remains a merge gate.

@senthilr-nv

senthilr-nv commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Reviewed current head e2cc24a.

The direction remains aligned with the Station golden path, but this is now a high-blast-radius host-preparation change rather than a narrow version-drift fix. It adds a privileged APT pre-install transaction guard, Docker repository compatibility handling, PCI-based GB300 qualification, and installer-output filtering, with 371 additions in scripts/prepare-dgx-station-host.sh.

Since today's Station release is focused on already physically qualified fixes, I recommend moving this PR from v0.0.88 to v0.0.89 and holding merge for a dedicated validation window. Before merge, please run the exact head on a clean generic Ubuntu 24.04 Station through package installation, reboot/resume, runtime verification, and an idempotent no-op rerun. The evidence should also confirm preserved Docker/container state, bounded failure cleanup, and mixed-GPU GB300 selection if that logic remains in scope.

This is not a rejection of the design; it is a request to match the qualification depth to the size and privilege of the change.

@apurvvkumaria
apurvvkumaria merged commit 4135b46 into main Jul 18, 2026
53 checks passed
@apurvvkumaria
apurvvkumaria deleted the fix/station-version-drift-warning branch July 18, 2026 23:35
ericksoa added a commit that referenced this pull request Jul 19, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Fresh generic Ubuntu Station host preparation no longer exits after APT
simulation. The installer now uses the existing Docker
restart-quiescence gate and invokes its root-only APT pre-install guard
through `/bin/bash`, which works when `/run` is mounted `noexec`.

## Related Issue

Fixes #7182

## Changes

- Remove the undefined `check_no_workloads` invocation added by #7090.
- Keep `require_docker_restart_quiescence` as the single fail-closed
gate before the package transaction.
- Keep the root-owned APT guard and package manifest private
(`0700`/`0600`), invoke the hook through `/bin/bash` for a `noexec`
`/run` mount, and bind APT protocol Version 3 to that executable command
token.
- Remove test-only `check_no_workloads` definitions that masked the
production failure and assert the real restart-quiescence call instead.
- Assert the generated guard command and private hook/manifest modes.

## 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

- [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: this restores the documented
Station preparation flow without changing commands, prompts,
prerequisites, defaults, recovery, or the Docker quiescence contract;
independent documentation review found no update necessary.
- [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: maintainer review
approved exact head `4ea0f4b59143000d69e6446d330f14d8c1981766` at
#7183 (review);
the APT Version=3 key, root-private `noexec`-safe hook command,
fail-closed restart-quiescence gate, and pre-install ordering assertion
were reviewed.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [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 — `npx vitest run --project
installer-integration test/install-station-package-transaction.test.ts
test/install-station-host-preparation.test.ts --testTimeout=15000` (62
passed)
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable: three-line production
correction with focused installer coverage; normal hooks passed.
- [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)

---
Signed-off-by: Senthil Ravichandran
<senthilr@nvidia.com>\nSigned-off-by: Aaron Erickson
<aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved DGX Station host/package preparation by removing the workload
recheck gate and enforcing Docker restart quiescence before installing
pinned prerequisite packages.
* Updated APT transaction guard behavior to consistently run plan
verification via an explicit `/bin/bash` wrapper.
* **Tests**
* Updated Station host/package transaction test scenarios and assertions
to match the new `/bin/bash`-prefixed guard hook behavior and adjusted
related permission/cleanup expectations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa mentioned this pull request Jul 20, 2026
21 tasks
ericksoa added a commit that referenced this pull request Jul 20, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical dated changelog entry for NemoClaw `v0.0.89` before
the release plan captures the tag commit.
The entry summarizes the user-visible Station preparation, inference,
recovery, policy-disclosure, and CLI-containment changes merged since
`v0.0.88`.

## Changes

- Add `docs/changelog/2026-07-20.mdx` with the exact `## v0.0.89`
release heading, parser-safe SPDX comment, summary, and detailed
bullets.
- Link each shipped theme to the most specific published OpenClaw
documentation routes.
- Keep internal E2E, advisory-registry, and review-workflow refactors
out of the user-facing release summary.

Source summary:

- #7214, #7241, #7237, #7223, #7204, #7202, #7183, and #7090 ->
`docs/changelog/2026-07-20.mdx`: Summarize qualified DGX Station
identity, package-state, PackageKit, DKMS, and reboot-handoff fixes.
- #7242, #7221, #7186, #7164, and #6874 ->
`docs/changelog/2026-07-20.mdx`: Summarize inference endpoint
provenance, provider attachment, managed vLLM cleanup and progress, and
Ollama selection guidance.
- #7225, #7216, #7192, #7136, #7096, and #6910 ->
`docs/changelog/2026-07-20.mdx`: Summarize sandbox readiness, recovery
guidance, rebuilt model routing, durable Hermes state, and gateway
restart behavior.
- #7187 -> `docs/changelog/2026-07-20.mdx`: Summarize complete effective
messaging-preset egress disclosure before policy mutation.
- #7218, #7165, and #7184 -> `docs/changelog/2026-07-20.mdx`: Summarize
structured output containment, terminal-safe route display, and Hermes
forward cleanup.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: This PR adds release-history
prose only; the focused changelog contract test validates its required
structure and routes.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [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 — `mise exec node@22.23.1 -- npx vitest
run test/changelog-docs.test.ts` (6 passed).
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable to this doc-only release
entry.
- [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) — `mise
exec node@22.23.1 -- npm run docs` completed with 0 errors and 2
existing site-wide warnings.
- [x] 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)
— the native changelog entry uses the required parser-safe MDX SPDX
comment and does not use frontmatter.

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Expanded DGX Station installation support for qualified GB300 and
OTA-upgraded environments.
* Preserved selected inference providers, endpoints, model pins, and
tuning settings during sandbox creation and rebuilds.
* Improved sandbox recovery by validating availability and stability
before restarting services.
* Added clearer policy output showing the complete effective messaging
egress configuration.

* **Bug Fixes**
* Hardened status and inference route displays by sanitizing terminal
control characters.
* Improved Hermes uninstall behavior by stopping detached dashboard
forwarding.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation, examples, guides, or docs build area: install Install, setup, prerequisites, or uninstall flow bug-fix PR fixes a bug or regression platform: dgx-station Affects DGX Station hardware or workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants