[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.
|
📝 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
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.
52d6e1f to
4c08deb
Compare
4ed04bf to
b5476ae
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/design/agenta-mobile/plans/2026-07-27-m3-live-relay.md (1)
227-243: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAdd publishers for the new SSE event types.
T2 now promises
records-changed,lifecycle, andinteraction, but T1 only publishes afterRecordsWorker.append_many, and T2 still says the endpoint yieldsrecords-changedframes. Without lifecycle and interaction publishers, the open-session liveness and approval updates do not arrive. Define their source transitions, channel payloads, and tests, or remove them from the M3 contract.Also applies to: 293-300
🟡 Minor comments (13)
web/mobile/src/features/sessions/SessionListScreen.tsx-74-76 (1)
74-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not report search-excluded sessions as unloaded.
When
searchis nonempty,mergedcontains only search matches butpendingBySessionstill contains every project pending session.pending.unloadedthen claims that search-excluded sessions are further down the list, although loading more pages cannot show them.Scope pending IDs to the search result, or suppress the unloaded count and load-more prompt while a search is active. Add a regression test for this state.
web/mobile/src/features/context/ContextResolver.tsx-30-31 (1)
30-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the loading state until
router.isReadyis true.If a saved context exists during the first client render,
shortcutdisables the query whileselectContextTargetreturnsnullbecause the router is not ready. Line 71 then skips the loading branch and renders “Something went wrong.” before the redirect starts.Proposed fix
- if (target || (!shortcut && query.isPending)) { + if (!router.isReady || target || (!shortcut && query.isPending)) {Also applies to: 71-74
web/mobile/src/features/auth/EmailPasswordForm.tsx-33-50 (1)
33-50: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winGive every auth input an accessible name. All the new sign-in fields rely on
placeholderalone. A placeholder is not a reliable accessible name, and it disappears once the user types. Add anaria-label, or add a visually hidden<label>bound withhtmlFor.
web/mobile/src/features/auth/EmailPasswordForm.tsx#L33-L50: addaria-label="Email"to the email input andaria-label="Password"to the password input.web/mobile/src/features/auth/EmailOtpForm.tsx#L84-L92: addaria-label="Email"to the email input.web/mobile/src/features/auth/EmailOtpForm.tsx#L106-L116: addaria-label="6-digit code"to the code input.web/mobile/src/features/auth/EmailPasswordForm.tsx-22-28 (1)
22-28: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the
onSuccess()call in both sign-in forms. Both formsawait onSuccess()and return without resetting their pending state.useAuthSuccesscallsqueryClient.invalidateQueriesandrouter.replace, and both can reject. If the promise rejects, the form stays disabled with no error, and the rejection is unhandled.
web/mobile/src/features/auth/EmailPasswordForm.tsx#L22-L28: wrapawait onSuccess()intry/catch; in the catch, callsetPending(false)and set an error message.web/mobile/src/features/auth/EmailOtpForm.tsx#L37-L41: wrapawait onSuccess()intry/catch; in the catch, dispatch{type: "failed", message: ...}so the phase leaves"verifying".🛡️ Proposed fix for EmailPasswordForm.tsx
const outcome = await signInWithEmailPassword(email.trim(), password) if (outcome.kind === "ok") { - await onSuccess() - return + try { + await onSuccess() + return + } catch { + setPending(false) + setError("Signed in, but navigation failed. Try again.") + return + } }web/mobile/src/features/auth/SignInScreen.tsx-71-73 (1)
71-73: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a page heading for the sign-in screen.
The header contains only a logo and a
<p>. Screen-reader users cannot reach the page purpose through heading navigation. Promote the description to an<h1>, or add a visually hidden<h1>.♿ Proposed fix
<header className="flex flex-col items-center gap-3"> <AgentaLogo className="text-foreground h-6 w-auto" /> - <p className="text-muted-foreground text-xs">Sign in or create an account.</p> + <h1 className="text-muted-foreground text-xs font-normal"> + Sign in or create an account. + </h1> </header>web/mobile/src/features/auth/SsoDiscoveryForm.tsx-97-105 (1)
97-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a programmatic label for the email input.
A placeholder is not an accessible label. Add an associated
<label>oraria-label="Work email"so screen reader users can identify the field.web/mobile/src/features/chat/ApprovalDock.tsx-64-75 (1)
64-75: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd an accessible name to the redirect textarea.
The text at Lines 64-66 is not associated with the textarea. Screen readers announce an unnamed control. Add a
<label>or anaria-label.web/mobile/src/features/chat/TurnRow.tsx-64-68 (1)
64-68: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAllow the approval tool label to shrink.
truncatecannot constrain this flex item while its intrinsic minimum width remains active. Long tool names can overflow the transcript on narrow screens. Addmin-w-0 max-w-fullto the row andmin-w-0to the label.web/packages/agenta-chat/src/state/sessionMessages.ts-158-168 (1)
158-168: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the watermark write against the same quota failure.
writeMessagesWithQuotaGuardhandlesQuotaExceededErrorfor the messages key. Theset(sessionRecordCountsAtom, counts)call at the end has no guard. If storage is full, this write can throw aDOMExceptionthat escapes the atom writer and reaches the effect inuseAgentConversation. The reported failure would then be an unhandled error instead of a skipped persistence.🛡️ Proposed guard
- set(sessionRecordCountsAtom, counts) + try { + set(sessionRecordCountsAtom, counts) + } catch (e) { + if (!isQuotaExceeded(e)) throw e + console.warn("[agent-chat] record-count store over quota; skipping watermark write") + }web/packages/agenta-chat/src/assets/transcriptToMessages.ts-139-159 (1)
139-159: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAttachment-only messages still add an empty text part.
draft.parts.push({type: "text", text: str(payload.text)})runs before the attachment loop. Amessagerecord that carries only attachments therefore replays with an empty text part in front of the file parts. If the renderer draws a bubble for an empty text part, the replayed turn shows a blank line. Confirm the OSS original behaves the same before changing it, because this file must keep parity.🐛 Optional guard
- draft.parts.push({type: "text", text: str(payload.text)}) + const text = str(payload.text) + if (text) draft.parts.push({type: "text", text})docs/design/agenta-mobile/design.md-176-183 (1)
176-183: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the WP0 closure status.
The banner says all five items are implemented, but Line 198 still lists the liveness filter as deferred and Fern regeneration as pending. State that the required items are closed, keep optional work explicitly deferred, and label the following list as historical gap analysis.
docs/design/agenta-sessions-consolidation/plan.md-47-50 (1)
47-50: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the fenced command block.
The fence at Line 47 triggers MD040. Use
shellfor thegrepcommands.Based on static analysis: markdownlint reported MD040 for this fence.
Source: Linters/SAST tools
docs/design/agenta-sessions-consolidation/plan.md-290-297 (1)
290-297: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRun each standing gate from the repository root.
In one shell, the first
cd web/packages/agenta-entitiespersists. The nextcd web/packages/agenta-chatthen resolves relative to that package and fails. Wrap each command in a subshell or reset to the repository root before every command.
🧹 Nitpick comments (7)
web/packages/agenta-shared/tests/unit/mobileGate.test.ts (1)
170-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: pass the callback cookie through the
inputhelper instead of mutatingi.cookie.Four tests reassign
i.cookieafterinput()returns. The helper already spreads...restover the built object, so acookiesoption would remove the mutation and the repetition.♻️ Proposed helper change
-const input = (overrides: Partial<GateInput> & {headers?: Record<string, string>}): GateInput => { - const {headers = {}, ...rest} = overrides - const cookies: Record<string, string> = {} +const input = ( + overrides: Partial<GateInput> & { + headers?: Record<string, string> + cookies?: Record<string, string> + }, +): GateInput => { + const {headers = {}, cookies = {}, ...rest} = overridesThen each test becomes:
- const i = input({pathname: "/auth/callback/github", headers: docHeaders(DESKTOP_UA)}) - i.cookie = (name) => (name === MOBILE_AUTH_CALLBACK_COOKIE ? "1" : undefined) - expect(decideDesktopGate(i)).toEqual({ + expect( + decideDesktopGate( + input({ + pathname: "/auth/callback/github", + headers: docHeaders(DESKTOP_UA), + cookies: {[MOBILE_AUTH_CALLBACK_COOKIE]: "1"}, + }), + ), + ).toEqual({ kind: "redirect", location: "/m/auth/callback/github", })web/mobile/src/components/FilterChip.tsx (1)
7-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce nonessential multi-line comments.
web/mobile/src/components/FilterChip.tsx#L7-L14: remove the UI rationale or reduce it to one short comment.web/mobile/src/features/sessions/SessionListScreen.tsx#L147-L148: remove the layout rationale or reduce it to one short comment.As per coding guidelines, "Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements."
Source: Coding guidelines
web/mobile/src/features/auth/EmailOtpForm.tsx (1)
128-128: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the cooldown copy from
OTP_RESEND_COOLDOWN_MS.The label hardcodes
60s.OTP_RESEND_COOLDOWN_MScontrols the actual wait. If the constant changes, this text becomes wrong.♻️ Proposed refactor
- {state.resendBlocked ? "Code sent — wait 60s to resend" : "Resend code"} + {state.resendBlocked + ? `Code sent — wait ${Math.round(OTP_RESEND_COOLDOWN_MS / 1000)}s to resend` + : "Resend code"}web/mobile/src/features/auth/SignInScreen.tsx (1)
46-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
providers.length === 0check is redundant.
ssoDiscoveryisisOidcEnabled(), andisOidcEnabledreturnstruewheneverlistOidcProvidersis non-empty (web/mobile/src/lib/auth/config.tslines 57-60). Soproviders.length > 0already impliesssoDiscovery. The condition is equivalent tomethods.mode === "disabled" && !methods.ssoDiscovery.Keeping the explicit check is defensible as documentation of intent. Remove it only if you want the tighter expression.
web/mobile/src/features/auth/SsoDiscoveryForm.tsx (1)
10-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten this comment.
This comment describes normal control flow. Replace it with one short line or remove it.
As per coding guidelines: “Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements.”
Source: Coding guidelines
web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts (1)
124-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the attachment replay path.
The new
message-with-attachmentsbranch and thefilenamefield onfileevents have no test here. A record-level fixture is enough: assert the file part'surl,mediaType,filename, andproviderMetadata.agenta.attachmentId. This locks the URL contract thatattachmentContentUrlproduces, which the PR description notes was not exercised against a real attachment session.docs/design/agenta-mobile/plans/2026-07-27-m3-live-relay.md (1)
259-266: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsolidate the M3 tenant-boundary guidance with the consolidated plan.
The M3 plan now specifies
project_idonly viarequest.state.project_id, but keep the same explicit instruction indocs/design/agenta-sessions-consolidation/plan.mdfor S4 and section 5.3 so the shared relay contract stays aligned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d81cb020-23f8-4fa9-8e4f-2c61becd864d
⛔ Files ignored due to path filters (1)
web/mobile/src/styles/theme.generated.cssis excluded by!**/*.generated.*
📒 Files selected for processing (83)
api/oss/src/tasks/asyncio/sessions/interactions_dispatcher.pydocs/design/agenta-mobile/README.mddocs/design/agenta-mobile/chat-headless-contract.mddocs/design/agenta-mobile/design.mddocs/design/agenta-mobile/plans/2026-07-12-wp0-sessions-query-and-stamping.mddocs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.mddocs/design/agenta-mobile/plans/2026-07-25-wp1-infra-tail.mddocs/design/agenta-mobile/plans/2026-07-26-wp5-device-gate.mddocs/design/agenta-mobile/plans/2026-07-27-m3-live-relay.mddocs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.mddocs/design/agenta-mobile/plans/2026-08-01-pr-stack-split.mddocs/design/agenta-sessions-consolidation/plan.mdweb/mobile/scripts/generate-shadcn-tokens.tsweb/mobile/src/components/AgentaLogo.tsxweb/mobile/src/components/FilterChip.tsxweb/mobile/src/components/ScreenScaffold.tsxweb/mobile/src/components/StatusTag.tsxweb/mobile/src/features/auth/AuthDivider.tsxweb/mobile/src/features/auth/EmailOtpForm.tsxweb/mobile/src/features/auth/EmailPasswordForm.tsxweb/mobile/src/features/auth/OidcCallbackScreen.tsxweb/mobile/src/features/auth/OidcProviderButtons.tsxweb/mobile/src/features/auth/SignInScreen.tsxweb/mobile/src/features/auth/SsoDiscoveryForm.tsxweb/mobile/src/features/auth/authStyles.tsweb/mobile/src/features/auth/states/AuthMethodsSkeleton.tsxweb/mobile/src/features/auth/states/NoAuthMethods.tsxweb/mobile/src/features/auth/useAuthSuccess.tsweb/mobile/src/features/chat/ApprovalDock.tsxweb/mobile/src/features/chat/AttachmentPart.tsxweb/mobile/src/features/chat/ChatScreen.tsxweb/mobile/src/features/chat/TurnRow.tsxweb/mobile/src/features/chat/steer.tsweb/mobile/src/features/chat/transcriptAdoption.tsweb/mobile/src/features/chat/useApprovalActions.tsweb/mobile/src/features/chat/useSessionTranscript.tsweb/mobile/src/features/chat/useSessionWatch.tsweb/mobile/src/features/context/ContextResolver.tsxweb/mobile/src/features/context/ProjectList.tsxweb/mobile/src/features/context/ProjectSwitcher.tsxweb/mobile/src/features/context/WorkspaceProjectList.tsxweb/mobile/src/features/context/WorkspaceSelector.tsxweb/mobile/src/features/context/contextTarget.tsweb/mobile/src/features/context/states/SignedOutNotice.tsxweb/mobile/src/features/context/workspaceGroups.tsweb/mobile/src/features/sessions/SessionListScreen.tsxweb/mobile/src/features/sessions/SessionRow.tsxweb/mobile/src/features/sessions/SessionSearchBar.tsxweb/mobile/src/features/sessions/mergeSessionRows.tsweb/mobile/src/features/sessions/pendingFilter.tsweb/mobile/src/features/sessions/states/SessionListStates.tsxweb/mobile/src/features/sessions/useLivenessPoll.tsweb/mobile/src/features/sessions/useSessionListHead.tsweb/mobile/src/features/sessions/useSessionsInfinite.tsweb/mobile/src/lib/auth.tsweb/mobile/src/lib/auth/config.tsweb/mobile/src/lib/auth/discover.tsweb/mobile/src/lib/auth/index.tsweb/mobile/src/lib/auth/otpMachine.tsweb/mobile/src/middleware.tsweb/mobile/src/pages/auth/callback/[[...provider]].tsxweb/mobile/src/pages/auth/index.tsxweb/mobile/src/styles/globals.cssweb/mobile/tests/unit/authConfig.test.tsweb/mobile/tests/unit/authDiscover.test.tsweb/mobile/tests/unit/contextTarget.test.tsweb/mobile/tests/unit/livenessBadge.test.tsweb/mobile/tests/unit/mergeSessionRows.test.tsweb/mobile/tests/unit/middleware.test.tsweb/mobile/tests/unit/otpMachine.test.tsweb/mobile/tests/unit/pendingFilter.test.tsweb/mobile/tests/unit/steer.test.tsweb/mobile/tests/unit/transcriptAdoption.test.tsweb/mobile/tests/unit/workspaceGroups.test.tsweb/packages/agenta-chat/src/assets/loadSession.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/state/sessionMessages.tsweb/packages/agenta-chat/tests/unit/assets/loadSession.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/state/sessionMessages.test.tsweb/packages/agenta-shared/src/utils/mobileGate/index.tsweb/packages/agenta-shared/tests/unit/mobileGate.test.ts
💤 Files with no reviewable changes (2)
- web/mobile/src/features/context/WorkspaceProjectList.tsx
- web/mobile/src/lib/auth.ts
| - `NoMobilePageWrapper` is retired when the gate is actually turned **on**, not when its code | ||
| ships. WP5 landed the gate default-off (`AGENTA_MOBILE_GATE`), so retiring the wrapper any | ||
| earlier would leave a mobile visitor with neither the gate nor the blocker. Retirement (T8) | ||
| belongs to the deployment window that flips the flag, once WP2 and WP4 can serve what the | ||
| redirect points at. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Separate T8 from the WP5 dependency row.
Lines 114-118 correctly require WP2, WP4, and an enabled AGENTA_MOBILE_GATE. Line 384 still includes retire NoMobilePageWrapper in WP5 with only WP1 as a dependency. A reader can apply T8 while the gate is off and remove the only mobile blocker. Update the table to list gate delivery and T8 retirement as separate milestones.
| session_streams). Both ORDER BY and the keyset cursor filters must ride the SAME | ||
| expression — ordering by one column while cursor-filtering on another paginates | ||
| incorrectly. | ||
|
|
||
| That expression is `coalesce(updated_at, created_at)`, not bare `updated_at`: | ||
| `updated_at` is nullable, and a DESC sort puts NULLs first in Postgres, so a session | ||
| that never got a heartbeat would sit above every active one. The full statement | ||
| therefore mentions `created_at` by design — assert on the coalesced expression, not on | ||
| the absence of that column. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the coalesced expression the implementation and test contract.
The new text requires coalesce(updated_at, created_at) for ordering and cursor predicates. The implementation steps still register raw DBE.updated_at, the sample assertions still require bare updated_at and forbid created_at, and the no-windowing fallback still uses raw updated_at. Update all references to use the same coalesced expression.
Also applies to: 149-156
| # Match the `/m` SEGMENT, not every path whose first two characters are `/m`: | ||
| # a bare PathPrefix(`/m`) would also capture /mobile, /metrics and friends and | ||
| # steal them from the web catch-all. The rule still auto-wins over PathPrefix(`/`) | ||
| # by length; no stripprefix — the app is built with basePath /m. | ||
| labels: | ||
| - "traefik.http.routers.web-mobile.rule=PathPrefix(`/m`)" | ||
| - "traefik.http.routers.web-mobile.rule=(Path(`/m`) || PathPrefix(`/m/`))" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching target:"
git ls-files | rg 'docs/design/agenta-mobile/plans/2026-07-25-wp1-infra-tail\.md|2026-07-25-wp1-infra-tail\.md' || true
echo
echo "Targeted matcher occurrences with line numbers:"
rg -n 'PathPrefix\(`/m`\)|Path\(`/m`\)|PathPrefix\(`/m/`\)|web-mobile|traefik\.http\.routers\.web-mobile' docs/design/agenta-mobile/plans/2026-07-25-wp1-infra-tail.md || true
echo
echo "Context around candidate lines:"
sed -n '360,475p' docs/design/agenta-mobile/plans/2026-07-25-wp1-infra-tail.mdRepository: Agenta-AI/agenta
Length of output: 15707
🌐 Web query:
Traefik PathPrefix matcher documentation
💡 Result:
In Traefik, the PathPrefix matcher is used to match incoming HTTP requests based on the URL path [1]. It identifies requests where the path begins with a specified prefix [1]. Key characteristics of PathPrefix include: 1. Functional scope: PathPrefix matches the specified prefix and any sub-paths or variations that start with that string [1]. For example, PathPrefix(/products) will match /products, /products/shoes, /products/, and even /products-for-sale [1]. 2. Non-regexp: PathPrefix does not support regular expressions [2]. If you require regex-based matching for paths, you must use the PathRegexp matcher instead [1][2]. 3. Syntax: Paths used with matchers must always start with a forward slash (/) [1]. Comparison with other path matchers: - Path: Used for an exact match of the path. For example, Path(/products) matches /products but does not match /products/ or /products/shoes [1]. - PathPrefix: Used for a prefix match, as described above [1]. - PathRegexp: Used for complex matching using regular expressions [1]. For further configuration details and usage examples, you can refer to the official Traefik documentation on HTTP routers rules and priority [1].
Citations:
- 1: https://doc.traefik.io/traefik/reference/routing-configuration/http/routing/rules-and-priority/
- 2: PathPrefix RegEx traefik/traefik#10526
🌐 Web query:
Traefik PathRegexp matcher documentation
💡 Result:
The PathRegexp matcher in Traefik allows you to match request paths using regular expressions [1][2]. It is distinct from the Path and PathPrefix matchers, which are used for exact and prefix-based matching, respectively [1]. Key characteristics and usage: - Syntax: PathRegexp uses the Go-flavored regular expression syntax [3][4]. - Flexibility: Unlike Path and PathPrefix, which always start with a slash (/), PathRegexp does not have this restriction [1]. - Dynamic Routing: In Traefik v3, PathRegexp is the recommended way to handle dynamic path segments (e.g., path parameters) that were previously supported via placeholders like {id} in v2 [3]. Common Examples: - Matching specific patterns with IDs: PathRegexp(^/products/(shoes|socks)/[0-9]+$) [1][4]. - Matching file extensions: PathRegexp(\.(jpeg|jpg|png)$) [1][4]. - Case-insensitive prefix matching: PathRegexp((?i)^/products) [1][4]. Migration Note: If you are migrating from Traefik v2 to v3, note that Path and PathPrefix no longer support path parameter placeholders (e.g., /route/{id}). You must update these rules to use PathRegexp instead (e.g., ^/route/[^/]+$) [3].
Citations:
- 1: https://doc.traefik.io/traefik/reference/routing-configuration/http/routing/rules-and-priority/
- 2: https://doc.traefik.io/traefik/master/reference/routing-configuration/http/routing/rules-and-priority/
- 3: https://doc.traefik.io/traefik/migrate/v2-to-v3-details/
- 4: https://doc.traefik.io/traefik/v3.4/routing/routers/
Use segment-safe Traefik mappers in every web-mobile rule.
PathPrefix(/m) also matches /mobile, /metrics, and similar /m* paths. OSS SSL and SS local still use PathPrefix(/m), and EE local says it uses the same labels as EE gh, which also uses PathPrefix(/m). Apply (Path(/m) || PathPrefix(/m/)) to every documented web-mobile router that uses PathPrefix(/m).
| evaluated once at connect; scope holds for the connection's lifetime. That is a real | ||
| window: revoking a user's `VIEW_SESSIONS` does not close streams they already hold, so | ||
| they keep receiving change notifications (never payloads — see the wire contract) until | ||
| they reconnect. **Not bounded as shipped** — there is no maximum stream age today | ||
| (`env.sessions` carries only `watch_heartbeat_seconds` and `watch_retry_milliseconds`). | ||
| Accepted for M3 because the stream carries no payloads, only "something changed" for a session | ||
| the client was already authorized to read, and every revalidation it triggers is a normal | ||
| authorized request that fails on its own once access is gone. The fix when that is not enough: | ||
| close the response after a configured max age and let `EventSource` reconnect, which re-runs | ||
| auth and `check_action_access`. Cheaper than re-checking mid-stream and gives the same upper | ||
| bound on staleness. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Bound the authorization lifetime of an open watch.
The plan accepts an unlimited connection after one VIEW_SESSIONS check. The later wire contract sends session_id and lifecycle or interaction status, so an already-open stream can expose state changes after access is revoked. Set a finite maximum age and reconnect, or reauthorize before each event. Do not describe these frames as payload-free.
| | --- | --- | --- | --- | --- | --- | --- | | ||
| | **S0** | Fix the drifted twin **before** anything else | Re-align `@agenta/chat/state/sessionMessages.ts` to `sessions.ts` byte-parity: pass `tabLocalStorage()` (port the `subscribe`-stripping storage), and either add the record-count watermark store or make the writer refuse to write the shared key without one | nothing changes (no consumer) | new `agenta-chat` unit test: writing through the package leaves `agenta:agent-chat:record-counts` consistent with the OSS reader's expectation | **No** | revert one file | | ||
| | **S1** | Activity key + ordering, one definition | New `session/core/ordering.ts`: `sessionActivityMs(stream)` (= `Date.parse(updated_at ?? created_at)`, NaN→0), `compareByActivityDesc`, `dedupeBySessionId` | `projectSessions.ts:55-70` imports them; `useSessionsInfinite.ts:29` and `SessionRow.tsx:35` import `sessionActivityMs` | entities unit tests over the three current behaviours (NaN, missing `updated_at`, dedupe tie) | yes, ~15 lines in one file | revert; the local copies are 5 lines each | | ||
| | **S2** | Shared list query **options** | `session/state/list.ts`: `sessionListQueryKey(projectId, {references, search, windowing})` + `sessionListQueryOptions(...)` returning `{queryKey, queryFn, staleTime}` — **not** a mounted query | `projectSessionsQueryAtomFamily` wraps it in `atomWithQuery`; `useSessionsInfinite` wraps it in `useInfiniteQuery` | key-shape unit test; mobile 83 unchanged; oss tsc | yes, `projectSessions.ts` only | revert; both call sites keep working with inline options | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg '(^|/)(session/state/list\.ts|session/state/project-sessions\.ts|.*projectSessions.*\.ts|.*useSessionsInfinite.*\.ts|plan\.md)$' || true
echo "== search identifiers =="
rg -n "sessionStateQueryKey|sessionListQueryKey|useSessionsInfinite|includeArchived|includeEnded|windowing|pageParam|queryKey" -S . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== plan lines around 307 =="
if [ -f docs/design/agenta-sessions-consolidation/plan.md ]; then
sed -n '280,330p' docs/design/agenta-sessions-consolidation/plan.md
fi
echo "== candidate file sizes =="
for f in $(git ls-files | rg '(^|/)(session/state/list\.ts|session/state/project-sessions\.ts|.*projectSessions.*\.ts|.*useSessionsInfinite.*\.ts)$'); do
wc -l "$f"
doneRepository: Agenta-AI/agenta
Length of output: 7095
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files list =="
git ls-files | rg '(^|/)session/state/(list|.*sessions).*\.ts$|web/(oss|mobile)/src/features/sessions/(useSessionsInfinite|sessions).ts$|projectSessions\.ts$' || true
echo "== focused symbol search =="
rg -n "sessionListQueryKey|sessionListQueryOptions|useSessionsInfinite|includeArchived|includeEnded|include_archived|include_ended|windowing|pageParam|sessionAt|queryKey" web/oss web/mobile/ --glob '*.ts' --glob '*.tsx' || true
echo "== session state files sizes =="
while IFS= read -r f; do
wc -l "$f"
done < <(git ls-files | rg '(^|/)src/[^/]+/[^/]+/(sessions\.ts|list\.ts)$')Repository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant implementation slices =="
for f in web/mobile/src/features/sessions/useSessionsInfinite.ts web/oss/src/components/AgentChatSlice/state/projectSessions.ts; do
if [ -f "$f" ]; then
echo "--- $f ($(wc -l < "$f") lines) ---"
sed -n '1,220p' "$f"
fi
done
for f in $(git ls-files | rg '(^|/)src/[^/]+/[^/]+/(sessions\.ts|list\.ts)$'); do
echo "--- $f ($(wc -l < "$f") lines) ---"
rg -n "function|const|queryKey|queryKeyFn|includeArch|includeEnd|includeArchived|includeEnded|windowing|pageParam|sessionListQueryKey|sessionListQueryOptions|atomWithQuery|useInfiniteQuery" "$f"
doneRepository: Agenta-AI/agenta
Length of output: 6855
Include the stable session list filters in the shared query key.
The desktop/OSS caller does not currently pass archive or ended filters, but the mobile infinite query uses includeArchived: false on every page. Model the shared stable filter shape: include references, search, includeArchived, and includeEnded; keep cursors out of windowing for the key and pass next/newest only through pageParam. Add key-shape tests covering the desktop/matrix call sites and infinite-scroll pages.
| const query = useQuery({ | ||
| queryKey: ["mobile", "projects"], | ||
| queryFn: () => fetchProjects(), | ||
| staleTime: 30_000, | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline web/mobile/src/features/context/ProjectSwitcher.tsx --items all
ast-grep outline web/mobile/src/features/sessions/useSessionListHead.ts --items all
rg -n -C 3 'atomWithQuery|useQuery' web/mobile/srcRepository: Agenta-AI/agenta
Length of output: 9167
Use query atoms for mobile API queries.
These useQuery calls bypass the mobile data-fetching boundary required by the atomWithQuery guideline.
- Keep
fetchProjects()behind a query atom and consume that atom inProjectSwitcherandContextResolver. - Move
useSessionListHead()’squerySessions()call into a query atom and keep the current input key, enablement, stale time, and refetch interval on that atom.
📍 Affects 2 files
web/mobile/src/features/context/ProjectSwitcher.tsx#L21-L25(this comment)web/mobile/src/features/sessions/useSessionListHead.ts#L20-L34
Source: Coding guidelines
| export function isOidcEnabled(read: EnvReader = getEnv): boolean { | ||
| if (read("NEXT_PUBLIC_AGENTA_AUTH_OIDC_ENABLED").toLowerCase() === "true") return true | ||
| return listOidcProviders(read).length > 0 | ||
| } | ||
|
|
||
| /** | ||
| * Effective email-auth mode. NEXT_PUBLIC_AGENTA_AUTHN_EMAIL wins; unset | ||
| * defaults to "password" only when no OIDC provider is enabled (otherwise the | ||
| * deployment is SSO-only and email must stay hidden). | ||
| */ | ||
| export function getEmailSignInMode(read: EnvReader = getEnv): EmailSignInMode { | ||
| const authnEmail = | ||
| read("NEXT_PUBLIC_AGENTA_AUTHN_EMAIL") || (isOidcEnabled(read) ? "" : "password") | ||
| if (authnEmail === "password" || authnEmail === "otp") return authnEmail | ||
| return "disabled" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Normalize the env values before comparison.
Line 58 lowercases the OIDC flag but does not trim it. Line 69 neither trims nor lowercases NEXT_PUBLIC_AGENTA_AUTHN_EMAIL.
getEnv can return values injected into window.__env at runtime, where trailing whitespace is easy to introduce. A value of "password " or "Password" falls through to "disabled". On a password-only deployment that removes the only sign-in method and SignInScreen renders NoAuthMethods. The failure is silent.
🐛 Proposed fix
export function isOidcEnabled(read: EnvReader = getEnv): boolean {
- if (read("NEXT_PUBLIC_AGENTA_AUTH_OIDC_ENABLED").toLowerCase() === "true") return true
+ if (read("NEXT_PUBLIC_AGENTA_AUTH_OIDC_ENABLED").trim().toLowerCase() === "true") return true
return listOidcProviders(read).length > 0
}
export function getEmailSignInMode(read: EnvReader = getEnv): EmailSignInMode {
const authnEmail =
- read("NEXT_PUBLIC_AGENTA_AUTHN_EMAIL") || (isOidcEnabled(read) ? "" : "password")
+ read("NEXT_PUBLIC_AGENTA_AUTHN_EMAIL").trim().toLowerCase() ||
+ (isOidcEnabled(read) ? "" : "password")
if (authnEmail === "password" || authnEmail === "otp") return authnEmail
return "disabled"
}Add unit cases for whitespace and mixed-case values in web/mobile/tests/unit/authConfig.test.ts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function isOidcEnabled(read: EnvReader = getEnv): boolean { | |
| if (read("NEXT_PUBLIC_AGENTA_AUTH_OIDC_ENABLED").toLowerCase() === "true") return true | |
| return listOidcProviders(read).length > 0 | |
| } | |
| /** | |
| * Effective email-auth mode. NEXT_PUBLIC_AGENTA_AUTHN_EMAIL wins; unset | |
| * defaults to "password" only when no OIDC provider is enabled (otherwise the | |
| * deployment is SSO-only and email must stay hidden). | |
| */ | |
| export function getEmailSignInMode(read: EnvReader = getEnv): EmailSignInMode { | |
| const authnEmail = | |
| read("NEXT_PUBLIC_AGENTA_AUTHN_EMAIL") || (isOidcEnabled(read) ? "" : "password") | |
| if (authnEmail === "password" || authnEmail === "otp") return authnEmail | |
| return "disabled" | |
| } | |
| export function isOidcEnabled(read: EnvReader = getEnv): boolean { | |
| if (read("NEXT_PUBLIC_AGENTA_AUTH_OIDC_ENABLED").trim().toLowerCase() === "true") return true | |
| return listOidcProviders(read).length > 0 | |
| } | |
| /** | |
| * Effective email-auth mode. NEXT_PUBLIC_AGENTA_AUTHN_EMAIL wins; unset | |
| * defaults to "password" only when no OIDC provider is enabled (otherwise the | |
| * deployment is SSO-only and email must stay hidden). | |
| */ | |
| export function getEmailSignInMode(read: EnvReader = getEnv): EmailSignInMode { | |
| const authnEmail = | |
| read("NEXT_PUBLIC_AGENTA_AUTHN_EMAIL").trim().toLowerCase() || | |
| (isOidcEnabled(read) ? "" : "password") | |
| if (authnEmail === "password" || authnEmail === "otp") return authnEmail | |
| return "disabled" | |
| } |
| export function parseDiscoveredSso(payload: unknown): DiscoveredSsoProvider[] { | ||
| const methods = (payload as {methods?: {sso?: {providers?: unknown}}} | null)?.methods | ||
| const providers = methods?.sso?.providers | ||
| if (!Array.isArray(providers)) return [] | ||
| return providers.flatMap((entry) => { | ||
| const record = entry as {id?: unknown; slug?: unknown; third_party_id?: unknown} | ||
| // No third_party_id ⇒ nothing to hand SuperTokens; drop it. | ||
| if (typeof record.id !== "string" || typeof record.third_party_id !== "string") return [] | ||
| const slug = typeof record.slug === "string" ? record.slug : record.third_party_id | ||
| return [ | ||
| { | ||
| id: record.id, | ||
| thirdPartyId: record.third_party_id, | ||
| label: formatSsoLabel(slug, record.third_party_id), | ||
| }, | ||
| ] | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline web/packages/agenta-sdk/src/resources.ts --items all
rg -n -C 3 '/auth/discover|discoverSsoProviders|safeParseWithLogging' webRepository: Agenta-AI/agenta
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '22,75p' web/mobile/src/lib/auth/discover.ts
sed -n '1,80p' web/packages/agenta-entities/src/shared/utils/zodSchema.ts 2>/dev/null || true
sed -n '40,70p' web/packages/agenta-entities/shared/utils/zodSchema.ts 2>/dev/null || true
rg -n -C 3 'postAuthDiscover|discover.*Sso|getAuth|Auth' web/packages/agenta-packages web/packages/agenta-sdk web/mobile/src/web.js web/packages web/packages/agenta-api-client/src/generated 2>/dev/null | head -n 200Repository: Agenta-AI/agenta
Length of output: 23686
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '\.access\.|getAccessClient|AccessClient|postAuthDiscover|auth/discover' web/mobile web/packages/agenta-sdk web/packages/*.json web/tsconfig*.json 2>/dev/null | head -n 120
python3 - <<'PY'
from pathlib import Path
import re
for p in [Path('web/mobile/src/lib/auth/discover.ts'), Path('web/mobile/src/features/auth/SsoDiscoveryForm.tsx')]:
text=p.read_text()
if p.name=='SsoDiscoveryForm.tsx': m=re.search(r'startOidcSignIn\(\s*provider\.thirdPartyId\s*,\s*",?\s*email"[^)]*\)', text, re.S)
else: m=None
print(f'{p}:')
if p.name=='discover.ts':
print('contains_fetch_to_auth_discover=', bool(re.search(r'fetch\(\s*[`"]\\$\{getApiUrl\}\(\)\s*/auth/discover[`"]', text)))
print('contains_raw_cast=', bool(re.search(r'\(payload\s+as\b|\(entry\s+as\b', text)))
print('allows_empty_third_party_id=', bool(re.search(r'typeof\s+record\.third_party_id\s*!==?\s*["\']string["\']', text)))
else:
print('startOidcSignIn_source=', m.group(0) if m else 'NOT FOUND')
PYRepository: Agenta-AI/agenta
Length of output: 970
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '45,65p' web/mobile/src/features/auth/SsoDiscoveryForm.tsx
sed -n '1,200p' web/packages/agenta-entities/src/shared/utils/zodSchema.tsRepository: Agenta-AI/agenta
Length of output: 7407
Use the required API boundary for SSO discovery.
discoverSsoProviders uses raw fetch and unchecked casts. Add a per-resource Fern accessor for /auth/discover to web/packages/agenta-sdk/src/resources.ts, then call that accessor behind a local safeParseWithLogging Zod boundary. Require non-empty id and third_party_id, otherwise an empty thirdPartyId reaches startOidcSignIn.
Source: Coding guidelines
| const FLAG = "NEXT_PUBLIC_AGENT_CHAT_STEER" | ||
|
|
||
| afterEach(() => { | ||
| delete process.env[FLAG] | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how the steer flag is read on mobile and how runtime env is exposed.
fd -g 'steer.ts' web/mobile -x cat -n {}
rg -nP -C3 '__env|NEXT_PUBLIC_AGENT_CHAT_STEER' web/mobile/srcRepository: Agenta-AI/agenta
Length of output: 6824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Entrypoint env generation:"
if [ -f web/entrypoint.sh ]; then
cat -n web/entrypoint.sh | sed -n '1,220p'
else
fd -a 'entrypoint.sh' . -x cat -n {}
fi
echo
echo "Steer tests:"
cat -n web/mobile/tests/unit/steer.test.ts
echo
echo "Config/runtime references:"
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' '__env\.js|NEXT_PUBLIC_AGENT_CHAT_STEER|NEXT_PUBLIC_.*ENV|process\.environment|window\.__env' web/mobile web -g '*.tsx' -g '*.ts' -g '*.js' -g '*.sh' -g 'entrypoint.sh' -g '*_document.tsx' | sed -n '1,240p'Repository: Agenta-AI/agenta
Length of output: 17717
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re, ast
env_file = Path("web/mobile/src/lib/env.ts")
test_file = Path("web/mobile/tests/unit/steer.test.ts")
entry = Path("web/entrypoint.sh")
print("env.ts source:")
print(env_file.read_text())
print("\nsteer.test.ts source:")
print(test_file.read_text())
print("\nentrypoint.sh contains NEXT_PUBLIC_AGENT_CHAT_STEER:")
print(entry.read_text().encode() if entry.exists else b"")
print(re.search(r"NEXT_PUBLIC_AGENT_CHAT_STEER", entry.read_text() if entry.exists else "") is not None)
print("\nMobile imports/use of getEnv in features/chat/steer.ts:")
feature = Path("web/mobile/src/features/chat/steer.ts")
if feature.exists:
text = feature.read_text()
print("imports getEnv:", "import {getEnv}" in text or "getEnv" in text)
print(text)
PYRepository: Agenta-AI/agenta
Length of output: 14685
Add mobile runtime config for NEXT_PUBLIC_AGENT_CHAT_STEER and restore the previous env value.
isSteerEnabled reads window.__env before process.env, but mobile /m/__env.js does not expose this flag. Add it through the same web/entrypoint.sh config path, with an OFF default, so browser behavior matches the test. Also save and restore process.env[FLAG] in afterEach so a pre-existing test-worker value is not removed.
| const adopt = (transcript: SessionTranscript | null) => { | ||
| if (cancelled || !transcript || transcript.messages.length === 0) return | ||
| const serverMsgs = transcript.messages | ||
| const prev = messagesRef.current | ||
| if (busyRef.current || serverMsgs.length <= prev.length) return | ||
| serverMsgs.forEach((m) => restoredIdsRef.current.add(m.id)) | ||
| setMessages(serverMsgs) | ||
| persistMessages({id: sessionId, messages: serverMsgs}) | ||
| persistMessages({ | ||
| id: sessionId, | ||
| messages: serverMsgs, | ||
| recordCount: transcript.recordCount, | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate any reader of the record-count watermark and the shared adoption rule.
rg -nP -C6 'shouldAdoptServerTranscript|shouldAdoptTranscript|sessionRecordCountsReadAtom' webRepository: Agenta-AI/agenta
Length of output: 44922
🏁 Script executed:
<tool_call>shell
#!/bin/bash
set -euo pipefail
# Inspect the desktop hook adoption code and any imports/exported watermark helpers.
fd -a 'useAgentConversation.ts|sessionMessages.ts' web/packages/agenta-chat | sed 's#^\./##'
echo '--- useAgentConversation outline ---'
ast-grep outline web/packages/agenta-chat/src/hooks/useAgentConversation.ts --view expanded | sed -n '1,200p'
echo '--- useAgentConversation lines 260-360 ---'
sed -n '260,360p' web/packages/agenta-chat/src/hooks/useAgentConversation.ts | nl -ba -v260
echo '--- sessionMessages imports/exports count ---'
sed -n '1,90p' web/packages/agenta-chat/src/state/sessionMessages.ts | nl -ba -v1
echo '--- desktop watermark/adoption symbols ---'
rg -n -C3 'recordCount|recordWatermark|shouldAdoptServerTranscript|persistSessionMessagesAtom|sessionRecordCountsReadAtom' web/packages/agenta-chat/src/hooks web/packages/agenta-chat/src/stateRepository: Agenta-AI/agenta
Length of output: 1782
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the desktop hook adoption code and any imports/exported watermark helpers.
fd -a 'useAgentConversation.ts|sessionMessages.ts' web/packages/agenta-chat | sed 's#^\./##'
echo '--- useAgentConversation outline ---'
ast-grep outline web/packages/agenta-chat/src/hooks/useAgentConversation.ts --view expanded | sed -n '1,200p'
echo '--- useAgentConversation lines 260-360 ---'
sed -n '260,360p' web/packages/agenta-chat/src/hooks/useAgentConversation.ts | nl -ba -v260
echo '--- sessionMessages imports/exports count ---'
sed -n '1,90p' web/packages/agenta-chat/src/state/sessionMessages.ts | nl -ba -v1
echo '--- desktop watermark/adoption symbols ---'
rg -n -C3 'recordCount|recordWatermark|shouldAdoptServerTranscript|persistSessionMessagesAtom|sessionRecordCountsReadAtom' web/packages/agenta-chat/src/hooks web/packages/agenta-chat/src/stateRepository: Agenta-AI/agenta
Length of output: 1730
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the desktop hook adoption code and any imports/exported watermark helpers.
fd -a 'useAgentConversation.ts|sessionMessages.ts' web/packages/agenta-chat | sed 's#^\./##'
echo '--- useAgentConversation outline ---'
ast-grep outline web/packages/agenta-chat/src/hooks/useAgentConversation.ts --view expanded | sed -n '1,200p'
echo '--- useAgentConversation lines 260-360 ---'
sed -n '260,360p' web/packages/agenta-chat/src/hooks/useAgentConversation.ts | nl -ba -v260
echo '--- sessionMessages imports/exports count ---'
sed -n '1,90p' web/packages/agenta-chat/src/state/sessionMessages.ts | nl -ba -v1
echo '--- desktop watermark/adoption symbols ---'
rg -n -C3 'recordCount|recordWatermark|shouldAdoptServerTranscript|persistSessionMessagesAtom|sessionRecordCountsReadAtom' web/packages/agenta-chat/src/hooks web/packages/agenta-chat/src/stateRepository: Agenta-AI/agenta
Length of output: 1731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=web/packages/agenta-chat/src/hooks/useAgentConversation.ts
echo '--- relevant counts/adoption lines ---'
grep -n -C8 'serverMsgs.length <= prev.length\|persistMessage' "$file" || true
echo '--- imports around shouldAdoptServerTranscript/sessionRecordCountsReadAtom occurrences ---'
grep -n -C3 'shouldAdoptServerTranscript|sessionRecordCountsReadAtom|recordCount|recordWatermark|persistSessionMessagesAtom' "$file" web/packages/agenta-chat/src/state/sessionMessages.ts || trueRepository: Agenta-AI/agenta
Length of output: 4851
Use the shared record-count watermark when adopting transcripts on desktop.
persistMessages persists recordCount, but both adoption paths decide with serverMsgs.length <= prev.length. A session turn can grow record-wise while leaving the message count unchanged, so that comparison rejects the updated transcript and the stored watermark becomes write-only. Read the persisted watermark and use the shared watermark-aware rule here.
1 participant
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.