Skip to content

feat(openshell): route session reads over pinned gRPC - #6791

Closed
ericksoa wants to merge 21 commits into
refactor/openshell-control-plane/aefrom
chore/openshell-grpc-protocol/ae
Closed

feat(openshell): route session reads over pinned gRPC#6791
ericksoa wants to merge 21 commits into
refactor/openshell-control-plane/aefrom
chore/openshell-grpc-protocol/ae

Conversation

@ericksoa

@ericksoa ericksoa commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Introduces the first complete direct OpenShell gRPC path: pinned v0.0.72 protocol sources, the bounded TypeScript client, authenticated gateway resolution, and the production OpenClaw session-list consumer. The OpenShell CLI remains a pre-dispatch-only compatibility fallback for this read-only operation.

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

Changes

  • Vendor, package, and integrity-pin the exact OpenShell v0.0.72 protocol sources loaded by the runtime client.
  • Add a direct SandboxService client with literal-loopback plaintext enforcement, TLS and bearer validation, bounded output capture, deadlines, nonzero-result preservation, and transport-error reporting.
  • Resolve named gateway endpoints and authentication material from OpenShell user or system metadata for plaintext, mTLS, and OIDC modes; fail closed for unsupported modes.
  • Route OpenClaw session listing through authenticated direct gRPC and retry through the supported CLI only when configuration or sandbox lookup fails before dispatch.
  • Apply the same bounded deadline to the direct attempt and CLI fallback, and never replay completed commands or post-dispatch failures.
  • Cover protocol drift and missing sources, IPv4 and IPv6 loopback acceptance, credential resolution, routing, lifecycle close behavior, fallback deadlines, and session passthrough behavior.

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
  • Docs not applicable — justification: the command-reference update is isolated in the immediate stacked docs-and-stdin follow-up feat(openshell): document sessions and carry stdin over gRPC #6795; this implementation slice updates inline CLI help together with the behavior.
  • 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: plaintext is literal-loopback-only; credentials are loaded from OpenShell-owned gateway directories; only pre-dispatch failures on a read-only command can retry; completed or post-dispatch operations are never replayed.
  • 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 — protocol-pin tests passed (4 tests); gateway-config, direct-client, routing, and session-passthrough tests passed (62 tests); npm run typecheck:cli passed
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — targeted and changed-graph validation were used; required CI supplies the broad repository 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)
  • 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 2 commits July 13, 2026 15:43
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: 3ab2544d-2d04-4253-b9af-26f792835e27

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 chore/openshell-grpc-protocol/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 refactor/openshell-c... branch.

TypeScript / code-coverage/cli

The overall coverage in the chore/openshell-grpc... branch remains at 80%, unchanged from the refactor/openshell-c... branch.

Show a code coverage summary of the most impacted files.
File refactor/openshell-c... 78e0e4e chore/openshell-grpc... 54be3b1 +/-
src/lib/domain/.../connect-env.ts 97% 89% -8%
src/lib/actions.../passthrough.ts 94% 94% 0%
src/lib/agent/s...store-reader.ts 90% 90% 0%
src/lib/sandbox...rce-identity.ts 91% 91% 0%
src/lib/inferen...er-lifecycle.ts 65% 71% +6%
src/lib/inferen...lama/process.ts 50% 100% +50%
src/lib/adapter...teway-config.ts 0% 90% +90%
src/lib/adapter...trol-routing.ts 0% 91% +91%
src/lib/adapter...dbox-control.ts 0% 98% +98%
src/lib/adapter...dbox-control.ts 0% 100% +100%

Updated July 19, 2026 12:04 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 — Blocking findings reported

Advisor assessment: Blockers require maintainer review
Next action: Review the blockers below.
Findings: 1 blocker · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 1 blocker · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 2 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized E2E selections match; Nemotron reported 1 fewer blocker, 2 more warnings, the same number of suggestions.

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: cloud-onboard, credential-sanitization, security-posture, onboard-repair, onboard-resume

