Skip to content

fix: resolve collection-code vs join_code mismatches (dashboard 404s, search 403, bridge-app self-stop)#492

Merged
thewrz merged 3 commits into
mainfrom
fix/dashboard-join-code-mismatch
Jun 19, 2026
Merged

fix: resolve collection-code vs join_code mismatches (dashboard 404s, search 403, bridge-app self-stop)#492
thewrz merged 3 commits into
mainfrom
fix/dashboard-join-code-mismatch

Conversation

@thewrz

@thewrz thewrz commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Why

After Event.code (collection code, DJ surfaces) and Event.join_code (live/QR public surfaces) became distinct values, several callers written when the two were identical kept passing the collection code into endpoints that resolve strictly by join_code. On production event 4GX43T this surfaced as:

  • The DJ dashboard spamming the console with repeating 404s on /api/public/e/{code}/nowplaying|bridge-status|history.
  • The "Search for Song" button returning 403 (the authenticated DJ has a JWT but no guest wrzdj_human cookie, and human-verification is enforced in prod).
  • (found while auditing the bridge) the bridge-app silently stopping a live bridge — its health check hit the join-code-only /nowplaying with the collection code → 404 → "Event was deleted".

The four join-code-only endpoints are a deliberate, test-locked contract (test_collection_code_is_rejected, post-#324/#328). So every fix corrects the caller — none loosen those endpoints.

What

  • Dashboard ((dj)/events/[code]/page.tsx): poll the three live-display endpoints by event.join_code, fetched in a second batch so the request queue isn't delayed behind getEvent.
  • Search owner-bypass (deps.py, events.py, lib/api.ts): new non-raising get_current_user_optional; the authenticated event owner skips the human-verification gate while anonymous guests still go through it. The dashboard now sends its JWT on eventSearch.
  • bridge-app (event-health-service.ts): health check now uses the dual-resolver /api/public/events/{code} (accepts either code; same 200/404/410 semantics). Bridge core + CLI were already correct.

A repo-wide audit (40+ code-bearing endpoints, all client surfaces incl. kiosk) found no other mismatches.

Testing

  • Backend pytest + 85% coverage gate (3061 passed, 88.57%); ruff/format/bandit clean
  • Dashboard lint + tsc + vitest (1341 passed); bridge 343; bridge-app 73
  • New regression tests: owner-bypass (anon 403 / owner 200 under enforcement), dashboard join_code polls, bridge-app dual-resolver URL
  • Manual verification on a local deploy: live owner-bypass proof (403 vs 200), dashboard polls hit join_code, endpoint contracts intact
  • CI green

🤖 Co-authored by Claude Opus 4.8 (1M context).

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Event health checks now query a more accurate public endpoint.
    • Live display data (now playing, play history, bridge status) now uses the correct event identifier.
  • New Features

    • Authenticated event owners can search without human verification; guests and non-owners remain subject to verification.
  • Tests

    • Added/updated coverage for live-display polling identifier usage.
    • Added/updated coverage for owner bypass vs guest/non-owner verification behavior in event search.

thewrz and others added 2 commits June 18, 2026 22:51
The DJ dashboard "Search for Song" tool reuses the public guest search
endpoint /api/events/{code}/search, gated by require_verified_human_soft.
With human_verification_enforced=True in production the authenticated DJ —
who holds a JWT but no guest wrzdj_human cookie — was rejected with 403 on
their own event.

Add get_current_user_optional (non-raising optional bearer auth) and let the
authenticated event owner skip the human gate; anonymous guests still pass
through it. The dashboard now sends its JWT on eventSearch so the bypass
applies.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
After the collection-code / join_code split, three live-display endpoints
(/api/public/e/{code}/nowplaying|bridge-status|history) resolve strictly by
join_code (post-#324/#328 public-URL contract), but callers holding the
collection code 404'd:

- The DJ dashboard event page polled them with the collection code →
  console-spam 404s in a 5s loop. It now polls by event.join_code, kept in a
  second fetch batch so the request queue is not delayed behind getEvent.
- The bridge-app health check hit the join-code-only nowplaying endpoint with
  the collection code → 404 → "Event was deleted" → it stopped a perfectly
  live bridge every cycle. It now uses the dual-resolver
  /api/public/events/{code}, which accepts either code (same 200/404/410).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bc815935-e415-4928-b1b6-8a8e07267e09

📥 Commits

Reviewing files that changed from the base of the PR and between d73edb2 and 3c12180.

📒 Files selected for processing (1)
  • dashboard/app/(dj)/events/[code]/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • dashboard/app/(dj)/events/[code]/page.tsx

📝 Walkthrough

Walkthrough

The PR fixes two join-code endpoint resolution bugs (bridge health checks now call /api/public/events/{code}; dashboard live-display polling uses eventData.join_code) and adds an owner-bypass path to the event search human-verification gate via an optional OAuth2 bearer dependency on the server and a conditional Authorization header in the dashboard API client.

Changes

Join-Code Endpoint Resolution

Layer / File(s) Summary
Bridge health check endpoint switch
bridge-app/src/main/event-health-service.ts, bridge-app/src/main/__tests__/event-health-service.test.ts
checkEventHealth now fetches /api/public/events/${encodeURIComponent(eventCode)} instead of the nowplaying endpoint. Comments document the reason. Tests updated to assert the new URL pattern including percent-encoding.
Dashboard live-display polling via join_code
dashboard/app/(dj)/events/[code]/page.tsx, dashboard/app/(dj)/events/[code]/__tests__/page.test.tsx
loadData is split into two sequential Promise.all batches: the first resolves eventData, and the second fetches now-playing, bridge status, and play history using eventData.join_code. Regression test asserts live-display calls use join_code (TSETJO) while event/request calls use the collection code (TEST).

Event Search Owner Bypass

Layer / File(s) Summary
Optional bearer auth dependency
server/app/api/deps.py
Adds oauth2_scheme_optional (auto_error=False) and get_current_user_optional, a resolver that returns User or None without raising on any auth failure including missing token, decode errors, unknown user, or version mismatch.
event_search owner-bypass gate
server/app/api/events.py, dashboard/lib/api.ts
event_search accepts an optional current_user and response, computes is_owner, and calls require_verified_human_soft only for non-owners. ApiClient.eventSearch conditionally adds an Authorization: Bearer header when a JWT token is available.
Owner-bypass tests
server/tests/test_events.py
Adds _enforce_human_verification DB helper and TestEventSearchOwnerBypass covering three scenarios: owner bypasses the gate, anonymous request receives 403, non-owner authenticated user receives 403.

Sequence Diagram(s)

sequenceDiagram
  participant DJ as DJ Browser
  participant Dashboard as EventQueuePage
  participant API as Dashboard API Client
  participant Server as WrzDJ Server

  rect rgba(70, 130, 180, 0.5)
    note over Dashboard,Server: Phase 1 — resolve event
    Dashboard->>API: getEvent(collectionCode)
    API->>Server: GET /api/events/{code}
    Server-->>API: eventData { join_code }
    API-->>Dashboard: eventData
  end

  rect rgba(60, 179, 113, 0.5)
    note over Dashboard,Server: Phase 2 — live display via join_code
    Dashboard->>API: getNowPlaying(join_code)
    Dashboard->>API: getBridgeStatus(join_code)
    Dashboard->>API: getPlayHistory(join_code)
    API->>Server: GET /api/public/events/{join_code}/...
    Server-->>Dashboard: live display data
  end
Loading
sequenceDiagram
  participant Owner as Authenticated Owner
  participant Guest as Anonymous Guest
  participant Search as event_search endpoint
  participant Dep as get_current_user_optional
  participant HV as require_verified_human_soft

  Owner->>Search: GET /search [Bearer token]
  Search->>Dep: resolve token → User
  Dep-->>Search: User (is_owner = true)
  Search-->>Owner: 200 search results

  Guest->>Search: GET /search [no token]
  Search->>Dep: resolve token → None
  Dep-->>Search: None (is_owner = false)
  Search->>HV: check verification cookie
  HV-->>Search: 403 human_verification_required
  Search-->>Guest: 403
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • wrzonance/WrzDJ#383: Introduced the dual-code routing for GET /api/public/events/{code} that this PR's bridge health check now targets.
  • wrzonance/WrzDJ#328: Changed public bridge endpoint resolution and kiosk/overlay links to route by join_code, directly related to this PR's dashboard live-display polling switch to join_code.
  • wrzonance/WrzDJ#327: Updated dashboard/app/(dj)/events/[code]/page.tsx to use join_code for QR/label display, sharing the same file and concern as this PR's two-phase loadData restructuring.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes across multiple surfaces: fixing collection-code vs join_code mismatches affecting the dashboard (404s), search (403s), and bridge-app (self-stop).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dashboard-join-code-mismatch

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
dashboard/app/(dj)/events/[code]/page.tsx (1)

363-388: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Second batch still blocks core queue/event rendering.

The code awaits live-display calls before committing event/requests state, so slow now-playing/bridge/history responses can delay the primary queue view even though the comment says they’re non-critical.

Suggested restructure
-      const [historyData, nowPlayingData, bridgeStatusData] = await Promise.all([
-        api.getPlayHistory(liveCode).catch((): undefined => undefined),
-        api.getNowPlaying(liveCode).catch((): undefined => undefined),
-        api.getBridgeStatus(liveCode).catch(() => ({ connected: false, device_name: null, last_seen: null, circuit_breaker_state: null, buffer_size: null, plugin_id: null, deck_count: null, uptime_seconds: null })),
-      ]);
       setEvent(eventData);
       setRequests(requestsData.requests);
       setRequestTotal(requestsData.total);
       setStatusCounts(normalizeStatusCounts(requestsData.status_counts));
+      void Promise.all([
+        api.getPlayHistory(liveCode).catch((): undefined => undefined),
+        api.getNowPlaying(liveCode).catch((): undefined => undefined),
+        api.getBridgeStatus(liveCode).catch(() => ({ connected: false, device_name: null, last_seen: null, circuit_breaker_state: null, buffer_size: null, plugin_id: null, deck_count: null, uptime_seconds: null })),
+      ]).then(([historyData, nowPlayingData, bridgeStatusData]) => {
+        if (historyData !== undefined) {
+          setPlayHistory(historyData.items);
+          setPlayHistoryTotal(historyData.total);
+        }
+        if (nowPlayingData !== undefined) {
+          setNowPlaying(nowPlayingData ?? null);
+        }
+        setBridgeConnected(bridgeStatusData.connected);
+        setBridgeDetails({
+          circuitBreakerState: bridgeStatusData.circuit_breaker_state,
+          bufferSize: bridgeStatusData.buffer_size,
+          pluginId: bridgeStatusData.plugin_id,
+          deckCount: bridgeStatusData.deck_count,
+          uptimeSeconds: bridgeStatusData.uptime_seconds,
+        });
+      });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dashboard/app/`(dj)/events/[code]/page.tsx around lines 363 - 388, The second
Promise.all() awaiting getPlayHistory, getNowPlaying, and getBridgeStatus is
blocking state commit of the core eventData and requestsData. To fix this,
restructure the code to commit the eventData and requestsData state immediately
after the first Promise.all() resolves (which fetches getEvent, getRequests,
getDisplaySettings, getTidalStatus, and getBeatportStatus), then handle the
second batch of live-display calls for historyData, nowPlayingData, and
bridgeStatusData independently without awaiting them before the primary state
update. This allows the queue and event rendering to proceed immediately while
the non-critical live-display data loads in the background.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@dashboard/app/`(dj)/events/[code]/page.tsx:
- Around line 363-388: The second Promise.all() awaiting getPlayHistory,
getNowPlaying, and getBridgeStatus is blocking state commit of the core
eventData and requestsData. To fix this, restructure the code to commit the
eventData and requestsData state immediately after the first Promise.all()
resolves (which fetches getEvent, getRequests, getDisplaySettings,
getTidalStatus, and getBeatportStatus), then handle the second batch of
live-display calls for historyData, nowPlayingData, and bridgeStatusData
independently without awaiting them before the primary state update. This allows
the queue and event rendering to proceed immediately while the non-critical
live-display data loads in the background.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a98d3eec-99a2-4765-ad8c-15ea951dbce5

📥 Commits

Reviewing files that changed from the base of the PR and between 903fcc0 and d73edb2.

📒 Files selected for processing (8)
  • bridge-app/src/main/__tests__/event-health-service.test.ts
  • bridge-app/src/main/event-health-service.ts
  • dashboard/app/(dj)/events/[code]/__tests__/page.test.tsx
  • dashboard/app/(dj)/events/[code]/page.tsx
  • dashboard/lib/api.ts
  • server/app/api/deps.py
  • server/app/api/events.py
  • server/tests/test_events.py

loadData awaited the live-display batch (now-playing / bridge-status /
history) before committing any event or request state, so a slow live
endpoint delayed the primary queue view and the page spinner — the exact
opposite of what the code comment promised.

Commit the core event/queue/settings state immediately after the first
round-trip resolves, then fire the live-display batch in the background
and commit its results when they arrive. The spinner (`loading`, cleared
in `finally`) and the request queue now render as soon as their data is
in. join_code routing for the live endpoints is unchanged.

Addresses CodeRabbit review feedback on #492.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thewrz

thewrz commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai Addressed your outside-diff comment on dashboard/app/(dj)/events/[code]/page.tsx ("Second batch still blocks core queue/event rendering") in commit 3c12180.

loadData now commits the core event/queue/settings state immediately after the first round-trip resolves, then fires the live-display batch (now-playing / bridge-status / history) in the background via void Promise.all(...).then(...) and commits those results when they arrive. return true runs right after the core commit, so finallysetLoading(false) clears the page spinner without waiting on the live-display hop — matching what the code comment already claimed.

join_code routing for the three live endpoints is unchanged, and the regression test (Live-display polls use join_code, not collection code) still passes — those calls are issued synchronously when the Promise.all array is constructed, so backgrounding the commit doesn't defer the call. Full dashboard suite: 1341/1341 green, tsc + ESLint clean.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@thewrz The fix looks exactly right — backgrounding the .then(...) commit while issuing the live-display calls synchronously at Promise.all construction time is the precise distinction needed. Core state commits and the spinner clears on the first round-trip, live-display data populates whenever the second hop resolves, and join_code routing stays intact throughout. Clean approach.

@thewrz thewrz merged commit 002f752 into main Jun 19, 2026
10 checks passed
@thewrz thewrz deleted the fix/dashboard-join-code-mismatch branch June 19, 2026 06:55
thewrz added a commit that referenced this pull request Jun 19, 2026
…requests (#495)

POST /api/events/{code}/requests hard-depended on require_verified_human_soft,
which raises 403 when human_verification_enforced=True for any caller without a
valid wrzdj_human cookie. The DJ dashboard "Add" button in the song-search modal
reuses this public endpoint; the DJ holds a JWT but no guest human cookie, so on
their own event the enforced gate rejected them with 403 — the track never added.

Mirror the owner-bypass already applied to event_search (#492): accept an
optional bearer (get_current_user_optional) + Response, compute is_owner, and
only run require_verified_human_soft for non-owners. Guests and non-owner
authenticated users are still gated; create_request already tolerates a null
guest_id, so the owner path is safe.

Regression: TestSubmitRequestOwnerBypass (owner bypasses, anonymous 403,
non-owner 403), mirroring TestEventSearchOwnerBypass.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thewrz added a commit that referenced this pull request Jun 23, 2026
…endpoint drift (#537)

Regenerate via types:export + types:generate. No source changes — reflects
already-merged backend drift: #530 default_factory fields drop default:[],
owner-JWT bypass adds security scheme to submit/search (#492/#495), and the
event_search docstring. Generated-types diff is JSDoc-only; no TS shape change.

Closes #536

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant