test(embed): take the panel session id from the test, not from a parsed href - #524
Conversation
…ed href The embed e2e helpers rebuilt the panel route prefix with interpolatePath and a probe id, then dissected the persisted href back into a session id. The tests now create the session they are about to exercise and pass its id to untilPanelDraft, so the id comes from the server that minted it. Deletes helpers/navigation.ts (panelHref, panelSessionIdOf, panelSessionId, SESSION_PROBE, RELATIVE_HREF_BASE) and the @tanstack/router-core devDep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 3 minutes Limit details: You’ve used all 3 included reviews currently available under your plan. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
💤 Files with no reviewable changes (2)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe embed E2E helpers now create sessions explicitly, wait for session boot traffic, and scope draft checks to session IDs. Related panel tests use the new flow. The embed package also removes an unused development dependency. ChangesEmbed E2E session scoping
Embed dependency cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The tests now use session IDs returned when they create sessions, but the boot flow does not explicitly confirm that the created session is the one opened afterward. If selection diverges, assertions could target the wrong session; the change is otherwise mergeable with explicit owner awareness or follow-up. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…n pill, not an rpc settle The helper waited on sessions.list/sessions.resolve wire traffic and swallowed the resolve timeout, so an unfired resolve let the launcher click fall through to /panel/latest and mint a different session than the one the test created. It now names the created session, picks it in the session selector, and blocks on the pill carrying that title. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Follow-up to #514. The embed test helpers reconstructed a session id by dissecting a persisted URL. They now take it from the test that created the session.
What was there
packages/embed/tests/e2e/helpers/navigation.tsbuilt a path with a fake id, split it back apart to recover the literal/panel/, then round-tripped to validate:The stated point was that the router should own path construction, so a route rename breaks the helper. That never held:
'/panel/$sessionId'is a bare string literal, so a rename changes nothing at compile time and the round-trip still passes against the renamed literal. What remained was the indirection without the guarantee.The right TanStack APIs for this are
router.buildLocation({to, params})androuter.matchRoutes(pathname), but both need a router instance. There is none in the Playwright node process — importing@conciv/app/routerthere fails withERR_UNKNOWN_FILE_EXTENSIONbecause the route tree pulls Solid.tsx, and the widget deliberately does not expose its router globally (packages/embed/src/mount-impl.tsx:99,109saves and restoreswindow.__TSR_ROUTER__).So the fix is not a better parsing API. The test process should not be reconstructing app knowledge at all — the same mistake as the seeding backdoor #514 removed, one layer down.
What it does now
The test creates the session, so the test knows the id. New
openPanelOnNewSession(page, suite)inhelpers/panel.tscreates the session, attaches the wire watcher beforegoto, settles boot traffic, opens the panel, and returns the id.The boot-traffic wait is load-bearing rather than defensive: without it the launcher click can beat the
sessions.list/sessions.resolvequeries and fall through to/panel/latest, which mints a different id. That is the same racewarm-session-resolve.it.test.tsalready waits on.sessions.resolve({})is not an alternative, and this is worth recording. With noidit does not return the currently-open session, nor the most recently updated one —packages/core/src/chat/session-rows.ts:70mints a fresh uuid and does not insert a row.panel.latest.tsxuses it as "give me a new session to land on". Reading it from a test would hand back an id no draft is ever written under.Deleted
helpers/navigation.tsentirely (panelHref,panelSessionIdOf,panelSessionId,SESSION_PROBE,RELATIVE_HREF_BASE, theinterpolatePathimport),panelDraft(only ever used insidedrafts.ts), and the@tanstack/router-coredevDependency — confirmed orphaned by grep andfallow dead-code --trace-dependency @tanstack/router-core→UNUSED (0 imports).One failure worth reporting
The first full run was 98 passed / 1 failed, in
native-widget:It passed in isolation. Root cause: that file shares one kit across tests, so an earlier test persists a navigation row and the later page boot restores that route instead of consulting the warm session — leaving the grab test on the earlier test's minted session. Fixed by creating the session in
beforeAll, before any page boots, so the restored row already points at it. Not retried, not quarantined.Evidence
Embed suite
99 passed× 3 real runs (cache-bypassed, since turbo went FULL TURBO). extension-testkit and apps/conciv green. Whole-repo typecheck 96/96, lint 101/101, format clean,fallow auditzero introduced,check-changesetsexit 0. Zero product-code delta.Note
panel-session-switch.it.test.ts(from #523) is not on this base and is untouched. When both land it must supply its own session ids the same way —resolve({})mints, so it cannot be used to disambiguate a switch.🤖 Generated with Claude Code
Summary by CodeRabbit