Browse Source

Merge pull request #842 from alvinunreal/feat/agent-task-rejection

feat(agents): add task rejection contract
Alvin 2 weeks ago
parent
commit
3636d1ab18

+ 7 - 0
docs/background-orchestration.md

@@ -266,6 +266,13 @@ Include:
 - validation to run or report,
 - what not to do.
 
+### Task-fit rejections
+
+If a task is outside a specialist's role, it must not attempt partial work. It
+returns a brief reason to the orchestrator.
+The orchestrator treats that reason as routing input to reroute or clarify the
+task and must not retry the unchanged task with the same specialist.
+
 Good background task prompt:
 
 ```text

+ 1 - 14
src/agents/fixer.ts

@@ -7,11 +7,6 @@ const FIXER_PROMPT = `You are Fixer - a fast, focused implementation specialist.
 
 **Behavior**:
 - Execute the task specification provided by the Orchestrator
-- Use the research context (file paths, documentation, patterns) provided
-- Read files before using edit/write tools and gather exact content before making changes
-- Be fast and direct - no research, no delegation, No multi-step research/planning; minimal execution sequence ok
-- Write or update tests when requested, especially for bounded tasks involving test files, fixtures, mocks, or test helpers
-- Run relevant validation when requested or clearly applicable (otherwise note as skipped with reason)
 - Report completion with summary of changes
 
 ${WRITABLE_FILE_OPERATIONS_RULES}
@@ -37,15 +32,7 @@ Brief summary of what was implemented
 - Tests passed: [yes/no/skip reason]
 - Validation: [passed/failed/skip reason]
 </verification>
