Commit Graph
5 Commits
Author SHA1 Message Date
Jim Meyer 8c7dd148a9 perf(net): set TCP_NODELAY on latency-sensitive TCP hops (#2220)
* perf(net): set TCP_NODELAY on all tunnel and proxy TCP hops

The sandbox tunnel added ~44 ms of latency to every small
request/response because no socket in the path disabled Nagle's
algorithm, so sub-MSS writes waited on delayed ACKs at each hop.

Set TCP_NODELAY on every latency-sensitive TCP socket:

- gateway: accepted connections on the public listener (gRPC relay
  frames and WS tunnel writes)
- CLI: edge tunnel local accept + underlying WebSocket TCP stream,
  insecure TLS connector (tonic's default connector already does
  this), and service-forward accepted sockets
- supervisor: direct-tcpip connect into the sandbox netns, TCP relay
  target dials, egress proxy accepted connections, and all upstream
  CONNECT/HTTP dials (via a new connect_upstream helper)

Setting TCP_NODELAY on connect is best-effort: a failure only costs
latency, so we log and continue rather than fail the connection.

The sandbox SSH transport rides a unix domain socket and gRPC client
channels use tonic defaults (nodelay on), so no change is needed there.

Fixes #2219

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

* refactor(net): house TCP_NODELAY helper in a shared net module

Address review feedback on the TCP_NODELAY change:

- Move the shared set-nodelay helper out of supervisor_session into a
  new crate-private `net` module in openshell-supervisor-process, so
  ssh and supervisor_session no longer reach across modules through a
  pub(crate) item.
- Make the best-effort comments at each call site terse and consistent.

No behavior change; the benchmark ladder reproduces the same numbers.

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

* refactor(net): consolidate TCP_NODELAY helpers into openshell_core::net

Move the best-effort TCP_NODELAY helpers into the shared openshell_core::net
module so every crate dials and configures sockets the same way:

- Add set_tcp_nodelay_best_effort (accepted/existing streams) and
  connect_tcp_nodelay_best_effort (dial + set) with unit tests.
- Migrate all call sites in openshell-cli, openshell-server, and the
  supervisor crates to the shared helpers.
- Remove the crate-private net module from openshell-supervisor-process.
- Document socket guidance in AGENTS.md (Network Sockets).

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

* perf(net): set TCP_NODELAY on exec bridge and metadata server

The gateway-side single-use SSH-over-relay loopback bridge and the
sandbox IMDS metadata server were missed latency-sensitive TCP hops.
Set TCP_NODELAY on the accepted client connection and both russh client
dials of the exec bridge — interactive keystrokes and line-buffered PTY
output are the most tinygram-heavy traffic in the system — and on the
metadata server's accepted connections.

Also log unrecognized MaybeTlsStream variants in the edge tunnel so a
future TLS-backend change surfaces a silent TCP_NODELAY miss instead of
skipping it quietly.

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

* perf(net): set TCP_NODELAY on openshell-sdk socket paths

The openshell-sdk crate landed on main with its own copies of the CLI's
hand-rolled sockets, which the CLI and TUI are meant to consume. Give
them the same treatment as the CLI equivalents:

- edge_tunnel: the accepted local tunnel connection and the WebSocket's
  underlying TCP socket (plain and rustls variants).
- transport: the dial in InsecureTlsConnector, tonic's custom-connector
  path.

Only these hand-rolled sockets need it. Tonic's own connector defaults
tcp_nodelay to true and applies it itself, so plain Endpoint::connect
callers were already covered.

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

---------

Signed-off-by: Jim Meyer <jim@meyer4hire.com>
2026-08-04 17:59:13 +00:00
Drew Newberry 59f7839f6b fix(auth): report gateway authentication status (#2435)
* fix(auth): report gateway authentication status

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(auth): reuse gateway info for status probe

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

---------

Signed-off-by: Drew Newberry <anewberry@nvidia.com>
2026-07-23 20:01:28 +00:00
Derek Carr 5952a5a23f feat(workspace): add workspace resource model with scoping, membershi… (#2243)
* feat(workspace): implement workspace model (Phase 1 of RFC 0011)

Implements workspace and membership model providing hard isolation
boundaries for multi-player OpenShell deployments.

Workspace CRUD with Kubernetes-style Terminating phase for graceful
deletion. All resources scoped by workspace via ObjectMeta. Membership
RPCs for workspace access control. Persistence migration shifts name
uniqueness to (object_type, workspace, name). Provider profiles support
platform and workspace scoping. Service routing uses workspace-prefixed
DNS labels. Inference routes renamed and workspace-scoped with
DeleteInferenceRoute RPC. Python SDK with WorkspaceClient, two-method
list pattern (workspace-scoped and for_all_workspaces), and workspace
parameter on all methods. CLI workspace flags, TUI workspace cycling.
K8s driver filters unmanaged CRs and uses delete preconditions. Podman
driver uses immutable container IDs. Label serialization fixed across
all put_if call sites.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(cli): delegate sandbox upload command to existing upload function

The standalone `sandbox upload` command reimplemented upload logic
inline with two bugs: it used `Path::exists()` which follows symlinks
(rejecting dangling symlinks), and it ran git-aware filtering on
symlink sources. The `run::sandbox_upload()` function already handles
both cases correctly via `sandbox_upload_plan()`. Replace the inline
logic with a call to the existing function.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(e2e): shorten sandbox names and fix test compatibility

Shorten the sandbox name in initial_sparse_policy_is_acknowledged_as_loaded
from 'e2e-2159-sparse-enrich' (22 chars) to 'e2e-sparse-enrich' (17 chars)
to comply with MAX_ROUTABLE_NAME_LEN (19 chars).

Also capture stderr in create_keep_with_args so future sandbox creation
failures include the actual CLI error instead of reporting empty output.

Signed-off-by: Derek Carr <decarr@redhat.com>

* test(workspace): add test coverage for workspace CRUD and persistence isolation

Add unit tests for workspace create happy path, get round-trip, get
not-found, get empty-name rejection, already-exists error, and
resolve_workspace not-found. Add persistence test proving cross-workspace
name uniqueness (same name in different workspaces produces separate
records). Add workspace name max-length boundary tests. Fix e2e harness
to include stderr in name-parse-failure error path. Align Python e2e
test_workspace_crud with try/finally pattern. Document provider profile
catalog workspace scoping gap in RFC 0011.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(examples): update examples for workspace model compatibility

Shorten sandbox names in demo scripts to fit the 19-character
MAX_ROUTABLE_NAME_LEN limit: policy-demo prefix to pd-, multi-agent
notepad derives a short SANDBOX_TAG from the run ID, governance
interceptor uses gs-PID-RANDOM. Update vscode-remote-sandbox.md SSH
host aliases from openshell-{name} to openshell-{name}.{workspace}
format.

Signed-off-by: Derek Carr <decarr@redhat.com>

* feat(sdk): add workspace-scoped client and workspace CRUD

Add WorkspaceScopedClient modeled after kube::Api::namespaced — captures
workspace once and injects it into every sandbox request. Add workspace
CRUD methods (create, get, list, delete) and list_sandboxes_all_workspaces
on OpenShellClient. Extend SandboxRef with workspace field and add
WorkspaceRef type. Include mock tests for all new operations.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(lint): resolve clippy warnings in workspace test assertions

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(docs): convert indented code blocks to fenced in RFC 0011

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(lint): resolve clippy warnings and apply cargo fmt across workspace

Auto-format with cargo fmt and fix clippy warnings exposed by the
reformat: unnecessary qualifications, map_unwrap_or, identical match
arms, unused variable prefix, dead code annotations, and let-unit-value
in e2e harness.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(workspace): address workspace scoping issues from review

- Add workspace field to settings JSON output (CLI)
- Skip Podman containers missing workspace label instead of defaulting
  to empty string, matching K8s driver behavior
- Add resource_version to list_by_scope SELECT in both SQLite and
  Postgres backends, with regression test
- Gate PolicyLocalContext proposal/lookup routes on workspace readiness,
  returning 503 when workspace is not yet discovered
- Block sandbox and provider creation in TUI all-workspaces mode
- Clear workspace vectors in TUI reset_sandbox_state

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(workspace): make provider profile catalog workspace-aware

Thread workspace through snapshot_catalog so the
EffectiveProviderProfileCatalog enforces workspace boundaries on both
read and write paths. UserProviderProfileSource now loads platform-scoped
profiles (workspace "") plus the target workspace's profiles, preventing
cross-workspace duplicate profile ID collisions that previously caused
global catalog failures.

Update RFC 0011 to reflect catalog scoping is implemented in Phase 1
rather than deferred to future work.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(persistence): include workspace column in atomic policy revision INSERT

put_policy_revision_atomic omitted the workspace column from the INSERT
into the objects table in both SQLite and Postgres backends, causing
atomically-written policy revisions to lose their workspace association.
Add workspace field to AtomicPolicyRevisionWrite and thread it through
both backend INSERT statements, matching the non-atomic put_policy_revision
path which already included it.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(proxy): skip ancestor walk when socket owner is the entrypoint

collect_ancestor_identities walked the entire process tree above the
entrypoint when the connecting process was the entrypoint itself,
SHA256-hashing every ancestor binary (IDE, shell, container runtime).
On dev machines with large binaries in the ancestor chain this exceeded
the 30-second test timeout. When start_pid == stop_pid there are no
intermediate ancestors to verify, so return an empty list immediately.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(workspace): make provider profile catalog scope-aware

Allow the same profile ID at platform and workspace scopes by
introducing layered catalog entries where workspace profiles shadow
platform profiles. Add source and scope fields to the ProviderProfile
proto and CLI output. Migrate List/Get handlers to the catalog,
fixing divergence with runtime profile resolution.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(e2e): align podman e2e labels with centralized driver constants

The podman driver moved its container labels to the centralized
openshell.ai/ prefix, but the e2e test harness and cleanup script
still referenced the old openshell.sandbox-* keys, causing the
local_driver_token_restart test to fail on container lookup.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(e2e): align python profile isolation test with scope-aware catalog

Platform profiles are now visible in workspace listings as fallbacks
per the layered catalog design. Update the assertion to match.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(workspace): honor profile_workspace in runtime profile resolution

Runtime profile lookups now consult provider.profile_workspace via
get_type_profile_for_scope. Providers created with --global-profile
(profile_workspace="") resolve to the platform profile even when a
workspace profile shadows the same ID. All 6 runtime call sites
updated; type-only call sites remain scope-agnostic.

Signed-off-by: Derek Carr <decarr@redhat.com>

---------

Signed-off-by: Derek Carr <decarr@redhat.com>
2026-07-21 00:59:18 +00:00
Drew Newberry 802932138b fix(sdk): initialize sandbox annotations (#2296)
The SDK merged after CreateSandboxRequest and ObjectMeta gained annotations on main, leaving stale struct initializers that prevented the crate and its tests from compiling. Initialize the curated request and mock metadata with empty annotation maps.

Signed-off-by: Drew Newberry <anewberry@nvidia.com>
2026-07-15 08:13:50 -07:00
Max Dubrinsky e6f319c76d feat(sdk): add openshell-sdk crate (#1862)
* feat(sdk): add openshell-sdk crate

Additive extraction of the shared async gRPC client core (transport, TLS,
OIDC single-flight refresh, edge tunnel, high-level sandbox surface, raw
escape hatch) as a new workspace crate. No existing consumers yet; CLI/TUI
migration follows in a separate PR.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

* refactor(sdk,core): reuse shared JWT exp decoder for refresh deadlines

Extract the signature-unverified JWT exp decode out of
openshell-core/grpc_client.rs into openshell_core::jwt::parse_exp_secs,
and have the openshell-sdk refresh path reuse it to derive a proactive
refresh deadline from a bearer JWT when the caller does not advertise
expires_at. Addresses review feedback to reuse pre-existing logic rather
than reimplement JWT expiry handling per client.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

* fix(sdk): harden refresh single-flight and redact tokens in Debug

Addresses review feedback on the openshell-sdk refresh path.

- Make the single-flight cleanup cancellation-safe. The in-flight slot is
  now cleared by the shared refresh computation itself (epoch-guarded)
  rather than the leader's post-await code. Previously, if the leader future
  was dropped (e.g. an FFI caller cancelling its promise) after a follower
  drove the refresh to completion, the completed future was stranded in the
  slot and later refresh_now() calls re-joined it, pinning the client to a
  stale or already-rejected token. Adds a regression test that cancels the
  leader and asserts the next refresh starts a fresh attempt.

- Redact bearer secrets from Debug. RefreshedToken and the oidc
  RefreshTokenInput/RefreshTokenOutput now use manual Debug impls that omit
  the access/refresh token fields via finish_non_exhaustive, matching the
  house style (e.g. SecretResolver, SandboxJwtIssuer). Prevents a stray
  {:?} or a containing struct's derived Debug from writing tokens to logs.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

* fix(sdk): fail refresh when the new token can't be encoded as metadata

store_bearer now returns an error instead of silently keeping the previous
bearer value. The TokenSource commits the refreshed token to its state
before the client writes it into the interceptor slot, so a silent drop
left the interceptor on the old (expiring) token with no path back to a
refresh. Surfacing the error fails the call loudly instead. Adds a unit
test covering a token that can't be encoded as gRPC metadata.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

* fix(sdk): refresh OIDC tokens on raw routes and harden rotation

Raw gRPC access never triggered OIDC refresh: a client that only used
raw_grpc/raw_inference kept sending the initial bearer until it expired,
with no proactive or reactive refresh. Add raw_grpc_fresh and
raw_inference_fresh accessors that refresh before returning the client,
plus force_refresh for reactive recovery after an Unauthenticated raw
RPC.

Guard the single-flight refresh commit against a concurrent replace().
The in-flight attempt now records the generation it started from and
skips its write when an external replace() has advanced it, so timer or
callback driven rotation is no longer clobbered by a slower refresh.

Remove TokenSource::snapshot(): it returned an empty string under write
contention and had no consumer on the CLI/TUI path. Tests read committed
state directly instead.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

* docs(sdk): rewrite crate README for consumers

Recast the openshell-sdk README as a usable crate README rather than an
RFC excerpt. Drop the Responsibilities/Non-responsibilities/Consumers
scope-boundary sections and the mTLS migration rationale, folding the
useful facts (explicit token, no disk/name resolution, Refresh trait,
SdkError mapping) into the intro, a new Auth and refresh section, and
Public surface. Remove the dead relative RFC link and status-label
prose so the doc renders cleanly wherever it is published.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

* fix(sdk): address review feedback on OIDC refresh and transport

Apply Drew's review notes on PR #1862:

- Drop unused `rustls-pemfile` dependency and move `tokio-stream` to
  dev-dependencies (only used by tests).
- Guard OIDC `expires_at` against u64 overflow with `saturating_add`.
- Fix stale `#[non_exhaustive]` rationale in `AuthConfig` (the struct
  `Oidc` variant it described as future already ships).
- Stop double-wrapping refresh errors: store the bare refresh-error text
  so the single `SdkError::auth` wrap happens once at await.
- Strip stale CLI porting breadcrumbs from `build_channel` docs, keeping
  the branch table.
- Treat proactive token refresh as best-effort: a transient failure falls
  through to the request instead of failing an RPC whose current token is
  still valid, with a regression test.
- Collapse `exec`'s inline auth retry into the shared `unary` helper.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

* fix(sdk): preserve transient/terminal distinction in refresh errors

The refresh single-flight collapsed both `RefreshError::Transient` and
`RefreshError::Terminal` into a stringified `SdkError::Auth`, so consumers
(CLI, TUI, future language bindings) had no machine-readable way to tell a
retryable IdP blip from a dead session that needs re-authentication.

Carry the `RefreshError` through the shared outcome (kept `Clone` for
`Shared`) instead of its rendered text, and map it at the await site to a
new `retryable` flag on `SdkError::Auth`. Add `SdkError::auth_retryable`
and a `SdkError::retryable()` accessor; transient refresh failures report
`true`, every other error `false`. Add classification tests.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

---------

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>
2026-07-14 22:35:46 -07:00