Skip to content

refactor(openshell): collapse SSHFS compatibility wrapper - #6808

Closed
ericksoa wants to merge 21 commits into
refactor/openshell-exec-process-recovery/aefrom
refactor/openshell-collapse-sshfs-compat/ae
Closed

ericksoa wants to merge 21 commits into
refactor/openshell-exec-process-recovery/aefrom
refactor/openshell-collapse-sshfs-compat/ae

Conversation

@ericksoa

@ericksoa ericksoa commented Jul 13, 2026 •

Copy link
Copy Markdown
Member

Summary

Collapses the remaining SSH-config compatibility wrapper into the one intentional SSHFS share call site.

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

Changes

  • Delete the dedicated SSH-config client and runtime wrappers and their redundant sandbox lookup.
  • Keep openshell sandbox ssh-config only at the explicit SSHFS share boundary.
  • Remove obsolete wrapper tests and the stale recovery SSH mock.
  • Shrink the reviewed legacy-transport inventory to the single supported SSHFS path.

This slice removes 119 lines and adds 8.

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

  • Existing tests cover changed behavior — justification: client, recovery, and legacy-transport inventory tests cover the collapsed boundary; build and repository checks pass.
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs not applicable — justification: this slice changes only internal transport plumbing and tests.
  • 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; the only retained SSH-config call is the supported OpenShell API required by the explicit SSHFS feature.
  • 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 — build plus 22 client tests and 31 recovery/inventory integration tests passed
  • Applicable broad gate passed — npm run check:diff passed at 65b4140d
  • 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) — not applicable
  • 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

ericksoa added 4 commits July 13, 2026 15:43
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>
@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: 8e60a72e-1e0b-45ea-a4c9-cddd3b4e8bf8

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-collapse-sshfs-compat/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 in commit f3242c4 in the refactor/openshell-c... branch remains at 96%, unchanged from commit 9fae0d4 in the refactor/openshell-e... branch.

TypeScript / code-coverage/cli

The overall coverage in commit f3242c4 in the refactor/openshell-c... branch is 79%. The coverage in commit ad26c30 in the refactor/openshell-e... branch is 80%.

Show a code coverage summary of the most impacted files.
File refactor/openshell-e... ad26c30 refactor/openshell-c... f3242c4 +/-
src/lib/inference/local.ts 82% 71% -11%
src/lib/shields/index.ts 72% 62% -10%
src/lib/inference/vllm.ts 87% 77% -10%
src/lib/policy/index.ts 58% 64% +6%
src/lib/adapter...teway-config.ts 0% 77% +77%
src/lib/adapter...dbox-control.ts 0% 78% +78%
src/lib/state/s...tore-payload.ts 0% 83% +83%
src/lib/adapter...dbox-control.ts 0% 94% +94%
src/lib/adapter...trol-routing.ts 0% 97% +97%
src/lib/adapter...ndbox-upload.ts 0% 99% +99%

Updated July 20, 2026 20:57 UTC

@ericksoa ericksoa mentioned this pull request Jul 13, 2026
11 of 20 tasks
@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): Failed

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, state-backup-restore, upgrade-stale-sandbox, cloud-onboard

3 optional E2E recommendations
  • snapshot-commands
  • sandbox-operations
  • 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.

@wscurran wscurran added area: architecture Architecture, design debt, major refactors, or maintainability area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery refactor PR restructures code without intended behavior change labels Jul 14, 2026
ericksoa added 6 commits July 14, 2026 20:08
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>
Base automatically changed from refactor/openshell-remove-temp-ssh-config/ae to refactor/openshell-exec-process-recovery/ae July 17, 2026 22:29
@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 6a9dce5. The share-only SSH-config boundary is gateway-scoped, the path preflight uses the reviewed read-only gRPC/CLI fallback without shell interpolation, and the legacy-transport audit tracks the retained exception. Local exact-head validation passed: CLI 71/71, integration 47/47, build, and diff hygiene. All 20 commits are GitHub Verified and there are no unresolved review threads. The current red protected E2E gate is not a product-lane failure: the trusted generate-matrix job failed before dispatch while this stacked head carries an older E2E planner/workflow generation than current main; it will need a stack refresh/re-dispatch before merge.

## Summary

Uses one binary-safe CLI capture path for both text and archive fallback
execution.

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

## Changes

- Decode UTF-8 responses from the existing binary-safe CLI capture
primitive.
- Remove the duplicate text-capture normalization path.
- Preserve raw stdout bytes only when the caller requests buffer output.
- Delete the unused CLI control singleton.

This slice removes 60 lines and adds 37.

## Stack

- Stack index: #6790
- Depends on: #6808
- Review this PR against base branch
`refactor/openshell-collapse-sshfs-compat/ae` to see only this slice.

## Type of Change

- [x] 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

- [x] Existing tests cover changed behavior — justification: CLI
control, routing, and direct gRPC adapter suites cover text, binary,
failure, and fallback behavior.
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs not applicable — justification: this slice changes only
internal transport implementation and tests.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: transport-boundary
self-review completed; mutation routing and no-replay behavior are
unchanged.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — build plus 30 CLI control, routing,
and gRPC adapter tests passed
- [x] Applicable broad gate passed — `npm run check:diff` passed at
`64e5e672`
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — not
applicable
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(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>
@ericksoa

Copy link
Copy Markdown
Member Author

Closing as superseded: NemoClaw will consume the upstream OpenShell TypeScript SDK from NVIDIA/OpenShell#2122 instead of maintaining this raw gRPC/proto transport stack.

@ericksoa ericksoa closed this Jul 23, 2026
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 refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants