Repository navigation
[feat] /m catches up to the desktop: sign-in, chat turns, and Next 16 - #6152
Merged
Merged
Conversation
The hover toolbar on /m rendered its metrics only when the message carried usage, and most turns do not: latency lives on the TRACE, and tokens/cost on the streamed usage. A turn with a trace and no usage therefore showed an empty slot next to Copy and View trace, while the desktop showed 13.94s for the same turn. The desktop's TraceMetrics moves to @agenta/entity-ui as TurnMetrics and both hosts render it, so the two surfaces cannot disagree about where a figure comes from again. It merges the trace summary with the usage (usage wins), and holds the latency slot with a fixed-size placeholder while the trace resolves.
…mponents /m's toolbar was missing the timestamp, and its metrics were incomplete. Prod reads "13h ago 23.41s 21.8K $0.003637"; /m read "8.4s" or nothing at all. Both halves were app-layer on the desktop: the relative stamp was a local MessageTimestamp over a local timeAgo (a byte-identical copy of the one already in @agenta/shared/utils), and the first-seen atoms sat in the OSS chat slice where /m could not reach them. So TurnMetrics and a new TurnTimestamp now live in @agenta/chat/components and both hosts render the same two, the stamp atoms move to @agenta/chat/state, and useAgentConversation stamps live turns the way the desktop hook already did. A turn's time still comes from the run's trace, never from when the browser first saw the message: a reload would otherwise back-date the whole transcript to page-load time. The client stamp is only the fallback for a turn with no trace yet.
… it by accident
/m rendered NOTHING for a client tool. An elicitation ('what is your favourite
colour?') showed only a 'Waiting for your answers' line, with no form to answer, no
Accept/Decline, and a composer that still invited you to type. Prod shows the form,
locks the composer, and queues what you type.
The cause was a default: useAgentConversation's client-tool predicate was optional and
/m never passed one, so every client tool folded into the plain 'used N tools' group
and the run stayed parked with nothing on screen. A host had to opt IN to elicitation.
It no longer can opt out — the predicate defaults to the shared registry's.
The widgets themselves were app-layer, so the move splits them by what they depend on:
the dispatcher and predicate go to @agenta/chat/clientTools (antd-free, so /m can ship
it — its package contract test enforces that), and the widgets go to
@agenta/entity-ui/clientTools, because the elicitation form is SchemaForm whose state
engine is antd Form.
The dispatcher reaches the widgets by VALUE, not by registering them as a module side
effect. The side-effect version looked right and failed silently: both packages declare
sideEffects:false, so the bare import was tree-shaken, the registry stayed empty, and
every elicitation auto-settled as 'not handled by this client'.
Composer lock: /m derived 'waiting on the user' from pending APPROVALS only. hitlPending
already covers both gates and parked client tools, so the hook now returns it and /m
uses it for the composer placeholder and the status line.
Also gives @agenta/entity-ui the JSX transform its own vitest config predicted it would
need — the moved elicitation render test is the .tsx suite it warned about.
…mponent Scrolling back through a transcript on /m left you to scroll all the way down again by hand. Prod floats a 'Jump to latest' pill once you leave the newest turn. The pill was app-layer in oss, so it moves to @agenta/ui as ChatJumpToLatest and both transcripts render it. /m also had no state to drive it: useTranscriptAutoScroll tracked 'am I near the bottom' in a REF, deliberately, so following appends causes no re-render. That fact now also mirrors into state, which is the half a rendered control needs. ScreenScaffold gains a scrollOverlay slot for it. The pill cannot live inside the scroller — an absolutely positioned child of a scrolling box scrolls away with the content — so the slot wraps the scroller in a positioned frame, and only when something is actually passed, leaving every other screen's box exactly as it was.
…vability 'View trace' on a chat turn did nothing. The drawer opens through an ATOM, so whoever renders it decides where it works — and /m rendered it inside the Observability screen alone. The action set the atom on a session page where no drawer existed. web/oss mounts it globally in AppGlobalWrappers for exactly this reason. The router seams move with it: they have to be bound wherever the drawer can open, not only where the traces table lives.
…across Tailwind majors The revealed toolbar on /m was loose text floating over the transcript. Prod puts it in an elevated bordered card so it reads as a control surface and does not sit transparently on top of streamed text. That chrome was a literal class string in the oss message component, so it becomes turnToolbarClass in @agenta/ui beside the reveal and row classes it belongs with, and both hosts use it. The shadow inside it is spelled out instead of `shadow-sm`. This string ships to two apps on DIFFERENT Tailwind majors: v4 renamed the scale, so v4's `shadow-sm` is v3's `shadow`, and /m rendered a visibly heavier drop (0.1 alpha, 3px blur) than the desktop's (0.05, 2px). Measured both; they now match exactly. /m also had the user-side toolbar at right-10, which is the assistant offset mirrored rather than prod's right-2 under the bubble.
A turn on /m offered Copy and View trace but no way to go back and re-run from it. Prod puts a rewind on every turn but the last: on a user turn it truncates the conversation and hands the message back to the composer to edit; on an assistant turn it regenerates. useAgentConversation has exposed `rewind` all along — it SCANS and returns a plan, and deliberately opens no dialogs, leaving the surface to warn and to refill the composer. /m simply never called it. It now does, with the same warning when tools that already ran would be rewound past (rewinding cannot undo them) and the same composer refill. The composer's input handle becomes an optional host-owned ref for that refill; it keeps its own when nothing is passed.
…ts own connection
Every agent run on /m failed with "connection '<slug>' not found for provider
'anthropic'", as if the vault secret were missing. It was not: the connection exists in
the project, and the same agent runs fine on oss/ee.
The run was resolving against the WRONG PROJECT. buildAgentRequest appends project_id to
the invoke URL only alongside an Authorization header, and /m registered no auth-header
provider — so it sent
/invoke?application_id=… (/m, before)
/invoke?application_id=…&project_id=… (prod)
The backend takes a request's project from that query param and, with none, falls back to
the caller's DEFAULT project. The runner then read the default project's vault, found no
connection with the agent's slug, and reported the agent's own connection missing.
web/oss registers the provider in DeferredAppBoot; /m now does the same at the providers
root. The token comes from a new getAccessToken in @agenta/auth rather than a second
SuperTokens call, since /m already runs that recipe.
Verified live: the invoke URL now carries project_id and the agent replies (23.3s, 29.1K,
$0.072717) where it previously errored.
/m grew a bar above the transcript on every turn — a 'running' chip and a Stop — that appeared when the run started and vanished when it ended, shifting the transcript twice per run. It also put a second Stop on screen: the composer's send button is already one. The desktop shows this only for a run THIS browser is not driving (another tab, another device), where the transcript would otherwise look frozen, and docks it above the composer with the approval and interaction strips rather than putting it on top. /m showed it for `running || streamingHere` — its own turns included — and in a header bar. So the condition narrows to `running && !streamingHere` and it moves down with the other strips. The strip itself moves from web/oss into @agenta/chat/components so both surfaces render one component; it gains an optional trailing action, which is how /m keeps the cross-device Stop the desktop's version has no equivalent for. Verified live on /m: streaming a turn now shows no bar, no layout shift, and the composer's send button as the only Stop. The running-elsewhere branch is NOT live-verified — local runs finish inside the liveness poll interval, so the second tab never caught the window.
…tartup narration Two halves of one gap. /m rendered the loading dots as a status line AFTER the whole turn list, so on a tall pane they floated far below the avatar they belonged to, detached from anything. And they were wordless: the runner narrates what it is doing while the environment boots (#6047, 'Starting the agent' / 'Agent ready') so a ~15s cold start reads as progress rather than a stalled session — /m dropped that narration entirely. The desktop renders the started-but-empty turn as the turn itself: avatar plus the label, in the bubble. /m now does the same, under the same gate (assistant turn, streaming, no content yet), and the status line keeps only the PARKED case it still owns. The lifecycle system was app-layer in oss, so it moves to @agenta/chat rather than getting a second implementation: startupPhases (the wire-label parser), turnClock (the per-session label atoms plus useStartupPhase), and TurnActivity (StartupActivity / WorkingDots / WaitingForInput). useAgentConversation now feeds the label from its own onData and clears it on every terminal status, which is what the desktop hook already did and the reason /m had nothing to show. @agenta/chat also gets the JSX transform its vitest config predicted it would need — the moved render test is the .tsx suite that note was about. Verified live on /m: the indicator now reads 'Starting the agent' at the same y as its turn row, beside the avatar, instead of below the list.
A transient 401 stranded a perfectly valid session on the sign-in page until a hard reload. Observed directly: /m sitting on /auth while /api/projects returned 200 with the full project list. Two independent faults made it permanent. The verdict was wrong to begin with. fetchProjects collapsed 'the refresh could not reach the backend' into the same false as 'the session is revoked', so an API that is up but not yet serving — it answers 401, and the refresh that follows fails for the same reason — read exactly like a sign-out. refreshSessionOutcome now reports which of the three things happened, and an unreachable refresh is reported as the transient error it is. And nothing could correct it. The gate ran its query only OFF /auth, and this app sets refetchOnWindowFocus: false, so once bounced there was no re-check trigger at all — staleTime marks data stale, it does not schedule anything. The gate now keeps asking on /auth, re-asks on focus, and routes both ways: a confirmed session sitting on the sign-in page is handed back to the root resolver. The routing is a pure function so the guards that stop it looping — and the one that never forwards out of the OIDC callback mid-exchange — are tested rather than argued about. Not live-verified: reproducing it needs a session that 401s once and then recovers, and the local session has since genuinely expired. Covered by unit tests instead.
The sign-in UI was already shared (@agenta/auth-ui): the desktop's auth components are thin wrappers over the package's forms. The FLOW was not. 760 lines of orchestration lived in the oss page, so /m had no email-first discovery, no org SSO, no returning-visitor promotion and no invite capture — it rendered whatever the env flags enabled and stopped there. Three layers move down. @agenta/auth (React-free, so the rules are unit-tested): signInPolicy derives what the screen offers — deriveEntry (promotion of the remembered provider or email, the heading), deriveMethods and soleSsoRedirect, the rule for redirecting into a lone SSO connection without asking. discovery widens to the whole /auth/discover payload rather than its sso slice, with abort as a first-class outcome, and the narrow discoverSsoProviders it supersedes is deleted. Plus mapAuthError, readInviteParams and readAuthConfig, which mirrors getEffectiveAuthConfig field for field including the AUTH_EMAIL_ENABLED-without-a-mode case getEmailSignInMode collapses. @agenta/auth-ui: useSignInFlow, the entry → methods → code machine both pages now drive — discovery with cancellation, the SSO redirect and its org-slug memo, invite capture, the interrupted code attempt, message auto-clear. Everything app-shaped is a seam, because the OIDC transport genuinely differs: the desktop redirects to its registered callback URI, /m routes through a cookie. AuthShell joins it as the frame (560px method column beside the marketing panel, which hides itself below lg), so /m stops being a second layout — the logo strip is positioned rather than stacked, which also fixes a form that centered on the space left under it instead of on the viewport. deriveMethods deliberately follows the deployment's AUTHN_EMAIL rather than the discovered per-email flags, matching what the page did inline: discovery adds org SSO on top, it does not switch an install between passwords and codes. The page keeps what is genuinely its own — region selector, auth-upgrade org switching, invite email-mismatch, its antd rendering — and drops from 760 lines to 465. /m gains the whole flow and loses SsoDiscoveryForm, since SSO now falls out of discovery instead of hiding behind its own entry point. Two notes. oss never called configureAuth, so the package would have thrown on render reading env and the API host; configureAuthPackage is the side-effect module the page imports (ee re-exports that page, so both are covered). And the hook reads config on mount, not during render: /m resolves env at runtime, so deriving from it in the first pass fails hydration. Verified on /m against the local stack: entry → discover (200) → password step with the address locked → back to entry, and a real sign-in. oss/ee is type- and lint-clean but unrun here — it needs a SuperTokens stack.
dosubot
Bot
added
the
size:XL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request centralizes authentication, adds shared chat and client-tool behavior, updates mobile application wiring, and changes shared layout components. It also updates build configuration, package exports, and runtime compatibility. ChangesAuthentication flow centralization
Shared chat and client-tool platform
Application platform and layout updates
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟡 Moderate · up to This PR unifies sign-in and mobile chat behavior, but the current head still has bounded correctness and integration risks: replayed client-tool failures may lose their persisted error, unsupported startup phases may produce invalid labels, submitted turns may lack progress feedback, and a package dependency cycle could cause build or runtime instability. Merge should wait for fixes or explicit owner acceptance. Sequence Diagram(s)sequenceDiagram
participant AuthPage
participant useSignInFlow
participant discoverAuthMethods
participant signInPolicy
AuthPage->>useSignInFlow: initialize sign-in state
useSignInFlow->>discoverAuthMethods: discover methods for email
discoverAuthMethods-->>useSignInFlow: return discovered methods
useSignInFlow->>signInPolicy: derive method presentation
signInPolicy-->>AuthPage: return sign-in stage and actions
sequenceDiagram
participant LiveConversation
participant useAgentConversation
participant TurnRow
participant ClientToolWidget
LiveConversation->>useAgentConversation: read conversation state
useAgentConversation-->>LiveConversation: return turns and hitlPending
LiveConversation->>TurnRow: render turn with session and handlers
TurnRow->>ClientToolWidget: resolve and render client-tool part
ClientToolWidget-->>TurnRow: return tool interaction state
🚥 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 |
Next 16 makes Turbopack the default bundler for `next build`, which hard-fails when a webpack config is present, and it removes or renames several config surfaces. Mobile is Pages Router, so the headline breaking change — async `params`/`searchParams`/`cookies()`/`headers()` — does not apply, and neither do parallel-route defaults, PPR, or the async image/sitemap props. That leaves a small, mechanical set of changes: - `eslint.ignoreDuringBuilds` is a removed config option; `next build` no longer lints, and the type gate already runs as its own turbo task. - `turbopack.root` was gated to development. Turbopack now drives the build too, so the workspace root has to apply unconditionally or the monorepo misresolves. - The `webpack.IgnorePlugin` that silenced the AI SDK's optional zod-alternative peers becomes `turbopack.resolveAlias` onto an empty module. The SDK guards those adapters with try/catch, so both spellings land in the same place. - `middleware.ts` becomes `proxy.ts` and the export is renamed. The gate is pure header/cookie/URL logic, so it ports unchanged to proxy's Node-only runtime. - `jsx: preserve` becomes `react-jsx`, and `next-env.d.ts` regenerates. Only `@agenta/playground-ui`'s `next: <16` peer actually objected; the pnpm `next@<15.5.18` overrides never capped anything, since a `^16` request does not match that selector. Both majors coexist cleanly: oss, ee, storybook and every shared package stay on 15.5.x with no peer warnings, and both desktop builds pass. Verified: mobile builds on Turbopack with no warnings and keeps its standalone output layout, tsc and the unit suite are green, and the oss and ee builds are unaffected. `next dev` and the proxy's runtime behaviour against a live request are not yet exercised.
…op of it On a phone viewport the sessions panel showed 'WAITING ON YOU' drawn over 'Sessions · 3 waiting', with a row's text visible through both. Two faults, one surface. PanelSection's header and SessionCardList's group heading were both `sticky top-0 z-10` in the SAME scroller, so they pinned to the same line and overlapped. And the heading's `bg-inherit` had nothing opaque to inherit — PanelSection's body set no surface — so it resolved transparent and rows scrolled visibly through it. The heading now pins at `--ag-panel-header-h`, which PanelSection measures from its own header and publishes. Measured rather than a constant: the variants are different heights (40px page, 48px rail, both measured live) and the header wraps on a narrow viewport, so any hardcoded offset is wrong somewhere. The 0px fallback keeps hosts WITHOUT a header of their own — the session-history popover, the full sessions list — pinning at the top exactly as before, so no caller has to opt in. The body now states its surface, which is what makes `bg-inherit` opaque. Verified live at 430x932 that the header pins and rows scroll under it on an opaque surface. NOT verified: the corrected offset itself — the dev server went down before I could see the heading pin at 40px, and an earlier attempt of mine was wrong here (it removed the stickiness instead of offsetting it), so treat this as needing a look.
ardaerzin
changed the title
mmabrouk
changed the base branch from
lane/mobile-extracted-packages
to
release/v0.113.0
August 21, 2026 13:50
The @agenta/oss build OOMs at the 4096 MB cap on this branch's package graph (both archs, two runs). Same change lands on release/v0.113.0 via #6162; identical lines merge clean.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (6)
web/packages/agenta-auth-ui/src/AuthShell.tsx (1)
43-45: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueEscape
displayFontUrlbefore interpolating it into CSS.The value is inserted raw inside
url("..."). A value that contains"or)terminates the declaration and injects arbitrary CSS into the auth page. The source is a deploy-time env var today, so the risk is low, but the escape is cheap.🛡️ Proposed hardening
- {displayFontUrl && ( - <style>{`@font-face{font-family:"Agenta Display";src:url("${displayFontUrl}");font-weight:300;font-display:swap;}`}</style> - )} + {displayFontUrl && ( + <style>{`@font-face{font-family:"Agenta Display";src:url("${encodeURI(displayFontUrl).replace(/["\\)]/g, "")}");font-weight:300;font-display:swap;}`}</style> + )}web/packages/agenta-auth-ui/src/useSignInFlow.ts (1)
150-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHoist the query values out of the dependency array.
The dependency array calls
firstQueryValue(...)inline. Thereact-hooks/exhaustive-depsrule rejects complex expressions in dependency arrays, and the array also omitsqueryandonInvite. Compute the values above the effect.♻️ Proposed refactor
+ const inviteToken = firstQueryValue(query.token) + const inviteOrganizationId = firstQueryValue(query.organization_id) + // Stash the invite before any redirect: the provider round trip drops the query string. useEffect(() => { const invite = readInviteParams(query) if (invite) onInvite?.(invite) // The invite is a property of the URL, so the query is the only dependency that matters. - }, [emailFromQuery, firstQueryValue(query.token), firstQueryValue(query.organization_id)]) + // eslint-disable-next-line react-hooks/exhaustive-deps -- the URL is the only input that matters + }, [emailFromQuery, inviteToken, inviteOrganizationId])web/mobile/src/features/app/AuthGate.tsx (1)
47-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResolve the
react-hooks/exhaustive-depswarning.The lint job reports a missing
routerdependency. Keep the array intentional and silence the rule, or capture the router in a ref. Frontend changes must passpnpm lint-fixbefore commit.♻️ Proposed change
useEffect(() => { if (target) void router.replace(target) + // eslint-disable-next-line react-hooks/exhaustive-deps -- the target string is the only trigger }, [target])As per coding guidelines: "Before committing frontend changes, run
pnpm lint-fixfrom thewebdirectory."Sources: Coding guidelines, Linters/SAST tools
web/mobile/src/features/auth/SignInScreen.tsx (1)
37-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse one OIDC transport for both entry paths.
startThirdPartyandstartProvidercallstartOidcSignInwith the same semantics, but they track loading and report failures in two different places. The hook already ownsredirectingandreportError. Two copies will drift when the transport changes.Route the entry-screen buttons through the flow, and keep
startOidcSignInin one callback.♻️ Proposed refactor
- const startProvider = async (providerId: string) => { - if (oidcLoading) return - setOidcLoading(true) - await startOidcSignIn(providerId) - setOidcLoading(false) - setMessage({message: "Could not reach that provider. Try again.", type: "error"}) - } + const startProvider = (providerId: string) => + flow.startSso({thirdPartyId: providerId, id: providerId, label: providerId})Adjust the argument to whatever
DiscoveredSsoProviderrequires, and drop the localoidcLoadingstate in favour offlow.redirecting.web/oss/src/pages/auth/[[...path]].tsx (1)
156-161: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMemoize the decorated provider lists.
otherProvidersbuilds a new array of new objects on every render and is passed as theprovidersprop toSocialAuth. The file already memoizesorgSelectOptionsfor the same reason.♻️ Proposed refactor
- const promotedProvider = entry.promotedProvider - ? withProviderIcon(entry.promotedProvider) - : undefined - const otherProviders = entry.otherProviders.map(withProviderIcon) + const promotedProvider = useMemo( + () => (entry.promotedProvider ? withProviderIcon(entry.promotedProvider) : undefined), + [entry.promotedProvider], + ) + const otherProviders = useMemo( + () => entry.otherProviders.map(withProviderIcon), + [entry.otherProviders], + )As per coding guidelines: "Memoize inline arrays containing objects or JSX when passing them as props to avoid unnecessary rerenders."
Source: Coding guidelines
web/packages/agenta-chat/src/assets/startupPhases.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten this comment.
This comment restates
STARTUP_LABELS. Use 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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 118b616c-3123-40bf-8419-982afb80b545
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (95)
web/ee/docker/Dockerfile.ghweb/mobile/docker/Dockerfile.ghweb/mobile/next-env.d.tsweb/mobile/next.config.tsweb/mobile/package.jsonweb/mobile/src/components/ScreenScaffold.tsxweb/mobile/src/features/app/AppProviders.tsxweb/mobile/src/features/app/AuthGate.tsxweb/mobile/src/features/app/ExecutionHeaders.tsxweb/mobile/src/features/app/GlobalDrawers.tsxweb/mobile/src/features/app/authRoute.tsweb/mobile/src/features/auth/SignInScreen.tsxweb/mobile/src/features/auth/SsoDiscoveryForm.tsxweb/mobile/src/features/chat/ChatScreen.tsxweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/mobile/src/features/chat/TurnRow.tsxweb/mobile/src/features/chat/useTranscriptAutoScroll.tsweb/mobile/src/features/observability/ObservabilityScreen.tsxweb/mobile/src/lib/context.tsweb/mobile/src/lib/emptyModule.tsweb/mobile/src/pages/_app.tsxweb/mobile/src/proxy.tsweb/mobile/tests/unit/authRoute.test.tsweb/mobile/tests/unit/proxy.test.tsweb/mobile/tsconfig.jsonweb/oss/docker/Dockerfile.ghweb/oss/src/components/AgentChatSlice/assets/transcriptReplayShapeContract.test.tsweb/oss/src/components/AgentChatSlice/components/AgentComposerDock.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/AgentChatSlice/components/AgentTranscript.tsxweb/oss/src/components/AgentChatSlice/components/AgentTurn.tsxweb/oss/src/components/AgentChatSlice/components/InteractionDock.tsxweb/oss/src/components/AgentChatSlice/components/SessionHistoryMenu.tsxweb/oss/src/components/AgentChatSlice/components/SessionRail.tsxweb/oss/src/components/AgentChatSlice/components/TranscriptPlaceholder.tsxweb/oss/src/components/AgentChatSlice/components/clientTools/index.tsweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/hooks/useStartupPhase.tsweb/oss/src/components/AgentChatSlice/state/sessions.tsweb/oss/src/components/pages/agent-home/components/YourAgentsTable/AgentActivityCell.tsxweb/oss/src/lib/auth/configureAuthPackage.tsweb/oss/src/pages/auth/[[...path]].tsxweb/packages/agenta-auth-ui/package.jsonweb/packages/agenta-auth-ui/src/AuthShell.tsxweb/packages/agenta-auth-ui/src/AuthSideBanner.tsxweb/packages/agenta-auth-ui/src/index.tsweb/packages/agenta-auth-ui/src/useSignInFlow.tsweb/packages/agenta-auth/src/authError.tsweb/packages/agenta-auth/src/client.tsweb/packages/agenta-auth/src/config.tsweb/packages/agenta-auth/src/discovery.tsweb/packages/agenta-auth/src/index.tsweb/packages/agenta-auth/src/invite.tsweb/packages/agenta-auth/src/signInPolicy.tsweb/packages/agenta-auth/tests/unit/signInPolicy.test.tsweb/packages/agenta-chat/package.jsonweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/startupPhases.tsweb/packages/agenta-chat/src/clientTools/ClientToolPart.tsxweb/packages/agenta-chat/src/clientTools/UnhandledClientTool.tsxweb/packages/agenta-chat/src/clientTools/index.tsweb/packages/agenta-chat/src/clientTools/meta.tsweb/packages/agenta-chat/src/components/RunningElsewhereStrip.tsxweb/packages/agenta-chat/src/components/TurnActivity.tsxweb/packages/agenta-chat/src/components/TurnMetrics.tsxweb/packages/agenta-chat/src/components/TurnTimestamp.tsxweb/packages/agenta-chat/src/components/index.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/skin/registry.tsweb/packages/agenta-chat/src/state/index.tsweb/packages/agenta-chat/src/state/messageStamps.tsweb/packages/agenta-chat/src/state/turnClock.tsweb/packages/agenta-chat/tests/unit/TurnActivity.test.tsxweb/packages/agenta-chat/tests/unit/assets/startupPhases.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/meta.test.tsweb/packages/agenta-chat/tests/unit/state/turnClock.test.tsweb/packages/agenta-chat/vitest.config.tsweb/packages/agenta-entity-ui/package.jsonweb/packages/agenta-entity-ui/src/clientTools/ConnectToolWidget.tsxweb/packages/agenta-entity-ui/src/clientTools/ElicitationWidget.tsxweb/packages/agenta-entity-ui/src/clientTools/index.tsweb/packages/agenta-entity-ui/src/clientTools/registry.tsxweb/packages/agenta-entity-ui/src/clientTools/useConnectFlow.tsweb/packages/agenta-entity-ui/src/secretProvider/ProviderDrawer.tsxweb/packages/agenta-entity-ui/tests/unit/ElicitationWidget.settleOnce.test.tsxweb/packages/agenta-entity-ui/tests/unit/useConnectFlow.test.tsweb/packages/agenta-entity-ui/vitest.config.tsweb/packages/agenta-playground-ui/package.jsonweb/packages/agenta-sessions-ui/src/SessionCardList.tsxweb/packages/agenta-ui/src/components/presentational/chat/index.tsxweb/packages/agenta-ui/src/components/presentational/index.tsweb/packages/agenta-ui/src/components/presentational/layout/PanelSection.tsxweb/packages/agenta-ui/tests/unit/EnhancedDrawerAntdProps.render.test.tsx
💤 Files with no reviewable changes (6)
- web/oss/src/components/AgentChatSlice/components/clientTools/index.ts
- web/oss/src/components/AgentChatSlice/state/sessions.ts
- web/oss/src/components/AgentChatSlice/hooks/useStartupPhase.ts
- web/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.ts
- web/mobile/src/features/auth/SsoDiscoveryForm.tsx
- web/mobile/src/features/observability/ObservabilityScreen.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review.
Comment on lines
+6
to
+10
| import {clientToolWidgets} from "@agenta/entity-ui/clientTools" | ||
| import {renderKindFor, type RenderHintLike} from "@agenta/playground" | ||
| import type {ToolUIPart} from "ai" | ||
|
|
||
| import {hasClientToolHandler} from "./registry" | ||
| import {hasClientToolWidget, type ClientToolMeta} from "../skin" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
The chat layer statically imports the entity UI registry, while that registry imports chat skin code, creating a circular workspace dependency. Keep the chat layer generic and inject the built-in client-tool registry from the host or a lower-level shared package.
📍 Affects 2 files
web/packages/agenta-chat/src/clientTools/meta.ts#L6-L10(this comment)web/packages/agenta-chat/src/clientTools/ClientToolPart.tsx#L11-L15
Member
There was a problem hiding this comment.
Already fixed in ac2016f, which landed before this review. @agenta/entity-ui no longer depends on @agenta/chat (or @agenta/settings-ui), so the cycle is gone. web/packages/agenta-shared/tests/unit/workspaceGraph.test.ts is a guard test that fails if any cycle is reintroduced, so no code change here.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/packages/agenta-chat/src/assets/startupPhases.ts (1)
15-16: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject inherited phase keys.
Line 15 accepts inherited keys such as
"toString"and"constructor". Those values return a function or object instead ofnull, despite the parser contract.Proposed fix
- return typeof phase === "string" && phase in STARTUP_LABELS + return ( + typeof phase === "string" && + Object.prototype.hasOwnProperty.call(STARTUP_LABELS, phase) + ) ? STARTUP_LABELS[phase as keyof typeof STARTUP_LABELS] : nullAdd cases for inherited keys to
startupPhases.test.ts.web/packages/agenta-entity-ui/src/clientTools/ConnectToolWidget.tsx (1)
88-92: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve replayed tool error text.
A remounted non-deferred
output-errorhas no localoutcomeand usually has nometa.output. Line 46 already reads the persistedpartErrorText, but Lines 88-92 ignore it. The widget then replaces the original failure with “Connection not completed”.Proposed fix
- const reason = outcome?.reason ?? output.reason + const reason = + outcome?.reason ?? + output.reason ?? + (typeof partErrorText === "string" && + !partErrorText.startsWith(DEFERRED_SENTINEL) + ? partErrorText + : undefined)
🧹 Nitpick comments (6)
web/packages/agenta-auth-ui/src/AuthShell.tsx (1)
43-45: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueEscape
displayFontUrlbefore interpolating it into CSS.The value is inserted raw inside
url("..."). A value that contains"or)terminates the declaration and injects arbitrary CSS into the auth page. The source is a deploy-time env var today, so the risk is low, but the escape is cheap.🛡️ Proposed hardening
- {displayFontUrl && ( - <style>{`@font-face{font-family:"Agenta Display";src:url("${displayFontUrl}");font-weight:300;font-display:swap;}`}</style> - )} + {displayFontUrl && ( + <style>{`@font-face{font-family:"Agenta Display";src:url("${encodeURI(displayFontUrl).replace(/["\\)]/g, "")}");font-weight:300;font-display:swap;}`}</style> + )}web/packages/agenta-auth-ui/src/useSignInFlow.ts (1)
150-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHoist the query values out of the dependency array.
The dependency array calls
firstQueryValue(...)inline. Thereact-hooks/exhaustive-depsrule rejects complex expressions in dependency arrays, and the array also omitsqueryandonInvite. Compute the values above the effect.♻️ Proposed refactor
+ const inviteToken = firstQueryValue(query.token) + const inviteOrganizationId = firstQueryValue(query.organization_id) + // Stash the invite before any redirect: the provider round trip drops the query string. useEffect(() => { const invite = readInviteParams(query) if (invite) onInvite?.(invite) // The invite is a property of the URL, so the query is the only dependency that matters. - }, [emailFromQuery, firstQueryValue(query.token), firstQueryValue(query.organization_id)]) + // eslint-disable-next-line react-hooks/exhaustive-deps -- the URL is the only input that matters + }, [emailFromQuery, inviteToken, inviteOrganizationId])web/mobile/src/features/app/AuthGate.tsx (1)
47-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResolve the
react-hooks/exhaustive-depswarning.The lint job reports a missing
routerdependency. Keep the array intentional and silence the rule, or capture the router in a ref. Frontend changes must passpnpm lint-fixbefore commit.♻️ Proposed change
useEffect(() => { if (target) void router.replace(target) + // eslint-disable-next-line react-hooks/exhaustive-deps -- the target string is the only trigger }, [target])As per coding guidelines: "Before committing frontend changes, run
pnpm lint-fixfrom thewebdirectory."Sources: Coding guidelines, Linters/SAST tools
web/mobile/src/features/auth/SignInScreen.tsx (1)
37-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse one OIDC transport for both entry paths.
startThirdPartyandstartProvidercallstartOidcSignInwith the same semantics, but they track loading and report failures in two different places. The hook already ownsredirectingandreportError. Two copies will drift when the transport changes.Route the entry-screen buttons through the flow, and keep
startOidcSignInin one callback.♻️ Proposed refactor
- const startProvider = async (providerId: string) => { - if (oidcLoading) return - setOidcLoading(true) - await startOidcSignIn(providerId) - setOidcLoading(false) - setMessage({message: "Could not reach that provider. Try again.", type: "error"}) - } + const startProvider = (providerId: string) => + flow.startSso({thirdPartyId: providerId, id: providerId, label: providerId})Adjust the argument to whatever
DiscoveredSsoProviderrequires, and drop the localoidcLoadingstate in favour offlow.redirecting.web/oss/src/pages/auth/[[...path]].tsx (1)
156-161: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMemoize the decorated provider lists.
otherProvidersbuilds a new array of new objects on every render and is passed as theprovidersprop toSocialAuth. The file already memoizesorgSelectOptionsfor the same reason.♻️ Proposed refactor
- const promotedProvider = entry.promotedProvider - ? withProviderIcon(entry.promotedProvider) - : undefined - const otherProviders = entry.otherProviders.map(withProviderIcon) + const promotedProvider = useMemo( + () => (entry.promotedProvider ? withProviderIcon(entry.promotedProvider) : undefined), + [entry.promotedProvider], + ) + const otherProviders = useMemo( + () => entry.otherProviders.map(withProviderIcon), + [entry.otherProviders], + )As per coding guidelines: "Memoize inline arrays containing objects or JSX when passing them as props to avoid unnecessary rerenders."
Source: Coding guidelines
web/packages/agenta-chat/src/assets/startupPhases.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten this comment.
This comment restates
STARTUP_LABELS. Use 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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 118b616c-3123-40bf-8419-982afb80b545
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (95)
web/ee/docker/Dockerfile.ghweb/mobile/docker/Dockerfile.ghweb/mobile/next-env.d.tsweb/mobile/next.config.tsweb/mobile/package.jsonweb/mobile/src/components/ScreenScaffold.tsxweb/mobile/src/features/app/AppProviders.tsxweb/mobile/src/features/app/AuthGate.tsxweb/mobile/src/features/app/ExecutionHeaders.tsxweb/mobile/src/features/app/GlobalDrawers.tsxweb/mobile/src/features/app/authRoute.tsweb/mobile/src/features/auth/SignInScreen.tsxweb/mobile/src/features/auth/SsoDiscoveryForm.tsxweb/mobile/src/features/chat/ChatScreen.tsxweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/mobile/src/features/chat/TurnRow.tsxweb/mobile/src/features/chat/useTranscriptAutoScroll.tsweb/mobile/src/features/observability/ObservabilityScreen.tsxweb/mobile/src/lib/context.tsweb/mobile/src/lib/emptyModule.tsweb/mobile/src/pages/_app.tsxweb/mobile/src/proxy.tsweb/mobile/tests/unit/authRoute.test.tsweb/mobile/tests/unit/proxy.test.tsweb/mobile/tsconfig.jsonweb/oss/docker/Dockerfile.ghweb/oss/src/components/AgentChatSlice/assets/transcriptReplayShapeContract.test.tsweb/oss/src/components/AgentChatSlice/components/AgentComposerDock.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/AgentChatSlice/components/AgentTranscript.tsxweb/oss/src/components/AgentChatSlice/components/AgentTurn.tsxweb/oss/src/components/AgentChatSlice/components/InteractionDock.tsxweb/oss/src/components/AgentChatSlice/components/SessionHistoryMenu.tsxweb/oss/src/components/AgentChatSlice/components/SessionRail.tsxweb/oss/src/components/AgentChatSlice/components/TranscriptPlaceholder.tsxweb/oss/src/components/AgentChatSlice/components/clientTools/index.tsweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/hooks/useStartupPhase.tsweb/oss/src/components/AgentChatSlice/state/sessions.tsweb/oss/src/components/pages/agent-home/components/YourAgentsTable/AgentActivityCell.tsxweb/oss/src/lib/auth/configureAuthPackage.tsweb/oss/src/pages/auth/[[...path]].tsxweb/packages/agenta-auth-ui/package.jsonweb/packages/agenta-auth-ui/src/AuthShell.tsxweb/packages/agenta-auth-ui/src/AuthSideBanner.tsxweb/packages/agenta-auth-ui/src/index.tsweb/packages/agenta-auth-ui/src/useSignInFlow.tsweb/packages/agenta-auth/src/authError.tsweb/packages/agenta-auth/src/client.tsweb/packages/agenta-auth/src/config.tsweb/packages/agenta-auth/src/discovery.tsweb/packages/agenta-auth/src/index.tsweb/packages/agenta-auth/src/invite.tsweb/packages/agenta-auth/src/signInPolicy.tsweb/packages/agenta-auth/tests/unit/signInPolicy.test.tsweb/packages/agenta-chat/package.jsonweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/startupPhases.tsweb/packages/agenta-chat/src/clientTools/ClientToolPart.tsxweb/packages/agenta-chat/src/clientTools/UnhandledClientTool.tsxweb/packages/agenta-chat/src/clientTools/index.tsweb/packages/agenta-chat/src/clientTools/meta.tsweb/packages/agenta-chat/src/components/RunningElsewhereStrip.tsxweb/packages/agenta-chat/src/components/TurnActivity.tsxweb/packages/agenta-chat/src/components/TurnMetrics.tsxweb/packages/agenta-chat/src/components/TurnTimestamp.tsxweb/packages/agenta-chat/src/components/index.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/skin/registry.tsweb/packages/agenta-chat/src/state/index.tsweb/packages/agenta-chat/src/state/messageStamps.tsweb/packages/agenta-chat/src/state/turnClock.tsweb/packages/agenta-chat/tests/unit/TurnActivity.test.tsxweb/packages/agenta-chat/tests/unit/assets/startupPhases.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/meta.test.tsweb/packages/agenta-chat/tests/unit/state/turnClock.test.tsweb/packages/agenta-chat/vitest.config.tsweb/packages/agenta-entity-ui/package.jsonweb/packages/agenta-entity-ui/src/clientTools/ConnectToolWidget.tsxweb/packages/agenta-entity-ui/src/clientTools/ElicitationWidget.tsxweb/packages/agenta-entity-ui/src/clientTools/index.tsweb/packages/agenta-entity-ui/src/clientTools/registry.tsxweb/packages/agenta-entity-ui/src/clientTools/useConnectFlow.tsweb/packages/agenta-entity-ui/src/secretProvider/ProviderDrawer.tsxweb/packages/agenta-entity-ui/tests/unit/ElicitationWidget.settleOnce.test.tsxweb/packages/agenta-entity-ui/tests/unit/useConnectFlow.test.tsweb/packages/agenta-entity-ui/vitest.config.tsweb/packages/agenta-playground-ui/package.jsonweb/packages/agenta-sessions-ui/src/SessionCardList.tsxweb/packages/agenta-ui/src/components/presentational/chat/index.tsxweb/packages/agenta-ui/src/components/presentational/index.tsweb/packages/agenta-ui/src/components/presentational/layout/PanelSection.tsxweb/packages/agenta-ui/tests/unit/EnhancedDrawerAntdProps.render.test.tsx
💤 Files with no reviewable changes (6)
- web/oss/src/components/AgentChatSlice/components/clientTools/index.ts
- web/oss/src/components/AgentChatSlice/state/sessions.ts
- web/oss/src/components/AgentChatSlice/hooks/useStartupPhase.ts
- web/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.ts
- web/mobile/src/features/auth/SsoDiscoveryForm.tsx
- web/mobile/src/features/observability/ObservabilityScreen.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review.
… build `next build` of @agenta/oss died in "Creating an optimized production build" with `uncaughtException RangeError: Invalid array length at Array.push`, and with a 4096 MB cap it died as a heap OOM instead. No stack, no module name, both CI arches, and reproducible locally. A bisect named 805792a (client tool elicitation on /m). That commit moved the client-tool widgets into @agenta/entity-ui, and the widgets import @agenta/chat (skin types and resolvers) and @agenta/settings-ui (`useToolsConnections`). Both packages already depend on @agenta/entity-ui, so the workspace graph gained its first two dependency cycles. The loop: pnpm links a workspace dependency as a symlink under `<pkg>/node_modules/@agenta/<dep>`, so a cycle makes a directory tree of unbounded depth. Webpack's `FileSystemInfo` hashes a context directory by walking it recursively and resolves each symlink to its absolute target before it recurses, so the walk ping-pongs between the two package directories and never ends. A wrapper around `Array.prototype.push` caught the throw at the call site: [pin-stack] push failed; this.length = 134217728 args.length = 1 at bundle5.js:29:303117 at FileSystemInfo._getUnresolvedContextTsh (bundle5.js:29:274683) at processQueue (bundle5.js:29:1448574) The fix removes both cycles and keeps the behaviour the culprit commit added: - The client-tool contract (`ClientToolMeta`, `SettleClientTool`, `ClientToolWidgetProps`, `ClientToolWidget`) moves DOWN to @agenta/shared/clientTools, beside `CLIENT_TOOL_DESCRIPTORS`. @agenta/chat/skin re-exports every one, so hosts and skins keep their import site. - `useToolsConnections` moves DOWN to @agenta/entities/gatewayTool, beside the connection API calls it wraps. @agenta/settings-ui re-exports it, so its public API is unchanged. - @agenta/entity-ui's clientTools registry drops the `registerChatSkin` side-effect call and the two resolvers it re-exported (nothing imported them). The dispatcher still reaches the widgets by VALUE, and its lookup still checks the registered store before that fallback, so a host override keeps winning. No host registers client-tool widgets today. - The elicitation "Asked by ..." label is now resolved by the dispatcher and passed as `askerLabel`, because the tool-display store lives in @agenta/chat and the widget must not import it back. - @agenta/entity-ui drops its @agenta/chat and @agenta/settings-ui dependencies. @agenta/shared gains `ai` (types only, optional peer). Adds `agenta-shared/tests/unit/workspaceGraph.test.ts`: the @agenta/* graph must stay acyclic. Nothing else catches this class of bug. TypeScript, ESLint, and every unit test pass with the cycle in place, the dev server runs fine, and only the production build dies, without saying why. Verified: `next build` of @agenta/oss and of @agenta/mobile both complete; tsc --noEmit clean for oss, ee and mobile; unit tests green for shared (362), chat (364), entity-ui (473) and entities (1330). Reintroducing the entity-ui -> chat dependency makes the new test fail with "Workspace dependency cycle: @agenta/entity-ui -> @agenta/chat -> @agenta/entity-ui".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d30afaa0-bb3b-494c-a2e7-becbd8106f25
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
web/packages/agenta-chat/src/clientTools/ClientToolPart.tsxweb/packages/agenta-chat/src/skin/types.tsweb/packages/agenta-chat/tests/unit/meta.test.tsweb/packages/agenta-entities/src/gatewayTool/hooks/index.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useToolsConnections.tsweb/packages/agenta-entities/src/gatewayTool/index.tsweb/packages/agenta-entity-ui/package.jsonweb/packages/agenta-entity-ui/src/clientTools/ConnectToolWidget.tsxweb/packages/agenta-entity-ui/src/clientTools/ElicitationWidget.tsxweb/packages/agenta-entity-ui/src/clientTools/registry.tsxweb/packages/agenta-entity-ui/src/clientTools/useConnectFlow.tsweb/packages/agenta-entity-ui/tests/unit/ElicitationWidget.settleOnce.test.tsxweb/packages/agenta-settings-ui/src/index.tsweb/packages/agenta-settings-ui/src/tools/ConnectModal.tsxweb/packages/agenta-settings-ui/src/tools/ConnectionsList.tsxweb/packages/agenta-shared/package.jsonweb/packages/agenta-shared/src/clientTools/index.tsweb/packages/agenta-shared/tests/unit/workspaceGraph.test.ts
💤 Files with no reviewable changes (1)
- web/packages/agenta-entity-ui/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- web/packages/agenta-entity-ui/tests/unit/ElicitationWidget.settleOnce.test.tsx
- web/packages/agenta-entity-ui/src/clientTools/useConnectFlow.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.
Contributor
Railway Preview Environment
|
…email (CodeRabbit) The auto-continue effect ran on mount while `config` was still null, so `resolved` was NO_METHODS_YET and an invite or prefilled-email link reported "No authentication methods are configured" and never retried. Gate the effect on the config and list it as a dependency. Also clear `discovering` when a probe aborts with no successor: `isAbort` matches abort-shaped errors the flow did not raise itself, and those left the entry button loading forever. Hoist the two invite query values out of the dependency array.
… URL (CodeRabbit)
`lastSsoOrgSlug` already ships unprefixed, written by AuthUpgradeModal and read by
usePostAuthRedirect, so renaming the new shared constant alone would have desynced writer from
reader. Move both existing sites onto the shared helpers instead: the key becomes
`agenta:lastSsoOrgSlug`, the read falls back to the old key and the clear removes both, so a user
who starts an SSO redirect on the old build still lands in the right org on the new one.
Reject a SuperTokens general-error marker that carries no usable string message; it produced an
AuthErrorCopy with an undefined message.
Percent-encode the characters that could close the `url("...")` in the AuthShell font face.
…provider lists (CodeRabbit) Replace the raw `--ag-c-*` palette literals with the semantic `--ag-colorBorderSecondary` and `--ag-colorTextSecondary` tokens the rest of the app uses, so the block follows light and dark appearance. Memoize the icon-decorated provider lists, which are passed as props.
…ited phase keys (CodeRabbit) `resolveToolDisplay` feeds its `input` to both the app resolver and the name parser, so omitting it gave a gateway or third-party tool a generic "Asked by" label. ApprovalCard already passes it. `phase in STARTUP_LABELS` accepted inherited keys such as "toString", returning a function where the contract promises `string | null`. Use hasOwnProperty and cover it with a test. Keep the replayed error text on a remounted ConnectToolWidget: after a reload there is no local outcome and usually no meta.output, so the original create failure was replaced by the generic "Connection not completed".
…Rabbit)
TurnStatusLine was hard-wired to `working={false}`, so between the submit and the first assistant
part nothing showed the request was in flight — PendingTurn cannot cover that window because no
assistant turn exists yet. The new `showTrailingWorkingPulse` helper is exactly the negation of
PendingTurn's own gate, so the two never render a pulse at the same time, and it is unit tested.
ChatScreen ran the auto-scroll hook but never rendered ChatJumpToLatest, so a reader who scrolled
up had no way back down. Add the same overlay LiveConversation uses.
Silence the AuthGate exhaustive-deps warning: the router changes identity on every navigation and
would re-fire the redirect. Rename the stale `middleware` handler in a proxy test comment.
`tabIndex={-1}` does not drop focus the button already has, so hiding it left a focused element
that was aria-hidden and invisible but still reachable by Enter. Blur it when it hides. `inert`
would be the tidier pattern but the package's React 18 peer renders `inert="false"` as present.
Member
|
@coderabbitai Here is what I did with the items that live only in the review summary, so they are not lost. Everything is pushed as of e6eef65. Outside diff range (2) — both applied
Nitpicks (6) — five applied, one skipped
Verification: typecheck clean on |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
The applied fixes address the two out-of-range items and the five accepted nitpicks. The decision to defer OIDC transport consolidation is reasonable for this release branch. The current paths have different error-copy behavior. Consolidation should preserve the specific provider-unreachable message before it replaces The reported type checks, unit tests, lint, and Prettier results provide good coverage for these changes. If you want, I can create a follow-up GitHub issue for the OIDC transport and You are interacting with an AI system. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/agenta-chat/src/assets/startupPhases.ts (1)
11-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep routine source comments to one short line.
web/packages/agenta-chat/src/assets/startupPhases.ts#L11-L12: remove the explanatory comment or reduce it to one short line.web/mobile/src/features/chat/turnStatus.ts#L1-L8: replace the multi-line JSDoc with one short line.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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: de66b304-a3e3-484b-ab80-78776d96a730
📒 Files selected for processing (20)
web/mobile/src/features/app/AuthGate.tsxweb/mobile/src/features/chat/ChatScreen.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/mobile/src/features/chat/turnStatus.tsweb/mobile/tests/unit/proxy.test.tsweb/mobile/tests/unit/turnStatus.test.tsweb/oss/src/components/Sidebar/components/AuthUpgradeModal.tsxweb/oss/src/hooks/usePostAuthRedirect.tsweb/oss/src/pages/auth/[[...path]].tsxweb/packages/agenta-auth-ui/src/AuthShell.tsxweb/packages/agenta-auth-ui/src/useSignInFlow.tsweb/packages/agenta-auth/src/authError.tsweb/packages/agenta-auth/src/signInPolicy.tsweb/packages/agenta-auth/tests/unit/lastSsoOrgSlug.test.tsweb/packages/agenta-auth/tests/unit/signInPolicy.test.tsweb/packages/agenta-chat/src/assets/startupPhases.tsweb/packages/agenta-chat/src/clientTools/ClientToolPart.tsxweb/packages/agenta-chat/tests/unit/assets/startupPhases.test.tsweb/packages/agenta-entity-ui/src/clientTools/ConnectToolWidget.tsxweb/packages/agenta-ui/src/components/presentational/chat/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/mobile/tests/unit/proxy.test.ts
- web/mobile/src/features/app/AuthGate.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
This was referenced Aug 21, 2026
pull Bot
pushed a commit
to jasonkneen/agenta
that referenced
this pull request
Aug 22, 2026
The @agenta/oss Next build dies at the 4096 MB Node heap cap since the package extraction grew the build graph (PR Agenta-AI#6152 CI shows the OOM; the build runners have 15.6 GB). Raise the cap to 8192 MB in the three gh Dockerfiles. Also run prettier on the two test files that fail the styling check on the release tip.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
#6112 closed the regressions the mobile-extraction lane introduced. Fourteen commits landed after it merged and never had a PR of their own. This is them, stacked on #6065 so that PR's diff stays clean.
Most of it is
/mcatching up to the desktop. A chat turn there was missing nearly everything the desktop turn offers: no timestamp, incomplete metrics, no rewind, no jump-to-latest, no working indicator inside the turn, and a client tool rendered nothing at all. Separately, the sign-in UI was already shared through@agenta/auth-uibut the flow was not, so 760 lines of orchestration sat in the oss page and/mhad no email-first discovery, no org SSO, no returning-visitor promotion and no invite capture. Mobile also moves to Next 16.Changes
One sign-in flow for oss, ee and /m. Three layers move down.
@agenta/authgets the React-free rules (signInPolicy,deriveEntry,deriveMethods,soleSsoRedirect,mapAuthError,readInviteParams,readAuthConfig), so they are unit-tested rather than argued about.@agenta/auth-uigetsuseSignInFlow, the entry to methods to code machine both pages now drive, plusAuthShellas the shared frame. Anything app-shaped stays a seam, because the OIDC transport genuinely differs: the desktop redirects to its registered callback URI,/mroutes through a cookie. The oss page keeps what is its own (region selector, auth-upgrade org switching, its antd rendering) and drops from 760 lines to 465.The chat turn reaches parity. Each piece was app-layer in oss, so it moves into a package rather than being copied:
TurnMetricsandTurnTimestampinto@agenta/chat/components,ChatJumpToLatestandturnToolbarClassinto@agenta/ui, the startup-narration lifecycle (startupPhases,turnClock,TurnActivity) into@agenta/chat, and the client-tool dispatcher into@agenta/chat/clientToolswith its widgets in@agenta/entity-ui/clientTools. Both surfaces now render the same components.Two of these were silent failures worth calling out.
Client tools rendered nothing on
/mbecauseuseAgentConversation's client-tool predicate was optional and/mnever passed one, so a host had to opt in to elicitation. It no longer can opt out. The first attempt at this registered widgets as a module side effect, which looked right and failed quietly: both packages declaresideEffects: false, so the bare import was tree-shaken, the registry stayed empty, and every elicitation auto-settled as "not handled by this client". The dispatcher now reaches the widgets by value.Every agent run on
/mfailed withconnection '<slug>' not found for provider 'anthropic', as if the vault secret were missing. It was not.buildAgentRequestappendsproject_idonly alongside anAuthorizationheader, and/mregistered no auth-header provider:With no
project_idthe backend falls back to the caller's default project, so the runner read the wrong vault and blamed the agent's connection./mnow registers the provider at its providers root, reading the token through a newgetAccessTokenin@agenta/auth.Two correctness fixes. A transient 401 stranded a valid session on the sign-in page until a hard reload, because
fetchProjectscollapsed "the refresh could not reach the backend" into the samefalseas "the session is revoked", and the gate ran its query only off/authwithrefetchOnWindowFocus: false, so nothing could ever correct it.refreshSessionOutcomenow distinguishes the three cases and an unreachable refresh is reported as transient. And the trace drawer opens through an atom, so whoever renders it decides where it works;/mmounted it inside the Observability screen alone, which is why "View trace" on a chat turn did nothing.Mobile moves to Next 16.3.
web/mobilegoes to 16.3.0 while oss, ee, storybook and every shared package stay on 15.5.x. The dual major coexists cleanly. Mobile is Pages Router, so the headline v16 break (asyncparams/searchParams/cookies()/headers()) is App-Router-only and did not apply. What did:eslint.ignoreDuringBuildsis a removed config option in 16;turbopack.rootmust be unconditional now that Turbopack drivesnext buildtoo; a webpack config hard-fails the build, so theIgnorePluginfor the AI SDK's optional peers becameturbopack.resolveAliasonto an empty module;middleware.tsbecameproxy.tswithexport function proxy;jsx: "preserve"became"react-jsx"; and@agenta/playground-ui'snextpeer went from>=15.5.21 <16to<17, the only pin that objected.One UI fix rides along. A session list's group headings pin below the panel header instead of on top of it (
SessionCardList,PanelSection).Tests
fix/post-112-reconcilerebased ontolane/mobile-extracted-packages, where two dropped as already upstream and the resulting tip tree came out byte-identical to the original, so their earlier verification carries over. The Next 16 commit is cherry-picked fromchore/mobile-next16, the UI fix fromfix/post-112-reconcile.pnpm install --lockfile-onlyagainst the result is a no-op, so the lockfile is self-consistent on this base. Themiddleware.tstoproxy.tsrename carried the branch's current gate logic unchanged; only the exported name differs.@agenta/chatand@agenta/entity-uigained the JSX transform their vitest configs predicted they would need, for the moved.tsxrender suites.What to QA
/mwith an email that maps to an org SSO connection. You get email-first discovery, then the lone connection redirects without asking. On oss the same flow still works, and the region selector and invite email-mismatch path are unchanged./m. It replies instead of failing with "connection not found". Check the invoke URL carriesproject_id./m. The toolbar shows a relative timestamp, latency, tokens, cost, Copy, View trace and Rewind, in a bordered card whose shadow matches the desktop. Rewind on a user turn refills the composer; on an assistant turn it regenerates, and it warns when tools that already ran would be rewound past./mand watch a cold start. "Starting the agent" appears beside the avatar in the turn, not below the list, and no bar appears above the transcript for your own run.pnpm --filter @agenta/mobile buildanddevboth run. The build printsƒ Proxy (Middleware), and the reverse gate still redirects a desktop UA off/m. oss and ee are untouched on 15.5.21.