fix(cli): forward harness args after --, honor saved provider, bypass proxies on loopback probes - #107
Conversation
WalkthroughChangesConfigure defaults
Loopback health checks
CLI robustness
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@switchyard/cli/configure_command.py`:
- Around line 425-437: Restrict the environment key and base-URL candidates
passed by the configure flow around resolve_provider_connectivity() to the
selected provider, so provider nvidia cannot consume an OPENROUTER_API_KEY (and
vice versa). Preserve provider/key consistency before assigning
prompt_default_api_key, and add a regression test covering mixed environment
variables with --provider nvidia.
In `@switchyard/cli/switchyard_cli.py`:
- Around line 1463-1470: Update the argument scanning around _SUBCOMMANDS,
sep_idx, and cmd_idx to skip values consumed by global options such as
--routing-profiles before identifying the real subcommand. Ensure a value equal
to a subcommand name is not treated as the command, then remove only a "--"
separator occurring before that resolved subcommand while preserving separators
after it.
In `@tests/test_launcher_proxy_bypass.py`:
- Around line 30-33: Update the proxy setup in the affected test to use a
test-owned proxy stub or dynamically allocated local port instead of hard-coded
127.0.0.1:9. Ensure the endpoint reliably returns a connection error for the
negative proxy case, and apply the dynamically selected endpoint to both
HTTP_PROXY and http_proxy.
In `@tests/test_user_config.py`:
- Around line 346-358: Update the test setup before saving the user
configuration by calling _clear_provider_env_vars(monkeypatch). Place it after
setting SWITCHYARD_CONFIG_DIR and before save_user_config so
resolve_provider_connectivity() cannot use ambient provider credentials instead
of the saved Nvidia credentials.
🪄 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: 8cc12026-f435-4d2f-8989-8c77ebe7ef06
📒 Files selected for processing (9)
switchyard/cli/configure_command.pyswitchyard/cli/launchers/launcher_runtime.pyswitchyard/cli/launchers/proxy_health_monitor.pyswitchyard/cli/switchyard_cli.pyswitchyard/server/verify.pytests/test_launch_claude.pytests/test_launcher_proxy_bypass.pytests/test_serve_profile_config.pytests/test_user_config.py
2e74ffb to
21551d1
Compare
Live before/after — CLI behaviorThese are CLI-behavior fixes, so the reproduction is command output rather than token usage. Ran the same commands on 1) 2) 3) Harness args after a single The doubled- The saved-provider, non-interactive env-key, and Audit follow-up in this push: |
21551d1 to
783977a
Compare
Reworked onto the new main (post-#98) — live before/after#98 landed and rewrote the 1) 2) Harness args after a single The saved-provider, non-interactive env-key, and |
3dce996 to
e57e5e0
Compare
e57e5e0 to
2323193
Compare
dca3cb1 to
74e7779
Compare
…, clarify serve errors Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com> (cherry picked from commit 967f72b)
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com> (cherry picked from commit aad16ca)
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
…ults Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
74e7779 to
4dacf4e
Compare
CLI bug fixes for
launch,configure, and the loopback health probes. No serve or routing behavior changes.Rebased onto current
main. The earlier serve-error wording change is dropped:mainremoved the--configserve flag when it deleted components-v2 (#119), soservekeeps its existing--routing-profilesmessage.launch <agent> -- <args>forwards harness args after a single--.Before,
main()stripped the first--anywhere in the command, solaunch claude -- --versionmade argparse reject--versionand exit 2. Now only a--that comes before the subcommand token is stripped; a--after it survives intoargparse.REMAINDERand reaches the harness. Same fix coverscodexandopenclaw.configureresolves the API key from the environment or the routing-profiles bundle, and honors the saveddefault_provider.--providerno longer defaults toopenrouterat the argparse level (the default is nowNone), soconfigureandconfigure --list-modelsstop overwriting a saveddefault_provider. When you pass no--api-keyand none is saved, non-interactiveconfigureresolves a key in this order:<PROVIDER>_API_KEY(orsecrets.json), then--routing-profilesbundle'sdefaults.api_key, with${VAR}expanded from the environment the same wayserve/launchexpand it.The env lookup is scoped to the chosen provider, so
configure --provider nvidiacan't save anOPENROUTER_API_KEYundernvidia. An unset${VAR}in the bundle stays unresolved, soconfigurestill fails closed and asks for--api-keyrather than saving a literal${VAR}. This makes the documented non-interactive flow (switchyard --routing-profiles routes.yaml -- configure --target provider --no-tui) work when the bundle carriesdefaults.api_key.Loopback health probes bypass env proxies.
wait_for_proxy_ready,ProxyHealthMonitor, andverify's round-trip send their127.0.0.1requests through an opener/client that ignoresHTTP_PROXY/HTTPS_PROXY, so a configured proxy no longer breaks the local readiness and health checks. The upstream backend probe still honors env proxies.Unchanged:
servewith--routing-profilesroute bundles, and every non-loopback request.Regression tests cover each behavior:
tests/test_launch_claude.py,tests/test_user_config.py,tests/test_launcher_proxy_bypass.py.