[feat] Sign-in parity, chat-package re-sync, and the mobile UX pass (12/12) - #5691
[feat] Sign-in parity, chat-package re-sync, and the mobile UX pass (12/12)#5691ardaerzin wants to merge 19 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change expands the mobile application with authentication, context and session navigation, chat transcript replay, approval steering, shared UI components, OAuth callback routing, and updated mobile design and execution documentation. ChangesMobile planning and contracts
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SignInScreen
participant MobileAuth
participant OAuthProvider
participant AuthCallback
participant MobileGate
SignInScreen->>MobileAuth: Start password, OTP, or OIDC sign-in
MobileAuth->>OAuthProvider: Redirect for OIDC authorization
OAuthProvider->>AuthCallback: Return callback parameters
AuthCallback->>MobileAuth: Complete OIDC sign-in
MobileAuth->>SignInScreen: Report authentication outcome
MobileGate->>AuthCallback: Pass callback route without mobile redirect
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
28314ae to
cc14c6a
Compare
a0ceb44 to
553ef34
Compare
cc14c6a to
0aa2522
Compare
553ef34 to
61f34b3
Compare
0aa2522 to
cfb9c84
Compare
61f34b3 to
ccdc697
Compare
cfb9c84 to
defca8b
Compare
ccdc697 to
5c01194
Compare
defca8b to
de37daa
Compare
5c01194 to
a60bf4e
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
de37daa to
94e03ca
Compare
a60bf4e to
59641db
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
94e03ca to
52d6e1f
Compare
59641db to
4ed04bf
Compare
52d6e1f to
4c08deb
Compare
|
me hoping that preview works :D |
02fb123 to
24ac72c
Compare
632cee9 to
0e73db2
Compare
Railway Preview Environment
|
The approval dock gains a flag-gated Redirect control that sends the denial and a short instruction in one respond call. Delivery is cold-replay-only: a warm-parked resume answers the harness gate on the still-pending original prompt and never prompts again, so the note is dropped (measured live; the model then re-tried the denied work with a sibling tool and raised a new gate). The harness has no reject-with-feedback channel either - PermissionReply is once|always|reject - so the control stays hidden unless NEXT_PUBLIC_AGENT_CHAT_STEER=true, matching the desktop flag.
Providers only ever redirect to the one URI registered for them, the desktop `/auth/callback/<providerId>`, so a mobile SSO flow could never land in /m and the mobile sign-in had to tell users to switch to the desktop app. The mobile app now marks the flow with a short-lived cookie before leaving for the provider. The desktop forward gate reads it and forwards the landing to `/m/auth/callback/...` with the query intact, where the OAuth state (same-origin sessionStorage) is still readable — no new redirect URI has to be registered with any provider. `?view=desktop` still wins, and the other desktop-only exceptions are untouched. The reverse gate also stops bouncing any `/auth/callback` off /m: an OAuth landing must complete where it lands or the one-time code is lost.
/m/auth only did email+password and told everyone else to use the desktop app. It now offers every method the deployment enables, derived from the same env flags as the desktop's getEffectiveAuthConfig: - Passwordless: request a code, verify it, resend on a 60s cooldown, with each SuperTokens status mapped to a retry-in-place or a restart. - Social: one button per configured OAuth client id, in the desktop's order, running the redirect flow through the shared device gate's mobile callback. - Organization SSO: /auth/discover by work email, then the same redirect flow (404 on OSS reads as "no SSO", not an error). - Email/password unchanged; an SSO-only deployment no longer shows an email form, and a deployment with nothing enabled says so instead of rendering an empty card. Every route lands the same way it already did — invalidate the cached ["mobile","projects"] verdict, then hand over to the root context resolver. The pure halves (provider derivation, mode selection, the OTP state machine, discovery parsing) are unit-tested; 40 -> 83 tests.
…calStorage key
The package copy of the agent-chat message store writes the same localStorage key
as the desktop original but had drifted twice: it used jotai's default storage,
re-enabling the cross-tab `storage` subscribe the original deliberately strips
(an incoming replacement unmounts a streaming conversation and orphans its stream
mid-turn), and its quota guard returned void, so there was no record-count
watermark counterpart to `agenta:agent-chat:record-counts`.
Port `tabLocalStorage()`, the watermark store and its read atom, the
`{evicted, persisted}` quota-guard result, `persistSessionMessagesAtom`'s
`recordCount`, and the joint deletion path (exposed as a write atom here, since
the scope-keyed delete/close/prune callers stay in the app layer).
…eam nest The badge read `flags.is_running` raw, so it disagreed with the desktop tab dot on every row the coarse filter returns but the flags contradict: a flagless or proc-dead (zombie) row was badged live, where the desktop dot reads it as idle. Split on `deriveStreamNest` instead — running wins, then alive, everything else is omitted — so both surfaces answer "running"/"alive" from one derivation. Polling cadence and the react-query shape are untouched.
…termark The chat screen hand-rolled its adoption logic on message counts alone, so it missed the case the shared guard exists for: a turn that grows in place — tool results landing, an approval round-trip completing — keeps its message count, and mobile kept rendering the partial copy (issue #5530). Mobile now calls `shouldAdoptServerTranscript`. Its inputs map as: `busy` is always false (mobile never holds a live stream), and the watermark is an in-memory ref rather than desktop's persisted record-counts store, since mobile caches no transcript. The mapping lives in `transcriptAdoption.ts` so it is testable without mounting the hook. The trailing-refresh queue, the cancellation flag and the loading/ready/empty transitions are unchanged. The guard needs a record count, so the package's `loadSessionMessages` regains the `SessionTranscript` return its OSS original already carries (the copy is byte-identical again below its header); `useAgentConversation` follows the new shape and now files the record count with the transcripts it persists.
… switchable Sign-in and the OIDC callback carry the Agenta wordmark, inlined rather than served from public/ (a bare <img src="/assets/..."> misses the /m basePath) and drawn in currentColor so one component covers both themes. Header pinning becomes one ScreenScaffold instead of a shape hand-copied per screen: a h-dvh column whose middle is the only scroller. The project picker never had it and scrolled its own title away. The chat dock is passed as a footer only when it renders, so the scaffold's safe-area rule holds instead of silently lapsing when no gate is pending. The workspace line is now a real selector: chips when there are several, a labelled name when there is one (the default workspace is called "Default", indistinguishable from a project of that name). Reaching the picker at all needed ?switch=1, since a stored context auto-forwards past it — and that decision is gated on router.isReady, without which the first render forwards before the switch intent is knowable. Signed-out copy no longer claims you signed out on a screen that also renders for someone who never signed in.
123 commits in one PR is not reviewable. Measured against candidate lanes, only three files are touched by two lanes, and in each the lanes stack in the order the edits happened — so a chronological path-partitioned replay needs no hunk surgery. Twelve lanes, with the empty diff against the branch tip as the correctness gate.
…er attachments The package copy had drifted from the original it was extracted from, in two ways that were both user-visible on mobile. Approval resume: no pause folding, no settle pass. A turn that paused for a gate and then resumed replayed as still parked, so a reload kept the approval dock up on a turn the user had already answered — worst when the gate was answered on another device, where the durable log carries no interaction_response at all and the settle pass is the only thing that closes it. Ported with the original's tool-call dedup and re-raise-under-a-new-id handling. Attachments: a message that carried files replayed as bare text. Ported the file parts and their content URL (on @agenta/shared/api rather than the OSS app layer), and taught mobile to render them — inline for images, a labelled link otherwise. Seven tests ported from the original alongside the settle logic; removing the settle pass fails the no-interaction_response case.
Path-partitioned replay was rejected during execution: a pre-flight check over the real commit x file matrix found 42 files touched by more than one lane, 22 with an edit order no lane ordering can respect — each of which would have had a lower lane replay a newer version underneath a higher one, silently dropping edits. Contiguous ranges of the linear history cannot have that failure, and need no replay at all. Records the 12 lanes as built, and that fixed merge order is the trade.
The list never refreshed. Only staleTime was set, no refetchInterval, and the app disables refetchOnWindowFocus globally — so a session created on desktop stayed invisible until the screen remounted. The badges on existing rows polled while the set of rows was frozen. Poll the newest page as a SEPARATE query rather than adding refetchInterval to the infinite one: refetching an infinite query refetches every page scrolled into, so its cost grows with scroll depth. This stays one request per tick. Foreground-only, plus a visibilitychange refetch so returning from the lock screen does not wait out the interval. The merge dedupes on session identity and lets the head win on order: a session that just became active is returned at its new position and would otherwise render twice — fresh at the top, stale in the page it was fetched in.
Statuses rendered as bare coloured words butted against a title, so "new session test running" read as a sentence rather than a name plus its state. One StatusTag pill with its own border and background now carries all of them: the row badges (running/live/approval/ ended), the chat running bar, the transcript's awaiting-approval marker, and the list header's pending count. A dot marks the two tones that mean something is happening, so the distinction does not rest on colour alone. The title truncates independently of its tags, which the old inline spans could not do — a long name pushed them off the row.
The live and muted tones had byte-identical class strings, so a warm session and an ended one differed only by a dot — which is why live read as inert grey. They are opposite states. Bridge the palette semantic success role into the mobile theme (it already carries error) and give live its own tone. The three states are now separable without reading the label: accent means act on this, success means healthy and warm, muted means nothing is happening.
"2 approvals pending" told you work existed and left you to find it. The count is now a toggle that narrows the list to the sessions waiting on you. It counts SESSIONS, not interactions — the filter shows rows, and one session can hold several gates, so the old number could exceed the rows it produced. The interactions poll is project-wide while the list is paged, so a waiting session can sit beyond what has been fetched. Rather than silently showing a short list, the filter reports how many are further down and offers to load them; the empty case says the same instead of claiming there is nothing to do. Answering the last gate clears the filter rather than stranding the user on an empty list.
…hrase "show only these" pointed at nothing — while the filter is off the rows it means are not on screen, and a pill next to an underlined phrase reads as two controls when it is one. One toggleable chip carries it: the icon supplies the affordance (funnel to apply, cross to clear) so the visible label stays the count, and the spoken label is the full sentence the visible text should not have to be. The unloaded-count copy loses its instruction too — the Load more button is directly beneath it.
Three stacked full-width rows plus 44px tap targets on text-sized controls left roughly 150px of chrome above the first session — most of a phone screen's useful top half. Search and the filter now share a row, since both narrow the same list. The chip and the project switcher keep their 44px touch targets through an `after` pseudo-element instead of their own height, so a 32px chip and an 18px switcher row still hit the minimum. Header is about 84px. The search input keeps text-base: below 16px iOS zooms the viewport on focus.
The workspace governs everything below it, but it sat inside the scrolling list — with 50 projects it scrolled away, leaving no indication of which workspace the rows belonged to and no way to change it without scrolling back. It moves into the pinned header beside the title: chips when there are several, a labelled name when there is one. The list below becomes the selected workspace's projects, which also drops the nested section-per-workspace shape it no longer needs. Splits the old component in three along the way — the selector, the list, and the grouping, which was an inline useMemo and is now a covered pure function.
Review of the plan set found the documents lagging the code in about twenty places. Each correction was checked against the tree, not against the plan: - WP0's residual gaps are all implemented. The section is re-headed as the analysis that produced the work, and the step text now carries the two things review changed during execution: ordering rides `coalesce(updated_at, created_at)` because `updated_at` is nullable and a DESC sort puts NULLs first, and the references hydration requires the batch `latest_turn_per_session` helper rather than the per-session one, which would make /sessions/query an N+1 path. - The infra-tail plan's snippets now match the workflow: dispatch inputs through `env:`, exact status assertions in the smoke checks, the segment-exact Traefik rule, `pipefail` before the piped verifications, and a bounded readiness poll with a cleanup trap instead of `sleep 3`. - The WP1 plan gains the repo-wide `cd web && pnpm lint-fix` step, the corrected Turbo inputs, package-relative paths in the shadcn verification (it runs after `cd web/mobile`), and the dark role-map values review actually landed. - `NoMobilePageWrapper` retires when the gate is turned on, not when its code ships. The design doc said the latter, which would leave a mobile visitor with neither the gate nor the blocker. - The m3 plan states the three event types the endpoint emits, records that the channel's project comes from the credential and never the URL, corrects the claim that a session-scoped stream can retire the list's project-wide badge polls, and states the unbounded stream age as a known gap with its fix rather than as an aside. - The approvals plan is re-statused as a pre-execution snapshot, since M2 shipped a different client payload than it sketches. - The chat contract's `useApprovalDock` line named an argument the hook does not take and a `renderer` it does not return, while omitting `open` and `responding`. - README: the WP5 status contradiction, the doubled path in the standalone check, a completed `.gitignore` chore still listed as pending, the commit count against the review record, and `run.sh` invocations that named neither the entrypoint nor the edition.
24ac72c to
02804ea
Compare
0e73db2 to
0105dfe
Compare
Context
The top of the stack: sign-in parity, the first consolidation work against the copy-drift the stack created, and the mobile UX pass that came out of using it on a real phone.
Changes
Sign-in parity.
/mgains email OTP, social providers, and org SSO. Providers only ever redirect to the one registered URI (the desktop/auth/callback/<id>), so the desktop hands a mobile-initiated callback back to/mrather than requiring a second registration.Consolidation, wave 0.
@agenta/chat's message store had drifted from the OSS original it was copied from on the same localStorage key: it had re-enabled the cross-tab sync OSS deliberately strips, and dropped the record-count watermark. The replay adapter had drifted too, missing the approval-resume handling (a resumed turn replayed as still parked, so a reload kept the approval dock up) and user attachments (a message with files replayed as bare text). Both re-synced, with the mobile side rendering attachments.Correctness. A replayed approval envelope now names its tool. Without it the runner's resume nudge said "Call the tool again", naming nothing the model could call, and the model narrated a fabricated execution instead of re-issuing the call.
UX pass. Statuses became real tags instead of bare coloured words (and
liveno longer renders identically toended). The pending count became a filter rather than a statistic. The session list polls its newest page, so a session created on desktop appears without a manual refresh. The header stopped eating the first screen, and the workspace selector moved into it.Tests / notes
@agenta/chatsuites both grew with this work; the settle fix is covered by tests ported from the OSS original, and I verified they fail without it.What to QA
/mwith Google, with an email OTP, and with org SSO. Each completes and lands you in the app.