Repository navigation
feat(mobile): ACP-backed workspace pages — restyled home, native workspace screen, diff surface - #5635
Merged
Conversation
…sions, restyled home list - Home list: drop the chat-card background; sessions hang off a thread rail under their workspace, date sections separate on whitespace alone - Workspace rows navigate to a new workspace page; a trailing secondary icon Button takes over the start-chat interaction - New workspace/[id] landing screen: chat list default tab + placeholder diff tab in a bottom tab bar - Home sessions and the workspace page list ACP sessions host-wide via acpSessions.list (new useHostAcpSessions hook); listSessions input is now optional/host-wide on the client - Composer target flow creates an ACP session and prompts it, landing in the ACP thread
…ss tabs - workspace/[id] uses the native stack header: floating back circle, centered GlassHeaderTitle pill with the workspace name, and a glass ellipsis menu (copy branch / rename) on the right - tabs move to expo-router NativeTabs ((tabs)/index + (tabs)/diff) so the bottom bar is the native liquid glass one - branch shown as a small centered line under the header on the chats tab; custom WorkspaceTabBar/DiffPlaceholder components removed
…itle + toolbar menu - plain native centered title (no GlassHeaderTitle pill) on the transparent header - ellipsis becomes a native Stack.Toolbar.Menu (copy branch / rename) composed on the (tabs) Stack.Screen, replacing the custom GlassView button + ActionSheetIOS
… subtitle, compose & menu toolbar - flatten navigation: the (tabs) screen now lives directly in the root authenticated stack (intermediate workspace Stack removed), so pushing from home gets the native back chevron for free - header config moves into the (tabs) layout via the Stack.Screen composition API (Stack.Title asChild + Stack.Toolbar must be DIRECT children — wrapper components are dropped by the matcher) - title view is name + branch subtitle, width-capped so it never slides under the trailing buttons - right side: native toolbar with a compose button (creates an ACP session, lands in the thread) and an ellipsis menu (copy branch, rename), sharing one glass capsule on iOS 26
…ons surface - tridot mirrors the home row menu: rename, copy branch / copy ID / share (inline group), destructive delete with the force-delete recovery flow (pops back to home on success); delete hidden for main - drop the Live sessions screen, route, and row-menu entry — ACP sessions are the chat list now
…, destructive delete last)
…me, delete, copy id/share)
…composer polish - Changes screen (files/commits/PR checks), files-changed + per-file views, commits formSheet with native pinned header and a continuous opaque TimelineRow rail - workspace page consolidated to a single screen: chat list, pinned composer with glass View-changes chip, full-viewport scroll surface - chat threads flattened into the root stack (native back chevron) with the session-title + workspace-name glass header - composer: whole-pill tap-to-focus (contentShape + onTapGesture), tighter SwiftUI edge padding; git router gains commit-files support
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe mobile authenticated navigation now exposes workspace screens for ACP sessions, changes, files, diffs, and commits. Home and workspace chat flows use ACP session data, with new hooks for git state, pull requests, workspace actions, and viewed-file persistence. ChangesMobile workspace and ACP experience
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
Ready to review this PR? Stage has broken it down into 9 individual chapters for you: 
Chapters generated by Stage for commit f59cb32 on Jul 12, 2026 6:21pm UTC. |
Contributor
Greptile SummaryThis PR rebuilds the mobile workspace flow around ACP sessions and a native changes surface. The main changes are:
Confidence Score: 4/5The changed diff aggregation path needs a fix before merging.
apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts
|
| Filename | Overview |
|---|---|
| apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts | Adds workspace status aggregation for the new changes screens; files are collapsed by path, which can hide changes from earlier git categories. |
| apps/mobile/screens/(authenticated)/workspace/[id]/files-changed/FilesChangedScreen.tsx | Adds the files-changed list with inline diff previews, viewed-file state, and per-file actions. |
| apps/mobile/screens/(authenticated)/(home)/home/hooks/useHostAcpSessions/useHostAcpSessions.ts | Adds host-wide ACP session polling and groups sessions by workspace for home and workspace lists. |
| apps/mobile/screens/(authenticated)/(home)/home/components/NewChatWidget/hooks/useStartWorkspaceChat/useStartWorkspaceChat.ts | Moves new workspace chat creation to ACP session creation and prompt submission. |
| packages/host-service/src/trpc/router/git/git.ts | Extends commit listing to include author email for the new commits sheet. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts:80
**Multi-Category File Changes Disappear**
When the same path has changes in more than one git category, this `byPath.set` keeps only the last category seen. A partially staged file can lose its staged or branch diff from the files list and totals, and the file viewer will fetch only the retained category, so the mobile diff surface can show an incomplete review.
Reviews (1): Last reviewed commit: "feat(mobile): workspace diff surface, co..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const [source, groupFiles] of groups) { | ||
| for (const file of groupFiles) { | ||
| byPath.set(file.path, { ...file, source }); | ||
| } |
Contributor
There was a problem hiding this comment.
When the same path has changes in more than one git category, this byPath.set keeps only the last category seen. A partially staged file can lose its staged or branch diff from the files list and totals, and the file viewer will fetch only the retained category, so the mobile diff surface can show an incomplete review.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts
Line: 80
Comment:
**Multi-Category File Changes Disappear**
When the same path has changes in more than one git category, this `byPath.set` keeps only the last category seen. A partially staged file can lose its staged or branch diff from the files list and totals, and the file viewer will fetch only the retained category, so the mobile diff surface can show an incomplete review.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
- files-changed and file-viewer cached different shapes (computed rows vs raw pair) under the same query key with infinite staleTime — whichever screen ran first poisoned the other; keys are now distinct - diff-row keys include the file's +/− stats so fresh edits bust the otherwise-immortal cache; viewer contents get a finite staleTime - changesets over 15 files rest collapsed so first open doesn't fire a full-contents git.getDiff per file through the relay
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
12 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/mobile/screens/(authenticated)/workspace/[id]/commits/components/AuthorAvatar/AuthorAvatar.tsx">
<violation number="1" location="apps/mobile/screens/(authenticated)/workspace/[id]/commits/components/AuthorAvatar/AuthorAvatar.tsx:41">
P2: `failed` state persists across URL changes — if an image load fails and then a valid URL becomes available (e.g., after `users` loads asynchronously), the fallback initial is still shown. Reset `failed` when `url` changes so new URLs get a fresh load attempt.</violation>
</file>
<file name="apps/mobile/screens/(authenticated)/(home)/home/hooks/useHostAcpSessions/useHostAcpSessions.ts">
<violation number="1" location="apps/mobile/screens/(authenticated)/(home)/home/hooks/useHostAcpSessions/useHostAcpSessions.ts:19">
P2: Query cache key collides when the same machine is a host in multiple orgs — include `organizationId` to scope by org, matching the routing key's org:machineId structure.</violation>
</file>
<file name="apps/mobile/screens/(authenticated)/workspace/[id]/diff/WorkspaceDiffScreen.tsx">
<violation number="1" location="apps/mobile/screens/(authenticated)/workspace/[id]/diff/WorkspaceDiffScreen.tsx:94">
P2: Pull-to-refresh only refetches the changeset, not commits. The `useWorkspaceCommits` hook uses `useQuery` with a 30s stale time and no auto-refetch, and doesn't expose a `refetch` method, so the commit list stays stale even after the user pulls to refresh. Consider exposing a `refetch` from `useWorkspaceCommits` and calling it alongside `changeset.refetch()` in `onRefresh`.</violation>
</file>
<file name="apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts">
<violation number="1" location="apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts:33">
P2: Query key omits `hostUrl`, so cached data isn't scoped to the specific host serving the workspace. If the workspace's owning host changes (e.g., reconnection with a different routing identity), stale data from the old host could be served until a refetch completes. The sibling `useVisibleDiffStats` hook scopes by hostUrl, making this inconsistency look like an oversight. Consider including `hostUrl` in the query key.</violation>
<violation number="2" location="apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts:68">
P3: The path-dedup + additions/deletions aggregation logic in this `useMemo` is a near-duplicate of `aggregateDiffStats` in `useVisibleDiffStats`. Both iterate againstBase→staged→unstaged, dedup by path (last writer wins), and sum totals. Extracting a shared utility (e.g., `aggregateChangesetRows`) into a common location would keep the two diff chips in sync automatically and reduce the risk of drift.</violation>
<violation number="3" location="apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts:79">
P2: When the same file path appears in multiple git status categories (e.g. partially staged — present in both `staged` and `unstaged`), `byPath.set` overwrites the earlier entry, retaining only the last category seen. This means the diff surface's file list, addition/deletion totals, and per-file viewer will only reflect the `unstaged` slice of the change; the staged portion disappears from the review.
Consider accumulating additions/deletions across categories before deduplicating, or emitting one `ChangesetFile` entry per unique `(path, source)` pair so the user can review each category independently.</violation>
<violation number="4" location="apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts:102">
P2: Opening a workspace before its host lookup settles briefly renders “No changes on this branch yet,” even when the workspace has changes. `hostUrl === null` currently means both “host is unavailable” and “still resolving,” so `isReady` becomes true too early. Consider incorporating `useWorkspaceHost(...).isResolving` before allowing the empty state to be ready.</violation>
</file>
<file name="apps/mobile/screens/(authenticated)/workspace/[id]/commits/CommitsSheet.tsx">
<violation number="1" location="apps/mobile/screens/(authenticated)/workspace/[id]/commits/CommitsSheet.tsx:144">
P3: Opening the sheet while the host and commit queries are still loading shows “No commits on this branch yet,” which incorrectly describes a branch that has not been fetched. Gate this empty state on the commits query having settled (and render a loading/error state as appropriate) so a cold mount does not present a false empty result.</violation>
</file>
<file name="apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx">
<violation number="1" location="apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx:43">
P2: The workspace header exposes only the options menu, so there is no native compose action in the toolbar. `startNewChat` is already implemented and returned by the hook but is never destructured or wired here; adding a toolbar button that calls it would make the advertised compose control reachable and eliminate the dead action.</violation>
</file>
<file name="apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceHeaderActions/useWorkspaceHeaderActions.ts">
<violation number="1" location="apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceHeaderActions/useWorkspaceHeaderActions.ts:150">
P2: Sharing the workspace URL via `Share.share({ url })` won't include the URL on Android — `url` is iOS-only and `message` is required on Android. Set `message` to the same URL so sharing works cross-platform.</violation>
<violation number="2" location="apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceHeaderActions/useWorkspaceHeaderActions.ts:150">
P3: A failed native share request is currently left as an unhandled rejection because `Share.share` is fire-and-forget. Handling the rejection lets the app show a share failure rather than producing an unobserved promise error.
(Based on your team's feedback about handling asynchronous failures explicitly.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/host-service/src/trpc/router/git/git.ts">
<violation number="1" location="packages/host-service/src/trpc/router/git/git.ts:142">
P2: Commit subjects containing a tab corrupt the newly added email and the timestamp because the git record is split on tabs even though `%s` can contain tabs. Use a delimiter that Git commit metadata cannot contain (such as NUL) and parse records/fields with that delimiter, so the commit sheet retains the actual author email and ISO date.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| (q) => q.from({ users: collections.users }), | ||
| [collections], | ||
| ); | ||
| const [failed, setFailed] = useState(false); |
Contributor
There was a problem hiding this comment.
P2: failed state persists across URL changes — if an image load fails and then a valid URL becomes available (e.g., after users loads asynchronously), the fallback initial is still shown. Reset failed when url changes so new URLs get a fresh load attempt.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/commits/components/AuthorAvatar/AuthorAvatar.tsx, line 41:
<comment>`failed` state persists across URL changes — if an image load fails and then a valid URL becomes available (e.g., after `users` loads asynchronously), the fallback initial is still shown. Reset `failed` when `url` changes so new URLs get a fresh load attempt.</comment>
<file context>
@@ -0,0 +1,73 @@
+ (q) => q.from({ users: collections.users }),
+ [collections],
+ );
+ const [failed, setFailed] = useState(false);
+
+ const url = useMemo(() => {
</file context>
| const SESSIONS_REFETCH_INTERVAL_MS = 30_000; | ||
| const SESSIONS_PAGE_LIMIT = 200; | ||
|
|
||
| export function getAcpSessionsQueryKey(machineId: string | null) { |
Contributor
There was a problem hiding this comment.
P2: Query cache key collides when the same machine is a host in multiple orgs — include organizationId to scope by org, matching the routing key's org:machineId structure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/(home)/home/hooks/useHostAcpSessions/useHostAcpSessions.ts, line 19:
<comment>Query cache key collides when the same machine is a host in multiple orgs — include `organizationId` to scope by org, matching the routing key's org:machineId structure.</comment>
<file context>
@@ -0,0 +1,73 @@
+const SESSIONS_REFETCH_INTERVAL_MS = 30_000;
+const SESSIONS_PAGE_LIMIT = 200;
+
+export function getAcpSessionsQueryKey(machineId: string | null) {
+ return ["acp-sessions", "list", machineId] as const;
+}
</file context>
| const onRefresh = useCallback(async () => { | ||
| setRefreshing(true); | ||
| try { | ||
| await changeset.refetch(); |
Contributor
There was a problem hiding this comment.
P2: Pull-to-refresh only refetches the changeset, not commits. The useWorkspaceCommits hook uses useQuery with a 30s stale time and no auto-refetch, and doesn't expose a refetch method, so the commit list stays stale even after the user pulls to refresh. Consider exposing a refetch from useWorkspaceCommits and calling it alongside changeset.refetch() in onRefresh.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/diff/WorkspaceDiffScreen.tsx, line 94:
<comment>Pull-to-refresh only refetches the changeset, not commits. The `useWorkspaceCommits` hook uses `useQuery` with a 30s stale time and no auto-refetch, and doesn't expose a `refetch` method, so the commit list stays stale even after the user pulls to refresh. Consider exposing a `refetch` from `useWorkspaceCommits` and calling it alongside `changeset.refetch()` in `onRefresh`.</comment>
<file context>
@@ -0,0 +1,252 @@
+ const onRefresh = useCallback(async () => {
+ setRefreshing(true);
+ try {
+ await changeset.refetch();
+ } finally {
+ setRefreshing(false);
</file context>
| const CHANGESET_STALE_MS = 15_000; | ||
| const CHANGESET_REFETCH_MS = 30_000; | ||
|
|
||
| export function getWorkspaceChangesetQueryKey(workspaceId: string | null) { |
Contributor
There was a problem hiding this comment.
P2: Query key omits hostUrl, so cached data isn't scoped to the specific host serving the workspace. If the workspace's owning host changes (e.g., reconnection with a different routing identity), stale data from the old host could be served until a refetch completes. The sibling useVisibleDiffStats hook scopes by hostUrl, making this inconsistency look like an oversight. Consider including hostUrl in the query key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts, line 33:
<comment>Query key omits `hostUrl`, so cached data isn't scoped to the specific host serving the workspace. If the workspace's owning host changes (e.g., reconnection with a different routing identity), stale data from the old host could be served until a refetch completes. The sibling `useVisibleDiffStats` hook scopes by hostUrl, making this inconsistency look like an oversight. Consider including `hostUrl` in the query key.</comment>
<file context>
@@ -0,0 +1,105 @@
+const CHANGESET_STALE_MS = 15_000;
+const CHANGESET_REFETCH_MS = 30_000;
+
+export function getWorkspaceChangesetQueryKey(workspaceId: string | null) {
+ return ["workspace-changeset", workspaceId] as const;
+}
</file context>
|
|
||
| const { workspace, host } = useWorkspaceHost(id ?? null); | ||
| const { sessionsByWorkspace, isReady } = useHostAcpSessions(host); | ||
| const { renameWorkspace, deleteWorkspace, copyId, shareWorkspace } = |
Contributor
There was a problem hiding this comment.
P2: The workspace header exposes only the options menu, so there is no native compose action in the toolbar. startNewChat is already implemented and returned by the hook but is never destructured or wired here; adding a toolbar button that calls it would make the advertised compose control reachable and eliminate the dead action.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx, line 43:
<comment>The workspace header exposes only the options menu, so there is no native compose action in the toolbar. `startNewChat` is already implemented and returned by the hook but is never destructured or wired here; adding a toolbar button that calls it would make the advertised compose control reachable and eliminate the dead action.</comment>
<file context>
@@ -0,0 +1,193 @@
+
+ const { workspace, host } = useWorkspaceHost(id ?? null);
+ const { sessionsByWorkspace, isReady } = useHostAcpSessions(host);
+ const { renameWorkspace, deleteWorkspace, copyId, shareWorkspace } =
+ useWorkspaceHeaderActions(workspace, host);
+ const changeset = useWorkspaceChangeset(id ?? null);
</file context>
| currentBranch: query.data?.currentBranch?.name ?? null, | ||
| baseBranch: query.data?.defaultBranch?.name ?? null, | ||
| hostUrl, | ||
| isReady: hostUrl === null || query.isSuccess || query.isError, |
Contributor
There was a problem hiding this comment.
P2: Opening a workspace before its host lookup settles briefly renders “No changes on this branch yet,” even when the workspace has changes. hostUrl === null currently means both “host is unavailable” and “still resolving,” so isReady becomes true too early. Consider incorporating useWorkspaceHost(...).isResolving before allowing the empty state to be ready.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts, line 102:
<comment>Opening a workspace before its host lookup settles briefly renders “No changes on this branch yet,” even when the workspace has changes. `hostUrl === null` currently means both “host is unavailable” and “still resolving,” so `isReady` becomes true too early. Consider incorporating `useWorkspaceHost(...).isResolving` before allowing the empty state to be ready.</comment>
<file context>
@@ -0,0 +1,105 @@
+ currentBranch: query.data?.currentBranch?.name ?? null,
+ baseBranch: query.data?.defaultBranch?.name ?? null,
+ hostUrl,
+ isReady: hostUrl === null || query.isSuccess || query.isError,
+ refetch: query.refetch,
+ };
</file context>
| ]; | ||
| for (const [source, groupFiles] of groups) { | ||
| for (const file of groupFiles) { | ||
| byPath.set(file.path, { ...file, source }); |
Contributor
There was a problem hiding this comment.
P2: When the same file path appears in multiple git status categories (e.g. partially staged — present in both staged and unstaged), byPath.set overwrites the earlier entry, retaining only the last category seen. This means the diff surface's file list, addition/deletion totals, and per-file viewer will only reflect the unstaged slice of the change; the staged portion disappears from the review.
Consider accumulating additions/deletions across categories before deduplicating, or emitting one ChangesetFile entry per unique (path, source) pair so the user can review each category independently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts, line 79:
<comment>When the same file path appears in multiple git status categories (e.g. partially staged — present in both `staged` and `unstaged`), `byPath.set` overwrites the earlier entry, retaining only the last category seen. This means the diff surface's file list, addition/deletion totals, and per-file viewer will only reflect the `unstaged` slice of the change; the staged portion disappears from the review.
Consider accumulating additions/deletions across categories before deduplicating, or emitting one `ChangesetFile` entry per unique `(path, source)` pair so the user can review each category independently.</comment>
<file context>
@@ -0,0 +1,105 @@
+ ];
+ for (const [source, groupFiles] of groups) {
+ for (const file of groupFiles) {
+ byPath.set(file.path, { ...file, source });
+ }
+ }
</file context>
| }, | ||
| }); | ||
|
|
||
| const { files, additions, deletions } = useMemo(() => { |
Contributor
There was a problem hiding this comment.
P3: The path-dedup + additions/deletions aggregation logic in this useMemo is a near-duplicate of aggregateDiffStats in useVisibleDiffStats. Both iterate againstBase→staged→unstaged, dedup by path (last writer wins), and sum totals. Extracting a shared utility (e.g., aggregateChangesetRows) into a common location would keep the two diff chips in sync automatically and reduce the risk of drift.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.ts, line 68:
<comment>The path-dedup + additions/deletions aggregation logic in this `useMemo` is a near-duplicate of `aggregateDiffStats` in `useVisibleDiffStats`. Both iterate againstBase→staged→unstaged, dedup by path (last writer wins), and sum totals. Extracting a shared utility (e.g., `aggregateChangesetRows`) into a common location would keep the two diff chips in sync automatically and reduce the risk of drift.</comment>
<file context>
@@ -0,0 +1,105 @@
+ },
+ });
+
+ const { files, additions, deletions } = useMemo(() => {
+ const status = query.data;
+ const byPath = new Map<string, ChangesetFile>();
</file context>
| </TimelineRow> | ||
| ); | ||
| }} | ||
| ListEmptyComponent={ |
Contributor
There was a problem hiding this comment.
P3: Opening the sheet while the host and commit queries are still loading shows “No commits on this branch yet,” which incorrectly describes a branch that has not been fetched. Gate this empty state on the commits query having settled (and render a loading/error state as appropriate) so a cold mount does not present a false empty result.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/commits/CommitsSheet.tsx, line 144:
<comment>Opening the sheet while the host and commit queries are still loading shows “No commits on this branch yet,” which incorrectly describes a branch that has not been fetched. Gate this empty state on the commits query having settled (and render a loading/error state as appropriate) so a cold mount does not present a false empty result.</comment>
<file context>
@@ -0,0 +1,154 @@
+ </TimelineRow>
+ );
+ }}
+ ListEmptyComponent={
+ <View className="items-center py-16">
+ <Text className="text-muted-foreground text-sm">
</file context>
|
|
||
| const shareWorkspace = () => { | ||
| if (!workspace) return; | ||
| void Share.share({ |
Contributor
There was a problem hiding this comment.
P3: A failed native share request is currently left as an unhandled rejection because Share.share is fire-and-forget. Handling the rejection lets the app show a share failure rather than producing an unobserved promise error.
(Based on your team's feedback about handling asynchronous failures explicitly.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceHeaderActions/useWorkspaceHeaderActions.ts, line 150:
<comment>A failed native share request is currently left as an unhandled rejection because `Share.share` is fire-and-forget. Handling the rejection lets the app show a share failure rather than producing an unobserved promise error.
(Based on your team's feedback about handling asynchronous failures explicitly.) </comment>
<file context>
@@ -0,0 +1,163 @@
+
+ const shareWorkspace = () => {
+ if (!workspace) return;
+ void Share.share({
+ url: `https://app.superset.sh/workspaces/${workspace.id}`,
+ });
</file context>
![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: 11
🧹 Nitpick comments (3)
apps/mobile/screens/(authenticated)/workspace/[id]/files-changed/FilesChangedScreen.tsx (1)
419-446: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReconsider
extraData={renderItem}and missingrecycleItems.
@legendapp/list's own performance guidance recommends against this pattern: "Avoid extraData when possible. It is a whole-list invalidation escape hatch...Changing extraData causes Legend List to re-render all items because the value is passed through every renderItem call."renderItemcloses overstartWorkspaceChat(auseMutationresult) andworkspace— both of which are unlikely to be stable across renders — soextraData's identity changes often, forcing a full re-render of every row (up toMAX_ROWSdiff lines per expanded file) on most state updates. Sinceitemsalready produces new object references per changed row via theuseMemoon Lines 269-300, LegendList should already pick up per-row changes withoutextraData.Additionally, rows here are stateless (no internal
useState), which the library flags as the safe case forrecycleItems, currently unset.⚡ Suggested prop changes
<LegendList className="bg-background flex-1" contentInsetAdjustmentBehavior="automatic" contentContainerStyle={{ paddingBottom: 48, paddingTop: 8 }} data={items} - extraData={renderItem} + recycleItems keyExtractor={itemKey} renderItem={renderItem}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/mobile/screens/`(authenticated)/workspace/[id]/files-changed/FilesChangedScreen.tsx around lines 419 - 446, Remove extraData={renderItem} from the LegendList in FilesChangedScreen so updates rely on the existing items references and avoid full-list invalidation. Enable recycleItems on this list, since its row components are stateless and safe to recycle.apps/mobile/screens/(authenticated)/workspace/[id]/files-changed/utils/computeFileDiff/computeFileDiff.ts (1)
20-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSolid diff/truncation logic — consider adding a co-located unit test.
The hunk-to-row conversion and
MAX_ROWStruncation logic (line-number tracking, "no newline" marker skipping) is non-trivial and has no accompanying test in this batch. As per coding guidelines, tests should be co-located with the utility they test.Consider adding
computeFileDiff.test.tsalongside this file covering: additions/deletions/context mixes, multi-hunk files, and theMAX_ROWStruncation boundary. Want me to draft the test cases?
As per coding guidelines, "Co-locate tests with the component, hook, or utility they test."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/mobile/screens/`(authenticated)/workspace/[id]/files-changed/utils/computeFileDiff/computeFileDiff.ts around lines 20 - 96, Add a co-located unit test file for computeFileDiff covering mixed additions, deletions, and context rows; multiple hunks with correct line numbers; skipped no-newline markers; and truncation at the MAX_ROWS boundary, including the hiddenCount result. Reuse the utility’s existing exports and test framework conventions without changing computeFileDiff behavior.Source: Coding guidelines
apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceHeaderActions/useWorkspaceHeaderActions.ts (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused chat action from
useWorkspaceHeaderActions
startNewChatandcreatingChataren’t used byWorkspaceScreen, and the session-creation flow already lives inuseStartWorkspaceChat. If there’s no separate toolbar action planned here, move this logic out ofuseWorkspaceHeaderActionsand keep the chat-starting code in the dedicated hook.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/mobile/screens/`(authenticated)/workspace/[id]/hooks/useWorkspaceHeaderActions/useWorkspaceHeaderActions.ts at line 24, Remove the unused chat state and startNewChat logic from useWorkspaceHeaderActions, including creatingChat and its setter. Keep chat/session creation exclusively in useStartWorkspaceChat, and preserve the remaining workspace header actions and their public API.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/mobile/app/`(authenticated)/(home)/_layout.tsx:
- Around line 3-41: Restore a locally scoped PromptInputProvider around the
HomeLayout Stack so Home’s attachments and textInput state remain isolated from
other authenticated screens. Keep the existing Stack screen configuration
unchanged and use the established provider component visible in the surrounding
layout implementation.
In
`@apps/mobile/screens/`(authenticated)/(home)/home/components/NewChatWidget/NewChatWidget.tsx:
- Around line 143-156: Update NewChatWidget’s expanded calculation and
storeTarget auto-focus useEffect so both only respond to storeTarget when
fixedTarget is absent. Preserve the existing focused behavior and prevent stale
shared selections from reopening or focusing a composer with a fixed target.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/AcpSessionThreadScreen.tsx:
- Around line 21-60: Update the AcpSessionThreadScreen routingKey fallback using
the host-resolution state, such as isResolving, so the ActivityIndicator appears
only while resolution is in progress. Once resolution finishes without a
routingKey, render an offline/not-found state instead of leaving the screen on a
permanent spinner, while preserving the existing SessionThread path when
routingKey is available.
In `@apps/mobile/screens/`(authenticated)/workspace/[id]/commits/CommitsSheet.tsx:
- Around line 29-51: Update the stat query key in the useQueries configuration
to include hostUrl alongside workspaceId and commit.hash, ensuring stats from
different hosts are cached separately while preserving the existing query
behavior.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/commits/components/AuthorAvatar/AuthorAvatar.tsx:
- Around line 41-50: Reset the failed state whenever the memoized avatar `url`
changes, using an effect tied to `url` so a newly resolved image can render
after a prior load error. Keep the existing failure handling and fallback
behavior unchanged.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/diff/WorkspaceDiffScreen.tsx:
- Around line 177-183: Update the conclusion-to-icon mapping in the checks map
to distinguish actual failure conclusions from other terminal non-success states
such as neutral, cancelled, or skipped. Keep null mapped to CHECK_ICON.pending
and success mapped to CHECK_ICON.success, while mapping only the explicit
failure state to CHECK_ICON.failure and using the appropriate neutral icon for
remaining conclusions.
- Around line 91-98: Extend the refresh flow in onRefresh so pull-to-refresh
updates all three panels, not only changeset. First expose refetch support from
useWorkspaceCommits and useWorkspacePullRequest as needed, then invoke those
refetch operations alongside changeset.refetch while preserving the existing
refreshing state cleanup.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/file/FileViewerScreen.tsx:
- Around line 52-72: Use the isResolving value returned by useWorkspaceHost in
FileViewerScreen and include it in the empty-state condition so “This file is
empty or was deleted” is not shown while host resolution is pending. Keep the
existing query behavior unchanged and display the loading state until resolution
completes.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/files-changed/FilesChangedScreen.tsx:
- Around line 156-201: Change the FilesChangedScreen diff-loading flow so
useQueries does not fetch every initially expanded file. Track files currently
visible or near the viewport using the screen’s list viewability mechanism, and
filter expandedFiles (or otherwise gate each query) to that visible set before
calling getHostServiceClientByUrl(...).git.getDiff.query. Preserve manual
expansion behavior while ensuring offscreen files do not trigger requests until
they become viewable.
- Around line 302-309: Update onRefresh in FilesChangedScreen to invalidate the
cached "workspace-file-diff" queries before or alongside changeset.refetch(), so
already-loaded diffs are fetched again despite their infinite staleTime.
Preserve the existing refreshing state cleanup and changeset refresh behavior.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx:
- Around line 41-56: Update the widgetWorkspaces useMemo in WorkspaceScreen to
derive hostReachable from the actual host online state, using host?.isOnline ===
true instead of hardcoding true. Preserve the existing empty-array behavior when
workspace is unavailable and keep the memo dependencies synchronized with the
host value.
---
Nitpick comments:
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/files-changed/FilesChangedScreen.tsx:
- Around line 419-446: Remove extraData={renderItem} from the LegendList in
FilesChangedScreen so updates rely on the existing items references and avoid
full-list invalidation. Enable recycleItems on this list, since its row
components are stateless and safe to recycle.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/files-changed/utils/computeFileDiff/computeFileDiff.ts:
- Around line 20-96: Add a co-located unit test file for computeFileDiff
covering mixed additions, deletions, and context rows; multiple hunks with
correct line numbers; skipped no-newline markers; and truncation at the MAX_ROWS
boundary, including the hiddenCount result. Reuse the utility’s existing exports
and test framework conventions without changing computeFileDiff behavior.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/hooks/useWorkspaceHeaderActions/useWorkspaceHeaderActions.ts:
- Line 24: Remove the unused chat state and startNewChat logic from
useWorkspaceHeaderActions, including creatingChat and its setter. Keep
chat/session creation exclusively in useStartWorkspaceChat, and preserve the
remaining workspace header actions and their public API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 049055e6-d2a2-44d8-87e7-023755e0afeb
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (56)
apps/mobile/app/(authenticated)/(home)/_layout.tsxapps/mobile/app/(authenticated)/_layout.tsxapps/mobile/app/(authenticated)/workspace/[id]/_layout.tsxapps/mobile/app/(authenticated)/workspace/[id]/chat/_layout.tsxapps/mobile/app/(authenticated)/workspace/[id]/chat/acp/index.tsxapps/mobile/app/(authenticated)/workspace/[id]/commits.tsxapps/mobile/app/(authenticated)/workspace/[id]/diff.tsxapps/mobile/app/(authenticated)/workspace/[id]/file.tsxapps/mobile/app/(authenticated)/workspace/[id]/files-changed.tsxapps/mobile/app/(authenticated)/workspace/[id]/index.tsxapps/mobile/lib/host/client.tsapps/mobile/package.jsonapps/mobile/screens/(authenticated)/(home)/home/HomeScreen.tsxapps/mobile/screens/(authenticated)/(home)/home/components/NewChatWidget/NewChatWidget.tsxapps/mobile/screens/(authenticated)/(home)/home/components/NewChatWidget/hooks/useStartWorkspaceChat/useStartWorkspaceChat.tsapps/mobile/screens/(authenticated)/(home)/home/components/SessionRow/SessionRow.tsxapps/mobile/screens/(authenticated)/(home)/home/components/WorkspaceRow/WorkspaceRow.tsxapps/mobile/screens/(authenticated)/(home)/home/components/WorkspaceRow/components/WorkspaceRowMenu/WorkspaceRowMenu.tsxapps/mobile/screens/(authenticated)/(home)/home/hooks/useHostAcpSessions/index.tsapps/mobile/screens/(authenticated)/(home)/home/hooks/useHostAcpSessions/useHostAcpSessions.tsapps/mobile/screens/(authenticated)/(home)/home/utils/sessionRows/index.tsapps/mobile/screens/(authenticated)/(home)/home/utils/sessionRows/sessionRows.tsapps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/AcpSessionsScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/AcpSessionThreadScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/commits/CommitsSheet.tsxapps/mobile/screens/(authenticated)/workspace/[id]/commits/components/AuthorAvatar/AuthorAvatar.tsxapps/mobile/screens/(authenticated)/workspace/[id]/commits/components/AuthorAvatar/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/commits/components/TimelineRow/TimelineRow.tsxapps/mobile/screens/(authenticated)/workspace/[id]/commits/components/TimelineRow/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/commits/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/diff/WorkspaceDiffScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/diff/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/file/FileViewerScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/file/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/files-changed/FilesChangedScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/files-changed/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/files-changed/stores/viewedFilesStore/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/files-changed/stores/viewedFilesStore/viewedFilesStore.tsapps/mobile/screens/(authenticated)/workspace/[id]/files-changed/utils/computeFileDiff/computeFileDiff.tsapps/mobile/screens/(authenticated)/workspace/[id]/files-changed/utils/computeFileDiff/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceChangeset/useWorkspaceChangeset.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceCommits/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceCommits/useWorkspaceCommits.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceHeaderActions/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspaceHeaderActions/useWorkspaceHeaderActions.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspacePullRequest/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useWorkspacePullRequest/useWorkspacePullRequest.tspackages/host-client/src/acp/acp.tspackages/host-service/src/trpc/router/git/git.tspackages/host-service/src/trpc/router/git/types.tspackages/host-service/test/integration/acp-host-client.e2e.test.tsplans/acp-host-client-unification.md
💤 Files with no reviewable changes (5)
- apps/mobile/app/(authenticated)/workspace/[id]/chat/acp/index.tsx
- apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/index.ts
- apps/mobile/app/(authenticated)/workspace/[id]/chat/_layout.tsx
- apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/AcpSessionsScreen.tsx
- apps/mobile/app/(authenticated)/workspace/[id]/_layout.tsx
Comment on lines
3
to
+41
| export default function HomeLayout() { | ||
| return ( | ||
| <PromptInputProvider> | ||
| <Stack | ||
| screenOptions={{ | ||
| headerBackButtonDisplayMode: "minimal", | ||
| headerShadowVisible: false, | ||
| <Stack | ||
| screenOptions={{ | ||
| headerBackButtonDisplayMode: "minimal", | ||
| headerShadowVisible: false, | ||
| }} | ||
| > | ||
| <Stack.Screen name="index" options={{ title: "" }} /> | ||
| <Stack.Screen | ||
| name="filter" | ||
| options={{ | ||
| presentation: "formSheet", | ||
| headerShown: false, | ||
| sheetAllowedDetents: [1.0], | ||
| sheetGrabberVisible: true, | ||
| }} | ||
| > | ||
| <Stack.Screen name="index" options={{ title: "" }} /> | ||
| <Stack.Screen | ||
| name="filter" | ||
| options={{ | ||
| presentation: "formSheet", | ||
| headerShown: false, | ||
| sheetAllowedDetents: [1.0], | ||
| sheetGrabberVisible: true, | ||
| }} | ||
| /> | ||
| <Stack.Screen | ||
| name="new-chat" | ||
| options={{ | ||
| presentation: "formSheet", | ||
| headerShown: false, | ||
| sheetAllowedDetents: [1.0], | ||
| sheetGrabberVisible: true, | ||
| }} | ||
| /> | ||
| <Stack.Screen | ||
| name="attachments" | ||
| options={{ | ||
| presentation: "formSheet", | ||
| headerShown: false, | ||
| // Single detent: multi-detent resizes corrupt expo-image frames. | ||
| // Content (incl. the screenshots grid) is sized to fit 0.5. | ||
| sheetAllowedDetents: [0.5], | ||
| sheetGrabberVisible: true, | ||
| }} | ||
| /> | ||
| </Stack> | ||
| </PromptInputProvider> | ||
| /> | ||
| <Stack.Screen | ||
| name="new-chat" | ||
| options={{ | ||
| presentation: "formSheet", | ||
| headerShown: false, | ||
| sheetAllowedDetents: [1.0], | ||
| sheetGrabberVisible: true, | ||
| }} | ||
| /> | ||
| <Stack.Screen | ||
| name="attachments" | ||
| options={{ | ||
| presentation: "formSheet", | ||
| headerShown: false, | ||
| // Single detent: multi-detent resizes corrupt expo-image frames. | ||
| // Content (incl. the screenshots grid) is sized to fit 0.5. | ||
| sheetAllowedDetents: [0.5], | ||
| sheetGrabberVisible: true, | ||
| }} | ||
| /> | ||
| </Stack> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "PromptInputProvider" apps/mobile -g '*.tsx' -C2
rg -n "usePromptInputController" apps/mobile -C2Repository: superset-sh/superset
Length of output: 8142
🏁 Script executed:
#!/bin/bash
sed -n '1,140p' apps/mobile/app/'(authenticated)'/_layout.tsx
printf '\n----\n'
sed -n '1,120p' apps/mobile/app/'(authenticated)'/'(home)'/_layout.tsx
printf '\n----\n'
sed -n '1,220p' apps/mobile/components/ai-elements/prompt-input.tsx
printf '\n----\n'
sed -n '1,220p' apps/mobile/screens/'(authenticated)'/'(home)'/home/components/NewChatWidget/NewChatWidget.tsxRepository: superset-sh/superset
Length of output: 16002
🏁 Script executed:
#!/bin/bash
rg -n "PromptInputProvider|usePromptInputController|useProviderAttachments|usePromptInputAttachments" apps/mobile -C2Repository: superset-sh/superset
Length of output: 13294
Keep Home’s PromptInputProvider scoped locally
Removing the nested provider makes Home share attachments and textInput with every other authenticated screen via app/(authenticated)/_layout.tsx. That means a draft or attachment picked in Home can leak into workspace/[id] composers and vice versa.
Restore the Home-scoped provider
+import { PromptInputProvider } from "`@/components/ai-elements/prompt-input`";
+
export default function HomeLayout() {
return (
- <Stack
- screenOptions={{
- headerBackButtonDisplayMode: "minimal",
- headerShadowVisible: false,
- }}
- >
- ...
- </Stack>
+ <PromptInputProvider>
+ <Stack
+ screenOptions={{
+ headerBackButtonDisplayMode: "minimal",
+ headerShadowVisible: false,
+ }}
+ >
+ ...
+ </Stack>
+ </PromptInputProvider>
);
}📝 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.
Suggested change
| export default function HomeLayout() { | |
| return ( | |
| <PromptInputProvider> | |
| <Stack | |
| screenOptions={{ | |
| headerBackButtonDisplayMode: "minimal", | |
| headerShadowVisible: false, | |
| <Stack | |
| screenOptions={{ | |
| headerBackButtonDisplayMode: "minimal", | |
| headerShadowVisible: false, | |
| }} | |
| > | |
| <Stack.Screen name="index" options={{ title: "" }} /> | |
| <Stack.Screen | |
| name="filter" | |
| options={{ | |
| presentation: "formSheet", | |
| headerShown: false, | |
| sheetAllowedDetents: [1.0], | |
| sheetGrabberVisible: true, | |
| }} | |
| > | |
| <Stack.Screen name="index" options={{ title: "" }} /> | |
| <Stack.Screen | |
| name="filter" | |
| options={{ | |
| presentation: "formSheet", | |
| headerShown: false, | |
| sheetAllowedDetents: [1.0], | |
| sheetGrabberVisible: true, | |
| }} | |
| /> | |
| <Stack.Screen | |
| name="new-chat" | |
| options={{ | |
| presentation: "formSheet", | |
| headerShown: false, | |
| sheetAllowedDetents: [1.0], | |
| sheetGrabberVisible: true, | |
| }} | |
| /> | |
| <Stack.Screen | |
| name="attachments" | |
| options={{ | |
| presentation: "formSheet", | |
| headerShown: false, | |
| // Single detent: multi-detent resizes corrupt expo-image frames. | |
| // Content (incl. the screenshots grid) is sized to fit 0.5. | |
| sheetAllowedDetents: [0.5], | |
| sheetGrabberVisible: true, | |
| }} | |
| /> | |
| </Stack> | |
| </PromptInputProvider> | |
| /> | |
| <Stack.Screen | |
| name="new-chat" | |
| options={{ | |
| presentation: "formSheet", | |
| headerShown: false, | |
| sheetAllowedDetents: [1.0], | |
| sheetGrabberVisible: true, | |
| }} | |
| /> | |
| <Stack.Screen | |
| name="attachments" | |
| options={{ | |
| presentation: "formSheet", | |
| headerShown: false, | |
| // Single detent: multi-detent resizes corrupt expo-image frames. | |
| // Content (incl. the screenshots grid) is sized to fit 0.5. | |
| sheetAllowedDetents: [0.5], | |
| sheetGrabberVisible: true, | |
| }} | |
| /> | |
| </Stack> | |
| import { PromptInputProvider } from "`@/components/ai-elements/prompt-input`"; | |
| export default function HomeLayout() { | |
| return ( | |
| <PromptInputProvider> | |
| <Stack | |
| screenOptions={{ | |
| headerBackButtonDisplayMode: "minimal", | |
| headerShadowVisible: false, | |
| }} | |
| > | |
| <Stack.Screen name="index" options={{ title: "" }} /> | |
| <Stack.Screen | |
| name="filter" | |
| options={{ | |
| presentation: "formSheet", | |
| headerShown: false, | |
| sheetAllowedDetents: [1.0], | |
| sheetGrabberVisible: true, | |
| }} | |
| /> | |
| <Stack.Screen | |
| name="new-chat" | |
| options={{ | |
| presentation: "formSheet", | |
| headerShown: false, | |
| sheetAllowedDetents: [1.0], | |
| sheetGrabberVisible: true, | |
| }} | |
| /> | |
| <Stack.Screen | |
| name="attachments" | |
| options={{ | |
| presentation: "formSheet", | |
| headerShown: false, | |
| // Single detent: multi-detent resizes corrupt expo-image frames. | |
| // Content (incl. the screenshots grid) is sized to fit 0.5. | |
| sheetAllowedDetents: [0.5], | |
| sheetGrabberVisible: true, | |
| }} | |
| /> | |
| </Stack> | |
| </PromptInputProvider> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/mobile/app/`(authenticated)/(home)/_layout.tsx around lines 3 - 41,
Restore a locally scoped PromptInputProvider around the HomeLayout Stack so
Home’s attachments and textInput state remain isolated from other authenticated
screens. Keep the existing Stack screen configuration unchanged and use the
established provider component visible in the surrounding layout implementation.
Comment on lines
+143
to
+156
| const storeTarget = useChatTargetStore((state) => state.target); | ||
| const clearChatTarget = useChatTargetStore((state) => state.clearTarget); | ||
| const startWorkspaceChat = useStartWorkspaceChat(resolveHostUrl); | ||
| const chatTarget = fixedTarget ?? storeTarget; | ||
| const startWorkspaceChat = useStartWorkspaceChat(workspaces); | ||
|
|
||
| // Collapse whenever the keyboard is away — a draft just clamps to one line. | ||
| // A workspace target keeps the composer open so its chip stays visible. | ||
| const expanded = focused || chatTarget !== null; | ||
| // A picked workspace target keeps the composer open so its chip stays | ||
| // visible; a fixed target is ambient and doesn't hold it open. | ||
| const expanded = focused || storeTarget !== null; | ||
| const showTargetRow = expanded && !fixedTarget; | ||
|
|
||
| useEffect(() => { | ||
| if (chatTarget) void fieldRef.current?.focus(); | ||
| }, [chatTarget]); | ||
| if (storeTarget) void fieldRef.current?.focus(); | ||
| }, [storeTarget]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "fixedTarget" apps/mobile -C3
rg -n "setTarget\(" apps/mobile -C3Repository: superset-sh/superset
Length of output: 6324
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== NewChatWidget relevant slice ==\n'
sed -n '130,170p' 'apps/mobile/screens/(authenticated)/(home)/home/components/NewChatWidget/NewChatWidget.tsx'
printf '\n== Chat target store ==\n'
rg -n "useChatTargetStore|create.*chatTarget|setTarget|clearTarget|target" apps/mobile -C3
printf '\n== Workspace screen slice ==\n'
sed -n '136,155p' 'apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx'
printf '\n== WorkspaceRow slice ==\n'
sed -n '116,136p' 'apps/mobile/screens/(authenticated)/(home)/home/components/WorkspaceRow/WorkspaceRow.tsx'Repository: superset-sh/superset
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'chatTarget' apps/mobileRepository: superset-sh/superset
Length of output: 358
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== clearTarget usage ==\n'
rg -n "clearTarget|setTarget\(" 'apps/mobile/screens/(authenticated)/(home)' -C2
printf '\n== WorkspaceScreen around NewChatWidget ==\n'
sed -n '138,154p' 'apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx'
printf '\n== WorkspaceRow target button context ==\n'
sed -n '118,132p' 'apps/mobile/screens/(authenticated)/(home)/home/components/WorkspaceRow/WorkspaceRow.tsx'Repository: superset-sh/superset
Length of output: 3377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== clearTarget usage in workspace/home chat flow ==\n'
rg -n "clearTarget" \
'apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx' \
'apps/mobile/screens/(authenticated)/(home)/home/components/NewChatWidget/NewChatWidget.tsx' \
'apps/mobile/screens/(authenticated)/(home)/home/components/WorkspaceRow/WorkspaceRow.tsx' \
'apps/mobile/screens/(authenticated)/(home)' -C2
printf '\n== WorkspaceScreen NewChatWidget props ==\n'
sed -n '142,150p' 'apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx'
printf '\n== WorkspaceRow target button ==\n'
sed -n '122,131p' 'apps/mobile/screens/(authenticated)/(home)/home/components/WorkspaceRow/WorkspaceRow.tsx'Repository: superset-sh/superset
Length of output: 3284
Ignore the shared target store when fixedTarget is set.
expanded and the auto-focus effect still key off the global storeTarget, so a stale Home selection can reopen and focus a workspace composer that already has a fixed target. Guard both with !fixedTarget:
Proposed fix
- const expanded = focused || storeTarget !== null;
+ const expanded = focused || (!fixedTarget && storeTarget !== null);
const showTargetRow = expanded && !fixedTarget;
useEffect(() => {
- if (storeTarget) void fieldRef.current?.focus();
- }, [storeTarget]);
+ if (!fixedTarget && storeTarget) void fieldRef.current?.focus();
+ }, [storeTarget, fixedTarget]);📝 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.
Suggested change
| const storeTarget = useChatTargetStore((state) => state.target); | |
| const clearChatTarget = useChatTargetStore((state) => state.clearTarget); | |
| const startWorkspaceChat = useStartWorkspaceChat(resolveHostUrl); | |
| const chatTarget = fixedTarget ?? storeTarget; | |
| const startWorkspaceChat = useStartWorkspaceChat(workspaces); | |
| // Collapse whenever the keyboard is away — a draft just clamps to one line. | |
| // A workspace target keeps the composer open so its chip stays visible. | |
| const expanded = focused || chatTarget !== null; | |
| // A picked workspace target keeps the composer open so its chip stays | |
| // visible; a fixed target is ambient and doesn't hold it open. | |
| const expanded = focused || storeTarget !== null; | |
| const showTargetRow = expanded && !fixedTarget; | |
| useEffect(() => { | |
| if (chatTarget) void fieldRef.current?.focus(); | |
| }, [chatTarget]); | |
| if (storeTarget) void fieldRef.current?.focus(); | |
| }, [storeTarget]); | |
| const storeTarget = useChatTargetStore((state) => state.target); | |
| const clearChatTarget = useChatTargetStore((state) => state.clearTarget); | |
| const chatTarget = fixedTarget ?? storeTarget; | |
| const startWorkspaceChat = useStartWorkspaceChat(workspaces); | |
| // Collapse whenever the keyboard is away — a draft just clamps to one line. | |
| // A picked workspace target keeps the composer open so its chip stays | |
| // visible; a fixed target is ambient and doesn't hold it open. | |
| const expanded = focused || (!fixedTarget && storeTarget !== null); | |
| const showTargetRow = expanded && !fixedTarget; | |
| useEffect(() => { | |
| if (!fixedTarget && storeTarget) void fieldRef.current?.focus(); | |
| }, [storeTarget, fixedTarget]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/mobile/screens/`(authenticated)/(home)/home/components/NewChatWidget/NewChatWidget.tsx
around lines 143 - 156, Update NewChatWidget’s expanded calculation and
storeTarget auto-focus useEffect so both only respond to storeTarget when
fixedTarget is absent. Preserve the existing focused behavior and prevent stale
shared selections from reopening or focusing a composer with a fixed target.
Comment on lines
+21
to
+60
| const { workspace, host } = useWorkspaceHost(id ?? null); | ||
| const { sessionsByWorkspace } = useHostAcpSessions(host); | ||
| const session = id | ||
| ? sessionsByWorkspace | ||
| .get(id) | ||
| ?.find((candidate) => candidate.sessionId === sessionId) | ||
| : undefined; | ||
|
|
||
| if (!sessionId) return null; | ||
|
|
||
| const header = ( | ||
| <Stack.Title asChild> | ||
| <View className="max-w-52 items-center"> | ||
| <Text className="font-semibold text-[17px]" numberOfLines={1}> | ||
| {session?.title ?? "New session"} | ||
| </Text> | ||
| {workspace?.name ? ( | ||
| <Text className="text-muted-foreground text-xs" numberOfLines={1}> | ||
| {workspace.name} | ||
| </Text> | ||
| ) : null} | ||
| </View> | ||
| </Stack.Title> | ||
| ); | ||
|
|
||
| if (!routingKey) { | ||
| return ( | ||
| <View className="bg-background flex-1 items-center justify-center"> | ||
| {header} | ||
| <ActivityIndicator /> | ||
| </View> | ||
| ); | ||
| } | ||
|
|
||
| return <SessionThread routingKey={routingKey} sessionId={sessionId} />; | ||
| return ( | ||
| <> | ||
| {header} | ||
| <SessionThread routingKey={routingKey} sessionId={sessionId} /> | ||
| </> | ||
| ); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Find useHostRoutingKey implementation and check offline handling
rg -nP "export (function|const) useHostRoutingKey" --type ts -A 30
# 2. Check if useHostRoutingKey filters by isOnline or returns null for offline hosts
rg -nP "isOnline|offline|null" --type ts -g "*useHostRoutingKey*"
# 3. Verify Stack.Title usage elsewhere in the codebase
rg -nP "Stack\.Title" --type ts -g "!*.test.*" | head -20
# 4. Check expo-router exports for Title
rg -nP "\bTitle\b" --type ts -g "*expo-router*" node_modules/expo-router/build/ 2>/dev/null | head -20 || echo "expo-router build dir not found, checking package.json exports"Repository: superset-sh/superset
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map relevant files and symbols
git ls-files 'apps/mobile/**/*AcpSessionThreadScreen.tsx' 'apps/mobile/**/*useHostRoutingKey*' 'apps/mobile/**/*useWorkspaceHost*' 'apps/mobile/**/*SessionThread*' | sed -n '1,200p'
printf '\n--- useHostRoutingKey candidates ---\n'
fd -a 'useHostRoutingKey' apps/mobile 2>/dev/null | sed -n '1,50p'
printf '\n--- useWorkspaceHost candidates ---\n'
fd -a 'useWorkspaceHost' apps/mobile 2>/dev/null | sed -n '1,50p'
printf '\n--- SessionThread candidates ---\n'
fd -a 'SessionThread' apps/mobile 2>/dev/null | sed -n '1,50p'
printf '\n--- Stack.Title usages ---\n'
rg -n "Stack\.Title" apps/mobile --glob '!**/*.test.*' || trueRepository: superset-sh/superset
Length of output: 6007
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the screen file and related hooks/components
fd -a 'AcpSessionThreadScreen.tsx' apps/mobile 2>/dev/null | sed -n '1,20p'
fd -a 'useHostRoutingKey' . 2>/dev/null | sed -n '1,20p'
fd -a 'useWorkspaceHost' . 2>/dev/null | sed -n '1,20p'
fd -a 'SessionThread' . 2>/dev/null | sed -n '1,20p'Repository: superset-sh/superset
Length of output: 2119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect relevant source files with line numbers
printf '\n--- useHostRoutingKey outline ---\n'
ast-grep outline 'apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useHostRoutingKey/useHostRoutingKey.ts' --view expanded || true
printf '\n--- useWorkspaceHost outline ---\n'
ast-grep outline 'apps/mobile/hooks/useWorkspaceHost/useWorkspaceHost.ts' --view expanded || true
printf '\n--- AcpSessionThreadScreen outline ---\n'
ast-grep outline 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/AcpSessionThreadScreen.tsx' --view expanded || true
printf '\n--- SessionThread outline ---\n'
ast-grep outline 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/SessionThread.tsx' --view expanded || trueRepository: superset-sh/superset
Length of output: 1038
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read relevant file sections
sed -n '1,220p' 'apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useHostRoutingKey/useHostRoutingKey.ts' | cat -n
printf '\n===== useWorkspaceHost =====\n'
sed -n '1,240p' 'apps/mobile/hooks/useWorkspaceHost/useWorkspaceHost.ts' | cat -n
printf '\n===== AcpSessionThreadScreen =====\n'
sed -n '1,220p' 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/AcpSessionThreadScreen.tsx' | cat -n
printf '\n===== SessionThread =====\n'
sed -n '1,260p' 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/SessionThread.tsx' | cat -nRepository: superset-sh/superset
Length of output: 14968
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show all Stack.Title and headerTitle usage near the relevant screen patterns
printf '\n--- Stack.Title occurrences with context ---\n'
rg -n -C 3 "Stack\.Title|headerTitle" apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId] apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen apps/mobile/screens/(authenticated)/workspace/[id]/commits/CommitsSheet.tsx || true
printf '\n--- expo-router package info ---\n'
if [ -f package.json ]; then
jq '.dependencies["expo-router"], .devDependencies["expo-router"], .dependencies["expo"], .devDependencies["expo"]' package.json 2>/dev/null || true
fi
if [ -f apps/mobile/package.json ]; then
jq '.dependencies["expo-router"], .devDependencies["expo-router"], .dependencies["expo"], .devDependencies["expo"]' apps/mobile/package.json 2>/dev/null || true
fiRepository: superset-sh/superset
Length of output: 257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for offline/null handling and isResolving in hooks
rg -n -C 3 "isResolving|isOnline|offline|null|routingKey" 'apps/mobile/screens/(authenticated)/workspace/[id]/hooks/useHostRoutingKey/useHostRoutingKey.ts' 'apps/mobile/hooks/useWorkspaceHost/useWorkspaceHost.ts' || trueRepository: superset-sh/superset
Length of output: 5733
🌐 Web query:
expo-router Stack.Title asChild headerTitle precedence Stack.Screen options headerTitle Title API
💡 Result:
In Expo Router, Stack.Screen.Title is the specialized component used to configure the screen title within a stack navigator [1][2]. Key details regarding your query: Stack.Screen.Title and asChild The Stack.Screen.Title component allows you to define a custom title for your screen [2]. When you use the asChild prop, you can pass a custom React component to serve as the title instead of a standard string [3][2]. This is useful for rendering complex UI elements within the header title area. <Stack.Screen name="index"> <Stack.Screen.Title asChild> </Stack.Screen.Title> </Stack.Screen> Usage and Precedence 1. Placement: Stack.Screen.Title can be used inside Stack.Screen within your _layout.tsx files or directly within page components [2]. 2. Precedence: If multiple instances of Stack.Screen.Title are rendered for the same screen (for example, if defined both in a layout and a page), the last one rendered in the component tree takes precedence [2]. 3. Deprecation Note: You may see references to Stack.Title. In newer versions of Expo Router, Stack.Title is often mapped to Stack.Screen.Title, and it is recommended to use the latter or verify if your specific version prefers one over the other [1][4]. headerTitle vs. Stack.Screen.Title While you can set the header title using the headerTitle option within the Stack.Screen options object [5], Stack.Screen.Title is a more declarative, component-based API specifically for defining the title [1][2]. For more advanced header configurations or if you need to access parent navigators to set options, you can use the useNavigation hook within your screen components to manually set or update the header options [6]. Top Results: [1] https://github.com/expo/expo/blob/97172ac0/packages/expo-router/src/layouts/stack-utils/StackScreen.tsx [3] https://docs.expo.dev/versions/v56.0.0/sdk/router/stack [2] https://github.com/expo/expo/blob/5cbac55c/packages/expo-router/build/layouts/stack-utils/screen/StackScreenTitle.d.ts [5] https://docs.expo.dev/versions/latest/sdk/router
Citations:
- 1: https://github.com/expo/expo/blob/97172ac0/packages/expo-router/src/layouts/stack-utils/StackScreen.tsx
- 2: https://github.com/expo/expo/blob/5cbac55c/packages/expo-router/build/layouts/stack-utils/screen/StackScreenTitle.d.ts
- 3: https://docs.expo.dev/versions/v56.0.0/sdk/router/stack
- 4: https://github.com/expo/expo/blob/97172ac0/packages/expo-router/build/layouts/stack-utils/StackScreen.d.ts
- 5: https://docs.expo.dev/versions/latest/sdk/router
- 6: Issue with custom header in nested Stack navigator expo/expo#29009
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- package versions ---\n'
jq '.dependencies["expo-router"], .devDependencies["expo-router"], .dependencies["expo"], .devDependencies["expo"]' package.json 2>/dev/null || true
jq '.dependencies["expo-router"], .devDependencies["expo-router"], .dependencies["expo"], .devDependencies["expo"]' apps/mobile/package.json 2>/dev/null || true
printf '\n--- Stack.Title and headerTitle context ---\n'
rg -n -C 2 "Stack\.Title|headerTitle" \
'apps/mobile/screens/(authenticated)/workspace/[id]/commits/CommitsSheet.tsx' \
'apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx' \
'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/AcpSessionThreadScreen.tsx' \
'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/SessionThread.tsx' || trueRepository: superset-sh/superset
Length of output: 4558
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' 'apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx' | cat -n
printf '\n===== CommitsSheet =====\n'
sed -n '1,140p' 'apps/mobile/screens/(authenticated)/workspace/[id]/commits/CommitsSheet.tsx' | cat -nRepository: superset-sh/superset
Length of output: 11887
Avoid a permanent spinner when the host can’t be resolved. routingKey stays null both while resolving and when no online host owns the workspace, so gate the ActivityIndicator on isResolving and render an offline/not-found state once resolution finishes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/AcpSessionThreadScreen.tsx
around lines 21 - 60, Update the AcpSessionThreadScreen routingKey fallback
using the host-resolution state, such as isResolving, so the ActivityIndicator
appears only while resolution is in progress. Once resolution finishes without a
routingKey, render an offline/not-found state instead of leaving the screen on a
permanent spinner, while preserving the existing SessionThread path when
routingKey is available.
Comment on lines
+29
to
+51
| const statQueries = useQueries({ | ||
| queries: statTargets.map((commit) => ({ | ||
| queryKey: ["workspace-commit-files", workspaceId, commit.hash] as const, | ||
| staleTime: Number.POSITIVE_INFINITY, | ||
| retry: 1, | ||
| networkMode: "always" as const, | ||
| queryFn: async () => { | ||
| const { files } = await getHostServiceClientByUrl( | ||
| hostUrl as string, | ||
| ).git.getCommitFiles.query({ | ||
| workspaceId: workspaceId as string, | ||
| commitHash: commit.hash, | ||
| }); | ||
| let additions = 0; | ||
| let deletions = 0; | ||
| for (const file of files) { | ||
| additions += file.additions; | ||
| deletions += file.deletions; | ||
| } | ||
| return { additions, deletions }; | ||
| }, | ||
| })), | ||
| }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Include hostUrl in the stat query key to prevent stale cross-host data.
The queryKey ["workspace-commit-files", workspaceId, commit.hash] omits hostUrl. If the host URL changes for the same workspaceId (e.g., host goes offline then returns with a different relay URL), useQueries will return cached stats from the previous host without refetching, since staleTime: Infinity prevents revalidation. Adding hostUrl to the key ensures data from different hosts is cached separately.
🔧 Proposed fix
queries: statTargets.map((commit) => ({
- queryKey: ["workspace-commit-files", workspaceId, commit.hash] as const,
+ queryKey: ["workspace-commit-files", workspaceId, hostUrl, commit.hash] as const,
staleTime: Number.POSITIVE_INFINITY,📝 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.
Suggested change
| const statQueries = useQueries({ | |
| queries: statTargets.map((commit) => ({ | |
| queryKey: ["workspace-commit-files", workspaceId, commit.hash] as const, | |
| staleTime: Number.POSITIVE_INFINITY, | |
| retry: 1, | |
| networkMode: "always" as const, | |
| queryFn: async () => { | |
| const { files } = await getHostServiceClientByUrl( | |
| hostUrl as string, | |
| ).git.getCommitFiles.query({ | |
| workspaceId: workspaceId as string, | |
| commitHash: commit.hash, | |
| }); | |
| let additions = 0; | |
| let deletions = 0; | |
| for (const file of files) { | |
| additions += file.additions; | |
| deletions += file.deletions; | |
| } | |
| return { additions, deletions }; | |
| }, | |
| })), | |
| }); | |
| const statQueries = useQueries({ | |
| queries: statTargets.map((commit) => ({ | |
| queryKey: ["workspace-commit-files", workspaceId, hostUrl, commit.hash] as const, | |
| staleTime: Number.POSITIVE_INFINITY, | |
| retry: 1, | |
| networkMode: "always" as const, | |
| queryFn: async () => { | |
| const { files } = await getHostServiceClientByUrl( | |
| hostUrl as string, | |
| ).git.getCommitFiles.query({ | |
| workspaceId: workspaceId as string, | |
| commitHash: commit.hash, | |
| }); | |
| let additions = 0; | |
| let deletions = 0; | |
| for (const file of files) { | |
| additions += file.additions; | |
| deletions += file.deletions; | |
| } | |
| return { additions, deletions }; | |
| }, | |
| })), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/mobile/screens/`(authenticated)/workspace/[id]/commits/CommitsSheet.tsx
around lines 29 - 51, Update the stat query key in the useQueries configuration
to include hostUrl alongside workspaceId and commit.hash, ensuring stats from
different hosts are cached separately while preserving the existing query
behavior.
Comment on lines
+41
to
+50
| const [failed, setFailed] = useState(false); | ||
|
|
||
| const url = useMemo(() => { | ||
| const normalized = (email ?? "").trim().toLowerCase(); | ||
| if (!normalized) return null; | ||
| const member = (users ?? []).find( | ||
| (user) => user.email.toLowerCase() === normalized, | ||
| ); | ||
| return member?.image ?? githubAvatarUrl(normalized); | ||
| }, [users, email]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset failed state when the avatar URL changes.
failed is set to true on image error but never reset. If the url recomputes (e.g., user's image is updated in the collection), the component keeps showing fallback initials even though the new URL may be valid.
🐛 Proposed fix
import { useMemo, useState } from "react";
+import { useEffect } from "react"; const [failed, setFailed] = useState(false);
const url = useMemo(() => {
const normalized = (email ?? "").trim().toLowerCase();
if (!normalized) return null;
const member = (users ?? []).find(
(user) => user.email.toLowerCase() === normalized,
);
return member?.image ?? githubAvatarUrl(normalized);
}, [users, email]);
+ useEffect(() => {
+ setFailed(false);
+ }, [url]);
+
const dimensions = { width: size, height: size, borderRadius: size / 2 };📝 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.
Suggested change
| const [failed, setFailed] = useState(false); | |
| const url = useMemo(() => { | |
| const normalized = (email ?? "").trim().toLowerCase(); | |
| if (!normalized) return null; | |
| const member = (users ?? []).find( | |
| (user) => user.email.toLowerCase() === normalized, | |
| ); | |
| return member?.image ?? githubAvatarUrl(normalized); | |
| }, [users, email]); | |
| const [failed, setFailed] = useState(false); | |
| const url = useMemo(() => { | |
| const normalized = (email ?? "").trim().toLowerCase(); | |
| if (!normalized) return null; | |
| const member = (users ?? []).find( | |
| (user) => user.email.toLowerCase() === normalized, | |
| ); | |
| return member?.image ?? githubAvatarUrl(normalized); | |
| }, [users, email]); | |
| useEffect(() => { | |
| setFailed(false); | |
| }, [url]); | |
| const dimensions = { width: size, height: size, borderRadius: size / 2 }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/commits/components/AuthorAvatar/AuthorAvatar.tsx
around lines 41 - 50, Reset the failed state whenever the memoized avatar `url`
changes, using an effect tied to `url` so a newly resolved image can render
after a prior load error. Keep the existing failure handling and fallback
behavior unchanged.
Comment on lines
+177
to
+183
| {checks.map((check) => { | ||
| const conclusion = | ||
| check.conclusion === "success" | ||
| ? CHECK_ICON.success | ||
| : check.conclusion == null | ||
| ? CHECK_ICON.pending | ||
| : CHECK_ICON.failure; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Non-failure check conclusions (neutral/cancelled/skipped) render as a failure (red X).
Only null is treated as pending; every other non-"success" conclusion is mapped to CHECK_ICON.failure, which misrepresents neutral/cancelled/skipped checks as failed.
♻️ Proposed fix: distinguish real failures from other terminal states
const conclusion =
check.conclusion === "success"
? CHECK_ICON.success
: check.conclusion == null
? CHECK_ICON.pending
- : CHECK_ICON.failure;
+ : check.conclusion === "failure"
+ ? CHECK_ICON.failure
+ : CHECK_ICON.none;📝 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.
Suggested change
| {checks.map((check) => { | |
| const conclusion = | |
| check.conclusion === "success" | |
| ? CHECK_ICON.success | |
| : check.conclusion == null | |
| ? CHECK_ICON.pending | |
| : CHECK_ICON.failure; | |
| {checks.map((check) => { | |
| const conclusion = | |
| check.conclusion === "success" | |
| ? CHECK_ICON.success | |
| : check.conclusion == null | |
| ? CHECK_ICON.pending | |
| : check.conclusion === "failure" | |
| ? CHECK_ICON.failure | |
| : CHECK_ICON.none; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/diff/WorkspaceDiffScreen.tsx
around lines 177 - 183, Update the conclusion-to-icon mapping in the checks map
to distinguish actual failure conclusions from other terminal non-success states
such as neutral, cancelled, or skipped. Keep null mapped to CHECK_ICON.pending
and success mapped to CHECK_ICON.success, while mapping only the explicit
failure state to CHECK_ICON.failure and using the appropriate neutral icon for
remaining conclusions.
Comment on lines
+52
to
+72
| const { host } = useWorkspaceHost(id ?? null); | ||
| const hostUrl = | ||
| host?.isOnline === true | ||
| ? buildRelayHostUrl(host.organizationId, host.machineId) | ||
| : null; | ||
|
|
||
| const category = (source ?? "unstaged") as ChangesetSource; | ||
|
|
||
| const query = useQuery({ | ||
| queryKey: ["workspace-file-diff", id ?? null, category, path] as const, | ||
| enabled: hostUrl !== null && !!id && !!path, | ||
| staleTime: Number.POSITIVE_INFINITY, | ||
| retry: 1, | ||
| networkMode: "always" as const, | ||
| queryFn: () => | ||
| getHostServiceClientByUrl(hostUrl as string).git.getDiff.query({ | ||
| workspaceId: id as string, | ||
| path: path as string, | ||
| category, | ||
| }), | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## locate useWorkspaceHost and FileViewerScreen\n'
rg -n "useWorkspaceHost|FileViewerScreen" apps/mobile -g '!**/node_modules/**'
printf '\n## inspect relevant files (outline if available)\n'
for f in \
"apps/mobile/screens/(authenticated)/workspace/[id]/file/FileViewerScreen.tsx" \
"apps/mobile"
do
[ -e "$f" ] && ast-grep outline "$f" --view expanded || true
doneRepository: superset-sh/superset
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the hook implementation and any type definitions.
rg -n "function useWorkspaceHost|const useWorkspaceHost|export .*useWorkspaceHost|useWorkspaceHost\\(" apps/mobile -g '!**/node_modules/**'
# If found, print the smallest useful slices around the implementation.
FILES=$(rg -l "useWorkspaceHost" apps/mobile -g '!**/node_modules/**' | head -n 20)
for f in $FILES; do
echo "### $f"
wc -l "$f"
doneRepository: superset-sh/superset
Length of output: 3651
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## useWorkspaceHost implementation"
sed -n '1,120p' apps/mobile/hooks/useWorkspaceHost/useWorkspaceHost.ts | cat -n
echo
echo "## FileViewerScreen loading/error/empty branches"
sed -n '1,180p' "apps/mobile/screens/(authenticated)/workspace/[id]/file/FileViewerScreen.tsx" | cat -nRepository: superset-sh/superset
Length of output: 7174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## useWorkspaceHost consumer patterns"
for f in \
"apps/mobile/screens/(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx" \
"apps/mobile/screens/(authenticated)/workspace/[id]/chat/[sessionId]/hooks/useChatThread/useChatThread.ts" \
"apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/AcpSessionThreadScreen.tsx"
do
echo
echo "### $f"
sed -n '1,170p' "$f" | cat -n | sed -n '1,170p'
doneRepository: superset-sh/superset
Length of output: 16657
Gate the empty state on isResolving
useWorkspaceHost already exposes isResolving, but this screen ignores it. While the host lookup is still pending, the disabled query falls through to “This file is empty or was deleted.” Keep the loading state up until resolution finishes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/mobile/screens/`(authenticated)/workspace/[id]/file/FileViewerScreen.tsx
around lines 52 - 72, Use the isResolving value returned by useWorkspaceHost in
FileViewerScreen and include it in the empty-state condition so “This file is
empty or was deleted” is not shown while host resolution is pending. Keep the
existing query behavior unchanged and display the loading state until resolution
completes.
Comment on lines
+302
to
+309
| const onRefresh = useCallback(async () => { | ||
| setRefreshing(true); | ||
| try { | ||
| await changeset.refetch(); | ||
| } finally { | ||
| setRefreshing(false); | ||
| } | ||
| }, [changeset.refetch]); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Pull-to-refresh never refreshes already-loaded diffs.
diffQueries uses staleTime: Number.POSITIVE_INFINITY (Line 183), so once a file's diff is fetched it is cached forever. onRefresh only calls changeset.refetch(), never invalidating the "workspace-file-diff" queries, so pull-to-refresh silently leaves stale diff content on screen even after new commits/edits land on the host — for the lifetime of the query cache, not just this screen instance.
🔄 Proposed fix: invalidate diff queries on refresh
+import { useQueries, useQueryClient } from "`@tanstack/react-query`";
...
+ const queryClient = useQueryClient();
const onRefresh = useCallback(async () => {
setRefreshing(true);
try {
await changeset.refetch();
+ await queryClient.invalidateQueries({
+ queryKey: ["workspace-file-diff", workspaceId],
+ });
} finally {
setRefreshing(false);
}
- }, [changeset.refetch]);
+ }, [changeset.refetch, queryClient, workspaceId]);📝 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.
Suggested change
| const onRefresh = useCallback(async () => { | |
| setRefreshing(true); | |
| try { | |
| await changeset.refetch(); | |
| } finally { | |
| setRefreshing(false); | |
| } | |
| }, [changeset.refetch]); | |
| const queryClient = useQueryClient(); | |
| const onRefresh = useCallback(async () => { | |
| setRefreshing(true); | |
| try { | |
| await changeset.refetch(); | |
| await queryClient.invalidateQueries({ | |
| queryKey: ["workspace-file-diff", workspaceId], | |
| }); | |
| } finally { | |
| setRefreshing(false); | |
| } | |
| }, [changeset.refetch, queryClient, workspaceId]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/files-changed/FilesChangedScreen.tsx
around lines 302 - 309, Update onRefresh in FilesChangedScreen to invalidate the
cached "workspace-file-diff" queries before or alongside changeset.refetch(), so
already-loaded diffs are fetched again despite their infinite staleTime.
Preserve the existing refreshing state cleanup and changeset refresh behavior.
Comment on lines
+41
to
+56
| const { workspace, host } = useWorkspaceHost(id ?? null); | ||
| const { sessionsByWorkspace, isReady } = useHostAcpSessions(host); | ||
| const { renameWorkspace, deleteWorkspace, copyId, shareWorkspace } = | ||
| useWorkspaceHeaderActions(workspace, host); | ||
| const changeset = useWorkspaceChangeset(id ?? null); | ||
| const pullRequest = useWorkspacePullRequest(id ?? null); | ||
|
|
||
| const sessionRows = useMemo( | ||
| () => buildSessionRows(id ? (sessionsByWorkspace.get(id) ?? []) : []), | ||
| [sessionsByWorkspace, id], | ||
| ); | ||
|
|
||
| const widgetWorkspaces = useMemo<HostWorkspaceItem[]>( | ||
| () => (workspace ? [{ ...workspace, hostReachable: true }] : []), | ||
| [workspace], | ||
| ); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
hostReachable is hardcoded to true regardless of actual host state.
widgetWorkspaces always sets hostReachable: true whenever workspace exists, but workspace can resolve independently of whether host is actually online (see useWorkspaceChangeset.ts which separately gates on host?.isOnline === true). This misrepresents host reachability to NewChatWidget, which is rendered here purely based on workspace truthiness (line 143) — a user could attempt to start a chat against an offline host.
🐛 Proposed fix
const widgetWorkspaces = useMemo<HostWorkspaceItem[]>(
- () => (workspace ? [{ ...workspace, hostReachable: true }] : []),
- [workspace],
+ () => (workspace ? [{ ...workspace, hostReachable: host?.isOnline === true }] : []),
+ [workspace, host],
);📝 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.
Suggested change
| const { workspace, host } = useWorkspaceHost(id ?? null); | |
| const { sessionsByWorkspace, isReady } = useHostAcpSessions(host); | |
| const { renameWorkspace, deleteWorkspace, copyId, shareWorkspace } = | |
| useWorkspaceHeaderActions(workspace, host); | |
| const changeset = useWorkspaceChangeset(id ?? null); | |
| const pullRequest = useWorkspacePullRequest(id ?? null); | |
| const sessionRows = useMemo( | |
| () => buildSessionRows(id ? (sessionsByWorkspace.get(id) ?? []) : []), | |
| [sessionsByWorkspace, id], | |
| ); | |
| const widgetWorkspaces = useMemo<HostWorkspaceItem[]>( | |
| () => (workspace ? [{ ...workspace, hostReachable: true }] : []), | |
| [workspace], | |
| ); | |
| const { workspace, host } = useWorkspaceHost(id ?? null); | |
| const { sessionsByWorkspace, isReady } = useHostAcpSessions(host); | |
| const { renameWorkspace, deleteWorkspace, copyId, shareWorkspace } = | |
| useWorkspaceHeaderActions(workspace, host); | |
| const changeset = useWorkspaceChangeset(id ?? null); | |
| const pullRequest = useWorkspacePullRequest(id ?? null); | |
| const sessionRows = useMemo( | |
| () => buildSessionRows(id ? (sessionsByWorkspace.get(id) ?? []) : []), | |
| [sessionsByWorkspace, id], | |
| ); | |
| const widgetWorkspaces = useMemo<HostWorkspaceItem[]>( | |
| () => (workspace ? [{ ...workspace, hostReachable: host?.isOnline === true }] : []), | |
| [workspace, host], | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/WorkspaceScreen/WorkspaceScreen.tsx
around lines 41 - 56, Update the widgetWorkspaces useMemo in WorkspaceScreen to
derive hostReachable from the actual host online state, using host?.isOnline ===
true instead of hardcoding true. Preserve the existing empty-array behavior when
workspace is unavailable and keep the memo dependencies synchronized with the
host value.
…mail doesn't match
5 of 6 tasks
This was referenced Aug 14, 2026
This branch was previously 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.
Summary
Reworks the mobile workspace UX end to end on the new ACP session architecture:
acpSessions.list(useHostAcpSessions, 30s poll + focus refetch); the composer creates an ACP session and prompts it; threads open atchat/acp/[sessionId]. The legacy live-sessions surface is removed.contentShape+onTapGesture, tighter SwiftUI edge padding,fixedTarget/aboveslots for the workspace page.host-clientlistSessionsaccepts optional host-wide input; git router gains commit-file stats; ACP client-unification proposal inplans/.Test plan
bun run lintand turbo typecheck green across mobile, host-client, host-serviceSummary by cubic
Rebuilt the mobile workspace around ACP: home shows per‑workspace session rails, a native workspace page hosts chats with a pinned composer and “View changes,” and a full diff surface covers files, commits, and PR checks. Also fixes diff cache correctness, a viewed‑files selector render loop, large‑changeset fetch stampedes, and improves commit avatar resolution.
New Features
acpSessions.listwith 30s polling; tap workspace opens its page; plus button starts a chat.workspace/[id]/chat/acp/[sessionId]route with a glass title header; legacy live‑sessions view removed.host-clientlistSessions(routingKey, input?)supports host‑wide input;gitrouter adds per‑file stats andauthorEmail; addeduseHostAcpSessions; new dependencydiff.Migration
/(authenticated)/workspace/[id]/chat/acp/[sessionId]; remove links to the old live‑sessions screen.listAcpSessionsto pass an optional input object (e.g.,{ workspaceId, cursor, limit }).diffinapps/mobileand the lockfile.acpSessions.listand the updatedgitprocedures (commit files andauthorEmail) for diff/commits views.Written for commit f59cb32. Summary will update on new commits.
Summary by CodeRabbit