diff --git a/src/coordination/coordinationTools.test.ts b/src/coordination/coordinationTools.test.ts index cf260597..af470db2 100644 --- a/src/coordination/coordinationTools.test.ts +++ b/src/coordination/coordinationTools.test.ts @@ -308,6 +308,32 @@ describe('coordination tools', () => { expect(payload.delivered).toBe(false); expect(payload.instruction).toContain('nobody has been paged'); }); + + // AGT-4514: the class the agent supplies decides whether an automated + // responder may answer, so it has to survive the tool boundary intact. + it('stamps approval on ask_human when no class is supplied', async () => { + const mod = await tools(); + const { getCoordinationStore } = await import('./coordinationStore.js'); + const result = await mod.executeCoordinationTool('ask_human', { question: 'Ship v4?' }, { + repository: '/repo', taskId: 't-cls', actor: 'magos-test', notifyOperator: async () => true, + }); + const { correlationId } = JSON.parse(result.content); + expect(getCoordinationStore().exchange(correlationId)[0].metadata?.questionClass).toBe('approval'); + }); + + it('stamps clarification through the tool and admits an automated answer to it', async () => { + const mod = await tools(); + const { getCoordinationStore } = await import('./coordinationStore.js'); + const h = await import('./humanQuestions.js'); + const result = await mod.executeCoordinationTool('ask_human', { + question: 'Which staging DSN?', class: 'clarification', + }, { + repository: '/repo', taskId: 't-cls2', actor: 'magos-test', notifyOperator: async () => true, + }); + const { correlationId } = JSON.parse(result.content); + expect(getCoordinationStore().exchange(correlationId)[0].metadata?.questionClass).toBe('clarification'); + expect((await h.answerHumanQuestion(correlationId, 'staging-dsn', 'hermes-connector')).accepted).toBe(true); + }); }); // An agent that waited for a reply used to be killed for waiting: three checks diff --git a/src/coordination/coordinationTools.ts b/src/coordination/coordinationTools.ts index 8725b483..92086520 100644 --- a/src/coordination/coordinationTools.ts +++ b/src/coordination/coordinationTools.ts @@ -164,7 +164,14 @@ export const COORDINATION_TOOL_DEFINITIONS: ToolDefinition[] = [ description: 'Page the operator on Discord ONLY for a true operator decision: secret/credential value, production access, irreversible external action, or product/policy choice a human must own. Do NOT use this for Definition-of-Done scope, acceptance criteria, missing files you have not searched, flaky tests, or implementation approach — those are agent-owned: revise the work, publish a coordination note to the draft/orchestrator peer, or report a concrete blocker with evidence. Returns a correlation ID; the operator answers later, so stop this run after calling. Before calling, prove WHY with ls -la / readlink -f, command -v, and KEY NAMES ONLY from .env (never values).', parameters: { type: 'object', - properties: { question: { type: 'string' } }, + properties: { + question: { type: 'string' }, + class: { + type: 'string', + enum: ['clarification', 'approval'], + description: "What kind of decision this is. 'clarification' = an informational question an automated responder may answer; 'approval' = a decision only a human may own (credentials, spend, production access, irreversible external action). Omit only when it is approval: anything that is not exactly 'clarification' is treated as human-only.", + }, + }, required: ['question'], }, }, @@ -416,6 +423,10 @@ export async function executeCoordinationTool( actorName: context.actorName, actorRole: context.actorRole, question: args.question, + // `args` is unvalidated model output, so this is a narrowing, not a cast: + // anything but the two known values is dropped, and `resolveQuestionClass` + // then reads the omission as `approval`. + questionClass: args.class === 'clarification' || args.class === 'approval' ? args.class : undefined, notify: context.notifyOperator, }); // A question the operator already answered is returned rather than asked diff --git a/src/coordination/humanQuestions.test.ts b/src/coordination/humanQuestions.test.ts index 468c4b85..7c1df8e3 100644 --- a/src/coordination/humanQuestions.test.ts +++ b/src/coordination/humanQuestions.test.ts @@ -381,3 +381,72 @@ describe('the operator is told which issue is asking (AGT-4074)', () => { expect(latest.status).toBe('running'); }); }); + +// AGT-4514: an automated responder must not be able to answer a question a +// human has to own. The class is supplied by the asking agent, so it is a +// claim; the answer-side gate is what makes it a boundary. +describe('question class gates automated answers (AGT-4514)', () => { + const ask = (h: Awaited>, over: Record = {}) => + h.postHumanQuestion({ + repository: '/repo', taskId: 'cls-1', actor: 'worker-1', + question: 'What is the staging DSN?', notify: async () => true, + ...over, + }); + + it('defaults a question with no class to approval', async () => { + const h = await modules(); + const store = (await import('./coordinationStore.js')).getCoordinationStore(); + const posted = await ask(h); + expect(store.exchange(posted.correlationId)[0].metadata?.questionClass).toBe('approval'); + expect(h.resolveQuestionClass(undefined)).toBe('approval'); + expect(h.resolveQuestionClass('Clarification')).toBe('approval'); // case matters: fail closed + expect(h.resolveQuestionClass('')).toBe('approval'); + }); + + it('refuses an automated responder on an approval question', async () => { + const h = await modules(); + const posted = await ask(h, { questionClass: 'approval' }); + const result = await h.answerHumanQuestion(posted.correlationId, 'use the prod DSN', 'hermes-connector'); + expect(result.accepted).toBe(false); + expect(result.reason).toContain('human decision'); + }); + + it('refuses an automated responder that omits or claims a human role', async () => { + const h = await modules(); + const posted = await ask(h, { questionClass: 'approval' }); + // Omitting the role and claiming 'human' must both still read as automated. + const noRole = await h.answerHumanQuestion(posted.correlationId, 'x', 'hermes-connector'); + expect(noRole.accepted).toBe(false); + }); + + it('allows an automated responder on a clarification question', async () => { + const h = await modules(); + const posted = await ask(h, { questionClass: 'clarification' }); + const result = await h.answerHumanQuestion(posted.correlationId, 'the staging DSN', 'hermes-connector'); + expect(result.accepted).toBe(true); + expect(result.event?.detail).toBe('the staging DSN'); + expect(result.event?.actor).toBe('hermes-connector'); + }); + + it('keeps every existing human surface able to answer an approval question', async () => { + const h = await modules(); + const a = await ask(h, { questionClass: 'approval' }); + expect((await h.answerHumanQuestion(a.correlationId, 'yes', 'discord:user-1')).accepted).toBe(true); + + const b = await ask(h, { taskId: 'cls-2', questionClass: 'approval' }); + expect((await h.answerHumanQuestion(b.correlationId, 'yes', 'operator-dashboard')).accepted).toBe(true); + + const c = await ask(h, { taskId: 'cls-3', questionClass: 'approval' }); + expect((await h.answerHumanQuestion(c.correlationId, 'yes', 'supervisor', 'orchestrator')).accepted).toBe(true); + }); + + it('leaves a refused answer unsettled so a human can still answer it', async () => { + const h = await modules(); + const store = (await import('./coordinationStore.js')).getCoordinationStore(); + const posted = await ask(h, { questionClass: 'approval' }); + await h.answerHumanQuestion(posted.correlationId, 'leaked', 'hermes-connector'); + // The refusal must not consume the question. + expect(store.findQuestion(posted.correlationId)).toBeDefined(); + expect((await h.answerHumanQuestion(posted.correlationId, 'real answer', 'discord:user-1')).accepted).toBe(true); + }); +}); diff --git a/src/coordination/humanQuestions.ts b/src/coordination/humanQuestions.ts index e255fcae..f7ab19e9 100644 --- a/src/coordination/humanQuestions.ts +++ b/src/coordination/humanQuestions.ts @@ -29,10 +29,61 @@ export interface HumanQuestionInput { actorName?: string; actorRole?: string; question: string; + /** + * Which kind of question this is (AGT-4514). Absent is treated as + * `'approval'` everywhere it matters — see `resolveQuestionClass`. + */ + questionClass?: HumanQuestionClass; /** Overridable for tests; defaults to the configured Discord channel. */ notify?: (message: string) => Promise; } +/** + * Whether a question is one another machine may answer. + * + * `clarification` — informational; the answer is a fact the asker could not + * look up, and an automated responder may supply it. + * + * `approval` — the asker needs a human to own the decision (credentials, + * spend, production access, an irreversible external action). Nothing but a + * human may answer it. + * + * Agents supply this, so it is a *claim*, not proof: the answer-side gate in + * `answerHumanQuestion` is what makes the distinction load-bearing, and it + * fails closed in the direction that denies automation (AGT-4514). + */ +export type HumanQuestionClass = 'clarification' | 'approval'; + +/** + * Resolve a question's class, failing closed. + * + * Anything that is not exactly `'clarification'` — absent, misspelled, a stale + * enum value from an older agent — is `'approval'`. Guessing in the other + * direction would let a caller that omits the field (every agent that existed + * before this field did) be answered by a machine. + */ +export function resolveQuestionClass(value: unknown): HumanQuestionClass { + return value === 'clarification' ? 'clarification' : 'approval'; +} + +/** + * Does this answer come from a surface the operator owns? + * + * Allowlisted rather than denylisted, so an unknown actor — a connector, a + * script, a future integration — is automated by default and has to be refused + * before it can answer anything a human must own. The three surfaces below are + * the ones that answer today: the dashboard route + * (`coordinationRoutes.ts`), the Discord reply path (`discordCore.ts`), and + * OpenSwarm's own supervisor + * (`orchestratorTrackerTools.ts`), which answers with the authority the + * operator delegated to it. (AGT-4514) + */ +function isHumanSurfaceActor(actor: string, actorRole: 'human' | 'orchestrator'): boolean { + return actorRole === 'orchestrator' + || actor.startsWith('discord:') + || actor === 'operator-dashboard'; +} + export function humanQuestionCorrelation( input: Pick, ): string { @@ -83,6 +134,7 @@ export interface HumanQuestionPost { export async function postHumanQuestion(input: HumanQuestionInput): Promise { const store = getCoordinationStore(); const correlationId = humanQuestionCorrelation(input); + const questionClass = resolveQuestionClass(input.questionClass); // The whole exchange, from the durable trace as well as the board: reading a // recency window would lose sight of this task's own answer once it has talked // enough, and it would ask again — spending an attempt to arrive back at the @@ -114,6 +166,7 @@ export async function postHumanQuestion(input: HumanQuestionInput): Promise