apps/web/src/components/Sidebar.tsx (1)

2020-2026: Consider a short-circuit "hasAny" variant for better render performance.

derivePendingApprovals and derivePendingUserInputs are called inside a render loop for every visible thread. Both functions copy, sort, iterate, and return full arrays—yet only .length > 0 is used here.

For a sidebar that re-renders on many interactions, this could become a hot path. A lightweight hasPendingApprovals(activities): boolean helper that returns early on the first match would avoid building the full result array when only presence is needed.

This is not a functional issue, but worth considering if profiling reveals render lag with many threads.

💡 Example early-exit helper (in session-logic.ts)

export function hasPendingApprovals(
  activities: ReadonlyArray<OrchestrationThreadActivity>,
): boolean {
  const resolved = new Set<string>();
  for (const activity of activities) {
    const payload =
      activity.payload && typeof activity.payload === "object"
        ? (activity.payload as Record<string, unknown>)
        : null;
    const requestId = payload && typeof payload.requestId === "string" ? payload.requestId : null;
    if (!requestId) continue;

    if (activity.kind === "approval.resolved") {
      resolved.add(requestId);
    } else if (activity.kind === "approval.requested" && !resolved.has(requestId)) {
      return true; // early exit
    }
  }
  return false;
}

Then in Sidebar.tsx:

-hasPendingApprovals:
-  derivePendingApprovals(thread.activities).length > 0,
+hasPendingApprovals: hasPendingApprovals(thread.activities),

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/Sidebar.tsx` around lines 2020 - 2026, The render
currently calls derivePendingApprovals(...) and derivePendingUserInputs(...) and
only uses .length > 0, which forces full array work for every visible thread;
add lightweight short-circuit helpers (e.g., hasPendingApprovals(activities:
ReadonlyArray<OrchestrationThreadActivity>): boolean and
hasPendingUserInputs(...): boolean) that iterate and return true on first match,
place them alongside existing session-logic helpers, and replace the calls in
resolveThreadStatusPill invocation (the call site using derivePendingApprovals
and derivePendingUserInputs in the Sidebar component) to use the new boolean
helpers so the render no longer allocates full result arrays.

apps/web/src/components/chat/ComposerPlanFollowUpBanner.tsx (1)

16-18: Remove the commented-out JSX before merging.

This dead copy makes the component harder to scan and can drift from the shipped UX. If it's intentionally deferred, track it outside the render tree instead.

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ComposerPlanFollowUpBanner.tsx` around lines 16
- 18, In ComposerPlanFollowUpBanner.tsx remove the commented-out JSX block (the
commented div containing "Review the plan") from the component render so no
dead/commented UI remains; if the markup must be preserved for later, move it to
a separate note or comment outside the render tree (e.g., developer TODO)
instead of leaving JSX commented inline.

apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx (1)

28-30: Minor: Redundant null check.

Line 30's check for !activePrompt is technically redundant since line 28 already ensures pendingUserInputs.length > 0. However, this is defensive and helps TypeScript narrow the type, so it's acceptable.

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx` around lines
28 - 30, The second null-check is redundant: after verifying
pendingUserInputs.length > 0, remove the extra if (!activePrompt) return null; —
keep the first guard (if (pendingUserInputs.length === 0) return null;), assign
const activePrompt = pendingUserInputs[0]; and proceed; if you prefer defensive
typing instead, replace the redundant check with a short comment explaining it's
for TypeScript narrowing and leave activePrompt as-is (symbols:
pendingUserInputs, activePrompt, ComposerPendingUserInputPanel).

apps/web/src/components/chat/ProviderHealthBanner.tsx (1)

15-29: Use the shared provider display label here.

status.provider will render internal ids like claudeCode and geminiCli in the title/default message. Pulling the same label source used by the picker keeps these banners consistent with the rest of the provider UI.

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ProviderHealthBanner.tsx` around lines 15 - 29,
The banner currently uses raw internal ids (status.provider) in AlertTitle and
defaultMessage; replace those usages with the shared display label function
(e.g. getProviderDisplayLabel/statusProviderLabel) used by the provider picker
so the UI shows the human-friendly name. Specifically, import and call the
shared label helper (getProviderDisplayLabel) with status.provider when building
defaultMessage and when rendering AlertTitle and AlertDescription, keeping the
same error/limited wording and fallback to status.message as before. Ensure you
only change the displayed label and not the underlying status logic in
ProviderHealthBanner or defaultMessage construction.

