Conversation
Guard the observer dashboard's `?key=<token>` auto-login path (used by the "Join as observer" links that embed either an `rk_live_` workspace key or an `ot_live_` observer token). The reader in `RelaySessionProvider` is functionally correct today but has silently regressed twice — once when the observer-token prefix was introduced (#230) and again when the stream-token minting reshuffled the effect body (#240) — because nothing directly asserted the URL param actually flows into the login POST. Extract the effect body into a pure `resolveObserverSession` helper and cover every branch (both prefixes, rejected key, no key with a cookie session, malformed key, network error, missing wsToken) with unit tests. Add a jsdom integration test that mounts the provider with `?key=ot_live_...` in the URL and asserts the login POST fires with the exact token and the authenticated children render. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A report claimed shared observer links fail on browsers that ever previously logged in — supposedly because the dashboard probes /session first, sees a 401 from stale cookies, and redirects to /login without ever using the URL `?key=`. Reproducing this against `next dev` + headless Chrome (fresh cookie jar for a revoked token, then navigating to /observer?key=<fresh valid token>) shows the current flow works: RelaySessionProvider POSTs /login first, which overwrites the stale cookies via Set-Cookie, and the follow-up /session GET returns the fresh identity. Same result on `main` and on this branch — no functional regression is present. Since the described failure mode (session-first ordering) *would* actually break the shared-link flow if it ever slipped back in, add explicit guards for it: - observer-auto-login.test.ts: a resolver-level test that fails loudly if /session is fetched before /login when a URL key is present. - RelaySessionProvider.test.tsx: a jsdom integration test that mounts the provider with `?key=<token>` and a fetch mock that throws if /session is probed before /login, and asserts the RelayProvider receives the URL identity (not any cookie-derived one). - observer-auto-login.ts: a "ORDERING IS LOAD-BEARING" doc comment on `resolveObserverSession` explaining exactly why URL-key POST must come before the session GET. No production code changes. Empirical verification: headless Chrome + `next dev` with a mock engine, cookies seeded to a revoked token, navigate to /observer?key=<fresh token>. The login POST fires, Set-Cookie overwrites the stale jar, /session returns the fresh identity, dashboard renders. Same result before and after these changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
|
Closing per empirical findings from two agents. Both verified end-to-end in a fresh browser via CDP: the observer at Rather than merge test coverage without the associated fix, closing this to keep the surface honest. The empirical evidence and both agents' full traces are preserved on the branch ( |
1 participant
Summary
?key=<token>auto-login path used by "Join as observer" links (accepts bothrk_live_workspace keys andot_live_observer tokens).RelaySessionProviderinto a pureresolveObserverSessionhelper so the URL-reader → login POST → session GET sequence is unit-testable without mounting React.wsToken) plus jsdom integration tests that mount the provider with?key=ot_live_...and assert the login POST fires with the exact token before the authenticated children render./sessionbefore/login(which would break shared observer links for any returning browser) is caught by tests, not in the field.Empirical repro attempt (for the reported "returning-user shared link" bug)
The report was: browsers with cookies from a prior (now-revoked) session that navigate to
/observer?key=<fresh valid token>supposedly hit/sessionfirst, 401 on the stale cookies, and redirect to/loginwithout ever POSTing the URL key.I could not reproduce this against the actual dashboard. Method:
next devon this branch, a mock engine that accepts a curated set of tokens (401s everything else), and headless Chrome driven over CDP. Test matrix:Same result on
mainand on this branch: the login POST fires before the session GET whenever the URL carries a valid?key=, exactly as required. The described session-first regression is not present.What this PR does anyway
Even though I couldn't reproduce the described failure, the failure mode is real: reversing the login/session order in
resolveObserverSession(or accidentally short-circuiting to/sessionfirst) would break the shared-link flow on any returning browser. That regression is now explicitly guarded:resolveObserverSessiongets an "ORDERING IS LOAD-BEARING" doc block explaining why URL-key POST must precede session GET.observer-auto-login.test.ts): fails loudly if/sessionis fetched before/loginwhen a URL key is present.RelaySessionProvider.test.tsx): mounts the provider with?key=<token>and a fetch mock that throws if/sessionis probed before/login, and asserts theRelayProviderreceives the URL identity.Test plan
packages/observer-dashboard:npx vitest run— 34/34 passing (13 unit tests inobserver-auto-login.test.tsincluding the new stale-cookies guard, 3 integration tests inRelaySessionProvider.test.tsxincluding the new stale-cookies guard, 18 pre-existing tests unchanged).next devwith a mock engine, cookies seeded to a revoked token, navigating to/observer?key=<fresh token>: login POST fires, Set-Cookie overwrites the stale jar, /session returns the fresh identity, dashboard renders.https://agentrelay.com/observer?key=ot_live_...once deployed (test token supplied out-of-band).🤖 Generated with Claude Code