-
-Use the following when no code changes were made:
-<summary>
-No changes required
-</summary>
-<verification>
-- Tests passed: [not run - reason]
-- Validation: [not run - reason]
-</verification>`;
+`;
 
 export function createFixerAgent(
   model: string,

+ 57 - 0
src/agents/index.test.ts

@@ -14,6 +14,7 @@ import {
   getDisabledAgents,
   isSubagent,
 } from './index';
+import { TASK_REJECTION_INSTRUCTION } from './task-rejection';
 
 function councilConfig() {
   const parsed = CouncilConfigSchema.parse({
@@ -467,6 +468,62 @@ describe('agent classification', () => {
 });
 
 describe('createAgents', () => {
+  test('keeps task-rejection instructions in default subagent prompts without modifying replacements', () => {
+    const agents = createAgents({
+      disabled_agents: [],
+      council: councilConfig(),
+      agents: {
+        explorer: {
+          model: 'test/explorer',
+          prompt: 'Replacement explorer prompt.',
+        },
+        reviewer: {
+          model: 'test/reviewer',
+          prompt: 'Custom reviewer prompt.',
+        },
+      },
+      acpAgents: {
+        bridge: {
+          command: 'bridge-acp',
+          args: [],
+          env: {},
+          timeoutMs: 0,
+          permissionMode: 'ask',
+        },
+      },
+    });
+
+    const orchestrator = agents.find((agent) => agent.name === 'orchestrator');
+    const explorer = agents.find((agent) => agent.name === 'explorer');
+
+    expect(explorer?.config.prompt).toBe('Replacement explorer prompt.');
+    expect(orchestrator?.config.prompt).not.toContain(
+      TASK_REJECTION_INSTRUCTION,
+    );
+    expect(agents.map((agent) => agent.name)).toEqual(
+      expect.arrayContaining([
+        'observer',
+        'council',
+        'councillor',
+        'councillor-alpha',
+        'reviewer',
+        'bridge',
+      ]),
+    );
+
+    for (const agent of agents.filter((agent) =>
+      [
+        'observer',
+        'council',
+        'councillor',
+        'councillor-alpha',
+        'bridge',
+      ].includes(agent.name),
+    )) {
+      expect(agent.config.prompt).toContain(TASK_REJECTION_INSTRUCTION);
+    }
+  });
+
   test('creates all agents without config', () => {
     const agents = createAgents();
     const names = agents.map((a) => a.name);

+ 14 - 2
src/agents/index.ts

@@ -30,6 +30,7 @@ import {
   createOrchestratorAgent,
   resolvePrompt,
 } from './orchestrator';
+import { appendTaskRejectionInstruction } from './task-rejection';
 
 export type { AgentDefinition } from './orchestrator';
 
@@ -231,7 +232,10 @@ function buildCustomAgentDefinition(
   filePrompt?: string,
   fileAppendPrompt?: string,
 ): AgentDefinition {
-  const basePrompt = override.prompt ?? `You are the ${name} specialist.`;
+  const defaultPrompt = appendTaskRejectionInstruction(
+    `You are the ${name} specialist.`,
+  );
+  const basePrompt = override.prompt ?? defaultPrompt;
   const primaryModel = getPrimaryModelFromOverride(override);
 
   return {
@@ -388,7 +392,9 @@ export function createAgents(
 
       const override = getAgentOverride(config, name);
       const inlinePrompt = override?.prompt;
-      const defaultPrompt = agent.config.prompt ?? '';
+      const defaultPrompt = appendTaskRejectionInstruction(
+        agent.config.prompt ?? '',
+      );
 
       const basePrompt =
         inlinePrompt !== undefined ? inlinePrompt : defaultPrompt;
@@ -508,6 +514,12 @@ export function createAgents(
     ...councillorAgents,
   ];
 
+  for (const agent of [...acpSubAgents, ...councillorAgents]) {
+    agent.config.prompt = appendTaskRejectionInstruction(
+      agent.config.prompt ?? '',
+    );
+  }
+
   // 3. Create Orchestrator (with its own overrides and custom prompts)
   // DEFAULT_MODELS.orchestrator is undefined; model is resolved via override or
   // left unset so the runtime chat.message hook can pick it from _modelArray.

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

@@ -11,4 +11,5 @@ describe('orchestrator prompt', () => {
     expect(prompt).toContain('small bounded set of options');
     expect(prompt).toContain('ordinary dialogue that does not block work');
   });
+
 });

+ 1 - 1
src/agents/orchestrator.ts

@@ -200,7 +200,7 @@ Balance: respect dependencies, avoid parallelizing what must be sequential, and
 ### Background Task Discipline
 - Prefer \`task(..., background: true)\` for delegated work that can run independently.
 - For work already chosen for delegation, launch independent specialist lanes in the background so the orchestrator stays unblocked and can reconcile results when they return.
-- Track each task's specialist, objective, task/session ID, and file/topic ownership.
+- Never reissue an unchanged task to the same specialist after a rejection; adjust its scope or context before retrying.
 - Continue orchestration only on non-overlapping work; otherwise briefly report what was launched and stop.
 - Before local edits or another writer task, compare against running task scopes.
 - Parallel background tasks are allowed only when their write scopes do not conflict.

+ 15 - 0
src/agents/task-rejection.test.ts

@@ -0,0 +1,15 @@
+import { describe, expect, test } from 'bun:test';
+import { TASK_REJECTION_INSTRUCTION } from './task-rejection';
+
+describe('task rejection instruction', () => {
+  test('requires a plain reason-only response', () => {
+    expect(TASK_REJECTION_INSTRUCTION).toBe(
+      'If a task is outside your role, do not attempt partial work. Return a brief reason to the orchestrator.',
+    );
+    expect(TASK_REJECTION_INSTRUCTION).not.toMatch(
+      /<|>|task_rejection|recommended[_ -]?agent/i,
+    );
+    expect(TASK_REJECTION_INSTRUCTION).not.toContain('permissions');
+    expect(TASK_REJECTION_INSTRUCTION).not.toContain('available context');
+  });
+});

+ 6 - 0
src/agents/task-rejection.ts

@@ -0,0 +1,6 @@
+export const TASK_REJECTION_INSTRUCTION =
+  'If a task is outside your role, do not attempt partial work. Return a brief reason to the orchestrator.';
+
+export function appendTaskRejectionInstruction(prompt: string): string {
+  return `${prompt}\n\n${TASK_REJECTION_INSTRUCTION}`;
+}

+ 1 - 1
src/hooks/__snapshots__/cache-payload.snapshot.test.ts.snap

@@ -151,7 +151,7 @@ Balance: respect dependencies, avoid parallelizing what must be sequential, and
 ### Background Task Discipline
 - Prefer \`task(..., background: true)\` for delegated work that can run independently.
 - For work already chosen for delegation, launch independent specialist lanes in the background so the orchestrator stays unblocked and can reconcile results when they return.
-- Track each task's specialist, objective, task/session ID, and file/topic ownership.
+- Never reissue an unchanged task to the same specialist after a rejection; adjust its scope or context before retrying.
 - Continue orchestration only on non-overlapping work; otherwise briefly report what was launched and stop.
 - Before local edits or another writer task, compare against running task scopes.
 - Parallel background tasks are allowed only when their write scopes do not conflict.