apps/web/src/components/chat/ComposerCommandMenu.tsx (1)

65-73: Consider handling additional triggerKind values in empty state message.

The empty state message defaults to "No matching command." for any triggerKind that isn't "path". If the menu supports model selection (as suggested by the model type in ComposerCommandItem), a "model" trigger kind might benefit from a more specific message like "No matching models."

💡 Possible enhancement

           {props.isLoading
             ? "Searching workspace files..."
             : props.triggerKind === "path"
               ? "No matching files or folders."
-              : "No matching command."}
+              : props.triggerKind === "model"
+                ? "No matching models."
+                : "No matching command."}

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ComposerCommandMenu.tsx` around lines 65 - 73,
The empty-state in ComposerCommandMenu currently falls back to "No matching
command." for any triggerKind other than "path"; update the conditional logic
that renders the message (where props.items.length === 0 is checked) to
explicitly handle a "model" triggerKind (used by ComposerCommandItem's model
type) and show "No matching models." when props.triggerKind === "model", keep
the existing "No matching files or folders." for "path" and preserve the default
fallback for other kinds; ensure props.isLoading still takes precedence to show
"Searching workspace files..." when loading.

apps/web/src/components/chat/ChangedFilesTree.tsx (1)

31-36: Initial state inconsistent with allDirectoriesExpanded prop.

The useState initializer hardcodes true (line 32), while the useEffect syncs with allDirectoryExpansionState which respects the allDirectoriesExpanded prop. If the prop is initially false, users will see a brief flash where directories appear expanded then collapse.

Consider initializing with the prop value directly:

♻️ Proposed fix

-  const [expandedDirectories, setExpandedDirectories] = useState<Record<string, boolean>>(() =>
-    buildDirectoryExpansionState(directoryPathsKey ? directoryPathsKey.split("\u0000") : [], true),
-  );
+  const [expandedDirectories, setExpandedDirectories] = useState<Record<string, boolean>>(() =>
+    buildDirectoryExpansionState(
+      directoryPathsKey ? directoryPathsKey.split("\u0000") : [],
+      allDirectoriesExpanded,
+    ),
+  );

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ChangedFilesTree.tsx` around lines 31 - 36, The
state initializer for expandedDirectories uses buildDirectoryExpansionState(...,
true) which hardcodes expansion and causes a flash when allDirectoriesExpanded
is false; update the useState initializer to derive the initial value from
allDirectoryExpansionState (or use allDirectoriesExpanded to build the initial
expansion state) so the initial expandedDirectories matches the prop-driven
allDirectoryExpansionState, keeping the existing useEffect that calls
setExpandedDirectories(allDirectoryExpansionState); ensure you reference
buildDirectoryExpansionState(directoryPathsKey ?
directoryPathsKey.split("\u0000") : [], ...) when constructing the initial
value.

apps/web/src/components/chat/ProposedPlanCard.tsx (1)

114-121: Use .finally() for cleanup instead of dual-callback .then().

After .catch() handles the rejection, it returns a resolved promise, so the rejection handler (lines 118-120) is unreachable. The .finally() method is the idiomatic pattern for cleanup that must run in both success and failure cases.

♻️ Suggested refactor

       .catch((error) => {
         toastManager.add({
           type: "error",
           title: "Could not save plan",
           description: error instanceof Error ? error.message : "An error occurred while saving.",
         });
       })
-      .then(
-        () => {
-          setIsSavingToWorkspace(false);
-        },
-        () => {
-          setIsSavingToWorkspace(false);
-        },
-      );
+      .finally(() => {
+        setIsSavingToWorkspace(false);
+      });

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ProposedPlanCard.tsx` around lines 114 - 121,
The promise chain in ProposedPlanCard uses a dual-callback .then to reset
setIsSavingToWorkspace which makes the rejection handler unreachable after
.catch; replace the dual-callback .then(...) with a single .then(...) for
success if needed and append .finally(() => setIsSavingToWorkspace(false)) (or
move the cleanup into .finally) so setIsSavingToWorkspace(false) always runs
regardless of success or failure, referencing the existing
setIsSavingToWorkspace call in the component.

apps/web/src/components/chat/ChangedFilesTree.tsx (1)

31-36: Simplify state initialization by reusing the memoized value.

The useState initializer duplicates the computation already performed in allDirectoryExpansionState. Since hooks execute in order, you can pass the memoized value directly.

♻️ Proposed simplification

-  const [expandedDirectories, setExpandedDirectories] = useState<Record<string, boolean>>(() =>
-    buildDirectoryExpansionState(
-      directoryPathsKey ? directoryPathsKey.split("\u0000") : [],
-      allDirectoriesExpanded,
-    ),
-  );
+  const [expandedDirectories, setExpandedDirectories] =
+    useState<Record<string, boolean>>(allDirectoryExpansionState);

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ChangedFilesTree.tsx` around lines 31 - 36, The
state initializer for expandedDirectories recomputes
buildDirectoryExpansionState even though allDirectoryExpansionState already
memoizes that result; replace the initializer usage so useState is called with
allDirectoryExpansionState directly (i.e., initialize expandedDirectories via
the memoized allDirectoryExpansionState) and keep setExpandedDirectories as-is;
update references to directoryPathsKey only where allDirectoryExpansionState is
computed so no duplicate splitting/processing occurs.

