🤖 fix: address review feedback for subagents implementation

- Fix processQueue parallel limit enforcement - re-check running count
  each iteration instead of once before the loop
- Add args field to tool-call-end event schema so TaskService can
  receive agent_report arguments for report delivery
- Fix config→metadata passing parentWorkspaceId/agentType/taskState
  fields so UI nesting works correctly
- Deduplicate sortWithNesting function - export from workspaceFiltering.ts
  and import in useSortedWorkspacesByProject.ts (DRY violation fix)
- Add error handling to TasksSection.tsx - properly catch API errors
  instead of leaving UI stuck in 'Loading...' state
- Extract magic numbers for indentation to named constants
  (BASE_PADDING_PX, NESTING_INDENT_PX)
- Update tests and mock types to include new args field in tool-call-end
  events

Change-Id: I7fcc69e8616f758a68b5864ebe6afb318dafbb85
Signed-off-by: Thomas Kosiewski <tk@coder.com>

Author: ThomasK33

src/browser/components/Settings/sections/TasksSection.tsx

@@ -18,19 +18,28 @@ export function TasksSection() {
   const { api } = useAPI();
   const [settings, setSettings] = useState<TaskSettings>(DEFAULT_TASK_SETTINGS);
   const [loaded, setLoaded] = useState(false);
+  const [loadError, setLoadError] = useState<string | null>(null);
 
   // Load settings on mount
   useEffect(() => {
     if (api) {
-      void api.general.getTaskSettings().then((taskSettings) => {
-        setSettings({
-          maxParallelAgentTasks:
-            taskSettings.maxParallelAgentTasks ?? DEFAULT_TASK_SETTINGS.maxParallelAgentTasks,
-          maxTaskNestingDepth:
-            taskSettings.maxTaskNestingDepth ?? DEFAULT_TASK_SETTINGS.maxTaskNestingDepth,
+      api.general
+        .getTaskSettings()
+        .then((taskSettings) => {
+          setSettings({
+            maxParallelAgentTasks:
+              taskSettings.maxParallelAgentTasks ?? DEFAULT_TASK_SETTINGS.maxParallelAgentTasks,
+            maxTaskNestingDepth:
+              taskSettings.maxTaskNestingDepth ?? DEFAULT_TASK_SETTINGS.maxTaskNestingDepth,
+          });
+          setLoaded(true);
+        })
+        .catch((error) => {
+          console.error("Failed to load task settings:", error);
+          setLoadError("Failed to load settings");
+          // Still mark as loaded so user can interact with defaults
+          setLoaded(true);
         });
-        setLoaded(true);
-      });
     }
   }, [api]);
 
@@ -82,6 +91,9 @@ export function TasksSection() {
         <p className="text-muted mb-4 text-xs">
           Configure limits for agent subworkspaces spawned via the task tool.
         </p>
+        {loadError && (
+          <p className="mb-4 text-xs text-red-500">{loadError}. Using default values.</p>
+        )}
 
         <div className="space-y-4">
           <div className="flex items-center justify-between">

src/browser/components/WorkspaceListItem.tsx

@@ -105,8 +105,12 @@ const WorkspaceListItemInner: React.FC<WorkspaceListItemProps> = ({
   const { canInterrupt, awaitingUserQuestion } = useWorkspaceSidebarState(workspaceId);
   const isWorking = canInterrupt && !awaitingUserQuestion;
 
-  // Calculate left padding based on nesting depth (base 9px + 16px per level)
-  const leftPadding = 9 + nestingDepth * 16;
+  // Sidebar indentation constants
+  const BASE_PADDING_PX = 9;
+  const NESTING_INDENT_PX = 16;
+
+  // Calculate left padding based on nesting depth
+  const leftPadding = BASE_PADDING_PX + nestingDepth * NESTING_INDENT_PX;
   const isAgentTask = Boolean(metadata.parentWorkspaceId);
 
   return (

src/browser/hooks/useSortedWorkspacesByProject.ts

@@ -3,65 +3,10 @@ import { useWorkspaceContext } from "@/browser/contexts/WorkspaceContext";
 import type { FrontendWorkspaceMetadata } from "@/common/types/workspace";
 import { useWorkspaceRecency } from "@/browser/stores/WorkspaceStore";
 import { useStableReference, compareMaps } from "@/browser/hooks/useStableReference";
+import { sortWithNesting, type WorkspaceWithNesting } from "@/browser/utils/ui/workspaceFiltering";
 
-/** Workspace metadata extended with computed nesting depth */
-export interface WorkspaceWithNesting extends FrontendWorkspaceMetadata {
-  /** Nesting depth (0 = top-level, 1 = direct child, etc.) */
-  nestingDepth: number;
-}
-
-/**
- * Sort workspaces so children appear immediately after their parent.
- * Maintains recency order within each level.
- */
-function sortWithNesting(
-  metadataList: FrontendWorkspaceMetadata[],
-  workspaceRecency: Record<string, number>
-): WorkspaceWithNesting[] {
-  // Build parent→children map
-  const childrenByParent = new Map<string, FrontendWorkspaceMetadata[]>();
-  const topLevel: FrontendWorkspaceMetadata[] = [];
-
-  for (const ws of metadataList) {
-    const parentId = ws.parentWorkspaceId;
-    if (parentId) {
-      const siblings = childrenByParent.get(parentId) ?? [];
-      siblings.push(ws);
-      childrenByParent.set(parentId, siblings);
-    } else {
-      topLevel.push(ws);
-    }
-  }
-
-  // Sort by recency (most recent first)
-  const sortByRecency = (a: FrontendWorkspaceMetadata, b: FrontendWorkspaceMetadata) => {
-    const aTs = workspaceRecency[a.id] ?? 0;
-    const bTs = workspaceRecency[b.id] ?? 0;
-    return bTs - aTs;
-  };
-
-  topLevel.sort(sortByRecency);
-  for (const children of childrenByParent.values()) {
-    children.sort(sortByRecency);
-  }
-
-  // Flatten: parent, then children recursively
-  const result: WorkspaceWithNesting[] = [];
-
-  const visit = (ws: FrontendWorkspaceMetadata, depth: number) => {
-    result.push({ ...ws, nestingDepth: depth });
-    const children = childrenByParent.get(ws.id) ?? [];
-    for (const child of children) {
-      visit(child, depth + 1);
-    }
-  };
-
-  for (const ws of topLevel) {
-    visit(ws, 0);
-  }
-
-  return result;
-}
+// Re-export for backward compatibility
+export type { WorkspaceWithNesting };
 
 export function useSortedWorkspacesByProject() {
   const { projects } = useProjectContext();

src/browser/utils/messages/StreamingMessageAggregator.status.test.ts

@@ -131,6 +131,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
     // Complete the tool call
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId,
@@ -173,6 +174,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId: "tool1",
@@ -197,6 +199,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId: "tool2",
@@ -237,6 +240,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId: "tool1",
@@ -290,6 +294,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
     // Complete with failure
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId: "tool1",
@@ -328,6 +333,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId: "msg1",
       toolCallId: "tool1",
@@ -393,6 +399,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
     // Complete with validation failure
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId: "tool1",
@@ -453,6 +460,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
     // Complete successfully
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId: "tool1",
@@ -699,6 +707,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId,
@@ -744,6 +753,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
     // Complete the tool call
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId,
@@ -788,6 +798,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId: "tool1",
@@ -812,6 +823,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId: "tool2",
@@ -840,6 +852,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId,
       toolCallId: "tool3",
@@ -883,6 +896,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId: messageId1,
       toolCallId: "tool1",
@@ -930,6 +944,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
 
     aggregator.handleToolCallEnd({
       type: "tool-call-end",
+      args: {},
       workspaceId: "workspace1",
       messageId: messageId2,
       toolCallId: "tool2",

src/browser/utils/messages/StreamingMessageAggregator.test.ts

@@ -171,6 +171,7 @@ describe("StreamingMessageAggregator", () => {
 
       aggregator.handleToolCallEnd({
         type: "tool-call-end",
+        args: {},
         workspaceId: "test-workspace",
         messageId: "msg1",
         toolCallId: "tool1",
@@ -228,6 +229,7 @@ describe("StreamingMessageAggregator", () => {
 
       aggregator.handleToolCallEnd({
         type: "tool-call-end",
+        args: {},
         workspaceId: "test-workspace",
         messageId: "msg1",
         toolCallId: "tool1",
@@ -363,6 +365,7 @@ describe("StreamingMessageAggregator", () => {
 
       aggregator.handleToolCallEnd({
         type: "tool-call-end",
+        args: {},
         workspaceId: "test-workspace",
         messageId: "msg1",
         toolCallId: "tool1",

src/browser/utils/ui/workspaceFiltering.ts

@@ -20,7 +20,7 @@ export interface WorkspaceWithNesting extends FrontendWorkspaceMetadata {
  * Sort workspaces so children appear immediately after their parent.
  * Maintains recency order within each level.
  */
-function sortWithNesting(
+export function sortWithNesting(
   metadataList: FrontendWorkspaceMetadata[],
   workspaceRecency: Record<string, number>
 ): WorkspaceWithNesting[] {

src/common/orpc/schemas/stream.ts

@@ -174,6 +174,7 @@ export const ToolCallEndEventSchema = z.object({
   messageId: z.string(),
   toolCallId: z.string(),
   toolName: z.string(),
+  args: z.unknown().meta({ description: "Tool input arguments (for listeners that need args)" }),
   result: z.unknown(),
   timestamp: z.number().meta({ description: "When tool call completed (Date.now())" }),
 });

src/node/config.ts

@@ -345,6 +345,10 @@ export class Config {
               // GUARANTEE: All workspaces must have runtimeConfig (apply default if missing)
               runtimeConfig: workspace.runtimeConfig ?? DEFAULT_RUNTIME_CONFIG,
               aiSettings: workspace.aiSettings,
+              // Agent task workspace fields (optional - only set for subagent workspaces)
+              parentWorkspaceId: workspace.parentWorkspaceId,
+              agentType: workspace.agentType,
+              taskState: workspace.taskState,
             };
 
             // Migrate missing createdAt to config for next load

src/node/services/mock/mockScenarioPlayer.ts

@@ -316,6 +316,7 @@ export class MockScenarioPlayer {
           messageId,
           toolCallId: event.toolCallId,
           toolName: event.toolName,
+          args: event.args,
           result: event.result,
           timestamp: Date.now(),
         };

src/node/services/mock/scenarioTypes.ts

@@ -61,6 +61,7 @@ export interface MockToolEndEvent extends MockAssistantEventBase {
   kind: "tool-end";
   toolCallId: string;
   toolName: string;
+  args?: unknown;
   result: unknown;
 }