Skip to content

test(server): keep relays queued during session setup behind SessionAccepted - #4689

Merged
FrostGod merged 1 commit into
NVIDIA:mainfrom
EmilienM:test/4687-session-accepted-before-relay/EmilienM
Nov 3, 2026
Merged

FrostGod merged 1 commit into
NVIDIA:mainfrom
EmilienM:test/4687-session-accepted-before-relay/EmilienM

Conversation

@EmilienM

@EmilienM EmilienM commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds regression tests for the session setup race where a RelayOpen could reach the supervisor before SessionAccepted and make it drop the session. #4320 fixed the ordering when it reworked session setup; these tests keep it fixed.

Related Issue

Closes #4687
Part of #3528

Changes

  • Two tests in crates/openshell-server/src/supervisor_session.rs, one per config delivery mode (poll and push). Each holds the sandbox mutation guard so setup stops after registration, opens a relay on the session before it's acknowledged, then checks that the stream yields SessionAccepted and then the queued RelayOpen.
  • No production change.

Testing

  • cargo test -p openshell-server --features test-support passes (2364 tests), and the two new tests passed 20 repeated runs.
  • On the commit before feat(config): deliver gateway configuration over the supervisor session #4320, with the test harness copied in, the poll test fails with the RelayOpen as the first frame. Push mode didn't exist there.
  • mise run pre-commit passes.
  • Checks appropriate to the affected code and behavior pass
  • Unit tests added/updated (if applicable)
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@copy-pr-bot

copy-pr-bot Bot commented Oct 18, 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.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

This pull request has had no activity for 14 days and is now marked stale.

@github-actions github-actions Bot added the state:stale Inactive item at risk of automatic closure. label Nov 2, 2026
…ccepted

The supervisor drops a session whose first frame is not SessionAccepted,
and the gateway registers a session before it acknowledges it. NVIDIA#4320
fixed the ordering by routing registry traffic through a private
channel whose forwarder starts only after the ack is queued, but
nothing tested it.

The new tests pause setup in its endpoint-status reset, open a relay on
the registered but unacknowledged session, and check that the stream
yields SessionAccepted and then the queued RelayOpen, in both poll and
push mode. With the test harness ported, the poll test fails on the code
before NVIDIA#4320.

Closes NVIDIA#4687

Signed-off-by: Emilien Macchi <emacchi@redhat.com>
@EmilienM
EmilienM force-pushed the test/4687-session-accepted-before-relay/EmilienM branch from b3b0158 to 91b43ec Compare November 2, 2026 20:46
@FrostGod
FrostGod self-requested a review November 3, 2026 06:45

@FrostGod FrostGod 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.

/lgtm

@github-actions github-actions Bot removed the state:stale Inactive item at risk of automatic closure. label Nov 3, 2026
@FrostGod

FrostGod commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 91b43ec

@FrostGod
FrostGod enabled auto-merge November 3, 2026 16:31
@FrostGod
FrostGod added this pull request to the merge queue Nov 3, 2026
Merged via the queue into NVIDIA:main with commit 53cfadc Nov 3, 2026
82 of 83 checks passed
@FrostGod
FrostGod deleted the test/4687-session-accepted-before-relay/EmilienM branch November 3, 2026 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(server): keep relays queued during supervisor session setup behind SessionAccepted

2 participants