Browse Source

feat(background): add ask_orchestrator non-blocking question relay

Background subagents can now relay questions to the orchestrator without
blocking. The ask_orchestrator tool records questions on the task and
returns immediately. Subagents continue with best judgment using
[ASSUMED: ...] markers.

Changes:
- Add questions[] field to BackgroundTask (persisted, loaded with
  backward-compatible fallback to [])
- Add addQuestion() to BackgroundTaskManager (resolves task via
  session ID from toolContext)
- Create ask_orchestrator tool in createBackgroundTools
- Surface relayed questions in background_output
- Add question: false to calculateToolPermissions for all background
  tasks (prevents blocking question tool)
- Tests: ask_orchestrator tool (3), background_output questions (2),
  addQuestion manager (3), question:false smoke (2)

Ref #290
ReqX 3 months ago
parent
commit
001bfcb8c9

+ 63 - 0
src/background/background-manager.test.ts

@@ -1965,5 +1965,68 @@ describe('BackgroundTaskManager', () => {
         });
       });
     });
+
+    describe('addQuestion', () => {
+      test('records question on the correct task via session ID', async () => {
+        const ctx = createMockContext();
+        const manager = new BackgroundTaskManager(ctx);
+
+        const task = manager.launch({
+          agent: 'explorer',
+          prompt: 'find patterns',
+          description: 'test',
+          parentSessionId: 'root-session',
+        });
+
+        await Promise.resolve();
+        await Promise.resolve();
+
+        const sessionId = task.sessionId;
+        if (!sessionId) throw new Error('Expected sessionId');
+
+        const result = manager.addQuestion(
+          sessionId,
+          'Should I search tests too?',
+        );
+
+        expect(result).toBe(true);
+        expect(task.questions).toEqual(['Should I search tests too?']);
+      });
+
+      test('returns false for unknown session', () => {
+        const ctx = createMockContext();
+        const manager = new BackgroundTaskManager(ctx);
+
+        const result = manager.addQuestion(
+          'nonexistent-session',
+          'Anybody there?',
+        );
+
+        expect(result).toBe(false);
+      });
+
+      test('accumulates multiple questions', async () => {
+        const ctx = createMockContext();
+        const manager = new BackgroundTaskManager(ctx);
+
+        const task = manager.launch({
+          agent: 'oracle',
+          prompt: 'review',
+          description: 'test',
+          parentSessionId: 'root-session',
+        });
+
+        await Promise.resolve();
+        await Promise.resolve();
+
+        const sessionId = task.sessionId;
+        if (!sessionId) throw new Error('Expected sessionId');
+
+        manager.addQuestion(sessionId, 'First question?');
+        manager.addQuestion(sessionId, 'Second question?');
+
+        expect(task.questions).toEqual(['First question?', 'Second question?']);
+      });
+    });
   });
 });

+ 21 - 0
src/background/background-manager.ts

