Skip to content

feat(api)!: define lifecycle semantics for bidirectional streams #3056

Open

Description

@drew

User Story

As a client implementing exec, TCP forwarding, or relay protocols, I want explicit stream lifecycle semantics, so that I can half-close input, drain output, cancel cleanly, and distinguish normal completion from transport failure.

Problem Statement

OpenShell bidirectional frame protocols define initialization and data payloads but do not consistently define explicit half-close or close frames. Cancellation, first-frame validation, cross-direction ordering, backpressure, deadlines, and retry behavior are incomplete or distributed across implementation details.

Impact / Why This Matters

SDKs must infer when closing their send direction should preserve the receive direction, and gateways/supervisors can interpret EOF differently. This creates hangs, truncated output, premature relay teardown, and language-specific behavior. Protocol ambiguity becomes harder to correct after the beta compatibility boundary.

Proposed Design

Specify lifecycle state machines for each public bidirectional RPC. Where transport EOF is insufficient or ambiguous, add explicit close/half-close frames carrying the channel and optional reason. Define:

  • required first frame and rejection behavior;
  • legal frame ordering;
  • input half-close while output continues;
  • normal completion versus cancellation and error;
  • backpressure and maximum frame behavior;
  • idle and operation deadlines; and
  • whether and how a failed stream can be retried.

Expose equivalent semantics through every SDK.

Acceptance Criteria

  • Each bidirectional RPC has a documented lifecycle/state machine.
  • TCP forward and relay protocols support clean half-close where the underlying TCP flow requires it.
  • Invalid first frames and illegal frame ordering return consistent status codes.
  • Client cancellation, server cancellation, idle timeout, and normal EOF are distinguishable.
  • Implementations do not truncate remaining output when the input direction closes normally.
  • Rust, Python, TypeScript, and Go SDK helpers expose consistent lifecycle behavior.
  • Tests cover both half-close directions, simultaneous close, cancellation, timeout, backpressure, and peer failure.
  • Gateway/supervisor version-skew expectations for new frames are documented.

Alternatives Considered

Use gRPC stream EOF as the only close signal. This is simple but cannot always express per-channel closure or preserve the opposite direction consistently. Add close semantics only in SDKs. That leaves raw clients and gateway/supervisor peers with an underspecified wire contract.

Agent Investigation

TcpForwardFrame and RelayFrame currently carry initialization and data without a common explicit close payload, while related control streams define separate relay-close messages. Capability/version-skew handling is related to #2949.

Related: #2565, #2949. Source audit: https://gist.github.com/mrunalp/e80942c1544a0225ee588796a41ab30b.

