feat(web): mobile UX parity with desktop interface

[chr1syy]

Summary

• Cue automation: Add useCue hook and CuePanel mobile component for managing Cue subscriptions and viewing automation activity
• Usage Dashboard & Achievements: Add UsageDashboardPanel and AchievementsPanel with token/cost analytics and read-only achievement display
• Group Chat: Add GroupChatPanel and GroupChatSetupSheet for multi-agent conversations on mobile
• Context Management: Add ContextManagementSheet for viewing/managing agent context on mobile
• Auto Run: Add AutoRunPanel, AutoRunSetupSheet, and AutoRunDocumentViewer for mobile Auto Run configuration
• Git integration: Add GitStatusPanel and GitDiffViewer for inline git status and diff viewing
• Settings & Notifications: Add SettingsPanel and NotificationSettingsSheet with full mobile settings support
• Navigation overhaul: Expand App.tsx with RightDrawer, SessionPillBar, AllSessionsView, and AgentCreationSheet for improved mobile navigation
• Backend WebSocket operations: Add message handlers for usage stats, achievements, cue, group chat, auto run, context management, and notifications
• Hook improvements: Update useGroupChat, useCue, useSettings, useWebSocket, useGitStatus, and others for better state management
• Formatting: Fix Prettier across all web/mobile files

Test plan

• All 602 test files pass (23,853 tests)
• TypeScript type checking passes (all tsconfig targets)
• ESLint passes
• Prettier passes (full repo)
• Production build succeeds
• Manual testing of mobile web interface

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Auto Run: browse, view, edit, save docs; launch/stop runs; mobile viewer & setup
  • Sessions & Groups: create/rename/delete agents and groups, move sessions between groups
  • Git: status and diff viewer
  • Group chat: create, message, live chat panels
  • Cue automation, usage dashboard, achievements, settings, enhanced notifications, command palette, context operations, mobile right-drawer and many panels

• Tests

  • Expanded mocks and updated touch/long-press tests to cover new remote and mobile flows

[coderabbitai]

📝 Walkthrough

Walkthrough

Added broad web-UI parity: new preload IPC request/response handlers, expanded WebServer callback setters and broadcasts, many new WebSocket message types and request/response correlation, renderer listeners and hooks for auto-run/settings/groups/git/chat/context/Cue/usage/achievements, numerous mobile UI panels/sheets, and matching test mock updates.

Changes

