Просмотр исходного кода

fix(task-session): suppress continuation while awaiting input

Alvin Unreal 2 недель назад
Родитель
Сommit
2498aa708b

+ 5 - 2
docs/background-orchestration.md

@@ -317,8 +317,11 @@ After an orchestrator session becomes idle, the plugin may send one internal,
 delayed continuation prompt when OpenCode reports incomplete todos. It is
 suppressed when the SDK reports the parent or any direct child as active, when a
 terminal child result has not yet been reconciled, during foreground fallback,
-or whenever SDK data is unavailable or malformed. A real subsequent user
-message rearms the one-shot nudge; internal prompts and todo updates do not.
+while OpenCode is waiting for a question or permission response, or whenever SDK
+data is unavailable or malformed. A matching reply, or a rejected question,
+clears that wait but does not itself inject a nudge; the normal session lifecycle
+decides whether a later nudge is needed. A real subsequent user message rearms
+the one-shot nudge; internal prompts and todo updates do not.
 
 This is a best-effort runtime check, not a scheduler or persisted state. After a
 plugin restart, the in-memory job board cannot establish prior result

+ 14 - 0
src/agents/orchestrator.test.ts

@@ -0,0 +1,14 @@
+import { describe, expect, test } from 'bun:test';
+import { buildOrchestratorPrompt } from './orchestrator';
+
+describe('orchestrator prompt', () => {
+  test('requires the question tool for blocking user input', () => {
+    const prompt = buildOrchestratorPrompt();
+
+    expect(prompt).toContain('use the `question` tool');
+    expect(prompt).toContain('Enable custom input');
+    expect(prompt).toContain('concise pasted response or command output');
+    expect(prompt).toContain('small bounded set of options');
+    expect(prompt).toContain('ordinary dialogue that does not block work');
+  });
+});

+ 2 - 0
src/agents/orchestrator.ts

@@ -237,6 +237,8 @@ Balance: respect dependencies, avoid parallelizing what must be sequential, and
 - If request is vague or has multiple valid interpretations, ask a targeted question before proceeding
 - Don't guess at critical details (file paths, API choices, architectural decisions)
 - Do make reasonable assumptions for minor details and state them briefly
