fix(mobile): refresh folder workspaces in Home catalog - #11767
fix(mobile): refresh folder workspaces in Home catalog#11767brennanb2025 wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughFolder workspace create, update, and delete IPC handlers now use a dedicated catalog notification path when available. The path synchronizes the worktree watcher and retains the legacy repository notification as a fallback. Main-window service wiring passes runtime context to the handlers. Host worktree refreshes now queue overlapping requests, suppress superseded responses, and replay queued refreshes. New tests cover IPC mutations, request coordination, and refresh options. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
Greptile SummaryThis PR fixes Mobile Home so that a folder workspace created at runtime on a paired desktop host immediately appears in the catalog when the host publishes
Confidence Score: 5/5Safe to merge. The change is narrowly scoped to catalog refresh routing and does not touch authentication, storage, or protocol handling. Both sides of the fix are logically correct. On the desktop, folder workspace mutations now route through the runtime's notifyFolderWorkspaceChanged(), which both invalidates the resolved worktree cache and emits reposChanged into the mobile client-events stream — neither of which happened before. On mobile, the new HostWorktreeRequestGate correctly handles multi-client isolation, coalesces trailing refreshes, and preserves the urgent/non-urgent modal priority distinction; all three gate scenarios are covered by unit tests. The clientRef cleanup return added to the useEffect is a correctness improvement that prevents stale follow-up calls after client change. The fallback path in notifyFolderWorkspaceCatalogChanged (no notifier → direct send) ensures existing callers are unaffected. No race conditions, duplicate notifications, or dropped notifications were identified. Files Needing Attention: No files require special attention. All changed files are well-covered by the new and updated tests.
|
| Filename | Overview |
|---|---|
| mobile/src/worktree/host-worktree-refresh.ts | Adds HostWorktreeRequestGate class and wires fetchWorktrees into the reposChanged handler; gate logic is correct and well-tested across all three priority/queuing scenarios. |
| mobile/app/h/[hostId]/index.tsx | Replaces boolean in-flight guard with HostWorktreeRequestGate, adds queueIfInFlight option to fetchWorktrees, and adds cleanup return to the clientRef effect; all changes are logically correct. |
| src/main/ipc/repos.ts | Adds FolderWorkspaceChangeNotifier type and notifyFolderWorkspaceCatalogChanged helper; folder workspace IPC handlers now route through the runtime notifier instead of sending repos:changed directly, with a correct fallback for callers that pass no notifier. |
| src/main/window/attach-main-window-services.ts | One-line change passing runtime as the folderWorkspaceChangeNotifier; wiring is verified by the accompanying test. |
| mobile/src/worktree/host-worktree-refresh.test.ts | Adds comprehensive HostWorktreeRequestGate unit tests covering coalescing, modal-priority preservation, and urgent-vs-ordinary gating; also updates startHostWorktreeRefresh test for the new reposChanged behavior. |
| src/main/ipc/repos-folder-workspace.test.ts | New test file verifying that create/update/delete IPC handlers invoke the notifier and watcher sync without directly calling repos:changed; also covers the no-op path for missing targets. |
| src/main/window/attach-main-window-services.test.ts | Adds notifyFolderWorkspaceChanged stub to the runtime stub and a test verifying the runtime is passed as the third argument to registerRepoHandlers. |
Sequence Diagram
sequenceDiagram
participant Desktop as Desktop IPC Handler<br/>(repos.ts)
participant Runtime as OrcaRuntime
participant DesktopRenderer as Desktop Renderer
participant MobileClient as Mobile Client<br/>(clientEvents stream)
participant MobileScreen as HostScreen<br/>(index.tsx)
note over Desktop,MobileScreen: Before this PR — reposChanged not emitted for folder workspace mutations
Desktop->>Desktop: folderWorkspaces:create / update / delete
Desktop->>DesktopRenderer: mainWindow.webContents.send("repos:changed")
note over MobileClient,MobileScreen: Mobile never received reposChanged → stale catalog
note over Desktop,MobileScreen: After this PR
Desktop->>Runtime: notifyFolderWorkspaceChanged()
Runtime->>Runtime: invalidateResolvedWorktreeCache()
Runtime->>Runtime: notifyReposChanged()
Runtime->>DesktopRenderer: notifier.reposChanged() → "repos:changed"
Runtime->>MobileClient: "emitClientEvent({ type: "reposChanged" })"
MobileClient->>MobileScreen: event stream message
MobileScreen->>MobileScreen: "fetchWorktrees({ allowDuringModal:true, queueIfInFlight:true })"
note over MobileScreen: HostWorktreeRequestGate: begin() → true (or queue)
MobileScreen->>Runtime: "worktree.ps { limit: 10000 }"
Runtime-->>MobileScreen: "{ worktrees: [...] }"
MobileScreen->>MobileScreen: isSuperseded? → apply or discard
MobileScreen->>MobileScreen: setWorktrees / setCachedWorktrees
note over MobileScreen: Folder workspace now visible in Home catalog
MobileScreen->>MobileScreen: "fetchRepoMetadata({ force:true, queueIfInFlight:true })"
Reviews (5): Last reviewed commit: "fix(mobile): preserve urgent catalog ref..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 935c8076-0327-4aeb-abc6-fb09ecbb10bf
📒 Files selected for processing (2)
src/main/ipc/repos-folder-workspace.test.tssrc/main/window/attach-main-window-services.test.ts
| it('does not publish catalog notifications for missing update or delete targets', async () => { | ||
| mockStore.updateFolderWorkspace.mockReturnValue(null) | ||
| mockStore.removeFolderWorkspace.mockReturnValue(false) | ||
|
|
||
| await handlers.get('folderWorkspaces:update')?.(null, { | ||
| folderWorkspaceId: 'missing-folder', | ||
| updates: { name: 'Missing folder' } | ||
| }) | ||
| await handlers.get('folderWorkspaces:delete')?.(null, { | ||
| folderWorkspaceId: 'missing-folder' | ||
| }) | ||
|
|
||
| expect(folderWorkspaceChangeNotifier.notifyFolderWorkspaceChanged).not.toHaveBeenCalled() | ||
| expect(scheduleWatcherSyncMock).not.toHaveBeenCalled() | ||
| expect(mainWindow.webContents.send).not.toHaveBeenCalledWith('repos:changed') | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the missing-target test execute both handlers.
When either handler is missing, optional chaining skips the call and the negative assertions still pass. The test also does not prove that either store mutation was attempted.
Capture both handlers, assert that they are defined, call them directly, and assert that both store methods were called once.
Proposed test fix
+ const update = handlers.get('folderWorkspaces:update')
+ const remove = handlers.get('folderWorkspaces:delete')
+ expect(update).toBeDefined()
+ expect(remove).toBeDefined()
+
- await handlers.get('folderWorkspaces:update')?.(null, {
+ await update!(null, {
folderWorkspaceId: 'missing-folder',
updates: { name: 'Missing folder' }
})
- await handlers.get('folderWorkspaces:delete')?.(null, {
+ await remove!(null, {
folderWorkspaceId: 'missing-folder'
})
+ expect(mockStore.updateFolderWorkspace).toHaveBeenCalledOnce()
+ expect(mockStore.removeFolderWorkspace).toHaveBeenCalledOnce()📝 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.
| it('does not publish catalog notifications for missing update or delete targets', async () => { | |
| mockStore.updateFolderWorkspace.mockReturnValue(null) | |
| mockStore.removeFolderWorkspace.mockReturnValue(false) | |
| await handlers.get('folderWorkspaces:update')?.(null, { | |
| folderWorkspaceId: 'missing-folder', | |
| updates: { name: 'Missing folder' } | |
| }) | |
| await handlers.get('folderWorkspaces:delete')?.(null, { | |
| folderWorkspaceId: 'missing-folder' | |
| }) | |
| expect(folderWorkspaceChangeNotifier.notifyFolderWorkspaceChanged).not.toHaveBeenCalled() | |
| expect(scheduleWatcherSyncMock).not.toHaveBeenCalled() | |
| expect(mainWindow.webContents.send).not.toHaveBeenCalledWith('repos:changed') | |
| }) | |
| it('does not publish catalog notifications for missing update or delete targets', async () => { | |
| mockStore.updateFolderWorkspace.mockReturnValue(null) | |
| mockStore.removeFolderWorkspace.mockReturnValue(false) | |
| const update = handlers.get('folderWorkspaces:update') | |
| const remove = handlers.get('folderWorkspaces:delete') | |
| expect(update).toBeDefined() | |
| expect(remove).toBeDefined() | |
| await update!(null, { | |
| folderWorkspaceId: 'missing-folder', | |
| updates: { name: 'Missing folder' } | |
| }) | |
| await remove!(null, { | |
| folderWorkspaceId: 'missing-folder' | |
| }) | |
| expect(mockStore.updateFolderWorkspace).toHaveBeenCalledOnce() | |
| expect(mockStore.removeFolderWorkspace).toHaveBeenCalledOnce() | |
| expect(folderWorkspaceChangeNotifier.notifyFolderWorkspaceChanged).not.toHaveBeenCalled() | |
| expect(scheduleWatcherSyncMock).not.toHaveBeenCalled() | |
| expect(mainWindow.webContents.send).not.toHaveBeenCalledWith('repos:changed') | |
| }) |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b002b283-38f8-44cb-adc8-a6277d5cc8e8
📒 Files selected for processing (3)
mobile/app/h/[hostId]/index.tsxmobile/src/worktree/host-worktree-refresh.test.tsmobile/src/worktree/host-worktree-refresh.ts
1 participant
Summary
Fix Orca Mobile Home so a runtime-created folder workspace can enter the existing workspace catalog as soon as its host publishes
reposChanged.This is a separate catalog invalidation fix from #11751. It does not change native-chat routing or readability.
Before / after
Before: a real folder workspace could exist on a paired host and own a terminal, while Mobile Home kept showing the stale four-worktree list. Search used that same stale catalog, and the
reposChangedpath refreshed only decorative repo metadata.After:
reposChangedrefreshes both the authoritativeworktree.pscatalog snapshot and repo metadata. Folder workspaces and Git worktrees continue through the same host-scoped catalog and existing search behavior.Screenshots
Exact-head iPad QA is authorized and pending after PR creation. Useful screenshots will be uploaded as PR attachments only; none are committed.
Native QA gap
Not yet run at PR opening. The authorized lane is UDID
C589AF84-2F0C-4978-952B-748933DE6804on existing port3100and helper PID10152. This section will be updated with exact source, artifact, and installed executable hashes plus runtime results. No other simulator or helper will be touched.Testing
git diff --checkpassed.origin/main651f707ce08bf0fb18961add08d835612098e762.AI Review Report
Fresh same-model
gpt-5.6-solxhigh review on the exact current-main tree returnedCLEAN. The review traced the runtime event source and Mobile catalog authority and checked multi-host isolation, SSH/relay behavior, folder-workspace and Git-worktree handling, search semantics, reconnect behavior, bounded refresh work, performance risk, UI scope, and basic security risk.Security Audit
No protocol, authentication, authorization, credential, storage, dependency, or executable changes. The event triggers an existing read-only host-scoped RPC through existing connection and in-flight guards. Both
pnpm-lock.yamlandmobile/pnpm-lock.yamlmatch current main byte-for-byte.Notes
The effective diff is two files and 5 additions / 1 deletion. It adds no polling, timers, reconnect loops, caches, dependencies, UI changes, screenshots, or lockfile changes. The PR is intentionally scoped to the Mobile catalog refresh lifecycle and must not be merged as part of #11751.