Activity

  1. varshaprasad96 commented on Sep 10, 2026

    @varshaprasad96
    Contributor

    📋 triage-agent

    Triage Assessment

    Classification: validated-feature

    Summary

    The lifecycle-contract gap is confirmed against current code. The proposal is technically coherent and feasible, with high-confidence evidence. It does not require deeper investigation to establish validity, though maintainers must define the exact wire semantics before implementation.

    Investigation

    Impact Signals

    • Affected users/scope: Clients and SDK authors using interactive exec, TCP forwarding, or gateway-supervisor relays across Rust, Python, TypeScript, and Go.
    • Regression: No; this is a pre-existing contract gap.
    • Workaround: Partial and language-specific use of gRPC EOF/cancellation; no consistent cross-SDK workaround.
    • Evidence quality: High; current protobufs, handlers, SDK implementations, related issue history, and the linked API audit directly confirm the gap.

    Human Decision Required

    Decide whether OpenShell should address this issue. If yes, apply
    state:accepted, associate it with a roadmap item, or do both, and decide
    whether the work remains human-owned. Either action records acceptance;
    roadmap placement additionally records sequencing.
    To queue investigation or planning for an unattended agent, also apply
    agent:plan-requested. You can instead directly ask an agent to use
    create-spike or build-from-issue on this issue; the agent will warn about
    missing expected workflow labels and continue without changing them. If no,
    close it as not planned and record the rationale.

  2. varshaprasad96 commented on Sep 10, 2026

    @varshaprasad96
    Contributor

    🏗️ build-plan

    Implementation Plan

    Issue type: feat
    Complexity: High
    Confidence: Medium — the protocol path is clear; breadth across runtimes and four SDKs is substantial.

    Summary

    Define explicit lifecycle contracts for all four bidirectional RPCs. Add an in-band, direction-implicit half-close frame only to ForwardTcp and RelayStream; retain transport EOF for ExecSandboxInteractive input and ConnectSupervisor, where it closes the only request direction without needing the stream to remain writable.

    Deliver this as two independently mergeable PRs: first the wire/runtime contract, CLI, documentation, and end-to-end behavior; second curated SDK parity across Rust, Python, TypeScript, and Go.

    Minimal Contract

    • Add an empty StreamHalfClose payload to TcpForwardFrame and RelayFrame.
      • Direction is implied by the stream direction.
      • It represents TCP FIN only, never cancellation or failure.
      • Data after half-close, duplicate half-close, repeated init, and empty/unknown payloads return InvalidArgument.
      • Transport request EOF remains a backward-compatible implicit half-close.
      • The response stream ends successfully only after both directions close and buffered data drains.
    • Use gRPC status for terminal failures:
      • InvalidArgument: first-frame or ordering violation.
      • ResourceExhausted: data frame exceeds the documented 64 KiB maximum.
      • Cancelled: intentional remote cancellation.
      • DeadlineExceeded: server-side idle timeout.
      • Unavailable: peer or transport failure.
      • Local SDK cancellation remains distinguishable because the helper records that its own cancellation token initiated it.
    • Extend ExecSandboxExit with an additive termination reason:
      • PROCESS_EXITED
      • OPERATION_TIMEOUT
      • unspecified is interpreted as PROCESS_EXITED for older gateways.
      • Preserve exit code 124 for CLI compatibility, but SDKs expose the reason.
    • Version skew:
      • Add capability strings to TcpForwardInit, SupervisorHello, and SessionAccepted.
      • Use stream-half-close-v1.
      • Gateway and supervisor use explicit relay half-close only when negotiated.
      • A new gateway uses legacy transport completion with an older supervisor.
      • A new supervisor emits new relay frames only when the gateway echoes the capability.
      • New SDKs advertise TCP half-close support. With older gateways they accept response EOF as the remote half-close fallback.
    • Retry contract:
      • Never automatically retry interactive exec, ForwardTcp, or active RelayStream calls.
      • Reconnect ConnectSupervisor with existing backoff after transport failure or idle timeout.
      • Do not reconnect after deliberate local shutdown or a clean terminal session close.

    Transport EOF Assessment

    • ExecSandboxInteractive: sufficient. Client request EOF means stdin/input is closed; the server must continue draining stdout/stderr until one terminal exit event and successful response EOF. Cancellation is not treated as stdin EOF.
    • ConnectSupervisor: sufficient. It has one session-level request direction and no underlying TCP half-open channel. Enforce Hello-first ordering and heartbeat liveness; use gRPC completion/status for termination.
    • ForwardTcp: insufficient in the server-to-client direction. Ending the response stream completes the RPC and can truncate a still-open client-to-target direction. Add StreamHalfClose.
    • RelayStream: insufficient in the gateway-to-supervisor direction for the same reason. Use explicit half-close in both directions for symmetry and deterministic state handling.

    Scope

    PR 1: Protocol, runtime, CLI, and documentation

    • proto/openshell.proto: add StreamHalfClose, frame variants, capability fields, documented lifecycle/status tables, 64 KiB data limit, and exec termination reason.
    • sdk/go/proto/openshellv1/*.pb.go: regenerate committed Go bindings.
    • crates/openshell-server/src/grpc/sandbox.rs: coordinate ForwardTcp directions; drain output after input close; propagate stable statuses; reject illegal interactive-exec frames; populate timeout reason.
    • crates/openshell-server/src/supervisor_session.rs: negotiate capabilities; implement relay half-close; enforce ConnectSupervisor ordering and heartbeat timeout.
    • crates/openshell-supervisor-process/src/supervisor_session.rs: negotiate capabilities; relay FIN independently; classify clean close, timeout, cancellation, and failure.
    • CLI forwarding code: advertise support, send half-close after local EOF, process remote half-close, and drain the opposite direction.
    • Server integration and relay E2E tests: cover both FIN directions, simultaneous close, compatibility, status propagation, backpressure, peer failure, and exec output drain.
    • architecture/gateway.md and published reference docs: document lifecycle, liveness, limits, retry, and version skew.

    PR 2: SDK parity

    • Rust SDK: add curated interactive-exec and TCP-forward helpers with graceful input close, forceful cancel, draining, termination reason, and typed status.
    • Python SDK: add equivalent bounded, thread-safe synchronous helpers and tests.
    • TypeScript SDK: expose closeInput(), cancel(), terminal reason, and local-versus-remote cancellation; preserve compatibility aliases where possible.
    • Go SDK: add CloseWrite/graceful input close separately from Cancel/full close; stop cancelling immediately after CloseSend.
    • SDK docs/examples: document write, graceful input close, drain, terminal outcome, and cancel.

    Implementation Steps

    1. Add protobuf fields, comments, capability constants, generated Go bindings, and descriptor compatibility tests.
    2. Implement pure ForwardTcp and RelayStream transition validation: AwaitInit → Open → LocalHalfClosed/RemoteHalfClosed → Closed, with fatal protocol/error states.
    3. Refactor gateway and supervisor bridges to drive those transitions while preserving per-direction ordering and backpressure.
    4. Tighten ExecSandboxInteractive and ConnectSupervisor ordering, cancellation, timeout, and reconnect behavior.
    5. Update both CLI ForwardTcp consumers and verify legacy capability fallback.
    6. Add server integration and full relay E2E tests; publish protocol and version-skew documentation.
    7. Add Rust and Python curated helpers with contract tests.
    8. Align TypeScript and Go helpers and error mapping with the same lifecycle contract.
    9. Run mise run pre-commit, mise run test, mise run go:ci, mise run sdk:ts:ci, and the relevant Docker E2E lane.

    Test Plan

    • Unit tests: protocol/state validation, exec output drain and timeout reason, ConnectSupervisor ordering/liveness, and SDK graceful-close/cancel/error behavior.
    • Integration tests: both half-close directions, delayed data after opposite FIN, simultaneous FIN, negotiated and legacy peers, backpressure, status propagation, and peer failure.
    • E2E tests: real TCP FIN behavior in both directions and interactive exec output drain after request EOF; run mise run e2e:docker.

    Risks & Open Questions

    • A streamed Status must terminate all bridge tasks deterministically; detached tasks must not outlive the RPC.
    • Capability negotiation must prevent an older supervisor from seeing an unknown half-close frame as an empty payload.
    • Python's synchronous gRPC API requires bounded queues and a background receive thread; cancellation must reliably unblock both.
    • Existing generic Close() meanings differ by language. Preserve compatibility aliases, but make graceful input close and forceful cancellation explicit.
    • The issue should close only after PR 2 lands; PR 1 provides the wire behavior but not all SDK acceptance criteria.

    Documentation Impact

    • Add a published streaming lifecycle reference and navigation entry.
    • Update architecture/gateway.md and the support matrix.
    • No docs/reference/gateway-config.mdx change: no gateway TOML field, driver option, default, or Helm rendering changes.
    • Review related public/debug skills for behavior drift; no skill inventory/workflow change is expected.

    LSM Compatibility

    No SELinux/AppArmor-specific impact is expected. This changes protobuf framing, socket shutdown, async coordination, and SDK APIs; it does not add /proc inspection, process identity lookup, privilege changes, filesystem labeling, or new binary execution.


    Revision 1 — initial plan against ea8eda6d5

  3. varshaprasad96 commented on Sep 10, 2026

    @varshaprasad96
    Contributor

    Will work on the open questions, and get back on the issue.

  4. varshaprasad96 commented on Sep 14, 2026

    @varshaprasad96
    Contributor

    Answers to some open questions, please let me know if there are objections:

    • Half-close: Use request EOF and capability-gated response half-close frames for forwarding and relays. Older peers retain legacy behavior.
    • Relay failures: Send typed, abortive RelayClose messages over the control stream; fall back to transport failure. Graceful closure drains output; abort does not guarantee draining.
    • Exec input: Closing input ends stdin and resize messages. Send SSH EOF without immediately closing the channel so output can drain.
    • Completion: Require an exit event followed by successful gRPC completion. Preserve the observed exit code if the stream subsequently fails.
    • Cancellation: Terminate the exec process group with bounded graceful/forced cleanup, protecting unrelated processes. Control session loss aborts its active relays.
    • Timeouts and reconnect: Bound initialization, heartbeat detection, and cleanup. Allow healthy silent or half-closed connections. Reconnect unexpected control-session failures, but never automatically replay exec or forwarding operations.
    • Limits and SDKs: Use a 64 KiB data-frame limit with automatic SDK splitting and bounded, lossless buffering. Preserve existing Close() meanings and add explicit graceful-close/cancel operations (does the limit sound reasonable?).

    A few tradeoffs behind these decisions:

    • Compatibility over mandatory negotiation: Older peers retain their existing behavior, so mixed-version deployments cannot guarantee full half-close semantics. We accept that limitation rather than require a handshake before application data without a demonstrated need.
    • Separate failure signaling: Control-stream failure messages can race with data or transport closure, so the precise reason may occasionally be unavailable. This lets the supervisor report failures even after half-closing its relay request stream.
    • Control loss interrupts active operations: We abort affected relays rather than migrate them across sessions. This keeps ownership and cleanup clear and avoids automatically replaying operations with side effects.
    • Cancellation means termination, not detach: Cancelling exec stops its process group to avoid leaving unattended work running. Cleanup must protect the sandbox main process and unrelated workloads.
    • Process exit is separate from stream success: A command may finish successfully while delivery subsequently fails. We retain the exit code but report the stream failure rather than claim complete success.
    • Healthy silence remains valid: No default application-idle or half-close timeout means abandoned connections may retain resources. This preserves legitimate long-running workloads; explicit deadlines and cancellation provide control.
    • Frame limits need a compatibility rollout: The 64 KiB limit bounds payload sizes, but existing raw clients may send larger frames. SDKs will split writes automatically; enforcement must account for older clients.
  5. varshaprasad96 commented on Sep 15, 2026

    @varshaprasad96
    Contributor
  6. varshaprasad96 commented on Sep 22, 2026

    @varshaprasad96
    Contributor

    PR #3359 is merged. It addresses the interactive-exec slice: request EOF closes input while output continues draining, process exit is distinguished from terminal gRPC completion, and Go/TypeScript expose explicit lifecycle controls while preserving existing interfaces.

    I propose tackling the remaining work in three scoped follow-ups:

    1. ForwardTcp and RelayStream: request EOF for input half-close, capability-gated response half-close, typed abortive RelayClose handling, consistent frame validation/status mapping, and mixed-version tests.
    2. Control-session and cancellation behavior: bounded initialization, heartbeat detection and cleanup; abort active relays on control loss; reconnect without replaying operations; verify exec process-group termination.
    3. SDK parity and closure coverage: Rust/Python helpers, forwarding lifecycle alignment across SDKs, frame-limit rollout, and complete lifecycle documentation and tests.

    For the first follow-up, we still need to pin down status/reason mappings, precedence when terminal events race, and exact legacy-peer fallback behavior.

    The September 14 decisions should supersede conflicting parts of the original plan: use request EOF plus capability-gated response half-close, and allow healthy silent connections rather than introducing a default application-idle timeout.

    Keeping #3056 open until the remaining cross-RPC, cross-SDK, and version-skew acceptance criteria are covered.

  7. varshaprasad96 commented on Sep 22, 2026

    @varshaprasad96
    Contributor

    While preparing the next ForwardTcp/RelayStream slice against current main, we found that HA routing also uses a gateway-to-gateway PeerRelay stream:

    client → receiving gateway → owning gateway → supervisor → target

    Recommendation: include PeerRelay's directional-close and error-propagation behavior in this slice. Otherwise, the new half-close guarantee would depend on routing: a direct connection could drain correctly while the same operation routed through another gateway retains the existing truncation risk. This is a proposed scope addition, not yet implemented.

    The main implications are:

    • Version skew: negotiate support independently on each hop. Older peers must not receive unknown half-close frames; a legacy hop retains the existing limitation during rolling upgrades.
    • Error propagation: preserve terminal failures across internal byte bridges rather than letting them appear as successful EOF.
    • Ownership and cleanup: cancellation must wake blocked I/O and release bridge tasks across both gateways and the supervisor. Stale sessions or unrelated channels must not be able to abort another relay.
    • Resource lifetime: a legitimate half-closed connection may remain open longer while its opposite direction drains. Preserve that behavior without adding a default application-idle timeout.
    • Validation scope: add two-gateway coverage for both FIN directions, simultaneous close, mixed-version peers, peer failure, and cancellation under backpressure.

    Capability advertisement should use existing initialization messages, avoiding a separate negotiation round trip before application data.

    The alternative is a smaller, explicitly single-gateway slice with a documented HA limitation. Including PeerRelay is preferable for a consistent end-to-end forwarding contract. Heartbeat/reconnect policy, exec process-group cancellation, and remaining SDK parity stay in their separate follow-ups.

  8. github-actions commented on Oct 7, 2026

    @github-actions

    This issue has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity. Comment or remove the state:stale label to keep it open.

  9. github-actions commented on Oct 22, 2026

    @github-actions

    Closing this issue after 14 days with no activity since it was marked stale. Reopen it if the work is still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions