fix(installer): select portable Podman override before preflight - #8408
Merged
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThe installer adds a ChangesPortable profile runtime
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Installer
participant systemctl
participant Podman
participant DockerCLI
participant Onboarding
Installer->>systemctl: Start podman.socket
Installer->>Podman: Query Host.RemoteSocket.Path
Podman-->>Installer: Return rootless socket
Installer->>DockerCLI: Set DOCKER_HOST
Installer->>Onboarding: Pass --experimental-profile portable
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit f7a2644 in the TypeScript / code-coverage/cliThe overall coverage in commit f7a2644 in the Show a code coverage summary of the most impacted files.
Updated |
![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)
src/lib/onboard/experimental/portable-host-preparation.ts (1)
189-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard portable onboarding from the fixed socket fallback.
src/lib/runner.tscallsdetectDockerHost()at module load and can setDOCKER_HOSTto a fixed Podman socket beforepreparePortableExperimentalHostruns. Skip fixed-candidate detection forNEMOCLAW_EXPERIMENTAL_PROFILE=portableor defer it until after preparation. Add a public portable-entrypoint test.🤖 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 `@src/lib/onboard/experimental/portable-host-preparation.ts` around lines 189 - 200, Update detectDockerHost in src/lib/runner.ts to skip fixed-candidate detection when NEMOCLAW_EXPERIMENTAL_PROFILE is portable, allowing preparePortableExperimentalHost to resolve the socket via Podman. Preserve existing detection behavior for other profiles, and add a test through the public portable entrypoint verifying preparation is not overridden by the fixed fallback.Source: Path instructions
🤖 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 `@scripts/install.sh`:
- Around line 5011-5014: Update the empty branch of the EXPERIMENTAL_PROFILE
case to unset NEMOCLAW_EXPERIMENTAL_PROFILE, ensuring an explicitly empty
--experimental-profile value clears any inherited portable profile; preserve the
existing export and error behavior for portable and unknown values.
In `@test/install-portable-profile.test.ts`:
- Around line 29-32: Update runPortableOverride to pass a fixed sentinel
DOCKER_HOST value in the child environment instead of inheriting the parent
value, then assert that the no-profile path runPortableOverride("") preserves
that sentinel when no override is applied.
---
Nitpick comments:
In `@src/lib/onboard/experimental/portable-host-preparation.ts`:
- Around line 189-200: Update detectDockerHost in src/lib/runner.ts to skip
fixed-candidate detection when NEMOCLAW_EXPERIMENTAL_PROFILE is portable,
allowing preparePortableExperimentalHost to resolve the socket via Podman.
Preserve existing detection behavior for other profiles, and add a test through
the public portable entrypoint verifying preparation is not overridden by the
fixed fallback.
🪄 Autofix
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: 4b2540ee-f798-4707-8981-ce85384835d0
📒 Files selected for processing (7)
.github/workflows/portable-profile-e2e.yamlscripts/install.shsrc/lib/onboard/experimental/portable-host-preparation.test.tssrc/lib/onboard/experimental/portable-host-preparation.tstest/e2e/live/portable-profile-rootless-linux.test.tstest/install-onboard-yes.test.tstest/install-portable-profile.test.ts
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
1 terminology difference from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
1 warning · 0 suggestionsWarningsWarnings do not block.
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Author
|
Exact-head review note for |
23 tasks
ericksoa
added a commit
that referenced
this pull request
Aug 6, 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. --> Portable onboarding selected the Podman driver but wrote the Docker-only `supervisor_bin` field into `[openshell.drivers.podman]`. OpenShell 0.0.85 rejected that table before the gateway could start. This change omits `supervisor_bin` for Podman and preserves the existing Docker configuration. It follows the portable runtime override restored by #8408. ## 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. --> - Omit `supervisor_bin` from the generated Podman driver table. - Preserve `supervisor_bin` in the generated Docker driver table. - Install pinned OpenShell 0.0.85 in the portable-profile E2E workflow. - Start the real rootless Podman gateway and fail if OpenShell rejects the generated configuration or exits. - Trigger the portable-profile workflow when its gateway-proof or pinned-runtime helpers change. ## 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: The change repairs generated configuration for an existing hidden experimental profile. No supported public command, default, or workflow changed. - [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: Aaron Erickson reviewed the driver-schema boundary. The [exact-head rootless runner](https://github.com/NVIDIA/NemoClaw/actions/runs/31062172724) installs pinned OpenShell 0.0.85 and starts the real Podman gateway. - [ ] 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: Omits a Docker-only field from the generated configuration for an existing hidden experimental profile and adds pinned OpenShell E2E evidence. No supported public command, default, or workflow changed. - Agent: Codex Desktop <!-- docs-review-head-sha: 9403b99 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## 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 validate:pr` passed after refreshing `origin/main` 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: [E2E / Portable Profile](https://github.com/NVIDIA/NemoClaw/actions/runs/31062172724) passed on `91daf0836`; `npm run typecheck` and `npm run test:e2e-phases:check` passed locally. - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: [CI / Pull Request](https://github.com/NVIDIA/NemoClaw/actions/runs/31062172715) passed on `91daf0836`. - [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: Aaron Erickson <aerickson@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@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
1 participant
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
Portable installer runs were reaching ordinary Docker bootstrap and host preflight before the hidden profile could select rootless Podman. This change applies the same Podman socket override used by the working GFN setup before those gates, then forwards the hidden profile into onboarding without disabling GPU.
Related Issue
Follow-up to #8376.
Changes
Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailable76CLI tests and21installer integration tests passed;npm run typecheckand semantic E2E phase validation passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests