feat(acp): Session persistence, connection health and UI polish#45
Merged
Conversation
- Add Rust-side heartbeat task with 15s ping interval and 3-strike disconnect - Clean up dead connections in `acp_connect` and `acp_check_health` - Emit structured connection log events via `acp:log` - Add `HeartbeatEvent`, `ConnectionLogEntry` and `ConnectionConfig` types - Add `chat_panel_size` field to session persistence
…istory - Add in-memory session cache with global event listener independent of React lifecycle - Keep DirectoryWorkspace mounted via CSS hidden when minimized - Add `persistedWorkspaceId` to AppContext for mount/visibility separation - Always call `acp_load_session` to replay full conversation from ACP backend - Restore cached messages on "already loaded" fallback - Add periodic 30s heartbeat in `useAcpConnection` for proactive disconnect detection - Remove auto-disconnect on unmount; move disconnect to explicit close/forget - Add connection logs panel with `useAcpLogs` hook and `ConnectionLogs` component - Add i18n strings for connection logs and ACP status
- Test connect/disconnect lifecycle, status events, workspace isolation - Test cached state restoration and health check on mount - Verify disconnect is not called on unmount - Test log capture, filtering and clear behavior
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBackend: Hardened ACP connection lifecycle with heartbeat, timed JSON-RPC requests, structured connection logs, and deterministic cleanup. Frontend: session-cache stores and subscriptions, connection log aggregation + UI, connect/disconnect flow, chat-panel size persistence, new hooks/tests, and added types/locales. Changes
Sequence Diagram(s)sequenceDiagram
participant FE as Frontend<br/>(useAcpConnection / UI)
participant Cmd as Rust Command<br/>(acp_connect / acp_check_health)
participant Conn as AcpConnection<br/>(heartbeat task)
participant Proc as Child Process<br/>(Agent)
participant EVT as Event Emitter<br/>(acp:connection-status / acp:heartbeat / acp:log)
FE->>Cmd: acp_connect(workspaceId, ...)
Cmd->>Cmd: remove existing connection from map
alt existing alive
Cmd-->>FE: reinsert & return (reuse)
else dead or missing
Cmd->>Proc: spawn agent
Cmd->>Conn: start reader + heartbeat
Cmd->>EVT: emit_log("info","connect","started")
end
loop heartbeat (every 30s or on visibility)
Conn->>Proc: JSON-RPC "ping" with timeout
alt response within timeout
Proc-->>Conn: pong
Conn->>EVT: emit heartbeat("healthy", latency)
else timeout/failure
Conn->>Conn: increment fail count
Conn->>EVT: emit heartbeat("degraded"/"disconnected")
alt fail_count >= 3
Conn->>Proc: terminate
Conn->>EVT: emit_log("error","heartbeat","disconnected")
end
end
end
FE->>EVT: listen events
EVT-->>FE: update UI, logs, cache
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…und" - Reset `initRef` when `isConnected` goes false so reconnect triggers `doInit` and reloads the session in the new copilot process - Auto-recover in `sendPrompt`: on "Session not found" (-32602), try `acp_load_session` then retry the prompt before surfacing the error
If no streaming event arrives within 60s, `isStreaming` is reset to false automatically. Prevents the Stop button from staying red indefinitely when `end_turn` is never received.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/tauri/src-tauri/src/acp/connection.rs (1)
343-358:⚠️ Potential issue | 🟠 MajorRemove pending request entries on all error exits.
send_request_with_timeoutinserts intopendingbut does not clean up on writer send failure, timeout, or dropped response channel.Suggested cleanup patch
self.writer_tx .send(line) .await - .map_err(|_| "Writer channel closed".to_string())?; + .map_err(|_| { + // ensure no stale pending entry on send failure + // (remove in a fire-and-forget async block is not possible here, + // so do it synchronously below before returning) + "Writer channel closed".to_string() + })?; - let result = tokio::time::timeout(timeout, rx) - .await - .map_err(|_| format!("Timeout waiting for response to {}", method))? - .map_err(|_| "Response channel dropped".to_string())?; + let result = match tokio::time::timeout(timeout, rx).await { + Ok(Ok(v)) => v, + Ok(Err(_)) => { + self.pending.lock().await.remove(&id); + return Err("Response channel dropped".to_string()); + } + Err(_) => { + self.pending.lock().await.remove(&id); + return Err(format!("Timeout waiting for response to {}", method)); + } + }; result.map_err(|e| e.to_string())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/acp/connection.rs` around lines 343 - 358, send_request_with_timeout currently inserts (id, tx) into self.pending but returns early on errors (writer_tx.send failure, timeout, or rx closed) without removing that entry; update the function so that any error path removes the pending entry for id before returning. Locate the block that creates oneshot::channel and inserts into self.pending (use symbols id, tx, rx, pending_guard, self.pending, writer_tx.send, tokio::time::timeout) and ensure you remove the pending entry (e.g., lock self.pending and remove(id)) on all failure branches (writer send Err, timeout Err, rx.map_err Err) — use an explicit cleanup before each early return or add a small scope-guard helper to automatically remove the entry on drop to avoid leaks.
🧹 Nitpick comments (8)
apps/tauri/src/components/ErrorConsole.tsx (1)
14-19: Keep error indicators on semantic destructive text tokensSwitching icon/message/count text to
text-foreground/*weakens error-state semantics. For this error console, use destructive foreground tokens for status indicators.♻️ Proposed patch
- <AlertCircle className="h-3.5 w-3.5 text-foreground/60 mt-0.5 shrink-0" /> - <p className="flex-1 font-mono text-xs text-foreground/70 break-words leading-relaxed"> + <AlertCircle className="h-3.5 w-3.5 text-destructive mt-0.5 shrink-0" /> + <p className="flex-1 font-mono text-xs text-destructive/90 break-words leading-relaxed"> {lastError} {errors.length > 1 && ( - <span className="ml-2 text-foreground/40">(+{errors.length - 1})</span> + <span className="ml-2 text-destructive/70">(+{errors.length - 1})</span> )} </p>Based on learnings: “Use semantic SHADCN UI tokens for status indicators… use
text-destructive(foreground variants) instead of non-semantic colors.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/ErrorConsole.tsx` around lines 14 - 19, The icon/message/count currently use non-semantic foreground tokens; update the ErrorConsole UI to use destructive foreground tokens for error semantics by replacing text-foreground/* with text-destructive/* where the error is rendered (in apps/tauri/src/components/ErrorConsole.tsx) — specifically adjust the AlertCircle className, the <p> that renders {lastError}, and the <span> that renders the count ( (+{errors.length - 1}) ) so they use text-destructive (and appropriate opacity variants) instead of text-foreground to preserve semantic error styling.apps/tauri/src/components/DirectoryWorkspace.tsx (1)
31-39: Consider adding workspaceId to dependency array for cache invalidation.The effect restores
cachedMountedSessionIdon mount, but ifworkspace.idchanges while sessions are still loading, the stalecachedMountedSessionIdfrom the previous workspace could be used since the variable is derived outside the effect.Currently safe because
cachedMountedSessionIdis re-derived on each render from the currentworkspace, but explicitly listingworkspace?.idin the dependency array would make the intent clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/DirectoryWorkspace.tsx` around lines 31 - 39, The useEffect that restores cachedMountedSessionId should also depend on workspace?.id to ensure the cached session from a previous workspace isn't reused; update the dependency array of the useEffect containing cachedMountedSessionId, local.sessions, and local.loading to include workspace?.id so the effect re-runs when the workspace changes and correctly validates mountedSessionRef/current and calls setBrowsing(false) only for the current workspace.apps/tauri/src/components/ConnectionLogs.tsx (3)
29-34: Consider adding user feedback after copy.The copy action completes silently. A brief toast or visual confirmation would improve UX.
♻️ Example using sonner toast
+import { toast } from "sonner"; + const handleCopy = () => { const text = logs .map((l) => `[${l.timestamp}] [${l.level.toUpperCase()}] ${l.event}: ${l.message}`) .join("\n"); - navigator.clipboard.writeText(text); + navigator.clipboard.writeText(text).then(() => { + toast.success(t("review.copied")); + }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/ConnectionLogs.tsx` around lines 29 - 34, handleCopy currently writes logs to the clipboard silently; update handleCopy to await navigator.clipboard.writeText(text) and then show user feedback (success or error) — e.g., call a toast or set a short-lived UI state — so users see confirmation on success and an error message on failure; reference the handleCopy function and the clipboard writeText call, and use your app's existing toast utility (or add sonner's toast.success / toast.error) to display the messages.
46-48: Consider using semantic token for error badge.The error indicator badge uses hardcoded
bg-orange-500. For theme consistency, considerbg-warningorbg-destructive.♻️ Proposed fix
{hasRecentErrors && ( - <span className="absolute top-0.5 right-0.5 w-1.5 h-1.5 rounded-full bg-orange-500" /> + <span className="absolute top-0.5 right-0.5 w-1.5 h-1.5 rounded-full bg-warning" /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/ConnectionLogs.tsx` around lines 46 - 48, Replace the hardcoded tailwind color on the error badge in the ConnectionLogs component: locate the JSX that renders the small indicator when hasRecentErrors (the <span className="... bg-orange-500" />) and swap the fixed color class for a semantic token class (e.g., bg-warning or bg-destructive) so the badge follows theme tokens; update the className on that span accordingly and run the UI to verify the badge adopts the theme color.
14-18: Use semantic SHADCN UI tokens instead of hardcoded Tailwind colors.Per coding guidelines, status indicators should use theme-aware semantic tokens rather than hardcoded colors for proper light/dark mode support.
♻️ Proposed fix using semantic tokens
const LEVEL_STYLES: Record<string, string> = { - info: "text-blue-500", - warn: "text-yellow-500", - error: "text-red-500", + info: "text-muted-foreground", + warn: "text-warning", + error: "text-destructive", };Based on learnings: "Use text-success, text-warning, and text-destructive tokens (foreground variants) instead of hardcoded colors."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/ConnectionLogs.tsx` around lines 14 - 18, Replace the hardcoded Tailwind color classes in the LEVEL_STYLES constant with the semantic shadcn UI foreground tokens so the UI respects light/dark themes; update the LEVEL_STYLES mapping (const LEVEL_STYLES) to use "text-success", "text-warning", and "text-destructive" (or the project’s equivalent foreground variants) for "info", "warn", and "error" respectively, and keep all usages (e.g., where LEVEL_STYLES[level] is applied in ConnectionLogs.tsx) unchanged so styling becomes theme-aware.apps/tauri/src/hooks/useAcpConnection.ts (1)
96-110: Potential duplicate status update on mount.When a cached connection exists, this effect calls
acp_check_healthwhich both:
- Returns the status (handled here at lines 100-104)
- Emits
acp:connection-statusevent (per context snippet from commands.rs line 279-280)The event listener at line 50 will also update state, causing a double state update. This is harmless but slightly redundant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/hooks/useAcpConnection.ts` around lines 96 - 110, The mount effect calling acp_check_health can cause duplicate updates because the same status is also emitted via the acp:connection-status event; to avoid redundant state writes, update connectedRef, setConnectionStatus and setConnectionError only if the returned status differs from current state: inside the .then handler for acp_check_health, compare the resolved status (s) against connectedRef.current or the current connectionStatus and only call connectedRef.current = s === "connected", setConnectionStatus(s), or setConnectionError(null) when there's an actual change; keep the event listener logic (the handler registered elsewhere) unchanged.apps/tauri/src/lib/session-cache.ts (2)
54-146: Consider extracting case handlers for improved testability.The
processSessionUpdatefunction handles multiple update types correctly, but its size (90+ lines) makes it harder to unit test individual handlers. Consider extracting each case into a named helper function.♻️ Example refactor for one handler
function handleAgentMessageChunk( msgs: AcpMessage[], p: Record<string, unknown>, nextMsgId: () => string ): { msgs: AcpMessage[]; isStreaming: boolean } { const content = p.content as Record<string, unknown> | undefined; const text = content?.type === "text" ? (content.text as string) : ""; if (!text) return { msgs, isStreaming: false }; if (/^(Warning:|Info:|🔬|Experimental)/.test(text)) { msgs.push({ id: nextMsgId(), role: "assistant", type: "notice", content: text, timestamp: new Date() }); } else { const last = msgs[msgs.length - 1]; if (last?.role === "assistant" && !last.type) { msgs[msgs.length - 1] = { ...last, content: last.content + text }; } else { msgs.push({ id: nextMsgId(), role: "assistant", content: text, timestamp: new Date() }); } } return { msgs, isStreaming: true }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/lib/session-cache.ts` around lines 54 - 146, The function processSessionUpdate is large and hard to unit-test; extract each switch case into small, named helper functions (e.g., handleAgentMessageChunk, handleAgentThoughtChunk, handleToolCall, handleToolCallUpdate, handleUserMessageChunk, handleCurrentModeUpdate) that accept the mutable pieces (msgs, p, nextMsgId, entry fields) and return updated fragments (msgs, isStreaming, agentPlanFilePath, currentMode) so processSessionUpdate becomes a thin dispatcher that calls these helpers and composes their results; ensure each helper is exported for unit tests and that existing symbols (nextMsgId, msgs array mutation logic, msg properties like toolCallId/toolTitle/toolStatus) are preserved exactly when refactoring.
158-174: Consider storing the unlisten function for proper cleanup.The
listen()call returns a Promise that resolves to an unlisten function, but it's currently discarded. While this singleton pattern is acceptable for production, it can cause issues during development with Hot Module Replacement (HMR), where the listener may accumulate across module reloads.♻️ Proposed fix to store unlisten function
let listenerSetup = false; +let unlistenFn: (() => void) | null = null; + function ensureGlobalListener() { if (listenerSetup) return; listenerSetup = true; - window.__TAURI__.event.listen<AcpSessionUpdate>("acp:session-update", (event: { payload: AcpSessionUpdate }) => { + window.__TAURI__.event.listen<AcpSessionUpdate>("acp:session-update", (event: { payload: AcpSessionUpdate }) => { const update = event.payload; const { workspaceId, sessionId } = update; const entry = sessions.get(workspaceId); if (!entry) return; if (entry.activeAcpSessionId && sessionId !== entry.activeAcpSessionId) return; const newEntry = processSessionUpdate(entry, update); sessions.set(workspaceId, newEntry); notifySubscribers(workspaceId, newEntry); - }).catch(console.error); + }).then((fn) => { + unlistenFn = fn; + }).catch(console.error); } + +// Optional: Export for cleanup during HMR or testing +export function teardownGlobalListener() { + if (unlistenFn) { + unlistenFn(); + unlistenFn = null; + listenerSetup = false; + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/lib/session-cache.ts` around lines 158 - 174, The ensureGlobalListener currently discards the Promise returned by window.__TAURI__.event.listen, causing orphaned listeners during HMR; capture and store the unlisten function (e.g., in a module-level variable like unlistenAcpSession) when listen() resolves, and before setting up a new listener (or when resetting listenerSetup) call that stored unlisten function to remove the previous listener; alternatively export a cleanup function (e.g., cleanupSessionCacheListeners) that calls the stored unlisten and resets listenerSetup so tests/HMR can call it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src-tauri/src/acp/commands.rs`:
- Around line 31-39: You're awaiting async operations while holding the
connections mutex; instead clone or take ownership of the connection under the
lock, drop the lock, then perform async checks/shutdowns. Concretely: when
handling state.connections.lock().await, if let Some(existing) =
connections.get(&workspace_id) { clone the connection (e.g., existing.clone() or
Arc::clone(existing)), drop the connections lock, await cloned.is_alive().await
and return if alive; if not alive, re-acquire the connections lock and remove
the entry (connections.remove(&workspace_id)), drop the lock, then await
dead.shutdown().await and finally lock state.configs to remove(&workspace_id).
Apply the same pattern to the other occurrence around lines 276-286.
In `@apps/tauri/src/components/ActiveSessionView.tsx`:
- Around line 127-136: The reconnect flow in handleReconnect currently aborts if
onDisconnect() rejects, so change handleReconnect to catch errors from
onDisconnect while still proceeding to call onConnect(); specifically, wrap the
await onDisconnect() call in a try/catch (or Promise.catch) and log or
setInitError with the disconnect error, but do not rethrow so that the
subsequent await onConnect() (and any initRef.current reset) always runs; update
references in handleReconnect to ensure onConnect is invoked regardless of
onDisconnect outcome and preserve existing state resets (initRef.current = false
and setInitError(null)).
In `@apps/tauri/src/components/ErrorConsole.tsx`:
- Around line 22-25: Replace the native <button> in ErrorConsole.tsx with the
shadcn/ui Button primitive (import Button from your app's Button/shadcn wrapper)
and replace the hardcoded aria-label "Dismiss" with a localized string from
useTranslation() (call useTranslation() in the ErrorConsole component and use
t('dismiss') or the appropriate key); keep the onClick handler (onClear) and
className/props behavior when migrating to Button so functionality and styles
remain the same.
In `@apps/tauri/src/components/TerminalMessage.tsx`:
- Line 73: Replace the hardcoded hex color class in the span inside
TerminalMessage.tsx (the element rendering the "done" status) with a semantic
SHADCN status token; for example swap text-[`#3dd68c`]/60 for a semantic class
such as text-success or text-success/60 (or text-warning/text-destructive where
appropriate) so the status color follows theme tokens and respects light/dark
styling.
- Around line 73-74: Import and use the useTranslation() hook in TerminalMessage
(e.g., const { t } = useTranslation()) and replace all hardcoded user-facing
strings ("done", "Thinking…", and the "{contentLines.length} lines..." message)
with translation keys using t('...') (add corresponding keys like terminal.done,
terminal.thinking, terminal.linesCount) and update the JSX spans where those
strings appear (including the occurrences around lines 80-83 and 100-103) to
call t and format the lines count (e.g., t('terminal.linesCount', { count:
contentLines.length })). Also replace the hardcoded tailwind color class
text-[`#3dd68c`]/60 with the semantic token text-success/60 to use theme tokens.
In `@apps/tauri/src/hooks/useAcpSession.ts`:
- Around line 73-82: The code only resets messages and isStreaming when loading
a cache entry via sessionStore.get(workspaceId), which can leave stale UI state
in currentMode, availableModes, activeAcpSessionId, agentPlanFilePath and
activeAcpSessionIdRef; create a single helper (e.g., resetAcpSessionState) that
clears/initializes all related state (setMessages, setIsStreaming,
setCurrentMode, setAvailableModes, setActiveAcpSessionId, setAgentPlanFilePath
and activeAcpSessionIdRef.current) and call it whenever an entry is missing or
you're replacing the cache entry (replace the current partial reset block around
sessionStore.get(workspaceId) and apply the same fix to the analogous blocks
around the other occurrences mentioned).
In `@apps/tauri/src/index.css`:
- Around line 599-605: Replace hardcoded hex colors in the CSS selectors (e.g.,
.terminal-markdown code and the other affected selectors around lines 721–730)
with theme-aware HSL CSS variables or Tailwind semantic tokens; update the color
declarations (currently using `#56d4dd` and `#3dd68c`) to reference your app's HSL
variables (for example --color-accent-h / --color-accent-s / --color-accent-l or
a single --color-accent) or Tailwind theme classes so they respect light/dark
modes, and keep other properties (background, border, padding, font-size)
unchanged; ensure you use the same variable names consistently across all
affected selectors to preserve theming.
- Around line 466-472: In .tool-content-inline replace the deprecated rule
word-break: break-word with overflow-wrap: anywhere and ensure color uses the
HSL theme variable (e.g., change color: hsl(var(--muted-foreground) / 0.7) if
not already); also find the places that use the hardcoded hex colors (`#56d4dd`
and `#3dd68c`) and replace them with the appropriate HSL CSS variables (e.g., use
hsl(var(--accent-foreground)) or another project-specific --* HSL variable) so
all color usages follow the HSL variable theming guideline.
---
Outside diff comments:
In `@apps/tauri/src-tauri/src/acp/connection.rs`:
- Around line 343-358: send_request_with_timeout currently inserts (id, tx) into
self.pending but returns early on errors (writer_tx.send failure, timeout, or rx
closed) without removing that entry; update the function so that any error path
removes the pending entry for id before returning. Locate the block that creates
oneshot::channel and inserts into self.pending (use symbols id, tx, rx,
pending_guard, self.pending, writer_tx.send, tokio::time::timeout) and ensure
you remove the pending entry (e.g., lock self.pending and remove(id)) on all
failure branches (writer send Err, timeout Err, rx.map_err Err) — use an
explicit cleanup before each early return or add a small scope-guard helper to
automatically remove the entry on drop to avoid leaks.
---
Nitpick comments:
In `@apps/tauri/src/components/ConnectionLogs.tsx`:
- Around line 29-34: handleCopy currently writes logs to the clipboard silently;
update handleCopy to await navigator.clipboard.writeText(text) and then show
user feedback (success or error) — e.g., call a toast or set a short-lived UI
state — so users see confirmation on success and an error message on failure;
reference the handleCopy function and the clipboard writeText call, and use your
app's existing toast utility (or add sonner's toast.success / toast.error) to
display the messages.
- Around line 46-48: Replace the hardcoded tailwind color on the error badge in
the ConnectionLogs component: locate the JSX that renders the small indicator
when hasRecentErrors (the <span className="... bg-orange-500" />) and swap the
fixed color class for a semantic token class (e.g., bg-warning or
bg-destructive) so the badge follows theme tokens; update the className on that
span accordingly and run the UI to verify the badge adopts the theme color.
- Around line 14-18: Replace the hardcoded Tailwind color classes in the
LEVEL_STYLES constant with the semantic shadcn UI foreground tokens so the UI
respects light/dark themes; update the LEVEL_STYLES mapping (const LEVEL_STYLES)
to use "text-success", "text-warning", and "text-destructive" (or the project’s
equivalent foreground variants) for "info", "warn", and "error" respectively,
and keep all usages (e.g., where LEVEL_STYLES[level] is applied in
ConnectionLogs.tsx) unchanged so styling becomes theme-aware.
In `@apps/tauri/src/components/DirectoryWorkspace.tsx`:
- Around line 31-39: The useEffect that restores cachedMountedSessionId should
also depend on workspace?.id to ensure the cached session from a previous
workspace isn't reused; update the dependency array of the useEffect containing
cachedMountedSessionId, local.sessions, and local.loading to include
workspace?.id so the effect re-runs when the workspace changes and correctly
validates mountedSessionRef/current and calls setBrowsing(false) only for the
current workspace.
In `@apps/tauri/src/components/ErrorConsole.tsx`:
- Around line 14-19: The icon/message/count currently use non-semantic
foreground tokens; update the ErrorConsole UI to use destructive foreground
tokens for error semantics by replacing text-foreground/* with
text-destructive/* where the error is rendered (in
apps/tauri/src/components/ErrorConsole.tsx) — specifically adjust the
AlertCircle className, the <p> that renders {lastError}, and the <span> that
renders the count ( (+{errors.length - 1}) ) so they use text-destructive (and
appropriate opacity variants) instead of text-foreground to preserve semantic
error styling.
In `@apps/tauri/src/hooks/useAcpConnection.ts`:
- Around line 96-110: The mount effect calling acp_check_health can cause
duplicate updates because the same status is also emitted via the
acp:connection-status event; to avoid redundant state writes, update
connectedRef, setConnectionStatus and setConnectionError only if the returned
status differs from current state: inside the .then handler for
acp_check_health, compare the resolved status (s) against connectedRef.current
or the current connectionStatus and only call connectedRef.current = s ===
"connected", setConnectionStatus(s), or setConnectionError(null) when there's an
actual change; keep the event listener logic (the handler registered elsewhere)
unchanged.
In `@apps/tauri/src/lib/session-cache.ts`:
- Around line 54-146: The function processSessionUpdate is large and hard to
unit-test; extract each switch case into small, named helper functions (e.g.,
handleAgentMessageChunk, handleAgentThoughtChunk, handleToolCall,
handleToolCallUpdate, handleUserMessageChunk, handleCurrentModeUpdate) that
accept the mutable pieces (msgs, p, nextMsgId, entry fields) and return updated
fragments (msgs, isStreaming, agentPlanFilePath, currentMode) so
processSessionUpdate becomes a thin dispatcher that calls these helpers and
composes their results; ensure each helper is exported for unit tests and that
existing symbols (nextMsgId, msgs array mutation logic, msg properties like
toolCallId/toolTitle/toolStatus) are preserved exactly when refactoring.
- Around line 158-174: The ensureGlobalListener currently discards the Promise
returned by window.__TAURI__.event.listen, causing orphaned listeners during
HMR; capture and store the unlisten function (e.g., in a module-level variable
like unlistenAcpSession) when listen() resolves, and before setting up a new
listener (or when resetting listenerSetup) call that stored unlisten function to
remove the previous listener; alternatively export a cleanup function (e.g.,
cleanupSessionCacheListeners) that calls the stored unlisten and resets
listenerSetup so tests/HMR can call it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d7e8c6e-d930-4b69-8093-826ad486150c
📒 Files selected for processing (26)
apps/tauri/src-tauri/src/acp/commands.rsapps/tauri/src-tauri/src/acp/connection.rsapps/tauri/src-tauri/src/acp/types.rsapps/tauri/src-tauri/src/lib.rsapps/tauri/src-tauri/src/sessions.rsapps/tauri/src/App.tsxapps/tauri/src/__tests__/hooks/useAcpConnection.test.tsapps/tauri/src/__tests__/hooks/useAcpLogs.test.tsapps/tauri/src/components/ActiveSessionView.tsxapps/tauri/src/components/ConnectionLogs.tsxapps/tauri/src/components/DirectoryWorkspace.tsxapps/tauri/src/components/ErrorConsole.tsxapps/tauri/src/components/MarkdownViewer.tsxapps/tauri/src/components/TerminalChat.tsxapps/tauri/src/components/TerminalMessage.tsxapps/tauri/src/contexts/AppContext.tsxapps/tauri/src/hooks/useAcpConnection.tsapps/tauri/src/hooks/useAcpLogs.tsapps/tauri/src/hooks/useAcpSession.tsapps/tauri/src/index.cssapps/tauri/src/lib/format-path.tsapps/tauri/src/lib/session-cache.tsapps/tauri/src/locales/en.jsonapps/tauri/src/locales/pt-BR.jsonapps/tauri/src/types/acp.tsapps/tauri/src/types/index.ts
Comment on lines
73
to
74
| <span className="ml-1.5 text-[#3dd68c]/60 text-[10px]">done</span> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file content around the mentioned lines
cat -n apps/tauri/src/components/TerminalMessage.tsx | sed -n '65,110p'Repository: devitools/arandu
Length of output: 2116
🏁 Script executed:
# Check if useTranslation is imported in this file
rg "useTranslation|i18n" apps/tauri/src/components/TerminalMessage.tsx -A 2 -B 2Repository: devitools/arandu
Length of output: 42
🏁 Script executed:
# Check existing translation files for these keys
find src/locales -name "*.json" -type f | head -5Repository: devitools/arandu
Length of output: 106
🏁 Script executed:
# Check if these strings are already translated
rg "done|Thinking|lines" src/locales/ -t jsonRepository: devitools/arandu
Length of output: 112
🏁 Script executed:
# Search for locale files
find . -name "*.json" -path "*locale*" -o -name "*.json" -path "*i18n*" | head -10Repository: devitools/arandu
Length of output: 128
🏁 Script executed:
# Look for i18n configuration
rg "i18n|locales" -t json -t js -t ts . --max-count=5 | head -20Repository: devitools/arandu
Length of output: 1591
🏁 Script executed:
# Check if there's an i18n config file
fd "i18n" . --type fRepository: devitools/arandu
Length of output: 88
🏁 Script executed:
# Check if TerminalMessage.tsx has useTranslation imported
head -30 apps/tauri/src/components/TerminalMessage.tsx | grep -i "import.*useTranslation\|react-i18next"Repository: devitools/arandu
Length of output: 42
🏁 Script executed:
# Check the entire locale file to see existing structure
cat apps/tauri/src/locales/en.jsonRepository: devitools/arandu
Length of output: 8672
🏁 Script executed:
# Check theme/color configuration
cat apps/tauri/src/lib/utils.ts | head -50Repository: devitools/arandu
Length of output: 228
🏁 Script executed:
# Check Tailwind theme or CSS variables
find . -name "tailwind.config.*" -o -name "globals.css" -o -name "*.css" | grep -E "(tailwind|global)" | head -5Repository: devitools/arandu
Length of output: 91
🏁 Script executed:
# Check tailwind config for theme colors
cat apps/tauri/tailwind.config.tsRepository: devitools/arandu
Length of output: 3924
🏁 Script executed:
# Check globals.css for color variables
find apps/tauri/src -name "globals.css" -o -name "*.css" | xargs cat 2>/dev/null | grep -E "color|--" | head -30Repository: devitools/arandu
Length of output: 984
🏁 Script executed:
# Check if the file imports useTranslation
head -15 apps/tauri/src/components/TerminalMessage.tsxRepository: devitools/arandu
Length of output: 467
Localize user-facing strings and use semantic color tokens.
The strings "done", "Thinking…", and {contentLines.length} lines... must be moved to translation keys. Additionally, replace the hardcoded color text-[#3dd68c]/60 with the semantic token text-success/60 to maintain theme consistency.
Per coding guidelines, use useTranslation() hook from react-i18next for all user-facing strings and TailwindCSS semantic color tokens instead of hardcoded hex values.
Also applies to: 80-83, 100-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/tauri/src/components/TerminalMessage.tsx` around lines 73 - 74, Import
and use the useTranslation() hook in TerminalMessage (e.g., const { t } =
useTranslation()) and replace all hardcoded user-facing strings ("done",
"Thinking…", and the "{contentLines.length} lines..." message) with translation
keys using t('...') (add corresponding keys like terminal.done,
terminal.thinking, terminal.linesCount) and update the JSX spans where those
strings appear (including the occurrences around lines 80-83 and 100-103) to
call t and format the lines count (e.g., t('terminal.linesCount', { count:
contentLines.length })). Also replace the hardcoded tailwind color class
text-[`#3dd68c`]/60 with the semantic token text-success/60 to use theme tokens.
Contributor
Author
There was a problem hiding this comment.
Valid follow-up. Will also localize the hardcoded "done" label via useTranslation() along with the hex color fix from the previous comment.
There was a problem hiding this comment.
@wilcorrea, sounds good! Here's a quick reference for the changes when you get to it:
Locale keys to add (in both en.json and pt-BR.json under a terminal section):
"terminal": {
"done": "done",
"thinking": "Thinking…",
"linesCount": "{{count}} lines..."
}Component update sketch:
import { useTranslation } from "react-i18next";
// inside the component:
const { t } = useTranslation();
// Replace hardcoded strings:
// "done" → t("terminal.done")
// "Thinking…" → t("terminal.thinking")
// `${contentLines.length} lines...` → t("terminal.linesCount", { count: contentLines.length })
// Replace color token:
// text-[`#3dd68c`]/60 → text-success/60Note that text-success is already defined in apps/tauri/src/index.css under both :root and .dark (as established in a previous PR), so the semantic token should be available directly.
🧠 Learnings used
Learnt from: CR
Repo: devitools/arandu PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T05:28:33.736Z
Learning: Applies to apps/tauri/src/**/*.{ts,tsx} : Use `useTranslation()` hook from react-i18next for all user-facing strings
Learnt from: wilcorrea
Repo: devitools/arandu PR: 41
File: apps/tauri/src/components/settings/DiagnosticsSettings.tsx:0-0
Timestamp: 2026-03-02T22:46:53.564Z
Learning: Enforce using semantic SHADCN UI tokens for status indicators in TSX components under apps/tauri/src. Use text-success, text-warning, and text-destructive tokens (foreground variants) instead of hardcoded colors. These tokens are defined in apps/tauri/src/index.css under both :root (light) and .dark (dark mode) and registered in tailwind.config.ts. Apply this pattern broadly to TSX files in apps/tauri/src (not just DiagnosticsSettings.tsx) to ensure theme-aware color usage for status icons and indicators.
Learnt from: wilcorrea
Repo: devitools/arandu PR: 41
File: apps/tauri/src/components/settings/DiagnosticsSettings.tsx:26-27
Timestamp: 2026-03-02T22:40:16.330Z
Learning: The shadcn/ui design system in this project currently only defines `text-destructive` as a semantic color token. Semantic tokens for success (`text-success`) and warning (`text-warning`) states need to be added to `apps/tauri/src/index.css` under both `:root` (light) and `.dark` (dark mode) sections before status icons can use theme-aware colors throughout the app.
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to **/*.{tsx,ts} : Use `useTranslation()` hook for all user-facing text in React components
Learnt from: CR
Repo: devitools/arandu PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T05:28:33.736Z
Learning: Applies to apps/tauri/src/**/*.{css,tsx} : Use TailwindCSS with HSL CSS variables for theming instead of hardcoded color values
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to src/lib/tray-sync.ts : For tray menu translations, update both the locale files and the `update_tray_labels` Rust command in `lib/tray-sync.ts`
Learnt from: CR
Repo: devitools/arandu PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T05:28:33.736Z
Learning: Applies to apps/tauri/src/**/*.{ts,tsx} : Use the `cn()` utility from `lib/utils.ts` for merging Tailwind CSS classes
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to src/lib/i18n.ts : Synchronize i18n across layers using `i18n.on('languageChanged')` for React → localStorage, `storage` event listener for localStorage → React, and `updateTrayLabels()` command for React → Rust
Contributor
Author
There was a problem hiding this comment.
Fixed in 2778aaf. Also localized the hardcoded "done" label via useTranslation() with key terminal.done.
There was a problem hiding this comment.
Rate Limit Exceeded
@wilcorrea have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 4 seconds before sending another message.
Contributor
Author
There was a problem hiding this comment.
Fixed in 2778aaf. Also localized the hardcoded "done" label via useTranslation() with key terminal.done.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
git show 2778aaf -- apps/tauri/src/components/TerminalMessage.tsx 2>/dev/null || echo "Commit not found, checking recent commits..."
git log --oneline -10
Length of output: 824
🏁 Script executed:
#!/bin/bash
# Check all branches for the commit
git fetch --all 2>/dev/null
git log --all --oneline | grep -i "2778aaf" || echo "Commit 2778aaf not found in any branch"
# Check if it exists in remote refs
git branch -a | head -20
# Also check the latest state of TerminalMessage.tsx to see if translation was applied
cat apps/tauri/src/components/TerminalMessage.tsx
Length of output: 5098
@wilcorrea, it looks like commit 2778aaf hasn't been pushed to the PR branch yet — it's not visible in the remote history and the current TerminalMessage.tsx still contains the unfixed items:
text-[#3dd68c]/60hardcoded hex color (Line 73)- Hardcoded
"done"string (Line 73) - Hardcoded
"Thinking…"string (Line 100) - Hardcoded
`${contentLines.length} lines...`string (Line 84) - No
useTranslationimport
Could you push the commit to the branch? I'll re-verify once it's available.
🧠 Learnings used
Learnt from: CR
Repo: devitools/arandu PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T05:28:33.736Z
Learning: Applies to apps/tauri/src/**/*.{ts,tsx} : Use `useTranslation()` hook from react-i18next for all user-facing strings
Learnt from: wilcorrea
Repo: devitools/arandu PR: 41
File: apps/tauri/src/components/settings/DiagnosticsSettings.tsx:0-0
Timestamp: 2026-03-02T22:46:53.564Z
Learning: Enforce using semantic SHADCN UI tokens for status indicators in TSX components under apps/tauri/src. Use text-success, text-warning, and text-destructive tokens (foreground variants) instead of hardcoded colors. These tokens are defined in apps/tauri/src/index.css under both :root (light) and .dark (dark mode) and registered in tailwind.config.ts. Apply this pattern broadly to TSX files in apps/tauri/src (not just DiagnosticsSettings.tsx) to ensure theme-aware color usage for status icons and indicators.
Learnt from: wilcorrea
Repo: devitools/arandu PR: 41
File: apps/tauri/src/components/settings/DiagnosticsSettings.tsx:26-27
Timestamp: 2026-03-02T22:40:16.330Z
Learning: The shadcn/ui design system in this project currently only defines `text-destructive` as a semantic color token. Semantic tokens for success (`text-success`) and warning (`text-warning`) states need to be added to `apps/tauri/src/index.css` under both `:root` (light) and `.dark` (dark mode) sections before status icons can use theme-aware colors throughout the app.
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to **/*.{tsx,ts} : Use `useTranslation()` hook for all user-facing text in React components
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to src/lib/tray-sync.ts : For tray menu translations, update both the locale files and the `update_tray_labels` Rust command in `lib/tray-sync.ts`
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to src/lib/i18n.ts : Synchronize i18n across layers using `i18n.on('languageChanged')` for React → localStorage, `storage` event listener for localStorage → React, and `updateTrayLabels()` command for React → Rust
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
apps/tauri/src/components/ActiveSessionView.tsx (1)
131-140:⚠️ Potential issue | 🟠 MajorDon’t let
onDisconnect()failure block reconnect.At Line 135, a rejection prevents Line 138 from running, so reconnect can get stuck in error state.
🔧 Suggested resilient reconnect flow
const handleReconnect = useCallback(async () => { initRef.current = false; setInitError(null); - if (onDisconnect) { - await onDisconnect(); - } + try { + if (onDisconnect) await onDisconnect(); + } catch (e) { + console.warn("[ActiveSessionView] disconnect during reconnect failed:", e); + } if (onConnect) { await onConnect(); } }, [onDisconnect, onConnect]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/ActiveSessionView.tsx` around lines 131 - 140, The reconnect handler handleReconnect currently awaits onDisconnect() which, if it rejects, prevents the subsequent onConnect() call; change handleReconnect so initRef.current = false and setInitError(null) remain, but call onDisconnect() inside a try/catch (or Promise.catch) and swallow or log the error so it doesn't rethrow, then always proceed to await onConnect(); reference handleReconnect, onDisconnect, onConnect, initRef, and setInitError when making the change.apps/tauri/src/hooks/useAcpSession.ts (1)
73-82:⚠️ Potential issue | 🟠 MajorReset the full session state when entry is missing/replaced.
Current logic resets only
messages/isStreamingin refresh paths and does nothing when a workspace has no cache entry, so stale values can leak (currentMode,availableModes,activeAcpSessionId,agentPlanFilePath, and ref state).🔧 Suggested consolidation helper
+ const applyEntryToState = useCallback((entry: SessionEntry) => { + setMessages(entry.messages); + setIsStreaming(entry.isStreaming); + setCurrentMode(entry.currentMode); + setAvailableModes(entry.availableModes); + setActiveAcpSessionId(entry.activeAcpSessionId); + setAgentPlanFilePath(entry.agentPlanFilePath); + activeAcpSessionIdRef.current = entry.activeAcpSessionId; + }, []); + useEffect(() => { const entry = sessionStore.get(workspaceId); - if (entry) { - setMessages(entry.messages); - setIsStreaming(entry.isStreaming); - setCurrentMode(entry.currentMode); - setAvailableModes(entry.availableModes); - setActiveAcpSessionId(entry.activeAcpSessionId); - setAgentPlanFilePath(entry.agentPlanFilePath); - activeAcpSessionIdRef.current = entry.activeAcpSessionId; - } + applyEntryToState(entry ?? EMPTY_SESSION); @@ - sessionStore.set(workspaceId, fresh); - setMessages([]); - setIsStreaming(false); + sessionStore.set(workspaceId, fresh); + applyEntryToState(fresh); @@ - sessionStore.set(workspaceId, fresh); - setMessages([]); - setIsStreaming(false); + sessionStore.set(workspaceId, fresh); + applyEntryToState(fresh);Also applies to: 121-128, 150-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/hooks/useAcpSession.ts` around lines 73 - 82, When sessionStore.get(workspaceId) returns no entry (or when an entry is replaced) the code only updates messages/isStreaming and leaves stale values in currentMode, availableModes, activeAcpSessionId, agentPlanFilePath and activeAcpSessionIdRef; update the logic in the blocks that call sessionStore.get(...) and then set* (e.g., the block using setMessages, setIsStreaming, setCurrentMode, setAvailableModes, setActiveAcpSessionId, setAgentPlanFilePath and activeAcpSessionIdRef.current) to reset all session state to explicit defaults (clear messages, set isStreaming false, set currentMode/availableModes/agentPlanFilePath to null/empty, set activeAcpSessionId to null and activeAcpSessionIdRef.current = null) whenever entry is missing or replaced; apply the same full-reset fix to the other similar sections that update state after fetching from sessionStore.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src/components/ActiveSessionView.tsx`:
- Line 12: Replace the relative import for the ConnectionLogs component with the
path-alias import using `@/` so it resolves from src; update the import statement
that references ConnectionLogs in ActiveSessionView.tsx to use the alias (e.g.,
import ConnectionLogs via "@/components/ConnectionLogs") so it follows the
project's "@/ -> ./src/" convention and aligns with other component imports.
- Around line 121-129: The effect watching isConnected should also reset/init
when the session identity changes: update the useEffect that references
isConnected and initRef to include session.id and session.acp_session_id (or
otherwise detect session identity changes) in its dependency check and set
initRef.current = false when those values change while connected so doInit (the
useCallback depending on session.id and session.acp_session_id) will run for the
new session; specifically adjust the useEffect containing initRef and doInit to
account for session.id and session.acp_session_id so initialization is
retriggered on session identity changes.
In `@apps/tauri/src/hooks/useAcpSession.ts`:
- Around line 102-103: The effect in useAcpSession currently only calls
unsubscribe on cleanup and leaves the idle timer running, which can later set
isStreaming to false for a new workspace; update the effect cleanup to also
clear the idle timer (the timeout/timer variable used to reset isStreaming—e.g.,
idleTimer, idleTimeout, or timerId) before or after calling unsubscribe so no
pending timeout survives a workspaceId change and incorrectly flips isStreaming
for the new session.
- Around line 130-133: Replace the imported invoke usage in the useAcpSession
hook with the global Tauri API: change all calls like
invoke<...>("acp_load_session", {...}) (and the six other command invocations in
the same file) to window.__TAURI__.core.invoke<...>(...) and remove the
now-unused import from `@tauri-apps/api/core`; ensure each call preserves its
original generic type and argument object (e.g., the acp_load_session call and
the other six commands referenced in the review) so only the call site is
switched to window.__TAURI__.core.invoke().
In `@apps/tauri/src/lib/session-cache.ts`:
- Around line 231-241: addUserMessage sets isStreaming: true but never calls
resetStreamingTimer(workspaceId), so the session can stay stuck in streaming if
no backend events arrive; after creating and sessions.set(workspaceId, newEntry)
call resetStreamingTimer(workspaceId) (ensure resetStreamingTimer is in
scope/imported) and then call notifySubscribers(workspaceId, newEntry) so the
streaming timeout is started whenever a user message begins streaming.
- Around line 185-207: The listenerSetup flag is being set before the async
listen promise resolves, preventing retries on failure; update
ensureGlobalListener so it does not mark listenerSetup true until the listen
promise fulfills: call window.__TAURI__.event.listen<AcpSessionUpdate>(...) and
in its .then() set listenerSetup = true (or set a separate pending flag while
registering and flip to true on success), keep the existing
.catch(console.error) so failures leave listenerSetup false to allow retries,
and preserve the existing handler logic that calls processSessionUpdate,
sessions.set, notifySubscribers, resetStreamingTimer/clearStreamingTimer.
---
Duplicate comments:
In `@apps/tauri/src/components/ActiveSessionView.tsx`:
- Around line 131-140: The reconnect handler handleReconnect currently awaits
onDisconnect() which, if it rejects, prevents the subsequent onConnect() call;
change handleReconnect so initRef.current = false and setInitError(null) remain,
but call onDisconnect() inside a try/catch (or Promise.catch) and swallow or log
the error so it doesn't rethrow, then always proceed to await onConnect();
reference handleReconnect, onDisconnect, onConnect, initRef, and setInitError
when making the change.
In `@apps/tauri/src/hooks/useAcpSession.ts`:
- Around line 73-82: When sessionStore.get(workspaceId) returns no entry (or
when an entry is replaced) the code only updates messages/isStreaming and leaves
stale values in currentMode, availableModes, activeAcpSessionId,
agentPlanFilePath and activeAcpSessionIdRef; update the logic in the blocks that
call sessionStore.get(...) and then set* (e.g., the block using setMessages,
setIsStreaming, setCurrentMode, setAvailableModes, setActiveAcpSessionId,
setAgentPlanFilePath and activeAcpSessionIdRef.current) to reset all session
state to explicit defaults (clear messages, set isStreaming false, set
currentMode/availableModes/agentPlanFilePath to null/empty, set
activeAcpSessionId to null and activeAcpSessionIdRef.current = null) whenever
entry is missing or replaced; apply the same full-reset fix to the other similar
sections that update state after fetching from sessionStore.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 324e50ae-ce5c-441b-afa2-5cee504c5197
📒 Files selected for processing (4)
apps/tauri/src/components/ActiveSessionView.tsxapps/tauri/src/components/SessionCard.tsxapps/tauri/src/hooks/useAcpSession.tsapps/tauri/src/lib/session-cache.ts
Comment on lines
130
to
133
| const info = await invoke<AcpSessionInfo>("acp_load_session", { | ||
| workspaceId, | ||
| sessionId: existingAcpSessionId, | ||
| cwd: workspacePath, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/tauri/src/hooks/useAcpSession.ts | head -50Repository: devitools/arandu
Length of output: 1616
🏁 Script executed:
rg "import.*invoke" apps/tauri/src/hooks/useAcpSession.tsRepository: devitools/arandu
Length of output: 106
🏁 Script executed:
cat -n apps/tauri/src/hooks/useAcpSession.ts | sed -n '125,140p'Repository: devitools/arandu
Length of output: 747
🏁 Script executed:
cat -n apps/tauri/src/hooks/useAcpSession.ts | sed -n '150,160p'Repository: devitools/arandu
Length of output: 514
🏁 Script executed:
cat -n apps/tauri/src/hooks/useAcpSession.ts | sed -n '175,200p'Repository: devitools/arandu
Length of output: 906
🏁 Script executed:
cat -n apps/tauri/src/hooks/useAcpSession.ts | sed -n '210,240p'Repository: devitools/arandu
Length of output: 1061
🏁 Script executed:
fd "vite.config" apps/tauri/Repository: devitools/arandu
Length of output: 85
🏁 Script executed:
cat apps/tauri/vite.config.ts 2>/dev/null | head -80Repository: devitools/arandu
Length of output: 2244
🏁 Script executed:
fd "tauri.conf" . --type fRepository: devitools/arandu
Length of output: 98
🏁 Script executed:
cat apps/tauri/vite.config.ts | tail -30Repository: devitools/arandu
Length of output: 759
🏁 Script executed:
cat apps/tauri/src-tauri/tauri.conf.json | head -50Repository: devitools/arandu
Length of output: 1364
Replace imported invoke() with window.__TAURI__.core.invoke() to align with configured Tauri API style.
The hook currently imports and uses invoke from @tauri-apps/api/core, but the project is configured with withGlobalTauri: true in tauri.conf.json, which requires using window.__TAURI__.core.invoke() for all Rust backend function calls.
Update all 7 command invocations at lines 130, 155, 180, 189 (2 calls), 194, 216, and 233 to use window.__TAURI__.core.invoke() instead of the imported invoke function.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/tauri/src/hooks/useAcpSession.ts` around lines 130 - 133, Replace the
imported invoke usage in the useAcpSession hook with the global Tauri API:
change all calls like invoke<...>("acp_load_session", {...}) (and the six other
command invocations in the same file) to window.__TAURI__.core.invoke<...>(...)
and remove the now-unused import from `@tauri-apps/api/core`; ensure each call
preserves its original generic type and argument object (e.g., the
acp_load_session call and the other six commands referenced in the review) so
only the call site is switched to window.__TAURI__.core.invoke().
Contributor
Author
There was a problem hiding this comment.
Acknowledged as follow-up analysis supporting the idle timer cleanup fix above.
There was a problem hiding this comment.
@wilcorrea, understood! The concern about using window.__TAURI__.core.invoke() instead of the imported invoke from @tauri-apps/api/core remains valid and should be addressed in a follow-up. Would you like me to open a GitHub issue to track this so it doesn't get lost?
🧠 Learnings used
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to src/hooks/*.ts : Custom hooks calling Tauri commands should use `window.__TAURI__.core.invoke()` and hooks listening to Tauri events should use `window.__TAURI__.event.listen()` with cleanup in `useEffect` return
Learnt from: CR
Repo: devitools/arandu PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T05:28:33.736Z
Learning: Applies to apps/tauri/src/**/*.{ts,tsx} : Use `window.__TAURI__.core.invoke()` for calling Rust backend functions (`withGlobalTauri: true` in config)
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to **/*.{ts,tsx} : Call Tauri commands from frontend using `window.__TAURI__.core.invoke('command_name', { args })`
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to src/__tests__/setup.ts : Mock `window.__TAURI__` in test setup file with implementations for `core.invoke`, `window.getCurrentWindow`, `dialog.open`, and `event.listen`
Learnt from: CR
Repo: devitools/arandu PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T05:28:33.736Z
Learning: Applies to apps/tauri/src/hooks/**/*.ts : New custom React hooks should follow the naming convention `useXxx()` and leverage Tauri commands and event listeners for backend communication
Learnt from: CR
Repo: devitools/arandu PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:29:04.553Z
Learning: Applies to src/hooks/*.ts : Use ref-based callbacks (`sendPromptRef`, `setModeRef`) to avoid dependency array churn in workflow state machines
Learnt from: CR
Repo: devitools/arandu PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T05:28:33.736Z
Learning: Applies to apps/tauri/src-tauri/src/**/*.rs : Define new Tauri commands as functions, register them in `invoke_handler`, and add permissions to `capabilities/default.json`
Learnt from: CR
Repo: devitools/arandu PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T05:28:33.736Z
Learning: Applies to apps/tauri/src/**/*.{ts,tsx} : Use localStorage for workspace persistence and language preference with `storage` event listeners for cross-window state sync
Comment on lines
+231
to
+241
| export function addUserMessage(workspaceId: string, text: string) { | ||
| const entry = sessions.get(workspaceId); | ||
| if (!entry) return; | ||
| const newEntry = { | ||
| ...entry, | ||
| messages: [...entry.messages, { id: nextMsgId(), role: "user" as const, content: text, timestamp: new Date() }], | ||
| isStreaming: true, | ||
| }; | ||
| sessions.set(workspaceId, newEntry); | ||
| notifySubscribers(workspaceId, newEntry); | ||
| } |
There was a problem hiding this comment.
Start streaming timeout when addUserMessage sets streaming true.
This path sets isStreaming: true but doesn’t call resetStreamingTimer(workspaceId). If backend events never arrive afterward, cache state can remain stuck as streaming.
🔧 Suggested fix
export function addUserMessage(workspaceId: string, text: string) {
@@
sessions.set(workspaceId, newEntry);
notifySubscribers(workspaceId, newEntry);
+ resetStreamingTimer(workspaceId);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function addUserMessage(workspaceId: string, text: string) { | |
| const entry = sessions.get(workspaceId); | |
| if (!entry) return; | |
| const newEntry = { | |
| ...entry, | |
| messages: [...entry.messages, { id: nextMsgId(), role: "user" as const, content: text, timestamp: new Date() }], | |
| isStreaming: true, | |
| }; | |
| sessions.set(workspaceId, newEntry); | |
| notifySubscribers(workspaceId, newEntry); | |
| } | |
| export function addUserMessage(workspaceId: string, text: string) { | |
| const entry = sessions.get(workspaceId); | |
| if (!entry) return; | |
| const newEntry = { | |
| ...entry, | |
| messages: [...entry.messages, { id: nextMsgId(), role: "user" as const, content: text, timestamp: new Date() }], | |
| isStreaming: true, | |
| }; | |
| sessions.set(workspaceId, newEntry); | |
| notifySubscribers(workspaceId, newEntry); | |
| resetStreamingTimer(workspaceId); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/tauri/src/lib/session-cache.ts` around lines 231 - 241, addUserMessage
sets isStreaming: true but never calls resetStreamingTimer(workspaceId), so the
session can stay stuck in streaming if no backend events arrive; after creating
and sessions.set(workspaceId, newEntry) call resetStreamingTimer(workspaceId)
(ensure resetStreamingTimer is in scope/imported) and then call
notifySubscribers(workspaceId, newEntry) so the streaming timeout is started
whenever a user message begins streaming.
Contributor
Author
There was a problem hiding this comment.
Valid. addUserMessage sets isStreaming: true but never calls resetStreamingTimer(workspaceId). If no backend events follow, the cache stays permanently stuck in streaming state. Will add the resetStreamingTimer call.
There was a problem hiding this comment.
Rate Limit Exceeded
@wilcorrea have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 50 seconds before sending another message.
Contributor
Author
There was a problem hiding this comment.
Fixed in 6ff7259. Added resetStreamingTimer(workspaceId) call in addUserMessage so the streaming state is always auto-cleared if no backend events follow.
There was a problem hiding this comment.
Rate Limit Exceeded
@wilcorrea have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 23 seconds before sending another message.
… in ErrorConsole
- Use Button (variant=ghost, size=icon) from shadcn/ui
- Add useTranslation() and use t('common.dismiss') for aria-label
- Add common.dismiss key to pt-BR.json ("Dispensar") and en.json ("Dismiss")
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
In `acp_connect` and `acp_check_health`, `is_alive().await` and `shutdown().await` were called while the connections mutex was held, potentially stalling other ACP commands behind long waits. Refactored both functions to remove the entry under the lock, drop the lock, perform async checks/shutdown outside it, then re-acquire the lock only to re-insert (if still alive) or finalize cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
In `handleReconnect`, if `onDisconnect()` rejected the promise chain would abort before calling `onConnect()`, leaving the UI stuck in an error state. Wrapped `onDisconnect` in try/catch so `onConnect` always executes regardless of disconnect errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/tauri/src-tauri/src/acp/commands.rs (1)
306-313:⚠️ Potential issue | 🟡 MinorAvoid awaiting multiple
shutdown()calls while holdingconnectionslock.The lock is held during all
shutdown().awaitcalls in the loop. While this is likely only called during app shutdown, it's inconsistent with the fixes applied elsewhere and could block if any shutdown hangs.🔧 Suggested fix
pub async fn disconnect_all(state: &AcpState) { state.configs.lock().await.clear(); - let mut connections = state.connections.lock().await; - for (id, conn) in connections.drain() { + let drained: Vec<_> = { + let mut connections = state.connections.lock().await; + connections.drain().collect() + }; + for (id, conn) in drained { eprintln!("[acp] Disconnecting workspace {}", id); conn.shutdown().await; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/acp/commands.rs` around lines 306 - 313, disconnect_all currently holds the state.connections lock across each conn.shutdown().await call; to fix, acquire the lock once, drain or collect the connections into a local Vec (e.g., let conns = state.connections.lock().await.drain().collect::<Vec<_>>()), then drop the lock before iterating over that Vec and calling conn.shutdown().await for each entry; keep the existing configs.clear() behavior but ensure the shutdown awaits happen after releasing the connections lock (referencing disconnect_all, state.connections, connections.lock(), and conn.shutdown()).apps/tauri/src/components/ActiveSessionView.tsx (1)
103-106:⚠️ Potential issue | 🟡 MinorUse global Tauri invoke API instead of
@tauri-apps/api/coreimport.Lines 103-106 and 147 call backend commands through imported
invoke; this file should usewindow.__TAURI__.core.invoke(...)per project convention withwithGlobalTauri: trueconfigured.Suggested change
-import { invoke } from "@tauri-apps/api/core"; @@ - await invoke("session_update_acp_id", { + await window.__TAURI__.core.invoke("session_update_acp_id", { id: session.id, acpSessionId: acpId, }); @@ - invoke("session_update_chat_panel_size", { id: session.id, size: sizes[0] }).catch(console.error); + window.__TAURI__.core + .invoke("session_update_chat_panel_size", { id: session.id, size: sizes[0] }) + .catch(console.error);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/ActiveSessionView.tsx` around lines 103 - 106, The code is calling backend commands using the imported invoke from `@tauri-apps/api/core` (e.g., the session_update_acp_id call that passes { id: session.id, acpSessionId: acpId } and other invoke usages around the file), but the project uses the global Tauri API; replace those imports/uses with the global call window.__TAURI__.core.invoke(...) (ensure the argument shape stays the same) and remove any direct import of invoke from `@tauri-apps/api/core` so the component relies on the configured withGlobalTauri: true global entrypoint.
♻️ Duplicate comments (2)
apps/tauri/src/components/ActiveSessionView.tsx (2)
12-12:⚠️ Potential issue | 🟡 MinorUse
@/alias forConnectionLogsimport.Line 12 regresses to a relative import; please switch to
@/components/ConnectionLogsfor consistency with repo import rules.Suggested change
-import { ConnectionLogs } from "./ConnectionLogs"; +import { ConnectionLogs } from "@/components/ConnectionLogs";As per coding guidelines:
Use path alias@/which maps to ./src/ for imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/ActiveSessionView.tsx` at line 12, The import in ActiveSessionView.tsx uses a relative path for ConnectionLogs; update the import to use the project alias by replacing the relative import with "@/components/ConnectionLogs" so the ConnectionLogs component is imported via the repository's `@/` (./src) alias to remain consistent with other imports and linting rules.
121-129:⚠️ Potential issue | 🟠 MajorRe-run initialization when session identity changes while connected.
At Line 129, the effect depends only on
isConnected. Ifsession.id/session.acp_session_idchanges without disconnecting,initRef.currentcan staytrueand skipdoInitfor the new session.Suggested change
+ useEffect(() => { + initRef.current = false; + setInitError(null); + }, [session.id, session.acp_session_id]); + useEffect(() => { if (!isConnected) { initRef.current = false; return; } if (initRef.current) return; initRef.current = true; - doInit(); - }, [isConnected]); + void doInit(); + }, [isConnected, doInit, session.id, session.acp_session_id]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/ActiveSessionView.tsx` around lines 121 - 129, The effect only depends on isConnected so when a new session identity arrives while connected initRef.current stays true and doInit is skipped; update the useEffect to also depend on the session identity (e.g., include session.id and session.acp_session_id or a single derived sessionKey) and reset initRef.current to false when the identity changes while connected so doInit runs for the new session; specifically modify the useEffect signature that references isConnected/initRef/doInit to include the session identity deps and add logic to set initRef.current = false when the session id/acp_session_id changes and isConnected is true.
🧹 Nitpick comments (1)
apps/tauri/src/components/ErrorConsole.tsx (1)
25-33: Minor indentation inconsistency inside the Button element.The Button's children and closing tag are indented with 8 spaces instead of 6, creating misalignment with sibling elements.
✨ Suggested fix for consistent indentation
<Button - variant="ghost" - size="icon" - onClick={onClear} - className="h-6 w-6 shrink-0 text-muted-foreground/40 hover:text-foreground" - aria-label={t("common.dismiss")} - > - <X className="h-3.5 w-3.5" /> - </Button> + variant="ghost" + size="icon" + onClick={onClear} + className="h-6 w-6 shrink-0 text-muted-foreground/40 hover:text-foreground" + aria-label={t("common.dismiss")} + > + <X className="h-3.5 w-3.5" /> + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/ErrorConsole.tsx` around lines 25 - 33, The Button JSX in ErrorConsole.tsx has its child <X .../> and closing </Button> indented 8 spaces instead of matching the 6-space indentation used for the Button attributes; adjust the indentation of the child element and the closing </Button> so they align with the Button opening tag/props (i.e., same indentation level as the other Button attributes) to restore consistent formatting in the Button element.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src-tauri/src/acp/commands.rs`:
- Around line 96-100: The code currently holds the state.connections mutex while
awaiting conn.shutdown().await; change the flow so you remove the connection
from the lock (using state.connections.lock().await and
connections.remove(&workspace_id)), then drop the mutex guard before awaiting
shutdown — e.g., store the returned conn in a local variable, let the
connections guard go out of scope (or call drop(connections)) and only then call
conn.emit_log("info", "disconnect", "Disconnected by user") and await
conn.shutdown().await; ensure you reference state.connections,
connections.remove, conn.emit_log, and conn.shutdown().await when making the
change.
---
Outside diff comments:
In `@apps/tauri/src-tauri/src/acp/commands.rs`:
- Around line 306-313: disconnect_all currently holds the state.connections lock
across each conn.shutdown().await call; to fix, acquire the lock once, drain or
collect the connections into a local Vec (e.g., let conns =
state.connections.lock().await.drain().collect::<Vec<_>>()), then drop the lock
before iterating over that Vec and calling conn.shutdown().await for each entry;
keep the existing configs.clear() behavior but ensure the shutdown awaits happen
after releasing the connections lock (referencing disconnect_all,
state.connections, connections.lock(), and conn.shutdown()).
In `@apps/tauri/src/components/ActiveSessionView.tsx`:
- Around line 103-106: The code is calling backend commands using the imported
invoke from `@tauri-apps/api/core` (e.g., the session_update_acp_id call that
passes { id: session.id, acpSessionId: acpId } and other invoke usages around
the file), but the project uses the global Tauri API; replace those imports/uses
with the global call window.__TAURI__.core.invoke(...) (ensure the argument
shape stays the same) and remove any direct import of invoke from
`@tauri-apps/api/core` so the component relies on the configured withGlobalTauri:
true global entrypoint.
---
Duplicate comments:
In `@apps/tauri/src/components/ActiveSessionView.tsx`:
- Line 12: The import in ActiveSessionView.tsx uses a relative path for
ConnectionLogs; update the import to use the project alias by replacing the
relative import with "@/components/ConnectionLogs" so the ConnectionLogs
component is imported via the repository's `@/` (./src) alias to remain
consistent with other imports and linting rules.
- Around line 121-129: The effect only depends on isConnected so when a new
session identity arrives while connected initRef.current stays true and doInit
is skipped; update the useEffect to also depend on the session identity (e.g.,
include session.id and session.acp_session_id or a single derived sessionKey)
and reset initRef.current to false when the identity changes while connected so
doInit runs for the new session; specifically modify the useEffect signature
that references isConnected/initRef/doInit to include the session identity deps
and add logic to set initRef.current = false when the session id/acp_session_id
changes and isConnected is true.
---
Nitpick comments:
In `@apps/tauri/src/components/ErrorConsole.tsx`:
- Around line 25-33: The Button JSX in ErrorConsole.tsx has its child <X .../>
and closing </Button> indented 8 spaces instead of matching the 6-space
indentation used for the Button attributes; adjust the indentation of the child
element and the closing </Button> so they align with the Button opening
tag/props (i.e., same indentation level as the other Button attributes) to
restore consistent formatting in the Button element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bd83e20a-c5c8-4a43-a119-31601e2bef53
📒 Files selected for processing (5)
apps/tauri/src-tauri/src/acp/commands.rsapps/tauri/src/components/ActiveSessionView.tsxapps/tauri/src/components/ErrorConsole.tsxapps/tauri/src/locales/en.jsonapps/tauri/src/locales/pt-BR.json
Flip listenerSetup flag only after the Tauri event listener resolves, allowing retries if setup fails. Also call resetStreamingTimer() in addUserMessage so the isStreaming flag is always auto-cleared if no backend events follow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
….css Add --terminal-code CSS variable and replace all #3dd68c and #56d4dd hex values with hsl(var(--success)) and hsl(var(--terminal-code)) respectively, ensuring colors respect the app theme system. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace hex color class with text-success/60 semantic token and localize the hardcoded "done" label via useTranslation(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rkspace change Reset all local state fields (currentMode, availableModes, activeAcpSessionId, agentPlanFilePath) when loading an existing session to prevent stale UI state. Also clear idleTimerRef in the workspaceId effect cleanup so timers don't fire on the wrong workspace. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…on change Replace relative import with @/ path alias. Add a dedicated effect to reset initRef when session identity changes so doInit() reruns when a different session is mounted on the same connected workspace. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…onnect Drop the connections lock before calling conn.shutdown().await, consistent with the same fix applied to acp_connect and acp_check_health. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wilcorrea
added a commit
that referenced
this pull request
Mar 6, 2026
* feat(acp): Add heartbeat, dead connection cleanup and connection logging
- Add Rust-side heartbeat task with 15s ping interval and 3-strike disconnect
- Clean up dead connections in `acp_connect` and `acp_check_health`
- Emit structured connection log events via `acp:log`
- Add `HeartbeatEvent`, `ConnectionLogEntry` and `ConnectionConfig` types
- Add `chat_panel_size` field to session persistence
* feat(acp): Persist session state across minimize/restore and reload history
- Add in-memory session cache with global event listener independent of React lifecycle
- Keep DirectoryWorkspace mounted via CSS hidden when minimized
- Add `persistedWorkspaceId` to AppContext for mount/visibility separation
- Always call `acp_load_session` to replay full conversation from ACP backend
- Restore cached messages on "already loaded" fallback
- Add periodic 30s heartbeat in `useAcpConnection` for proactive disconnect detection
- Remove auto-disconnect on unmount; move disconnect to explicit close/forget
- Add connection logs panel with `useAcpLogs` hook and `ConnectionLogs` component
- Add i18n strings for connection logs and ACP status
* test(acp): Add tests for connection hook and logs hook
- Test connect/disconnect lifecycle, status events, workspace isolation
- Test cached state restoration and health check on mount
- Verify disconnect is not called on unmount
- Test log capture, filtering and clear behavior
* style(ui): Replace X icon with Trash2 on session delete button
* fix(acp): Reset session init on disconnect and auto-reload on "not found"
- Reset `initRef` when `isConnected` goes false so reconnect triggers
`doInit` and reloads the session in the new copilot process
- Auto-recover in `sendPrompt`: on "Session not found" (-32602), try
`acp_load_session` then retry the prompt before surfacing the error
* fix(acp): Add 60s streaming timeout to auto-clear stale Stop button
If no streaming event arrives within 60s, `isStreaming` is reset to
false automatically. Prevents the Stop button from staying red
indefinitely when `end_turn` is never received.
* fix: replace native button with shadcn Button and localize aria-label in ErrorConsole
- Use Button (variant=ghost, size=icon) from shadcn/ui
- Add useTranslation() and use t('common.dismiss') for aria-label
- Add common.dismiss key to pt-BR.json ("Dispensar") and en.json ("Dismiss")
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(acp): Avoid awaiting async ops while holding connections mutex
In `acp_connect` and `acp_check_health`, `is_alive().await` and
`shutdown().await` were called while the connections mutex was held,
potentially stalling other ACP commands behind long waits.
Refactored both functions to remove the entry under the lock, drop the
lock, perform async checks/shutdown outside it, then re-acquire the
lock only to re-insert (if still alive) or finalize cleanup.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(acp): Ensure `onConnect` runs even if `onDisconnect` rejects
In `handleReconnect`, if `onDisconnect()` rejected the promise chain
would abort before calling `onConnect()`, leaving the UI stuck in an
error state. Wrapped `onDisconnect` in try/catch so `onConnect`
always executes regardless of disconnect errors.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(acp): Fix listener guard and add streaming timer in session-cache
Flip listenerSetup flag only after the Tauri event listener resolves,
allowing retries if setup fails. Also call resetStreamingTimer() in
addUserMessage so the isStreaming flag is always auto-cleared if no
backend events follow.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(ui): Replace hardcoded hex colors with HSL CSS variables in index.css
Add --terminal-code CSS variable and replace all #3dd68c and #56d4dd
hex values with hsl(var(--success)) and hsl(var(--terminal-code))
respectively, ensuring colors respect the app theme system.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(ui): Use semantic token and localize done label in TerminalMessage
Replace hex color class with text-success/60 semantic token and
localize the hardcoded "done" label via useTranslation().
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(acp): Full state reset on session load and clear idle timer on workspace change
Reset all local state fields (currentMode, availableModes,
activeAcpSessionId, agentPlanFilePath) when loading an existing session
to prevent stale UI state. Also clear idleTimerRef in the workspaceId
effect cleanup so timers don't fire on the wrong workspace.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(ui): Use @/ alias for ConnectionLogs and re-trigger init on session change
Replace relative import with @/ path alias. Add a dedicated effect to
reset initRef when session identity changes so doInit() reruns when a
different session is mounted on the same connected workspace.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(acp): Avoid holding connections mutex during shutdown in acp_disconnect
Drop the connections lock before calling conn.shutdown().await,
consistent with the same fix applied to acp_connect and acp_check_health.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Mar 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
session-cache.ts) with pub/sub, surviving minimize/restore and workspace switches without losing chat historysession/promptJSON-RPC requests now use a 10-minute timeout instead of 30 seconds, preventing false timeout errors during long agent operationsChanges
Backend (Rust)
connection.rs: Addsend_request_with_timeoutfor configurable JSON-RPC timeouts; heartbeat with 3-strike disconnect; connection logging eventscommands.rs:acp_send_promptuses 600s timeout; new session commandssessions.rs: Newsession_update_chat_panel_sizecommand for layout persistencetypes.rs: New event types for connection status, heartbeat, and logsFrontend (TypeScript/React)
session-cache.ts: New centralized session state store with pub/sub patternuseAcpSession.ts: Refactored to use session-cache store instead of local stateuseAcpConnection.ts: Heartbeat monitoring and status eventsuseAcpLogs.ts: New hook for connection log collectionConnectionLogs.tsx: New diagnostics panel componentTerminalMessage.tsx: Collapsible long user messages with<details>toggleActiveSessionView.tsx: Session name prepended to initial prompt; layout persistenceSessionCard.tsx: Trash icon replaces X for delete actionMarkdownViewer.tsx: Review panel button repositionedTests
useAcpConnection.test.ts: 10 tests for connection lifecycleuseAcpLogs.test.ts: 7 tests for log collection and filteringTest plan
npm test— 86 tests passing (15 test files)cargo check— Rust compilation cleanSummary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements
Tests