Cohort / File(s)	Summary
Core Types & Contracts src/main/web-server/types.ts	Added ~40 new exported types and callback signatures for settings, groups, auto-run docs, git, group chat, context ops, Cue, usage, and achievements.
WebServer Runtime & Callback Registry src/main/web-server/WebServer.ts, src/main/web-server/managers/CallbackRegistry.ts	Added 30+ new set*Callback setters, safe wrapper getters, and many broadcast methods for notifications, settings, groups, auto-run, chat, context, Cue, usage, achievements.
WebSocket Message Handling src/main/web-server/handlers/messageHandlers.ts	Added 20+ new message routes with input validation (including filename sanitization), typed responses and errors, allowlists, and extended MessageHandlerCallbacks.
Broadcast Service src/main/web-server/services/broadcastService.ts	Added broadcast APIs for notifications/settings/groups/autorun/chat/context/cue and tracking of previous autorun state to emit task/complete events and session-state notifications.
Preload IPC & Global API Types src/main/preload/process.ts, src/renderer/global.d.ts	Implemented many onRemote*/sendRemote*Response request-response pairs and updated global MaestroAPI.process typings.
Web-server Factory Wiring & Tests src/main/web-server/web-server-factory.ts, src/__tests__/main/web-server/web-server-factory.test.ts	Wired request-response callbacks (random response channels, timeouts) into factory; expanded test mock to include new setter methods.
WebSocket Hook & Request Correlation src/web/hooks/useWebSocket.ts, src/web/hooks/index.ts	Added pending-request tracking, sendRequest with timeouts, new server message types and broadcast handlers, and re-exported new hooks/types.
Renderer Integration & Remote Handlers src/renderer/App.tsx, src/renderer/hooks/remote/useRemoteIntegration.ts	Registered many window/custom-event listeners for auto-run/settings/sessions/groups/git/chat/context/Cue/achievements and wired responses via process.sendRemote*Response.
New Web Hooks src/web/hooks/* (useAutoRun.ts,useSettings.ts,useGitStatus.ts,useGroupChat.ts,useCue.ts,useAgentManagement.ts)	Added hooks providing request/response flows over WebSocket for auto-run, settings, git status/diff, group chat, Cue, and agent/group management.
Long-press / Command Palette src/web/hooks/useLongPressMenu.ts,src/web/hooks/useMobileKeyboardHandler.ts,src/web/mobile/QuickActionsMenu.tsx,src/web/mobile/CommandInputBar.tsx	Refactored long-press to open command palette; replaced anchored quick-actions with searchable dialog; added Cmd/Ctrl+K & Escape handling and test updates.
Mobile Panels & UI Components src/web/mobile/*.tsx (AutoRun, RightDrawer, GitStatus/GitDiff, GroupChat, Cue, Achievements, Agent Creation, Context Management, Notification Settings, etc.)	Added 13+ mobile components/panels/sheets with UI, state, and sendRequest wiring (AutoRun viewer/setup/panel, RightDrawer, Git panels, GroupChat, CuePanel, Achievements, ContextManagement, AgentCreation, NotificationSettings, etc.).
App Integration & Small Changes src/web/mobile/App.tsx, src/renderer/stores/agentStore.ts, src/renderer/hooks/batch/useBatchHandlers.ts	Integrated new panels and handlers into mobile app, minor agentStore cleanup, and exposed stopBatchRun in batch handlers.
Tests & Test Adjustments src/__tests__/web/*, src/__tests__/web/mobile/*, src/__tests__/renderer/hooks/*	Updated many tests: expanded sendRequest/process mocks, switched interaction tests to touch/long-press flows, updated QuickActions → CommandPalette expectations, and added new mock handlers.

Sequence Diagram(s)

sequenceDiagram
    participant UI as Renderer (UI)
    participant Preload as Preload/process
    participant Main as Main (WebServer)
    participant Broadcast as BroadcastService

    UI->>Preload: onRemoteCreateSession(payload, responseChannel)
    Preload->>Main: ipc-> "remote:create_session" (responseChannel)
    Main->>Main: CallbackRegistry.createSession(...) (app logic)
    Main-->>Preload: sendRemoteCreateSessionResponse(responseChannel, { sessionId }|null)
    Preload-->>UI: deliver response via sendRemoteCreateSessionResponse
    Main->>Broadcast: broadcastGroupsChanged(...) / broadcastSettingsChanged(...)
    Broadcast-->>UI: WebSocket broadcast -> client handlers (onGroupsChanged/onSettingsChanged)

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• Add CLI IPC commands for remote Maestro control #530 — overlaps IPC/preload and WebServer message/callback additions for auto-run and remote session/file operations.
• fix: conform merged main changes to refactored code #462 — modifies renderer remote-integration and session handling; likely to overlap with added useRemoteIntegration handlers and tests.
• refactor: centralize agent config state management and fix context groomer spawning (#316, #317) #521 — touches context-management wiring (merge/transfer/summarize) that overlaps with new context callbacks and handlers.

Suggested labels

runmaestro.ai

Poem

🐰 I hopped through channels, quick and neat,

I stitched requests and responses to greet,
Docs, groups, git, and chats tied in a bow,
Long-press brings palettes, haptics set aglow,
Thump-thump — carrot-coded, ready to go!

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

[greptile-apps]

Greptile Summary

This PR adds substantial mobile UX parity for the Maestro web interface — introducing a new sendRequest request-response layer over WebSocket, ~25 new backend message handlers, and a full suite of mobile panels and hooks (Cue, Group Chat, Auto Run, Git, Settings, Achievements, Context Management). The architecture is well-structured: a typed CallbackRegistry, a clean sendRequest/correlation pattern in useWebSocket, and consistent optimistic-update + broadcast patterns across the new hooks.

Issues found:

• sendError never echoes requestId — all ~25 new handler error paths send { type: 'error' } without the originating correlation ID, causing every server-side error to hang the client for 10 seconds instead of failing fast.
• autoScroll and notificationsEnabled read/write key mismatch — getSettings reads from autoScrollAiMode / osNotificationsEnabled but set_setting writes to autoScroll / notificationsEnabled, so toggling either setting silently reverts after reconnect.
• Optimistic update not rolled back on success: false — useSettings.setSetting only rolls back on exception, not on an explicit server rejection.
• sendRequest resolves rather than rejects error responses — the correlation handler unconditionally resolves, requiring callers to inspect response.type manually instead of relying on catch.

Confidence Score: 4/5

Safe to merge after addressing the sendError requestId omission and the two settings key mismatches.

Two P1 findings: the sendError/requestId gap causes 10s frozen UI on any server-side error across all new request types, and the autoScroll/notificationsEnabled key mismatch silently drops setting changes on reconnect. Neither causes data loss or security risk, but both are present defects on the changed path.

src/main/web-server/handlers/messageHandlers.ts (sendError missing requestId) and src/main/web-server/web-server-factory.ts (autoScroll/notificationsEnabled key mismatch)

Important Files Changed

Filename	Overview
src/main/web-server/handlers/messageHandlers.ts	Adds ~25 new message handlers; all error paths call sendError without requestId, causing 10s client timeouts on every server-side failure.
src/main/web-server/web-server-factory.ts	Wires new callbacks; getSettings reads autoScrollAiMode/osNotificationsEnabled but set_setting writes to autoScroll/notificationsEnabled — persistent key mismatch for two settings.
src/web/hooks/useWebSocket.ts	Introduces sendRequest with requestId correlation and pending map; cleanup on disconnect is correct, but resolves error-typed responses instead of rejecting them.
src/web/hooks/useSettings.ts	New settings hook with optimistic update; rollback only on exception, not on success:false, leaving UI out of sync on validation failures.
src/web/mobile/App.tsx	Major navigation overhaul adding RightDrawer, SessionPillBar, AllSessionsView, AgentCreationSheet, and feature panels; header refactored to hamburger + icon bar with overflow menu.
src/renderer/hooks/remote/useRemoteIntegration.ts	Adds IPC listeners for remote auto-run, git, settings, and session/group management; each effect has proper cleanup and CustomEvent dispatch.
src/main/web-server/managers/CallbackRegistry.ts	Extends callback registry with 25 new typed callbacks and consistent fallback defaults.

Sequence Diagram

sequenceDiagram
    participant Mobile as Mobile Web Client
    participant Hook as useWebSocket
    participant Handler as messageHandlers
    participant Factory as web-server-factory
    participant Desktop as Desktop Renderer

    Mobile->>Hook: sendRequest set_setting autoScroll true
    Hook->>Handler: WebSocket message with correlation ID
    Handler->>Handler: Validate key against allowlist
    Handler->>Factory: setSetting callback
    Factory->>Desktop: IPC sends autoScroll key
    Desktop-->>Factory: success response
    Factory-->>Handler: resolved
    Handler-->>Mobile: type=set_setting_result with correlation ID
    Note over Factory,Desktop: BUG reads autoScrollAiMode but writes autoScroll

    Handler->>Mobile: type=error no correlation ID
    Note over Hook: sendRequest waits full 10s timeout before rejecting

Loading

Comments Outside Diff (2)

1. src/main/web-server/handlers/messageHandlers.ts, line 416-500 (link)

   sendError omits requestId, causing sendRequest timeouts instead of fast failures

   When any validation or execution error occurs in the new handlers (e.g., missing sessionId, unregistered callback, thrown exception), sendError sends a bare { type: 'error', message: '...' } without the originating requestId. In useWebSocket.ts, the sendRequest correlation logic only intercepts messages that carry back the requestId:

   const requestId = (message as any).requestId as string | undefined;
   if (requestId && pendingRequestsRef.current.has(requestId)) {
       pending.resolve(message);
       return;
   }

   The case 'error': block just calls setError and onError — it does not reject any pending sendRequest promise. So every error path in the ~25 new handlers causes the client to silently wait until the timeoutMs (default 10 s) fires, making the UI appear hung.

   Fix: thread message.requestId through sendError so the client can fast-fail:

   // Replace:
   this.sendError(client, 'Missing sessionId');
   // With:
   this.sendError(client, 'Missing sessionId', { requestId: message.requestId });

   This affects every new handler (handleGetAutoRunDocs, handleSaveAutoRunDocument, handleStopAutoRun, handleGetSettings, handleSetSetting, handleCreateSession, handleGetGroups, handleGetGitStatus, handleStartGroupChat, handleGetCueSubscriptions, etc.).

2. src/main/web-server/web-server-factory.ts, line 153-155 (link)

   Settings read/write key mismatch for autoScroll and notificationsEnabled

   getSettings maps two fields to different electron-store keys than what set_setting writes:

   WebSettings field	Read key	Write key
   autoScroll	'autoScrollAiMode'	'autoScroll'
   notificationsEnabled	'osNotificationsEnabled'	'notificationsEnabled'

   The write lands on a different store key than the read, so toggling either setting from mobile appears to succeed but reverts after page reload or reconnect.

   Fix — align the write keys with the read keys:

   // messageHandlers.ts ALLOWED_SETTING_KEYS:
   // 'autoScroll'           -> 'autoScrollAiMode'
   // 'notificationsEnabled' -> 'osNotificationsEnabled'
   
   // useSettings.ts SETTING_KEY_TO_FIELD:
   // autoScrollAiMode: 'autoScroll',
   // osNotificationsEnabled: 'notificationsEnabled',
   
   // useSettings.ts typed setters:
   const setAutoScroll = useCallback(
       (value: boolean) => setSetting('autoScrollAiMode', value), [setSetting]);
   const setNotificationsEnabled = useCallback(
       (value: boolean) => setSetting('osNotificationsEnabled', value), [setSetting]);

_{Reviews (1): Last reviewed commit: "style: fix Prettier formatting in 5 file..." | Re-trigger Greptile}

[greptile-apps]

Optimistic update not rolled back when server returns success: false

The rollback only runs in the catch block. If the server responds with { success: false }, the optimistic update persists and the UI shows a value the backend rejected.

try {
    const response = await sendRequest<{ success?: boolean }>('set_setting', { key, value });
    if (!response.success) {
        if (prev) setSettings(prev); // roll back on explicit failure
    }
    return response.success ?? false;
} catch {
    if (prev) setSettings(prev);
    return false;
}

[greptile-apps]

sendRequest resolves error responses rather than rejecting them

The correlation handler unconditionally calls pending.resolve(message) even when type === 'error'. Rejecting on error-typed responses lets every hook's catch block handle server errors uniformly:

if (requestId && pendingRequestsRef.current.has(requestId)) {
    clearTimeout(pending.timer);
    pendingRequestsRef.current.delete(requestId);
    if ((message as any).type === 'error') {
        pending.reject(new Error((message as any).message ?? 'Server error'));
    } else {
        pending.resolve(message);
    }
    return;
}

[coderabbitai]

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/web/mobile/AllSessionsView.tsx (1)

191-252: ⚠️ Potential issue | 🟠 Major

Don't mount the rename editor inside the card button.

When isRenaming is true, this renders an <input> inside the outer session <button>. That's invalid interactive nesting and tends to break focus, click, and screen-reader behavior. Swap the card container away from button semantics while editing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/AllSessionsView.tsx` around lines 191 - 252, The session card
currently renders the rename <input> inside the outer <button> (isRenaming
branch), which creates invalid interactive nesting; update the render so the
interactive <input> (renameInputRef, renameValue, handleRenameKeyDown,
onRenameConfirm) is not mounted inside the button: when isRenaming is true,
render the card container as a non-button element (e.g., <div> or a separate
wrapper) or move the <input> out of the <button> so the outer element is a
<button> only when not renaming; ensure you preserve click/touch handlers
(handleClick, handleTouchStart/End/Move/Cancel, handleContextMenu), aria
attributes (aria-pressed/aria-label), styling and focus handling, and keep the
same behavior for entering/exiting rename mode (isRenaming, renameInputRef) and
stopping propagation on input events.

🟡 Minor comments (7)

src/web/mobile/AutoRunIndicator.tsx-40-42 (1)

40-42: ⚠️ Potential issue | 🟡 Minor

Missing keyboard handler for accessible button behavior.

When onTap is provided, the element receives role="button" and tabIndex={0}, making it focusable. However, keyboard users won't be able to activate it via Enter or Space since only onClick is wired. Add an onKeyDown handler for complete accessibility.

♿ Proposed fix to add keyboard support

 		<div
 			onClick={onTap}
+			onKeyDown={
+				onTap
+					? (e: React.KeyboardEvent) => {
+							if (e.key === 'Enter' || e.key === ' ') {
+								e.preventDefault();
+								onTap();
+							}
+						}
+					: undefined
+			}
 			role={onTap ? 'button' : undefined}
 			tabIndex={onTap ? 0 : undefined}

Based on learnings: "For focus issues, add tabIndex={0} or tabIndex={-1}, add outline-none class" — the tabIndex is handled correctly, but interactive elements need full keyboard support.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/AutoRunIndicator.tsx` around lines 40 - 42, AutoRunIndicator
currently sets role="button" and tabIndex when onTap is provided but only uses
onClick; add a keyboard handler so Enter and Space activate the same action:
implement an onKeyDown handler (e.g., handleKeyDown) in the AutoRunIndicator
component that, when onTap is present, listens for key === 'Enter' or key === '
' / code === 'Space' and calls onTap (and calls event.preventDefault() for Space
to avoid page scroll); wire this handler into the element via onKeyDown={onTap ?
handleKeyDown : undefined} so keyboard users can activate the button
consistently with onClick.

src/__tests__/web/mobile/App.test.tsx-515-530 (1)

515-530: ⚠️ Potential issue | 🟡 Minor

Render drawer children in the RightDrawer mock.

This stub drops its children, so the history tests only prove the drawer opens with activeTab="history". A regression where MobileHistoryPanel stops mounting inside the drawer would still pass.

Suggested fix

 vi.mock('../../../web/mobile/RightDrawer', () => ({
 	RightDrawer: ({
 		activeTab,
 		onClose,
+		children,
 	}: {
 		sessionId: string;
 		activeTab?: string;
 		onClose: () => void;
+		children?: React.ReactNode;
 	}) => (
 		<div data-testid="right-drawer" data-active-tab={activeTab}>
+			<div data-testid="right-drawer-body">{children}</div>
 			<button data-testid="close-right-drawer" onClick={onClose}>
 				Close
 			</button>
 		</div>
 	),
 }));

Then assert the history content is actually inside the drawer, e.g. within(drawer).getByTestId('mobile-history-panel').

Also applies to: 1471-1548

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/web/mobile/App.test.tsx` around lines 515 - 530, The
RightDrawer mock currently ignores its children which prevents mounted drawer
content from being asserted; update the mock for RightDrawer (the
function/component named RightDrawer in the test) to accept and render its
children inside the returned div (preserve data-testid="right-drawer" and
data-active-tab) and then update the history-related assertions to check the
actual mounted panel inside the drawer (e.g., use
within(drawer).getByTestId('mobile-history-panel') or similar) so the tests
verify MobileHistoryPanel is mounted inside RightDrawer.

src/web/hooks/useMobileKeyboardHandler.ts-58-63 (1)

58-63: ⚠️ Potential issue | 🟡 Minor

Guard the palette shortcuts before calling preventDefault().

These props are optional, but Cmd/Ctrl+K and Escape always consume the event first. Reusing this hook without palette handlers will hijack the shortcut and still do nothing.

Also applies to: 87-103

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/hooks/useMobileKeyboardHandler.ts` around lines 58 - 63, In
useMobileKeyboardHandler, avoid always calling
event.preventDefault()/stopPropagation for the command-palette shortcuts; first
check the relevant optional props and state (onOpenCommandPalette,
onCloseCommandPalette, isCommandPaletteOpen) and only consume the event when
there is an actual handler to run: for Cmd/Ctrl+K only call
preventDefault/stopPropagation and invoke onOpenCommandPalette when
onOpenCommandPalette is defined (and skip if it's undefined), and for Escape
only consume the event and call onCloseCommandPalette when onCloseCommandPalette
is defined and isCommandPaletteOpen is true; apply the same guard logic to the
other shortcut handling in the same hook (the block around the other key
handling) so the hook does not hijack keys when handlers are not provided.

src/renderer/hooks/remote/useRemoteIntegration.ts-611-617 (1)

611-617: ⚠️ Potential issue | 🟡 Minor

Use the destination path for renamed files.

git status --porcelain emits old -> new for renames/copies. Taking the left-hand side here means downstream diff requests ask Git for the stale path, so renamed files won't open the right diff.

💡 Suggested fix

-						const filePath = line.substring(3).split(' -> ')[0];
+						const pathField = line.substring(3);
+						const renameParts = pathField.split(' -> ');
+						const filePath = renameParts[renameParts.length - 1] || pathField;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/renderer/hooks/remote/useRemoteIntegration.ts` around lines 611 - 617,
The current mapping of statusLines to files uses the left-hand path for
renames/copies, causing diffs to request the old path; update the mapping in
useRemoteIntegration (the statusLines => files block) to detect the " -> " token
and, when present, use the destination/right-hand path as filePath (trimmed)
instead of the source; keep existing logic for status and staged determination
(status, staged variables) but ensure filePath is the new path for renamed
entries so downstream diff requests target the correct file.

src/web/mobile/AutoRunSetupSheet.tsx-29-42 (1)

29-42: ⚠️ Potential issue | 🟡 Minor

Reinitialize the draft when this sheet is reused for another session.

selectedFiles, prompt, and the loop settings are only seeded on first mount. If sessionId changes while this sheet stays mounted, the previous session's config can be launched against the new one.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/AutoRunSetupSheet.tsx` around lines 29 - 42, AutoRunSetupSheet
currently seeds selectedFiles, prompt, loopEnabled and maxLoops only on initial
mount, so when sessionId (_sessionId) changes while the component stays mounted
the old draft remains; add a useEffect inside AutoRunSetupSheet that watches
_sessionId (and documents) and reinitializes state by calling
setSelectedFiles(new Set(documents.map(d => d.filename))), setPrompt(''),
setLoopEnabled(false) and setMaxLoops(3) (and optionally setIsVisible(false) if
you want to reset visibility) to ensure a fresh draft for each new session.

src/web/mobile/AutoRunPanel.tsx-216-220 (1)

216-220: ⚠️ Potential issue | 🟡 Minor

Hide the task counter until totals are known.

Defaulting missing counts to 0 makes the running banner render Task 1/0 during startup or partial state updates. Only show the task/doc text once the corresponding totals are greater than zero.

Also applies to: 374-380

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/AutoRunPanel.tsx` around lines 216 - 220, The banner should
not render "Task 1/0" during startup — stop defaulting totalCounts to 0 and
instead only show task/doc counters when the corresponding totals are known
(>0). Update the variable handling for totalTasks, completedTasks and
currentTaskIndex (from autoRunState?.totalTasks ?? 0 etc.) so totals remain
undefined/null when missing, compute progress only if totalTasks > 0, and change
the render logic that uses isRunning, progress, currentTaskIndex, totalTasks and
completedTasks to conditionally display the "Task X/Y" (and similar doc
counters) only when the relevant total is > 0; apply the same pattern for the
other occurrence that uses these same symbols.

src/main/web-server/managers/CallbackRegistry.ts-303-305 (1)

303-305: ⚠️ Potential issue | 🟡 Minor

Use an explicit unavailable state for unregistered content callbacks.

Lines 304 and 392 return valid empty payloads, so callers cannot distinguish "callback not wired" from "empty document / no diff". That makes registration bugs look like real data. Prefer null/undefined or an exception here so the UI can show an unavailable state instead of a misleading empty one.

Also applies to: 391-393

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/web-server/managers/CallbackRegistry.ts` around lines 303 - 305, The
methods currently return an empty string when the callback is not registered,
which hides "unavailable" vs "empty" states; update getAutoRunDocContent (and
the other similar method at lines 391-393) to return null when the corresponding
callback is not wired: change the return type from Promise<string> to
Promise<string | null>, have the early branch return null (or throw a
descriptive Error if you prefer explicit failure), and update any callers/type
annotations to handle null (or the thrown error) instead of assuming an empty
string.

🧹 Nitpick comments (4)

src/__tests__/main/web-server/web-server-factory.test.ts (1)

44-72: Add registration assertions for the new set*Callback methods.

The mock now includes the full parity surface, but the registration section still only checks the older callbacks. If web-server-factory.ts stops wiring any of the newly added setters, this file will still pass.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/main/web-server/web-server-factory.test.ts` around lines 44 -
72, The test's registration assertions are missing checks for the newly added
setter mocks (e.g., setGetAutoRunDocsCallback, setGetAutoRunDocContentCallback,
setSaveAutoRunDocCallback, setStopAutoRunCallback, setGetSettingsCallback,
setSetSettingCallback, setGetGroupsCallback, setCreateGroupCallback,
setRenameGroupCallback, setDeleteGroupCallback, setMoveSessionToGroupCallback,
setCreateSessionCallback, setDeleteSessionCallback, setRenameSessionCallback,
setGetGitStatusCallback, setGetGitDiffCallback, setGetGroupChatsCallback,
setStartGroupChatCallback, setGetGroupChatStateCallback,
setStopGroupChatCallback, setSendGroupChatMessageCallback,
setMergeContextCallback, setTransferContextCallback,
setSummarizeContextCallback, setGetCueSubscriptionsCallback,
setToggleCueSubscriptionCallback, setGetCueActivityCallback,
setGetUsageDashboardCallback, setGetAchievementsCallback); update the
registration/assertions block in web-server-factory.test.ts to assert each of
these setters was called (or called with the expected handler) so the test fails
if web-server-factory.ts stops wiring any of them. Ensure you add one assertion
per new setter using the same pattern as the existing registration assertions.

src/__tests__/renderer/hooks/useRemoteIntegration.test.ts (1)

138-190: Assert that the new remote channels are actually registered.

This mock expansion prevents crashes, but the suite still passes if useRemoteIntegration forgets to subscribe to any of these new onRemote* handlers because none of them are asserted after renderHook(). A small table-driven expectation would make the new wiring fail loudly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/renderer/hooks/useRemoteIntegration.test.ts` around lines 138 -
190, Add assertions after calling renderHook(useRemoteIntegration) that verify
each new onRemote* handler was registered: create a list of the handler symbols
(onRemoteGetAutoRunDocs, onRemoteGetAutoRunDocContent, onRemoteSaveAutoRunDoc,
onRemoteStopAutoRun, onRemoteSetSetting, onRemoteCreateSession,
onRemoteDeleteSession, onRemoteRenameSession, onRemoteCreateGroup,
onRemoteRenameGroup, onRemoteDeleteGroup, onRemoteMoveSessionToGroup,
onRemoteGetGitStatus, onRemoteGetGitDiff) and assert in a table-driven loop that
each corresponding mock (the onRemote* function on the mocked ipcRenderer) was
called/used by renderHook(useRemoteIntegration) so missing subscriptions fail
the test loudly.

src/web/hooks/useNotifications.ts (1)

232-244: Reuse the existing notification icon default here.

showNotification() already centralizes the icon/badge assets, but handleNotificationEvent() overrides the icon with a second hardcoded path. Keeping two asset paths here makes event notifications drift or break independently.

Also applies to: 357-361

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/hooks/useNotifications.ts` around lines 232 - 244, showNotification
centralizes the icon/badge defaults but handleNotificationEvent currently
hardcodes a different icon path causing drift; update handleNotificationEvent to
reuse the same default icon/badge used by showNotification (extract or reference
the shared default values used when creating Notification in showNotification)
so both functions use the same asset variables (e.g., the default icon/badge
constant or the same options object) rather than duplicating the hardcoded
'/maestro-icon-192.png' path.

src/web/hooks/useLongPressMenu.ts (1)

37-42: Trim the unused mode props from this hook's public API.

inputMode is still required in UseLongPressMenuOptions, but this hook no longer reads it. Keeping it here forces callers to thread dead state through every usage.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/hooks/useLongPressMenu.ts` around lines 37 - 42, Remove the dead
inputMode prop from the public API by deleting inputMode from the
UseLongPressMenuOptions interface and updating all related typings/usages; keep
onModeToggle (and its signature) as the only mode-related prop, update the
useLongPressMenu hook signature/type references (UseLongPressMenuOptions) and
any callers to stop passing or expecting inputMode, and run the typechecker to
fix any remaining places that were threading inputMode through the codebase.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/web/mobile/AllSessionsView.tsx`:
- Around line 191-252: The session card currently renders the rename <input>
inside the outer <button> (isRenaming branch), which creates invalid interactive
nesting; update the render so the interactive <input> (renameInputRef,
renameValue, handleRenameKeyDown, onRenameConfirm) is not mounted inside the
button: when isRenaming is true, render the card container as a non-button
element (e.g., <div> or a separate wrapper) or move the <input> out of the
<button> so the outer element is a <button> only when not renaming; ensure you
preserve click/touch handlers (handleClick, handleTouchStart/End/Move/Cancel,
handleContextMenu), aria attributes (aria-pressed/aria-label), styling and focus
handling, and keep the same behavior for entering/exiting rename mode
(isRenaming, renameInputRef) and stopping propagation on input events.

---

Major comments:
In `@src/__tests__/web/mobile/QuickActionsMenu.test.tsx`:
- Around line 24-42: The test replaces window.localStorage with localStorageMock
and never restores it; save the original storage (e.g., const
originalLocalStorage = window.localStorage) before calling
Object.defineProperty(window, 'localStorage', ...) and restore it in an afterAll
hook (restore window.localStorage = originalLocalStorage), or replace the manual
approach with vi.stubGlobal('localStorage', localStorageMock) so the test
runtime automatically restores the global; update references to
localStorageMock/Object.defineProperty accordingly.

In `@src/main/preload/process.ts`:
- Around line 519-1079: Several IPC listeners in this block either swallow
errors and return empty payloads (e.g., onRemoteGetAutoRunDocs,
onRemoteGetAutoRunDocContent, onRemoteSaveAutoRunDoc, onRemoteGetGitStatus,
onRemoteGetGitDiff) or call the callback directly without ensuring a response on
failure (many onRemote* handlers that just do callback(...)). Update each
listener to follow the onRemoteConfigureAutoRun pattern: import
captureException/captureMessage from the Sentry util, wrap the handler body so
Promise.resolve(callback(...)) is awaited/catched, call
captureException/captureMessage on throw/rejection, and then send a single,
well-typed failure response back via ipcRenderer.send(responseChannel,
<typed-failure>) exactly once (e.g., []/''/false or structured error object
matching the success path) to avoid silent timeouts; apply this change to the
named handlers (onRemoteGetAutoRunDocs, onRemoteGetAutoRunDocContent,
onRemoteSaveAutoRunDoc, onRemoteGetGitStatus, onRemoteGetGitDiff and other
request-response onRemote* functions that accept a responseChannel).

In `@src/main/web-server/handlers/messageHandlers.ts`:
- Around line 1168-1191: The handler handleGetAutoRunState is reading an
autoRunState property from the object returned by callbacks.getSessionDetail
even though SessionDetailForHandler does not expose that field; add a dedicated
callback (e.g., callbacks.getSessionAutoRunState(sessionId): AutoRunState |
null) to the callback interface/CallbackRegistry and implement it in
LiveSessionManager, update the handler to call
callbacks.getSessionAutoRunState(sessionId) (and return/send an error if the
callback is not configured) instead of inspecting detail.autoRunState, and
update types (callback interface and any consumers) so auto-run state is
retrieved via the explicit method rather than by reading an untyped extra field.
- Around line 1136-2158: Validation and catch branches in the request/response
handlers (e.g., handleGetAutoRunDocs, handleGetAutoRunState,
handleGetAutoRunDocument, handleSaveAutoRunDocument, handleStopAutoRun,
handleSetSetting, handleCreateSession, handleDeleteSession, handleRenameSession,
handleMoveSessionToGroup, handleGetGitStatus, handleGetGitDiff,
handleGetGroupChats, handleStartGroupChat, handleGetGroupChatState,
handleSendGroupChatMessage, handleStopGroupChat, handleMergeContext,
handleTransferContext, handleSummarizeContext, handleGetCueSubscriptions,
handleToggleCueSubscription, handleGetCueActivity, handleGetUsageDashboard,
handleGetAchievements) currently call sendError without the original requestId;
update every validation failure and every .catch(...) to include
message.requestId in the sendError payload (or implement a small helper wrapper
around sendError that always adds message.requestId) so the client receives
error replies that match the original requestId.

In `@src/main/web-server/managers/CallbackRegistry.ts`:
- Around line 366-373: Update the CreateSessionCallback type to include optional
sshRemoteConfig, customPath, customArgs, and customEnvVars, then change the
CallbackRegistry.createSession method signature to accept and forward those same
optional parameters to this.callbacks.createSession; also update the preload API
in src/main/preload/process.ts to accept and pass these new params into the main
createSession call, and modify the remote session handler in
src/renderer/App.tsx so after receiving the created session it populates
customPath, customArgs, customEnvVars and sessionSshRemoteConfig on the session
object before use so spawned agents receive the correct SSH and custom agent
configuration.

In `@src/main/web-server/types.ts`:
- Around line 372-377: The AutoRunDocument struct is keyed only by filename
which causes collisions for files with the same basename; update the model and
callbacks to use a stable document identifier (e.g., add an id:string to
AutoRunDocument or use the full path field as the key) and change all read/save
callback lookups to key by that stable id (replace usages that reference
filename-only to use the new id or path+filename combo), including the other
callback pair noted in the file; ensure the identifier is generated/used
consistently for reads, writes and comparisons.

In `@src/main/web-server/web-server-factory.ts`:
- Around line 1154-1168: The callback registered with
server.setStopAutoRunCallback currently returns true immediately after
mainWindow.webContents.send('remote:stopAutoRun', sessionId) which only confirms
IPC dispatch, not the stop result; change it to perform a request/response
handshake similar to configureAutoRun: when getMainWindow() and
isWebContentsAvailable(...) are OK, send a request with a unique reply channel
or correlation id (e.g., 'remote:stopAutoRun:request' + sessionId) and await the
renderer's acknowledgement or failure (use ipcMain.invoke/ipcRenderer.handle or
send + ipcMain.once with a timeout), then return true only on a positive
renderer response and false on timeout/error so stop_auto_run_result reflects
the actual outcome. Ensure the correlation id and channel names match the
renderer handler (or add a new renderer handler) and include a reasonable
timeout to avoid hanging.
- Around line 741-755: The setSetSettingCallback currently forwards web keys
unchanged so writes from the renderer (via window.maestro.settings.set) end up
saved to wrong internal keys; update the handler registered via
setSetSettingCallback to remap incoming web keys to the internal store keys
before persisting using settingsStore (e.g. map "autoScroll" ->
"autoScrollAiMode" and "notificationsEnabled" -> "osNotificationsEnabled", and
pass through other keys unchanged), or alternatively normalize these keys in the
renderer before calling window.maestro.settings.set; locate the logic around
setSetSettingCallback / getSettings and ensure the same key translations used in
getSettings (autoScrollAiMode → autoScroll, osNotificationsEnabled →
notificationsEnabled) are applied in reverse when writing.

In `@src/renderer/App.tsx`:
- Around line 2201-2233: The deletion handler currently swallows all errors from
window.maestro.process.kill and proceeds to remove the session, which can orphan
processes and leave a stale activeSessionId; update the handler in the useEffect
(the async handler function that reads sessionsRef.current and calls
window.maestro.process.kill for `${sessionId}-ai`, `${sessionId}-terminal`, and
`${sessionId}-terminal-${tab.id}`) to: attempt each kill and treat an "already
exited"/ENOENT-like expected error as benign but report any other error to
Sentry (use your Sentry utilities), abort the session removal if any non-benign
kill error occurs, and when removing the session via setSessions ensure that if
the filtered list is empty you call setActiveSessionId('') (and otherwise update
activeSessionId only if the deleted session was active) so activeSessionId never
remains stale.

In `@src/web/hooks/useAgentManagement.ts`:
- Around line 104-105: The catch blocks in the useAgentManagement hook that
simply return fallback values (e.g., returning `groups`, `false`, or `null`) are
hiding real backend failures; update each such catch (including the ones around
the lines you noted) to only swallow known, recoverable errors (e.g., inspect
the caught error for expected status codes or validation errors) and for any
unexpected error call the Sentry reporting utility (e.g., captureException) and
rethrow the error so it bubbles up; where you do keep a fallback return, log or
report the known condition first and document why it's safe to recover.
- Around line 63-84: The effect sets hasFetchedRef.current = true before the
async sendRequest('get_groups') completes, so a transient failure prevents
future auto-loads; move or guard the flag so it's only set after a successful
response (e.g. set hasFetchedRef.current = true inside the .then when
response.groups is processed) or ensure it is cleared in the .catch/.finally
path; update the useEffect containing isConnected, hasFetchedRef, sendRequest,
setIsLoading and setGroups so failures reset or don't flip hasFetchedRef.current
so retries on reconnect will still occur.

In `@src/web/hooks/useAutoRun.ts`:
- Around line 87-100: In loadDocumentContent, do not overwrite the current
selection with an empty document on any sendRequest failure; instead preserve
the previous selectedDoc or set an explicit error state so transport/backend
errors are distinguishable from an empty file. Modify the catch block in
loadDocumentContent to either return without calling setSelectedDoc (keeping the
existing selection) or call a setter for an error flag (e.g.,
setSelectedDoc(prev => ({ ...prev, filename, loadError: true })) or
setDocumentLoadError(filename)) and log the error from sendRequest so the UI can
show an error message; do not set content: '' in the catch. Ensure references:
loadDocumentContent, sendRequest, and setSelectedDoc are updated accordingly.
- Line 50: The hook's launchAutoRun currently returns a synchronous boolean
after socket.send(), which only queues the frame locally; change launchAutoRun
in useAutoRun to await the server's configure_auto_run_result event and return a
Promise<boolean> that resolves/rejects based on that typed response (or a
timeout/error), i.e., replace the immediate boolean return with logic that
registers a one-time socket.on listener for "configure_auto_run_result"
(matching the request's sessionId), cleans up the listener on resolve/reject,
and maps the server response to true/false (and handle validation/runtime
failure details), and update callers to await the new async signature.

In `@src/web/hooks/useCue.ts`:
- Around line 78-148: Both loadSubscriptions and loadActivity currently share a
single boolean isLoading and clear data on any catch; change them to use a
simple pending counter (e.g., increment/decrement a local ref like
subscriptionsPending/activityPending or a single numeric pendingRef used to
drive isLoading) or separate isLoadingSubscriptions/isLoadingActivity flags so
one request finishing doesn’t flip the global loading state; in each catch
preserve existing cached data (don’t call setSubscriptions([]) or
setActivity([]) on recoverable errors), only clear state when a deliberate empty
result is returned, and explicitly distinguish expected errors vs unexpected
ones by checking the error code/type and rethrowing unexpected exceptions so
Sentry (global error handler) can capture them; update loadSubscriptions,
loadActivity, and the useEffect auto-load logic accordingly and ensure
setIsLoading (or derived isLoading) reflects pending count > 0.

In `@src/web/hooks/useGitStatus.ts`:
- Around line 82-84: In useGitStatus.ts's useGitStatus hook, replace the broad
empty catch blocks around the Git fetch/refresh logic so they do not silently
swallow errors: in each try-catch (the blocks that currently just comment
"Status fetch failed — will retry on next call"), catch and explicitly handle
expected recoverable errors (e.g., network/timeout errors or a specific
NETWORK_ERROR from fetchGitStatus/fetchX), update hook state to reflect a fetch
failure (so the UI can distinguish "no data" vs "request failed"), and for any
non-recoverable/unexpected exceptions rethrow or pass them to the global error
reporter so they reach Sentry; ensure you reference and use the existing
functions/variables in this file (useGitStatus, fetchGitStatus/fetchStatus,
setState/setError) to implement the conditional handling and rethrowing instead
of swallowing.
- Around line 76-80: Responses from sendRequest in useGitStatus.ts can arrive
after the active session changed and overwrite state; capture the session id at
call time (e.g., const callSid = sid) or use a per-call token and, immediately
after each await, compare that token against the current active session
identifier (sessionIdRef.current or the current sid getter) before calling
setStatus or setDiff. Apply this guard in the sendRequest handling shown (the
block that sets status after get_git_status), in loadDiff, and in both branches
inside refresh so late responses for a previous session are ignored.

In `@src/web/hooks/useGroupChat.ts`:
- Around line 55-65: In loadChats, don't clear the current chat list on a
transient failure: remove the setChats([]) call from the catch block in
useGroupChat.ts so the existing chats (managed by setChats) are preserved on
errors; instead capture the error and surface it separately (e.g., call a
setLoadError(error) or dispatch a toast/notification with the caught error from
sendRequest in the catch block) while leaving setIsLoading(false) in the
finally; ensure you include the actual caught error when surfacing the failure
so debugging information is preserved.

In `@src/web/hooks/useLongPressMenu.ts`:
- Around line 76-109: The long-press action currently triggers
onOpenCommandPalette but does not prevent the subsequent normal click/send—add a
longPressFiredRef (useRef<boolean>(false)) and set it to true inside the timeout
callback (where triggerHapticFeedback and onOpenCommandPalette are called) to
mark the gesture consumed; in handleTouchEnd check longPressFiredRef.current
and, if true, call e.preventDefault() and e.stopPropagation() and then reset
longPressFiredRef.current = false (and still call clearLongPressTimer and reset
transform), and also guard the send/click handler (the existing click/send
function that uses sendButtonRef) to no-op when longPressFiredRef.current is
true so the long-press does not also send the message.

In `@src/web/hooks/useSettings.ts`:
- Around line 111-131: The optimistic-update in setSetting (referencing
SETTING_KEY_TO_FIELD, sendRequest, setSettings, and settings) only rolls back on
thrown errors; change the logic so after awaiting sendRequest('set_setting',
...) you check response.success and if it's falsy explicitly call
setSettings(prev) to restore the previous state and return false (treat
undefined as failure), otherwise keep the optimistic update and return true;
keep capturing prev before the optimistic set and preserve the existing
try/catch for thrown errors.

In `@src/web/mobile/AgentCreationSheet.tsx`:
- Around line 104-117: The catch block in the createAgent flow swallows
unexpected failures—modify the try/catch around createAgent in
AgentCreationSheet so you only handle known/recoverable errors (e.g.,
NETWORK_ERROR) locally (triggerHaptic, setIsSubmitting) and for all other
exceptions call the renderer Sentry utility (import captureException from
src/renderer/utils/sentry.ts) with context (agentName, selectedType, cwd) and
then rethrow or propagate the error so upstream can handle it; keep the existing
success path (triggerHaptic(HAPTIC_PATTERNS.success),
onCreated(result.sessionId), handleClose()) unchanged and ensure
setIsSubmitting(false) still runs for handled errors.

In `@src/web/mobile/AllSessionsView.tsx`:
- Around line 179-188: The Escape key press in handleRenameKeyDown should not
bubble to the document-level Escape listener (which calls onClose); update
handleRenameKeyDown to call e.stopPropagation() and e.preventDefault() when
handling Escape (and optionally for Enter) before calling onRenameCancel() (or
onRenameConfirm()) so the event doesn't reach the global listener; reference the
handleRenameKeyDown callback and the document-level Escape listener that invokes
onClose to locate where to add stopPropagation()/preventDefault().
- Around line 626-655: MoveToGroupSheet’s handleMove currently fires onMove and
closes the sheet immediately; change it to await the Promise<boolean> returned
by onMove (use async/await with try/catch), only call handleClose (and clear
local state) when the awaited result is true, and on false or caught errors keep
the sheet open and surface an error/allow retry (e.g., show a toast) instead of
dismissing; apply the same pattern to the other equivalent move/rename/delete
handlers in this file so all places that call onMove/onRename/onDelete await the
result and only dismiss on success (keep references to
MoveToGroupSheet.handleMove, onMove, handleClose, triggerHaptic to locate the
code).

In `@src/web/mobile/App.tsx`:
- Around line 2169-2189: The "Launch Auto Run" action only calls
setShowAutoRunSetup(true) but the panel is conditional on showAutoRunPanel, so
update the action for the act with id 'autorun-launch' to also enable the panel
by calling setShowAutoRunPanel(true) (i.e., ensure the action invokes both
setShowAutoRunSetup(true) and setShowAutoRunPanel(true)) so the setup UI becomes
visible; locate this in the acts push where id === 'autorun-launch' and modify
the action callback accordingly.

In `@src/web/mobile/AutoRunDocumentViewer.tsx`:
- Around line 52-83: The effect's loadContent currently wipes content/edit
buffers on fetch error and never resets editing flags, risking accidental
overwrites; update the load flow in useEffect/loadContent so that at start
(before awaiting) you reset editor state flags (call setIsEditing(false) and
setIsDirty(false) and clear any previous load error), on success
setContent(response.content ?? '') and setEditContent(loaded) and explicitly
clear any load error and ensure isDirty/isEditing are false, and on catch do NOT
call setContent('') or setEditContent('') but instead set a load error state
(e.g., setLoadError(error or true)) so the UI can surface the failure; keep the
finally block to setIsLoading(false) only if not cancelled. Ensure all
references use the existing symbols loadContent, setContent, setEditContent,
setIsLoading, setIsEditing, setIsDirty (and add setLoadError state if missing).

In `@src/web/mobile/CommandInputBar.tsx`:
- Around line 244-256: The long-press command-palette hook useLongPressMenu is
created and returns sendButtonRef and touch handlers (handleTouchStart/End/Move)
but these are not forwarded to the full-width ExpandedModeSendInterruptButton,
so long-press on that button can't open the palette; fix by passing
sendButtonRef and the touch handlers into ExpandedModeSendInterruptButton (or
its wrapper) wherever the expanded AI composer button is rendered (ensure the
component accepts a ref prop or use forwardedRef and attach onTouchStart,
onTouchEnd, onTouchMove props), and mirror the same prop forwarding used for the
normal send button so both layouts receive sendButtonRef,
handleSendButtonTouchStart, handleSendButtonTouchEnd, and
handleSendButtonTouchMove (also apply same change at the other occurrence noted
around lines 631-636).

In `@src/web/mobile/ContextManagementSheet.tsx`:
- Around line 95-102: The effect currently only clears autoCloseTimerRef on
unmount but doesn't clear the progress interval started by sendRequest, so
ensure you also clear the interval reference (e.g., progressIntervalRef or
whatever ref holds the setInterval) in the cleanup function of useEffect and
also clear it in the finally block of sendRequest; update sendRequest to
clearInterval(progressIntervalRef.current) and null it after clearing, and add
the same clear/cleanup logic to the existing unmount return so setProgress will
not be called after the component unmounts.
- Around line 198-203: The catch in ContextManagementSheet.tsx currently
swallows all errors; change it to catch the error object (catch (error)) and
discriminate expected network/timeouts vs unexpected errors, e.g., if
isExpectedNetworkError(error) handle locally, otherwise call
Sentry.captureException(error) (or the project's Sentry/reporting utility)
before calling clearInterval(progressInterval) and updating state
(setExecutionState, setProgress, setResultMessage, triggerHaptic); ensure you
import the Sentry/reporting utility and keep the existing UI updates after
reporting.
- Around line 84-93: The Escape key handler in the useEffect calls handleClose
unconditionally, allowing the sheet to close while an operation is running;
update the handleKeyDown callback to only call handleClose when executionState
!== 'executing' (i.e., if (e.key === 'Escape' && executionState !== 'executing')
handleClose()), and add executionState to the effect dependency array so the
listener behavior stays in sync with the current executionState; reference the
useEffect block, the handleKeyDown function, and the handleClose symbol when
applying the change.

In `@src/web/mobile/GitDiffViewer.tsx`:
- Around line 111-118: The root element of GitDiffViewer should be a true
fullscreen overlay instead of a normal flex child; update the outer <div> in
GitDiffViewer to use fixed positioning (position: 'fixed'), cover the viewport
(top: 0, left: 0, right: 0, bottom: 0 or width/height 100vw/100vh), and set a
high zIndex so it sits above MobileApp, while retaining the current flex layout
and backgroundColor; keep or remove the existing height: '100%' as needed after
switching to fixed positioning so the viewer fully replaces the underlying UI
when mounted.
- Around line 35-64: The parser in GitDiffViewer.tsx is treating diff metadata
like "--- a/file", "+++ b/file", and "\ No newline at end of file" as regular
add/remove lines which skews coloring and line numbers; update the loop that
iterates rawLines (the code that sets oldLine/newLine and pushes result entries)
to first detect these metadata lines (e.g., line.startsWith('--- '),
line.startsWith('+++ '), and line === '\\ No newline at end of file') and push
them as a neutral meta/context row (type 'meta' or 'context') with empty
oldNum/newNum without incrementing oldLine or newLine, and skip the subsequent
add/remove/context branches so they are not counted as changed lines.

In `@src/web/mobile/GitStatusPanel.tsx`:
- Around line 26-49: The untracked-file filtering logic compares the full status
string to '?' which fails for git porcelain codes like '??' and causes
misclassification; update the filter used where files are grouped (the arrays
computed around the existing files.filter calls) to normalize status the same
way statusIcon/statusColor do: trim the status and use .charAt(0) when comparing
to '?' (and likewise when checking for 'M'/'T' etc.), e.g. replace comparisons
like f.status === '?' or f.status === 'M' with checks that use
f.status?.trim().charAt(0) === '?' or 'M' so grouping of untracked/modified
matches statusIcon/statusColor behavior.

In `@src/web/mobile/NotificationSettingsSheet.tsx`:
- Around line 16-20: The component currently calls
Notification.requestPermission() directly, bypassing the useNotifications hook;
add a requestPermission callback to NotificationSettingsSheetProps (e.g.,
requestPermission: () => Promise<NotificationPermission> or () =>
NotificationPermission) and replace all direct Notification.requestPermission()
invocations inside the component (the handlers around the badge and the other
occurrences referenced at 76-81 and 125-130) to call the passed-in
requestPermission prop instead, and propagate the returned permission to the
existing onPreferencesChange/permission handling so the hook's state and
callbacks remain the single source of truth.

In `@src/web/mobile/QuickActionsMenu.tsx`:
- Around line 104-145: When searchQuery is empty recent actions (from
loadRecentActions()) are added to items and then duplicated by the subsequent
grouping over filteredActions; fix this by capturing the recentIds set when you
call loadRecentActions() and skip any action whose id is in that set when
building byCategory (the loop over filteredActions) so recent actions are not
re-added. Update the logic around loadRecentActions, filteredActions, and
byCategory/CATEGORY_ORDER to use that exclusion set when populating items.

In `@src/web/mobile/RightDrawer.tsx`:
- Around line 46-50: The drawer currently defaults to the non-functional "files"
tab via the TABS constant and the RightDrawer's activeTab handling; change
behavior so the Files tab is not the default or not shown: either remove the {
id: 'files', ... } entry from TABS (hiding the placeholder) or keep it but
change the default active tab used in RightDrawer (where activeTab prop or
internal state is initialized) to an implemented tab such as
'history'/'autorun'/'git'; also update any logic that assumes 'files' exists
(search for TABS, RightDrawerTab, and places that read activeTab) so selection,
tab rendering, and any initial open behavior use the new default/visible tabs
consistently.
- Around line 301-324: fetchHistory can commit stale results when multiple calls
overlap; update it to use an AbortController (or an incrementing requestId) so
only the latest invocation updates state: create an AbortController per
fetchHistory call (store controller in a ref or create/cleanup in the effect
that calls fetchHistory), pass controller.signal to fetch, and in the catch
ignore AbortError (or check if requestId matches) and only call
setEntries/setError/setIsLoading when not aborted and the requestId matches;
also abort the previous controller in the effect cleanup or before starting a
new fetch to ensure late responses don't overwrite the newer state (refer to
fetchHistory, setEntries, setIsLoading, setError).

In `@src/web/mobile/SessionPillBar.tsx`:
- Around line 772-777: The "+" create-agent affordance is only rendered when
sessions.length > 0, so when sessions.length === 0 the onOpenCreateAgent
callback is never reachable; update SessionPillBar to always render the
create-agent pill/button (or a visible fallback) in the zero-session state and
wire its onPress/onClick to the existing onOpenCreateAgent prop (reference:
SessionPillBar component and prop onOpenCreateAgent). Concretely, move or
duplicate the "+" pill render logic out of the populated-scroller conditional
(or add a zero-session branch) so the button is visible and accessible when
there are no sessions; ensure it uses the same handler and
accessibility/touchable wrapper as the populated version.

---

Minor comments:
In `@src/__tests__/web/mobile/App.test.tsx`:
- Around line 515-530: The RightDrawer mock currently ignores its children which
prevents mounted drawer content from being asserted; update the mock for
RightDrawer (the function/component named RightDrawer in the test) to accept and
render its children inside the returned div (preserve data-testid="right-drawer"
and data-active-tab) and then update the history-related assertions to check the
actual mounted panel inside the drawer (e.g., use
within(drawer).getByTestId('mobile-history-panel') or similar) so the tests
verify MobileHistoryPanel is mounted inside RightDrawer.

In `@src/main/web-server/managers/CallbackRegistry.ts`:
- Around line 303-305: The methods currently return an empty string when the
callback is not registered, which hides "unavailable" vs "empty" states; update
getAutoRunDocContent (and the other similar method at lines 391-393) to return
null when the corresponding callback is not wired: change the return type from
Promise<string> to Promise<string | null>, have the early branch return null (or
throw a descriptive Error if you prefer explicit failure), and update any
callers/type annotations to handle null (or the thrown error) instead of
assuming an empty string.

In `@src/renderer/hooks/remote/useRemoteIntegration.ts`:
- Around line 611-617: The current mapping of statusLines to files uses the
left-hand path for renames/copies, causing diffs to request the old path; update
the mapping in useRemoteIntegration (the statusLines => files block) to detect
the " -> " token and, when present, use the destination/right-hand path as
filePath (trimmed) instead of the source; keep existing logic for status and
staged determination (status, staged variables) but ensure filePath is the new
path for renamed entries so downstream diff requests target the correct file.

In `@src/web/hooks/useMobileKeyboardHandler.ts`:
- Around line 58-63: In useMobileKeyboardHandler, avoid always calling
event.preventDefault()/stopPropagation for the command-palette shortcuts; first
check the relevant optional props and state (onOpenCommandPalette,
onCloseCommandPalette, isCommandPaletteOpen) and only consume the event when
there is an actual handler to run: for Cmd/Ctrl+K only call
preventDefault/stopPropagation and invoke onOpenCommandPalette when
onOpenCommandPalette is defined (and skip if it's undefined), and for Escape
only consume the event and call onCloseCommandPalette when onCloseCommandPalette
is defined and isCommandPaletteOpen is true; apply the same guard logic to the
other shortcut handling in the same hook (the block around the other key
handling) so the hook does not hijack keys when handlers are not provided.

In `@src/web/mobile/AutoRunIndicator.tsx`:
- Around line 40-42: AutoRunIndicator currently sets role="button" and tabIndex
when onTap is provided but only uses onClick; add a keyboard handler so Enter
and Space activate the same action: implement an onKeyDown handler (e.g.,
handleKeyDown) in the AutoRunIndicator component that, when onTap is present,
listens for key === 'Enter' or key === ' ' / code === 'Space' and calls onTap
(and calls event.preventDefault() for Space to avoid page scroll); wire this
handler into the element via onKeyDown={onTap ? handleKeyDown : undefined} so
keyboard users can activate the button consistently with onClick.

In `@src/web/mobile/AutoRunPanel.tsx`:
- Around line 216-220: The banner should not render "Task 1/0" during startup —
stop defaulting totalCounts to 0 and instead only show task/doc counters when
the corresponding totals are known (>0). Update the variable handling for
totalTasks, completedTasks and currentTaskIndex (from autoRunState?.totalTasks
?? 0 etc.) so totals remain undefined/null when missing, compute progress only
if totalTasks > 0, and change the render logic that uses isRunning, progress,
currentTaskIndex, totalTasks and completedTasks to conditionally display the
"Task X/Y" (and similar doc counters) only when the relevant total is > 0; apply
the same pattern for the other occurrence that uses these same symbols.

In `@src/web/mobile/AutoRunSetupSheet.tsx`:
- Around line 29-42: AutoRunSetupSheet currently seeds selectedFiles, prompt,
loopEnabled and maxLoops only on initial mount, so when sessionId (_sessionId)
changes while the component stays mounted the old draft remains; add a useEffect
inside AutoRunSetupSheet that watches _sessionId (and documents) and
reinitializes state by calling setSelectedFiles(new Set(documents.map(d =>
d.filename))), setPrompt(''), setLoopEnabled(false) and setMaxLoops(3) (and
optionally setIsVisible(false) if you want to reset visibility) to ensure a
fresh draft for each new session.

---

Nitpick comments:
In `@src/__tests__/main/web-server/web-server-factory.test.ts`:
- Around line 44-72: The test's registration assertions are missing checks for
the newly added setter mocks (e.g., setGetAutoRunDocsCallback,
setGetAutoRunDocContentCallback, setSaveAutoRunDocCallback,
setStopAutoRunCallback, setGetSettingsCallback, setSetSettingCallback,
setGetGroupsCallback, setCreateGroupCallback, setRenameGroupCallback,
setDeleteGroupCallback, setMoveSessionToGroupCallback, setCreateSessionCallback,
setDeleteSessionCallback, setRenameSessionCallback, setGetGitStatusCallback,
setGetGitDiffCallback, setGetGroupChatsCallback, setStartGroupChatCallback,
setGetGroupChatStateCallback, setStopGroupChatCallback,
setSendGroupChatMessageCallback, setMergeContextCallback,
setTransferContextCallback, setSummarizeContextCallback,
setGetCueSubscriptionsCallback, setToggleCueSubscriptionCallback,
setGetCueActivityCallback, setGetUsageDashboardCallback,
setGetAchievementsCallback); update the registration/assertions block in
web-server-factory.test.ts to assert each of these setters was called (or called
with the expected handler) so the test fails if web-server-factory.ts stops
wiring any of them. Ensure you add one assertion per new setter using the same
pattern as the existing registration assertions.

In `@src/__tests__/renderer/hooks/useRemoteIntegration.test.ts`:
- Around line 138-190: Add assertions after calling
renderHook(useRemoteIntegration) that verify each new onRemote* handler was
registered: create a list of the handler symbols (onRemoteGetAutoRunDocs,
onRemoteGetAutoRunDocContent, onRemoteSaveAutoRunDoc, onRemoteStopAutoRun,
onRemoteSetSetting, onRemoteCreateSession, onRemoteDeleteSession,
onRemoteRenameSession, onRemoteCreateGroup, onRemoteRenameGroup,
onRemoteDeleteGroup, onRemoteMoveSessionToGroup, onRemoteGetGitStatus,
onRemoteGetGitDiff) and assert in a table-driven loop that each corresponding
mock (the onRemote* function on the mocked ipcRenderer) was called/used by
renderHook(useRemoteIntegration) so missing subscriptions fail the test loudly.

In `@src/web/hooks/useLongPressMenu.ts`:
- Around line 37-42: Remove the dead inputMode prop from the public API by
deleting inputMode from the UseLongPressMenuOptions interface and updating all
related typings/usages; keep onModeToggle (and its signature) as the only
mode-related prop, update the useLongPressMenu hook signature/type references
(UseLongPressMenuOptions) and any callers to stop passing or expecting
inputMode, and run the typechecker to fix any remaining places that were
threading inputMode through the codebase.

In `@src/web/hooks/useNotifications.ts`:
- Around line 232-244: showNotification centralizes the icon/badge defaults but
handleNotificationEvent currently hardcodes a different icon path causing drift;
update handleNotificationEvent to reuse the same default icon/badge used by
showNotification (extract or reference the shared default values used when
creating Notification in showNotification) so both functions use the same asset
variables (e.g., the default icon/badge constant or the same options object)
rather than duplicating the hardcoded '/maestro-icon-192.png' path.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7f64a58b-6505-435a-991a-d46e524816a9

📥 Commits

Reviewing files that changed from the base of the PR and between 91024db and 3cda177.

📒 Files selected for processing (51)

• src/__tests__/main/web-server/web-server-factory.test.ts
• src/__tests__/renderer/hooks/useRemoteIntegration.test.ts
• src/__tests__/web/hooks/useLongPressMenu.test.ts
• src/__tests__/web/mobile/AllSessionsView.test.tsx
• src/__tests__/web/mobile/App.test.tsx
• src/__tests__/web/mobile/CommandInputBar.test.tsx
• src/__tests__/web/mobile/QuickActionsMenu.test.tsx
• src/main/preload/process.ts
• src/main/web-server/WebServer.ts
• src/main/web-server/handlers/messageHandlers.ts
• src/main/web-server/managers/CallbackRegistry.ts
• src/main/web-server/services/broadcastService.ts
• src/main/web-server/types.ts
• src/main/web-server/web-server-factory.ts
• src/renderer/App.tsx
• src/renderer/global.d.ts
• src/renderer/hooks/batch/useBatchHandlers.ts
• src/renderer/hooks/remote/useRemoteIntegration.ts
• src/renderer/stores/agentStore.ts
• src/web/hooks/index.ts
• src/web/hooks/useAgentManagement.ts
• src/web/hooks/useAutoRun.ts
• src/web/hooks/useCue.ts
• src/web/hooks/useGitStatus.ts
• src/web/hooks/useGroupChat.ts
• src/web/hooks/useLongPressMenu.ts
• src/web/hooks/useMobileKeyboardHandler.ts
• src/web/hooks/useNotifications.ts
• src/web/hooks/useSettings.ts
• src/web/hooks/useWebSocket.ts
• src/web/mobile/AchievementsPanel.tsx
• src/web/mobile/AgentCreationSheet.tsx
• src/web/mobile/AllSessionsView.tsx
• src/web/mobile/App.tsx
• src/web/mobile/AutoRunDocumentViewer.tsx
• src/web/mobile/AutoRunIndicator.tsx
• src/web/mobile/AutoRunPanel.tsx
• src/web/mobile/AutoRunSetupSheet.tsx
• src/web/mobile/CommandInputBar.tsx
• src/web/mobile/ContextManagementSheet.tsx
• src/web/mobile/CuePanel.tsx
• src/web/mobile/GitDiffViewer.tsx
• src/web/mobile/GitStatusPanel.tsx
• src/web/mobile/GroupChatPanel.tsx
• src/web/mobile/GroupChatSetupSheet.tsx
• src/web/mobile/NotificationSettingsSheet.tsx
• src/web/mobile/QuickActionsMenu.tsx
• src/web/mobile/RightDrawer.tsx
• src/web/mobile/SessionPillBar.tsx
• src/web/mobile/SettingsPanel.tsx
• src/web/mobile/UsageDashboardPanel.tsx

💤 Files with no reviewable changes (1)

• src/renderer/stores/agentStore.ts

[coderabbitai]

🧹 Nitpick comments (7)

src/renderer/hooks/remote/useRemoteIntegration.ts (3)

640-647: Empty catch block silently swallows errors.

Errors from git operations (invalid cwd, permission issues, non-git repos) are discarded without logging, making debugging difficult. Consistent with the file's existing error-handling pattern (line 218), log the error before returning the fallback response.

♻️ Proposed fix

-			} catch {
+			} catch (error) {
+				console.error('[Remote] Failed to get git status:', error);
 				window.maestro.process.sendRemoteGetGitStatusResponse(responseChannel, {
 					branch: '',
 					files: [],
 					ahead: 0,
 					behind: 0,
 				});
 			}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/renderer/hooks/remote/useRemoteIntegration.ts` around lines 640 - 647,
The catch is swallowing errors when calling git status and returning the
fallback response via window.maestro.process.sendRemoteGetGitStatusResponse;
update the catch to log the caught error (use the same logger used elsewhere in
this file, e.g., processLogger.error or console.error) with a short contextual
message before sending the fallback response so failures (invalid cwd,
permissions, non-git repos) are recorded for debugging.

---

688-693: Empty catch block silently swallows errors.

Same pattern issue as the git status handler—errors are silently discarded.

♻️ Proposed fix

-			} catch {
+			} catch (error) {
+				console.error('[Remote] Failed to get git diff:', error);
 				window.maestro.process.sendRemoteGetGitDiffResponse(responseChannel, {
 					diff: '',
 					files: [],
 				});
 			}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/renderer/hooks/remote/useRemoteIntegration.ts` around lines 688 - 693,
The catch block in the get-git-diff handler swallows errors; change it to catch
the error (e.g., catch (err)) and include error details in the response sent via
window.maestro.process.sendRemoteGetGitDiffResponse (add an error or message
field such as error: err instanceof Error ? err.message : String(err)), and
optionally log the full error to console or the app logger; update the handler
in useRemoteIntegration.ts where responseChannel and
sendRemoteGetGitDiffResponse are used.

---

561-577: Empty catch block silently swallows errors.

The error is discarded without any logging, making failures difficult to debug. The existing code in this file logs errors (e.g., lines 218, 347-356). For consistency and debuggability, log the error before returning the failure response.

♻️ Proposed fix

 		const unsubscribe = window.maestro.process.onRemoteSetSetting(
 			async (key: string, value: unknown, responseChannel: string) => {
 				try {
 					await window.maestro.settings.set(key, value);
 					window.maestro.process.sendRemoteSetSettingResponse(responseChannel, true);
-				} catch {
+				} catch (error) {
+					console.error('[Remote] Failed to set setting:', key, error);
 					window.maestro.process.sendRemoteSetSettingResponse(responseChannel, false);
 				}
 			}
 		);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/renderer/hooks/remote/useRemoteIntegration.ts` around lines 561 - 577,
The catch block in the onRemoteSetSetting handler currently swallows errors;
update the catch to log the error before sending the failure response.
Specifically, in the async callback passed to
window.maestro.process.onRemoteSetSetting (which calls
window.maestro.settings.set and
window.maestro.process.sendRemoteSetSettingResponse), capture the caught error
and log it (e.g., console.error("Failed to set remote setting", key, error) or
use the existing process logger) and then call
window.maestro.process.sendRemoteSetSettingResponse(responseChannel, false).

src/web/mobile/ContextManagementSheet.tsx (1)

71-71: Remove unused progressListenerRef.

The progressListenerRef is declared on line 71 and has a cleanup effect on lines 224-231, but it's never actually assigned or used anywhere in the component. This appears to be leftover code from a previous implementation.

🧹 Remove unused ref and effect

 const autoCloseTimerRef = useRef<ReturnType<typeof setTimeout>>();
-const progressListenerRef = useRef<((event: MessageEvent) => void) | null>(null);

And remove the cleanup effect (lines 224-231):

-// Clean up WebSocket listener on unmount
-useEffect(() => {
-	return () => {
-		if (progressListenerRef.current) {
-			progressListenerRef.current = null;
-		}
-	};
-}, []);

Also applies to: 224-231

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/ContextManagementSheet.tsx` at line 71, Remove the unused
progressListenerRef declared with useRef in the ContextManagementSheet component
and delete its corresponding cleanup effect that references progressListenerRef
(the useRef declaration named progressListenerRef and the useEffect return block
that removes an event listener using progressListenerRef.current). Ensure no
other code references progressListenerRef before removing both the declaration
and the cleanup effect.

src/web/mobile/RightDrawer.tsx (2)

423-433: Several parameters are unused in AutoRunTabContent.

The sessionId, onOpenDocument, sendRequest, and send parameters are declared but never used in the function body. This suggests either incomplete implementation or over-provisioning of props.

🧹 Remove unused parameters or add TODO comment

If these are planned for future use:

 function AutoRunTabContent({
+	// TODO: Implement document list with onOpenDocument callback
+	sessionId: _sessionId,
 	autoRunState,
+	onOpenDocument: _onOpenDocument,
 	onOpenSetup,
+	sendRequest: _sendRequest,
+	send: _send,
 }: {

Or remove them if not needed:

 function AutoRunTabContent({
 	autoRunState,
 	onOpenSetup,
 }: {
-	sessionId: string;
 	autoRunState: AutoRunState | null;
-	onOpenDocument?: (filename: string) => void;
 	onOpenSetup?: () => void;
-	sendRequest: UseWebSocketReturn['sendRequest'];
-	send: UseWebSocketReturn['send'];
 }) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/RightDrawer.tsx` around lines 423 - 433, The AutoRunTabContent
function signature declares unused props (sessionId, onOpenDocument,
sendRequest, send) which should be removed or marked for future use; update the
AutoRunTabContent declaration to only accept the actually used props
(autoRunState, onOpenSetup) or, if these will be used later, add a clear TODO
comment above the function and prefix unused parameter names with an underscore
(e.g., _sessionId) to avoid lint warnings, and adjust any related type
annotations so the component prop type matches the updated parameter list.

---

297-298: Consider using a more specific type instead of any[] for history entries.

The entries state is typed as any[]. Consider defining a proper interface for history entries to improve type safety.

💡 Define HistoryEntry interface

+interface HistoryEntry {
+	id: string;
+	type: string;
+	timestamp: string | number;
+	summary?: string;
+}

 function HistoryTabContent({
 	sessionId,
 	projectPath,
 }: {
 	sessionId: string;
 	projectPath?: string;
 }) {
 	const colors = useThemeColors();
-	const [entries, setEntries] = useState<any[]>([]);
+	const [entries, setEntries] = useState<HistoryEntry[]>([]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/RightDrawer.tsx` around lines 297 - 298, The entries state is
typed as any[], so define a specific interface (e.g., HistoryEntry) describing
the shape of a history item (fields like id, title, timestamp, url, etc. used by
RightDrawer) and replace useState<any[]>([]) with useState<HistoryEntry[]>([])
in RightDrawer.tsx; update any functions that read/write entries (setEntries,
map/render logic) to use the new HistoryEntry type so TypeScript enforces
correct properties across operations.

src/web/mobile/GitStatusPanel.tsx (1)

443-449: Consider scoping the keyframe animation to avoid global pollution.

The @keyframes gitRefreshSpin is injected as a global style. While this works, it could potentially conflict with other animations if the same name is used elsewhere.

💡 Optional: Use a more unique animation name

 <style>{`
-	`@keyframes` gitRefreshSpin {
+	`@keyframes` maestroGitRefreshSpin {
 		from { transform: rotate(0deg); }
 		to { transform: rotate(360deg); }
 	}
 `}</style>

And update the reference on line 345:

-	animation: isLoading ? 'gitRefreshSpin 1s linear infinite' : 'none',
+	animation: isLoading ? 'maestroGitRefreshSpin 1s linear infinite' : 'none',

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/GitStatusPanel.tsx` around lines 443 - 449, The global
`@keyframes` name gitRefreshSpin can conflict with other code; rename it to a
component-scoped name (e.g., GitStatusPanel_gitRefreshSpin or similar) inside
the style block defined in the GitStatusPanel component and update any places
that reference it (the refresh button / refresh icon animation usage inside
GitStatusPanel) to use the new name; ensure the animation declaration (animation
or animationName) on the refresh element matches the new keyframe identifier so
the spin still works.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/renderer/hooks/remote/useRemoteIntegration.ts`:
- Around line 640-647: The catch is swallowing errors when calling git status
and returning the fallback response via
window.maestro.process.sendRemoteGetGitStatusResponse; update the catch to log
the caught error (use the same logger used elsewhere in this file, e.g.,
processLogger.error or console.error) with a short contextual message before
sending the fallback response so failures (invalid cwd, permissions, non-git
repos) are recorded for debugging.
- Around line 688-693: The catch block in the get-git-diff handler swallows
errors; change it to catch the error (e.g., catch (err)) and include error
details in the response sent via
window.maestro.process.sendRemoteGetGitDiffResponse (add an error or message
field such as error: err instanceof Error ? err.message : String(err)), and
optionally log the full error to console or the app logger; update the handler
in useRemoteIntegration.ts where responseChannel and
sendRemoteGetGitDiffResponse are used.
- Around line 561-577: The catch block in the onRemoteSetSetting handler
currently swallows errors; update the catch to log the error before sending the
failure response. Specifically, in the async callback passed to
window.maestro.process.onRemoteSetSetting (which calls
window.maestro.settings.set and
window.maestro.process.sendRemoteSetSettingResponse), capture the caught error
and log it (e.g., console.error("Failed to set remote setting", key, error) or
use the existing process logger) and then call
window.maestro.process.sendRemoteSetSettingResponse(responseChannel, false).

In `@src/web/mobile/ContextManagementSheet.tsx`:
- Line 71: Remove the unused progressListenerRef declared with useRef in the
ContextManagementSheet component and delete its corresponding cleanup effect
that references progressListenerRef (the useRef declaration named
progressListenerRef and the useEffect return block that removes an event
listener using progressListenerRef.current). Ensure no other code references
progressListenerRef before removing both the declaration and the cleanup effect.

In `@src/web/mobile/GitStatusPanel.tsx`:
- Around line 443-449: The global `@keyframes` name gitRefreshSpin can conflict
with other code; rename it to a component-scoped name (e.g.,
GitStatusPanel_gitRefreshSpin or similar) inside the style block defined in the
GitStatusPanel component and update any places that reference it (the refresh
button / refresh icon animation usage inside GitStatusPanel) to use the new
name; ensure the animation declaration (animation or animationName) on the
refresh element matches the new keyframe identifier so the spin still works.

In `@src/web/mobile/RightDrawer.tsx`:
- Around line 423-433: The AutoRunTabContent function signature declares unused
props (sessionId, onOpenDocument, sendRequest, send) which should be removed or
marked for future use; update the AutoRunTabContent declaration to only accept
the actually used props (autoRunState, onOpenSetup) or, if these will be used
later, add a clear TODO comment above the function and prefix unused parameter
names with an underscore (e.g., _sessionId) to avoid lint warnings, and adjust
any related type annotations so the component prop type matches the updated
parameter list.
- Around line 297-298: The entries state is typed as any[], so define a specific
interface (e.g., HistoryEntry) describing the shape of a history item (fields
like id, title, timestamp, url, etc. used by RightDrawer) and replace
useState<any[]>([]) with useState<HistoryEntry[]>([]) in RightDrawer.tsx; update
any functions that read/write entries (setEntries, map/render logic) to use the
new HistoryEntry type so TypeScript enforces correct properties across
operations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c206df1e-81fd-4400-8e7e-c1d649a2d94b

📥 Commits

Reviewing files that changed from the base of the PR and between 3cda177 and 471506a.

📒 Files selected for processing (10)

• src/renderer/hooks/remote/useRemoteIntegration.ts
• src/web/hooks/useMobileKeyboardHandler.ts
• src/web/hooks/useSettings.ts
• src/web/hooks/useWebSocket.ts
• src/web/mobile/AllSessionsView.tsx
• src/web/mobile/AutoRunPanel.tsx
• src/web/mobile/AutoRunSetupSheet.tsx
• src/web/mobile/ContextManagementSheet.tsx
• src/web/mobile/GitStatusPanel.tsx
• src/web/mobile/RightDrawer.tsx

✅ Files skipped from review due to trivial changes (1)

• src/web/mobile/AutoRunSetupSheet.tsx

🚧 Files skipped from review as they are similar to previous changes (3)

• src/web/mobile/AllSessionsView.tsx
• src/web/hooks/useMobileKeyboardHandler.ts
• src/web/mobile/AutoRunPanel.tsx

[chr1syy]

Nitpick Review Comments — Analysis & Resolution

Addressed the CodeRabbit nitpick comments. Two were fixed, four were analyzed and intentionally skipped:

Fixed ✅

ContextManagementSheet.tsx:71 — Unused progressListenerRef
Removed the dead useRef declaration and its cleanup useEffect. The ref was never assigned a listener — pure dead code.

RightDrawer.tsx:423-433 — Unused props in AutoRunTabContent
Removed sessionId, onOpenDocument, sendRequest, and send from both the component's type signature and call site. These were declared but never read.

Intentionally Skipped ⏭️

useRemoteIntegration.ts:640-647, 688-693, 561-577 — Add error logging in catch blocks
These are remote IPC handlers for git status, git diff, and settings. The catch blocks return typed fallback responses (empty arrays, false, etc.) which correctly signal failure to the caller. The caught errors are expected scenarios (non-git directories, invalid cwd, permission errors) — adding console.error here would create log noise for recoverable cases. The CLAUDE.md error handling guidelines say to let unexpected exceptions bubble up, but these are explicitly expected/recoverable.

GitStatusPanel.tsx:443-449 — Rename @keyframes gitRefreshSpin
The keyframe name is already specific (gitRefreshSpin), and this is a mobile-only component with a single inline <style> block. Collision risk is negligible. Renaming to GitStatusPanel_gitRefreshSpin would add verbosity for no practical benefit.

RightDrawer.tsx:297-298 — Type entries as specific interface instead of any[]
Valid improvement, but requires tracing the server response shape through sendRequest('get_history', ...) to define a correct HistoryEntry type. This is a typing exercise better done when the history feature is more fleshed out — deferring to avoid scope creep.

---

Commit: 85bee58

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

src/web/mobile/ContextManagementSheet.tsx (1)

223-238: Consider adding modal accessibility attributes.

The modal backdrop lacks role="dialog" and aria-modal="true", and there's no focus trap for keyboard users. While this is mobile-focused, adding these attributes improves accessibility for assistive technology users.

♿ Proposed accessibility enhancement

 		<div
 			onClick={handleBackdropTap}
+			role="dialog"
+			aria-modal="true"
+			aria-labelledby="context-management-title"
 			style={{
 				position: 'fixed',

And add the ID to the heading:

 					<h2
+						id="context-management-title"
 						style={{

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/ContextManagementSheet.tsx` around lines 223 - 238, The
backdrop div in ContextManagementSheet currently lacks accessibility attributes
and focus management; add role="dialog" and aria-modal="true" to that element,
ensure the sheet container has an accessible heading with a stable id (e.g.,
contextManagementHeading) and set aria-labelledby on the dialog to that id, and
implement a simple focus trap when isVisible is true (move focus into the sheet
on open, restore focus on close, and constrain Tab/Shift+Tab to cycle within the
sheet) by updating the component lifecycle or using a small focus-trap utility;
reference the handleBackdropTap handler and the top-level JSX node for where to
add these attributes and focus logic.

src/web/mobile/RightDrawer.tsx (1)

58-74: Clarify whether activeTab is controlled or just an initial value.

currentTab is seeded from activeTab once and then diverges forever. If callers can change activeTab after mount, the drawer will ignore them; if they cannot, the prop name is misleading. Either sync it in an effect or rename it to initialTab.

As per coding guidelines, "Surface assumptions early before implementing non-trivial work. Explicitly state assumptions and format as: 'Assumptions: 1) X, 2) Y. Correct me now or I proceed.' Never silently fill in ambiguous requirements."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/RightDrawer.tsx` around lines 58 - 74, The RightDrawer
currently seeds local state currentTab from the prop activeTab and then never
updates it; decide whether activeTab is controlled or an initial value and
implement accordingly: either (A) treat it as controlled—add a useEffect inside
RightDrawer that watches activeTab and calls setCurrentTab(activeTab) so the
drawer follows prop changes (referencing currentTab, setCurrentTab, and the
activeTab prop), or (B) treat it as an initial value—rename the prop to
initialTab and update RightDrawerProps and usages to avoid implying control; in
both cases add an explicit assumptions comment at the top of the component like
"Assumptions: 1) activeTab is [controlled|initial] — correct me now or I
proceed."

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/web/mobile/ContextManagementSheet.tsx`:
- Around line 640-648: The code can crash when sessions is empty because
sessions.find(...) || sessions[0] may be undefined and
getSessionLabel(undefined) will throw; update the rendering to guard against an
empty sessions array by either (a) checking sessions.length > 0 before calling
getSessionLabel and rendering a safe fallback string like "Unknown session", or
(b) making getSessionLabel accept an undefined/nullable argument and return a
safe label when session is missing; change the call site in
ContextManagementSheet (around the JSX using getSessionLabel and
currentSessionId) to use the chosen safe fallback.
- Around line 197-203: The catch block that currently swallows all errors
(surrounding clearInterval(progressInterval); setExecutionState('failure');
setProgress(0); setResultMessage(...); triggerHaptic(...)) must differentiate
expected/recoverable errors from unexpected ones: explicitly check the thrown
error's type/code (e.g. timeout or network error) and handle those by clearing
the interval and setting failure state/messages as today, but for any
other/unknown error call Sentry.captureException(error) (or rethrow the error)
so it is reported and not silently dropped; update the catch to reference
clearInterval, progressInterval, setExecutionState, setProgress,
setResultMessage, and triggerHaptic accordingly while either rethrowing
unexpected errors or sending them to Sentry before failing gracefully.

In `@src/web/mobile/RightDrawer.tsx`:
- Around line 84-125: The swipe gesture currently used by RightDrawer expects
useSwipeGestures to consistently return a numeric offsetX (positive for
rightward movement) and a boolean isSwiping so the drawer can follow the finger;
update the useSwipeGestures hook to always include offsetX (default 0),
isSwiping, and handlers in its return value, ensure the trackOffset option
actively updates offsetX during pointer moves and resets it on end/cancel, and
make sure the sign/direction matches RightDrawer's expectation (offsetX > 0
moves drawer right); also export/update the hook's TypeScript return type so
RightDrawer's destructuring (handlers: swipeHandlers, offsetX, isSwiping) is
always valid.
- Around line 294-316: The fetchHistory function is currently catching all
exceptions and turning them into UI state, which hides unexpected failures;
update fetchHistory to only handle expected network/connectivity errors (e.g.,
check response.ok or inspect err.name/message for fetch/TypeError) and setError
with a user-friendly connectivity message in that case, but for all other
exceptions call captureException (import from src/renderer/utils/sentry.ts) and
rethrow or let them bubble so Sentry can record them; ensure you import
captureException, use it with context (e.g., include apiUrl, projectPath,
sessionId), and apply the same change for the similar error-handling block later
in the file (the other fetch/history handling around the second occurrence).

---

Nitpick comments:
In `@src/web/mobile/ContextManagementSheet.tsx`:
- Around line 223-238: The backdrop div in ContextManagementSheet currently
lacks accessibility attributes and focus management; add role="dialog" and
aria-modal="true" to that element, ensure the sheet container has an accessible
heading with a stable id (e.g., contextManagementHeading) and set
aria-labelledby on the dialog to that id, and implement a simple focus trap when
isVisible is true (move focus into the sheet on open, restore focus on close,
and constrain Tab/Shift+Tab to cycle within the sheet) by updating the component
lifecycle or using a small focus-trap utility; reference the handleBackdropTap
handler and the top-level JSX node for where to add these attributes and focus
logic.

In `@src/web/mobile/RightDrawer.tsx`:
- Around line 58-74: The RightDrawer currently seeds local state currentTab from
the prop activeTab and then never updates it; decide whether activeTab is
controlled or an initial value and implement accordingly: either (A) treat it as
controlled—add a useEffect inside RightDrawer that watches activeTab and calls
setCurrentTab(activeTab) so the drawer follows prop changes (referencing
currentTab, setCurrentTab, and the activeTab prop), or (B) treat it as an
initial value—rename the prop to initialTab and update RightDrawerProps and
usages to avoid implying control; in both cases add an explicit assumptions
comment at the top of the component like "Assumptions: 1) activeTab is
[controlled|initial] — correct me now or I proceed."

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 49cce4a0-7444-4d74-a116-217674316355

📥 Commits

Reviewing files that changed from the base of the PR and between 471506a and 85bee58.

📒 Files selected for processing (2)

• src/web/mobile/ContextManagementSheet.tsx
• src/web/mobile/RightDrawer.tsx

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Catch block swallows all errors without differentiation or Sentry reporting.

Per coding guidelines, expected/recoverable errors (timeout, network disconnect) should be caught and handled gracefully, but unexpected errors should be rethrown or reported to Sentry. The current catch block treats all errors uniformly and discards error details.

🛠️ Proposed fix: differentiate error handling and report unexpected errors

-		} catch {
+		} catch (error) {
 			clearInterval(progressInterval);
 			setExecutionState('failure');
 			setProgress(0);
-			setResultMessage('Operation failed — check connection');
+			const isExpectedError =
+				error instanceof Error &&
+				(error.message === 'Request timed out' ||
+					error.message === 'WebSocket not connected');
+			if (isExpectedError) {
+				setResultMessage('Operation failed — check connection');
+			} else {
+				// Report unexpected errors to Sentry
+				import('../utils/sentry').then(({ captureException }) => {
+					captureException(error, {
+						extra: { selectedOp, sourceId, targetId },
+					});
+				});
+				setResultMessage(
+					error instanceof Error ? error.message : 'Operation failed unexpectedly'
+				);
+			}
 			triggerHaptic(HAPTIC_PATTERNS.error);
 		}

As per coding guidelines: "For expected/recoverable errors, catch them explicitly, check the error code, and handle gracefully. For unexpected errors, rethrow to let Sentry capture them."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/ContextManagementSheet.tsx` around lines 197 - 203, The catch
block that currently swallows all errors (surrounding
clearInterval(progressInterval); setExecutionState('failure'); setProgress(0);
setResultMessage(...); triggerHaptic(...)) must differentiate
expected/recoverable errors from unexpected ones: explicitly check the thrown
error's type/code (e.g. timeout or network error) and handle those by clearing
the interval and setting failure state/messages as today, but for any
other/unknown error call Sentry.captureException(error) (or rethrow the error)
so it is reported and not silently dropped; update the catch to reference
clearInterval, progressInterval, setExecutionState, setProgress,
setResultMessage, and triggerHaptic accordingly while either rethrowing
unexpected errors or sending them to Sentry before failing gracefully.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Potential crash if sessions array is empty.

If sessions.find() returns undefined and sessions[0] is also undefined (empty array), getSessionLabel(undefined) will throw a runtime error when trying to access session.name.

🛡️ Proposed defensive fix

 								This will compress the context of the current agent
 								<strong style={{ color: colors.textMain }}>
 									{' '}
-									{getSessionLabel(sessions.find((s) => s.id === currentSessionId) || sessions[0])}
+									{getSessionLabel(
+										sessions.find((s) => s.id === currentSessionId) ??
+											sessions[0] ?? { name: 'Unknown', id: 'unknown' }
+									)}
 								</strong>{' '}
 								to reduce token usage while preserving key information.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/ContextManagementSheet.tsx` around lines 640 - 648, The code
can crash when sessions is empty because sessions.find(...) || sessions[0] may
be undefined and getSessionLabel(undefined) will throw; update the rendering to
guard against an empty sessions array by either (a) checking sessions.length > 0
before calling getSessionLabel and rendering a safe fallback string like
"Unknown session", or (b) making getSessionLabel accept an undefined/nullable
argument and return a safe label when session is missing; change the call site
in ContextManagementSheet (around the JSX using getSessionLabel and
currentSessionId) to use the chosen safe fallback.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fix the swipe-offset contract before shipping this gesture.

Line 86 destructures offsetX, and Line 124 uses it to drive the drawer transform, but the current useSwipeGestures implementation in src/web/hooks/useSwipeGestures.ts is not returning that property consistently. In this state the drawer may still close once the threshold is met, but it will not track the finger during the swipe, which makes the gesture feel broken.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/RightDrawer.tsx` around lines 84 - 125, The swipe gesture
currently used by RightDrawer expects useSwipeGestures to consistently return a
numeric offsetX (positive for rightward movement) and a boolean isSwiping so the
drawer can follow the finger; update the useSwipeGestures hook to always include
offsetX (default 0), isSwiping, and handlers in its return value, ensure the
trackOffset option actively updates offsetX during pointer moves and resets it
on end/cancel, and make sure the sign/direction matches RightDrawer's
expectation (offsetX > 0 moves drawer right); also export/update the hook's
TypeScript return type so RightDrawer's destructuring (handlers: swipeHandlers,
offsetX, isSwiping) is always valid.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Don’t swallow unexpected failures in fetchHistory.

This catch turns every exception into local UI state, so schema regressions, JSON parse failures, and other bugs disappear instead of reaching Sentry. It also makes the “Make sure the desktop app is running” hint misleading for non-network failures. Catch only the expected connectivity case here; report or rethrow everything else.

As per coding guidelines, "Do not silently swallow errors with try-catch blocks that only log. Let exceptions bubble up to Sentry for error tracking in production. Only catch and handle expected/recoverable errors explicitly (e.g., NETWORK_ERROR)." and "Use Sentry utilities for explicit reporting: import captureException and captureMessage from src/main/utils/sentry.ts or src/renderer/utils/sentry.ts to report exceptions with context and notable events."

Also applies to: 338-344

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/RightDrawer.tsx` around lines 294 - 316, The fetchHistory
function is currently catching all exceptions and turning them into UI state,
which hides unexpected failures; update fetchHistory to only handle expected
network/connectivity errors (e.g., check response.ok or inspect err.name/message
for fetch/TypeError) and setError with a user-friendly connectivity message in
that case, but for all other exceptions call captureException (import from
src/renderer/utils/sentry.ts) and rethrow or let them bubble so Sentry can
record them; ensure you import captureException, use it with context (e.g.,
include apiUrl, projectPath, sessionId), and apply the same change for the
similar error-handling block later in the file (the other fetch/history handling
around the second occurrence).

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/web/mobile/AllSessionsView.tsx (1)

194-245: ⚠️ Potential issue | 🟠 Major

Keep the card inert while inline rename is active.

The rename input stops click and touchstart, but the actual selection path is onTouchEnd on the outer card. On mobile, tapping into the input can still bubble up to the card and select/close the session list while the user is trying to edit the name.

🐛 Suggested fix

 		<CardContainer
-			onClick={handleClick}
-			onTouchStart={handleTouchStart}
-			onTouchEnd={handleTouchEnd}
-			onTouchMove={handleTouchMove}
-			onTouchCancel={handleTouchCancel}
-			onContextMenu={handleContextMenu}
+			onClick={isRenaming ? undefined : handleClick}
+			onTouchStart={isRenaming ? undefined : handleTouchStart}
+			onTouchEnd={isRenaming ? undefined : handleTouchEnd}
+			onTouchMove={isRenaming ? undefined : handleTouchMove}
+			onTouchCancel={isRenaming ? undefined : handleTouchCancel}
+			onContextMenu={isRenaming ? undefined : handleContextMenu}
 			style={{
@@
 						onKeyDown={handleRenameKeyDown}
 						onBlur={onRenameConfirm}
 						onClick={(e) => e.stopPropagation()}
 						onTouchStart={(e) => e.stopPropagation()}
+						onTouchEnd={(e) => e.stopPropagation()}
+						onTouchCancel={(e) => e.stopPropagation()}
 						style={{

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/AllSessionsView.tsx` around lines 194 - 245, When isRenaming
is true the outer CardContainer still receives touch/end/click handlers and the
outer onTouchEnd (handleTouchEnd) can activate selection even when editing; fix
by not attaching interactive handlers when renaming: render CardContainer as
'div' when isRenaming (already done) and conditionally omit or replace
handleClick, handleTouchStart, handleTouchEnd, handleTouchMove,
handleTouchCancel, and handleContextMenu with no-ops (or skip passing them) when
isRenaming so touch events inside the rename input (renameInputRef) cannot
bubble to the card and trigger selection.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/web/hooks/useWebSocket.ts`:
- Around line 709-721: The client-side sendRequest/pendingRequestsRef flow
expects server failure messages to include the original requestId so
pendingRequestsRef.current.has(requestId) matches and rejects immediately;
update the server handlers (notably create_session and rename_session in
messageHandlers.ts and any other paths around lines 1120-1152, 1440-1453,
1513-1523) to echo the incoming requestId when calling
sendError/sendErrorResponse (i.e., attach requestId: incomingRequestId to the
error message payload), so the client can correlate and settle the pending
promise instead of waiting for the timeout.

In `@src/web/mobile/AllSessionsView.tsx`:
- Around line 632-662: The sheet currently calls handleClose immediately after
invoking onMove, ignoring the Promise<boolean> result so failures still dismiss
the UI; change handleMove in MoveToGroupSheet to be async, await the result of
onMove(session.id, groupId), and only call handleClose (and any success
haptics/feedback) when the awaited boolean is true; if false, keep the sheet
open and surface an error (or leave as-is) so the user retains context. Apply
the same pattern to the other async callbacks in this file that call
onRename/onDelete (the rename/delete/move handlers referenced elsewhere) so they
await the Promise<boolean> and only dismiss on success.
- Around line 124-150: handleTouchEnd currently opens a session whenever
isLongPressTriggeredRef.current is false, so a touch-move (scroll) that cleared
the long-press timer still triggers onSelect; add a movement guard: create a ref
(e.g., isTouchMovedRef) initialized false, set isTouchMovedRef.current = true
inside handleTouchMove (and reset to false in handleTouchStart), then update
handleTouchEnd to bail out (skip triggerHaptic and onSelect) if
isTouchMovedRef.current is true (in addition to the long-press check), and
ensure you reset isTouchMovedRef.current = false after handling so subsequent
touches behave normally.

---

Outside diff comments:
In `@src/web/mobile/AllSessionsView.tsx`:
- Around line 194-245: When isRenaming is true the outer CardContainer still
receives touch/end/click handlers and the outer onTouchEnd (handleTouchEnd) can
activate selection even when editing; fix by not attaching interactive handlers
when renaming: render CardContainer as 'div' when isRenaming (already done) and
conditionally omit or replace handleClick, handleTouchStart, handleTouchEnd,
handleTouchMove, handleTouchCancel, and handleContextMenu with no-ops (or skip
passing them) when isRenaming so touch events inside the rename input
(renameInputRef) cannot bubble to the card and trigger selection.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b8e4bef4-371f-4f6f-afa2-3fb8e638e6dc

📥 Commits

Reviewing files that changed from the base of the PR and between 85bee58 and fac2201.

📒 Files selected for processing (3)

• src/main/web-server/WebServer.ts
• src/web/hooks/useWebSocket.ts
• src/web/mobile/AllSessionsView.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• src/main/web-server/WebServer.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Echo requestId on failure responses too.

sendRequest() now only settles when the server replies with a matching requestId, but the current create_session and rename_session failure paths in src/main/web-server/handlers/messageHandlers.ts (Lines 1440-1453 and 1513-1523 in the provided snippets) still call sendError() without it. Those requests will sit in pendingRequestsRef until the 10s timeout instead of rejecting immediately, so common validation/server failures look like a hung UI.

Also applies to: 1120-1152

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/hooks/useWebSocket.ts` around lines 709 - 721, The client-side
sendRequest/pendingRequestsRef flow expects server failure messages to include
the original requestId so pendingRequestsRef.current.has(requestId) matches and
rejects immediately; update the server handlers (notably create_session and
rename_session in messageHandlers.ts and any other paths around lines 1120-1152,
1440-1453, 1513-1523) to echo the incoming requestId when calling
sendError/sendErrorResponse (i.e., attach requestId: incomingRequestId to the
error message payload), so the client can correlate and settle the pending
promise instead of waiting for the timeout.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Cancel tap selection after a touch-move.

handleTouchMove() clears the long-press timer, but handleTouchEnd() still selects the session whenever isLongPressTriggeredRef.current is false. That means a normal scroll gesture that starts on a card can still open the session on finger lift.

🐛 Suggested fix

 	const longPressTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
 	const isLongPressTriggeredRef = useRef(false);
+	const didTouchMoveRef = useRef(false);
 	const renameInputRef = useRef<HTMLInputElement>(null);

 	const handleTouchStart = useCallback(
 		(e: React.TouchEvent) => {
 			isLongPressTriggeredRef.current = false;
+			didTouchMoveRef.current = false;
 			const touch = e.touches[0];
 			const x = touch.clientX;
 			const y = touch.clientY;
 			longPressTimerRef.current = setTimeout(() => {
 				isLongPressTriggeredRef.current = true;
@@
 	const handleTouchEnd = useCallback(() => {
 		clearLongPressTimer();
-		if (!isLongPressTriggeredRef.current) {
+		if (!isLongPressTriggeredRef.current && !didTouchMoveRef.current) {
 			triggerHaptic(HAPTIC_PATTERNS.tap);
 			onSelect(session.id);
 		}
 		isLongPressTriggeredRef.current = false;
+		didTouchMoveRef.current = false;
 	}, [clearLongPressTimer, onSelect, session.id]);

 	const handleTouchMove = useCallback(() => {
+		didTouchMoveRef.current = true;
 		clearLongPressTimer();
 	}, [clearLongPressTimer]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/AllSessionsView.tsx` around lines 124 - 150, handleTouchEnd
currently opens a session whenever isLongPressTriggeredRef.current is false, so
a touch-move (scroll) that cleared the long-press timer still triggers onSelect;
add a movement guard: create a ref (e.g., isTouchMovedRef) initialized false,
set isTouchMovedRef.current = true inside handleTouchMove (and reset to false in
handleTouchStart), then update handleTouchEnd to bail out (skip triggerHaptic
and onSelect) if isTouchMovedRef.current is true (in addition to the long-press
check), and ensure you reset isTouchMovedRef.current = false after handling so
subsequent touches behave normally.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Only dismiss the rename/delete/move UI after the async action succeeds.

These callbacks are explicitly typed as Promise<boolean>, but the return value is ignored everywhere. Right now a failed rename/delete/move still clears the editor/dialog/sheet, so the user loses context and the failure looks like success.

🐛 Suggested fix

 function MoveToGroupSheet({
 	session,
 	groups,
 	onMove,
 	onClose,
 }: {
 	session: Session;
 	groups: GroupData[];
-	onMove: (sessionId: string, groupId: string | null) => void;
+	onMove: (sessionId: string, groupId: string | null) => Promise<boolean>;
 	onClose: () => void;
 }) {
@@
-	const handleMove = useCallback(
-		(groupId: string | null) => {
+	const handleMove = useCallback(
+		async (groupId: string | null) => {
 			triggerHaptic(HAPTIC_PATTERNS.tap);
-			onMove(session.id, groupId);
-			handleClose();
+			const moved = await onMove(session.id, groupId);
+			if (moved) {
+				handleClose();
+			}
 		},
 		[session.id, onMove, handleClose]
 	);
@@
 	const handleRenameConfirm = useCallback(async () => {
 		if (!renamingSessionId || !renameValue.trim() || !onRenameAgent) {
 			setRenamingSessionId(null);
 			return;
 		}
-		await onRenameAgent(renamingSessionId, renameValue.trim());
-		setRenamingSessionId(null);
+		const renamed = await onRenameAgent(renamingSessionId, renameValue.trim());
+		if (renamed) {
+			setRenamingSessionId(null);
+		}
 	}, [renamingSessionId, renameValue, onRenameAgent]);
@@
 	const handleDeleteConfirm = useCallback(async () => {
 		if (!deleteSession || !onDeleteAgent) return;
-		await onDeleteAgent(deleteSession.id);
-		setDeleteSession(null);
+		const deleted = await onDeleteAgent(deleteSession.id);
+		if (deleted) {
+			setDeleteSession(null);
+		}
 	}, [deleteSession, onDeleteAgent]);
@@
 	const handleMoveToGroup = useCallback(
 		async (sessionId: string, groupId: string | null) => {
-			if (!onMoveToGroup) return;
-			await onMoveToGroup(sessionId, groupId);
+			if (!onMoveToGroup) return false;
+			return onMoveToGroup(sessionId, groupId);
 		},
 		[onMoveToGroup]
 	);

Also applies to: 1251-1279

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/AllSessionsView.tsx` around lines 632 - 662, The sheet
currently calls handleClose immediately after invoking onMove, ignoring the
Promise<boolean> result so failures still dismiss the UI; change handleMove in
MoveToGroupSheet to be async, await the result of onMove(session.id, groupId),
and only call handleClose (and any success haptics/feedback) when the awaited
boolean is true; if false, keep the sheet open and surface an error (or leave
as-is) so the user retains context. Apply the same pattern to the other async
callbacks in this file that call onRename/onDelete (the rename/delete/move
handlers referenced elsewhere) so they await the Promise<boolean> and only
dismiss on success.