+- When user input is required before work can continue—including clarification, permission, or command output—use the \`question\` tool rather than leaving an ordinary assistant prompt waiting. Enable custom input, request a concise pasted response or command output, and provide a small bounded set of options whenever the tool schema requires options.
+- For ordinary dialogue that does not block work, answer normally and do not use the question tool gratuitously.
 
 ## Concise Execution
 - Answer directly, no preamble

+ 341 - 0
src/hooks/task-session-manager/index.test.ts

@@ -2733,6 +2733,316 @@ describe('task-session-manager hook', () => {
     );
   });
 
+  test('does not evaluate or nudge while a question or permission waits', async () => {
+    const todo = mock(async () => ({ data: [{ status: 'pending' }] }));
+    const promptAsync = mock(async () => ({}));
+    const { hook } = createHook({
+      idleReconcileDelayMs: 0,
+      sessionClient: {
+        todo,
+        children: mock(async () => ({ data: [] })),
+        status: mock(async () => ({ data: {} })),
+        promptAsync,
+      },
+    });
+
+    await hook.event({
+      event: {
+        type: 'question.asked',
+        properties: { sessionID: 'parent-1', id: 'question-1' },
+      },
+    });
+    await hook.event({
+      event: {
+        type: 'permission.asked',
+        properties: { sessionID: 'parent-1', id: 'permission-1' },
+      },
+    });
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await flushContinuation();
+
+    expect(todo).not.toHaveBeenCalled();
+    expect(promptAsync).not.toHaveBeenCalled();
+  });
+
+  test('cancels a scheduled continuation when an input wait arrives before its timer fires', async () => {
+    const todo = mock(async () => ({ data: [{ status: 'pending' }] }));
+    const children = mock(async () => ({ data: [] }));
+    const status = mock(async () => ({ data: {} }));
+    const promptAsync = mock(async () => ({}));
+    const { hook } = createHook({
+      idleReconcileDelayMs: 0,
+      sessionClient: { todo, children, status, promptAsync },
+    });
+
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await hook.event({
+      event: {
+        type: 'permission.asked',
+        properties: { sessionID: 'parent-1', id: 'permission-1' },
+      },
+    });
+    await flushContinuation();
+
+    expect(todo).not.toHaveBeenCalled();
+    expect(children).not.toHaveBeenCalled();
+    expect(status).not.toHaveBeenCalled();
+    expect(promptAsync).not.toHaveBeenCalled();
+  });
+
+  test('clears only the resolved input wait and resumes on a later idle', async () => {
+    const promptAsync = mock(async () => ({}));
+    const { hook } = createHook({
+      idleReconcileDelayMs: 0,
+      sessionClient: {
+        todo: mock(async () => ({ data: [{ status: 'pending' }] })),
+        children: mock(async () => ({ data: [] })),
+        status: mock(async () => ({ data: {} })),
+        promptAsync,
+      },
+    });
+
+    await hook.event({
+      event: {
+        type: 'question.asked',
+        properties: { sessionID: 'parent-1', id: 'question-1' },
+      },
+    });
+    await hook.event({
+      event: {
+        type: 'permission.asked',
+        properties: { sessionID: 'parent-1', id: 'permission-1' },
+      },
+    });
+    await hook.event({
+      event: {
+        type: 'question.asked',
+        properties: { sessionID: 'parent-1', id: 'question-2' },
+      },
+    });
+    await hook.event({
+      event: {
+        type: 'question.replied',
+        properties: { sessionID: 'parent-1', requestID: 'unknown-question' },
+      },
+    });
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await flushContinuation();
+
+    expect(promptAsync).not.toHaveBeenCalled();
+
+    await hook.event({
+      event: {
+        type: 'question.replied',
+        properties: { sessionID: 'parent-1', requestID: 'question-1' },
+      },
+    });
+    await hook.event({
+      event: {
+        type: 'permission.replied',
+        properties: { sessionID: 'parent-1', requestID: 'permission-1' },
+      },
+    });
+    await flushContinuation();
+    expect(promptAsync).not.toHaveBeenCalled();
+
+    await hook.event({
+      event: {
+        type: 'question.rejected',
+        properties: { sessionID: 'parent-1', requestID: 'question-2' },
+      },
+    });
+    await flushContinuation();
+    expect(promptAsync).not.toHaveBeenCalled();
+
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await flushContinuation();
+
+    expect(promptAsync).toHaveBeenCalledTimes(1);
+  });
+
+  test('resumes on a later idle after a question rejection', async () => {
+    const promptAsync = mock(async () => ({}));
+    const { hook } = createHook({
+      idleReconcileDelayMs: 0,
+      sessionClient: {
+        todo: mock(async () => ({ data: [{ status: 'pending' }] })),
+        children: mock(async () => ({ data: [] })),
+        status: mock(async () => ({ data: {} })),
+        promptAsync,
+      },
+    });
+
+    await hook.event({
+      event: {
+        type: 'question.asked',
+        properties: { sessionID: 'parent-1', id: 'question-1' },
+      },
+    });
+    await hook.event({
+      event: {
+        type: 'question.rejected',
+        properties: { sessionID: 'parent-1', requestID: 'question-1' },
+      },
+    });
+    await flushContinuation();
+
+    expect(promptAsync).not.toHaveBeenCalled();
+
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await flushContinuation();
+
+    expect(promptAsync).toHaveBeenCalledTimes(1);
+  });
+
+  test('invalidates an in-flight continuation when an input wait arrives', async () => {
+    let resolveTodo!: (value: { data: { status: string }[] }) => void;
+    const todo = mock(
+      () =>
+        new Promise<{ data: { status: string }[] }>((resolve) => {
+          resolveTodo = resolve;
+        }),
+    );
+    const promptAsync = mock(async () => ({}));
+    const { hook } = createHook({
+      idleReconcileDelayMs: 0,
+      sessionClient: {
+        todo,
+        children: mock(async () => ({ data: [] })),
+        status: mock(async () => ({ data: {} })),
+        promptAsync,
+      },
+    });
+
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await flushContinuation();
+    expect(todo).toHaveBeenCalledTimes(1);
+
+    await hook.event({
+      event: {
+        type: 'question.asked',
+        properties: { sessionID: 'parent-1', id: 'question-1' },
+      },
+    });
+    resolveTodo({ data: [{ status: 'pending' }] });
+    await flushContinuation();
+
+    expect(promptAsync).not.toHaveBeenCalled();
+  });
+
+  test('internal and synthetic messages do not clear an input wait', async () => {
+    const todo = mock(async () => ({ data: [{ status: 'pending' }] }));
+    const promptAsync = mock(async () => ({}));
+    const { hook } = createHook({
+      idleReconcileDelayMs: 0,
+      sessionClient: {
+        todo,
+        children: mock(async () => ({ data: [] })),
+        status: mock(async () => ({ data: {} })),
+        promptAsync,
+      },
+    });
+
+    await hook.event({
+      event: {
+        type: 'question.asked',
+        properties: { sessionID: 'parent-1', id: 'question-1' },
+      },
+    });
+    hook.observeChatMessage(
+      {},
+      {
+        message: { role: 'user', sessionID: 'parent-1' },
+        parts: [
+          { type: 'text', synthetic: true, text: 'synthetic response' },
+          createInternalAgentTextPart('internal response'),
+        ],
+      },
+    );
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await flushContinuation();
+
+    expect(todo).not.toHaveBeenCalled();
+    expect(promptAsync).not.toHaveBeenCalled();
+  });
+
+  test('retains input waits across a session error', async () => {
+    const todo = mock(async () => ({ data: [{ status: 'pending' }] }));
+    const promptAsync = mock(async () => ({}));
+    const { hook } = createHook({
+      idleReconcileDelayMs: 0,
+      sessionClient: {
+        todo,
+        children: mock(async () => ({ data: [] })),
+        status: mock(async () => ({ data: {} })),
+        promptAsync,
+      },
+    });
+
+    await hook.event({
+      event: {
+        type: 'question.asked',
+        properties: { sessionID: 'parent-1', id: 'question-1' },
+      },
+    });
+    await hook.event({
+      event: { type: 'session.error', properties: { sessionID: 'parent-1' } },
+    });
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await flushContinuation();
+
+    expect(todo).not.toHaveBeenCalled();
+    expect(promptAsync).not.toHaveBeenCalled();
+  });
+
+  test('clears stale input waits on session and server cleanup', async () => {
+    for (const lifecycleEvent of [
+      { type: 'session.deleted', properties: { sessionID: 'parent-1' } },
+      { type: 'server.instance.disposed' },
+    ]) {
+      const promptAsync = mock(async () => ({}));
+      const { hook } = createHook({
+        idleReconcileDelayMs: 0,
+        sessionClient: {
+          todo: mock(async () => ({ data: [{ status: 'pending' }] })),
+          children: mock(async () => ({ data: [] })),
+          status: mock(async () => ({ data: {} })),
+          promptAsync,
+        },
+      });
+
+      await hook.event({
+        event: {
+          type: 'question.asked',
+          properties: { sessionID: 'parent-1', id: 'question-1' },
+        },
+      });
+      await hook.event({ event: lifecycleEvent });
+      await hook.event({
+        event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+      });
+      await flushContinuation();
+
+      expect(promptAsync).toHaveBeenCalledTimes(1);
+    }
+  });
+
   test('coalesces paired idle events and suppresses active children', async () => {
     const promptAsync = mock(async () => ({}));
     const children = mock(async () => ({ data: [{ id: 'child-1' }] }));
@@ -2792,6 +3102,37 @@ describe('task-session-manager hook', () => {
     expect(promptAsync).toHaveBeenCalledTimes(2);
   });
 
+  test('file-only external messages rearm a consumed nudge', async () => {
+    const promptAsync = mock(async () => ({}));
+    const { hook } = createHook({
+      idleReconcileDelayMs: 0,
+      sessionClient: {
+        todo: mock(async () => ({ data: [{ status: 'pending' }] })),
+        children: mock(async () => ({ data: [] })),
+        status: mock(async () => ({ data: {} })),
+        promptAsync,
+      },
+    });
+
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await flushContinuation();
+    hook.observeChatMessage(
+      {},
+      {
+        message: { role: 'user', sessionID: 'parent-1' },
+        parts: [{ type: 'file', filename: 'command-output.txt' }],
+      },
+    );
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'parent-1' } },
+    });
+    await flushContinuation();
+
+    expect(promptAsync).toHaveBeenCalledTimes(2);
+  });
+
   test('synthetic completion messages do not rearm a consumed nudge', async () => {
     const promptAsync = mock(async () => ({}));
     const { hook } = createHook({

+ 86 - 4
src/hooks/task-session-manager/index.ts

@@ -52,6 +52,27 @@ const IDLE_RECONCILE_DELAY_MS = 2_000;
 
 const CONTINUATION_NUDGE =
   'Continue coordinating the remaining incomplete todos. Do not finalize while work remains.';
+const INPUT_WAIT_ASK_EVENTS = {
+  'permission.asked': 'permission',
+  'question.asked': 'question',
+} as const;
+const INPUT_WAIT_RESOLUTION_EVENTS = {
+  'permission.replied': 'permission',
+  'question.replied': 'question',
+  'question.rejected': 'question',
+} as const;
+
+function isInputWaitAskEvent(
+  type: string,
+): type is keyof typeof INPUT_WAIT_ASK_EVENTS {
+  return Object.hasOwn(INPUT_WAIT_ASK_EVENTS, type);
+}
+
+function isInputWaitResolutionEvent(
+  type: string,
+): type is keyof typeof INPUT_WAIT_RESOLUTION_EVENTS {
+  return Object.hasOwn(INPUT_WAIT_RESOLUTION_EVENTS, type);
+}
 
 function djb2Hash(str: string): string {
   let hash = 5381;
@@ -133,6 +154,7 @@ export function createTaskSessionManagerHook(
   const continuationSessionTokens = new Map<string, symbol>();
   const activeContinuationEvaluations = new Map<string, Set<symbol>>();
   const continuationConsumed = new Set<string>();
+  const inputWaitsByParent = new Map<string, Set<string>>();
   const idleReconcileDelayMs =
     options.idleReconcileDelayMs ?? IDLE_RECONCILE_DELAY_MS;
 
@@ -183,6 +205,51 @@ export function createTaskSessionManagerHook(
     continuationConsumed.delete(sessionID);
   }
 
+  function inputWaitKey(kind: 'permission' | 'question', requestID: string) {
+    return `${kind}:${requestID}`;
+  }
+
+  function hasInputWait(sessionID: string): boolean {
+    return (inputWaitsByParent.get(sessionID)?.size ?? 0) > 0;
+  }
+
+  function clearInputWaits(sessionID: string): void {
+    inputWaitsByParent.delete(sessionID);
+  }
+
+  function trackInputWait(event: {
+    type: string;
+    properties?: { id?: string; requestID?: string; sessionID?: string };
+  }): void {
+    const sessionID = event.properties?.sessionID;
+    if (!sessionID || !options.shouldManageSession(sessionID)) {
+      return;
+    }
+
+    if (isInputWaitAskEvent(event.type)) {
+      const requestID = event.properties?.id;
+      if (!requestID) return;
+      const key = inputWaitKey(INPUT_WAIT_ASK_EVENTS[event.type], requestID);
+      const waits = inputWaitsByParent.get(sessionID) ?? new Set<string>();
+      waits.add(key);
+      inputWaitsByParent.set(sessionID, waits);
+      invalidateContinuation(sessionID);
+      return;
+    }
+
+    if (!isInputWaitResolutionEvent(event.type)) return;
+    const requestID = event.properties?.requestID;
+    if (!requestID) return;
+    const key = inputWaitKey(
+      INPUT_WAIT_RESOLUTION_EVENTS[event.type],
+      requestID,
+    );
+    const waits = inputWaitsByParent.get(sessionID);
+    if (!waits) return;
+    waits.delete(key);
+    if (waits.size === 0) clearInputWaits(sessionID);
+  }
+
   function isActiveStatus(
     status: Record<string, unknown>,
     sessionID: string,
@@ -202,6 +269,7 @@ export function createTaskSessionManagerHook(
 
     if (
       continuationConsumed.has(parentSessionID) ||
+      hasInputWait(parentSessionID) ||
       !isCurrentContinuation(parentSessionID, sessionToken, evaluationToken) ||
       options.isFallbackInProgress?.(parentSessionID) ||
       backgroundJobBoard.hasTerminalUnreconciled(parentSessionID) ||
@@ -281,6 +349,7 @@ export function createTaskSessionManagerHook(
           (child) => isObjectRecord(child) && typeof child.id === 'string',
         ) ||
         continuationConsumed.has(parentSessionID) ||
+        hasInputWait(parentSessionID) ||
         !isCurrentContinuation(
           parentSessionID,
           sessionToken,
@@ -304,6 +373,7 @@ export function createTaskSessionManagerHook(
 
       if (
         continuationConsumed.has(parentSessionID) ||
+        hasInputWait(parentSessionID) ||
         !isCurrentContinuation(
           parentSessionID,
           sessionToken,
@@ -340,6 +410,7 @@ export function createTaskSessionManagerHook(
   function scheduleIdleReconciliation(parentSessionID: string): void {
     if (
       idleReconcileTimers.has(parentSessionID) ||
+      hasInputWait(parentSessionID) ||
       options.isFallbackInProgress?.(parentSessionID)
     ) {
       return;
@@ -363,6 +434,7 @@ export function createTaskSessionManagerHook(
   if (options.coordinator) {
     options.coordinator.onSessionDeleted((sessionId) => {
       clearContinuation(sessionId);
+      clearInputWaits(sessionId);
       // During a foreground fallback abort/re-prompt cycle, the session
       // is being torn down and immediately recreated with a fallback model.
       // Dropping the job from the board here would make the orchestrator
@@ -667,10 +739,11 @@ export function createTaskSessionManagerHook(
         !parts.some(
           (part) =>
             isObjectRecord(part) &&
-            part.type === 'text' &&
-            typeof part.text === 'string' &&
             part.synthetic !== true &&
-            !isInternalInitiatorPart(part),
+            !isInternalInitiatorPart(part) &&
+            ((part.type === 'text' && typeof part.text === 'string') ||
+              part.type === 'file' ||
+              part.type === 'image'),
         )
       ) {
         return;
@@ -937,12 +1010,16 @@ export function createTaskSessionManagerHook(
         type: string;
         properties?: {
           info?: { id?: string; parentID?: string };
+          id?: string;
+          requestID?: string;
           sessionID?: string;
           status?: { type?: string };
           error?: { name?: string };
         };
       };
     }): Promise<void> => {
+      trackInputWait(input.event);
+
       if (input.event.type === 'session.created') {
         const info = input.event.properties?.info;
         log('[task-session-manager] session.created observed', {
@@ -997,9 +1074,11 @@ export function createTaskSessionManagerHook(
           ...continuationSessionTokens.keys(),
           ...activeContinuationEvaluations.keys(),
           ...continuationConsumed,
+          ...inputWaitsByParent.keys(),
         ]);
         for (const sessionID of continuationSessionIDs) {
           clearContinuation(sessionID);
+          clearInputWaits(sessionID);
         }
         return;
       }
@@ -1064,7 +1143,9 @@ export function createTaskSessionManagerHook(
       if (input.event.type === 'session.error') {
         const sessionId =
           input.event.properties?.info?.id || input.event.properties?.sessionID;
-        if (sessionId) invalidateContinuation(sessionId);
+        if (sessionId) {
+          invalidateContinuation(sessionId);
+        }
         if (sessionId && options.shouldManageSession(sessionId)) {
           // Only clear injected terminal jobs for fatal errors.
           // Rate-limit errors are recovered by ForegroundFallbackManager
@@ -1129,6 +1210,7 @@ export function createTaskSessionManagerHook(
       if (!sessionId) return;
 
       clearContinuation(sessionId);
+      clearInputWaits(sessionId);
 
       log('[task-session-manager] session.deleted observed', {
         sessionID: sessionId,