Skip to content

Conversation

jason-ha
Copy link
Contributor

@jason-ha jason-ha commented Aug 29, 2025

Update presence multi-client testing to have tests with a single writer and all others read only.

Add test infrastructure support to have uniquely reader clients in a session. Token provider need only specify write scope during attach.

  Presence with AzureClient
    ✔ announces 'attendeeConnected' when remote client joins session [5 clients, 5 writers]
    ✔ announces 'attendeeDisconnected' when remote client disconnects [5 clients, 5 writers]
    ✔ announces 'attendeeConnected' when remote client joins session [5 clients, 1 writers]
    ✔ announces 'attendeeDisconnected' when remote client disconnects [5 clients, 1 writers]
    ✔ announces 'attendeeConnected' when remote client joins session [20 clients, 20 writers]
    ✔ announces 'attendeeDisconnected' when remote client disconnects [20 clients, 20 writers]
    ✔ announces 'attendeeConnected' when remote client joins session [20 clients, 1 writers]
    - announces 'attendeeDisconnected' when remote client disconnects [20 clients, 1 writers]

@jason-ha jason-ha requested a review from WillieHabi August 29, 2025 00:53
WillieHabi
WillieHabi previously approved these changes Aug 29, 2025
@jason-ha jason-ha force-pushed the presence/test/readonly-client-support branch from 71f55fd to e414272 Compare August 29, 2025 18:33
Base automatically changed from presence/test/import-cleanup to main August 31, 2025 08:52
@jason-ha jason-ha dismissed WillieHabi’s stale review August 31, 2025 08:52

The base branch was changed.

Configure presence multi-client testing to have a single writer and all others read only.

Add test infrastructure support to have uniquely reader clients in a session. Token provider need only specify write scope during attach.
@jason-ha jason-ha force-pushed the presence/test/readonly-client-support branch from e414272 to ca2c5b4 Compare August 31, 2025 08:55
@jason-ha jason-ha marked this pull request as ready for review August 31, 2025 08:55
@Copilot Copilot AI review requested due to automatic review settings August 31, 2025 08:55
@github-actions github-actions bot added area: runtime Runtime related issues base: main PRs targeted against main branch labels Aug 31, 2025
@jason-ha jason-ha enabled auto-merge (squash) August 31, 2025 08:56
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for read-only clients in multi-client presence testing by introducing scope-based access control. The change allows configuring one client as a writer (leader) and all others as read-only participants.

  • Add scope parameters to control client permissions during connection and container creation
  • Refactor token provider to support different scopes for attach vs regular operations
  • Update multi-process test infrastructure to designate first client as writer and others as readers

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
orchestratorUtils.ts Modified to create first client with write permissions and added scope parameters to connection messages
messageTypes.ts Added scope fields to ConnectCommand interface for specifying client permissions
childClient.ts Updated to pass scope parameters through to token provider and removed unused imports
AzureTokenFactory.ts Added attachScopes parameter to support different permissions for container creation
insecureTokenProvider.ts Refactored to consolidate token generation logic and support separate attach scopes

@jason-ha jason-ha disabled auto-merge September 2, 2025 16:15
@jason-ha jason-ha enabled auto-merge (squash) September 2, 2025 16:15
@jason-ha jason-ha requested a review from WillieHabi September 2, 2025 16:36
childErrorPromise,
);
});
for (const writeClients of [numClients, 1]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is just a question -- so here we want to test both with attendee connect and disconnect with all write clients and then with all read clients?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not quite - all write and then 1 write with rest read

@WillieHabi WillieHabi self-requested a review September 2, 2025 16:40
@jason-ha jason-ha disabled auto-merge September 2, 2025 16:55
@jason-ha jason-ha enabled auto-merge (squash) September 2, 2025 16:55
@jason-ha jason-ha merged commit 81df72b into main Sep 2, 2025
37 checks passed
@jason-ha jason-ha deleted the presence/test/readonly-client-support branch September 2, 2025 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: runtime Runtime related issues base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants