feat(cli): connect setup to remote platforms - #835
Merged
tylersbray merged 8 commits intoJul 23, 2026
Conversation
Contributor
|
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:
📝 WalkthroughWalkthrough
ChangesRemote setup and authentication
Sequence Diagram(s)sequenceDiagram
participant User
participant setup_command
participant Platform
participant Config
participant OIDC
User->>setup_command: Select remote connection
setup_command->>Platform: Probe remote reachability
setup_command->>Config: Save base URL and workspace
setup_command->>OIDC: Ensure platform authentication
OIDC->>Platform: Request tokens
OIDC->>Config: Persist credentials
setup_command->>Platform: Continue setup with auth headers
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
![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: 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 `@docs/cli/reference.mdx`:
- Around line 45-50: Update the source CLI help text that produces the affected
“Set up NeMo Platform” documentation instead of editing docs/cli/reference.mdx
directly, then regenerate the reference with make generate-cli-reference-docs so
the generated file reflects the change.
🪄 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: 66858791-2e66-44b1-828f-0cc3f34e2582
⛔ Files ignored due to path filters (3)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/manifest_registry.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_agent.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/test_app.pyis excluded by!sdk/**
📒 Files selected for processing (4)
docs/cli/reference.mdxpackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.pypackages/nemo_platform_ext/tests/cli/commands/test_agent.pypackages/nemo_platform_ext/tests/cli/test_app.py
Let collaborative users configure the local CLI against an existing deployment while preserving the default local setup path. Reuse the established OIDC flow and persist the selected deployment in the active context. AIRCORE-934 Signed-off-by: Tyler Bray <tbray@nvidia.com>
Keep TopLevelEntry.help aligned with setup_command so test_manifest_help_matches_loaded_manual_entry passes after the remote-connect wording change. AIRCORE-934 Signed-off-by: Tyler Bray <tbray@nvidia.com>
tylersbray
force-pushed
the
aircore-934-nemo-cli-setup-remote-configuration/tbray
branch
from
July 22, 2026 01:29
78ace30 to
4169615
Compare
Contributor
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 `@packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/setup.py`:
- Around line 1786-1789: Update the connect_remote branch in the setup command
to determine whether the workspace parameter was explicitly provided via
ctx.get_parameter_source("workspace"). Reuse the currently configured workspace
when the flag was not explicit, and pass the explicit workspace value only when
supplied to _configure_remote_connection, preserving the active workspace during
reconnects.
🪄 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: 547b5d67-d964-419d-8450-647244eebafb
⛔ Files ignored due to path filters (7)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/manifest_registry.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/setup.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_agent.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_setup.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_setup_cli.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/test_app.pyis excluded by!sdk/**
📒 Files selected for processing (10)
docs/cli/configuration.mdxdocs/cli/reference.mdxdocs/get-started/setup.mdxpackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/setup.pypackages/nemo_platform_ext/tests/cli/commands/test_agent.pypackages/nemo_platform_ext/tests/cli/commands/test_setup.pypackages/nemo_platform_ext/tests/cli/commands/test_setup_cli.pypackages/nemo_platform_ext/tests/cli/test_app.py
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/cli/configuration.mdx
- packages/nemo_platform_ext/tests/cli/commands/test_agent.py
- packages/nemo_platform_ext/tests/cli/test_app.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.py
- docs/cli/reference.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
Only overwrite the configured workspace during remote setup when --workspace was passed explicitly. AIRCORE-934 Signed-off-by: Tyler Bray <tbray@nvidia.com>
Signed-off-by: Tyler Bray <tbray@nvidia.com>
Contributor
There was a problem hiding this comment.
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 (1)
packages/nemo_platform_ext/tests/cli/commands/test_setup.py (1)
2564-2573: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert remote setup ordering.
This test verifies that configuration, authentication, and interactive setup each run, but not their order. It would pass if setup continued before authentication. Record the calls or attach the mocks to one parent and assert configure → authenticate → run.
Also applies to: 2577-2606
🤖 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 `@packages/nemo_platform_ext/tests/cli/commands/test_setup.py` around lines 2564 - 2573, Update test_cancelling_initial_connection_prompt_exits_cleanly and the related setup tests to track the configuration, authentication, and interactive setup calls through a shared parent mock or call recorder. Assert they execute in the required configure → authenticate → run order while preserving the existing cancellation and exit-code assertions.
🧹 Nitpick comments (1)
packages/nemo_platform_ext/tests/config/test_config.py (1)
870-889: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCover file-level chmod failure.
This test exercises only parent-directory failure; it would pass even if config-file chmod were silently skipped. Add a case where chmod fails for the target file and assert the intended secure behavior.
🤖 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 `@packages/nemo_platform_ext/tests/config/test_config.py` around lines 870 - 889, Extend test_save_tolerates_unowned_parent_directory to make os.chmod fail when called for config_path, not only tmp_path, and assert the intended secure behavior for the written configuration file. Keep the existing parent-directory scenario intact while ensuring the test verifies file-level chmod failures are handled correctly.
🤖 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 `@packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/setup.py`:
- Around line 448-450: Update the setup health-reporting flow around
_verify_platform_health so the detail "Hosted deployment does not publish
/status." is treated as informational rather than a health failure. Keep the
existing warning output for genuine health verification failures, while
preserving successful hosted setup behavior.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py`:
- Around line 33-38: Credential-file permission failures must remain fatal while
parent-directory permission failures remain tolerated. In
packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py lines 33-38,
update _secure_chmod to suppress PermissionError only for directory chmod
operations; retain tolerant parent-directory handling at line 277, but enforce
secure 600 file creation/replacement and abort before writing when that cannot
be guaranteed at line 290. Apply the same separation in
packages/nemo_platform_ext/src/nemo_platform_ext/quickstart/config.py lines
22-27, retaining parent-directory tolerance at line 240 and enforcing secure
file creation/replacement at line 262.
In `@packages/nemo_platform_ext/tests/cli/commands/test_setup.py`:
- Around line 167-176: Update test_reachable_via_status and the /cluster-info
fallback test to patch client_verify_from_env() with a known TLS verification
value, then assert that exact value in each httpx.get call. Remove the
tautological mock_get.call_args.kwargs["verify"] assertion while preserving the
existing endpoint and timeout checks.
- Around line 2744-2751: Update _check_controller_health so a 404 from /status
is considered healthy only when the URL is verified as a supported
hosted-platform endpoint, preserving the existing hosted deployment test. Add a
negative test using a local or unknown host to assert that its 404 result is
unhealthy, and ensure reachability through another endpoint cannot make that
case pass.
---
Outside diff comments:
In `@packages/nemo_platform_ext/tests/cli/commands/test_setup.py`:
- Around line 2564-2573: Update
test_cancelling_initial_connection_prompt_exits_cleanly and the related setup
tests to track the configuration, authentication, and interactive setup calls
through a shared parent mock or call recorder. Assert they execute in the
required configure → authenticate → run order while preserving the existing
cancellation and exit-code assertions.
---
Nitpick comments:
In `@packages/nemo_platform_ext/tests/config/test_config.py`:
- Around line 870-889: Extend test_save_tolerates_unowned_parent_directory to
make os.chmod fail when called for config_path, not only tmp_path, and assert
the intended secure behavior for the written configuration file. Keep the
existing parent-directory scenario intact while ensuring the test verifies
file-level chmod failures are handled correctly.
🪄 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: d30eb464-3bf1-4821-8725-22d597554998
⛔ Files ignored due to path filters (2)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/setup.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**
📒 Files selected for processing (5)
packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/setup.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/src/nemo_platform_ext/quickstart/config.pypackages/nemo_platform_ext/tests/cli/commands/test_setup.pypackages/nemo_platform_ext/tests/config/test_config.py
rrhyne
approved these changes
Jul 22, 2026
rrhyne
left a comment
rrhyne
left a comment
Contributor
There was a problem hiding this comment.
Tested end to end and it works well for me both locally and remotely.
Address CodeRabbit review and lint-sdk-vendored drift: gate /status 404 on /cluster-info evidence, keep parent-dir chmod tolerant while enforcing 600 credential files, and re-vendor the SDK copy from source. Signed-off-by: Tyler Bray <tbray@nvidia.com>
…934) When setup finds a reachable remote URL, offer continue, start local, or connect to a different host instead of silently continuing. Document URL override escape hatches in help and prefill the remote URL prompt. Signed-off-by: Tyler Bray <tbray@nvidia.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
mckornfield
approved these changes
Jul 23, 2026
- Fix CodeQL "incomplete URL substring sanitization" by asserting the full prompt message instead of a hostname substring check. - Use a StrEnum for the reachable-remote connection choices. - Reword the secure-config-write docstring. - Re-vendor SDK copies so lint-sdk-vendored/lint-cli stay in sync. Signed-off-by: Tyler Bray <tbray@nvidia.com>
Collapse the repeated typer Context / CLIContext and with-patch stacks in TestNonTtyEarlyExit and TestSetupCommandRemoteFlow into shared helpers. Signed-off-by: Tyler Bray <tbray@nvidia.com>
tylersbray
deleted the
aircore-934-nemo-cli-setup-remote-configuration/tbray
branch
July 23, 2026 20:07
AnuradhaKaruppiah
pushed a commit
to AnuradhaKaruppiah/nemo-platform
that referenced
this pull request
Jul 24, 2026
* feat(cli): connect setup to remote platforms Let collaborative users configure the local CLI against an existing deployment while preserving the default local setup path. Reuse the established OIDC flow and persist the selected deployment in the active context. AIRCORE-934 Signed-off-by: Tyler Bray <tbray@nvidia.com> * fix(cli): sync setup help text in manifest registry Keep TopLevelEntry.help aligned with setup_command so test_manifest_help_matches_loaded_manual_entry passes after the remote-connect wording change. AIRCORE-934 Signed-off-by: Tyler Bray <tbray@nvidia.com> * fix(cli): preserve active workspace on remote reconnect Only overwrite the configured workspace during remote setup when --workspace was passed explicitly. AIRCORE-934 Signed-off-by: Tyler Bray <tbray@nvidia.com> * fix(cli): support hosted setup endpoints (NVIDIA-NeMo#934) Signed-off-by: Tyler Bray <tbray@nvidia.com> * fix(cli): harden hosted setup probes and config file perms Address CodeRabbit review and lint-sdk-vendored drift: gate /status 404 on /cluster-info evidence, keep parent-dir chmod tolerant while enforcing 600 credential files, and re-vendor the SDK copy from source. Signed-off-by: Tyler Bray <tbray@nvidia.com> * feat(cli): re-prompt when remote platform is reachable during setup (NVIDIA-NeMo#934) When setup finds a reachable remote URL, offer continue, start local, or connect to a different host instead of silently continuing. Document URL override escape hatches in help and prefill the remote URL prompt. Signed-off-by: Tyler Bray <tbray@nvidia.com> * fix(cli): address setup review nits and CodeQL alert (NVIDIA-NeMo#934) - Fix CodeQL "incomplete URL substring sanitization" by asserting the full prompt message instead of a hostname substring check. - Use a StrEnum for the reachable-remote connection choices. - Reword the secure-config-write docstring. - Re-vendor SDK copies so lint-sdk-vendored/lint-cli stay in sync. Signed-off-by: Tyler Bray <tbray@nvidia.com> * refactor(cli): extract setup_command test helpers (NVIDIA-NeMo#934) Collapse the repeated typer Context / CLIContext and with-patch stacks in TestNonTtyEarlyExit and TestSetupCommandRemoteFlow into shared helpers. Signed-off-by: Tyler Bray <tbray@nvidia.com> --------- Signed-off-by: Tyler Bray <tbray@nvidia.com>
This was referenced Jul 28, 2026
Merged
This was referenced Aug 4, 2026
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
4 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
CliRunnercoverage for local, remote, and manual-start variantsLinear: https://linear.app/nvidia/issue/AIRCORE-934/nemo-cli-setup-command-should-offer-remote-configuration
Test plan
uv run --frozen pytest packages/nemo_platform_ext/tests/cli/commands/test_setup.py packages/nemo_platform_ext/tests/cli/commands/test_setup_cli.py packages/nemo_platform_ext/tests/cli/commands/test_auth.py -q(229 passed)uv run --frozen pytest sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_setup.py sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_setup_cli.py -q(201 passed)ty checkfor modified setup/auth modulesSummary by CodeRabbit
nemo setupcan now connect to an existing remote Platform when local services are unreachable.--autowithNMP_BASE_URL/NMP_ACCESS_TOKENand--no-start-services.