refactor(cli): route credentials through OpenShell provider adapter - #10149
Conversation
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
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:
📝 WalkthroughWalkthroughCredential actions now use an injectable, gateway-scoped OpenShell provider adapter. The CLI adapter adds structured errors, profile handling, secret redaction, and sandbox-detachment recovery. Provider classification, diagnostics, tests, and reset guidance were updated. ChangesProvider adapter credential lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🔵 Low · up to The refactor routes credential operations through a typed provider adapter while preserving public behavior, but profile import/export and endpointless reconciliation may misplace the gateway selector and fail for named gateways; the PR is mergeable with explicit owner awareness and follow-up validation. Sequence Diagram(s)sequenceDiagram
participant CredentialsAction
participant OpenShellProviderAdapter
participant OpenShellCLI
CredentialsAction->>OpenShellProviderAdapter: provider operation with gateway and timeout
OpenShellProviderAdapter->>OpenShellCLI: execute gateway-scoped command
OpenShellCLI-->>OpenShellProviderAdapter: captured output or command failure
OpenShellProviderAdapter-->>CredentialsAction: typed result or redacted structured error
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
I reviewed commit 7418fa2. I found no concrete code defect in the provider-adapter and credential-action slice. CodeRabbit reports no actionable findings, and GitHub reports no review threads. The commit is Verified, the PR DCO check passes, and the PR body contains the contributor sign-off.
I cannot approve this draft under the Product Scope Gate. Issue #9806 still labels this work as draft scope, has no acceptance decision, and the PR warns that scope is pending. A maintainer must accept the provider-adapter capability slice and define its ownership, lifecycle, compatibility, security, and validation expectations before product approval. This is a scope decision, not a code-defect finding.
The exact-commit Advisor jobs were skipped. CodeQL, checks, commit lint, DCO, and CodeRabbit pass. Several earlier specialist jobs and the code-growth check failed, and the latest full test jobs were skipped. I did not request changes for those gate results. Local targeted tests could not start because this detached worktree has no installed Vitest dependencies. The PR body records prior focused validation: 39 targeted tests, CLI build and type-check, and repository checks.
Disposition: neutral pending the maintainer scope decision and complete required-check evidence.
|
@coderabbitai review |
|
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
I re-reviewed exact head 159095ca97194a925243a3e647ca3028f39a832b against base 101246c10c7a9811270e2d6d80424da4f80d858f. I found no concrete correctness, security, or test defect in the complete 10-file provider-adapter diff. The new merge commit only refreshes the dependency branch and relocates equivalent doctor-observation tests; it does not change this PR's production diff. No mechanical repair was warranted, so I made no commit or push.
Product Scope remains unaccepted. Issue #9806 still says the work is pending scoping, calls its contents “Draft scope,” and has no acceptance comment. This PR remains a draft and repeats the pending-scope warning. The required ownership, lifecycle, compatibility, security, and validation decision for the provider-adapter product surface is therefore still missing. I cannot approve under the Product Scope Gate. This is a product decision, not a code-defect finding, so I am not requesting changes.
Exact-head evidence: both PR commits are GitHub Verified; the PR body contains Rebecca Sliter's Signed-off-by declaration and dco-check passes; CodeRabbit reports no actionable findings; GitHub reports 0 review threads. The Advisor specialist jobs failed and synthesis/publish were skipped, so there is no accepted exact-head Advisor conclusion. checks, commit lint, hash checks, ShellCheck, Python CodeQL, and the aggregate CodeQL check pass; JavaScript/TypeScript CodeQL is still in progress, while codebase-growth and specialist checks are failing and the full CI jobs are skipped. The PR body records passing CLI build/typecheck, 39 focused tests, and repository checks. I did not treat gate state alone as a code defect.
Disposition: neutral pending accepted Product Scope and complete exact-head gate evidence.
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
wscurran
added
area: cli
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit 9e43b0d in the TypeScript / code-coverage/cliThe overall line coverage in commit 9e43b0d in the Show a line coverage summary of the most impacted files.
Updated |
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
I reviewed commit 9bbf1113101640b830027573316a753e2bb95a8d against base commit 5dbab5d614129dc71efe8d418a2e0fe7123334ce. I found no concrete correctness, security, or test defect in the complete 10-file provider-adapter diff. The latest merge commit refreshes the dependency branch and preserves this PR's production diff. No mechanical repair was warranted, so I made no commit or push.
The Product Scope Gate prevents approval. Issue #9806 still marks the work as “Draft scope,” contains no acceptance comment, and defers provider v2 product decisions outside accepted behavior. This PR also remains a draft. A maintainer must accept this provider-adapter capability slice and define ownership, lifecycle, compatibility, security, and validation expectations before product approval. This is a product decision, not a code defect, so I am not requesting changes.
Exact-commit evidence:
- All four PR commits are GitHub Verified.
- The PR body contains Rebecca Sliter's
Signed-off-bydeclaration, anddco-checkpasses. - CodeRabbit reports no actionable findings.
- GitHub reports 0 review threads, and the complete thread read is not truncated.
- The exact-commit PR Advisor synthesis and publish jobs were skipped. Earlier specialist jobs failed, so no accepted Advisor conclusion applies to this commit.
checks, commit lint, hash checks, ShellCheck, CodeQL, and CodeRabbit pass. Other full CI jobs are skipped, and codebase-growth and earlier specialist checks fail. I did not use those gate results as a reason for this disposition.- The PR body records passing CLI build and type-check, 57 focused CLI tests, 14 focused integration tests, and repository checks.
Disposition: neutral pending an accepted Product Scope decision.
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
I reassessed exact head 9bbf1113101640b830027573316a753e2bb95a8d after checks stopped. I found no new concrete code defect and made no code change. CodeRabbit has 0 actionable findings, and the complete GitHub thread read has 0 threads.
Approval remains unavailable because required evidence and the product decision are missing:
- All nine PR Advisor specialists failed before analysis because OpenShell inference configuration failed or the advisor credential was unavailable. Their session artifacts were absent; Synthesis and Publish advisor were skipped. There is no exact-head Advisor conclusion.
codebase-growth-guardrailsfailed while reading the PR files API with HTTP 403. This is infrastructure evidence loss, not a reported growth violation.- The full
build-typecheck,cli-tests,cli-test-shards,static-checks, andinstaller-integrationjobs were skipped. The PR body records passing focused evidence, but the stopped check set does not supply complete broad CI evidence or a maintainer waiver for the non-success checks. - My local focused rerun could not start because this isolated worktree has no installed Vitest dependencies (
vitest/configandvitest/nodeare unavailable). - Issue #9806 still labels the proposal
Draft scope, has no acceptance comment, and does not establish the ownership, lifecycle, compatibility, security, and validation decision required by the Product Scope Gate for this provider-adapter surface. The PR remains a draft.
Passing exact-head evidence includes checks, DCO, commit lint, hash checks, ShellCheck, CodeQL, and CodeRabbit. No review threads or automated-review findings remain. Because the remaining blockers are missing infrastructure/evidence and an unaccepted product decision rather than a concrete defect, I am not requesting changes. Disposition: neutral; do not approve until the scope decision is accepted and the missing exact-head evidence passes or receives a documented maintainer waiver.
|
Addressed the Design/Architecture and Trust findings from exact-head Advisor run 33521873430 in commit
Local validation: 152 focused tests across six files; exact credential-command replay 12/12 with a 60-second cold-load allowance; growth guard 33/33; CLI type-check; repository checks; docs build; commit hooks; and A fresh PR Advisor run on this exact head SHA is required before approval. |
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
…' into codex/openshell-provider-adapter
|
Candidate update for
Awaiting a fresh PR Advisor review and required CI for this exact head before dispatching new exact-SHA |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
Advisor remediation for
Awaiting a fresh PR Advisor run and required CI for this exact SHA before dispatching replacement |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
Advisor remediation for
Awaiting a fresh PR Advisor run and required CI for this exact SHA before dispatching replacement |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
@prekshivyas I reproduced the current CI failures locally at exact head |
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
|
PR Review Advisor finished for commit |
|
Final merge-candidate verification
No broad live E2E matrix was dispatched for this final candidate, per maintainer direction. |
5 participants
Summary
Credential actions previously called OpenShell provider commands and interpreted CLI output inside business logic. This PR adds a typed provider boundary backed by the existing CLI, moves credential add, list, and reset onto it, and closes the credential-custody and endpoint-validation gaps found during review.
Related Issue
Partial #9806
Changes
--from-existingprovider creation diagnostics because NemoClaw cannot redact unknown stored credential values.OPENAI_BASE_URLvalues on the credential-bearing provider path. Reject DNS hostnames because this path cannot enforce admission-time address pins, and route hostname or trusted-private endpoints through onboarding.Provider get, update, and attach operations remain deferred. Onboarding, messaging, managed-provider, general recovery and cleanup, and additional inference consumers also remain deferred. SDK and gRPC provider implementations are separate Phase 2 work.
Type of Change
Quality Gates
DGX Station Hardware Evidence
Verification
Signed-off-by:linenpm run build:clinpm run typecheck:clinpm run checks:repositorynpm run docs: 0 errors, 2 warningsgit diff --checkdocs-updatedc5d5ec202eagainstc67be3984f2. The snapshot clone, provider-profile recovery, and live credential-reset test titles now end with their owning issue references without changing behavior. The quickstart uses the provider reset argument correctly. The command reference accurately documents gateway-wide provider rebuild and reset behavior, public IP-literal configuration, DNS hostname rejection, and onboarding for endpoints that require address pins. Recovery diagnostics remain consistent with the tested allowlist, redaction, gateway scoping, uncertain-mutation reconciliation, and reset behavior.Signed-off-by: Rebecca Sliter 571084+rsliter@users.noreply.github.com
Summary by CodeRabbit