@@ -53,6 +53,7 @@ interface PersistedTask {
   error?: string;
   startedAt: string;
   completedAt?: string;
+  questions?: string[];
 }
 
 function persistTask(task: BackgroundTask): void {
@@ -72,6 +73,7 @@ function persistTask(task: BackgroundTask): void {
       error: task.error,
       startedAt: task.startedAt.toISOString(),
       completedAt: task.completedAt?.toISOString(),
+      questions: task.questions,
     };
     fs.writeFileSync(
       path.join(dir, `${task.id}.json`),
@@ -100,6 +102,7 @@ function loadPersistedTask(taskId: string): BackgroundTask | null {
       completedAt: data.completedAt ? new Date(data.completedAt) : undefined,
       prompt: data.prompt,
       config: data.config,
+      questions: data.questions ?? [],
     };
   } catch {
     return null;
@@ -131,6 +134,7 @@ export interface BackgroundTask {
   startedAt: Date; // Task creation timestamp
   completedAt?: Date; // Task completion/failure timestamp
   prompt: string; // Initial prompt
+  questions: string[]; // Questions relayed via ask_orchestrator
 }
 
 /**
@@ -277,6 +281,7 @@ export class BackgroundTaskManager {
       },
       parentSessionId: opts.parentSessionId,
       prompt: opts.prompt,
+      questions: [],
     };
 
     this.tasks.set(task.id, task);
@@ -745,6 +750,22 @@ export class BackgroundTaskManager {
     return fromDisk;
   }
 
+  /**
+   * Add a question relayed from a background subagent via ask_orchestrator.
+   * Resolves the task from the session ID in toolContext.
+   * Returns true if the question was recorded, false if the task wasn't found.
+   */
+  addQuestion(sessionId: string, question: string): boolean {
+    const taskId = this.tasksBySessionId.get(sessionId);
+    if (!taskId) return false;
+
+    const task = this.tasks.get(taskId);
+    if (!task) return false;
+
+    task.questions.push(question);
+    return true;
+  }
+
   /**
    * Wait for a task to complete.
    *

+ 127 - 0
src/tools/background.test.ts

@@ -22,11 +22,13 @@ function createMockManager() {
         config: { maxConcurrentStarts: 10 },
         parentSessionId: opts.parentSessionId,
         prompt: opts.prompt,
+        questions: [],
       }),
     ),
     getResult: mock(() => null),
     waitForCompletion: mock(async () => null),
     cancel: mock(() => 0),
+    addQuestion: mock(() => true),
   };
 }
 
@@ -98,3 +100,128 @@ describe('createBackgroundTools displayName runtime aliasing', () => {
     });
   });
 });
+
+describe('ask_orchestrator tool', () => {
+  test('records question via manager.addQuestion with session context', async () => {
+    const manager = createMockManager();
+    const tools = createBackgroundTools(
+      {} as any,
+      manager as any,
+      undefined,
+      undefined,
+    );
+
+    const result = await tools.ask_orchestrator.execute(
+      { question: 'Should I use REST or GraphQL?' },
+      { sessionID: 'session-bg-1' } as any,
+    );
+
+    expect(manager.addQuestion).toHaveBeenCalledWith(
+      'session-bg-1',
+      'Should I use REST or GraphQL?',
+    );
+    expect(result).toContain('Question recorded');
+    expect(result).toContain('[ASSUMED:');
+  });
+
+  test('returns non-blocking response even without session context', async () => {
+    const manager = createMockManager();
+    const tools = createBackgroundTools(
+      {} as any,
+      manager as any,
+      undefined,
+      undefined,
+    );
+
+    const result = await tools.ask_orchestrator.execute(
+      { question: 'What framework should I use?' },
+      undefined,
+    );
+
+    expect(manager.addQuestion).not.toHaveBeenCalled();
+    expect(result).toContain('Continue');
+  });
+
+  test('returns non-blocking response when task not found', async () => {
+    const manager = createMockManager();
+    manager.addQuestion = mock(() => false); // task not found
+    const tools = createBackgroundTools(
+      {} as any,
+      manager as any,
+      undefined,
+      undefined,
+    );
+
+    const result = await tools.ask_orchestrator.execute(
+      { question: 'Should I add tests?' },
+      { sessionID: 'session-cleaned-up' } as any,
+    );
+
+    expect(manager.addQuestion).toHaveBeenCalledWith(
+      'session-cleaned-up',
+      'Should I add tests?',
+    );
+    // Still non-blocking
+    expect(result).toContain('Continue');
+  });
+});
+
+describe('background_output surfaces questions', () => {
+  test('includes relayed questions in completed task output', async () => {
+    const manager = createMockManager();
+    const completedAt = new Date();
+    manager.getResult = mock(() => ({
+      id: 'bg_test1234',
+      description: 'Architecture analysis',
+      status: 'completed',
+      result: 'Use REST for this endpoint.',
+      startedAt: new Date(completedAt.getTime() - 5000),
+      completedAt,
+      questions: ['Should I use REST or GraphQL?', 'Should I add pagination?'],
+    }));
+
+    const tools = createBackgroundTools(
+      {} as any,
+      manager as any,
+      undefined,
+      undefined,
+    );
+
+    const result = await tools.background_output.execute({
+      task_id: 'bg_test1234',
+    });
+
+    expect(result).toContain('Use REST for this endpoint.');
+    expect(result).toContain('Questions relayed from subagent');
+    expect(result).toContain('Should I use REST or GraphQL?');
+    expect(result).toContain('Should I add pagination?');
+  });
+
+  test('no questions section when questions array is empty', async () => {
+    const manager = createMockManager();
+    const completedAt = new Date();
+    manager.getResult = mock(() => ({
+      id: 'bg_test1234',
+      description: 'Simple task',
+      status: 'completed',
+      result: 'Done.',
+      startedAt: new Date(completedAt.getTime() - 1000),
+      completedAt,
+      questions: [],
+    }));
+
+    const tools = createBackgroundTools(
+      {} as any,
+      manager as any,
+      undefined,
+      undefined,
+    );
+
+    const result = await tools.background_output.execute({
+      task_id: 'bg_test1234',
+    });
+
+    expect(result).toContain('Done.');
+    expect(result).not.toContain('Questions relayed');
+  });
+});

+ 51 - 1
src/tools/background.ts

@@ -148,6 +148,14 @@ Returns: results if completed, error if failed, status if running.`,
         output += '(Task still running)';
       }
 
+      // Surface relayed questions if any
+      if (task.questions.length > 0) {
+        output += '\n\n---\n\n**Questions relayed from subagent:**\n';
+        for (const q of task.questions) {
+          output += `- ${q}\n`;
+        }
+      }
+
       return output;
     },
   });
@@ -183,5 +191,47 @@ Only cancels pending/starting/running tasks.`,
     },
   });
 
-  return { background_task, background_output, background_cancel };
+  // Non-blocking question relay for background subagents
+  const ask_orchestrator = tool({
+    description: `Record a question for the orchestrator. NON-BLOCKING — you will NOT receive an answer.
+
+Use this when you need clarification but can proceed with a reasonable assumption.
+State your assumption explicitly using [ASSUMED: ...] markers before continuing.
+
+Example: "Should I use REST or GraphQL for this endpoint?" → pick one, mark [ASSUMED: using REST], keep working.`,
+    args: {
+      question: z
+        .string()
+        .describe(
+          'The question you need answered. Be specific so the orchestrator can evaluate your assumption.',
+        ),
+    },
+    async execute(args, toolContext) {
+      const sessionId =
+        toolContext &&
+        typeof toolContext === 'object' &&
+        'sessionID' in toolContext
+          ? (toolContext as { sessionID: string }).sessionID
+          : undefined;
+
+      if (!sessionId) {
+        return 'Question recorded (no session context). Continue with your assumption.';
+      }
+
+      const recorded = manager.addQuestion(sessionId, String(args.question));
+      if (!recorded) {
+        // Task already completed/cleaned up — still non-blocking
+        return 'Question recorded. Continue with your assumption.';
+      }
+
+      return 'Question recorded for orchestrator review. Continue with your best judgment using [ASSUMED: ...] markers.';
+    },
+  });
+
+  return {
+    background_task,
+    background_output,
+    background_cancel,
+    ask_orchestrator,
+  };
 }