Merge branch 'main' into mk-windows-sandboxing

packages/core/src/agents/subagent-tool-wrapper.ts

@@ -10,7 +10,7 @@ import {
   type ToolInvocation,
   type ToolResult,
 } from '../tools/tools.js';
-import type { Config } from '../config/config.js';
+
 import { type AgentLoopContext } from '../config/agent-loop-context.js';
 import type { AgentDefinition, AgentInputs } from './types.js';
 import { LocalSubagentInvocation } from './local-invocation.js';
@@ -54,10 +54,6 @@ export class SubagentToolWrapper extends BaseDeclarativeTool<
     );
   }
 
-  private get config(): Config {
-    return this.context.config;
-  }
-
   /**
    * Creates an invocation instance for executing the subagent.
    *
@@ -89,7 +85,7 @@ export class SubagentToolWrapper extends BaseDeclarativeTool<
     // Special handling for browser agent - needs async MCP setup
     if (definition.name === BROWSER_AGENT_NAME) {
       return new BrowserAgentInvocation(
-        this.config,
+        this.context,
         params,
         effectiveMessageBus,
         _toolName,

packages/core/src/scheduler/policy.test.ts

@@ -676,6 +676,43 @@ describe('policy.ts', () => {
         }),
       );
     });
+
+    it('should work when context is created via Object.create (prototype chain)', async () => {
+      const mockConfig = {
+        setApprovalMode: vi.fn(),
+      } as unknown as Mocked<Config>;
+      const mockMessageBus = {
+        publish: vi.fn(),
+      } as unknown as Mocked<MessageBus>;
+
+      const baseContext = {
+        config: mockConfig,
+        messageBus: mockMessageBus,
+      };
+      const protoContext: AgentLoopContext = Object.create(baseContext);
+
+      expect(Object.keys(protoContext)).toHaveLength(0);
+      expect(protoContext.config).toBe(mockConfig);
+      expect(protoContext.messageBus).toBe(mockMessageBus);
+
+      const tool = { name: 'test-tool' } as AnyDeclarativeTool;
+
+      await updatePolicy(
+        tool,
+        ToolConfirmationOutcome.ProceedAlways,
+        undefined,
+        protoContext,
+        mockMessageBus,
+      );
+
+      expect(mockMessageBus.publish).toHaveBeenCalledWith(
+        expect.objectContaining({
+          type: MessageBusType.UPDATE_POLICY,
+          toolName: 'test-tool',
+          persist: false,
+        }),
+      );
+    });
   });
 
   describe('getPolicyDenialError', () => {

packages/core/src/services/trackerTypes.ts

@@ -13,6 +13,12 @@ export enum TaskType {
 }
 export const TaskTypeSchema = z.nativeEnum(TaskType);
 
+export const TASK_TYPE_LABELS: Record<TaskType, string> = {
+  [TaskType.EPIC]: '[EPIC]',
+  [TaskType.TASK]: '[TASK]',
+  [TaskType.BUG]: '[BUG]',
+};
+
 export enum TaskStatus {
   OPEN = 'open',
   IN_PROGRESS = 'in_progress',

packages/core/src/tools/trackerTools.test.ts

@@ -14,12 +14,14 @@ import {
   TrackerUpdateTaskTool,
   TrackerVisualizeTool,
   TrackerAddDependencyTool,
+  buildTodosReturnDisplay,
 } from './trackerTools.js';
 import * as fs from 'node:fs/promises';
 import * as path from 'node:path';
 import * as os from 'node:os';
 
 import { TaskStatus, TaskType } from '../services/trackerTypes.js';
+import type { TrackerService } from '../services/trackerService.js';
 
 describe('Tracker Tools Integration', () => {
   let tempDir: string;
@@ -142,4 +144,90 @@ describe('Tracker Tools Integration', () => {
     expect(vizResult.llmContent).toContain('Child Task');
     expect(vizResult.llmContent).toContain(childId);
   });
+
+  describe('buildTodosReturnDisplay', () => {
+    it('returns empty list for no tasks', async () => {
+      const mockService = {
+        listTasks: async () => [],
+      } as unknown as TrackerService;
+      const result = await buildTodosReturnDisplay(mockService);
+      expect(result.todos).toEqual([]);
+    });
+
+    it('returns formatted todos', async () => {
+      const parent = {
+        id: 'p1',
+        title: 'Parent',
+        type: TaskType.TASK,
+        status: TaskStatus.IN_PROGRESS,
+        dependencies: [],
+      };
+      const child = {
+        id: 'c1',
+        title: 'Child',
+        type: TaskType.EPIC,
+        status: TaskStatus.OPEN,
+        parentId: 'p1',
+        dependencies: [],
+      };
+      const closedLeaf = {
+        id: 'leaf',
+        title: 'Closed Leaf',
+        type: TaskType.BUG,
+        status: TaskStatus.CLOSED,
+        parentId: 'c1',
+        dependencies: [],
+      };
+
+      const mockService = {
+        listTasks: async () => [parent, child, closedLeaf],
+      } as unknown as TrackerService;
+      const display = await buildTodosReturnDisplay(mockService);
+
+      expect(display.todos).toEqual([
+        {
+          description: `[p1] [TASK] Parent`,
+          status: 'in_progress',
+        },
+        {
+          description: `  [c1] [EPIC] Child`,
+          status: 'pending',
+        },
+        {
+          description: `    [leaf] [BUG] Closed Leaf`,
+          status: 'completed',
+        },
+      ]);
+    });
+
+    it('detects cycles', async () => {
+      // Since TrackerTask only has a single parentId, a true cycle is unreachable from roots.
+      // We simulate a database corruption (two tasks with same ID, one root, one child)
+      // just to exercise the protective cycle detection branch.
+      const rootP1 = {
+        id: 'p1',
+        title: 'Parent',
+        type: TaskType.TASK,
+        status: TaskStatus.OPEN,
+        dependencies: [],
+      };
+      const childP1 = { ...rootP1, parentId: 'p1' };
+
+      const mockService = {
+        listTasks: async () => [rootP1, childP1],
+      } as unknown as TrackerService;
+      const display = await buildTodosReturnDisplay(mockService);
+
+      expect(display.todos).toEqual([
+        {
+          description: `[p1] [TASK] Parent`,
+          status: 'pending',
+        },
+        {
+          description: `  [CYCLE DETECTED: p1]`,
+          status: 'cancelled',
+        },
+      ]);
+    });
+  });
 });

packages/core/src/tools/trackerTools.ts

@@ -23,11 +23,69 @@ import {
   TRACKER_UPDATE_TASK_TOOL_NAME,
   TRACKER_VISUALIZE_TOOL_NAME,
 } from './tool-names.js';
-import type { ToolResult } from './tools.js';
+import type { ToolResult, TodoList } from './tools.js';
 import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
 import { ToolErrorType } from './tool-error.js';
 import type { TrackerTask, TaskType } from '../services/trackerTypes.js';
-import { TaskStatus } from '../services/trackerTypes.js';
+import { TaskStatus, TASK_TYPE_LABELS } from '../services/trackerTypes.js';
+import type { TrackerService } from '../services/trackerService.js';
+
+export async function buildTodosReturnDisplay(
+  service: TrackerService,
+): Promise<TodoList> {
+  const tasks = await service.listTasks();
+  const childrenMap = new Map<string, TrackerTask[]>();
+  const roots: TrackerTask[] = [];
+
+  for (const task of tasks) {
+    if (task.parentId) {
+      if (!childrenMap.has(task.parentId)) {
+        childrenMap.set(task.parentId, []);
+      }
+      childrenMap.get(task.parentId)!.push(task);
+    } else {
+      roots.push(task);
+    }
+  }
+
+  const todos: TodoList['todos'] = [];
+
+  const addTask = (task: TrackerTask, depth: number, visited: Set<string>) => {
+    if (visited.has(task.id)) {
+      todos.push({
+        description: `${'  '.repeat(depth)}[CYCLE DETECTED: ${task.id}]`,
+        status: 'cancelled',
+      });
+      return;
+    }
+    visited.add(task.id);
+
+    let status: 'pending' | 'in_progress' | 'completed' | 'cancelled' =
+      'pending';
+    if (task.status === TaskStatus.IN_PROGRESS) {
+      status = 'in_progress';
+    } else if (task.status === TaskStatus.CLOSED) {
+      status = 'completed';
+    }
+
+    const indent = '  '.repeat(depth);
+    const description = `${indent}[${task.id}] ${TASK_TYPE_LABELS[task.type]} ${task.title}`;
+
+    todos.push({ description, status });
+
+    const children = childrenMap.get(task.id) ?? [];
+    for (const child of children) {
+      addTask(child, depth + 1, visited);
+    }
+    visited.delete(task.id);
+  };
+
+  for (const root of roots) {
+    addTask(root, 0, new Set());
+  }
+
+  return { todos };
+}
 
 // --- tracker_create_task ---
 
@@ -71,7 +129,7 @@ class TrackerCreateTaskInvocation extends BaseToolInvocation<
       });
       return {
         llmContent: `Created task ${task.id}: ${task.title}`,
-        returnDisplay: `Created task ${task.id}.`,
+        returnDisplay: await buildTodosReturnDisplay(this.service),
       };
     } catch (error) {
       const errorMessage =
@@ -155,7 +213,7 @@ class TrackerUpdateTaskInvocation extends BaseToolInvocation<
       const task = await this.service.updateTask(id, updates);
       return {
         llmContent: `Updated task ${task.id}. Status: ${task.status}`,
-        returnDisplay: `Updated task ${task.id}.`,
+        returnDisplay: await buildTodosReturnDisplay(this.service),
       };
     } catch (error) {
       const errorMessage =
@@ -239,7 +297,7 @@ class TrackerGetTaskInvocation extends BaseToolInvocation<
     }
     return {
       llmContent: JSON.stringify(task, null, 2),
-      returnDisplay: `Retrieved task ${task.id}.`,
+      returnDisplay: await buildTodosReturnDisplay(this.service),
     };
   }
 }
@@ -327,7 +385,7 @@ class TrackerListTasksInvocation extends BaseToolInvocation<
       .join('\n');
     return {
       llmContent: content,
-      returnDisplay: `Listed ${tasks.length} tasks.`,
+      returnDisplay: await buildTodosReturnDisplay(this.service),
     };
   }
 }
@@ -427,7 +485,7 @@ class TrackerAddDependencyInvocation extends BaseToolInvocation<
       await this.service.updateTask(task.id, { dependencies: newDeps });
       return {
         llmContent: `Linked ${task.id} -> ${dep.id}.`,
-        returnDisplay: 'Dependency added.',
+        returnDisplay: await buildTodosReturnDisplay(this.service),
       };
     } catch (error) {
       const errorMessage =
@@ -516,12 +574,6 @@ class TrackerVisualizeInvocation extends BaseToolInvocation<
       closed: '✅',
     };
 
-    const typeLabels: Record<TaskType, string> = {
-      epic: '[EPIC]',
-      task: '[TASK]',
-      bug: '[BUG]',
-    };
-
     const childrenMap = new Map<string, TrackerTask[]>();
     const roots: TrackerTask[] = [];
 
@@ -550,14 +602,15 @@ class TrackerVisualizeInvocation extends BaseToolInvocation<
       visited.add(task.id);
 
       const indent = '  '.repeat(depth);
-      output += `${indent}${statusEmojis[task.status]} ${task.id} ${typeLabels[task.type]} ${task.title}\n`;
+      output += `${indent}${statusEmojis[task.status]} ${task.id} ${TASK_TYPE_LABELS[task.type]} ${task.title}\n`;
       if (task.dependencies.length > 0) {
         output += `${indent}  └─ Depends on: ${task.dependencies.join(', ')}\n`;
       }
       const children = childrenMap.get(task.id) ?? [];
       for (const child of children) {
-        renderTask(child, depth + 1, new Set(visited));
+        renderTask(child, depth + 1, visited);
       }
+      visited.delete(task.id);
     };
 
     for (const root of roots) {
@@ -566,7 +619,7 @@ class TrackerVisualizeInvocation extends BaseToolInvocation<
 
     return {
       llmContent: output,
-      returnDisplay: output,
+      returnDisplay: await buildTodosReturnDisplay(this.service),
     };
   }
 }