Skip to content

feat(streams)!: define forwarding and relay lifecycle - #3669

Open
varshaprasad96 wants to merge 3 commits into
NVIDIA:mainfrom
varshaprasad96:feat/3056-forward-relay-lifecycle/varshaprasad96
Open

feat(streams)!: define forwarding and relay lifecycle#3669
varshaprasad96 wants to merge 3 commits into
NVIDIA:mainfrom
varshaprasad96:feat/3056-forward-relay-lifecycle/varshaprasad96

Conversation

@varshaprasad96

@varshaprasad96 varshaprasad96 commented Sep 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Allow TCP forwarding and supervisor relays to close one byte direction while draining the other. Negotiate response FIN across the CLI, gateway peers, and supervisor, and preserve downstream final status so a later relay error does not appear as successful EOF.

Breaking behavior: malformed post-init frames now fail explicitly, relay failures propagate to callers, RelayClose actively aborts its owned operation, and negotiated streams wait for both directions to finish. The protobuf additions are capability-gated and existing SDK Close meanings are unchanged. Flag these behavioral changes for release review, especially if merged after September 25, 2026.

Related Issue

Refs #3056. Follow-up to #3359, covering ForwardTcp, RelayStream, and PeerRelay. This does not close the broader issue: equivalent curated SDK lifecycle helpers and the remaining timeout/retry policy work are outside this slice.

Changes

  • Add negotiated response FIN and typed, session-scoped relay cancellation.
  • Own both directional pumps and carry downstream RPC completion separately from byte EOF, including cancellation while waiting for final status.
  • Preserve delayed target replies through legacy internal relays and propagate capability limitations across gateway peers.
  • Keep established data relays alive when only their control session disconnects; retain legacy SSH stdio behavior.
  • Update generated Go bindings, architecture and user documentation, CLI skill guidance, and regression coverage.

Testing

  • mise run pre-commit passes, including the commit hook; final core Clippy check passes.
  • Unit and integration tests added/updated: 14 lifecycle regressions, 1,758 server unit tests, server integration suites including 8 supervisor relay tests, and 67 supervisor tests passed. Coverage includes delayed replies, multi-hop FIN, late error trailers, cancellation, and control-session loss.
  • Python E2E cases added/updated with workspace-scoped authorization; both collect successfully and Python lint passes.
  • Full test suite
  • mise run ci
  • Docker E2E execution

Checklist

  • Follows Conventional Commits.
  • Commits are signed off (DCO).
  • Architecture and user-facing documentation updated.
  • Related agent skill guidance reviewed and updated.
  • Breaking behavioral changes identified above.

@varshaprasad96
feat(streams)!: enforce forwarding and relay lifecycle
302ad2e 
Preserve directional EOF across ForwardTcp, RelayStream, and PeerRelay.
Negotiate response FIN support, propagate downstream legacy limitations,
and preserve typed aborts through owned bridges and session-scoped relays.
Update CLI forwarding, generated bindings, documentation, and regressions.

BREAKING CHANGE: malformed post-init frames now fail explicitly, relay I/O
and trailer failures reach callers, and RelayClose actively aborts its
owned operation. Negotiated streams wait for both directions to finish.
Wire additions remain capability-gated; existing SDK Close meanings stay
unchanged. Flag these behavioral changes for release review, particularly
if the merge occurs after September 25, 2026.

Validation: lifecycle regressions, gateway and supervisor tests, Python
and TypeScript tests, and Go SDK CI passed. Full-suite validation is
blocked by missing e2fsprogs for five VM tests. Docker E2E is blocked by
read-only BuildKit storage before fixture creation.

Refs NVIDIA#3056

Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
@varshaprasad96
fix(streams): preserve relay draining and final status
f4257f3 
Preserve legacy relay replies and wait for downstream completion after FIN. Keep established data relays alive across control-session loss, authenticate forwarding E2E requests, and add lifecycle regression coverage.

Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 24, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@varshaprasad96
chore(streams): merge upstream main into lifecycle branch
e4fdd0a 
Combine upstream supervisor shutdown tracking and SSH recovery imports with relay lifecycle handling. Refresh the public API fingerprint while preserving durable schema fingerprints.

Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
2000krysztof
2000krysztof suggested changes Sep 25, 2026
View reviewed changes
Comment thread skills/openshell-cli/SKILL.md
Comment on lines +845 to +850
For protocols that send EOF before receiving a reply, use `forward service`
and keep reading after closing the local socket's write direction. Independent
response FIN requires support throughout the CLI/gateway/supervisor path. See
[forwarding stream lifecycle](https://docs.nvidia.com/openshell/latest/reference/stream-lifecycle.md)
for mixed-version fallback, deadlines, and cancellation behavior.

@2000krysztof 2000krysztof Sep 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add reference/stream-lifecycle.mdx to docs/index.yml? The CLI skill links directly to its published .md endpoint, but Fern navigation is explicitly defined there. Without an entry, the page may be undiscoverable or absent from the published docs. Please also run mise run docs:build:strict to verify the link.

@Ygnas

Ygnas commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

@varshaprasad96 I think there’s one case missing here. abort_relay() only checks active_relays, but the supervisor sends RelayOpenResult(success=true) before starting relay_stream(). If that fails before the gateway claims the relay, it sends RelayClose while the entry is still in pending_relays.

The caller can then wait for the full 10-second timeout, and the entry keeps counting against the pending limit until cleanup.

Would it make sense to handle pending relays here too? Keep the current-session and sandbox checks, remove the matching entry, and send Err(stream_lifecycle::close_status(close)) through its oneshot. A close-before-claim test would cover this case.

This branch has not been deployed

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

Reviewers

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
@sjenning sjenning Awaiting requested review from sjenning sjenning is a code owner
@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner
1 more reviewer
@2000krysztof 2000krysztof 2000krysztof requested changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@varshaprasad96 @Ygnas @2000krysztof