apps/web/src/components/chat/ProposedPlanCard.tsx (1)

57-75: ⚠️ Potential issue | 🟡 Minor

Keep the save action gated everywhere, not just in openSaveDialog().

The opener now shows a toast when native support is missing, but the menu item at Line 134 is still enabled whenever workspaceRoot exists, and handleSaveToWorkspace() at Line 81 still silently returns if that prerequisite disappears after the dialog opens. Reuse one availability check/helper for the trigger and the submit path so the UI and handler stay in sync.

Also applies to: 78-83, 134-136

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ProposedPlanCard.tsx` around lines 57 - 75, The
save action is inconsistently gated: openSaveDialog shows a toast when native
support is missing but the menu item and handleSaveToWorkspace still
allow/ignore the action; add a single helper (e.g., canSaveToWorkspace or
isSaveAvailable) that returns Boolean(workspaceRoot && readNativeApi()), use
that helper to disable the menu item/trigger (so the UI reflects availability),
and call the same helper at the start of handleSaveToWorkspace and
openSaveDialog to early-return with the same toast error message when
unavailable; update references to openSaveDialog and handleSaveToWorkspace to
use this helper so the UI and handler stay in sync.

apps/web/src/components/chat/ChangedFilesTree.tsx (1)

71-75: Minor: Inconsistent stat display condition between directories and files.

Directory nodes use hasNonZeroStat(node.stat) (line 71) while file nodes use node.stat (line 104). This means files will show "+0 -0" when stats exist but are zero, while directories won't.

If this is intentional (e.g., to indicate file renames/mode changes), consider adding a brief comment. Otherwise, consider aligning the conditions:

♻️ Optional: Align stat display logic

-        {node.stat && (
+        {hasNonZeroStat(node.stat) && (
           <span className="ml-auto shrink-0 font-mono text-[10px] tabular-nums">
             <DiffStatLabel additions={node.stat.additions} deletions={node.stat.deletions} />
           </span>
         )}

Also applies to: 104-108

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ChangedFilesTree.tsx` around lines 71 - 75, The
directory branch currently guards the stat display with
hasNonZeroStat(node.stat) while the file branch uses a truthy node.stat check,
causing files with zeroed stats to render "+0 -0"; update the file node render
to use the same predicate (hasNonZeroStat(node.stat)) or, if the differing
behavior was intentional, add an inline comment near the file node render
explaining why node.stat alone is used; locate the file-specific block that
renders <DiffStatLabel additions={node.stat.additions}
deletions={node.stat.deletions} /> and change its condition to mirror the
directory branch (or add the explanatory comment) so the logic is consistent and
easy to understand.

apps/web/src/components/chat/ProposedPlanCard.tsx (1)

80-97: Client-side validation would improve UX, but server-side validation already prevents path escapes.

The server's resolveWorkspaceWritePath() properly canonicalizes the path and rejects absolute paths and parent-directory traversal (../) before writing. The test confirms ../escape.md is correctly rejected. However, consider adding client-side validation in ProposedPlanCard (around line 80) to provide immediate feedback before the request is sent, rather than only trimming the input.

🤖 Prompt for AI Agents

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

In `@apps/web/src/components/chat/ProposedPlanCard.tsx` around lines 80 - 97, Add
client-side path validation in ProposedPlanCard before calling
api.projects.writeFile: after computing relativePath from savePath.trim(),
validate that relativePath is non-empty and does not start with "/" or contain
".." segments (e.g., "../") and if invalid call toastManager.add with a warning
and return (and ensure setIsSavingToWorkspace is not set). Keep existing
empty-check behavior, then proceed to call api.projects.writeFile (the same call
as in the diff) only when the client-side checks pass.