Skip to content

refactor(openshell): remove skill SSH transport - #6796

Merged
apurvvkumaria merged 25 commits into
feat/openshell-grpc-stdin/aefrom
refactor/openshell-grpc-skill-control/ae
Jul 20, 2026
Merged

refactor(openshell): remove skill SSH transport#6796
apurvvkumaria merged 25 commits into
feat/openshell-grpc-stdin/aefrom
refactor/openshell-grpc-skill-control/ae

Conversation

@ericksoa

@ericksoa ericksoa commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Moves skill discovery and installation off host-managed SSH and onto the OpenShell sandbox-control boundary.

This is PR 4 of 21 in the OpenShell gRPC migration stack.

Changes

  • Use direct gRPC for skill reads and mutations.
  • Select one mutation transport before dispatch and never replay a failed mutation.
  • Remove skill-specific SSH configuration and invocation code.

Stack

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
  • 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: transport-boundary self-review completed; mutations select one transport before dispatch and are never replayed automatically.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — skill install/remote tests, transport inventory, and docs validation passed
  • Applicable broad gate passed — targeted and changed-graph tests were used; CI will run the repository-required broad gates
  • 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) — docs validation passed with 2 existing Fern warnings
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6ea9fee4-060f-4543-957c-9140b1d3d870

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/openshell-grpc-skill-control/ae

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

@github-code-quality

github-code-quality Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the feat/openshell-grpc-... branch.

TypeScript / code-coverage/cli

The overall coverage in the refactor/openshell-g... branch remains at 79%, unchanged from the feat/openshell-grpc-... branch.

Show a code coverage summary of the most impacted files.
File feat/openshell-grpc-... 20130dd refactor/openshell-g... 58fc44b +/-
src/lib/messagi...nes/template.ts 100% 95% -5%
src/lib/adapter.../docker/pull.ts 83% 79% -4%
src/lib/onboard...er-gpu-patch.ts 91% 88% -3%
src/lib/skill-install.ts 79% 84% +5%
src/lib/adapter...ocker/volume.ts 70% 85% +15%
src/lib/core/pr...mpt-activity.ts 67% 92% +25%
src/lib/adapter...teway-config.ts 0% 91% +91%
src/lib/adapter...trol-routing.ts 0% 94% +94%
src/lib/adapter...dbox-control.ts 0% 98% +98%
src/lib/adapter...dbox-control.ts 0% 100% +100%

Updated July 14, 2026 14:57 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / medium confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: onboard-repair, onboard-resume

2 optional E2E recommendations
  • openclaw-skill-cli
  • openshell-gateway-auth-contract

Workflow run details

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

ericksoa added 24 commits July 13, 2026 16:18
# Conflicts:
#	src/lib/adapters/openshell/sandbox-control-routing.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

# Conflicts:
#	src/lib/adapters/openshell/sandbox-control-routing.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…/openshell-grpc-skill-control/ae

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…/openshell-grpc-skill-control/ae

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…/openshell-grpc-skill-control/ae

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…/openshell-grpc-skill-control/ae

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@wscurran wscurran added area: architecture Architecture, design debt, major refactors, or maintainability area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery area: skills Skills, agent behaviors, prompts, or skill packaging refactor PR restructures code without intended behavior change labels Jul 14, 2026
@apurvvkumaria apurvvkumaria self-assigned this Jul 19, 2026

@apurvvkumaria apurvvkumaria 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 exact head 58fc44b against stacked base c6975ba. The skill install/remove migration selects one gateway-scoped control plane before mutation, fails closed on configuration or existence-probe errors, never replays a dispatched mutation across transports, bounds exec requests through the shared OpenShell validation, preserves binary stdin, validates skill names and remote paths, and closes direct gRPC control safely. The focused unit coverage exercises direct gRPC, Cloudflare edge-tunnel preselection, fail-closed routing, binary upload, sentinel handling, retry convergence, and orchestration cleanup. All exact-head CI, CodeQL, DCO, automated advisors, and E2E PR Gate checks are green with no unresolved threads. No blocking finding. This remains stacked on #6795.

@apurvvkumaria
apurvvkumaria merged commit dad8701 into feat/openshell-grpc-stdin/ae Jul 20, 2026
51 checks passed
@apurvvkumaria
apurvvkumaria deleted the refactor/openshell-grpc-skill-control/ae branch July 20, 2026 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: architecture Architecture, design debt, major refactors, or maintainability area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery area: skills Skills, agent behaviors, prompts, or skill packaging refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants