From a9d76bf503239659c613fd7e33d0129f94b2fc2a Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Wed, 29 Apr 2026 04:00:50 +0000 Subject: [PATCH 01/15] created plan of action --- .agent/plans/control-authority-plan.md | 265 +++++++++++++++++++++++++ 1 file changed, 265 insertions(+) create mode 100644 .agent/plans/control-authority-plan.md diff --git a/.agent/plans/control-authority-plan.md b/.agent/plans/control-authority-plan.md new file mode 100644 index 00000000..1a608113 --- /dev/null +++ b/.agent/plans/control-authority-plan.md @@ -0,0 +1,265 @@ +# Control Authority Plan + +## Status: Design / Pre-Implementation + +This document is the working implementation plan for issue `#245`: replace fragile multi-instructor command races with an explicit server-owned control authority model that activities can opt into. + +## Goal + +Provide a shared, reusable control-authority framework so activities with manager-driven runtime commands can designate one instructor as authoritative at a time, while still supporting embedded parent/child inheritance and intentional local override when needed. + +## Problem Summary + +Some activities currently allow multiple instructor surfaces to emit authoritative runtime commands at the same time. In practice this can create feedback loops such as repeated play/pause churn when two instructor views are both trying to drive the same session. + +This is already showing up across more than one activity surface: +- SyncDeck manager runtime control +- embedded Video Sync inside a SyncDeck session + +The current pattern of local flags, echo suppression, and cooldowns is not a strong enough foundation. The fix should move authority into explicit shared session state enforced by the server. + +## Resolved Product Decisions + +- [x] First instructor connected to an authority-enabled session becomes the default owner. +- [x] Non-owning instructors should see disabled controls with clear feedback rather than silent failures. +- [x] `scope: 'inherited'` means parent authority is the default for embedded sessions, but an instructor may explicitly take control of the child session and override the inherited owner. +- [x] Activities need a way to decide which manager commands are authority-gated without forcing shared code to understand every activity protocol. +- [x] Ownership uses only stable instructor instance identity; no instructor display name is required. +- [x] There is no explicit release-control action. Any instructor may take control at any time. +- [x] If the current owner disconnects, ownership remains on that instance id until another instructor takes control. There is no auto-release or auto-claim behavior. +- [x] Simultaneous `take-control` attempts resolve as last-write-wins on the server. +- [x] `scope: 'inherited'` without a controlling parent falls back to local session authority. +- [x] Multiple embedded child sessions may each override inherited authority independently on a per-child-session basis. + +## Proposed Shared Activity Config + +Add a shared optional activity config section: + +```ts +controlAuthority: { + mode: 'single-instructor', + scope: 'session' | 'inherited', + gating: 'all' | 'none' | 'activity', +} +``` + +Interpretation: +- `mode: 'single-instructor'`: the activity uses explicit single-owner control authority +- `scope: 'session'`: the session owns its own authority state +- `scope: 'inherited'`: embedded sessions default to parent authority when a parent authority session exists, but may be locally overridden; without a controlling parent, this falls back to local session authority +- `gating: 'all'`: all manager runtime commands are authority-gated +- `gating: 'none'`: authority state may still exist for inheritance, UI state, or client-side-only coordination, but no shared server-side manager runtime filtering happens +- `gating: 'activity'`: the activity exports a classifier callback used by shared enforcement + +Default behavior: +- no `controlAuthority` config means no shared authority enforcement +- `scope: 'inherited'` only inherits when the session is actually embedded under a controlling parent; otherwise the runtime falls back to local session authority + +## Activity-Owned Hook + +When `gating: 'activity'`, the activity should be able to export a callback from its server/runtime entry that shared enforcement can call: + +```ts +export function isAuthorityGatedManagerMessage(message: unknown): boolean +``` + +Why callback over shared message schema: +- keeps activity protocol ownership inside the activity boundary +- avoids forcing all manager command traffic into one shared wire format before we actually need that +- lets shared code own enforcement while activities own command semantics + +## Shared Runtime Model + +### Resolved Authority + +Shared code should compute runtime authority resolution separate from raw config: + +```ts +interface ResolvedControlAuthority { + configuredScope: 'session' | 'inherited' + effectiveScope: 'session' | 'inherited' + authoritySessionId: string + inheritedFromSessionId: string | null +} +``` + +This lets the runtime distinguish: +- standalone activity using local session authority +- embedded activity currently following parent authority +- embedded activity that has taken local override + +### Session Authority State + +Authority-enabled sessions should persist an owner record similar to: + +```ts +interface SessionControlAuthorityState { + mode: 'single-instructor' + ownerInstanceId: string | null + ownerTakenAt: number | null + overrideInherited: boolean +} +``` + +Notes: +- `ownerInstanceId` should represent a stable instructor browser/tab identity, not a websocket connection id +- first connected instructor auto-claims ownership when no owner exists +- `ownerTakenAt` is display/history metadata only and should not be load-bearing for lifecycle decisions +- inherited authority may be reflected in child status without immediately copying parent ownership into child state unless the child is explicitly overridden +- owner disconnect does not auto-release authority; another instructor may take control explicitly at any time +- on rollout or when normalizing older sessions, an empty authority state should remain valid until the first manager connection auto-claims ownership + +### Instructor Identity Persistence + +The control owner id should survive normal reloads so an instructor does not silently lose ownership on refresh. + +Recommended direction: +- use a durable browser-scoped id stored in `localStorage` +- optionally combine it with a tab-scoped discriminator if the implementation needs distinct instructor identities per tab +- do not rely on `sessionStorage` alone for the owner identity + +## UI Direction + +Use a shared manager-facing control-authority status component that activities can place in their own UI. + +Expected behavior: +- owner sees active status such as `You have control` +- non-owner sees status such as `Another instructor currently has control` +- authority-gated controls render disabled for non-owners +- disabled controls include clear feedback such as `Take control to use playback controls` +- `Take Control` button is available to instructors who do not currently own the effective authority surface +- inherited child sessions can show `Following parent instructor` until locally overridden +- sibling embedded child sessions may each override inherited authority independently + +Placement guidance: +- full-width status card or toolbar section for top-level manager views +- compact banner/chip for embedded manager panels + +## API / WebSocket Direction + +Prefer websocket-first authority updates because this is live session runtime state. + +Client identity: +- each instructor manager tab gets a stable `instructorInstanceId` +- preserve it in `sessionStorage` or equivalent tab-scoped storage + +Authority actions: +- client sends `take-control` +- server updates authority state on the resolved authority session +- server broadcasts authority updates to connected manager clients + +Representative messages: + +```ts +{ type: 'take-control', instructorInstanceId: 'inst_abc123' } +``` + +```ts +{ + type: 'control-authority-updated', + payload: { + mode: 'single-instructor', + ownerInstanceId: 'inst_abc123', + ownerTakenAt: 1777459200000, + authoritySessionId: 'session-123', + overrideInherited: false, + } +} +``` + +Shared enforcement rule: +- if the activity is not authority-enabled, accept commands normally +- if authority is enabled and a manager command is authority-gated, only the effective owner may emit it +- when a non-owner attempts a gated command, server rejects or ignores it and can send explanatory feedback +- if two instructors issue `take-control` at effectively the same time, whichever request is processed last becomes the owner and the resulting authority update is broadcast to all connected instructors + +## Implementation Checklist + +### 1. Shared config and typing + +- [ ] Add `controlAuthority` to shared activity config types. +- [ ] Add schema validation for `mode`, `scope`, and `gating`. +- [ ] Document fallback semantics for `scope: 'inherited'` without a controlling parent. +- [ ] Add runtime helpers to resolve effective authority scope and authority session id. + +### 2. Shared session model + +- [ ] Define shared authority session-state shape. +- [ ] Add normalizer support so authority-enabled sessions recover safely after restart. +- [ ] Define how embedded child sessions discover parent authority context. +- [ ] Define local override semantics for inherited child sessions. +- [ ] Define the persisted-empty-state behavior for older in-flight sessions so first manager connect auto-claims ownership intentionally. + +### 3. Shared server enforcement + +- [ ] Add shared instructor instance identity handling for manager websocket/API traffic. +- [ ] Auto-assign first connected instructor as owner when no owner exists. +- [ ] Add `take-control` server action and broadcast path. +- [ ] Add generic authority checks before processing manager runtime commands. +- [ ] Wire `gating: 'all' | 'none' | 'activity'` into enforcement. +- [ ] Add activity callback lookup/invocation for `gating: 'activity'`. +- [ ] Return explicit non-owner feedback for gated command attempts while keeping the server state unchanged. +- [ ] Document that owner disconnect does not auto-release authority and that explicit takeover is the only reassignment path. +- [ ] Document last-write-wins behavior for concurrent `take-control` requests. + +### 4. Shared client plumbing + +- [ ] Generate and persist stable `instructorInstanceId` values for manager tabs. +- [ ] Subscribe authority-enabled manager views to live authority status updates. +- [ ] Expose authority state to activity manager UIs through a shared hook/helper. +- [ ] Provide shared disabled-state helpers and feedback text for gated controls. + +### 5. Shared UI primitives + +- [ ] Build a reusable control-authority status component. +- [ ] Support at least one compact variant for embedded manager surfaces. +- [ ] Include accessible disabled-state explanations and button labeling. +- [ ] Ensure live authority updates are announced appropriately for assistive tech where needed. + +### 6. Activity adoption + +- [ ] Add `controlAuthority` config to SyncDeck. +- [ ] Decide SyncDeck gating mode. Current expectation: `gating: 'activity'`. +- [ ] Add `controlAuthority` config to Video Sync. +- [ ] Decide Video Sync gating mode. Current expectation: `gating: 'activity'` or `all`, depending on final command surface. +- [ ] Implement activity-owned command classifiers where `gating: 'activity'` is used. +- [ ] Ensure embedded Video Sync defaults to inherited authority when launched under SyncDeck. +- [ ] Ensure embedded Video Sync can be locally overridden by explicit `Take Control`. + +### 7. Validation + +- [ ] Add unit tests for config validation and authority resolution helpers. +- [ ] Add server tests for first-instructor auto-ownership. +- [ ] Add server tests for `take-control` handoff. +- [ ] Add server tests for inherited authority resolution and local child override. +- [ ] Add server tests for non-owner gated-command rejection. +- [ ] Add server tests covering older sessions with empty authority state that auto-initialize on first manager connect. +- [ ] Add client tests for disabled controls and authority status messaging. +- [ ] Add activity-specific tests for SyncDeck command classification. +- [ ] Add activity-specific tests for Video Sync command classification. +- [ ] Add browser-level E2E coverage for two instructor views on the same authority-enabled session: first instructor default owner, second instructor disabled, live takeover, and disabled-state flip after handoff. +- [ ] Add browser-level E2E coverage for embedded inherited authority plus local child override behavior. +- [ ] Run scope-appropriate repo validation, likely `npm test`, and include `npm run test:e2e` for the multi-instructor manager and embedded authority scenarios when the harness can support them. + +## Suggested Rollout Order + +1. Land shared config/types/schema support. +2. Land shared authority session model and runtime resolution. +3. Land shared server enforcement with `gating: 'all' | 'none'`. +4. Add activity callback support for `gating: 'activity'`. +5. Land shared client identity/status plumbing and reusable UI. +6. Adopt in Video Sync and SyncDeck. +7. Add embedded inheritance and child override coverage. +8. Validate and update docs/knowledge notes with implementation discoveries. + +## Risks To Watch + +- Shared code accidentally becoming SyncDeck-specific instead of remaining generic. +- Embedded override semantics becoming confusing if parent and child ownership are not clearly labeled in UI. +- Reconnect behavior causing accidental ownership churn if instance identity is not stable across normal refreshes. +- Activity callback interfaces drifting across activities without a well-defined contract. +- Server/client disagreement about which commands are gated if any activity duplicates logic on both sides. + +## Validation Notes + +Because this feature changes live runtime coordination and embedded activity behavior, expect both server-level and browser-visible risk. If sandbox limits block full browser verification, keep `npm test` as the minimum merge gate, use `npm run test:codex` if port-binding is the blocker, and record any environment limitation in the implementation notes. From 00256047a4cbac608dfb5cb0ab76c3a80b13357c Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Wed, 29 Apr 2026 04:11:03 +0000 Subject: [PATCH 02/15] Add shared control authority foundation --- server/activityConfigSchema.test.ts | 84 +++++++ server/controlAuthority.test.ts | 371 ++++++++++++++++++++++++++++ server/controlAuthority.ts | 199 +++++++++++++++ types/activity.ts | 22 ++ types/activityConfigSchema.ts | 33 +++ 5 files changed, 709 insertions(+) create mode 100644 server/controlAuthority.test.ts create mode 100644 server/controlAuthority.ts diff --git a/server/activityConfigSchema.test.ts b/server/activityConfigSchema.test.ts index 24e51ac1..9f13660a 100644 --- a/server/activityConfigSchema.test.ts +++ b/server/activityConfigSchema.test.ts @@ -75,6 +75,11 @@ void test('parseActivityConfig accepts valid shared contracts', () => { embeddedRuntime: { instructorGated: 'runtime', }, + controlAuthority: { + mode: 'single-instructor', + scope: 'inherited', + gating: 'activity', + }, reportEndpoint: '/api/syncdeck/s1/report', utilMode: true, waitingRoom: { @@ -119,6 +124,11 @@ void test('parseActivityConfig accepts valid shared contracts', () => { assert.deepEqual(parsed.createSessionBootstrap?.selectedOptionsToSessionData, ['presentationUrl']) assert.equal(parsed.manageDashboard?.customPersistentLinkBuilder, true) assert.equal(parsed.embeddedRuntime?.instructorGated, 'runtime') + assert.deepEqual(parsed.controlAuthority, { + mode: 'single-instructor', + scope: 'inherited', + gating: 'activity', + }) assert.equal(parsed.reportEndpoint, '/api/syncdeck/s1/report') assert.deepEqual(parsed.utilities, [ { @@ -414,6 +424,77 @@ void test('parseActivityConfig rejects invalid shared contract enums and shapes' /embeddedRuntime.*instructorGated.*runtime.*waiting-room/, ) + assert.throws( + () => + parseActivityConfig( + { + id: 'bad-control-mode', + name: 'BadControlMode', + description: 'desc', + color: 'gray', + standaloneEntry: { + enabled: true, + supportsDirectPath: true, + supportsPermalink: true, + showOnHome: true, + }, + controlAuthority: { + mode: 'multi-instructor', + }, + }, + 'bad-control-mode', + ), + /controlAuthority.*mode.*single-instructor/, + ) + + assert.throws( + () => + parseActivityConfig( + { + id: 'bad-control-scope', + name: 'BadControlScope', + description: 'desc', + color: 'gray', + standaloneEntry: { + enabled: true, + supportsDirectPath: true, + supportsPermalink: true, + showOnHome: true, + }, + controlAuthority: { + mode: 'single-instructor', + scope: 'parent', + }, + }, + 'bad-control-scope', + ), + /controlAuthority.*scope.*session.*inherited/, + ) + + assert.throws( + () => + parseActivityConfig( + { + id: 'bad-control-gating', + name: 'BadControlGating', + description: 'desc', + color: 'gray', + standaloneEntry: { + enabled: true, + supportsDirectPath: true, + supportsPermalink: true, + showOnHome: true, + }, + controlAuthority: { + mode: 'single-instructor', + gating: 'server-only', + }, + }, + 'bad-control-gating', + ), + /controlAuthority.*gating.*all.*none.*activity/, + ) + assert.throws( () => parseActivityConfig( @@ -451,6 +532,7 @@ void test('parseActivityConfig removes optional keys when input provides null', }, title: null, deepLinkOptions: null, + controlAuthority: null, reportEndpoint: null, }, 'null-config', @@ -458,8 +540,10 @@ void test('parseActivityConfig removes optional keys when input provides null', assert.equal(parsed.title, undefined) assert.equal(parsed.deepLinkOptions, undefined) + assert.equal(parsed.controlAuthority, undefined) assert.equal(parsed.reportEndpoint, undefined) assert.equal('title' in parsed, false) assert.equal('deepLinkOptions' in parsed, false) + assert.equal('controlAuthority' in parsed, false) assert.equal('reportEndpoint' in parsed, false) }) diff --git a/server/controlAuthority.test.ts b/server/controlAuthority.test.ts new file mode 100644 index 00000000..0aad1382 --- /dev/null +++ b/server/controlAuthority.test.ts @@ -0,0 +1,371 @@ +import assert from 'node:assert/strict' +import test from 'node:test' +import type { ActivityConfig } from '../types/activity.js' +import { + activityUsesControlAuthority, + claimSessionControlAuthority, + getEmbeddedParentSessionId, + getResolvedControlAuthorityOwnerInstanceId, + getSessionControlAuthorityState, + isInstructorControlOwner, + normalizeInstructorInstanceId, + normalizeSessionControlAuthorityState, + resolveControlAuthority, + setSessionControlAuthorityState, + shouldAutoClaimControlAuthority, +} from './controlAuthority.js' + +function buildActivityConfig(overrides: Partial = {}): ActivityConfig { + return { + id: 'activity', + name: 'Activity', + description: 'desc', + color: 'blue', + standaloneEntry: { + enabled: false, + supportsDirectPath: false, + supportsPermalink: false, + showOnHome: false, + }, + ...overrides, + } +} + +void test('normalizeSessionControlAuthorityState normalizes sparse and invalid input', () => { + assert.deepEqual(normalizeSessionControlAuthorityState(null), { + mode: 'single-instructor', + ownerInstanceId: null, + ownerTakenAt: null, + overrideInherited: false, + }) + + assert.deepEqual( + normalizeSessionControlAuthorityState({ + ownerInstanceId: ' inst-1 ', + ownerTakenAt: 123, + overrideInherited: true, + }), + { + mode: 'single-instructor', + ownerInstanceId: 'inst-1', + ownerTakenAt: 123, + overrideInherited: true, + }, + ) +}) + +void test('normalizeInstructorInstanceId trims usable instance ids only', () => { + assert.equal(normalizeInstructorInstanceId(' inst-1 '), 'inst-1') + assert.equal(normalizeInstructorInstanceId(' '), null) + assert.equal(normalizeInstructorInstanceId(null), null) +}) + +void test('getSessionControlAuthorityState reads normalized state from session data', () => { + assert.deepEqual( + getSessionControlAuthorityState({ + data: { + controlAuthority: { + ownerInstanceId: 'owner-1', + ownerTakenAt: 456, + overrideInherited: false, + }, + }, + }), + { + mode: 'single-instructor', + ownerInstanceId: 'owner-1', + ownerTakenAt: 456, + overrideInherited: false, + }, + ) +}) + +void test('setSessionControlAuthorityState persists normalized authority state into session data', () => { + const session = { data: {} } + const storedState = setSessionControlAuthorityState(session, { + mode: 'single-instructor', + ownerInstanceId: 'owner-1', + ownerTakenAt: 789, + overrideInherited: true, + }) + + assert.deepEqual(storedState, { + mode: 'single-instructor', + ownerInstanceId: 'owner-1', + ownerTakenAt: 789, + overrideInherited: true, + }) + assert.deepEqual((session.data as Record).controlAuthority, storedState) +}) + +void test('claimSessionControlAuthority stores the active owner and timestamp', () => { + const session = { data: {} } + + const claimedState = claimSessionControlAuthority({ + session, + instructorInstanceId: ' owner-2 ', + takenAt: 999, + }) + + assert.deepEqual(claimedState, { + mode: 'single-instructor', + ownerInstanceId: 'owner-2', + ownerTakenAt: 999, + overrideInherited: false, + }) +}) + +void test('getEmbeddedParentSessionId returns trimmed embedded parent ids only', () => { + assert.equal(getEmbeddedParentSessionId(null), null) + assert.equal(getEmbeddedParentSessionId({ id: 'child', data: {} }), null) + assert.equal( + getEmbeddedParentSessionId({ + id: 'child', + data: { embeddedParentSessionId: ' parent-1 ' }, + }), + 'parent-1', + ) +}) + +void test('activityUsesControlAuthority only enables configured single-instructor activities', () => { + assert.equal(activityUsesControlAuthority(buildActivityConfig()), false) + assert.equal( + activityUsesControlAuthority( + buildActivityConfig({ + controlAuthority: { + mode: 'single-instructor', + }, + }), + ), + true, + ) +}) + +void test('resolveControlAuthority uses local session authority by default', () => { + const activityConfig = buildActivityConfig({ + controlAuthority: { + mode: 'single-instructor', + scope: 'session', + }, + }) + + assert.deepEqual( + resolveControlAuthority({ + session: { id: 'session-1', data: {} }, + activityConfig, + }), + { + mode: 'single-instructor', + configuredScope: 'session', + effectiveScope: 'session', + authoritySessionId: 'session-1', + inheritedFromSessionId: null, + }, + ) +}) + +void test('resolveControlAuthority inherits parent authority for embedded sessions when configured', () => { + const childConfig = buildActivityConfig({ + controlAuthority: { + mode: 'single-instructor', + scope: 'inherited', + }, + }) + const parentConfig = buildActivityConfig({ + id: 'syncdeck', + name: 'SyncDeck', + controlAuthority: { + mode: 'single-instructor', + scope: 'session', + }, + }) + + assert.deepEqual( + resolveControlAuthority({ + session: { + id: 'child-1', + data: { embeddedParentSessionId: 'parent-1' }, + }, + activityConfig: childConfig, + parentSession: { + id: 'parent-1', + data: {}, + }, + parentActivityConfig: parentConfig, + }), + { + mode: 'single-instructor', + configuredScope: 'inherited', + effectiveScope: 'inherited', + authoritySessionId: 'parent-1', + inheritedFromSessionId: 'parent-1', + }, + ) +}) + +void test('resolveControlAuthority falls back to local session authority without a controlling parent', () => { + const childConfig = buildActivityConfig({ + controlAuthority: { + mode: 'single-instructor', + scope: 'inherited', + }, + }) + + assert.deepEqual( + resolveControlAuthority({ + session: { + id: 'child-1', + data: { embeddedParentSessionId: 'missing-parent' }, + }, + activityConfig: childConfig, + parentSession: null, + parentActivityConfig: null, + }), + { + mode: 'single-instructor', + configuredScope: 'inherited', + effectiveScope: 'session', + authoritySessionId: 'child-1', + inheritedFromSessionId: null, + }, + ) +}) + +void test('resolveControlAuthority falls back to local session authority when inherited control is locally overridden', () => { + const childConfig = buildActivityConfig({ + controlAuthority: { + mode: 'single-instructor', + scope: 'inherited', + }, + }) + const parentConfig = buildActivityConfig({ + id: 'syncdeck', + name: 'SyncDeck', + controlAuthority: { + mode: 'single-instructor', + }, + }) + + assert.deepEqual( + resolveControlAuthority({ + session: { + id: 'child-1', + data: { + embeddedParentSessionId: 'parent-1', + controlAuthority: { + overrideInherited: true, + }, + }, + }, + activityConfig: childConfig, + parentSession: { + id: 'parent-1', + data: {}, + }, + parentActivityConfig: parentConfig, + }), + { + mode: 'single-instructor', + configuredScope: 'inherited', + effectiveScope: 'session', + authoritySessionId: 'child-1', + inheritedFromSessionId: null, + }, + ) +}) + +void test('resolveControlAuthority returns null for activities without control authority configured', () => { + assert.equal( + resolveControlAuthority({ + session: { id: 'session-1', data: {} }, + activityConfig: buildActivityConfig(), + }), + null, + ) +}) + +void test('getResolvedControlAuthorityOwnerInstanceId reads from the effective authority session', () => { + const inheritedResolution = { + mode: 'single-instructor', + configuredScope: 'inherited', + effectiveScope: 'inherited', + authoritySessionId: 'parent-1', + inheritedFromSessionId: 'parent-1', + } as const + + assert.equal( + getResolvedControlAuthorityOwnerInstanceId({ + resolvedAuthority: inheritedResolution, + session: { data: { controlAuthority: { ownerInstanceId: 'child-owner' } } }, + parentSession: { data: { controlAuthority: { ownerInstanceId: 'parent-owner' } } }, + }), + 'parent-owner', + ) + + assert.equal( + getResolvedControlAuthorityOwnerInstanceId({ + resolvedAuthority: { + mode: 'single-instructor', + configuredScope: 'session', + effectiveScope: 'session', + authoritySessionId: 'child-1', + inheritedFromSessionId: null, + }, + session: { data: { controlAuthority: { ownerInstanceId: 'child-owner' } } }, + parentSession: { data: { controlAuthority: { ownerInstanceId: 'parent-owner' } } }, + }), + 'child-owner', + ) +}) + +void test('isInstructorControlOwner compares the instructor instance id against the effective owner', () => { + const resolvedAuthority = { + mode: 'single-instructor', + configuredScope: 'session', + effectiveScope: 'session', + authoritySessionId: 'session-1', + inheritedFromSessionId: null, + } as const + + assert.equal( + isInstructorControlOwner({ + resolvedAuthority, + session: { data: { controlAuthority: { ownerInstanceId: 'owner-1' } } }, + instructorInstanceId: 'owner-1', + }), + true, + ) + assert.equal( + isInstructorControlOwner({ + resolvedAuthority, + session: { data: { controlAuthority: { ownerInstanceId: 'owner-1' } } }, + instructorInstanceId: 'owner-2', + }), + false, + ) +}) + +void test('shouldAutoClaimControlAuthority is true only when the effective owner is empty', () => { + const resolvedAuthority = { + mode: 'single-instructor', + configuredScope: 'session', + effectiveScope: 'session', + authoritySessionId: 'session-1', + inheritedFromSessionId: null, + } as const + + assert.equal( + shouldAutoClaimControlAuthority({ + resolvedAuthority, + session: { data: {} }, + }), + true, + ) + assert.equal( + shouldAutoClaimControlAuthority({ + resolvedAuthority, + session: { data: { controlAuthority: { ownerInstanceId: 'owner-1' } } }, + }), + false, + ) +}) diff --git a/server/controlAuthority.ts b/server/controlAuthority.ts new file mode 100644 index 00000000..d05f3deb --- /dev/null +++ b/server/controlAuthority.ts @@ -0,0 +1,199 @@ +import type { + ActivityConfig, + ResolvedControlAuthority, + SessionControlAuthorityState, +} from '../types/activity.js' +import type { SessionRecord } from './core/sessions.js' + +interface ControlAuthoritySessionLike { + id: string + data?: unknown +} + +function isPlainObject(value: unknown): value is Record { + return value != null && typeof value === 'object' && !Array.isArray(value) +} + +function normalizeNonEmptyString(value: unknown): string | null { + if (typeof value !== 'string') { + return null + } + + const trimmed = value.trim() + return trimmed.length > 0 ? trimmed : null +} + +function normalizeFiniteNumber(value: unknown): number | null { + return typeof value === 'number' && Number.isFinite(value) ? value : null +} + +function ensureSessionDataRecord(session: Pick): Record { + if (isPlainObject(session.data)) { + return session.data + } + + const nextData: Record = {} + ;(session as SessionRecord).data = nextData + return nextData +} + +export function normalizeInstructorInstanceId(value: unknown): string | null { + return normalizeNonEmptyString(value) +} + +export function normalizeSessionControlAuthorityState(value: unknown): SessionControlAuthorityState { + const source = isPlainObject(value) ? value : {} + + return { + mode: 'single-instructor', + ownerInstanceId: normalizeNonEmptyString(source.ownerInstanceId), + ownerTakenAt: normalizeFiniteNumber(source.ownerTakenAt), + overrideInherited: source.overrideInherited === true, + } +} + +export function getSessionControlAuthorityState(session: Pick): SessionControlAuthorityState { + const data = isPlainObject(session.data) ? session.data : {} + return normalizeSessionControlAuthorityState(data.controlAuthority) +} + +export function setSessionControlAuthorityState( + session: Pick, + state: SessionControlAuthorityState, +): SessionControlAuthorityState { + const data = ensureSessionDataRecord(session) + data.controlAuthority = { + mode: 'single-instructor', + ownerInstanceId: state.ownerInstanceId, + ownerTakenAt: state.ownerTakenAt, + overrideInherited: state.overrideInherited, + } satisfies SessionControlAuthorityState + + return getSessionControlAuthorityState(session) +} + +export function claimSessionControlAuthority(params: { + session: Pick + instructorInstanceId: string + takenAt?: number + overrideInherited?: boolean +}): SessionControlAuthorityState { + const normalizedInstructorInstanceId = normalizeInstructorInstanceId(params.instructorInstanceId) + if (!normalizedInstructorInstanceId) { + throw new Error('claimSessionControlAuthority requires a non-empty instructorInstanceId') + } + + return setSessionControlAuthorityState(params.session, { + mode: 'single-instructor', + ownerInstanceId: normalizedInstructorInstanceId, + ownerTakenAt: normalizeFiniteNumber(params.takenAt) ?? Date.now(), + overrideInherited: params.overrideInherited === true, + }) +} + +export function getEmbeddedParentSessionId(session: ControlAuthoritySessionLike | null | undefined): string | null { + const data = isPlainObject(session?.data) ? session.data : {} + return normalizeNonEmptyString(data.embeddedParentSessionId) +} + +function getConfiguredControlAuthorityScope(activityConfig: ActivityConfig | null | undefined): 'session' | 'inherited' { + return activityConfig?.controlAuthority?.scope === 'inherited' ? 'inherited' : 'session' +} + +export function activityUsesControlAuthority(activityConfig: ActivityConfig | null | undefined): boolean { + return activityConfig?.controlAuthority?.mode === 'single-instructor' +} + +export function resolveControlAuthority(params: { + session: ControlAuthoritySessionLike + activityConfig: ActivityConfig | null | undefined + parentSession?: ControlAuthoritySessionLike | null + parentActivityConfig?: ActivityConfig | null | undefined +}): ResolvedControlAuthority | null { + const { session, activityConfig, parentSession = null, parentActivityConfig = null } = params + if (!activityUsesControlAuthority(activityConfig)) { + return null + } + + const configuredScope = getConfiguredControlAuthorityScope(activityConfig) + const localState = getSessionControlAuthorityState(session as Pick) + const embeddedParentSessionId = getEmbeddedParentSessionId(session) + const canInherit = + configuredScope === 'inherited' + && localState.overrideInherited !== true + && embeddedParentSessionId != null + && parentSession?.id === embeddedParentSessionId + && activityUsesControlAuthority(parentActivityConfig) + + if (canInherit) { + return { + mode: 'single-instructor', + configuredScope, + effectiveScope: 'inherited', + authoritySessionId: parentSession.id, + inheritedFromSessionId: parentSession.id, + } + } + + return { + mode: 'single-instructor', + configuredScope, + effectiveScope: 'session', + authoritySessionId: session.id, + inheritedFromSessionId: null, + } +} + +export function getResolvedControlAuthorityOwnerInstanceId(params: { + resolvedAuthority: ResolvedControlAuthority | null + session: Pick + parentSession?: Pick | null +}): string | null { + const { resolvedAuthority, session, parentSession = null } = params + if (!resolvedAuthority) { + return null + } + + if (resolvedAuthority.effectiveScope === 'inherited') { + return parentSession ? getSessionControlAuthorityState(parentSession).ownerInstanceId : null + } + + return getSessionControlAuthorityState(session).ownerInstanceId +} + +export function isInstructorControlOwner(params: { + resolvedAuthority: ResolvedControlAuthority | null + session: Pick + instructorInstanceId: string | null | undefined + parentSession?: Pick | null +}): boolean { + const normalizedInstructorInstanceId = normalizeInstructorInstanceId(params.instructorInstanceId) + if (!normalizedInstructorInstanceId) { + return false + } + + const ownerInstanceId = getResolvedControlAuthorityOwnerInstanceId(params) + return ownerInstanceId === normalizedInstructorInstanceId +} + +export function shouldAutoClaimControlAuthority(params: { + resolvedAuthority: ResolvedControlAuthority | null + session: Pick + parentSession?: Pick | null +}): boolean { + return getResolvedControlAuthorityOwnerInstanceId(params) == null +} + +export default { + activityUsesControlAuthority, + claimSessionControlAuthority, + getEmbeddedParentSessionId, + getResolvedControlAuthorityOwnerInstanceId, + getSessionControlAuthorityState, + isInstructorControlOwner, + normalizeInstructorInstanceId, + normalizeSessionControlAuthorityState, + resolveControlAuthority, + setSessionControlAuthorityState, + shouldAutoClaimControlAuthority, +} diff --git a/types/activity.ts b/types/activity.ts index 288bf523..a573e993 100644 --- a/types/activity.ts +++ b/types/activity.ts @@ -94,6 +94,27 @@ export interface ActivityEmbeddedRuntimeConfig { instructorGated?: 'runtime' | 'waiting-room' } +export interface ActivityControlAuthorityConfig { + mode: 'single-instructor' + scope?: 'session' | 'inherited' + gating?: 'all' | 'none' | 'activity' +} + +export interface SessionControlAuthorityState { + mode: 'single-instructor' + ownerInstanceId: string | null + ownerTakenAt: number | null + overrideInherited: boolean +} + +export interface ResolvedControlAuthority { + mode: 'single-instructor' + configuredScope: 'session' | 'inherited' + effectiveScope: 'session' | 'inherited' + authoritySessionId: string + inheritedFromSessionId: string | null +} + export type ActivityReportScope = 'activity-session' | 'student-cross-activity' | 'session-summary' export interface ActivityReportStudentRef { @@ -200,6 +221,7 @@ export interface ActivityConfig { expandShell?: boolean } embeddedRuntime?: ActivityEmbeddedRuntimeConfig + controlAuthority?: ActivityControlAuthorityConfig reportEndpoint?: string waitingRoom?: ActivityWaitingRoomConfig isDev?: boolean diff --git a/types/activityConfigSchema.ts b/types/activityConfigSchema.ts index f19dff29..112d2c76 100644 --- a/types/activityConfigSchema.ts +++ b/types/activityConfigSchema.ts @@ -1,5 +1,6 @@ import type { ActivityConfig, + ActivityControlAuthorityConfig, ActivityCreateSessionBootstrapConfig, ActivityCreateSessionBootstrapSessionStorageEntry, ActivityDeepLinkOption, @@ -305,6 +306,36 @@ function parseEmbeddedRuntime(raw: unknown, context: string): ActivityConfig['em } } +function parseControlAuthority(raw: unknown, context: string): ActivityControlAuthorityConfig | undefined { + if (raw == null) { + return undefined + } + if (!isRecord(raw)) { + throw new Error(`${context}: "controlAuthority" must be an object when provided`) + } + + const mode = raw.mode + if (mode !== 'single-instructor') { + throw new Error(`${context}.controlAuthority: "mode" must be "single-instructor"`) + } + + const scopeRaw = raw.scope + if (scopeRaw !== undefined && scopeRaw !== null && scopeRaw !== 'session' && scopeRaw !== 'inherited') { + throw new Error(`${context}.controlAuthority: "scope" must be "session" or "inherited" when provided`) + } + + const gatingRaw = raw.gating + if (gatingRaw !== undefined && gatingRaw !== null && gatingRaw !== 'all' && gatingRaw !== 'none' && gatingRaw !== 'activity') { + throw new Error(`${context}.controlAuthority: "gating" must be "all", "none", or "activity" when provided`) + } + + return { + mode: 'single-instructor', + ...(scopeRaw === 'session' || scopeRaw === 'inherited' ? { scope: scopeRaw } : {}), + ...(gatingRaw === 'all' || gatingRaw === 'none' || gatingRaw === 'activity' ? { gating: gatingRaw } : {}), + } +} + function parseUtilities(raw: unknown, context: string): ActivityUtility[] | undefined { if (raw == null) { return undefined @@ -538,6 +569,7 @@ export function parseActivityConfig(rawConfig: unknown, sourceLabel = 'activity. const manageDashboard = parseManageDashboard(rawConfig.manageDashboard, context) const manageLayout = parseManageLayout(rawConfig.manageLayout, context) const embeddedRuntime = parseEmbeddedRuntime(rawConfig.embeddedRuntime, context) + const controlAuthority = parseControlAuthority(rawConfig.controlAuthority, context) const reportEndpoint = readOptionalString(rawConfig, 'reportEndpoint', context) const waitingRoom = parseWaitingRoom(rawConfig.waitingRoom, context) @@ -557,6 +589,7 @@ export function parseActivityConfig(rawConfig: unknown, sourceLabel = 'activity. assignOptionalField(parsed, 'manageDashboard', manageDashboard) assignOptionalField(parsed, 'manageLayout', manageLayout) assignOptionalField(parsed, 'embeddedRuntime', embeddedRuntime) + assignOptionalField(parsed, 'controlAuthority', controlAuthority) assignOptionalField(parsed, 'reportEndpoint', reportEndpoint) assignOptionalField(parsed, 'waitingRoom', waitingRoom) From a2ee795443c5c4b7be7bd0dd47734daa23982588 Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Wed, 29 Apr 2026 04:18:25 +0000 Subject: [PATCH 03/15] Add video sync control identity plumbing --- .../client/manager/VideoSyncManager.test.ts | 17 ++- .../client/manager/VideoSyncManager.tsx | 28 ++++- activities/video-sync/server/routes.test.ts | 100 ++++++++++++++++++ activities/video-sync/server/routes.ts | 51 +++++++++ .../common/instructorControlIdentity.test.ts | 77 ++++++++++++++ .../common/instructorControlIdentity.ts | 56 ++++++++++ 6 files changed, 323 insertions(+), 6 deletions(-) create mode 100644 client/src/components/common/instructorControlIdentity.test.ts create mode 100644 client/src/components/common/instructorControlIdentity.ts diff --git a/activities/video-sync/client/manager/VideoSyncManager.test.ts b/activities/video-sync/client/manager/VideoSyncManager.test.ts index a10c29f2..d67abae6 100644 --- a/activities/video-sync/client/manager/VideoSyncManager.test.ts +++ b/activities/video-sync/client/manager/VideoSyncManager.test.ts @@ -208,13 +208,28 @@ void test('shouldFetchEmbeddedBootstrapSourceUrl only fetches for embedded child void test('buildManagerWsUrl omits instructor credentials from the websocket URL', () => { assert.equal( buildManagerWsUrl({ + instructorInstanceId: 'inst-123', sessionId: 'session-123', location: { protocol: 'https:', host: 'bits.example.test', }, }), - 'wss://bits.example.test/ws/video-sync?sessionId=session-123&role=instructor', + 'wss://bits.example.test/ws/video-sync?sessionId=session-123&role=instructor&instructorInstanceId=inst-123', + ) +}) + +void test('buildManagerWsUrl requires an instructor instance id', () => { + assert.equal( + buildManagerWsUrl({ + instructorInstanceId: null, + sessionId: 'session-123', + location: { + protocol: 'https:', + host: 'bits.example.test', + }, + }), + null, ) }) diff --git a/activities/video-sync/client/manager/VideoSyncManager.tsx b/activities/video-sync/client/manager/VideoSyncManager.tsx index f9843775..f12fd1ef 100644 --- a/activities/video-sync/client/manager/VideoSyncManager.tsx +++ b/activities/video-sync/client/manager/VideoSyncManager.tsx @@ -1,5 +1,6 @@ import SessionHeader from '@src/components/common/SessionHeader' import { fetchEmbeddedLaunchSelectedOptions } from '@src/components/common/embeddedLaunchBootstrap' +import { resolveOrCreateInstructorControlInstanceId } from '@src/components/common/instructorControlIdentity' import { consumeCreateSessionBootstrapPayload } from '@src/components/common/manageDashboardUtils' import { isEmbeddedChildSessionId } from '@src/components/common/sessionHeaderUtils' import Button from '@src/components/ui/Button' @@ -180,14 +181,15 @@ export function resolveBootstrapInstructorPasscode(params: { export function buildManagerWsUrl(params: { sessionId: string | null | undefined + instructorInstanceId: string | null | undefined location: Pick | null | undefined }): string | null { - if (!params.sessionId || params.location == null) { + if (!params.sessionId || !params.instructorInstanceId || params.location == null) { return null } const protocol = params.location.protocol === 'https:' ? 'wss:' : 'ws:' - return `${protocol}//${params.location.host}/ws/video-sync?sessionId=${encodeURIComponent(params.sessionId)}&role=instructor` + return `${protocol}//${params.location.host}/ws/video-sync?sessionId=${encodeURIComponent(params.sessionId)}&role=instructor&instructorInstanceId=${encodeURIComponent(params.instructorInstanceId)}` } export function createManagerWsAuthMessage(instructorPasscode: string | null | undefined): string | null { @@ -344,6 +346,19 @@ export default function VideoSyncManager() { const [autoStartStatus, setAutoStartStatus] = useState('idle') const [embeddedBootstrapSourceUrl, setEmbeddedBootstrapSourceUrl] = useState(null) const [persistentRecoverySourceUrl, setPersistentRecoverySourceUrl] = useState(null) + const instructorInstanceId = useMemo(() => { + if (typeof window === 'undefined') { + return null + } + + return resolveOrCreateInstructorControlInstanceId( + { + localStorage: window.localStorage, + sessionStorage: window.sessionStorage, + }, + () => window.crypto.randomUUID(), + ) + }, []) const playerContainerRef = useRef(null) const playerRef = useRef(null) @@ -453,6 +468,7 @@ export default function VideoSyncManager() { payload.positionSec = clampNumber(options.positionSec) } payload.instructorPasscode = instructorPasscode + payload.instructorInstanceId = instructorInstanceId try { const response = await fetch(`/api/video-sync/${sessionId}/command`, { @@ -485,7 +501,7 @@ export default function VideoSyncManager() { } return false } - }, [instructorPasscode, sessionId]) + }, [instructorInstanceId, instructorPasscode, sessionId]) const flushManagerPlaybackIntent = useCallback(async (): Promise => { clearPlaybackCommandFlushTimer() @@ -604,10 +620,11 @@ export default function VideoSyncManager() { const buildWsUrl = useCallback(() => { if (typeof window === 'undefined') return null return buildManagerWsUrl({ + instructorInstanceId, sessionId, location: window.location, }) - }, [sessionId]) + }, [instructorInstanceId, sessionId]) useEffect(() => { if (!sessionId || typeof window === 'undefined') { @@ -883,6 +900,7 @@ export default function VideoSyncManager() { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ instructorPasscode, + instructorInstanceId, sourceUrl: sourceUrlValue, stopSec: stopSecValue, }), @@ -907,7 +925,7 @@ export default function VideoSyncManager() { setErrorMessage(message) return false } - }, [applyManagerStateUpdate, instructorPasscode, isPasscodeReady, sessionId]) + }, [applyManagerStateUpdate, instructorInstanceId, instructorPasscode, isPasscodeReady, sessionId]) const saveConfig = useCallback(async (): Promise => { await saveConfigWithValues(sourceUrlInput, hasStopTime, stopSecInput) diff --git a/activities/video-sync/server/routes.test.ts b/activities/video-sync/server/routes.test.ts index 9ca2c6d8..69f8c203 100644 --- a/activities/video-sync/server/routes.test.ts +++ b/activities/video-sync/server/routes.test.ts @@ -776,6 +776,106 @@ void test('session get route returns projected playback without persisting ordin } }) +void test('take control claims video-sync session control authority for the requesting instructor instance', async () => { + const app = createMockApp() + const ws = createMockWs() as unknown as WsRouter + const storeState = createSessionStore({ s1: createVideoSyncSession('s1') }) + + setupVideoSyncRoutes(app, storeState.sessions, ws) + + const handler = app.handlers.post['/api/video-sync/:sessionId/control-authority/take'] + assert.equal(typeof handler, 'function') + + const res = createResponse() + await handler?.( + { + params: { sessionId: 's1' }, + body: { + instructorPasscode: TEST_INSTRUCTOR_PASSCODE, + instructorInstanceId: 'inst-1', + }, + }, + res, + ) + + assert.equal(res.statusCode, 200) + assert.deepEqual(res.body, { + success: true, + controlAuthority: { + mode: 'single-instructor', + ownerInstanceId: 'inst-1', + ownerTakenAt: (res.body as { controlAuthority: { ownerTakenAt: number } }).controlAuthority.ownerTakenAt, + overrideInherited: false, + }, + }) + + const persisted = storeState.store.s1?.data as { + controlAuthority?: Record + } + assert.equal(persisted.controlAuthority?.ownerInstanceId, 'inst-1') + assert.equal(persisted.controlAuthority?.overrideInherited, false) +}) + +void test('take control marks embedded video-sync sessions as locally overriding inherited authority', async () => { + const app = createMockApp() + const ws = createMockWs() as unknown as WsRouter + const session = createVideoSyncSession('child-1') + ;(session.data as Record).embeddedParentSessionId = 'parent-syncdeck' + const storeState = createSessionStore({ 'child-1': session }) + + setupVideoSyncRoutes(app, storeState.sessions, ws) + + const handler = app.handlers.post['/api/video-sync/:sessionId/control-authority/take'] + assert.equal(typeof handler, 'function') + + const res = createResponse() + await handler?.( + { + params: { sessionId: 'child-1' }, + body: { + instructorPasscode: TEST_INSTRUCTOR_PASSCODE, + instructorInstanceId: 'inst-2', + }, + }, + res, + ) + + assert.equal(res.statusCode, 200) + const persisted = storeState.store['child-1']?.data as { + controlAuthority?: Record + } + assert.equal(persisted.controlAuthority?.ownerInstanceId, 'inst-2') + assert.equal(persisted.controlAuthority?.overrideInherited, true) +}) + +void test('take control rejects missing instructor instance ids', async () => { + const app = createMockApp() + const ws = createMockWs() as unknown as WsRouter + const storeState = createSessionStore({ s1: createVideoSyncSession('s1') }) + + setupVideoSyncRoutes(app, storeState.sessions, ws) + + const handler = app.handlers.post['/api/video-sync/:sessionId/control-authority/take'] + assert.equal(typeof handler, 'function') + + const res = createResponse() + await handler?.( + { + params: { sessionId: 's1' }, + body: { + instructorPasscode: TEST_INSTRUCTOR_PASSCODE, + }, + }, + res, + ) + + assert.equal(res.statusCode, 400) + assert.deepEqual(res.body, { + error: 'INVALID_INSTRUCTOR_INSTANCE_ID', + message: 'Valid instructorInstanceId is required', + }) +}) + void test('session get route persists the session when projected playback reaches stopSec', async () => { const originalDateNow = Date.now const nowMs = 12_000 diff --git a/activities/video-sync/server/routes.ts b/activities/video-sync/server/routes.ts index 4224d6f7..15647045 100644 --- a/activities/video-sync/server/routes.ts +++ b/activities/video-sync/server/routes.ts @@ -1,6 +1,10 @@ import { createSession, type SessionRecord, type SessionStore } from 'activebits-server/core/sessions.js' import { registerSessionNormalizer } from 'activebits-server/core/sessionNormalization.js' import { createBroadcastSubscriptionHelper } from 'activebits-server/core/broadcastUtils.js' +import { + claimSessionControlAuthority, + normalizeInstructorInstanceId, +} from '../../../server/controlAuthority.js' import { findHashBySessionId, resolvePersistentSessionEntryPolicy, @@ -205,6 +209,14 @@ function normalizeInstructorPasscode(value: unknown): string | null { return normalized.toLowerCase() } +function readInstructorInstanceId(body: unknown): string | null { + if (!isPlainObject(body)) { + return null + } + + return normalizeInstructorInstanceId(body.instructorInstanceId) +} + function verifyInstructorPasscode(expected: string, candidate: string): boolean { if ( expected.length !== INSTRUCTOR_PASSCODE_LENGTH || @@ -1666,6 +1678,45 @@ export default function setupVideoSyncRoutes( res.json({ success: true, telemetry: data.telemetry }) }) + app.post('/api/video-sync/:sessionId/control-authority/take', async (req, res) => { + const sessionId = resolveSessionId(req) + if (!sessionId) { + res.status(400).json({ error: 'INVALID_SESSION_ID', message: 'sessionId is required' }) + return + } + + const session = await getVideoSyncSession(sessions, sessionId) + if (!session) { + res.status(404).json({ error: 'NOT_FOUND', message: 'Session not found' }) + return + } + + const instructorPasscode = readInstructorPasscode(req.body) + if (!instructorPasscode || !verifyInstructorPasscode(session.data.instructorPasscode, instructorPasscode)) { + res.status(403).json({ error: 'FORBIDDEN', message: 'Valid instructorPasscode is required' }) + return + } + + const instructorInstanceId = readInstructorInstanceId(req.body) + if (!instructorInstanceId) { + res.status(400).json({ error: 'INVALID_INSTRUCTOR_INSTANCE_ID', message: 'Valid instructorInstanceId is required' }) + return + } + + const embeddedParentContext = readEmbeddedParentSessionContext(session.data) + const controlAuthority = claimSessionControlAuthority({ + session, + instructorInstanceId, + overrideInherited: embeddedParentContext != null, + }) + await sessions.set(session.id, session) + + res.json({ + success: true, + controlAuthority, + }) + }) + ws.register('/ws/video-sync', (socket, query) => { const sessionId = query.get('sessionId') const roleParam = query.get('role') diff --git a/client/src/components/common/instructorControlIdentity.test.ts b/client/src/components/common/instructorControlIdentity.test.ts new file mode 100644 index 00000000..4d49d153 --- /dev/null +++ b/client/src/components/common/instructorControlIdentity.test.ts @@ -0,0 +1,77 @@ +import assert from 'node:assert/strict' +import test from 'node:test' +import { + buildInstructorControlInstanceId, + resolveBrowserInstructorControlId, + resolveOrCreateInstructorControlInstanceId, + resolveTabInstructorControlId, + type InstructorControlIdentityStorageLike, +} from './instructorControlIdentity' + +function createStorage(): InstructorControlIdentityStorageLike { + const values = new Map() + + return { + getItem(key) { + return values.get(key) ?? null + }, + setItem(key, value) { + values.set(key, value) + }, + } +} + +void test('buildInstructorControlInstanceId joins browser and tab ids', () => { + assert.equal(buildInstructorControlInstanceId('browser-1', 'tab-1'), 'browser-1:tab-1') +}) + +void test('resolveOrCreateInstructorControlInstanceId creates and persists browser and tab ids', () => { + const localStorage = createStorage() + const sessionStorage = createStorage() + const createdIds = ['browser-1', 'tab-1'] + + const instanceId = resolveOrCreateInstructorControlInstanceId( + { localStorage, sessionStorage }, + () => createdIds.shift() ?? 'unexpected-id', + ) + + assert.equal(instanceId, 'browser-1:tab-1') + assert.equal(resolveBrowserInstructorControlId(localStorage), 'browser-1') + assert.equal(resolveTabInstructorControlId(sessionStorage), 'tab-1') +}) + +void test('resolveOrCreateInstructorControlInstanceId reuses existing ids on reload', () => { + const localStorage = createStorage() + const sessionStorage = createStorage() + + localStorage.setItem('activebits:instructor-control:browser-id', 'browser-1') + sessionStorage.setItem('activebits:instructor-control:tab-id', 'tab-1') + + let createCalls = 0 + const instanceId = resolveOrCreateInstructorControlInstanceId( + { localStorage, sessionStorage }, + () => { + createCalls += 1 + return `generated-${createCalls}` + }, + ) + + assert.equal(instanceId, 'browser-1:tab-1') + assert.equal(createCalls, 0) +}) + +void test('resolveOrCreateInstructorControlInstanceId preserves browser identity while creating a new tab id', () => { + const localStorage = createStorage() + const sessionStorage = createStorage() + + localStorage.setItem('activebits:instructor-control:browser-id', 'browser-1') + + const instanceId = resolveOrCreateInstructorControlInstanceId( + { localStorage, sessionStorage }, + () => 'tab-2', + ) + + assert.equal(instanceId, 'browser-1:tab-2') + assert.equal(resolveBrowserInstructorControlId(localStorage), 'browser-1') + assert.equal(resolveTabInstructorControlId(sessionStorage), 'tab-2') +}) diff --git a/client/src/components/common/instructorControlIdentity.ts b/client/src/components/common/instructorControlIdentity.ts new file mode 100644 index 00000000..36ef684c --- /dev/null +++ b/client/src/components/common/instructorControlIdentity.ts @@ -0,0 +1,56 @@ +export interface InstructorControlIdentityStorageLike { + getItem(key: string): string | null + setItem(key: string, value: string): void +} + +export interface InstructorControlIdentityStorageSet { + localStorage: InstructorControlIdentityStorageLike + sessionStorage: InstructorControlIdentityStorageLike +} + +const BROWSER_ID_STORAGE_KEY = 'activebits:instructor-control:browser-id' +const TAB_ID_STORAGE_KEY = 'activebits:instructor-control:tab-id' + +function normalizeStoredId(value: string | null): string | null { + if (typeof value !== 'string') { + return null + } + + const trimmed = value.trim() + return trimmed.length > 0 ? trimmed : null +} + +export function buildInstructorControlInstanceId(browserId: string, tabId: string): string { + return `${browserId}:${tabId}` +} + +export function resolveOrCreateInstructorControlInstanceId( + storage: InstructorControlIdentityStorageSet, + createId: () => string, +): string { + let browserId = normalizeStoredId(storage.localStorage.getItem(BROWSER_ID_STORAGE_KEY)) + if (!browserId) { + browserId = createId().trim() + storage.localStorage.setItem(BROWSER_ID_STORAGE_KEY, browserId) + } + + let tabId = normalizeStoredId(storage.sessionStorage.getItem(TAB_ID_STORAGE_KEY)) + if (!tabId) { + tabId = createId().trim() + storage.sessionStorage.setItem(TAB_ID_STORAGE_KEY, tabId) + } + + return buildInstructorControlInstanceId(browserId, tabId) +} + +export function resolveBrowserInstructorControlId( + localStorage: InstructorControlIdentityStorageLike, +): string | null { + return normalizeStoredId(localStorage.getItem(BROWSER_ID_STORAGE_KEY)) +} + +export function resolveTabInstructorControlId( + sessionStorage: InstructorControlIdentityStorageLike, +): string | null { + return normalizeStoredId(sessionStorage.getItem(TAB_ID_STORAGE_KEY)) +} From e2fa9c77e8009d814705b6b2585df1358008f85b Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Wed, 29 Apr 2026 04:37:50 +0000 Subject: [PATCH 04/15] Add video sync authority status plumbing --- .agent/plans/control-authority-plan.md | 63 ++++++---- .../client/manager/VideoSyncManager.test.ts | 68 +++++++++++ .../client/manager/VideoSyncManager.tsx | 108 ++++++++++++++++-- activities/video-sync/client/protocol.test.ts | 1 - activities/video-sync/client/protocol.ts | 48 +++++++- activities/video-sync/server/routes.test.ts | 12 ++ activities/video-sync/server/routes.ts | 9 ++ 7 files changed, 272 insertions(+), 37 deletions(-) diff --git a/.agent/plans/control-authority-plan.md b/.agent/plans/control-authority-plan.md index 1a608113..62c4d112 100644 --- a/.agent/plans/control-authority-plan.md +++ b/.agent/plans/control-authority-plan.md @@ -1,6 +1,6 @@ # Control Authority Plan -## Status: Design / Pre-Implementation +## Status: In Progress This document is the working implementation plan for issue `#245`: replace fragile multi-instructor command races with an explicit server-owned control authority model that activities can opt into. @@ -141,7 +141,7 @@ Prefer websocket-first authority updates because this is live session runtime st Client identity: - each instructor manager tab gets a stable `instructorInstanceId` -- preserve it in `sessionStorage` or equivalent tab-scoped storage +- persist it with a durable browser id plus a tab-scoped discriminator so normal reloads keep ownership while separate tabs remain distinct Authority actions: - client sends `take-control` @@ -177,24 +177,24 @@ Shared enforcement rule: ### 1. Shared config and typing -- [ ] Add `controlAuthority` to shared activity config types. -- [ ] Add schema validation for `mode`, `scope`, and `gating`. -- [ ] Document fallback semantics for `scope: 'inherited'` without a controlling parent. -- [ ] Add runtime helpers to resolve effective authority scope and authority session id. +- [x] Add `controlAuthority` to shared activity config types. +- [x] Add schema validation for `mode`, `scope`, and `gating`. +- [x] Document fallback semantics for `scope: 'inherited'` without a controlling parent. +- [x] Add runtime helpers to resolve effective authority scope and authority session id. ### 2. Shared session model -- [ ] Define shared authority session-state shape. +- [x] Define shared authority session-state shape. - [ ] Add normalizer support so authority-enabled sessions recover safely after restart. -- [ ] Define how embedded child sessions discover parent authority context. -- [ ] Define local override semantics for inherited child sessions. -- [ ] Define the persisted-empty-state behavior for older in-flight sessions so first manager connect auto-claims ownership intentionally. +- [x] Define how embedded child sessions discover parent authority context. +- [x] Define local override semantics for inherited child sessions. +- [x] Define the persisted-empty-state behavior for older in-flight sessions so first manager connect auto-claims ownership intentionally. ### 3. Shared server enforcement -- [ ] Add shared instructor instance identity handling for manager websocket/API traffic. +- [x] Add shared instructor instance identity handling for manager websocket/API traffic. - [ ] Auto-assign first connected instructor as owner when no owner exists. -- [ ] Add `take-control` server action and broadcast path. +- [x] Add `take-control` server action and broadcast path. - [ ] Add generic authority checks before processing manager runtime commands. - [ ] Wire `gating: 'all' | 'none' | 'activity'` into enforcement. - [ ] Add activity callback lookup/invocation for `gating: 'activity'`. @@ -204,9 +204,9 @@ Shared enforcement rule: ### 4. Shared client plumbing -- [ ] Generate and persist stable `instructorInstanceId` values for manager tabs. -- [ ] Subscribe authority-enabled manager views to live authority status updates. -- [ ] Expose authority state to activity manager UIs through a shared hook/helper. +- [x] Generate and persist stable `instructorInstanceId` values for manager tabs. +- [x] Subscribe authority-enabled manager views to live authority status updates. +- [x] Expose authority state to activity manager UIs through a shared hook/helper. - [ ] Provide shared disabled-state helpers and feedback text for gated controls. ### 5. Shared UI primitives @@ -221,25 +221,42 @@ Shared enforcement rule: - [ ] Add `controlAuthority` config to SyncDeck. - [ ] Decide SyncDeck gating mode. Current expectation: `gating: 'activity'`. - [ ] Add `controlAuthority` config to Video Sync. -- [ ] Decide Video Sync gating mode. Current expectation: `gating: 'activity'` or `all`, depending on final command surface. +- [x] Decide Video Sync gating mode. Current expectation: `gating: 'activity'` or `all`, depending on final command surface. - [ ] Implement activity-owned command classifiers where `gating: 'activity'` is used. -- [ ] Ensure embedded Video Sync defaults to inherited authority when launched under SyncDeck. -- [ ] Ensure embedded Video Sync can be locally overridden by explicit `Take Control`. +- [x] Ensure embedded Video Sync defaults to inherited authority when launched under SyncDeck. +- [x] Ensure embedded Video Sync can be locally overridden by explicit `Take Control`. ### 7. Validation -- [ ] Add unit tests for config validation and authority resolution helpers. +- [x] Add unit tests for config validation and authority resolution helpers. - [ ] Add server tests for first-instructor auto-ownership. -- [ ] Add server tests for `take-control` handoff. -- [ ] Add server tests for inherited authority resolution and local child override. +- [x] Add server tests for `take-control` handoff. +- [x] Add server tests for inherited authority resolution and local child override. - [ ] Add server tests for non-owner gated-command rejection. - [ ] Add server tests covering older sessions with empty authority state that auto-initialize on first manager connect. -- [ ] Add client tests for disabled controls and authority status messaging. +- [x] Add client tests for disabled controls and authority status messaging. - [ ] Add activity-specific tests for SyncDeck command classification. - [ ] Add activity-specific tests for Video Sync command classification. - [ ] Add browser-level E2E coverage for two instructor views on the same authority-enabled session: first instructor default owner, second instructor disabled, live takeover, and disabled-state flip after handoff. - [ ] Add browser-level E2E coverage for embedded inherited authority plus local child override behavior. -- [ ] Run scope-appropriate repo validation, likely `npm test`, and include `npm run test:e2e` for the multi-instructor manager and embedded authority scenarios when the harness can support them. +- [x] Run scope-appropriate repo validation, likely `npm test`, and include `npm run test:e2e` for the multi-instructor manager and embedded authority scenarios when the harness can support them. + +## Current Progress Snapshot + +Implemented on this branch so far: +- shared `controlAuthority` activity config shape and schema validation +- shared authority state normalization, ownership helpers, and inherited/session resolution helpers +- durable browser-plus-tab instructor identity generation for manager clients +- Video Sync takeover endpoint and embedded-child local override handling +- Video Sync manager authority status plumbing and `Take Control` UI wiring +- Video Sync protocol/session payload support for `controlAuthority` + +Still pending before this feature is complete: +- first-instructor auto-ownership on live manager connect +- actual non-owner command rejection/disabled playback behavior in Video Sync +- SyncDeck adoption and command classification +- websocket broadcast/update path for authority changes across instructor views +- browser-level E2E coverage for the multi-instructor handoff scenario ## Suggested Rollout Order diff --git a/activities/video-sync/client/manager/VideoSyncManager.test.ts b/activities/video-sync/client/manager/VideoSyncManager.test.ts index d67abae6..e1394a39 100644 --- a/activities/video-sync/client/manager/VideoSyncManager.test.ts +++ b/activities/video-sync/client/manager/VideoSyncManager.test.ts @@ -9,7 +9,9 @@ import { buildManagerWsUrl, clearManagerPlayerLoadError, createManagerWsAuthMessage, + getVideoSyncControlStatusLabel, getManagerPlaybackIntentForStateChange, + isVideoSyncControlOwner, parseManagerStopTimeInput, readBootstrapInstructorPasscode, readBootstrapSourceUrl, @@ -245,6 +247,72 @@ void test('createManagerWsAuthMessage serializes the post-connect auth payload', assert.equal(createManagerWsAuthMessage(null), null) }) +void test('isVideoSyncControlOwner matches the current instructor instance against the owner id', () => { + assert.equal( + isVideoSyncControlOwner( + { + mode: 'single-instructor', + ownerInstanceId: 'inst-123', + ownerTakenAt: 1, + overrideInherited: false, + }, + 'inst-123', + ), + true, + ) + assert.equal( + isVideoSyncControlOwner( + { + mode: 'single-instructor', + ownerInstanceId: 'inst-123', + ownerTakenAt: 1, + overrideInherited: false, + }, + 'inst-456', + ), + false, + ) +}) + +void test('getVideoSyncControlStatusLabel reports owner, non-owner, and unclaimed states', () => { + assert.equal( + getVideoSyncControlStatusLabel({ + controlAuthority: { + mode: 'single-instructor', + ownerInstanceId: 'inst-123', + ownerTakenAt: 1, + overrideInherited: false, + }, + instructorInstanceId: 'inst-123', + }), + 'You have control', + ) + assert.equal( + getVideoSyncControlStatusLabel({ + controlAuthority: { + mode: 'single-instructor', + ownerInstanceId: 'inst-123', + ownerTakenAt: 1, + overrideInherited: false, + }, + instructorInstanceId: 'inst-456', + }), + 'Another instructor currently has control', + ) + assert.equal( + getVideoSyncControlStatusLabel({ + controlAuthority: { + mode: 'single-instructor', + ownerInstanceId: null, + ownerTakenAt: null, + overrideInherited: false, + }, + instructorInstanceId: 'inst-456', + }), + 'Control owner is being established', + ) +}) + void test('shouldAutoStartBootstrapSource requires setup mode, source url, and ready credentials', () => { assert.equal( shouldAutoStartBootstrapSource({ diff --git a/activities/video-sync/client/manager/VideoSyncManager.tsx b/activities/video-sync/client/manager/VideoSyncManager.tsx index f12fd1ef..3a095525 100644 --- a/activities/video-sync/client/manager/VideoSyncManager.tsx +++ b/activities/video-sync/client/manager/VideoSyncManager.tsx @@ -10,6 +10,7 @@ import { useLocation, useNavigate, useParams } from 'react-router-dom' import { parseVideoSyncErrorMessagePayload, parseVideoSyncEnvelope, + type VideoSyncControlAuthority, parseVideoSyncStateMessagePayload, parseVideoSyncTelemetryMessagePayload, type VideoSyncState, @@ -34,6 +35,7 @@ interface SessionResponse { data?: { state?: VideoSyncState telemetry?: VideoSyncTelemetry + controlAuthority?: VideoSyncControlAuthority } } @@ -41,6 +43,7 @@ interface ConfigResponse { data?: { state?: VideoSyncState telemetry?: VideoSyncTelemetry + controlAuthority?: VideoSyncControlAuthority } } @@ -48,6 +51,7 @@ interface CommandResponse { data?: { state?: VideoSyncState telemetry?: VideoSyncTelemetry + controlAuthority?: VideoSyncControlAuthority } } @@ -76,6 +80,13 @@ const EMPTY_TELEMETRY: VideoSyncTelemetry = { error: { code: null, message: null }, } +const EMPTY_CONTROL_AUTHORITY: VideoSyncControlAuthority = { + mode: 'single-instructor', + ownerInstanceId: null, + ownerTakenAt: null, + overrideInherited: false, +} + const DEFAULT_STATE: VideoSyncState = { provider: 'youtube', videoId: '', @@ -245,6 +256,32 @@ export function clearManagerPlayerLoadError(message: string | null): string | nu return message === YOUTUBE_MANAGER_LOAD_ERROR ? null : message } +export function isVideoSyncControlOwner( + controlAuthority: VideoSyncControlAuthority | null | undefined, + instructorInstanceId: string | null | undefined, +): boolean { + return ( + controlAuthority?.ownerInstanceId != null && + instructorInstanceId != null && + controlAuthority.ownerInstanceId === instructorInstanceId + ) +} + +export function getVideoSyncControlStatusLabel(params: { + controlAuthority: VideoSyncControlAuthority | null | undefined + instructorInstanceId: string | null | undefined +}): string { + if (isVideoSyncControlOwner(params.controlAuthority, params.instructorInstanceId)) { + return 'You have control' + } + + if (params.controlAuthority?.ownerInstanceId) { + return 'Another instructor currently has control' + } + + return 'Control owner is being established' +} + export function sanitizeManagerApiErrorMessage( message: unknown, fallback: string, @@ -346,6 +383,7 @@ export default function VideoSyncManager() { const [autoStartStatus, setAutoStartStatus] = useState('idle') const [embeddedBootstrapSourceUrl, setEmbeddedBootstrapSourceUrl] = useState(null) const [persistentRecoverySourceUrl, setPersistentRecoverySourceUrl] = useState(null) + const [controlAuthority, setControlAuthority] = useState(EMPTY_CONTROL_AUTHORITY) const instructorInstanceId = useMemo(() => { if (typeof window === 'undefined') { return null @@ -490,6 +528,9 @@ export default function VideoSyncManager() { if (updated.data?.telemetry) { setTelemetry(updated.data.telemetry) } + if (updated.data?.controlAuthority) { + setControlAuthority(updated.data.controlAuthority) + } setErrorMessage(null) return true } catch (error) { @@ -610,6 +651,9 @@ export default function VideoSyncManager() { if (data.data?.telemetry) { setTelemetry(data.data.telemetry) } + if (data.data?.controlAuthority) { + setControlAuthority(data.data.controlAuthority) + } setErrorMessage(null) } catch (error) { const message = error instanceof Error ? error.message : 'Failed to load video-sync session' @@ -705,20 +749,26 @@ export default function VideoSyncManager() { const handleEnvelope = useCallback((envelope: VideoSyncWsEnvelope) => { if (envelope.type === 'state-update' || envelope.type === 'state-snapshot' || envelope.type === 'heartbeat') { const payload = parseVideoSyncStateMessagePayload(envelope.payload) - if (payload?.state) { - applyManagerStateUpdate(payload.state) - } - if (payload?.telemetry) { - setTelemetry(payload.telemetry) + if (payload?.state) { + applyManagerStateUpdate(payload.state) + } + if (payload?.telemetry) { + setTelemetry(payload.telemetry) + } + if (payload?.controlAuthority) { + setControlAuthority(payload.controlAuthority) + } + return } - return - } if (envelope.type === 'telemetry-update') { const payload = parseVideoSyncTelemetryMessagePayload(envelope.payload) if (payload?.telemetry) { setTelemetry(payload.telemetry) } + if (payload?.controlAuthority) { + setControlAuthority(payload.controlAuthority) + } return } @@ -1001,6 +1051,40 @@ export default function VideoSyncManager() { } const displayPosition = useMemo(() => computeDesiredPositionSec(state), [state]) + const hasControl = isVideoSyncControlOwner(controlAuthority, instructorInstanceId) + const controlStatusLabel = getVideoSyncControlStatusLabel({ + controlAuthority, + instructorInstanceId, + }) + const takeControl = useCallback(async (): Promise => { + if (!sessionId || !instructorPasscode || !instructorInstanceId) { + return + } + + try { + const response = await fetch(`/api/video-sync/${sessionId}/control-authority/take`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + instructorPasscode, + instructorInstanceId, + }), + }) + if (!response.ok) { + const failure = (await response.json()) as { message?: string } + throw new Error(sanitizeManagerApiErrorMessage(failure.message, 'Failed to take control')) + } + + const payload = (await response.json()) as { controlAuthority?: VideoSyncControlAuthority } + if (payload.controlAuthority) { + setControlAuthority(payload.controlAuthority) + } + setErrorMessage(null) + } catch (error) { + const message = error instanceof Error ? error.message : 'Failed to take control' + setErrorMessage(message) + } + }, [instructorInstanceId, instructorPasscode, sessionId]) if (setupMode) { const shouldShowAutoStartSplash = bootstrapSourceUrl != null && autoStartStatus !== 'failed' @@ -1079,6 +1163,14 @@ export default function VideoSyncManager() { +
+ {controlStatusLabel} + {!hasControl ? ( + + ) : null} +
)} @@ -1097,6 +1189,7 @@ export default function VideoSyncManager() { Session: {sessionId ?? '—'}
+ {!hasControl ? : null}
@@ -1123,6 +1216,7 @@ export default function VideoSyncManager() {
Video: {state.videoId || 'Not configured'} + {controlStatusLabel} Playing: {state.isPlaying ? 'Yes' : 'No'} Position: {displayPosition.toFixed(2)}s Connections: {telemetry.connections.activeCount} diff --git a/activities/video-sync/client/protocol.test.ts b/activities/video-sync/client/protocol.test.ts index d1024a49..5f1cc100 100644 --- a/activities/video-sync/client/protocol.test.ts +++ b/activities/video-sync/client/protocol.test.ts @@ -142,7 +142,6 @@ void test('parseVideoSyncStateMessagePayload normalizes legacy manager updates t updatedBy: 'instructor', serverTimestampMs: 1234, }, - telemetry: undefined, }, ) }) diff --git a/activities/video-sync/client/protocol.ts b/activities/video-sync/client/protocol.ts index 692a3477..4717353e 100644 --- a/activities/video-sync/client/protocol.ts +++ b/activities/video-sync/client/protocol.ts @@ -5,6 +5,13 @@ export type VideoSyncMessageType = | 'telemetry-update' | 'error' +export interface VideoSyncControlAuthority { + mode: 'single-instructor' + ownerInstanceId: string | null + ownerTakenAt: number | null + overrideInherited: boolean +} + export interface VideoSyncState { provider: 'youtube' videoId: string @@ -47,10 +54,12 @@ export interface VideoSyncWsEnvelope { export interface VideoSyncStateMessagePayload { state?: VideoSyncState telemetry?: VideoSyncTelemetry + controlAuthority?: VideoSyncControlAuthority } export interface VideoSyncTelemetryMessagePayload { telemetry?: VideoSyncTelemetry + controlAuthority?: VideoSyncControlAuthority } export interface VideoSyncErrorMessagePayload { @@ -69,6 +78,19 @@ function isNullableFiniteNumber(value: unknown): value is number | null { return value === null || isFiniteNumber(value) } +export function isVideoSyncControlAuthority(value: unknown): value is VideoSyncControlAuthority { + if (!isRecord(value)) { + return false + } + + return ( + value.mode === 'single-instructor' && + (value.ownerInstanceId === null || typeof value.ownerInstanceId === 'string') && + isNullableFiniteNumber(value.ownerTakenAt) && + typeof value.overrideInherited === 'boolean' + ) +} + function normalizeUpdatedBy(value: unknown): VideoSyncState['updatedBy'] | null { if (value === 'instructor' || value === 'manager') { return 'instructor' @@ -147,15 +169,23 @@ export function parseVideoSyncStateMessagePayload(payload: unknown): VideoSyncSt if ('telemetry' in payload && payload.telemetry !== undefined && !isVideoSyncTelemetry(payload.telemetry)) { return null } + if ('controlAuthority' in payload && payload.controlAuthority !== undefined && !isVideoSyncControlAuthority(payload.controlAuthority)) { + return null + } return { - state: isVideoSyncState(payload.state) + ...(isVideoSyncState(payload.state) ? { - ...payload.state, - updatedBy: normalizeUpdatedBy(payload.state.updatedBy) ?? 'system', + state: { + ...payload.state, + updatedBy: normalizeUpdatedBy(payload.state.updatedBy) ?? 'system', + }, } - : undefined, - telemetry: isVideoSyncTelemetry(payload.telemetry) ? payload.telemetry : undefined, + : {}), + ...(isVideoSyncTelemetry(payload.telemetry) ? { telemetry: payload.telemetry } : {}), + ...(isVideoSyncControlAuthority(payload.controlAuthority) + ? { controlAuthority: payload.controlAuthority } + : {}), } } @@ -167,9 +197,15 @@ export function parseVideoSyncTelemetryMessagePayload(payload: unknown): VideoSy if ('telemetry' in payload && payload.telemetry !== undefined && !isVideoSyncTelemetry(payload.telemetry)) { return null } + if ('controlAuthority' in payload && payload.controlAuthority !== undefined && !isVideoSyncControlAuthority(payload.controlAuthority)) { + return null + } return { - telemetry: isVideoSyncTelemetry(payload.telemetry) ? payload.telemetry : undefined, + ...(isVideoSyncTelemetry(payload.telemetry) ? { telemetry: payload.telemetry } : {}), + ...(isVideoSyncControlAuthority(payload.controlAuthority) + ? { controlAuthority: payload.controlAuthority } + : {}), } } diff --git a/activities/video-sync/server/routes.test.ts b/activities/video-sync/server/routes.test.ts index 69f8c203..77a25269 100644 --- a/activities/video-sync/server/routes.test.ts +++ b/activities/video-sync/server/routes.test.ts @@ -414,6 +414,12 @@ void test('session get route redacts instructor-only fields from public payload' assert.equal(payload.data?.standaloneMode, false) assert.equal(typeof payload.data?.state, 'object') assert.equal(typeof payload.data?.telemetry, 'object') + assert.deepEqual(payload.data?.controlAuthority, { + mode: 'single-instructor', + ownerInstanceId: null, + ownerTakenAt: null, + overrideInherited: false, + }) assert.equal('instructorPasscode' in (payload.data ?? {}), false) }) @@ -1230,6 +1236,12 @@ void test('session patch can mark a configured session as standalone', async () : undefined, }, telemetry: updated.telemetry, + controlAuthority: { + mode: 'single-instructor', + ownerInstanceId: null, + ownerTakenAt: null, + overrideInherited: false, + }, }, }) }) diff --git a/activities/video-sync/server/routes.ts b/activities/video-sync/server/routes.ts index 15647045..62106153 100644 --- a/activities/video-sync/server/routes.ts +++ b/activities/video-sync/server/routes.ts @@ -3,6 +3,7 @@ import { registerSessionNormalizer } from 'activebits-server/core/sessionNormali import { createBroadcastSubscriptionHelper } from 'activebits-server/core/broadcastUtils.js' import { claimSessionControlAuthority, + getSessionControlAuthorityState, normalizeInstructorInstanceId, } from '../../../server/controlAuthority.js' import { @@ -64,6 +65,7 @@ interface PublicVideoSyncSessionData { standaloneMode: boolean state: VideoSyncState telemetry: VideoSyncTelemetry + controlAuthority: ReturnType } interface VideoSyncSession extends SessionRecord { @@ -961,6 +963,7 @@ function toPublicSessionData(data: VideoSyncSessionData): PublicVideoSyncSession standaloneMode: data.standaloneMode, state: data.state, telemetry: data.telemetry, + controlAuthority: getSessionControlAuthorityState({ data }), } } @@ -1512,6 +1515,7 @@ export default function setupVideoSyncRoutes( const envelope = createEnvelope(sessionId, 'state-update', { state: data.state, telemetry: data.telemetry, + controlAuthority: getSessionControlAuthorityState(session), reason: 'config-updated', }) await broadcastEnvelope(sessions, ws, sessionId, envelope) @@ -1593,6 +1597,7 @@ export default function setupVideoSyncRoutes( const envelope = createEnvelope(sessionId, 'state-update', { state: data.state, telemetry: data.telemetry, + controlAuthority: getSessionControlAuthorityState(session), reason: body.type, }) await broadcastEnvelope(sessions, ws, sessionId, envelope) @@ -1671,6 +1676,7 @@ export default function setupVideoSyncRoutes( const envelope = createEnvelope(sessionId, 'telemetry-update', { telemetry: data.telemetry, + controlAuthority: getSessionControlAuthorityState(session), reason: body.type, }) await broadcastEnvelope(sessions, ws, sessionId, envelope) @@ -1757,6 +1763,7 @@ export default function setupVideoSyncRoutes( const disconnectTelemetryUpdate = createEnvelope(sessionId, 'telemetry-update', { telemetry: currentData.telemetry, + controlAuthority: getSessionControlAuthorityState(currentSession), reason: 'connection-change', }) await broadcastEnvelope(sessions, ws, sessionId, disconnectTelemetryUpdate) @@ -1825,6 +1832,7 @@ export default function setupVideoSyncRoutes( const snapshot = createEnvelope(sessionId, 'state-snapshot', { state: data.state, telemetry: data.telemetry, + controlAuthority: getSessionControlAuthorityState(session), role, }) @@ -1834,6 +1842,7 @@ export default function setupVideoSyncRoutes( const telemetryUpdate = createEnvelope(sessionId, 'telemetry-update', { telemetry: data.telemetry, + controlAuthority: getSessionControlAuthorityState(session), reason: 'connection-change', }) await broadcastEnvelope(sessions, ws, sessionId, telemetryUpdate) From 06abcdafb4a1bb1ced8244bf0b1c4332500aeed4 Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Wed, 29 Apr 2026 04:52:29 +0000 Subject: [PATCH 05/15] Enforce video sync control ownership --- .agent/knowledge/testing-patterns.md | 9 ++ .agent/plans/control-authority-plan.md | 18 +-- .../client/manager/VideoSyncManager.test.ts | 59 +++++++++ .../client/manager/VideoSyncManager.tsx | 61 +++++++-- activities/video-sync/server/routes.test.ts | 118 +++++++++++++++--- activities/video-sync/server/routes.ts | 90 +++++++++++++ 6 files changed, 323 insertions(+), 32 deletions(-) diff --git a/.agent/knowledge/testing-patterns.md b/.agent/knowledge/testing-patterns.md index 20572f80..cb09d617 100644 --- a/.agent/knowledge/testing-patterns.md +++ b/.agent/knowledge/testing-patterns.md @@ -15,6 +15,15 @@ Capture reusable test setup patterns, common failure modes, and reliability guid ## Entries +- Date: 2026-04-29 +- Scope: integration +- Pattern: Server-side maintenance timers created during route handling should call `unref?.()` when they are only background cleanup helpers, especially in activity route modules that schedule delayed pruning or telemetry refresh work. +- Why it helps: Real `setTimeout` handles created during focused route tests can keep the Node process alive and make a healthy suite hang with `Promise resolution is still pending but the event loop has already resolved`, even when the assertions themselves pass. `unref()` keeps production cleanup behavior while letting tests and short-lived processes exit cleanly. +- Example (file/path): `activities/video-sync/server/routes.ts`; `activities/video-sync/server/routes.test.ts` +- Failure signal: A route test file appears to pass in narrowed subsets but the full file hangs until external timeout because background timers remain referenced after the test completes. +- Follow-up action: Prefer `unref?.()` for non-critical cleanup timers, and if a test needs to inspect timer scheduling directly, patch `setTimeout` locally while still returning an object shape that tolerates optional `unref`. +- Owner: Codex + - Date: 2026-03-04 - Scope: integration - Pattern: Session-store mocks for route tests should return deep clones from `get()` and store clones on `set()` when production storage serializes records between calls. diff --git a/.agent/plans/control-authority-plan.md b/.agent/plans/control-authority-plan.md index 62c4d112..4022a2b5 100644 --- a/.agent/plans/control-authority-plan.md +++ b/.agent/plans/control-authority-plan.md @@ -198,9 +198,9 @@ Shared enforcement rule: - [ ] Add generic authority checks before processing manager runtime commands. - [ ] Wire `gating: 'all' | 'none' | 'activity'` into enforcement. - [ ] Add activity callback lookup/invocation for `gating: 'activity'`. -- [ ] Return explicit non-owner feedback for gated command attempts while keeping the server state unchanged. -- [ ] Document that owner disconnect does not auto-release authority and that explicit takeover is the only reassignment path. -- [ ] Document last-write-wins behavior for concurrent `take-control` requests. +- [x] Return explicit non-owner feedback for gated command attempts while keeping the server state unchanged. +- [x] Document that owner disconnect does not auto-release authority and that explicit takeover is the only reassignment path. +- [x] Document last-write-wins behavior for concurrent `take-control` requests. ### 4. Shared client plumbing @@ -225,6 +225,8 @@ Shared enforcement rule: - [ ] Implement activity-owned command classifiers where `gating: 'activity'` is used. - [x] Ensure embedded Video Sync defaults to inherited authority when launched under SyncDeck. - [x] Ensure embedded Video Sync can be locally overridden by explicit `Take Control`. +- [x] Enforce Video Sync non-owner command rejection for config and playback command routes. +- [x] Disable Video Sync gated controls locally with explicit takeover feedback. ### 7. Validation @@ -232,11 +234,11 @@ Shared enforcement rule: - [ ] Add server tests for first-instructor auto-ownership. - [x] Add server tests for `take-control` handoff. - [x] Add server tests for inherited authority resolution and local child override. -- [ ] Add server tests for non-owner gated-command rejection. +- [x] Add server tests for non-owner gated-command rejection. - [ ] Add server tests covering older sessions with empty authority state that auto-initialize on first manager connect. - [x] Add client tests for disabled controls and authority status messaging. - [ ] Add activity-specific tests for SyncDeck command classification. -- [ ] Add activity-specific tests for Video Sync command classification. +- [x] Add activity-specific tests for Video Sync command classification helpers. - [ ] Add browser-level E2E coverage for two instructor views on the same authority-enabled session: first instructor default owner, second instructor disabled, live takeover, and disabled-state flip after handoff. - [ ] Add browser-level E2E coverage for embedded inherited authority plus local child override behavior. - [x] Run scope-appropriate repo validation, likely `npm test`, and include `npm run test:e2e` for the multi-instructor manager and embedded authority scenarios when the harness can support them. @@ -250,10 +252,12 @@ Implemented on this branch so far: - Video Sync takeover endpoint and embedded-child local override handling - Video Sync manager authority status plumbing and `Take Control` UI wiring - Video Sync protocol/session payload support for `controlAuthority` +- Video Sync server-side owner checks for playback/config commands plus standalone first-owner auto-claim +- Video Sync manager disabled-state enforcement and takeover feedback for non-owners +- Video Sync unsynced-student prune timers now `unref()` so maintenance timers do not keep tests or Node processes alive unnecessarily Still pending before this feature is complete: -- first-instructor auto-ownership on live manager connect -- actual non-owner command rejection/disabled playback behavior in Video Sync +- shared/generic first-instructor auto-ownership support beyond the current Video Sync adoption - SyncDeck adoption and command classification - websocket broadcast/update path for authority changes across instructor views - browser-level E2E coverage for the multi-instructor handoff scenario diff --git a/activities/video-sync/client/manager/VideoSyncManager.test.ts b/activities/video-sync/client/manager/VideoSyncManager.test.ts index e1394a39..6b067df5 100644 --- a/activities/video-sync/client/manager/VideoSyncManager.test.ts +++ b/activities/video-sync/client/manager/VideoSyncManager.test.ts @@ -7,6 +7,7 @@ import { import { autoConfigureBootstrapSource, buildManagerWsUrl, + canUseVideoSyncInstructorControls, clearManagerPlayerLoadError, createManagerWsAuthMessage, getVideoSyncControlStatusLabel, @@ -313,6 +314,64 @@ void test('getVideoSyncControlStatusLabel reports owner, non-owner, and unclaime ) }) +void test('canUseVideoSyncInstructorControls allows owners and standalone unclaimed sessions, but blocks claimed or embedded-unclaimed sessions', () => { + assert.equal( + canUseVideoSyncInstructorControls({ + controlAuthority: { + mode: 'single-instructor', + ownerInstanceId: 'inst-123', + ownerTakenAt: 1, + overrideInherited: false, + }, + instructorInstanceId: 'inst-123', + sessionId: 'session-123', + }), + true, + ) + + assert.equal( + canUseVideoSyncInstructorControls({ + controlAuthority: { + mode: 'single-instructor', + ownerInstanceId: null, + ownerTakenAt: null, + overrideInherited: false, + }, + instructorInstanceId: 'inst-456', + sessionId: 'session-123', + }), + true, + ) + + assert.equal( + canUseVideoSyncInstructorControls({ + controlAuthority: { + mode: 'single-instructor', + ownerInstanceId: 'inst-123', + ownerTakenAt: 1, + overrideInherited: false, + }, + instructorInstanceId: 'inst-456', + sessionId: 'session-123', + }), + false, + ) + + assert.equal( + canUseVideoSyncInstructorControls({ + controlAuthority: { + mode: 'single-instructor', + ownerInstanceId: null, + ownerTakenAt: null, + overrideInherited: false, + }, + instructorInstanceId: 'inst-456', + sessionId: 'CHILD:parent:abcde:video-sync', + }), + false, + ) +}) + void test('shouldAutoStartBootstrapSource requires setup mode, source url, and ready credentials', () => { assert.equal( shouldAutoStartBootstrapSource({ diff --git a/activities/video-sync/client/manager/VideoSyncManager.tsx b/activities/video-sync/client/manager/VideoSyncManager.tsx index 3a095525..aefe6716 100644 --- a/activities/video-sync/client/manager/VideoSyncManager.tsx +++ b/activities/video-sync/client/manager/VideoSyncManager.tsx @@ -282,6 +282,22 @@ export function getVideoSyncControlStatusLabel(params: { return 'Control owner is being established' } +export function canUseVideoSyncInstructorControls(params: { + controlAuthority: VideoSyncControlAuthority | null | undefined + instructorInstanceId: string | null | undefined + sessionId: string | null | undefined +}): boolean { + if (isVideoSyncControlOwner(params.controlAuthority, params.instructorInstanceId)) { + return true + } + + if (params.controlAuthority?.ownerInstanceId) { + return false + } + + return !isEmbeddedChildSessionId(params.sessionId ?? undefined) +} + export function sanitizeManagerApiErrorMessage( message: unknown, fallback: string, @@ -412,6 +428,16 @@ export default function VideoSyncManager() { const autoStartAttemptKeyRef = useRef(null) const queryBootstrapSourceUrl = useMemo(() => readBootstrapSourceUrl(location.search), [location.search]) const bootstrapSourceUrl = persistentRecoverySourceUrl ?? queryBootstrapSourceUrl ?? embeddedBootstrapSourceUrl + const hasControl = isVideoSyncControlOwner(controlAuthority, instructorInstanceId) + const canUseInstructorControls = canUseVideoSyncInstructorControls({ + controlAuthority, + instructorInstanceId, + sessionId, + }) + const controlStatusLabel = getVideoSyncControlStatusLabel({ + controlAuthority, + instructorInstanceId, + }) useEffect(() => { if (!shouldFetchEmbeddedBootstrapSourceUrl({ sessionId, queryBootstrapSourceUrl })) { @@ -494,6 +520,12 @@ export default function VideoSyncManager() { if (!sessionId) { return false } + if (!canUseInstructorControls) { + if (options?.reportErrors !== false) { + setErrorMessage('Take control to use playback controls for this session.') + } + return false + } if (!instructorPasscode) { if (options?.reportErrors !== false) { setErrorMessage('Instructor credentials missing. Open this session from the dashboard or authenticated permalink.') @@ -542,7 +574,7 @@ export default function VideoSyncManager() { } return false } - }, [instructorInstanceId, instructorPasscode, sessionId]) + }, [canUseInstructorControls, instructorInstanceId, instructorPasscode, sessionId]) const flushManagerPlaybackIntent = useCallback(async (): Promise => { clearPlaybackCommandFlushTimer() @@ -924,6 +956,10 @@ export default function VideoSyncManager() { stopSecTextValue: string, ): Promise => { if (!sessionId) return false + if (!canUseInstructorControls) { + setErrorMessage('Take control to change the video configuration for this session.') + return false + } if (!isPasscodeReady) { setErrorMessage('Loading instructor credentials...') return false @@ -968,6 +1004,9 @@ export default function VideoSyncManager() { if (updated.data?.telemetry) { setTelemetry(updated.data.telemetry) } + if (updated.data?.controlAuthority) { + setControlAuthority(updated.data.controlAuthority) + } setErrorMessage(null) return true } catch (error) { @@ -975,7 +1014,7 @@ export default function VideoSyncManager() { setErrorMessage(message) return false } - }, [applyManagerStateUpdate, instructorInstanceId, instructorPasscode, isPasscodeReady, sessionId]) + }, [applyManagerStateUpdate, canUseInstructorControls, instructorInstanceId, instructorPasscode, isPasscodeReady, sessionId]) const saveConfig = useCallback(async (): Promise => { await saveConfigWithValues(sourceUrlInput, hasStopTime, stopSecInput) @@ -1051,11 +1090,6 @@ export default function VideoSyncManager() { } const displayPosition = useMemo(() => computeDesiredPositionSec(state), [state]) - const hasControl = isVideoSyncControlOwner(controlAuthority, instructorInstanceId) - const controlStatusLabel = getVideoSyncControlStatusLabel({ - controlAuthority, - instructorInstanceId, - }) const takeControl = useCallback(async (): Promise => { if (!sessionId || !instructorPasscode || !instructorInstanceId) { return @@ -1126,6 +1160,7 @@ export default function VideoSyncManager() { onChange={(event) => setSourceUrlInput(event.target.value)} placeholder="https://www.youtube.com/watch?v=...&t=1m23s or https://youtu.be/..." aria-label="YouTube URL" + disabled={!canUseInstructorControls} /> Shared URLs can include `t`, `start`, and `end` timestamps like `1m23s`. @@ -1139,6 +1174,7 @@ export default function VideoSyncManager() { onChange={(event) => setHasStopTime(event.target.checked)} aria-controls="video-sync-stop-time" aria-expanded={hasStopTime} + disabled={!canUseInstructorControls} /> Set stop time @@ -1153,6 +1189,7 @@ export default function VideoSyncManager() { onChange={(event) => setStopSecInput(event.target.value)} placeholder="2m10s or 130" aria-label="Stop at" + disabled={!canUseInstructorControls} /> Accepts seconds or `h/m/s` format. @@ -1160,7 +1197,7 @@ export default function VideoSyncManager() { ) : null} -
@@ -1206,6 +1243,14 @@ export default function VideoSyncManager() { {state.videoId ? (
+ {!canUseInstructorControls ? ( +
+
+

Take control to use playback controls for this session.

+ +
+
+ ) : null}
) : (
diff --git a/activities/video-sync/server/routes.test.ts b/activities/video-sync/server/routes.test.ts index 77a25269..ffa82e17 100644 --- a/activities/video-sync/server/routes.test.ts +++ b/activities/video-sync/server/routes.test.ts @@ -14,6 +14,8 @@ import setupVideoSyncRoutes, { waitForInstructorAuthMessage } from './routes.js' const TEST_INSTRUCTOR_PASSCODE = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' const ALT_TEST_INSTRUCTOR_PASSCODE = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb' +const TEST_INSTRUCTOR_INSTANCE_ID = 'inst-123' +const ALT_TEST_INSTRUCTOR_INSTANCE_ID = 'inst-456' type RouteHandler = ( req: { params: Record; body?: unknown; cookies?: Record }, @@ -981,7 +983,7 @@ void test('session patch returns invalid source url for unsupported non-YouTube await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://vimeo.com/1234', instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://vimeo.com/1234', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1007,7 +1009,7 @@ void test('session patch returns invalid source url for malformed url input', as await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'not a url', instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'not a url', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1033,7 +1035,7 @@ void test('session patch returns invalid video id for YouTube url without a usab await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://www.youtube.com/watch?list=abc123', instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://www.youtube.com/watch?list=abc123', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1059,7 +1061,7 @@ void test('session patch accepts youtu.be urls with extra path segments by using await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ/extra-segment?t=45', instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ/extra-segment?t=45', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1085,7 +1087,7 @@ void test('session patch returns invalid video id for malformed youtu.be ids', a await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgX$Q', instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgX$Q', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1111,7 +1113,7 @@ void test('session patch returns invalid time range when stop time is before par await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 20, instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 20, instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1137,7 +1139,7 @@ void test('session patch returns invalid stopSec when stopSec is not numeric', a await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: '120', instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: '120', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1163,7 +1165,7 @@ void test('session patch normalizes youtube source and publishes extensible enve await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1209,6 +1211,7 @@ void test('session patch can mark a configured session as standalone', async () body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, + instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, standaloneMode: true, }, }, @@ -1238,8 +1241,10 @@ void test('session patch can mark a configured session as standalone', async () telemetry: updated.telemetry, controlAuthority: { mode: 'single-instructor', - ownerInstanceId: null, - ownerTakenAt: null, + ownerInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, + ownerTakenAt: (updated.controlAuthority != null && typeof updated.controlAuthority === 'object') + ? (updated.controlAuthority as { ownerTakenAt?: unknown }).ownerTakenAt + : undefined, overrideInherited: false, }, }, @@ -1265,6 +1270,7 @@ void test('session patch preserves existing standaloneMode when request omits th body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, + instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, }, }, res, @@ -1290,7 +1296,7 @@ void test('session patch ignores partially numeric timestamp query values', asyn await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=83abc', instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=83abc', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1317,7 +1323,7 @@ void test('session patch falls back to valid t param when start param is malform await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?start=oops&t=1m23s', instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?start=oops&t=1m23s', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1348,7 +1354,7 @@ void test('session patch rejects reconfiguration after a video is already set', await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1376,7 +1382,7 @@ void test('session patch publishes through broadcast channel without direct loca await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1425,7 +1431,7 @@ void test('session patch falls back to direct local websocket send when pubsub p await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1485,7 +1491,7 @@ void test('session patch falls back to direct local websocket send when publishB await handler?.( { params: { sessionId: 's1' }, - body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', stopSec: 120, instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1519,7 +1525,7 @@ void test('command route updates playback and emits extensible envelope', async await handler?.( { params: { sessionId: 's1' }, - body: { type: 'play', instructorPasscode: TEST_INSTRUCTOR_PASSCODE }, + body: { type: 'play', instructorPasscode: TEST_INSTRUCTOR_PASSCODE, instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID }, }, res, ) @@ -1541,6 +1547,75 @@ void test('command route updates playback and emits extensible envelope', async assert.equal(message.type, 'state-update') }) +void test('command route rejects non-owner instructor instances after control is claimed', async () => { + const app = createMockApp() + const ws = createMockWs() as unknown as WsRouter + const session = createVideoSyncSession('s1') + ;(session.data as Record).controlAuthority = { + mode: 'single-instructor', + ownerInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, + ownerTakenAt: 123, + overrideInherited: false, + } + const storeState = createSessionStore({ s1: session }) + + setupVideoSyncRoutes(app, storeState.sessions, ws) + + const handler = app.handlers.post['/api/video-sync/:sessionId/command'] + assert.equal(typeof handler, 'function') + + const res = createResponse() + await handler?.( + { + params: { sessionId: 's1' }, + body: { + type: 'play', + instructorPasscode: TEST_INSTRUCTOR_PASSCODE, + instructorInstanceId: ALT_TEST_INSTRUCTOR_INSTANCE_ID, + }, + }, + res, + ) + + assert.equal(res.statusCode, 403) + assert.deepEqual(res.body, { + error: 'CONTROL_AUTHORITY_REQUIRED', + message: 'Take control to use instructor controls for this session.', + }) +}) + +void test('session patch rejects embedded child updates until the instructor explicitly takes control', async () => { + const app = createMockApp() + const ws = createMockWs() as unknown as WsRouter + const session = createVideoSyncSession('child-1') + ;(session.data as Record).embeddedParentSessionId = 'parent-syncdeck' + const storeState = createSessionStore({ 'child-1': session }) + + setupVideoSyncRoutes(app, storeState.sessions, ws) + + const handler = app.handlers.patch['/api/video-sync/:sessionId/session'] + assert.equal(typeof handler, 'function') + + const res = createResponse() + await handler?.( + { + params: { sessionId: 'child-1' }, + body: { + sourceUrl: 'https://youtu.be/dQw4w9WgXcQ?t=43', + instructorPasscode: TEST_INSTRUCTOR_PASSCODE, + instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, + }, + }, + res, + ) + + assert.equal(res.statusCode, 403) + assert.deepEqual(res.body, { + error: 'CONTROL_AUTHORITY_REQUIRED', + message: 'Take control to use instructor controls for this session.', + }) +}) + void test('session patch rejects requests without a valid instructor passcode', async () => { const app = createMockApp() const ws = createMockWs() as unknown as WsRouter @@ -1948,6 +2023,7 @@ void test('instructor websocket rejects connections without a valid instructor p const recorder = createMockSocket() handler?.(recorder.socket, new URLSearchParams({ + instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', })) @@ -1980,6 +2056,7 @@ void test('instructor websocket rejects oversized instructor passcodes before ve const recorder = createMockSocket() handler?.(recorder.socket, new URLSearchParams({ + instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', })) @@ -2018,6 +2095,7 @@ void test('instructor websocket accepts connections with a valid instructor pass const recorder = createMockSocket() handler?.(recorder.socket, new URLSearchParams({ + instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', })) @@ -2036,6 +2114,10 @@ void test('instructor websocket accepts connections with a valid instructor pass const telemetryEnvelope = JSON.parse(recorder.sent[1] ?? '{}') as { type?: string; payload?: { reason?: string } } assert.equal(telemetryEnvelope.type, 'telemetry-update') assert.equal(telemetryEnvelope.payload?.reason, 'connection-change') + const persistedControlAuthority = storeState.store.s1?.data as { + controlAuthority?: { ownerInstanceId?: string | null } + } + assert.equal(persistedControlAuthority.controlAuthority?.ownerInstanceId, TEST_INSTRUCTOR_INSTANCE_ID) recorder.emit('close') await new Promise((resolve) => setTimeout(resolve, 0)) }) @@ -2052,6 +2134,7 @@ void test('instructor websocket accepts uppercase instructor passcodes by canoni const recorder = createMockSocket() handler?.(recorder.socket, new URLSearchParams({ + instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', })) @@ -2080,6 +2163,7 @@ void test('legacy manager websocket role is normalized to instructor', async () const recorder = createMockSocket() handler?.(recorder.socket, new URLSearchParams({ + instructorInstanceId: TEST_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'manager', })) diff --git a/activities/video-sync/server/routes.ts b/activities/video-sync/server/routes.ts index 62106153..5834f5ab 100644 --- a/activities/video-sync/server/routes.ts +++ b/activities/video-sync/server/routes.ts @@ -112,6 +112,7 @@ interface VideoSyncWsMessageEnvelope { interface VideoSyncSocket extends ActiveBitsWebSocket { sessionId?: string | null videoSyncRole?: VideoSyncRole + instructorInstanceId?: string | null } interface VideoSyncInstructorAuthMessage { @@ -219,6 +220,61 @@ function readInstructorInstanceId(body: unknown): string | null { return normalizeInstructorInstanceId(body.instructorInstanceId) } +function buildControlAuthorityRequiredError(): { error: 'CONTROL_AUTHORITY_REQUIRED'; message: string } { + return { + error: 'CONTROL_AUTHORITY_REQUIRED', + message: 'Take control to use instructor controls for this session.', + } +} + +function ensureInstructorControlAuthority(params: { + session: VideoSyncSession + instructorInstanceId: string | null + allowInlineAutoClaim: boolean +}): { ok: true } | { ok: false; status: number; body: { error: string; message: string } } { + const normalizedInstructorInstanceId = normalizeInstructorInstanceId(params.instructorInstanceId) + if (!normalizedInstructorInstanceId) { + return { + ok: false, + status: 400, + body: { + error: 'INVALID_INSTRUCTOR_INSTANCE_ID', + message: 'Valid instructorInstanceId is required', + }, + } + } + + const embeddedParentContext = readEmbeddedParentSessionContext(params.session.data) + const controlAuthority = getSessionControlAuthorityState(params.session) + + if (controlAuthority.ownerInstanceId == null) { + if (!params.allowInlineAutoClaim || embeddedParentContext != null) { + return { + ok: false, + status: 403, + body: buildControlAuthorityRequiredError(), + } + } + + claimSessionControlAuthority({ + session: params.session, + instructorInstanceId: normalizedInstructorInstanceId, + overrideInherited: false, + }) + return { ok: true } + } + + if (controlAuthority.ownerInstanceId !== normalizedInstructorInstanceId) { + return { + ok: false, + status: 403, + body: buildControlAuthorityRequiredError(), + } + } + + return { ok: true } +} + function verifyInstructorPasscode(expected: string, candidate: string): boolean { if ( expected.length !== INSTRUCTOR_PASSCODE_LENGTH || @@ -925,6 +981,7 @@ function scheduleUnsyncedStudentsPrune( console.error(`Failed to prune stale video-sync unsynced students for session ${sessionId}:`, error) }) }, delayMs) + timer.unref?.() unsyncedStudentPruneTimersBySession.set(sessionId, timer) } @@ -1418,6 +1475,15 @@ export default function setupVideoSyncRoutes( res.status(403).json({ error: 'FORBIDDEN', message: 'Valid instructorPasscode is required' }) return } + const instructorAuthority = ensureInstructorControlAuthority({ + session, + instructorInstanceId: readInstructorInstanceId(req.body), + allowInlineAutoClaim: true, + }) + if (!instructorAuthority.ok) { + res.status(instructorAuthority.status).json(instructorAuthority.body) + return + } const data = ensureVideoSyncSessionData(session) if (data.state.videoId.length > 0) { @@ -1541,6 +1607,15 @@ export default function setupVideoSyncRoutes( res.status(403).json({ error: 'FORBIDDEN', message: 'Valid instructorPasscode is required' }) return } + const instructorAuthority = ensureInstructorControlAuthority({ + session, + instructorInstanceId: readInstructorInstanceId(req.body), + allowInlineAutoClaim: true, + }) + if (!instructorAuthority.ok) { + res.status(instructorAuthority.status).json(instructorAuthority.body) + return + } const body = isPlainObject(req.body) ? (req.body as CommandBody) : {} if (!isCommandType(body.type)) { @@ -1813,12 +1888,27 @@ export default function setupVideoSyncRoutes( const role: VideoSyncRole = isInstructorRoleParam(roleParam) ? 'instructor' : 'student' typedSocket.sessionId = sessionId typedSocket.videoSyncRole = role + typedSocket.instructorInstanceId = role === 'instructor' + ? normalizeInstructorInstanceId(query.get('instructorInstanceId')) + : null if (cleanedUp || typedSocket.readyState !== WS_OPEN_READY_STATE) { handleSocketClosed() return } + if (role === 'instructor') { + const embeddedParentContext = readEmbeddedParentSessionContext(session.data) + const controlAuthority = getSessionControlAuthorityState(session) + if (controlAuthority.ownerInstanceId == null && embeddedParentContext == null && typedSocket.instructorInstanceId) { + claimSessionControlAuthority({ + session, + instructorInstanceId: typedSocket.instructorInstanceId, + overrideInherited: false, + }) + } + } + ensureBroadcastSubscription(sessionId) upsertSubscriber(sessionId, typedSocket) isSubscribed = true From 4c19cb1f5f3530631c8388ce7e37226fb298de54 Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Wed, 29 Apr 2026 05:08:17 +0000 Subject: [PATCH 06/15] Add syncdeck control authority foundation --- .agent/knowledge/data-contracts.md | 9 ++ .agent/plans/control-authority-plan.md | 12 +- activities/syncdeck/activity.config.ts | 5 + .../client/manager/SyncDeckManager.test.tsx | 22 +++ .../client/manager/SyncDeckManager.tsx | 146 +++++++++++++++++- .../manager/SyncDeckManager.wsAuth.test.ts | 5 +- activities/syncdeck/server/routes.test.ts | 109 +++++++++++++ activities/syncdeck/server/routes.ts | 92 +++++++++++ activities/video-sync/activity.config.ts | 5 + 9 files changed, 394 insertions(+), 11 deletions(-) diff --git a/.agent/knowledge/data-contracts.md b/.agent/knowledge/data-contracts.md index 535ec638..d62120d6 100644 --- a/.agent/knowledge/data-contracts.md +++ b/.agent/knowledge/data-contracts.md @@ -15,6 +15,15 @@ Document API and data-shape assumptions that must stay compatible over time. ## Contracts +- Date: 2026-04-29 +- Surface: internal module | activity interface +- Contract: Activity-owned session normalizers that return a typed `data` object must preserve the shared `controlAuthority` record whenever the activity opts into single-instructor control ownership. +- Compatibility constraints: Shared ownership helpers store authority in `session.data.controlAuthority`, but activity-local normalizers like SyncDeck's may rebuild `data` field-by-field. If they omit `controlAuthority`, ownership appears to claim successfully in one code path and then vanishes on the next normalized read or websocket update. +- Validation rules: Normalization should round-trip `{ mode, ownerInstanceId, ownerTakenAt, overrideInherited }`, defaulting through the shared control-authority normalizer rather than trusting arbitrary persisted values. +- Evidence (schema/tests/path): `server/controlAuthority.ts`; `activities/syncdeck/server/routes.ts`; `activities/syncdeck/server/routes.test.ts` +- Follow-up action: When another activity with its own `normalizeSessionData(...)` adopts control authority, audit that normalizer immediately instead of assuming shared helper writes are enough. +- Owner: Codex + - Date: 2026-04-17 - Surface: activity interface | websocket | internal module - Contract: SyncDeck student-side instructor sync suppression must derive incoming slide indices from all Reveal state shapes that can drive a `setState`, including `payload.indices`, `payload.navigation.current`, `payload.revealState`, and top-level state fields. Same-horizontal vertical instructor moves must remain suppressible even when the deck emits `revealState` without a separate `indices` object. diff --git a/.agent/plans/control-authority-plan.md b/.agent/plans/control-authority-plan.md index 4022a2b5..b4ff1e69 100644 --- a/.agent/plans/control-authority-plan.md +++ b/.agent/plans/control-authority-plan.md @@ -218,15 +218,16 @@ Shared enforcement rule: ### 6. Activity adoption -- [ ] Add `controlAuthority` config to SyncDeck. +- [x] Add `controlAuthority` config to SyncDeck. - [ ] Decide SyncDeck gating mode. Current expectation: `gating: 'activity'`. -- [ ] Add `controlAuthority` config to Video Sync. +- [x] Add `controlAuthority` config to Video Sync. - [x] Decide Video Sync gating mode. Current expectation: `gating: 'activity'` or `all`, depending on final command surface. - [ ] Implement activity-owned command classifiers where `gating: 'activity'` is used. - [x] Ensure embedded Video Sync defaults to inherited authority when launched under SyncDeck. - [x] Ensure embedded Video Sync can be locally overridden by explicit `Take Control`. - [x] Enforce Video Sync non-owner command rejection for config and playback command routes. - [x] Disable Video Sync gated controls locally with explicit takeover feedback. +- [x] Add first-pass SyncDeck manager authority plumbing: instructor instance identity, takeover route, websocket authority status, and local gating for the websocket relay controls. ### 7. Validation @@ -249,15 +250,22 @@ Implemented on this branch so far: - shared `controlAuthority` activity config shape and schema validation - shared authority state normalization, ownership helpers, and inherited/session resolution helpers - durable browser-plus-tab instructor identity generation for manager clients +- activity config opt-in for Video Sync (`inherited`) and SyncDeck (`session`) - Video Sync takeover endpoint and embedded-child local override handling - Video Sync manager authority status plumbing and `Take Control` UI wiring - Video Sync protocol/session payload support for `controlAuthority` - Video Sync server-side owner checks for playback/config commands plus standalone first-owner auto-claim - Video Sync manager disabled-state enforcement and takeover feedback for non-owners - Video Sync unsynced-student prune timers now `unref()` so maintenance timers do not keep tests or Node processes alive unnecessarily +- SyncDeck manager websocket identity + authority status plumbing +- SyncDeck takeover endpoint plus first-owner auto-claim on instructor websocket auth +- SyncDeck session normalization now preserves shared `controlAuthority` state instead of dropping it on activity-local normalization +- SyncDeck manager shows first-pass control status UI and blocks non-owner websocket relay controls locally +- SyncDeck websocket relay path rejects non-owner instructor updates and reflects current ownership back to the non-owner socket Still pending before this feature is complete: - shared/generic first-instructor auto-ownership support beyond the current Video Sync adoption +- broader SyncDeck gating coverage for non-websocket manager actions like configure and embedded activity lifecycle controls - SyncDeck adoption and command classification - websocket broadcast/update path for authority changes across instructor views - browser-level E2E coverage for the multi-instructor handoff scenario diff --git a/activities/syncdeck/activity.config.ts b/activities/syncdeck/activity.config.ts index e69a22b1..eb8c5057 100644 --- a/activities/syncdeck/activity.config.ts +++ b/activities/syncdeck/activity.config.ts @@ -57,6 +57,11 @@ const syncdeckConfig: ActivityConfig = { }, ], }, + controlAuthority: { + mode: 'single-instructor', + scope: 'session', + gating: 'all', + }, manageDashboard: { customPersistentLinkBuilder: true, }, diff --git a/activities/syncdeck/client/manager/SyncDeckManager.test.tsx b/activities/syncdeck/client/manager/SyncDeckManager.test.tsx index 379028de..255164fc 100644 --- a/activities/syncdeck/client/manager/SyncDeckManager.test.tsx +++ b/activities/syncdeck/client/manager/SyncDeckManager.test.tsx @@ -43,6 +43,7 @@ import { resolveManagerActivityRequestBatchInputs } from './SyncDeckManager.js' import { resolveManagerPreloadRequestBatchInputs } from './SyncDeckManager.js' import { processManagerBundlePreloadRequests } from './SyncDeckManager.js' import { processManagerPreloadRequests } from './SyncDeckManager.js' +import { buildSyncDeckInstructorWsUrl } from './SyncDeckManager.js' import { runEmbeddedStartWithPendingRetry } from './SyncDeckManager.js' import { resolveCompletedEmbeddedBootstrapChildSessionIds } from './SyncDeckManager.js' import { resolveEmbeddedBootstrapBackfillRetryDelayMs } from './SyncDeckManager.js' @@ -113,6 +114,27 @@ void test('SyncDeckManager pre-fills presentation URL from query params', () => assert.match(html, /value="https:\/\/slides\.example\/deck"/i) }) +void test('buildSyncDeckInstructorWsUrl includes instructor instance identity for manager sockets', () => { + assert.equal( + buildSyncDeckInstructorWsUrl({ + sessionId: 'session-123', + instructorInstanceId: 'inst-123', + location: { protocol: 'https:', host: 'bits.example.test' }, + isConfigurePanelOpen: false, + }), + 'wss://bits.example.test/ws/syncdeck?instructorInstanceId=inst-123&sessionId=session-123&role=instructor', + ) + assert.equal( + buildSyncDeckInstructorWsUrl({ + sessionId: 'session-123', + instructorInstanceId: null, + location: { protocol: 'https:', host: 'bits.example.test' }, + isConfigurePanelOpen: false, + }), + null, + ) +}) + void test('activitySupportsEmbeddedReport reflects shared activity config metadata', () => { const entries = resolveSyncDeckActivityPickerEntries([ { id: 'gallery-walk', name: 'Gallery Walk', description: 'Peer feedback', reportEndpoint: '/api/gallery-walk/:sessionId/report' }, diff --git a/activities/syncdeck/client/manager/SyncDeckManager.tsx b/activities/syncdeck/client/manager/SyncDeckManager.tsx index 53cceff8..5c8c5ea2 100644 --- a/activities/syncdeck/client/manager/SyncDeckManager.tsx +++ b/activities/syncdeck/client/manager/SyncDeckManager.tsx @@ -1,4 +1,5 @@ import { useResilientWebSocket } from '@src/hooks/useResilientWebSocket' +import { resolveOrCreateInstructorControlInstanceId } from '@src/components/common/instructorControlIdentity' import { storeCreateSessionBootstrapPayload } from '@src/components/common/manageDashboardUtils' import { resolvePersistentSessionEntryPolicy, type PersistentSessionEntryPolicy } from '../../../../types/waitingRoom.js' import { runSyncDeckPresentationPreflight } from '../shared/presentationPreflight.js' @@ -99,6 +100,28 @@ interface SyncDeckInstructorAuthMessage { instructorPasscode: string } +interface SyncDeckControlAuthority { + mode: 'single-instructor' + ownerInstanceId: string | null + ownerTakenAt: number | null + overrideInherited: boolean +} + +const EMPTY_SYNCDECK_CONTROL_AUTHORITY: SyncDeckControlAuthority = { + mode: 'single-instructor', + ownerInstanceId: null, + ownerTakenAt: null, + overrideInherited: false, +} + +function createInstructorControlId(): string { + if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') { + return crypto.randomUUID() + } + + return `${Date.now()}-${Math.random().toString(16).slice(2)}` +} + interface RevealCommandPayload { [key: string]: unknown } @@ -160,15 +183,17 @@ function isPlainObject(value: unknown): value is Record { export function buildSyncDeckInstructorWsUrl(params: { sessionId: string | null | undefined + instructorInstanceId: string | null | undefined location: Pick | null | undefined isConfigurePanelOpen: boolean }): string | null { - if (!params.sessionId || params.location == null || params.isConfigurePanelOpen) { + if (!params.sessionId || !params.instructorInstanceId || params.location == null || params.isConfigurePanelOpen) { return null } const protocol = params.location.protocol === 'https:' ? 'wss:' : 'ws:' const query = new URLSearchParams({ + instructorInstanceId: params.instructorInstanceId, sessionId: params.sessionId, role: 'instructor', }) @@ -189,6 +214,28 @@ export function createSyncDeckInstructorWsAuthMessage( return JSON.stringify(message) } +function isSyncDeckControlOwner( + controlAuthority: SyncDeckControlAuthority | null | undefined, + instructorInstanceId: string | null | undefined, +): boolean { + return ( + controlAuthority?.ownerInstanceId != null && + instructorInstanceId != null && + controlAuthority.ownerInstanceId === instructorInstanceId + ) +} + +function canUseSyncDeckInstructorControls( + controlAuthority: SyncDeckControlAuthority | null | undefined, + instructorInstanceId: string | null | undefined, +): boolean { + if (controlAuthority?.ownerInstanceId == null) { + return true + } + + return isSyncDeckControlOwner(controlAuthority, instructorInstanceId) +} + function stripOverviewFromStateEnvelope(data: unknown): unknown { if ( data == null || @@ -1817,6 +1864,16 @@ const SyncDeckManager: FC = () => { const { sessionId } = useParams<{ sessionId?: string }>() const location = useLocation() const navigate = useNavigate() + const instructorInstanceId = useMemo(() => { + if (sessionId == null || typeof window === 'undefined') { + return null + } + + return resolveOrCreateInstructorControlInstanceId({ + localStorage: window.localStorage, + sessionStorage: window.sessionStorage, + }, createInstructorControlId) + }, [sessionId]) const [copiedValue, setCopiedValue] = useState(null) const [presentationUrl, setPresentationUrl] = useState(() => { const params = new URLSearchParams(location.search) @@ -1834,6 +1891,7 @@ const SyncDeckManager: FC = () => { const [hasAutoStarted, setHasAutoStarted] = useState(false) const [instructorConnectionState, setInstructorConnectionState] = useState<'connected' | 'disconnected'>('disconnected') const [instructorConnectionTooltip, setInstructorConnectionTooltip] = useState('Not connected to sync server') + const [controlAuthority, setControlAuthority] = useState(EMPTY_SYNCDECK_CONTROL_AUTHORITY) const [isInstructorSyncEnabled, setIsInstructorSyncEnabled] = useState(true) const [connectedStudentCount, setConnectedStudentCount] = useState(0) const [students, setStudents] = useState([]) @@ -2127,17 +2185,26 @@ const SyncDeckManager: FC = () => { } }, [presentationUrlError, isConfigurePanelOpen]) + const hasControl = isSyncDeckControlOwner(controlAuthority, instructorInstanceId) + const canUseInstructorControls = canUseSyncDeckInstructorControls(controlAuthority, instructorInstanceId) + const controlStatusLabel = hasControl + ? 'You have control' + : controlAuthority.ownerInstanceId + ? 'Another instructor has control' + : 'Take control to present from this manager' + const buildInstructorWsUrl = useCallback((): string | null => { if (typeof window === 'undefined') { return null } return buildSyncDeckInstructorWsUrl({ + instructorInstanceId, sessionId, location: window.location, isConfigurePanelOpen, }) - }, [sessionId, isConfigurePanelOpen]) + }, [instructorInstanceId, sessionId, isConfigurePanelOpen]) const { connect: connectInstructorWs, disconnect: disconnectInstructorWs, socketRef: instructorSocketRef } = useResilientWebSocket({ @@ -2168,7 +2235,22 @@ const SyncDeckManager: FC = () => { }, onMessage: (event) => { try { - const message = JSON.parse(event.data) as SyncDeckStudentPresenceMessage + const message = JSON.parse(event.data) as SyncDeckStudentPresenceMessage & { + payload?: SyncDeckControlAuthority | SyncDeckStudentPresenceMessage['payload'] + } + if (message.type === 'syncdeck-control-authority') { + const payload = message.payload + if (payload && typeof payload === 'object') { + const controlPayload = payload as Record + setControlAuthority({ + mode: 'single-instructor', + ownerInstanceId: typeof controlPayload.ownerInstanceId === 'string' ? controlPayload.ownerInstanceId : null, + ownerTakenAt: typeof controlPayload.ownerTakenAt === 'number' && Number.isFinite(controlPayload.ownerTakenAt) ? controlPayload.ownerTakenAt : null, + overrideInherited: controlPayload.overrideInherited === true, + }) + } + return + } const statePayload = extractSyncDeckStatePayload(message) if (statePayload != null) { const embeddedLifecyclePayload = parseEmbeddedLifecyclePayload(statePayload) @@ -2625,6 +2707,35 @@ const SyncDeckManager: FC = () => { setTimeout(() => setCopiedValue((current) => (current === value ? null : current)), 1500) } + const takeControl = async (): Promise => { + if (!sessionId || !instructorPasscode || !instructorInstanceId) { + return + } + + try { + const response = await fetch(`/api/syncdeck/${encodeURIComponent(sessionId)}/control-authority/take`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + instructorPasscode, + instructorInstanceId, + }), + }) + const payload = await response.json() as { controlAuthority?: SyncDeckControlAuthority } + if (!response.ok) { + throw new Error('Unable to take control.') + } + if (payload.controlAuthority) { + setControlAuthority(payload.controlAuthority) + } + setStartError(null) + setStartSuccess(null) + } catch { + setStartError('Unable to take control right now.') + setStartSuccess(null) + } + } + const handleEndSession = async (): Promise => { if (!sessionId) return @@ -2654,6 +2765,12 @@ const SyncDeckManager: FC = () => { return } + if (!canUseInstructorControls) { + setStartError('Take control to use instructor presentation controls.') + setStartSuccess(null) + return + } + const socket = instructorSocketRef.current if (!socket || socket.readyState !== WS_OPEN_READY_STATE) { return @@ -4006,7 +4123,7 @@ const SyncDeckManager: FC = () => { }`} title={isInstructorSyncEnabled ? 'Disable instructor sync' : 'Enable instructor sync'} aria-label={isInstructorSyncEnabled ? 'Disable instructor sync' : 'Enable instructor sync'} - disabled={isConfigurePanelOpen} + disabled={isConfigurePanelOpen || !canUseInstructorControls} > 🔗 @@ -4016,7 +4133,7 @@ const SyncDeckManager: FC = () => { className="ml-2 px-2 py-1 rounded border border-gray-300 text-gray-700 hover:bg-gray-50 disabled:opacity-50 disabled:cursor-not-allowed" title="Force sync students to current position" aria-label="Force sync students to current position" - disabled={isConfigurePanelOpen || instructorConnectionState !== 'connected' || !isInstructorSyncEnabled} + disabled={isConfigurePanelOpen || instructorConnectionState !== 'connected' || !isInstructorSyncEnabled || !canUseInstructorControls} > 📍 @@ -4030,7 +4147,7 @@ const SyncDeckManager: FC = () => { }`} title={isPresentationPaused ? 'Resume presentation' : 'Pause presentation'} aria-label={isPresentationPaused ? 'Resume presentation' : 'Pause presentation'} - disabled={isConfigurePanelOpen} + disabled={isConfigurePanelOpen || !canUseInstructorControls} > ⬛ @@ -4044,7 +4161,7 @@ const SyncDeckManager: FC = () => { }`} title="Toggle chalkboard screen" aria-label="Toggle chalkboard screen" - disabled={isConfigurePanelOpen} + disabled={isConfigurePanelOpen || !canUseInstructorControls} > 🖍️ @@ -4058,7 +4175,7 @@ const SyncDeckManager: FC = () => { }`} title="Toggle pen overlay" aria-label="Toggle pen overlay" - disabled={isConfigurePanelOpen} + disabled={isConfigurePanelOpen || !canUseInstructorControls} > ✏️ @@ -4092,6 +4209,19 @@ const SyncDeckManager: FC = () => { > Students: {connectedStudentCount} +
+ {controlStatusLabel} + +
Join Code: { assert.equal( buildSyncDeckInstructorWsUrl({ + instructorInstanceId: 'inst-123', sessionId: 'session-123', location: { protocol: 'https:', @@ -15,13 +16,14 @@ void test('buildSyncDeckInstructorWsUrl omits instructor credentials from websoc }, isConfigurePanelOpen: false, }), - 'wss://bits.example.test/ws/syncdeck?sessionId=session-123&role=instructor', + 'wss://bits.example.test/ws/syncdeck?instructorInstanceId=inst-123&sessionId=session-123&role=instructor', ) }) void test('buildSyncDeckInstructorWsUrl returns null when configure panel is open or session missing', () => { assert.equal( buildSyncDeckInstructorWsUrl({ + instructorInstanceId: 'inst-123', sessionId: 'session-123', location: { protocol: 'https:', @@ -33,6 +35,7 @@ void test('buildSyncDeckInstructorWsUrl returns null when configure panel is ope ) assert.equal( buildSyncDeckInstructorWsUrl({ + instructorInstanceId: 'inst-123', sessionId: null, location: { protocol: 'https:', diff --git a/activities/syncdeck/server/routes.test.ts b/activities/syncdeck/server/routes.test.ts index 623f161f..67be74fc 100644 --- a/activities/syncdeck/server/routes.test.ts +++ b/activities/syncdeck/server/routes.test.ts @@ -20,6 +20,8 @@ import '../../resonance/server/routes.js' import '../../video-sync/server/routes.js' const DEFAULT_SYNCDECK_ENTRY_POLICY = 'instructor-required' +const PRIMARY_INSTRUCTOR_INSTANCE_ID = 'inst-primary' +const PEER_INSTRUCTOR_INSTANCE_ID = 'inst-peer' interface RouteRequest { params: Record @@ -602,6 +604,7 @@ void test('syncdeck websocket sends latest state snapshot to instructor on conne handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -662,6 +665,7 @@ void test('syncdeck websocket replays existing embedded activity starts to instr handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -749,6 +753,7 @@ void test('syncdeck websocket stops replaying embedded activity starts after soc handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -789,6 +794,7 @@ void test('syncdeck websocket does not issue forbidden close after auth wait res handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -832,6 +838,7 @@ void test('syncdeck websocket sends latest position snapshot to instructor when handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -883,6 +890,7 @@ void test('syncdeck websocket relays instructor updates to students in session', handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -941,6 +949,7 @@ void test('syncdeck websocket relays instructor updates to other instructors in handler?.( primaryInstructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -950,6 +959,7 @@ void test('syncdeck websocket relays instructor updates to other instructors in handler?.( peerInstructorSocket, new URLSearchParams({ + instructorInstanceId: PEER_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -981,6 +991,68 @@ void test('syncdeck websocket relays instructor updates to other instructors in ) }) +void test('syncdeck websocket ignores non-owner instructor updates after another instructor auto-claims control', async () => { + const app = createMockApp() + const ws = createMockWs() + const state = createSessionStore({ + s1: createSyncDeckSession('s1', 'teacher-pass'), + }) + + setupSyncDeckRoutes(app, state.sessions, ws) + const handler = ws.registered['/ws/syncdeck'] + assert.equal(typeof handler, 'function') + + const primaryInstructorSocket = new MockSocket() + const peerInstructorSocket = new MockSocket() + ws.wss.clients.add(primaryInstructorSocket) + ws.wss.clients.add(peerInstructorSocket) + + handler?.( + primaryInstructorSocket, + new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, + sessionId: 's1', + role: 'instructor', + }), + ws.wss, + ) + emitInstructorAuth(primaryInstructorSocket, 'teacher-pass') + + handler?.( + peerInstructorSocket, + new URLSearchParams({ + instructorInstanceId: PEER_INSTRUCTOR_INSTANCE_ID, + sessionId: 's1', + role: 'instructor', + }), + ws.wss, + ) + emitInstructorAuth(peerInstructorSocket, 'teacher-pass') + await new Promise((resolve) => setTimeout(resolve, 0)) + + peerInstructorSocket.emit( + 'message', + JSON.stringify({ + type: 'syncdeck-state-update', + payload: { type: 'slidechanged', payload: { h: 9, v: 0, f: 0 } }, + }), + ) + await new Promise((resolve) => setTimeout(resolve, 0)) + + const updatedSession = state.store.s1?.data as { + controlAuthority?: { ownerInstanceId?: string | null } + lastInstructorPayload?: unknown + } + assert.equal(updatedSession.controlAuthority?.ownerInstanceId, PRIMARY_INSTRUCTOR_INSTANCE_ID) + assert.equal(updatedSession.lastInstructorPayload, null) + + const controlAuthorityMessages = peerInstructorSocket.sent + .map((entry) => JSON.parse(entry) as { type?: string; payload?: { ownerInstanceId?: string | null } }) + .filter((entry) => entry.type === 'syncdeck-control-authority') + assert.ok(controlAuthorityMessages.length >= 2) + assert.equal(controlAuthorityMessages[controlAuthorityMessages.length - 1]?.payload?.ownerInstanceId, PRIMARY_INSTRUCTOR_INSTANCE_ID) +}) + void test('syncdeck websocket replays buffered chalkboard snapshot and delta to student on connect', async () => { const app = createMockApp() const ws = createMockWs() @@ -1097,6 +1169,7 @@ void test('syncdeck websocket replays buffered chalkboard snapshot and delta to handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -1216,6 +1289,7 @@ void test('syncdeck websocket updates and clears chalkboard buffer from instruct handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -1347,6 +1421,7 @@ void test('syncdeck websocket persists drawing tool mode updates from instructor handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -1403,6 +1478,7 @@ void test('syncdeck websocket broadcasts student presence count to instructor', handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -1476,6 +1552,7 @@ void test('syncdeck session normalization filters malformed persisted students a handler?.( instructorSocket, new URLSearchParams({ + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, sessionId: 's1', role: 'instructor', }), @@ -2163,6 +2240,38 @@ void test('embedded-context route rejects unknown parent identity', async () => assert.deepEqual(res.body, { error: 'forbidden' }) }) +void test('control-authority take route claims syncdeck control for the requesting instructor instance', async () => { + const app = createMockApp() + const ws = createMockWs() + const state = createSessionStore({ + s1: createSyncDeckSession('s1', 'teacher-pass'), + }) + + setupSyncDeckRoutes(app, state.sessions, ws) + const handler = app.handlers.post['/api/syncdeck/:sessionId/control-authority/take'] + assert.equal(typeof handler, 'function') + + const res = createResponse() + await handler?.( + { + params: { sessionId: 's1' }, + body: { + instructorPasscode: 'teacher-pass', + instructorInstanceId: PRIMARY_INSTRUCTOR_INSTANCE_ID, + }, + }, + res, + ) + + assert.equal(res.statusCode, 200) + assert.equal((res.body as { controlAuthority?: { ownerInstanceId?: string | null } }).controlAuthority?.ownerInstanceId, PRIMARY_INSTRUCTOR_INSTANCE_ID) + + const persisted = state.store.s1?.data as { + controlAuthority?: { ownerInstanceId?: string | null } + } + assert.equal(persisted.controlAuthority?.ownerInstanceId, PRIMARY_INSTRUCTOR_INSTANCE_ID) +}) + void test('embedded-activity start route creates a child session, stores keyed map state, and broadcasts tokens', async () => { const app = createMockApp() const ws = createMockWs() diff --git a/activities/syncdeck/server/routes.ts b/activities/syncdeck/server/routes.ts index 2c2c538d..e4d2f0f0 100644 --- a/activities/syncdeck/server/routes.ts +++ b/activities/syncdeck/server/routes.ts @@ -25,6 +25,12 @@ import { import { storeSessionEntryParticipant } from 'activebits-server/core/sessionEntryParticipants.js' import { randomBytes, timingSafeEqual } from 'node:crypto' import type { ActiveBitsWebSocket, WsRouter } from '../../../types/websocket.js' +import { + claimSessionControlAuthority, + getSessionControlAuthorityState, + normalizeSessionControlAuthorityState, + normalizeInstructorInstanceId, +} from '../../../server/controlAuthority.js' import { REVEAL_SYNC_PROTOCOL_VERSION, assessRevealSyncProtocolCompatibility, @@ -136,6 +142,7 @@ interface SyncDeckSessionData extends Record { presentationUrl: string | null standaloneMode: boolean instructorPasscode: string + controlAuthority: ReturnType instructorState: SyncDeckInstructorState | null lastInstructorPayload: unknown lastInstructorStatePayload: unknown @@ -152,6 +159,7 @@ interface SyncDeckSession extends SessionRecord { interface SyncDeckSocket extends ActiveBitsWebSocket { isInstructor?: boolean + instructorInstanceId?: string | null sessionId?: string | null studentId?: string | null } @@ -171,6 +179,7 @@ const WS_OPEN_READY_STATE = 1 const SYNCDECK_WS_UPDATE_TYPE = 'syncdeck-state-update' const SYNCDECK_WS_BROADCAST_TYPE = 'syncdeck-state' const SYNCDECK_WS_STUDENTS_TYPE = 'syncdeck-students' +const SYNCDECK_WS_CONTROL_AUTHORITY_TYPE = 'syncdeck-control-authority' const DEFAULT_INSTRUCTOR_AUTH_TIMEOUT_MS = 5_000 const MAX_CHALKBOARD_DELTA_STROKES = 200 const SYNCDECK_EMBEDDED_OWNER = 'syncdeck-instructor' @@ -571,6 +580,7 @@ function normalizeSessionData(data: unknown): SyncDeckSessionData { typeof source.instructorPasscode === 'string' && source.instructorPasscode.length > 0 ? source.instructorPasscode : createInstructorPasscode(), + controlAuthority: normalizeSessionControlAuthorityState(source.controlAuthority), instructorState: null, lastInstructorPayload: normalizedLastInstructorPayload, lastInstructorStatePayload: normalizedLastInstructorStatePayload, @@ -780,6 +790,21 @@ function sendSyncDeckState(socket: ActiveBitsWebSocket, payload: unknown): void } } +function sendSyncDeckControlAuthority(socket: ActiveBitsWebSocket, session: Pick): void { + if (socket.readyState !== WS_OPEN_READY_STATE) { + return + } + + try { + socket.send(JSON.stringify({ + type: SYNCDECK_WS_CONTROL_AUTHORITY_TYPE, + payload: getSessionControlAuthorityState(session), + })) + } catch { + // Ignore socket send failures. + } +} + function normalizeStudentId(value: unknown): string | null { if (typeof value !== 'string') { return null @@ -819,6 +844,14 @@ function normalizeInstructorPasscode(value: unknown): string | null { return trimmed } +function readInstructorInstanceId(body: unknown): string | null { + if (!isPlainObject(body)) { + return null + } + + return normalizeInstructorInstanceId(body.instructorInstanceId) +} + function asSyncDeckSession(session: SessionRecord | null): SyncDeckSession | null { if (!session || session.type !== 'syncdeck') { return null @@ -1618,6 +1651,45 @@ export default function setupSyncDeckRoutes(app: SyncDeckRouteApp, sessions: Ses response.status(403).json({ error: 'forbidden' }) }) + app.post('/api/syncdeck/:sessionId/control-authority/take', async (req, res) => { + const sessionId = req.params.sessionId + if (!sessionId) { + res.status(400).json({ error: 'missing sessionId' }) + return + } + + const session = await getSyncDeckSessionWithEmbeddedKeepalive(sessions, sessionId) + if (!session) { + res.status(404).json({ error: 'invalid session' }) + return + } + + const instructorPasscode = normalizeInstructorPasscode(readStringField(req.body, 'instructorPasscode')) + if (!instructorPasscode || !verifyInstructorPasscode(session.data.instructorPasscode, instructorPasscode)) { + res.status(403).json({ error: 'forbidden' }) + return + } + + const instructorInstanceId = readInstructorInstanceId(req.body) + if (!instructorInstanceId) { + res.status(400).json({ error: 'invalid instructorInstanceId' }) + return + } + + const controlAuthority = claimSessionControlAuthority({ + session, + instructorInstanceId, + }) + await sessions.set(session.id, session) + for (const peer of ws.wss.clients as Set) { + if (peer.sessionId !== session.id || peer.isInstructor !== true) { + continue + } + sendSyncDeckControlAuthority(peer, session) + } + res.json({ ok: true, controlAuthority }) + }) + app.post('/api/syncdeck/:sessionId/embedded-activity/start', async (req, res) => { const sessionId = req.params.sessionId if (!sessionId) { @@ -2107,6 +2179,9 @@ export default function setupSyncDeckRoutes(app: SyncDeckRouteApp, sessions: Ses const client = socket as SyncDeckSocket client.sessionId = query.get('sessionId') client.isInstructor = false + client.instructorInstanceId = query.get('role') === 'instructor' + ? normalizeInstructorInstanceId(query.get('instructorInstanceId')) + : null client.studentId = normalizeStudentId(query.get('studentId')) const sessionId = client.sessionId @@ -2137,8 +2212,17 @@ export default function setupSyncDeckRoutes(app: SyncDeckRouteApp, sessions: Ses socket.close(1008, 'forbidden') return } + const controlAuthority = getSessionControlAuthorityState(session) + if (controlAuthority.ownerInstanceId == null && client.instructorInstanceId) { + claimSessionControlAuthority({ + session, + instructorInstanceId: client.instructorInstanceId, + }) + await sessions.set(session.id, session) + } client.isInstructor = true await replayEmbeddedActivityStartsToSocket(client, session, null) + sendSyncDeckControlAuthority(socket, session) if (session.data.lastInstructorStatePayload != null) { if (!parseChalkboardCommand(session.data.lastInstructorStatePayload)) { sendSyncDeckState(socket, session.data.lastInstructorStatePayload) @@ -2235,6 +2319,14 @@ export default function setupSyncDeckRoutes(app: SyncDeckRouteApp, sessions: Ses return } + if ( + !client.instructorInstanceId || + getSessionControlAuthorityState(session).ownerInstanceId !== client.instructorInstanceId + ) { + sendSyncDeckControlAuthority(socket, session) + return + } + session.data.lastInstructorPayload = message.payload ?? null if (extractIndicesFromInstructorPayload(message.payload) != null) { session.data.lastInstructorStatePayload = message.payload diff --git a/activities/video-sync/activity.config.ts b/activities/video-sync/activity.config.ts index 8021494f..cb4806e4 100644 --- a/activities/video-sync/activity.config.ts +++ b/activities/video-sync/activity.config.ts @@ -32,6 +32,11 @@ const videoSyncConfig: ActivityConfig = { createSessionBootstrap: { historyState: ['instructorPasscode'], }, + controlAuthority: { + mode: 'single-instructor', + scope: 'inherited', + gating: 'all', + }, manageLayout: { expandShell: true, }, From 038124eb4a947ea0969a46ddeabc8d23a45e56b4 Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Wed, 29 Apr 2026 23:59:06 +0000 Subject: [PATCH 07/15] Gate syncdeck REST controls by ownership --- .agent/plans/control-authority-plan.md | 8 +- .../client/manager/SyncDeckManager.tsx | 34 +++- activities/syncdeck/server/routes.test.ts | 153 +++++++++++++++++- activities/syncdeck/server/routes.ts | 97 ++++++++++- 4 files changed, 275 insertions(+), 17 deletions(-) diff --git a/.agent/plans/control-authority-plan.md b/.agent/plans/control-authority-plan.md index b4ff1e69..accd3551 100644 --- a/.agent/plans/control-authority-plan.md +++ b/.agent/plans/control-authority-plan.md @@ -228,6 +228,8 @@ Shared enforcement rule: - [x] Enforce Video Sync non-owner command rejection for config and playback command routes. - [x] Disable Video Sync gated controls locally with explicit takeover feedback. - [x] Add first-pass SyncDeck manager authority plumbing: instructor instance identity, takeover route, websocket authority status, and local gating for the websocket relay controls. +- [x] Enforce SyncDeck non-owner rejection for configure and embedded activity start/end routes. +- [x] Disable SyncDeck configure, activity launch, and embedded end controls locally for non-owners. ### 7. Validation @@ -262,11 +264,13 @@ Implemented on this branch so far: - SyncDeck session normalization now preserves shared `controlAuthority` state instead of dropping it on activity-local normalization - SyncDeck manager shows first-pass control status UI and blocks non-owner websocket relay controls locally - SyncDeck websocket relay path rejects non-owner instructor updates and reflects current ownership back to the non-owner socket +- SyncDeck configure and embedded lifecycle REST routes require the owning instructor instance and auto-claim only when ownership is empty +- SyncDeck manager sends `instructorInstanceId` with configure, embedded start, and embedded end requests Still pending before this feature is complete: - shared/generic first-instructor auto-ownership support beyond the current Video Sync adoption -- broader SyncDeck gating coverage for non-websocket manager actions like configure and embedded activity lifecycle controls -- SyncDeck adoption and command classification +- decide whether lower-risk SyncDeck report/download routes should remain passcode-only or also require current control ownership +- SyncDeck adoption and command classification cleanup - websocket broadcast/update path for authority changes across instructor views - browser-level E2E coverage for the multi-instructor handoff scenario diff --git a/activities/syncdeck/client/manager/SyncDeckManager.tsx b/activities/syncdeck/client/manager/SyncDeckManager.tsx index 5c8c5ea2..e8272ad1 100644 --- a/activities/syncdeck/client/manager/SyncDeckManager.tsx +++ b/activities/syncdeck/client/manager/SyncDeckManager.tsx @@ -2585,7 +2585,7 @@ const SyncDeckManager: FC = () => { }, [hostProtocol, sessionId, userAgent]) useEffect(() => { - if (!sessionId || !instructorPasscode) { + if (!sessionId || !instructorPasscode || !instructorInstanceId) { return } @@ -2615,6 +2615,7 @@ const SyncDeckManager: FC = () => { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ instructorPasscode, + instructorInstanceId, activityId: request.activityId, instanceKey: request.instanceKey, ...(request.location ? { location: request.location } : {}), @@ -2695,7 +2696,7 @@ const SyncDeckManager: FC = () => { pendingEmbeddedBootstrapChildSessionIdsRef.current.delete(request.childSessionId) } } - }, [clearEmbeddedBootstrapBackfillRetryTimeout, embeddedActivities, embeddedBootstrapBackfillRetryNonce, instructorPasscode, sessionId]) + }, [clearEmbeddedBootstrapBackfillRetryTimeout, embeddedActivities, embeddedBootstrapBackfillRetryNonce, instructorInstanceId, instructorPasscode, sessionId]) const copyValue = async (value: string): Promise => { if (!value || typeof navigator === 'undefined' || navigator.clipboard === undefined) { @@ -2949,7 +2950,7 @@ const SyncDeckManager: FC = () => { return false } - if (!instructorPasscode) { + if (!instructorPasscode || !instructorInstanceId) { if (!background) { setStartError('Instructor passcode missing. Refresh SyncDeck manager and try again.') setStartSuccess(null) @@ -2957,6 +2958,14 @@ const SyncDeckManager: FC = () => { return false } + if (!canUseInstructorControls) { + if (!background) { + setStartError('Take control to launch embedded activities.') + setStartSuccess(null) + } + return false + } + return await runEmbeddedStartWithPendingRetry({ instanceKey: request.instanceKey, background, @@ -2972,6 +2981,7 @@ const SyncDeckManager: FC = () => { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ instructorPasscode, + instructorInstanceId, activityId: request.activityId, instanceKey: request.instanceKey, ...(request.location ? { location: request.location } : {}), @@ -3071,7 +3081,7 @@ const SyncDeckManager: FC = () => { }, }) }, - [sessionId, instructorPasscode, embeddedActivities, instructorIndicesState], + [canUseInstructorControls, sessionId, instructorInstanceId, instructorPasscode, embeddedActivities, instructorIndicesState], ) const loadDeckActivityRequests = useCallback(async (): Promise => { @@ -3355,6 +3365,7 @@ const SyncDeckManager: FC = () => { presentationUrl: normalizedUrl, entryPolicy, instructorPasscode, + instructorInstanceId, ...(urlHash ? { urlHash } : {}), }), }) @@ -3969,7 +3980,13 @@ const SyncDeckManager: FC = () => { } const endEmbeddedActivity = async (instanceKey: string): Promise => { - if (!sessionId || !instructorPasscode) { + if (!sessionId || !instructorPasscode || !instructorInstanceId) { + return + } + + if (!canUseInstructorControls) { + setStartError('Take control to end embedded activities.') + setStartSuccess(null) return } @@ -3980,6 +3997,7 @@ const SyncDeckManager: FC = () => { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ instructorPasscode, + instructorInstanceId, instanceKey, }), }) @@ -4198,7 +4216,7 @@ const SyncDeckManager: FC = () => { className="px-2 py-1 rounded border border-gray-300 text-sm text-gray-700 hover:bg-gray-50 disabled:opacity-50 disabled:cursor-not-allowed" aria-expanded={isActivityPickerOpen} aria-controls="syncdeck-activity-picker-panel" - disabled={isConfigurePanelOpen} + disabled={isConfigurePanelOpen || !canUseInstructorControls} > Activities @@ -4311,7 +4329,7 @@ const SyncDeckManager: FC = () => { onClick={() => { handleEmbeddedEndControlClick(instanceKey) }} - disabled={endingEmbeddedInstanceKey === instanceKey} + disabled={endingEmbeddedInstanceKey === instanceKey || !canUseInstructorControls} className="px-2 py-1 rounded border border-red-600 text-xs font-semibold text-red-600 hover:bg-red-50 disabled:opacity-60" > {endingEmbeddedInstanceKey === instanceKey @@ -4395,7 +4413,7 @@ const SyncDeckManager: FC = () => { -
- {controlStatusLabel} - -
+ { + void takeControl() + }} + buttonClassName="rounded border border-gray-300 bg-white px-2 py-1 text-xs font-semibold text-gray-700 hover:bg-gray-50 disabled:cursor-not-allowed disabled:opacity-50" + /> Join Code: void saveConfig()}> {isPasscodeReady ? 'Start instructor view' : 'Loading instructor access...'} -
- {controlStatusLabel} - {!hasControl ? ( - - ) : null} -
+ void takeControl()} + /> )}
diff --git a/client/src/components/common/ControlAuthorityStatus.test.tsx b/client/src/components/common/ControlAuthorityStatus.test.tsx new file mode 100644 index 00000000..4f2e04f8 --- /dev/null +++ b/client/src/components/common/ControlAuthorityStatus.test.tsx @@ -0,0 +1,36 @@ +import assert from 'node:assert/strict' +import test from 'node:test' +import { renderToStaticMarkup } from 'react-dom/server' +import ControlAuthorityStatus from './ControlAuthorityStatus' + +void test('ControlAuthorityStatus renders current control state and disables owner button', () => { + const html = renderToStaticMarkup( + {}} + />, + ) + + assert.match(html, /aria-live="polite"/) + assert.match(html, /You have control/) + assert.match(html, /Instructor control is active in this view/) + assert.match(html, /disabled/) + assert.match(html, /In Control/) +}) + +void test('ControlAuthorityStatus can hide the action when the current instructor owns control', () => { + const html = renderToStaticMarkup( + {}} + />, + ) + + assert.match(html, /You have control/) + assert.doesNotMatch(html, /In Control/) +}) diff --git a/client/src/components/common/ControlAuthorityStatus.tsx b/client/src/components/common/ControlAuthorityStatus.tsx new file mode 100644 index 00000000..1b96b53f --- /dev/null +++ b/client/src/components/common/ControlAuthorityStatus.tsx @@ -0,0 +1,46 @@ +import * as React from 'react' + +void React + +export interface ControlAuthorityStatusProps { + statusLabel: string + hasControl: boolean + canTakeControl: boolean + onTakeControl: () => void + hideButtonWhenOwner?: boolean + className?: string + buttonClassName?: string + takeControlLabel?: string + inControlLabel?: string +} + +export default function ControlAuthorityStatus({ + statusLabel, + hasControl, + canTakeControl, + onTakeControl, + hideButtonWhenOwner = false, + className = 'flex items-center gap-2 rounded border border-gray-200 bg-gray-50 px-2 py-1', + buttonClassName = 'rounded border border-gray-300 bg-white px-3 py-1 text-sm font-semibold text-gray-700 hover:bg-gray-50 disabled:cursor-not-allowed disabled:opacity-50', + takeControlLabel = 'Take Control', + inControlLabel = 'In Control', +}: ControlAuthorityStatusProps) { + const shouldShowButton = !hideButtonWhenOwner || !hasControl + + return ( +
+ {statusLabel} + {shouldShowButton ? ( + + ) : null} +
+ ) +} From 87269827ca94240691b10c55799702933025756d Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Thu, 30 Apr 2026 00:20:29 +0000 Subject: [PATCH 11/15] Update control authority plan status --- .agent/plans/control-authority-plan.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.agent/plans/control-authority-plan.md b/.agent/plans/control-authority-plan.md index f6ba0885..499af1aa 100644 --- a/.agent/plans/control-authority-plan.md +++ b/.agent/plans/control-authority-plan.md @@ -220,10 +220,10 @@ Shared enforcement rule: ### 6. Activity adoption - [x] Add `controlAuthority` config to SyncDeck. -- [ ] Decide SyncDeck gating mode. Current expectation: `gating: 'activity'`. +- [x] Decide SyncDeck gating mode. Current decision: `gating: 'all'` because manager runtime controls are the command surface. - [x] Add `controlAuthority` config to Video Sync. - [x] Decide Video Sync gating mode. Current expectation: `gating: 'activity'` or `all`, depending on final command surface. -- [ ] Implement activity-owned command classifiers where `gating: 'activity'` is used. +- [x] Implement activity-owned command classifiers where `gating: 'activity'` is used or explicitly document that the adopted activity uses `gating: 'all'`. - [x] Ensure embedded Video Sync defaults to inherited authority when launched under SyncDeck. - [x] Ensure embedded Video Sync can be locally overridden by explicit `Take Control`. - [x] Enforce Video Sync non-owner command rejection for config and playback command routes. @@ -242,7 +242,7 @@ Shared enforcement rule: - [x] Add server tests for non-owner gated-command rejection. - [ ] Add server tests covering older sessions with empty authority state that auto-initialize on first manager connect. - [x] Add client tests for disabled controls and authority status messaging. -- [ ] Add activity-specific tests for SyncDeck command classification. +- [x] Add activity-specific tests for SyncDeck command classification or confirm `gating: 'all'` avoids an activity classifier. - [x] Add activity-specific tests for Video Sync command classification helpers. - [x] Add browser-level E2E coverage for two instructor views on the same authority-enabled session: first instructor default owner, second instructor disabled, live takeover, and disabled-state flip after handoff. - [ ] Add browser-level E2E coverage for embedded inherited authority plus local child override behavior. @@ -275,8 +275,7 @@ Implemented on this branch so far: Still pending before this feature is complete: - shared/generic first-instructor auto-ownership support beyond the current Video Sync adoption - decide whether lower-risk SyncDeck report/download routes should remain passcode-only or also require current control ownership -- SyncDeck adoption and command classification cleanup -- websocket broadcast/update path for authority changes across instructor views +- shared/generic command filtering beyond the current Video Sync and SyncDeck activity-local enforcement - browser-level E2E coverage for embedded inherited authority plus local child override behavior ## Suggested Rollout Order From 2f4938e59bca1c2cd7f30d0984077d7e05e00778 Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Thu, 30 Apr 2026 00:23:11 +0000 Subject: [PATCH 12/15] Fix syncdeck route test lint --- activities/syncdeck/server/routes.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activities/syncdeck/server/routes.test.ts b/activities/syncdeck/server/routes.test.ts index 9fd2e15a..b76e6824 100644 --- a/activities/syncdeck/server/routes.test.ts +++ b/activities/syncdeck/server/routes.test.ts @@ -3178,7 +3178,7 @@ void test('embedded-activity end route rejects non-owner instructor instances', assert.equal(res.statusCode, 403) assert.deepEqual(res.body, { error: 'control authority required' }) const storedParentSession = storeState.store.s1 as SessionRecord - assert.ok(asRecord(storedParentSession.data)?.embeddedActivities) + assert.notEqual(asRecord(storedParentSession.data)?.embeddedActivities, undefined) assert.ok(storeState.store['CHILD:s1:abc12:video-sync']) }) From 9ab102ae025eb9d591a4859ec4f0418ecadb3ee8 Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Thu, 30 Apr 2026 00:28:36 +0000 Subject: [PATCH 13/15] Fix syncdeck solo launch identity tests --- activities/syncdeck/client/index.test.ts | 13 +++++-------- activities/syncdeck/client/shared/sessionLaunch.ts | 3 ++- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/activities/syncdeck/client/index.test.ts b/activities/syncdeck/client/index.test.ts index b98c9dd7..7220482f 100644 --- a/activities/syncdeck/client/index.test.ts +++ b/activities/syncdeck/client/index.test.ts @@ -63,14 +63,11 @@ void test('launchSyncDeckPersistentSoloEntry creates and configures a solo sessi assert.equal(requests.length, 2) assert.equal(requests[0]?.input, '/api/syncdeck/create') assert.equal(requests[1]?.input, '/api/syncdeck/syncdeck-solo-1/configure') - assert.deepEqual( - JSON.parse(String(requests[1]?.init?.body ?? '{}')), - { - presentationUrl: 'https://slides.example/deck', - instructorPasscode: 'pass-123', - standaloneMode: true, - }, - ) + const configureBody = JSON.parse(String(requests[1]?.init?.body ?? '{}')) as Record + assert.equal(configureBody.presentationUrl, 'https://slides.example/deck') + assert.equal(configureBody.instructorPasscode, 'pass-123') + assert.equal(configureBody.standaloneMode, true) + assert.equal(typeof configureBody.instructorInstanceId, 'string') } finally { globalThis.fetch = originalFetch } diff --git a/activities/syncdeck/client/shared/sessionLaunch.ts b/activities/syncdeck/client/shared/sessionLaunch.ts index a82a88c1..b8d80f21 100644 --- a/activities/syncdeck/client/shared/sessionLaunch.ts +++ b/activities/syncdeck/client/shared/sessionLaunch.ts @@ -1,4 +1,5 @@ import { + buildInstructorControlInstanceId, createDefaultInstructorControlId, resolveOrCreateInstructorControlInstanceId, } from '@src/components/common/instructorControlIdentity' @@ -51,7 +52,7 @@ function resolveLaunchInstructorInstanceId(value: string | null | undefined): st } if (typeof window === 'undefined') { - return null + return buildInstructorControlInstanceId(createDefaultInstructorControlId(), createDefaultInstructorControlId()) } return resolveOrCreateInstructorControlInstanceId( From e0681982c77f921175656dd25ff56bca43b24213 Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Fri, 8 May 2026 01:40:51 +0000 Subject: [PATCH 14/15] Avoid storing SyncDeck passcode in e2e storage --- .agent/knowledge/security-notes.md | 9 +++++++++ playwright/control-authority.spec.ts | 17 ++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/.agent/knowledge/security-notes.md b/.agent/knowledge/security-notes.md index 8a4c500c..178bfd7e 100644 --- a/.agent/knowledge/security-notes.md +++ b/.agent/knowledge/security-notes.md @@ -15,6 +15,15 @@ Track security-relevant boundaries, risks, and mitigation decisions. ## Notes +- Date: 2026-05-08 +- Area: Playwright SyncDeck instructor bootstrap +- Threat or risk: Seeding E2E manager pages by writing a real `instructorPasscode` into browser `sessionStorage` creates the same clear-text credential persistence pattern the product avoids where possible, and triggers CodeQL clear-text storage findings. +- Control or mitigation: The control-authority Playwright test now exposes the passcode through a test-only `sessionStorage.getItem` shim for the exact SyncDeck passcode key instead of calling `sessionStorage.setItem` with the secret. +- Residual risk: The passcode still exists in test process memory and is returned to app code as part of the manager bootstrap path; the mitigation only removes browser storage persistence from the harness. +- Validation (test/review/path): `playwright/control-authority.spec.ts` +- Follow-up action: Prefer route/cookie/bootstrap shims or same-tab memory helpers over Web Storage writes when E2E tests need manager credentials. +- Owner: Codex + - Date: 2026-03-22 - Area: Playwright production-mode test secret handling - Threat or risk: Committing a fixed `PERSISTENT_SESSION_SECRET` in the Playwright harness creates secret-scanner noise and normalizes checking pseudo-secrets into the repo, even when the value is test-only. diff --git a/playwright/control-authority.spec.ts b/playwright/control-authority.spec.ts index b0e540cb..a771b005 100644 --- a/playwright/control-authority.spec.ts +++ b/playwright/control-authority.spec.ts @@ -1,7 +1,5 @@ import { expect, test, type APIRequestContext, type Browser, type Page } from '@playwright/test' -const SYNCDECK_PASSCODE_KEY_PREFIX = 'syncdeck_instructor_' - async function createConfiguredSyncDeckSession(request: APIRequestContext): Promise<{ sessionId: string instructorPasscode: string @@ -41,15 +39,24 @@ async function openSyncDeckInstructorPage(params: { const context = await params.browser.newContext() const page = await context.newPage() await page.addInitScript( - ({ sessionId, instructorPasscode, passcodeKeyPrefix, browserId, tabId }) => { + ({ sessionId, instructorPasscode, browserId, tabId }) => { + const passcodeStorageKey = `syncdeck_instructor_${sessionId}` + const originalGetItem = window.sessionStorage.getItem.bind(window.sessionStorage) + + window.sessionStorage.getItem = (key: string): string | null => { + if (key === passcodeStorageKey) { + return instructorPasscode + } + + return originalGetItem(key) + } + window.localStorage.setItem('activebits:instructor-control:browser-id', browserId) window.sessionStorage.setItem('activebits:instructor-control:tab-id', tabId) - window.sessionStorage.setItem(`${passcodeKeyPrefix}${sessionId}`, instructorPasscode) }, { sessionId: params.sessionId, instructorPasscode: params.instructorPasscode, - passcodeKeyPrefix: SYNCDECK_PASSCODE_KEY_PREFIX, browserId: params.browserId, tabId: params.tabId, }, From 705cc8eb086f6208b33a401d4bc585c94abec394 Mon Sep 17 00:00:00 2001 From: Brian Dahlem Date: Fri, 8 May 2026 02:08:38 +0000 Subject: [PATCH 15/15] Avoid persisting SyncDeck passcodes in storage --- .agent/knowledge/security-notes.md | 4 +-- activities/syncdeck/activity.config.ts | 8 ++--- .../client/manager/SyncDeckManager.tsx | 34 +++++++++++++------ .../components/common/ActivityLauncher.tsx | 3 +- .../src/components/common/ManageDashboard.tsx | 3 +- .../src/components/common/SessionRouter.tsx | 5 ++- .../common/manageDashboardUtils.test.ts | 20 +++++++++++ .../components/common/manageDashboardUtils.ts | 8 +++++ playwright/control-authority.spec.ts | 25 ++++++-------- server/activityConfigSchema.test.ts | 2 ++ types/activity.ts | 1 + types/activityConfigSchema.ts | 2 ++ 12 files changed, 79 insertions(+), 36 deletions(-) diff --git a/.agent/knowledge/security-notes.md b/.agent/knowledge/security-notes.md index 178bfd7e..06e6c65d 100644 --- a/.agent/knowledge/security-notes.md +++ b/.agent/knowledge/security-notes.md @@ -18,8 +18,8 @@ Track security-relevant boundaries, risks, and mitigation decisions. - Date: 2026-05-08 - Area: Playwright SyncDeck instructor bootstrap - Threat or risk: Seeding E2E manager pages by writing a real `instructorPasscode` into browser `sessionStorage` creates the same clear-text credential persistence pattern the product avoids where possible, and triggers CodeQL clear-text storage findings. -- Control or mitigation: The control-authority Playwright test now exposes the passcode through a test-only `sessionStorage.getItem` shim for the exact SyncDeck passcode key instead of calling `sessionStorage.setItem` with the secret. -- Residual risk: The passcode still exists in test process memory and is returned to app code as part of the manager bootstrap path; the mitigation only removes browser storage persistence from the harness. +- Control or mitigation: SyncDeck manager bootstrap no longer persists instructor passcodes in browser storage, and the control-authority Playwright test now fulfills the manager passcode API from the test runner instead of injecting the secret into browser storage. +- Residual risk: The passcode still exists in test process memory and is returned to app code through the manager bootstrap API path; the mitigation removes browser storage persistence from the product and harness. - Validation (test/review/path): `playwright/control-authority.spec.ts` - Follow-up action: Prefer route/cookie/bootstrap shims or same-tab memory helpers over Web Storage writes when E2E tests need manager credentials. - Owner: Codex diff --git a/activities/syncdeck/activity.config.ts b/activities/syncdeck/activity.config.ts index eb8c5057..afd6c134 100644 --- a/activities/syncdeck/activity.config.ts +++ b/activities/syncdeck/activity.config.ts @@ -49,13 +49,9 @@ const syncdeckConfig: ActivityConfig = { }, ], createSessionBootstrap: { + historyState: ['instructorPasscode'], + transientOnly: true, selectedOptionsToSessionData: ['presentationUrl'], - sessionStorage: [ - { - keyPrefix: 'syncdeck_instructor_', - responseField: 'instructorPasscode', - }, - ], }, controlAuthority: { mode: 'single-instructor', diff --git a/activities/syncdeck/client/manager/SyncDeckManager.tsx b/activities/syncdeck/client/manager/SyncDeckManager.tsx index b27394c4..7f7c96eb 100644 --- a/activities/syncdeck/client/manager/SyncDeckManager.tsx +++ b/activities/syncdeck/client/manager/SyncDeckManager.tsx @@ -7,7 +7,6 @@ import ControlAuthorityStatus from '@src/components/common/ControlAuthorityStatu import { storeCreateSessionBootstrapPayload } from '@src/components/common/manageDashboardUtils' import { resolvePersistentSessionEntryPolicy, type PersistentSessionEntryPolicy } from '../../../../types/waitingRoom.js' import { runSyncDeckPresentationPreflight } from '../shared/presentationPreflight.js' -import { buildSyncDeckPasscodeKey } from '../shared/authStorage.js' import { getStudentPresentationCompatibilityError, } from '../shared/presentationUrlCompatibility.js' @@ -577,6 +576,20 @@ export function normalizeStoredInstructorPasscode(value: string | null): string return trimmed.length > 0 ? trimmed : null } +function readBootstrapInstructorPasscode(state: unknown): string | null { + if (state == null || typeof state !== 'object' || Array.isArray(state)) { + return null + } + + const payload = (state as { createSessionPayload?: unknown }).createSessionPayload + if (payload == null || typeof payload !== 'object' || Array.isArray(payload)) { + return null + } + + const value = (payload as { instructorPasscode?: unknown }).instructorPasscode + return typeof value === 'string' && value.trim().length > 0 ? value.trim() : null +} + export function validatePresentationUrl(value: string, hostProtocol?: string | null, userAgent?: string | null): boolean { return value.trim().length > 0 && getStudentPresentationCompatibilityError({ value, @@ -1881,6 +1894,10 @@ const SyncDeckManager: FC = () => { const [, setStartSuccess] = useState(null) const [isConfigurePanelOpen, setIsConfigurePanelOpen] = useState(true) const [instructorPasscode, setInstructorPasscode] = useState(null) + const bootstrapInstructorPasscode = useMemo( + () => readBootstrapInstructorPasscode(location.state), + [location.state], + ) const [persistentUrlHashFallback, setPersistentUrlHashFallback] = useState(null) const [persistentEntryPolicyFallback, setPersistentEntryPolicyFallback] = useState(null) const [isPasscodeReady, setIsPasscodeReady] = useState(false) @@ -2502,12 +2519,8 @@ const SyncDeckManager: FC = () => { let isCancelled = false const loadInstructorPasscode = async (): Promise => { - const cachedPasscode = normalizeStoredInstructorPasscode( - window.sessionStorage.getItem(buildSyncDeckPasscodeKey(sessionId)), - ) - if (!isCancelled) { - setInstructorPasscode(cachedPasscode) + setInstructorPasscode(bootstrapInstructorPasscode) setPersistentUrlHashFallback(null) setPersistentEntryPolicyFallback(null) } @@ -2518,7 +2531,7 @@ const SyncDeckManager: FC = () => { }) if (!response.ok) { if (!isCancelled) { - if (!cachedPasscode) { + if (!bootstrapInstructorPasscode) { setInstructorPasscode(null) } setPersistentUrlHashFallback(null) @@ -2529,11 +2542,10 @@ const SyncDeckManager: FC = () => { const payload = (await response.json()) as InstructorPasscodeResponsePayload if (typeof payload.instructorPasscode === 'string' && payload.instructorPasscode.length > 0) { - window.sessionStorage.setItem(buildSyncDeckPasscodeKey(sessionId), payload.instructorPasscode) if (!isCancelled) { setInstructorPasscode(payload.instructorPasscode) } - } else if (!isCancelled && !cachedPasscode) { + } else if (!isCancelled && !bootstrapInstructorPasscode) { setInstructorPasscode(null) } @@ -2559,7 +2571,7 @@ const SyncDeckManager: FC = () => { } } catch { if (!isCancelled) { - if (!cachedPasscode) { + if (!bootstrapInstructorPasscode) { setInstructorPasscode(null) } setPersistentUrlHashFallback(null) @@ -2578,7 +2590,7 @@ const SyncDeckManager: FC = () => { return () => { isCancelled = true } - }, [hostProtocol, sessionId, userAgent]) + }, [bootstrapInstructorPasscode, hostProtocol, sessionId, userAgent]) useEffect(() => { if (!sessionId || !instructorPasscode || !instructorInstanceId) { diff --git a/client/src/components/common/ActivityLauncher.tsx b/client/src/components/common/ActivityLauncher.tsx index 53611864..fca21bdb 100644 --- a/client/src/components/common/ActivityLauncher.tsx +++ b/client/src/components/common/ActivityLauncher.tsx @@ -12,6 +12,7 @@ import { } from './activityLauncherUtils' import { persistCreateSessionBootstrapToSessionStorage, + shouldPersistCreateSessionBootstrapPayload, storeCreateSessionBootstrapPayload, } from './manageDashboardUtils' @@ -64,7 +65,7 @@ function ActivityLauncherBody({ const navigationState = buildStandaloneActivityLauncherState(activity, payload) persistCreateSessionBootstrapToSessionStorage(activity.createSessionBootstrap, payload.id, payload) - if (navigationState != null) { + if (navigationState != null && shouldPersistCreateSessionBootstrapPayload(activity.createSessionBootstrap)) { storeCreateSessionBootstrapPayload(activity.id, payload.id, navigationState.createSessionPayload) } diff --git a/client/src/components/common/ManageDashboard.tsx b/client/src/components/common/ManageDashboard.tsx index e6b45b27..21c46112 100644 --- a/client/src/components/common/ManageDashboard.tsx +++ b/client/src/components/common/ManageDashboard.tsx @@ -26,6 +26,7 @@ import { parseDeepLinkGenerator, parseDeepLinkOptions, resolvePersistentLinkPreflightValue, + shouldPersistCreateSessionBootstrapPayload, validateDeepLinkSelection, type DeepLinkSelection, } from './manageDashboardUtils' @@ -271,7 +272,7 @@ export default function ManageDashboard({ const navigationState = activity ? buildStandaloneActivityLauncherState(activity, payload) : null persistCreateSessionBootstrapToSessionStorage(activity?.createSessionBootstrap, payload.id, payload) - if (navigationState != null) { + if (navigationState != null && shouldPersistCreateSessionBootstrapPayload(activity?.createSessionBootstrap)) { storeCreateSessionBootstrapPayload(activityId, payload.id, navigationState.createSessionPayload) } diff --git a/client/src/components/common/SessionRouter.tsx b/client/src/components/common/SessionRouter.tsx index 15395ade..dec216d1 100644 --- a/client/src/components/common/SessionRouter.tsx +++ b/client/src/components/common/SessionRouter.tsx @@ -7,6 +7,7 @@ import WaitingRoom from './WaitingRoom' import LoadingFallback from './LoadingFallback' import { persistCreateSessionBootstrapToSessionStorage, + shouldPersistCreateSessionBootstrapPayload, storeCreateSessionBootstrapPayload, } from './manageDashboardUtils' import { @@ -467,7 +468,9 @@ const SessionRouter = () => { if (createSessionPayload) { persistCreateSessionBootstrapToSessionStorage(activity?.createSessionBootstrap, nextSessionId, createSessionPayload) - storeCreateSessionBootstrapPayload(payload.activityName, nextSessionId, createSessionPayload) + if (shouldPersistCreateSessionBootstrapPayload(activity?.createSessionBootstrap)) { + storeCreateSessionBootstrapPayload(payload.activityName, nextSessionId, createSessionPayload) + } } const path = await resolveTeacherManagePath(payload.activityName, nextSessionId, '') diff --git a/client/src/components/common/manageDashboardUtils.test.ts b/client/src/components/common/manageDashboardUtils.test.ts index a4c91b6c..9a049a30 100644 --- a/client/src/components/common/manageDashboardUtils.test.ts +++ b/client/src/components/common/manageDashboardUtils.test.ts @@ -18,6 +18,7 @@ import { parseDeepLinkGenerator, persistCreateSessionBootstrapToSessionStorage, parseDeepLinkOptions, + shouldPersistCreateSessionBootstrapPayload, storeCreateSessionBootstrapPayload, validateDeepLinkSelection, } from './manageDashboardUtils' @@ -319,16 +320,35 @@ void test('parseCreateSessionBootstrap validates sessionStorage bootstrap metada { keyPrefix: 'x_', responseField: '' }, ], historyState: [' instructorPasscode ', '', 42], + transientOnly: true, }), { sessionStorage: [ { keyPrefix: 'syncdeck_instructor_', responseField: 'instructorPasscode' }, ], historyState: ['instructorPasscode'], + transientOnly: true, }, ) }) +void test('shouldPersistCreateSessionBootstrapPayload respects transient-only bootstrap payloads', () => { + assert.equal(shouldPersistCreateSessionBootstrapPayload(null), true) + assert.equal( + shouldPersistCreateSessionBootstrapPayload({ + historyState: ['instructorPasscode'], + transientOnly: true, + }), + false, + ) + assert.equal( + shouldPersistCreateSessionBootstrapPayload({ + historyState: ['instructorPasscode'], + }), + true, + ) +}) + void test('persistCreateSessionBootstrapToSessionStorage stores declared create response fields', () => { const originalWindow = globalThis.window const { backing: writes, storage: fakeSessionStorage } = createFakeSessionStorage() diff --git a/client/src/components/common/manageDashboardUtils.ts b/client/src/components/common/manageDashboardUtils.ts index 52c35527..372445c2 100644 --- a/client/src/components/common/manageDashboardUtils.ts +++ b/client/src/components/common/manageDashboardUtils.ts @@ -38,6 +38,7 @@ export interface CreateSessionBootstrapSessionStorageEntry { export interface CreateSessionBootstrapConfig { sessionStorage: CreateSessionBootstrapSessionStorageEntry[] historyState?: string[] + transientOnly?: boolean } export type DeepLinkOptions = Record @@ -344,6 +345,7 @@ export function parseCreateSessionBootstrap(rawCreateSessionBootstrap: unknown): .map((entry) => entry.trim()) .filter((entry) => entry.length > 0) : [] + const transientOnly = rawCreateSessionBootstrap.transientOnly === true if (sessionStorage.length === 0 && historyState.length === 0) { return null @@ -352,9 +354,15 @@ export function parseCreateSessionBootstrap(rawCreateSessionBootstrap: unknown): return { sessionStorage, ...(historyState.length > 0 ? { historyState } : {}), + ...(transientOnly ? { transientOnly } : {}), } } +export function shouldPersistCreateSessionBootstrapPayload(rawCreateSessionBootstrap: unknown): boolean { + const createSessionBootstrap = parseCreateSessionBootstrap(rawCreateSessionBootstrap) + return createSessionBootstrap?.transientOnly !== true +} + export function persistCreateSessionBootstrapToSessionStorage( rawCreateSessionBootstrap: unknown, sessionId: string, diff --git a/playwright/control-authority.spec.ts b/playwright/control-authority.spec.ts index a771b005..9d4b9b75 100644 --- a/playwright/control-authority.spec.ts +++ b/playwright/control-authority.spec.ts @@ -38,25 +38,22 @@ async function openSyncDeckInstructorPage(params: { }): Promise { const context = await params.browser.newContext() const page = await context.newPage() + await page.route( + `**/api/syncdeck/${encodeURIComponent(params.sessionId)}/instructor-passcode`, + async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ instructorPasscode: params.instructorPasscode }), + }) + }, + ) await page.addInitScript( - ({ sessionId, instructorPasscode, browserId, tabId }) => { - const passcodeStorageKey = `syncdeck_instructor_${sessionId}` - const originalGetItem = window.sessionStorage.getItem.bind(window.sessionStorage) - - window.sessionStorage.getItem = (key: string): string | null => { - if (key === passcodeStorageKey) { - return instructorPasscode - } - - return originalGetItem(key) - } - + ({ browserId, tabId }) => { window.localStorage.setItem('activebits:instructor-control:browser-id', browserId) window.sessionStorage.setItem('activebits:instructor-control:tab-id', tabId) }, { - sessionId: params.sessionId, - instructorPasscode: params.instructorPasscode, browserId: params.browserId, tabId: params.tabId, }, diff --git a/server/activityConfigSchema.test.ts b/server/activityConfigSchema.test.ts index 9f13660a..1ad0987f 100644 --- a/server/activityConfigSchema.test.ts +++ b/server/activityConfigSchema.test.ts @@ -61,6 +61,7 @@ void test('parseActivityConfig accepts valid shared contracts', () => { }, createSessionBootstrap: { historyState: ['instructorPasscode'], + transientOnly: true, selectedOptionsToSessionData: ['presentationUrl'], sessionStorage: [ { @@ -121,6 +122,7 @@ void test('parseActivityConfig accepts valid shared contracts', () => { responseField: 'instructorPasscode', }) assert.deepEqual(parsed.createSessionBootstrap?.historyState, ['instructorPasscode']) + assert.equal(parsed.createSessionBootstrap?.transientOnly, true) assert.deepEqual(parsed.createSessionBootstrap?.selectedOptionsToSessionData, ['presentationUrl']) assert.equal(parsed.manageDashboard?.customPersistentLinkBuilder, true) assert.equal(parsed.embeddedRuntime?.instructorGated, 'runtime') diff --git a/types/activity.ts b/types/activity.ts index a573e993..c8276016 100644 --- a/types/activity.ts +++ b/types/activity.ts @@ -67,6 +67,7 @@ export interface ActivityCreateSessionBootstrapSessionStorageEntry { export interface ActivityCreateSessionBootstrapConfig { sessionStorage?: ActivityCreateSessionBootstrapSessionStorageEntry[] historyState?: string[] + transientOnly?: boolean selectedOptionsToSessionData?: string[] } diff --git a/types/activityConfigSchema.ts b/types/activityConfigSchema.ts index 112d2c76..7643205b 100644 --- a/types/activityConfigSchema.ts +++ b/types/activityConfigSchema.ts @@ -256,6 +256,7 @@ function parseCreateSessionBootstrap(raw: unknown, context: string): ActivityCre const sessionStorage = parseCreateSessionBootstrapSessionStorage(raw.sessionStorage, `${context}.createSessionBootstrap`) const historyState = parseCreateSessionBootstrapHistoryState(raw.historyState, `${context}.createSessionBootstrap`) + const transientOnly = raw.transientOnly === true const selectedOptionsToSessionData = parseCreateSessionBootstrapSelectedOptionsToSessionData( raw.selectedOptionsToSessionData, `${context}.createSessionBootstrap`, @@ -263,6 +264,7 @@ function parseCreateSessionBootstrap(raw: unknown, context: string): ActivityCre return { ...(sessionStorage !== undefined ? { sessionStorage } : {}), ...(historyState !== undefined ? { historyState } : {}), + ...(transientOnly ? { transientOnly } : {}), ...(selectedOptionsToSessionData !== undefined ? { selectedOptionsToSessionData } : {}), } }