Skip to content

fix(cli): warn on incomplete inference route - #9473

Merged
prekshivyas merged 1 commit into
mainfrom
codex/fix-doctor-route-9435
Aug 18, 2026
Merged

fix(cli): warn on incomplete inference route#9473
prekshivyas merged 1 commit into
mainfrom
codex/fix-doctor-route-9435

Conversation

@rsliter

@rsliter rsliter commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

sandbox doctor previously reported the inference route as ok when only the provider or model was known. It now requires both fields and preserves the existing warning and recovery hint for either half-resolved state.

Related Issue

Fixes #9435

Changes

  • Require both provider and model before the doctor Route check reports ok.
  • Cover complete, provider-only, model-only, and fully unknown routes with exact status, detail, and hint assertions.

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: Existing docs already define inference routes through provider and model and explain that doctor warnings do not fail the command.
  • 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: fix(cli): warn on incomplete inference route #9473 (review)
  • Non-success, skipped, or missing CI check accepted by maintainer; check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: The change corrects an undocumented half-route classification. The sandbox doctor command documentation already describes the supported route and warning behavior.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

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 validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above; command/result or justification: npx vitest run --project cli src/lib/actions/sandbox/doctor-inference.test.ts passed 22 tests at fc7faffb8. Both half-resolved cases failed before the fix.
  • Applicable broad gate passed: npm test for broad runtime or shared test-harness changes; npm run check for repository-wide validation or coverage changes. Command/result: not applicable; the two-file change tightens one pure diagnostic classification and adds its focused boundary tests.
  • 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)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Rebecca Sliter 571084+rsliter@users.noreply.github.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved inference route health reporting by requiring both the provider and model to be recognized.
    • Unrecognized or partially identified routes now show a warning with guidance to check gateway health.
    • Added coverage for recognized, unknown, and mixed provider/model combinations.

Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
@rsliter rsliter self-assigned this Aug 18, 2026
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b0feb67b-c908-49da-b0b3-8cc86928bfa8

📥 Commits

Reviewing files that changed from the base of the PR and between 9ab3cd3 and fc7faff.

📒 Files selected for processing (2)
  • src/lib/actions/sandbox/doctor-inference.test.ts
  • src/lib/actions/sandbox/doctor-inference.ts

Included review availability: Your plan includes up to 12 reviews per rolling hour; 10 remain after this review.


📝 Walkthrough

Walkthrough

The inference route check now reports ok only when both provider and model are known. Parameterized tests cover complete, partial, and unknown routes, including recovery hints.

Changes

Inference route status

Layer / File(s) Summary
Require complete inference routes and add regression coverage
src/lib/actions/sandbox/doctor-inference.ts, src/lib/actions/sandbox/doctor-inference.test.ts
The route check requires both provider and model to be known. Parameterized tests verify statuses, details, and recovery hints for recognized and unrecognized combinations.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to fc7fa

The change correctly requires both the provider and model before reporting a healthy inference route, while preserving warnings for incomplete routes; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: ericksoa

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change requires both provider and model for an ok route and adds regression coverage for all four route states [#9435].
Out of Scope Changes check ✅ Passed All changes directly support the linked issue by fixing route classification and adding focused regression tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: warning when the inference route is incomplete.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-doctor-route-9435

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

@github-code-quality

github-code-quality Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit fc7faff in the codex/fix-doctor-rou... branch remains at 96%, unchanged from commit 9ab3cd3 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit fc7faff in the codex/fix-doctor-rou... branch remains at 83%, unchanged from commit 9ab3cd3 in the main branch.

Show a code coverage summary of the most impacted files.
File main 9ab3cd3 codex/fix-doctor-rou... fc7faff +/-
src/lib/securit...ntial-filter.ts 96% 84% -12%
src/lib/onboard...der/snapshot.ts 83% 75% -8%
src/lib/onboard...press-resume.ts 82% 79% -3%
src/lib/state/o...oard-session.ts 87% 88% +1%
src/lib/state/registry.ts 90% 92% +2%
src/lib/state/o...d-checkpoint.ts 87% 91% +4%
src/lib/trace.ts 90% 94% +4%
src/lib/cua/run...ime-manifest.ts 84% 91% +7%
src/lib/cua/contract.ts 80% 87% +7%
src/lib/cua/bounded-file.ts 84% 94% +10%

Updated August 18, 2026 14:15 UTC

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

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 · 0 warnings · 1 suggestion
  • Model comparison: normalized findings differ; normalized terminology decisions match; normalized E2E selections match; Nemotron reported the same number of blockers, the same number of warnings, 1 more suggestion.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: inference-routing

Manual-only E2E: onboard-repair, onboard-resume, network-policy
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

Workflow run details

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

@rsliter rsliter left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Security review: PASS at fc7faffb8eeb66c05aadab2d51df0eddab1eb487 against base 9ab3cd3a569e6d01aef26dee876461d281aef06a.

I reviewed authentication and authorization, command construction, credential and log handling, filesystem and network behavior, dependencies, privileges, concurrency, and failure behavior.

The production change only tightens a pure diagnostic classification from either route field being known to both fields being known. It does not add a probe, mutate route state, read credentials, change policy, touch the filesystem, or perform an external write. The four boundary states have exact status, detail, and recovery-hint coverage.

All substantive repository gates pass at the reviewed commit. The remaining failed Nemotron advisor job reports that its analysis did not complete and contains no code finding.

@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression labels Aug 18, 2026

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

Reviewed at fc7faff. No actionable findings. Focused validation passed: 22 tests.

@prekshivyas
prekshivyas merged commit 0db083c into main Aug 18, 2026
91 of 92 checks passed
@prekshivyas
prekshivyas deleted the codex/fix-doctor-route-9435 branch August 18, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sandbox doctor reports [ok] for a half-resolved inference route and suppresses its own recovery hint

3 participants