3 optional E2E recommendations
  • sessions-agents-cli
  • openshell-gateway-auth-contract
  • openshell-version-pin

Blockers

PRA-1 Blocker — Preserve the end-to-end timeout budget across CLI fallback

  • Location: src/lib/adapters/openshell/sandbox-control-routing.ts:90
  • Category: correctness
  • Problem: The request contract defines timeoutMs as an end-to-end lookup-and-execution deadline, but a pre-dispatch gRPC lookup failure is followed by a CLI execution with the original full timeout. Time spent in gRPC client creation and lookup is not deducted, so a request can run beyond its documented deadline.
  • Impact: A session-list request during a gateway lookup failure can occupy the caller longer than its configured operation deadline, delaying cancellation/error handling and violating the shared timeout contract.
  • Fix: Establish one absolute deadline before the gRPC attempt and pass only the remaining duration to the CLI fallback; if no time remains, return a timeout result instead of dispatching the fallback.
  • Verification: Inspect the fallback request after a deliberately delayed pre-dispatch lookup and confirm the CLI timeout is reduced by elapsed lookup time and no CLI dispatch occurs after expiry.
  • Test coverage: Add a routing test with a controlled clock or delayed pre-dispatch gRPC failure that asserts the CLI receives only remaining timeoutMs, plus an expiry case that asserts it is not called.
  • Evidence: src/lib/adapters/openshell/sandbox-control.ts documents timeoutMs as an end-to-end lookup and execution deadline. src/lib/adapters/openshell/sandbox-control-routing.ts forwards request.timeoutMs unchanged to gRPC and again to the CLI fallback. src/lib/adapters/openshell/sandbox-control-routing.test.ts asserts the fallback receives the original bounded timeout after a pre-dispatch failure.

Workflow run details

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

ericksoa added 6 commits July 13, 2026 16:00
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>
@ericksoa ericksoa changed the title chore(openshell): pin gRPC protocol sources feat(openshell): add pinned direct gRPC sandbox client Jul 14, 2026
## Summary

Hardens the new direct gRPC client so plaintext transport is accepted
only for literal loopback IP addresses. DNS hostnames such as
`localhost` must use TLS because their resolution is not fixed by the
endpoint string.

This is PR 3 of 26 in the OpenShell gRPC migration stack.

## Changes

- Validate plaintext endpoints with Node's IP parser instead of a
hostname or dotted-string heuristic.
- Accept IPv4 `127.0.0.0/8` and IPv6 `::1` literals only.
- Reject `localhost`, non-loopback IPs, malformed dotted addresses, and
non-HTTP(S) endpoint forms.
- Update the direct-client fixtures and focused endpoint-safety
coverage.

## Stack

- Stack index: #6790
- Depends on: #6791
- Review this PR against base branch `chore/openshell-grpc-protocol/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] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: this slice tightens an
internal transport safety check before any product call site selects
direct gRPC.
- [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: plaintext is restricted
to literal kernel-recognized loopback addresses; hostnames and malformed
or non-loopback addresses fail closed.
- [ ] 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 — focused direct gRPC client tests
passed (19 tests); `npm run typecheck:cli` passed
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — targeted and changed-graph validation were
used; required CI supplies the broad repository gates
- [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)
- [ ] 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>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa changed the title feat(openshell): add pinned direct gRPC sandbox client feat(openshell): route session reads over pinned gRPC Jul 14, 2026
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa added 9 commits July 13, 2026 22:52
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>
@wscurran wscurran added feature PR adds or expands user-visible functionality integration: openclaw OpenClaw integration behavior labels Jul 14, 2026
@apurvvkumaria apurvvkumaria self-assigned this Jul 19, 2026
Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@ericksoa

Copy link
Copy Markdown
Contributor 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

feature PR adds or expands user-visible functionality integration: openclaw OpenClaw integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants