From 131d6fdec7bc82bcf1de4cbde82d5f1d1754ea15 Mon Sep 17 00:00:00 2001 From: Yanek Yuk Date: Thu, 20 Aug 2026 18:05:34 -0500 Subject: [PATCH] feat(mcp): the question flow reaches external clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since #1472 an MCP client can SEE a park — "open question 3, 'Timing'" — and could do nothing about it. This closes the read-and-act story: get_negotiation says the park (same predicate, same shared record as the listing), answer_pending_question routes the answer over the existing #1466 host, reject/accept_opportunity give session-authenticated owners the #1471 verdict lane, the status filter gains input_required, the activity summary counts parked negotiations instead of the retired questions table, and the guidance/tool descriptions say all of it — including what external turns are NOT protected by. Claude-Session: https://claude.ai/code/session_01N1FQ3gV1oWWXQHMGGroqcV --- bun.lock | 2 +- packages/protocol/CHANGELOG.md | 72 ++++ packages/protocol/package.json | 2 +- .../src/mcp/mcp.authorization-policy.ts | 15 + packages/protocol/src/mcp/mcp.server.ts | 3 + .../tests/mcp.authorization-policy.spec.ts | 59 +++ .../negotiations/negotiation.answer.tools.ts | 156 ++++++++ .../negotiations/negotiation.detail-reader.ts | 103 +++++- .../src/negotiations/negotiation.module.ts | 1 + .../negotiations/negotiation.tools.port.ts | 9 + .../src/negotiations/negotiation.tools.ts | 53 ++- .../tests/negotiation.answer.tools.spec.ts | 267 ++++++++++++++ .../negotiation.tools.detail-park.spec.ts | 349 ++++++++++++++++++ .../src/opportunities/opportunity.module.ts | 3 + .../opportunity.verdict.tools.ts | 138 +++++++ .../tests/opportunity.verdict.tools.spec.ts | 148 ++++++++ .../src/shared/agent/canonical-guidance.ts | 32 +- .../protocol/src/shared/agent/tool.helpers.ts | 13 + .../src/shared/agent/tool.registry.ts | 14 +- .../api/src/adapters/chat.database.adapter.ts | 74 +++- ...gent-activity.database.adapter.isolated.ts | 85 ++++- .../api/src/controllers/mcp.controller.ts | 7 + services/api/tests/mcp.spec.ts | 55 ++- 23 files changed, 1608 insertions(+), 52 deletions(-) create mode 100644 packages/protocol/src/negotiations/negotiation.answer.tools.ts create mode 100644 packages/protocol/src/negotiations/tests/negotiation.answer.tools.spec.ts create mode 100644 packages/protocol/src/negotiations/tests/negotiation.tools.detail-park.spec.ts create mode 100644 packages/protocol/src/opportunities/opportunity.verdict.tools.ts create mode 100644 packages/protocol/src/opportunities/tests/opportunity.verdict.tools.spec.ts diff --git a/bun.lock b/bun.lock index c5fe688b4..851fc9449 100644 --- a/bun.lock +++ b/bun.lock @@ -89,7 +89,7 @@ }, "packages/protocol": { "name": "@indexnetwork/protocol", - "version": "23.5.1", + "version": "23.6.0", "dependencies": { "@langchain/core": "1.1.48", "@langchain/langgraph": "1.3.2", diff --git a/packages/protocol/CHANGELOG.md b/packages/protocol/CHANGELOG.md index d47c07150..a1cb370cb 100644 --- a/packages/protocol/CHANGELOG.md +++ b/packages/protocol/CHANGELOG.md @@ -20,6 +20,78 @@ went 6.7.1 → 8.0.2 with no 7.x in between because the whole 7.x line shipped a prereleases between the two promotions. To track every change, read `rc`; to pin a supported release, use `latest`. +## 23.6.0 - 2026-08-20 + +### Added + +- **The MCP surface can now ACT on the question flow, not just see it.** Since + 23.5.1 `list_negotiations` says the park — "open question 3, 'Timing'" — to + an external client that had no tool to answer it: the answer lane was the + negotiator persona's chat-only appended tool, and the owner's verdict levers + did not exist on MCP at all. Three tools close the read-and-act gap, all + registered on the MCP tool registry surface only: + + - `answer_pending_question(negotiationId, question, answer)` routes the + principal's answer to the open question a parked negotiation is waiting on, + over the EXISTING `NegotiatorAnswerToolsHost` — the same + `readOpenQuestionsForIntent` numbering, the same serialized consumption + queue, the same #1432 resume spine. MCP has no pinned intent, so the tool + resolves scope from the negotiation the client is looking at: the id the + park annotation sits on → the caller's own actor intent on that pairing → + the host. The `question` number passes through untouched; it and the park + annotation come from one enumeration and cannot drift (the #1470 rule). + - `reject_opportunity` / `accept_opportunity` (intentId, numbered + counterparty, optional reason in the owner's own words) reuse the + negotiator verdict host verbatim — the same Radar Skip/Start-Chat service + call, outcome hooks and question retirement in its wake, positions never + ids. Capability access is `human_only`: exactly the session-authenticated + class the IND-593 owner-provenance binding admits, and the handler + re-checks the host-bound provenance so an API-key agent is refused even if + the tool were ever mis-listed. Hermes negotiator credentials fail closed as + unclassified for all three tools. + +- **`get_negotiation` says the park** — the #1472 incident, one level down: it + is the tool the poller prompt says to call FIRST, and on a parked negotiation + it narrated a lifecycle built without the park. The detail reader now runs + the same canonical park predicate the listing runs (`classifyInflightPark` / + `classifyPostStallPark`) over the task and messages it already holds, names + the open question through the same shared host record, projects the park + top-level and into `lifecycle` (superseding the status label), and — for an + external seat that could not see dimensions or `settles` — projects the + persisted `askUser` and `checklist` payloads on turns that carry them. + Non-parked negotiations render byte-for-byte as before. + +- **`get_negotiation` re-stamps principal reachability from the live read.** + The persisted `turnContext` is a park-time snapshot; `ownUser.principalUnreachable` + is now corrected in both directions through the host's + `isPrincipalUnreachable` port — the same re-stamp REST pickup applies — so an + external seat is never told it can consult a principal nobody is behind, or + the reverse. + +- **`list_negotiations` can filter for the parked state.** The `status` filter + gains `input_required`; `active` and `all` behave exactly as before. A + post-stall park lives on a `completed` negotiation and is documented as such + rather than smuggled into the status filter. + +### Fixed + +- **`read_activity_summary` stops counting questions from the retired card + table.** Pending question counts now come from the parked negotiations — + a mid-flight `input_required` consult naming the owner as recipient, or a + post-stall park trailing the authored gap — keyed by the same + `negotiation_inflight`/`negotiation` modes the projection already maps to the + negotiations domain, so permission inheritance is unchanged. Leftover + pending rows in the retired `questions` table contribute nothing; answered + history still reads the table, where it legitimately lives. + +- **Honest words where external clients read them.** The negotiations guidance + topic, the workflows topic, and `MCP_INSTRUCTIONS` now carry the park stage — + what a park is, what the `park` fields mean, and that answering is the only + thing that resumes a parked negotiation. The three negotiation tool + descriptions state plainly that turns submitted through external surfaces are + NOT run through the conclusion floor, decline law, or copy-loop guard: until + external write parity ships, the docs must not imply otherwise. + ## 23.5.1 - 2026-08-20 ### Fixed diff --git a/packages/protocol/package.json b/packages/protocol/package.json index feefc048a..fbba41cf3 100644 --- a/packages/protocol/package.json +++ b/packages/protocol/package.json @@ -1,6 +1,6 @@ { "name": "@indexnetwork/protocol", - "version": "23.5.1", + "version": "23.6.0", "type": "module", "main": "./dist/index.js", "types": "./dist/index.d.ts", diff --git a/packages/protocol/src/mcp/mcp.authorization-policy.ts b/packages/protocol/src/mcp/mcp.authorization-policy.ts index 2bbaae892..40d6795cb 100644 --- a/packages/protocol/src/mcp/mcp.authorization-policy.ts +++ b/packages/protocol/src/mcp/mcp.authorization-policy.ts @@ -180,6 +180,21 @@ export const CANONICAL_MCP_TOOL_ACCESS_RULES = defineMcpToolAccessRules({ get_negotiation: { access: 'permission', actions: ['manage:negotiations'], reach: 'network' }, respond_to_negotiation: { access: 'permission', actions: ['manage:negotiations'], reach: 'network' }, + // The question flow (conversational-questions). `answer_pending_question` is + // the recipient principal's own answer lane: session humans (blanket allow) + // and the user's own agents holding manage:negotiations — the same trust the + // sibling negotiation tools extend, with the host scoping every read to the + // recipient's side. Principal reach: an open question is meta-network (it + // indexes the recipient's own signal), like the question aggregates in + // read_activity_summary. The owner VERDICT tools are `human_only` — exactly + // the session-authenticated class the IND-593 owner-provenance binding + // admits; an API-key agent must never gain an owner-verdict lever. None of + // the three appears in HERMES_AGENT_MCP_TOOL_PERMISSIONS, deliberately: + // Hermes negotiator credentials fail closed as unclassified. + answer_pending_question: { access: 'permission', actions: ['manage:negotiations'], reach: 'principal' }, + reject_opportunity: { access: 'human_only', reach: 'network' }, + accept_opportunity: { access: 'human_only', reach: 'network' }, + // H2A chat history. list_conversations: { access: 'human_only', reach: 'principal' }, get_conversation: { access: 'human_only', reach: 'principal' }, diff --git a/packages/protocol/src/mcp/mcp.server.ts b/packages/protocol/src/mcp/mcp.server.ts index 273ac47f1..f0faf21c7 100644 --- a/packages/protocol/src/mcp/mcp.server.ts +++ b/packages/protocol/src/mcp/mcp.server.ts @@ -351,6 +351,9 @@ NEVER dump raw JSON or expose IDs (except actionable ones like conversationId). # Authentication & Opportunity Lifecycle API key in \`x-api-key\` header. Opportunities: draft → pending → accepted/rejected. Agent acceptance ≠ owner approval. Only call update_opportunity with accepted after explicit user confirmation. +# Parked Negotiations & Answers +A negotiation can PARK waiting on a person's answer while its opportunity still reads \`negotiating\` — opportunity status never answers "is anything waiting on the user?". list_negotiations/get_negotiation annotate a parked negotiation with \`park\` (\`waitingOn: "you" | "counterparty"\`, plus the open question's number and label for the user's own side). When \`park.waitingOn\` is "you": relay the question, and route the user's answer with answer_pending_question using the shown question number — the ONLY thing that resumes a parked negotiation. A park on the counterparty's side names no question content. + # Tool Guidance Read each tool's description for usage rules (when, prerequisites, follow-ups). Tools contain workflow patterns. diff --git a/packages/protocol/src/mcp/tests/mcp.authorization-policy.spec.ts b/packages/protocol/src/mcp/tests/mcp.authorization-policy.spec.ts index 844473b97..1ed139d70 100644 --- a/packages/protocol/src/mcp/tests/mcp.authorization-policy.spec.ts +++ b/packages/protocol/src/mcp/tests/mcp.authorization-policy.spec.ts @@ -897,3 +897,62 @@ describe('signals read/write split (IND-588)', () => { } }); }); + +describe('MCP question flow (answer lane + owner verdicts)', () => { + const sessionHuman = () => resolveMcpCapabilitySubject({ + identity: identity({ isSessionAuth: true }), + }); + + test('answer_pending_question is the recipient principal lane: humans and manage:negotiations agents', () => { + expect(CANONICAL_MCP_TOOL_ACCESS_RULES.get('answer_pending_question')).toEqual({ + access: 'permission', + actions: ['manage:negotiations'], + reach: 'principal', + }); + + expect(policy.authorize(sessionHuman(), 'answer_pending_question')).toMatchObject({ + allowed: true, + reason: 'session_human', + }); + expect(policy.authorize(globalAgentSubject(['manage:negotiations']), 'answer_pending_question')).toMatchObject({ + allowed: true, + reason: 'permission_granted', + }); + expect(policy.authorize(globalAgentSubject(['manage:intents']), 'answer_pending_question')).toMatchObject({ + allowed: false, + reason: 'permission_missing', + }); + }); + + test('owner verdicts are human_only: exactly the class the IND-593 provenance binding admits', () => { + for (const tool of ['reject_opportunity', 'accept_opportunity'] as const) { + expect(CANONICAL_MCP_TOOL_ACCESS_RULES.get(tool)).toEqual({ access: 'human_only', reach: 'network' }); + expect(policy.authorize(sessionHuman(), tool)).toMatchObject({ allowed: true, reason: 'session_human' }); + // Every agent principal class is refused — including one holding every + // canonical permission: no grant releases an owner verdict. + expect(policy.authorize(globalAgentSubject([...MCP_PERMISSION_ACTIONS]), tool)).toMatchObject({ + allowed: false, + reason: 'human_only', + }); + } + }); + + test('Hermes negotiator credentials fail closed as unclassified on all three question-flow tools', () => { + const hermes = resolveMcpCapabilitySubject({ + identity: identity({ agentId: AGENT_ID, isHermesAgent: true }), + agent: agentSnapshot({ + permissions: [{ + agentId: AGENT_ID, + userId: USER_ID, + scope: 'global', + scopeId: null, + actions: [...MCP_PERMISSION_ACTIONS], + }], + }), + }); + for (const tool of ['answer_pending_question', 'reject_opportunity', 'accept_opportunity']) { + expect(HERMES_AGENT_MCP_TOOL_PERMISSIONS.get(tool)).toBeUndefined(); + expect(policy.authorize(hermes, tool)).toEqual({ allowed: false, reason: 'tool_unclassified' }); + } + }); +}); diff --git a/packages/protocol/src/negotiations/negotiation.answer.tools.ts b/packages/protocol/src/negotiations/negotiation.answer.tools.ts new file mode 100644 index 000000000..a55368839 --- /dev/null +++ b/packages/protocol/src/negotiations/negotiation.answer.tools.ts @@ -0,0 +1,156 @@ +import { z } from 'zod'; + +import type { DefineTool } from '../shared/agent/tool.helpers.js'; +import { success, error } from '../shared/agent/tool.helpers.js'; +import { focusedNetworkId } from '../shared/agent/tool.scope.js'; +import { protocolLogger } from '../shared/observability/protocol.logger.js'; +import type { NegotiationToolDeps } from './negotiation.tools.port.js'; + +/** + * The MCP-surface answer lane (`answer_pending_question`). + * + * `list_negotiations` and `get_negotiation` have said the park since #1472: + * "open question 3, 'Timing'" — to a client that, on this surface, had no tool + * to answer it. This closes the read-and-act gap over the EXISTING host + * (`NegotiatorAnswerToolsHost`): same `readOpenQuestionsForIntent` numbering, + * same serialized consumption queue, same #1432 resume spine underneath. + * Nothing is re-implemented. + * + * MCP has no pinned intent, so the tool resolves its scope from the + * negotiation the client is looking at: `negotiationId` (the id the listing + * and detail hand out next to the park) → the caller's own actor intent on + * that negotiation's opportunity → the host. The `question` number passes + * through untouched — it is the number the park annotation printed, and both + * come from the same enumeration, so they cannot drift. + * + * Registered on the MCP surface only (tool.registry): the chat lane is the + * persona-append tool (#1466) and the REST Tool API deliberately does not + * carry this. Access is decided by the capability matrix + * (`CANONICAL_MCP_TOOL_ACCESS_RULES`); the handler enforces the rest — + * participant admission here, recipient-side scoping in the host (a question + * parked on the counterparty never enumerates for this caller). + */ + +const logger = protocolLogger('McpTools:NegotiationAnswer'); + +const SCOPE_DENIAL = 'Access denied: this negotiation is not in your bound network scope.'; + +/** Registers the MCP-surface `answer_pending_question` tool. */ +export function createNegotiationAnswerTools(defineTool: DefineTool, deps: NegotiationToolDeps) { + const { negotiationDatabase } = deps; + + const answerPendingQuestion = defineTool({ + name: 'answer_pending_question', + description: + 'Route the user\'s answer to the open question a PARKED negotiation is waiting on, resuming it. This is the ONLY thing ' + + 'that resumes a parked negotiation — responding, editing the signal, or updating the opportunity does not.\n\n' + + '**When to use:** `list_negotiations` / `get_negotiation` showed a `park` with `waitingOn: "you"` and a `question` number, ' + + 'and the user has answered that question. Pass the negotiation\'s id, the exact `park.question` number, and the user\'s answer ' + + 'in their own words. Never invent an answer, and never add a preference or constraint the user did not state.\n\n' + + '**Numbering:** the `question` number and the number shown in the park annotation come from the same open-question record, ' + + 'so pass the number exactly as shown. If it no longer names an open question you will get `unknown_question` with the current count — ' + + 're-read the listing rather than guessing.\n\n' + + '**One answer, every negotiation it unparks:** a question can unblock several parked negotiations; the host resumes them all from one answer.', + querySchema: z.object({ + negotiationId: z.string() + .describe('The parked negotiation\'s task id, from list_negotiations or get_negotiation.'), + question: z.number().int().min(1) + .describe('Which open question is being answered — the `park.question` number exactly as the listing or detail showed it.'), + answer: z.string().min(1).max(4000) + .describe('The user\'s answer, in their own words, restated only enough to stand alone.'), + }), + handler: async ({ context, query }) => { + try { + const host = deps.negotiatorAnswerTools; + if (!host) { + return error('The answer lane is not available on this deployment. Tell the user honestly that the answer was not routed.'); + } + + const task = await negotiationDatabase.getTask(query.negotiationId); + if (!task) { + return error('Negotiation not found.'); + } + const meta = task.metadata as { + type?: string; + sourceUserId?: string; + candidateUserId?: string; + opportunityId?: string; + networkId?: string; + turnContext?: { indexContext?: { networkId?: string } }; + } | null; + if (meta?.type !== 'negotiation') { + return error('Negotiation not found.'); + } + + // Network-scope check (mirrors get_negotiation): a network-bound agent + // must not act on negotiations outside its bound network. Run before + // the participant check so no existence-vs-membership signal leaks. + const scopedNetworkId = focusedNetworkId(context); + if (scopedNetworkId) { + const metaNetworkId = typeof meta.networkId === 'string' && meta.networkId.trim() + ? meta.networkId + : meta.turnContext?.indexContext?.networkId ?? null; + if (metaNetworkId !== scopedNetworkId) { + return error(SCOPE_DENIAL); + } + } + + const isParty = meta.sourceUserId === context.userId || meta.candidateUserId === context.userId; + if (!isParty) { + return error('Access denied: you are not a party to this negotiation.'); + } + + const opportunityId = typeof meta.opportunityId === 'string' ? meta.opportunityId.trim() : ''; + if (!opportunityId) { + return error('This negotiation carries no opportunity, so it has no question to answer.'); + } + + // The caller's own actor intent on this pairing — the signal whose + // open-question block the number indexes into. A caller who is not the + // question's recipient resolves no open question through the host and + // gets `no_open_question`/`unknown_question`, never a resume. + const intentIds = await negotiationDatabase.getIntentIdsForOpportunities([opportunityId], context.userId); + const intentId = intentIds[opportunityId]; + if (!intentId) { + return error('Could not resolve your signal for this negotiation, so the answer was not routed.'); + } + + const result = await host.answerOpenQuestion(context.userId, { + intentId, + question: query.question, + answer: query.answer, + }); + + switch (result.status) { + case 'routed': + return success({ + status: 'routed', + negotiationId: task.id, + question: result.label, + message: + 'The answer is on its way to the negotiation that was waiting on it. Confirm to the user in one short sentence ' + + 'what you took as their answer, and do not also change their signal on the strength of it.', + }); + case 'no_open_question': + return error( + 'Nothing is waiting on the user for this signal any more — the parked negotiations resolved or expired. ' + + 'Tell them that plainly rather than implying their answer was recorded.', + ); + case 'unknown_question': + return error( + `That number does not name an open question (${result.open} currently open). ` + + 'Re-read the park annotations from list_negotiations and call this again with the number shown there.', + ); + case 'error': + default: + return error('Could not route that answer. Tell the user honestly that it did not go through.'); + } + } catch (err) { + logger.error('Failed to route MCP pending-question answer', { err }); + return error('Could not route that answer. Tell the user honestly that it did not go through.'); + } + }, + }); + + return [answerPendingQuestion] as const; +} diff --git a/packages/protocol/src/negotiations/negotiation.detail-reader.ts b/packages/protocol/src/negotiations/negotiation.detail-reader.ts index a5bf58334..d79b44927 100644 --- a/packages/protocol/src/negotiations/negotiation.detail-reader.ts +++ b/packages/protocol/src/negotiations/negotiation.detail-reader.ts @@ -1,6 +1,9 @@ import type { NegotiationOpportunityLifecycle } from '../shared/interfaces/database.interface.js'; +import type { ListingOpenQuestion } from '../shared/interfaces/negotiation-listing-park.interface.js'; +import { classifyInflightPark, classifyPostStallPark } from './negotiation.answer-consumption.js'; import { expectedNegotiationSpeaker } from './negotiation.expected-speaker.js'; -import { buildLifecycleNarration } from './negotiation.lifecycle-narration.js'; +import { buildLifecycleNarration, parkLifecycleLabel } from './negotiation.lifecycle-narration.js'; +import type { NegotiationParkNarration } from './negotiation.lifecycle-narration.js'; import { allowedActionsFor, readProtocolVersion, resolveSeat } from './negotiation.protocol.js'; import type { SeedAssessment, UserNegotiationContext } from './negotiation.state.js'; @@ -34,6 +37,8 @@ export interface NegotiationDetailMessage { senderId: string; parts: unknown[]; createdAt: Date; + /** Originating negotiation task; used by post-stall park classification. */ + taskId?: string | null; } export interface NegotiationDetailArtifact { @@ -52,6 +57,22 @@ export interface AuthorizedNegotiationDetailReaderInput { opportunityIds: string[], ownerUserId: string, ) => Promise>; + /** + * The open question this negotiation's park is asking of the CALLER, + * resolved through the shared question record (#1472, one level down: the + * detail must say the park out of the same record the listing and + * `answer_pending_question` read, so the numbers cannot drift). Optional: + * without it the detail still says whether the negotiation is parked and on + * whose side, it just cannot name the question's number. + */ + readOpenQuestion?: (opportunityId: string) => Promise; + /** + * Live principal reachability for the caller. The persisted `turnContext` + * is a park-time snapshot; the reachability flag must never be served stale + * (the same re-stamp REST pickup applies). Optional: without it the + * persisted context is returned verbatim. + */ + resolvePrincipalUnreachable?: (userId: string) => Promise; } /** @@ -65,9 +86,39 @@ export async function readAuthorizedNegotiationDetail( const { task, metadata, callerUserId, callerRole } = input; const isSource = callerRole === 'source'; const counterpartyId = isSource ? metadata.candidateUserId : metadata.sourceUserId; - const negotiationContext = metadata.turnContext + const lifecycleOpportunityId = metadata.opportunityId?.trim() || undefined; + + // These independent reads must stay concurrent: message/artifact latency must + // not delay lifecycle evidence, and missing lifecycle evidence remains fail-open. + // The reachability read rides along; a failed read resolves to null and the + // persisted context is served verbatim rather than failing the detail. + const [messages, artifacts, opportunityLifecycles, livePrincipalUnreachable] = await Promise.all([ + input.readMessages(task.conversationId), + input.readArtifacts(task.id), + input.readLifecycleEvidence(lifecycleOpportunityId ? [lifecycleOpportunityId] : [], callerUserId), + input.resolvePrincipalUnreachable + ? input.resolvePrincipalUnreachable(callerUserId).catch(() => null) + : Promise.resolve(null), + ]); + + // Persisted turnContext, projected into the caller's perspective — with the + // reachability flag re-stamped from the live read, both directions: a task + // parked before reachability was persisted carries nothing, and a task whose + // principal became reachable since must not keep advertising the opposite. + // Absent a live read, the persisted flag stands verbatim. + const persistedOwnUser = metadata.turnContext + ? (isSource ? metadata.turnContext.sourceUser : metadata.turnContext.candidateUser) + : null; + const ownUser = persistedOwnUser === null || livePrincipalUnreachable === null + ? persistedOwnUser + : livePrincipalUnreachable + ? { ...persistedOwnUser, principalUnreachable: true } + : persistedOwnUser.principalUnreachable + ? { ...persistedOwnUser, principalUnreachable: false } + : persistedOwnUser; + const negotiationContext = metadata.turnContext && ownUser ? { - ownUser: isSource ? metadata.turnContext.sourceUser : metadata.turnContext.candidateUser, + ownUser, otherUser: isSource ? metadata.turnContext.candidateUser : metadata.turnContext.sourceUser, indexContext: metadata.turnContext.indexContext, seedAssessment: metadata.turnContext.seedAssessment, @@ -75,15 +126,33 @@ export async function readAuthorizedNegotiationDetail( ...(metadata.turnContext.discoveryQuery && { discoveryQuery: metadata.turnContext.discoveryQuery }), } : null; - const lifecycleOpportunityId = metadata.opportunityId?.trim() || undefined; - // These independent reads must stay concurrent: message/artifact latency must - // not delay lifecycle evidence, and missing lifecycle evidence remains fail-open. - const [messages, artifacts, opportunityLifecycles] = await Promise.all([ - input.readMessages(task.conversationId), - input.readArtifacts(task.id), - input.readLifecycleEvidence(lifecycleOpportunityId ? [lifecycleOpportunityId] : [], callerUserId), - ]); + // #1472, one level down: this is the tool the poller prompt says to call + // FIRST, so on a parked negotiation the detail must say the park — through + // the SAME canonical predicate the listing runs over the task and messages + // it already holds, with the question's number resolved through the shared + // question record. A park on the counterparty's side is narrated but never + // quoted; that question is not this caller's to read. + let park: NegotiationParkNarration | null = null; + if (lifecycleOpportunityId && task.state === 'input_required') { + const classification = classifyInflightPark( + // Callers pass the full runtime metadata object (the typed shape above is + // a projection of it), so the ask-user binding is present for the + // classifier even though this reader never names it. + { id: task.id, state: task.state, metadata: metadata as unknown as Record }, + { opportunityId: lifecycleOpportunityId, userId: callerUserId }, + ); + if (classification.kind === 'wrong_recipient') park = { waitingOn: 'counterparty', kind: 'mid_flight' }; + else if (classification.kind === 'inflight') park = { waitingOn: 'you', kind: 'mid_flight' }; + } else if (lifecycleOpportunityId && task.state === 'completed') { + const classification = classifyPostStallPark(task, messages, { userId: callerUserId }); + if (classification.kind === 'wrong_recipient') park = { waitingOn: 'counterparty', kind: 'post_stall' }; + else if (classification.kind === 'post_stall') park = { waitingOn: 'you', kind: 'post_stall' }; + } + if (park?.waitingOn === 'you' && lifecycleOpportunityId && input.readOpenQuestion) { + const openQuestion = await input.readOpenQuestion(lifecycleOpportunityId).catch(() => undefined); + if (openQuestion) park = { ...park, question: openQuestion.question, questionLabel: openQuestion.label }; + } // Sender IDs, rather than parity, determine continuation turns. The parity // fallback is retained for legacy rows that do not record a sender ID. @@ -93,6 +162,8 @@ export async function readAuthorizedNegotiationDetail( action?: string; assessment?: { reasoning?: string; suggestedRoles?: unknown }; message?: string; + askUser?: unknown; + checklist?: unknown; } | undefined; const turnNumber = index + 1; const speaker = message.senderId @@ -108,6 +179,12 @@ export async function readAuthorizedNegotiationDetail( reasoning: turnData?.assessment?.reasoning ?? null, suggestedRoles: turnData?.assessment?.suggestedRoles ?? null, message: turnData?.message ?? null, + // The persisted ask/checklist payloads, verbatim and only when present: + // an external seat cannot see the consult's dimensions or a checklist's + // `settles` declarations without them, and turns that never carried + // either keep their prior shape byte-for-byte. + ...(turnData?.askUser != null ? { askUser: turnData.askUser } : {}), + ...(turnData?.checklist != null ? { checklist: turnData.checklist } : {}), createdAt: message.createdAt, }; }); @@ -147,9 +224,13 @@ export async function readAuthorizedNegotiationDetail( turnsAdded: Math.max(0, turnCount - priorTurnCount), turns, outcome, + // Top-level and inside `lifecycle` both, mirroring the listing: a park is + // the first thing that must be true about a negotiation that holds one. + ...(park ? { park: { ...park, label: parkLifecycleLabel(park) } } : {}), lifecycle: buildLifecycleNarration( status, lifecycleOpportunityId ? opportunityLifecycles[lifecycleOpportunityId] : undefined, + park ?? undefined, ), context: negotiationContext, createdAt: task.createdAt, diff --git a/packages/protocol/src/negotiations/negotiation.module.ts b/packages/protocol/src/negotiations/negotiation.module.ts index a55fced6b..16987cbfa 100644 --- a/packages/protocol/src/negotiations/negotiation.module.ts +++ b/packages/protocol/src/negotiations/negotiation.module.ts @@ -4,6 +4,7 @@ * Anything outside this capability imports from here and nowhere else. */ export { AMBIENT_PARK_WINDOW_MS, createNegotiationTools } from "./negotiation.tools.js"; +export { createNegotiationAnswerTools } from "./negotiation.answer.tools.js"; export { buildLifecycleNarration, parkLifecycleLabel } from "./negotiation.lifecycle-narration.js"; export type { NegotiationLifecycleNarration, NegotiationParkNarration } from "./negotiation.lifecycle-narration.js"; export { buildFallbackDigest, NegotiationSummarizer } from "./negotiation.summarizer.js"; diff --git a/packages/protocol/src/negotiations/negotiation.tools.port.ts b/packages/protocol/src/negotiations/negotiation.tools.port.ts index 848cdb800..f4302318b 100644 --- a/packages/protocol/src/negotiations/negotiation.tools.port.ts +++ b/packages/protocol/src/negotiations/negotiation.tools.port.ts @@ -2,6 +2,7 @@ import type { NegotiationGraphDatabase } from "../shared/interfaces/database.int import type { AgentDispatcher } from "../shared/interfaces/agent-dispatcher.interface.js"; import type { NegotiationTimeoutQueue } from "../shared/interfaces/negotiation-events.interface.js"; import type { NegotiationListingParkHost } from "../shared/interfaces/negotiation-listing-park.interface.js"; +import type { NegotiatorAnswerToolsHost } from "../shared/interfaces/negotiator-answer.interface.js"; /** * Host capabilities consumed by the negotiation tool factory. @@ -23,4 +24,12 @@ export interface NegotiationToolDeps { * parked and on whose side, it just cannot name the question's number. */ negotiationListingPark?: NegotiationListingParkHost; + /** + * Host bridge behind the MCP-surface `answer_pending_question` tool — the + * same host, numbering, and consumption path the negotiator persona's chat + * tool uses (#1466). Optional: without it the tool reports an honest + * unavailability error rather than not registering, so the MCP tool + * inventory stays static. + */ + negotiatorAnswerTools?: NegotiatorAnswerToolsHost; } diff --git a/packages/protocol/src/negotiations/negotiation.tools.ts b/packages/protocol/src/negotiations/negotiation.tools.ts index aa0d84a00..6ae93d21e 100644 --- a/packages/protocol/src/negotiations/negotiation.tools.ts +++ b/packages/protocol/src/negotiations/negotiation.tools.ts @@ -255,11 +255,20 @@ export function createNegotiationTools(defineTool: DefineTool, deps: Negotiation '`pending` still awaits owner review. When the negotiation is parked, `lifecycle.lifecycleLabel` states the park (it supersedes the ' + 'status label) and `lifecycle.connectionState` is `parked_awaiting_your_answer` or `parked_awaiting_counterparty`. ' + '`directConversationEvidence` is `not_provided`, so this tool never establishes that an H2H message thread exists.\n\n' + + '**Answering a park:** when `park.waitingOn` is "you", relay the question to the user and route their answer with ' + + '`answer_pending_question` (negotiationId + the `park.question` number). Nothing else resumes a parked negotiation.\n\n' + + '**External turns:** a turn submitted through this surface (respond_to_negotiation) or the REST respond endpoint is NOT run ' + + 'through the negotiation graph\'s conclusion floor, decline law, or copy-loop guard — those protections apply only to graph-run turns. ' + + 'Do not describe external turns as protected by them.\n\n' + '**When to use:** To see ongoing and past negotiations, check which negotiations need attention, ' + 'or find a negotiation ID for get_negotiation or respond_to_negotiation.', querySchema: z.object({ - status: z.enum(['active', 'waiting_for_agent', 'completed', 'all']).optional() - .describe('Filter by negotiation status. Omit or use "all" to return all negotiations.'), + status: z.enum(['active', 'waiting_for_agent', 'input_required', 'completed', 'all']).optional() + .describe( + 'Filter by negotiation status. Omit or use "all" to return all negotiations. ' + + '"input_required" returns the negotiations parked mid-flight on a person\'s answer — read `park` for whose. ' + + 'A post-stall park lives on a "completed" negotiation and carries a `park` object there; it is not returned by this filter.', + ), scope: z.enum(['signal', 'all']).optional() .describe('Scope to the pinned signal (requires an intent-pinned session), or pass "all" for the full negotiation history.'), since: z.string().datetime().optional() @@ -285,6 +294,7 @@ export function createNegotiationTools(defineTool: DefineTool, deps: Negotiation // For 'active', query 'working' state tasks const dbState = stateFilter === 'active' ? 'working' : stateFilter === 'waiting_for_agent' ? 'waiting_for_agent' + : stateFilter === 'input_required' ? 'input_required' : stateFilter === 'completed' ? 'completed' : undefined; @@ -498,10 +508,19 @@ export function createNegotiationTools(defineTool: DefineTool, deps: Negotiation 'Negotiations are bilateral exchanges where two AI agents negotiate on behalf of users. Each turn contains an action ' + '(propose, accept, reject, counter, question), an assessment with reasoning and suggested roles, and an optional message.\n\n' + '**Access control:** You must be a party to the negotiation (source or candidate) to view it.\n\n' + - '**Statuses:** `active` — in progress. `waiting_for_agent` — waiting for an agent response or timeout. `completed` — the agents concluded, not that the owner accepted or a connection/message thread exists.\n\n' + + '**Statuses:** `active` — in progress. `waiting_for_agent` — waiting for an agent response or timeout. ' + + '`input_required` — PARKED on a person\'s answer; read `park` for whose. `completed` — the agents concluded, not that the owner accepted or a connection/message thread exists.\n\n' + + '**Parked negotiations:** A negotiation waiting on a person carries a `park` object (`waitingOn: "you" | "counterparty"`, and for the ' + + 'user\'s own side the open question\'s `question` number and `questionLabel`, from the SAME record `answer_pending_question` routes against). ' + + 'When the negotiation is parked, `lifecycle.lifecycleLabel` states the park (it supersedes the status label) — `park.waitingOn="you"` means the user has ' + + 'something to answer RIGHT NOW, whatever the opportunity status reads; route their answer with `answer_pending_question`. A `park` on the ' + + 'counterparty\'s side names no question content. Turns carrying an `ask_user` consult include the persisted `askUser` payload (question, dimension, answerhood) ' + + 'and the `checklist` the turn scored, so the consult\'s dimensions and `settles` declarations are visible.\n\n' + '**Lifecycle narration:** The additive `lifecycle` object is authoritative for user-facing wording. A turn action of `accept` is agent-side; ' + 'only `lifecycle.ownerAction=accepted` records this owner as the human acceptor. `conversationType=agent_negotiation` identifies the returned ' + 'conversationId as the A2A negotiation transcript; this result does not provide H2H conversation evidence.\n\n' + + '**External turns:** turns submitted through respond_to_negotiation or the REST respond endpoint are NOT run through the negotiation graph\'s ' + + 'conclusion floor, decline law, or copy-loop guard; do not imply those protections for them.\n\n' + '**When to use:** To review the full negotiation history before responding, to understand why the agents ' + 'accepted or rejected a potential match, or to see the current state of an active negotiation.\n\n' + '**Negotiation-turn-mode usage.** If you are running as a silent background subagent (dispatched by the ' + @@ -572,6 +591,30 @@ export function createNegotiationTools(defineTool: DefineTool, deps: Negotiation readArtifacts: (taskId) => negotiationDatabase.getArtifactsForTask(taskId), readLifecycleEvidence: (opportunityIds, ownerUserId) => readOpportunityLifecycles(negotiationDatabase, opportunityIds, ownerUserId), + // #1472, one level down: the open question's number and label come + // from the same host record the listing and `answer_pending_question` + // read — resolved through the caller's own actor intent, so a park on + // the counterparty's side can never be quoted here. + readOpenQuestion: async (opportunityId) => { + const parkHost = deps.negotiationListingPark; + if (!parkHost) return undefined; + try { + const intentIds = await negotiationDatabase.getIntentIdsForOpportunities([opportunityId], context.userId); + const intentId = intentIds[opportunityId]; + if (!intentId) return undefined; + const openQuestions = await parkHost.readOpenQuestions(context.userId, intentId); + return openQuestions.find((question) => question.opportunityId === opportunityId); + } catch (openErr) { + logger.warn('Failed to read open question for negotiation detail', { opportunityId, err: openErr }); + return undefined; + } + }, + // The persisted turnContext is a park-time snapshot; reachability is + // re-stamped from the live read the way REST pickup does, so the + // flag is never served stale in either direction. + ...(negotiationDatabase.isPrincipalUnreachable + ? { resolvePrincipalUnreachable: (userId: string) => negotiationDatabase.isPrincipalUnreachable!(userId) } + : {}), }); return success(detail); } catch (err) { @@ -600,6 +643,10 @@ export function createNegotiationTools(defineTool: DefineTool, deps: Negotiation '- `question` — Ask the other side a clarifying question (message is required). The negotiation continues.\n\n' + '**What happens after:** Terminal actions (accept/reject/withdraw/decline) finalize the negotiation immediately. ' + 'Counter/question continues — if the counterparty has an agent, the negotiation yields again; otherwise the AI agent responds inline.\n\n' + + '**What this surface does NOT run:** turns submitted here are NOT run through the negotiation graph\'s conclusion floor ' + + '(the checklist gate an in-graph accept must clear), the decline law, or the copy-loop guard. Those protections apply only to ' + + 'graph-run turns; a turn submitted here is persisted as given. Do not tell the user those checks were applied, and do not submit ' + + 'ask_user/checklist payloads through this tool — it does not carry them.\n\n' + '**Silent-subagent response contract.** In negotiation-turn mode, submit exactly ONE call to this tool ' + 'per dispatch with an action from your seat\'s allowed set and the assessment (reasoning + suggestedRoles). ' + 'If the decision is ambiguous, pick the most conservative action — usually `counter` with specific objections. ' + diff --git a/packages/protocol/src/negotiations/tests/negotiation.answer.tools.spec.ts b/packages/protocol/src/negotiations/tests/negotiation.answer.tools.spec.ts new file mode 100644 index 000000000..84b2d44e9 --- /dev/null +++ b/packages/protocol/src/negotiations/tests/negotiation.answer.tools.spec.ts @@ -0,0 +1,267 @@ +/** + * The MCP answer lane (`answer_pending_question`). + * + * The listing says "open question 3, 'Timing'" to a client that could do + * nothing about it. These specs pin the lane that closes the gap: the tool + * resolves scope from the negotiation the client is looking at, hands the + * SHOWN number to the same host record the listing's numbers come from + * (anti-divergence, the #1470 pattern: one fixture, both surfaces), and + * refuses — before the host — anyone who is not a party. + */ +import { describe, expect, test } from "bun:test"; +import { z } from "zod"; +import { createNegotiationAnswerTools } from "../negotiation.answer.tools.js"; +import { createNegotiationTools } from "../negotiation.tools.js"; +import { negotiationQuestionSettlementId } from "../negotiation.question-safety.js"; +import type { ToolDeps, ResolvedToolContext } from "../../shared/agent/tool.helpers.js"; +import type { NegotiationToolDeps } from "../negotiation.tools.port.js"; + +const VIEWER = "user-src"; +const COUNTERPARTY = "user-cand"; +const OUTSIDER = "user-else"; +const INTENT_ID = "intent-1"; +const OPPORTUNITY_ID = "opp-1"; +const TASK_ID = "task-1"; + +function makeContext(userId = VIEWER, opts: { scopeNetworkId?: string; pinIntent?: boolean } = {}): ResolvedToolContext { + return { + userId, + user: { id: userId, name: "Alice", email: "a@test" }, + userProfile: null, + userNetworks: [], + isMcp: true, + ...(opts.scopeNetworkId ? { scopeType: "network" as const, scopeId: opts.scopeNetworkId } : {}), + ...(opts.pinIntent ? { scopeType: "intent" as const, scopeId: INTENT_ID } : {}), + } as unknown as ResolvedToolContext; +} + +function captureTool(name: string, register: (defineTool: never) => void) { + let captured: { handler: (i: { context: ResolvedToolContext; query: unknown }) => Promise; querySchema: z.ZodType } | undefined; + const defineTool = (def: { name: string; handler: unknown; querySchema: z.ZodType }) => { + if (def.name === name) captured = def as typeof captured; + return def; + }; + register(defineTool as never); + return captured!; +} + +function parkedTask(recipientUserId = VIEWER) { + return { + id: TASK_ID, + conversationId: "conv-1", + state: "input_required", + metadata: { + type: "negotiation", + sourceUserId: VIEWER, + candidateUserId: COUNTERPARTY, + opportunityId: OPPORTUNITY_ID, + networkId: "network-1", + turnContext: { + askUserBinding: { + settlementId: negotiationQuestionSettlementId(TASK_ID), + recipientUserId, + recipientIntentId: INTENT_ID, + networkId: "network-1", + opportunityId: OPPORTUNITY_ID, + }, + }, + }, + createdAt: new Date("2026-08-20T18:00:00Z"), + updatedAt: new Date("2026-08-20T20:04:00Z"), + }; +} + +function askUserMessage(recipientUserId = VIEWER) { + return { + senderId: `agent:${recipientUserId}`, + taskId: TASK_ID, + parts: [{ kind: "data", data: { action: "ask_user", assessment: { reasoning: "Negotiation parked pending the client's answer." }, message: null } }], + }; +} + +/** + * ONE question record for both surfaces (the #1470 pattern): the listing's + * park host and the answer host both read this enumeration, so the number the + * listing prints must be the number that routes the answer. + */ +function sharedQuestionRecord(question = 3, label = "Timing") { + const record = [{ opportunityId: OPPORTUNITY_ID, question, label }]; + const hostCalls: Array<{ userId: string; intentId: string; question: number; answer: string }> = []; + return { + record, + hostCalls, + listingParkHost: { readOpenQuestions: async () => record }, + answerHost: { + answerOpenQuestion: async (userId: string, input: { intentId: string; question: number; answer: string }) => { + hostCalls.push({ userId, ...input }); + const match = record.find((entry) => entry.question === input.question); + return match + ? { status: "routed" as const, label: match.label } + : { status: "unknown_question" as const, open: record.length }; + }, + }, + }; +} + +function makeDeps(overrides: { + shared?: ReturnType; + withHost?: boolean; + task?: Record | null; + intentIds?: Record; +} = {}) { + const shared = overrides.shared ?? sharedQuestionRecord(); + const task = overrides.task === undefined ? parkedTask() : overrides.task; + const deps = { + negotiationDatabase: { + getTask: async () => task, + getTasksForUser: async () => (task ? [task] : []), + getNegotiationMessages: async () => [askUserMessage()], + getMessagesForConversation: async () => [askUserMessage()], + getIntentIdsForOpportunities: async () => overrides.intentIds ?? { [OPPORTUNITY_ID]: INTENT_ID }, + getOpportunityLifecyclesForNegotiations: async () => ({ + [OPPORTUNITY_ID]: { status: "negotiating", acceptedByOwner: false }, + }), + }, + negotiationListingPark: shared.listingParkHost, + ...(overrides.withHost === false ? {} : { negotiatorAnswerTools: shared.answerHost }), + }; + return { deps, shared }; +} + +async function invoke(deps: Record, context: ResolvedToolContext, query: Record) { + const tool = captureTool("answer_pending_question", (dt) => + createNegotiationAnswerTools(dt, deps as unknown as NegotiationToolDeps)); + return JSON.parse(await tool.handler({ context, query })); +} + +describe("answer_pending_question (MCP)", () => { + test("the number the listing shows is the number that routes — one record, both surfaces", async () => { + const { deps, shared } = makeDeps(); + + // Surface 1: the listing annotates the park with the record's number. + const listing = captureTool("list_negotiations", (dt) => + createNegotiationTools(dt, deps as unknown as ToolDeps)); + const listed = JSON.parse(await listing.handler({ context: makeContext(VIEWER, { pinIntent: true }), query: {} })); + expect(listed.success).toBe(true); + const shownNumber = listed.data.negotiations[0].park.question; + expect(shownNumber).toBe(3); + + // Surface 2: answering with exactly the shown number routes to the record. + const result = await invoke(deps, makeContext(VIEWER), { + negotiationId: TASK_ID, + question: shownNumber, + answer: "This week works.", + }); + expect(result.success).toBe(true); + expect(result.data.status).toBe("routed"); + expect(result.data.question).toBe("Timing"); + expect(shared.hostCalls).toEqual([{ + userId: VIEWER, + intentId: INTENT_ID, + question: 3, + answer: "This week works.", + }]); + }); + + test("resolves the intent from the caller's own actor on the negotiation's opportunity", async () => { + const { deps, shared } = makeDeps(); + const result = await invoke(deps, makeContext(VIEWER), { negotiationId: TASK_ID, question: 3, answer: "yes" }); + expect(result.success).toBe(true); + expect(shared.hostCalls[0]!.intentId).toBe(INTENT_ID); + }); + + test("a non-party is refused before the host runs", async () => { + const { deps, shared } = makeDeps(); + const result = await invoke(deps, makeContext(OUTSIDER), { negotiationId: TASK_ID, question: 3, answer: "yes" }); + expect(result.success).toBe(false); + expect(result.error).toContain("not a party"); + expect(shared.hostCalls).toEqual([]); + }); + + test("a network-bound agent cannot reach a negotiation outside its scope", async () => { + const { deps, shared } = makeDeps(); + const result = await invoke(deps, makeContext(VIEWER, { scopeNetworkId: "network-other" }), { + negotiationId: TASK_ID, question: 3, answer: "yes", + }); + expect(result.success).toBe(false); + expect(result.error).toContain("not in your bound network scope"); + expect(shared.hostCalls).toEqual([]); + }); + + test("an unknown number is an honest re-read instruction, never a resume", async () => { + const { deps, shared } = makeDeps(); + const result = await invoke(deps, makeContext(VIEWER), { negotiationId: TASK_ID, question: 9, answer: "yes" }); + expect(result.success).toBe(false); + expect(result.error).toContain("does not name an open question"); + expect(result.error).toContain("1 currently open"); + expect(shared.hostCalls.length).toBe(1); + }); + + test("a missing host is an honest unavailability error", async () => { + const { deps } = makeDeps({ withHost: false }); + const result = await invoke(deps, makeContext(VIEWER), { negotiationId: TASK_ID, question: 3, answer: "yes" }); + expect(result.success).toBe(false); + expect(result.error).toContain("not available"); + }); + + test("an unresolvable caller intent refuses rather than routing blind", async () => { + const { deps, shared } = makeDeps({ intentIds: { [OPPORTUNITY_ID]: null } }); + const result = await invoke(deps, makeContext(VIEWER), { negotiationId: TASK_ID, question: 3, answer: "yes" }); + expect(result.success).toBe(false); + expect(result.error).toContain("Could not resolve your signal"); + expect(shared.hostCalls).toEqual([]); + }); +}); + +describe("list_negotiations — the input_required filter", () => { + function captureListing(deps: Record) { + return captureTool("list_negotiations", (dt) => createNegotiationTools(dt, deps as unknown as ToolDeps)); + } + + test("input_required maps to the parked task state", async () => { + const seenStates: Array = []; + const { deps } = makeDeps(); + const spiedDeps = { + ...deps, + negotiationDatabase: { + ...deps.negotiationDatabase, + getTasksForUser: async (_userId: string, options?: { state?: string }) => { + seenStates.push(options?.state); + return [parkedTask()]; + }, + }, + }; + const listing = captureListing(spiedDeps); + const result = JSON.parse(await listing.handler({ + context: makeContext(VIEWER, { pinIntent: true }), + query: { status: "input_required" }, + })); + expect(result.success).toBe(true); + expect(seenStates).toEqual(["input_required"]); + expect(result.data.negotiations.length).toBe(1); + expect(result.data.negotiations[0].status).toBe("input_required"); + expect(result.data.negotiations[0].park).toMatchObject({ waitingOn: "you", question: 3 }); + }); + + test("active still maps to working, untouched", async () => { + const seenStates: Array = []; + const { deps } = makeDeps(); + const spiedDeps = { + ...deps, + negotiationDatabase: { + ...deps.negotiationDatabase, + getTasksForUser: async (_userId: string, options?: { state?: string }) => { + seenStates.push(options?.state); + return []; + }, + }, + }; + const listing = captureListing(spiedDeps); + const result = JSON.parse(await listing.handler({ + context: makeContext(VIEWER, { pinIntent: true }), + query: { status: "active" }, + })); + expect(result.success).toBe(true); + expect(seenStates).toEqual(["working"]); + }); +}); diff --git a/packages/protocol/src/negotiations/tests/negotiation.tools.detail-park.spec.ts b/packages/protocol/src/negotiations/tests/negotiation.tools.detail-park.spec.ts new file mode 100644 index 000000000..8d7a603fd --- /dev/null +++ b/packages/protocol/src/negotiations/tests/negotiation.tools.detail-park.spec.ts @@ -0,0 +1,349 @@ +/** + * `get_negotiation` and the park (#1472, one level down). + * + * The listing learned to say the park in 23.5.1; the DETAIL — the tool the + * poller prompt says to call FIRST — still narrated a lifecycle built without + * it: "input_required" status beside "the agents are still negotiating". These + * specs pin four things: + * + * - a park on the caller is named, with the number the shared question record + * assigned; + * - a park on the counterparty is named without its content; + * - the persisted `askUser`/`checklist` payloads are projected on the turns + * that carry them (an external seat cannot see dimensions or `settles` + * without them), while plain turns keep their prior shape; + * - `ownUser.principalUnreachable` is re-stamped from the live read, both + * directions, and a non-parked negotiation renders byte-for-byte as before. + */ +import { describe, expect, test } from "bun:test"; +import { z } from "zod"; +import { createNegotiationTools } from "../negotiation.tools.js"; +import { negotiationQuestionSettlementId } from "../negotiation.question-safety.js"; +import { NEGOTIATION_PARK_REASONING } from "../negotiation.stall-gap.js"; +import type { ToolDeps, ResolvedToolContext } from "../../shared/agent/tool.helpers.js"; + +const VIEWER = "user-src"; +const COUNTERPARTY = "user-cand"; +const INTENT_ID = "intent-1"; +const OPPORTUNITY_ID = "opp-1"; +const TASK_ID = "task-1"; + +function makeContext(userId = VIEWER): ResolvedToolContext { + return { + userId, + user: { id: userId, name: "Alice", email: "a@test" }, + userProfile: null, + userNetworks: [], + isMcp: true, + } as unknown as ResolvedToolContext; +} + +function captureDetail(deps: Record) { + let captured: { handler: (i: { context: ResolvedToolContext; query: unknown }) => Promise; querySchema?: z.ZodType } | undefined; + const defineTool = (def: { name: string; handler: unknown; querySchema?: z.ZodType }) => { + if (def.name === "get_negotiation") captured = def as typeof captured; + return def; + }; + createNegotiationTools(defineTool as never, deps as unknown as ToolDeps); + return captured!; +} + +const ASK_USER_PAYLOAD = { + reason: "missing_fact", + dimension: "Timing", + answerhood: { ok_when: "a week is named", conflict_when: "no availability this quarter" }, + question: { + title: "Timing", + prompt: "When does this need to happen?", + options: [ + { label: "This week", description: "" }, + { label: "This month", description: "" }, + ], + }, +}; + +const CHECKLIST = [ + { name: "Timing", kind: "hard_constraint", status: "unknown", settles: "a concrete week both sides can commit to" }, +]; + +function parkedTask(recipientUserId: string, overrides: { state?: string; turnContextExtra?: Record } = {}) { + return { + id: TASK_ID, + conversationId: "conv-1", + state: overrides.state ?? "input_required", + metadata: { + type: "negotiation", + sourceUserId: VIEWER, + candidateUserId: COUNTERPARTY, + protocolVersion: "v2", + maxTurns: 6, + opportunityId: OPPORTUNITY_ID, + turnContext: { + sourceUser: { id: VIEWER, intents: [], profile: {} }, + candidateUser: { id: COUNTERPARTY, intents: [], profile: {} }, + indexContext: { networkId: "network-1" }, + seedAssessment: { reasoning: "seed", valencyRole: "peer" }, + askUserBinding: { + settlementId: negotiationQuestionSettlementId(TASK_ID), + recipientUserId, + recipientIntentId: INTENT_ID, + networkId: "network-1", + opportunityId: OPPORTUNITY_ID, + }, + ...(overrides.turnContextExtra ?? {}), + }, + }, + createdAt: new Date("2026-08-20T18:00:00Z"), + updatedAt: new Date("2026-08-20T20:04:00Z"), + }; +} + +function askUserMessage(senderUserId: string) { + return { + id: "msg-2", + senderId: `agent:${senderUserId}`, + role: "agent" as const, + taskId: TASK_ID, + parts: [{ + kind: "data", + data: { + action: "ask_user", + assessment: { reasoning: NEGOTIATION_PARK_REASONING, suggestedRoles: { ownUser: "peer", otherUser: "peer" } }, + message: null, + askUser: ASK_USER_PAYLOAD, + checklist: CHECKLIST, + }, + }], + createdAt: new Date("2026-08-20T20:04:00Z"), + }; +} + +function plainMessage(senderUserId: string) { + return { + id: "msg-1", + senderId: `agent:${senderUserId}`, + role: "agent" as const, + taskId: TASK_ID, + parts: [{ kind: "data", data: { action: "outreach", assessment: { reasoning: "why", suggestedRoles: { ownUser: "peer", otherUser: "peer" } }, message: "hello" } }], + createdAt: new Date("2026-08-20T19:00:00Z"), + }; +} + +function incidentDeps(overrides: { + recipientUserId?: string; + task?: Record; + messages?: Array>; + openQuestions?: Array<{ opportunityId: string; question: number; label: string }>; + withHost?: boolean; + isPrincipalUnreachable?: (userId: string) => Promise; + lifecycleStatus?: string; +} = {}) { + const recipientUserId = overrides.recipientUserId ?? VIEWER; + const task = overrides.task ?? parkedTask(recipientUserId); + const messages = overrides.messages ?? [plainMessage(VIEWER), askUserMessage(recipientUserId)]; + return { + negotiationDatabase: { + getTask: async () => task, + getNegotiationMessages: async () => messages, + getMessagesForConversation: async () => messages, + getArtifactsForTask: async () => [], + getIntentIdsForOpportunities: async () => ({ [OPPORTUNITY_ID]: INTENT_ID }), + getOpportunityLifecyclesForNegotiations: async () => ({ + [OPPORTUNITY_ID]: { status: overrides.lifecycleStatus ?? "negotiating", acceptedByOwner: false }, + }), + ...(overrides.isPrincipalUnreachable ? { isPrincipalUnreachable: overrides.isPrincipalUnreachable } : {}), + }, + ...(overrides.withHost === false ? {} : { + negotiationListingPark: { + readOpenQuestions: async () => overrides.openQuestions + ?? [{ opportunityId: OPPORTUNITY_ID, question: 1, label: "Timing: This week" }], + }, + }), + }; +} + +async function getDetail(deps: Record, context = makeContext(VIEWER)) { + const tool = captureDetail(deps); + const result = JSON.parse(await tool.handler({ context, query: { negotiationId: TASK_ID } })); + expect(result.success).toBe(true); + return result.data; +} + +describe("get_negotiation — a park on the caller", () => { + test("names the open question with the number the shared record assigned", async () => { + const detail = await getDetail(incidentDeps()); + + expect(detail.status).toBe("input_required"); + expect(detail.park).toMatchObject({ + waitingOn: "you", + kind: "mid_flight", + question: 1, + questionLabel: "Timing: This week", + }); + expect(detail.park.label).toContain("open question 1"); + // The narration is built WITH the park: the sentence the incident produced + // ("still negotiating") can no longer be the lifecycle label. + expect(detail.lifecycle.connectionState).toBe("parked_awaiting_your_answer"); + expect(detail.lifecycle.lifecycleLabel).toContain("PARKED"); + expect(detail.lifecycle.lifecycleLabel).not.toContain("still negotiating"); + expect(detail.lifecycle.opportunityStatus).toBe("negotiating"); + }); + + test("carries whatever number the record assigned, never a fresh one", async () => { + const detail = await getDetail(incidentDeps({ + openQuestions: [{ opportunityId: OPPORTUNITY_ID, question: 3, label: "Budget" }], + })); + expect(detail.park.question).toBe(3); + expect(detail.park.label).toContain("open question 3"); + }); + + test("still says the park without the host, just without the number", async () => { + const detail = await getDetail(incidentDeps({ withHost: false })); + expect(detail.park).toMatchObject({ waitingOn: "you", kind: "mid_flight" }); + expect(detail.park.question).toBeUndefined(); + expect(detail.lifecycle.connectionState).toBe("parked_awaiting_your_answer"); + }); + + test("a post-stall park on the caller is a park too", async () => { + const detail = await getDetail(incidentDeps({ + task: parkedTask(VIEWER, { state: "completed" }), + lifecycleStatus: "stalled", + openQuestions: [{ opportunityId: OPPORTUNITY_ID, question: 2, label: "Rate" }], + })); + expect(detail.status).toBe("completed"); + expect(detail.park).toMatchObject({ waitingOn: "you", kind: "post_stall", question: 2, questionLabel: "Rate" }); + expect(detail.lifecycle.connectionState).toBe("parked_awaiting_your_answer"); + }); + + test("projects the persisted askUser and checklist on the turns that carry them", async () => { + const detail = await getDetail(incidentDeps()); + + const [plain, ask] = detail.turns; + expect("askUser" in plain).toBe(false); + expect("checklist" in plain).toBe(false); + // The persisted shape, minus nothing: dimension, answerhood, question and + // the checklist's `settles` are all visible to the external seat. + expect(ask.askUser).toEqual(ASK_USER_PAYLOAD); + expect(ask.checklist).toEqual(CHECKLIST); + expect(ask.checklist[0].settles).toBe("a concrete week both sides can commit to"); + }); +}); + +describe("get_negotiation — a park on the counterparty", () => { + test("is named, and its content is not", async () => { + const detail = await getDetail(incidentDeps({ + recipientUserId: COUNTERPARTY, + openQuestions: [{ opportunityId: OPPORTUNITY_ID, question: 1, label: "Their private ask" }], + })); + + expect(detail.park).toMatchObject({ waitingOn: "counterparty", kind: "mid_flight" }); + expect(detail.park.question).toBeUndefined(); + expect(detail.park.questionLabel).toBeUndefined(); + expect(detail.park.label).toContain("counterparty"); + expect(detail.lifecycle.connectionState).toBe("parked_awaiting_counterparty"); + }); +}); + +describe("get_negotiation — principal reachability is re-stamped live", () => { + test("a stale persisted `false` is corrected to unreachable", async () => { + const detail = await getDetail(incidentDeps({ + isPrincipalUnreachable: async () => true, + })); + expect(detail.context.ownUser.principalUnreachable).toBe(true); + }); + + test("a stale persisted `true` is corrected to reachable", async () => { + const detail = await getDetail(incidentDeps({ + task: parkedTask(VIEWER, { + turnContextExtra: { + sourceUser: { id: VIEWER, intents: [], profile: {}, principalUnreachable: true }, + }, + }), + isPrincipalUnreachable: async () => false, + })); + expect(detail.context.ownUser.principalUnreachable).toBe(false); + }); + + test("without a live read the persisted flag stands verbatim", async () => { + const detail = await getDetail(incidentDeps({ + task: parkedTask(VIEWER, { + turnContextExtra: { + sourceUser: { id: VIEWER, intents: [], profile: {}, principalUnreachable: true }, + }, + }), + })); + expect(detail.context.ownUser.principalUnreachable).toBe(true); + }); +}); + +describe("get_negotiation — no park", () => { + test("renders exactly as it did before the park projection existed", async () => { + const task = { + id: TASK_ID, + conversationId: "conv-1", + state: "working", + metadata: { + type: "negotiation", + sourceUserId: VIEWER, + candidateUserId: COUNTERPARTY, + maxTurns: 6, + opportunityId: OPPORTUNITY_ID, + }, + createdAt: new Date("2026-08-20T18:00:00Z"), + updatedAt: new Date("2026-08-20T20:04:00Z"), + }; + const message = { + id: "msg-1", + senderId: `agent:${VIEWER}`, + role: "agent" as const, + taskId: TASK_ID, + parts: [{ kind: "data", data: { action: "propose", assessment: { reasoning: "why", suggestedRoles: { ownUser: "peer", otherUser: "peer" } }, message: "hello" } }], + createdAt: new Date("2026-08-20T19:00:00Z"), + }; + const detail = await getDetail(incidentDeps({ task, messages: [message], openQuestions: [] })); + + // The fixture pin: field-for-field what this tool returned before the + // park projection. `park` is absent, turns carry no askUser/checklist + // keys, and the lifecycle narrates from opportunity status alone. + expect(detail).toEqual({ + id: TASK_ID, + conversationId: "conv-1", + conversationType: "agent_negotiation", + status: "active", + role: "source", + seat: "initiator", + protocolVersion: "v1", + allowedActions: ["propose", "accept", "reject", "counter", "question"], + counterpartyId: COUNTERPARTY, + turnCount: 1, + isUsersTurn: false, + isContinuation: false, + priorTurnCount: 0, + turnsAdded: 1, + turns: [{ + turnNumber: 1, + speaker: "source", + senderId: `agent:${VIEWER}`, + action: "propose", + actionActor: "agent", + reasoning: "why", + suggestedRoles: { ownUser: "peer", otherUser: "peer" }, + message: "hello", + createdAt: message.createdAt.toISOString(), + }], + outcome: null, + lifecycle: { + agentNegotiation: "in_progress", + opportunityStatus: "negotiating", + connectionState: "agents_negotiating", + ownerAction: "not_recorded", + directConversationEvidence: "not_provided", + lifecycleLabel: "The agents are still negotiating; no owner decision is recorded.", + }, + context: null, + createdAt: task.createdAt.toISOString(), + updatedAt: task.updatedAt.toISOString(), + }); + expect("park" in detail).toBe(false); + }); +}); diff --git a/packages/protocol/src/opportunities/opportunity.module.ts b/packages/protocol/src/opportunities/opportunity.module.ts index ae59e62de..1777f2cee 100644 --- a/packages/protocol/src/opportunities/opportunity.module.ts +++ b/packages/protocol/src/opportunities/opportunity.module.ts @@ -70,6 +70,9 @@ export type { export { createOpportunityTools, } from "./opportunity.tools.js"; +export { + createOpportunityVerdictTools, +} from "./opportunity.verdict.tools.js"; export { DISCOVERY_EVALUATOR_MIN_SCORE_DEFAULT, DISCOVERY_MIN_SIMILARITY_DEFAULT, diff --git a/packages/protocol/src/opportunities/opportunity.verdict.tools.ts b/packages/protocol/src/opportunities/opportunity.verdict.tools.ts new file mode 100644 index 000000000..10f8d8615 --- /dev/null +++ b/packages/protocol/src/opportunities/opportunity.verdict.tools.ts @@ -0,0 +1,138 @@ +import { z } from 'zod'; + +import type { DefineTool, ResolvedToolContext, ToolDeps } from '../shared/agent/tool.helpers.js'; +import { success, error } from '../shared/agent/tool.helpers.js'; +import { protocolLogger } from '../shared/observability/protocol.logger.js'; +import type { NegotiatorVerdictInput, NegotiatorVerdictResult } from '../shared/interfaces/negotiator-verdict.interface.js'; +import { ownerApprovalProvenanceFor } from './opportunity.owner-provenance.js'; + +/** + * Owner verdict tools on the MCP surface (`reject_opportunity` / + * `accept_opportunity`, #1471 one surface over). + * + * The owner's three decisions on a signal are ANSWER, EDIT, and VERDICT. On + * MCP, answer ships with `answer_pending_question` and edit is + * `update_intent`; the verdict had no lane — `update_opportunity` refuses a + * `negotiating` pairing outright and its IND-593 proof flow is built for + * agent-mediated writes. These tools reuse the negotiator verdict host + * verbatim: the same numbered-counterparty mapping, the same Radar + * Skip/Start-Chat service call underneath, the same outcome hooks (question + * retirement, DM resolution, contact memberships) in its wake. + * + * Admission is exactly the class IND-593 admits for a direct owner + * interaction: a SESSION-AUTHENTICATED principal, proven by the host-bound + * owner-approval provenance (`bindOwnerApprovalProvenance`, mcp.server.ts) — + * never a caller-supplied field. The capability matrix already hides these + * tools from every agent principal (`human_only`); this handler check is the + * fail-closed second layer, so a mis-listed surface still refuses. API-key + * agents are refused, deliberately: a verdict is the owner's own gate, and the + * boundary is the feature. + * + * Positions, never ids — the host resolves the number against the same + * oldest-first enumeration the DM prompt renders, and the executed result + * names WHO the write landed on. An unknown number returns the current + * numbered list rather than deciding anything. + */ + +const logger = protocolLogger('McpTools:OpportunityVerdict'); + +const VerdictQuerySchema = z.object({ + intentId: z.string() + .describe('The signal (intent id, from read_intents) whose counterparty the owner is deciding on.'), + counterparty: z.number().int().min(1) + .describe('Which counterparty, by 1-based position in the signal\'s actionable list (oldest pairing first). An out-of-range number returns the current numbered list without deciding anything.'), + reason: z.string().min(1).max(500).optional() + .describe('Why, in the owner\'s OWN words, if they gave a reason. For the record only — never invent or infer one.'), +}); + +/** One verdict's result copy, per host status — honest in the same way the chat lane is. */ +function describeVerdict(verdict: 'rejected' | 'accepted', result: NegotiatorVerdictResult): string { + switch (result.status) { + case 'executed': + return success({ + status: 'executed', + counterparty: result.counterparty, + message: verdict === 'rejected' + ? `Done — the ${result.counterparty} pairing is declined; they will not be contacted further about it. Confirm that to the user, naming ${result.counterparty}. Do NOT also edit their signal.` + : `Done — the ${result.counterparty} pairing is accepted on the user's behalf. That is one side of it: the connection is made when ${result.counterparty} accepts too, so say plainly that it now waits on them. Do NOT also edit their signal.`, + }); + case 'none_actionable': + return error('There is no counterparty left to decide on for this signal — they have concluded or expired. Tell the user that plainly rather than implying a decision was recorded.'); + case 'unknown_counterparty': + return error( + `That number does not name a counterparty of this signal; nothing was decided. The current list (${result.count}): ` + + result.actionable.map((label, index) => `${index + 1}. ${label}`).join('; ') + + '. Call again with the right number, or ask the user which of them they meant.', + ); + case 'already_decided': + return error(`The user has already acted on the ${result.counterparty} pairing, so nothing changed just now. Say so plainly — for an accept, it is ${result.counterparty}'s move next, not theirs.`); + case 'error': + default: + return error(`Could not record that ${verdict === 'rejected' ? 'rejection' : 'acceptance'}. Tell the user honestly that it did not go through, and do not describe the pairing as ${verdict}.`); + } +} + +/** Registers the MCP-surface owner verdict tools. */ +export function createOpportunityVerdictTools(defineTool: DefineTool, deps: ToolDeps) { + const run = async ( + verdict: 'rejected' | 'accepted', + toolName: string, + context: ResolvedToolContext, + query: z.infer, + execute: (userId: string, input: NegotiatorVerdictInput) => Promise, + ): Promise => { + try { + // Fail-closed owner boundary (IND-593 class): only the host-bound + // provenance of a session-authenticated principal admits a verdict. + // The capability matrix already refuses agent principals; this refusal + // stands even if the tool is ever listed on a surface without it. + const provenance = ownerApprovalProvenanceFor(context); + if (context.agentId || provenance?.sessionAuthenticated !== true) { + return error('Owner verdicts require the owner\'s own authenticated session. This principal cannot pass one.'); + } + const result = await execute(context.userId, { + intentId: query.intentId, + counterparty: query.counterparty, + ...(query.reason ? { reason: query.reason } : {}), + }); + return describeVerdict(verdict, result); + } catch (err) { + logger.error('MCP owner verdict failed', { toolName, err }); + return describeVerdict(verdict, { status: 'error' }); + } + }; + + const rejectOpportunity = defineTool({ + name: 'reject_opportunity', + description: + 'Decline one of a signal\'s counterparties, because the user (the signal\'s owner, in their own authenticated session) told you to. ' + + 'This is the ONLY tool that declines a live or parked pairing — `update_opportunity` refuses a `negotiating` one, and saying it does nothing. ' + + 'It executes the same owner reject the Radar card\'s Skip performs, so the pairing\'s open question retires with it.\n\n' + + '**Access:** owner session only. Agent principals cannot pass owner verdicts; this tool is absent from their inventory and refused if called.\n\n' + + '**Never on your own judgment** — only on the user\'s explicit instruction about a specific counterparty.', + querySchema: VerdictQuerySchema, + handler: async ({ context, query }) => { + const host = deps.negotiatorVerdictTools; + if (!host) return error('The verdict lane is not available on this deployment.'); + return run('rejected', 'reject_opportunity', context, query, (userId, input) => host.rejectOpportunity(userId, input)); + }, + }); + + const acceptOpportunity = defineTool({ + name: 'accept_opportunity', + description: + 'Accept one of a signal\'s counterparties on the user\'s (the owner\'s) explicit instruction, from their own authenticated session. ' + + 'This records the owner\'s acceptance — one side of a two-party decision: the connection is made only when the counterparty accepts too, ' + + 'so never say they are connected on the strength of this alone.\n\n' + + '**Access:** owner session only. Agent principals cannot pass owner verdicts; this tool is absent from their inventory and refused if called.\n\n' + + '**Never on your own judgment** — only on the user\'s explicit instruction about a specific counterparty.', + querySchema: VerdictQuerySchema, + handler: async ({ context, query }) => { + const host = deps.negotiatorVerdictTools; + if (!host) return error('The verdict lane is not available on this deployment.'); + return run('accepted', 'accept_opportunity', context, query, (userId, input) => host.acceptOpportunity(userId, input)); + }, + }); + + return [rejectOpportunity, acceptOpportunity] as const; +} diff --git a/packages/protocol/src/opportunities/tests/opportunity.verdict.tools.spec.ts b/packages/protocol/src/opportunities/tests/opportunity.verdict.tools.spec.ts new file mode 100644 index 000000000..b77ef3431 --- /dev/null +++ b/packages/protocol/src/opportunities/tests/opportunity.verdict.tools.spec.ts @@ -0,0 +1,148 @@ +/** + * Owner verdict tools on MCP (`reject_opportunity` / `accept_opportunity`). + * + * The boundary IS the feature: exactly the session-authenticated class the + * IND-593 owner-provenance binding admits may pass a verdict, and an API-key + * agent principal is refused in the handler even if the tool were ever + * mis-listed (the capability matrix already hides it — see + * mcp.authorization-policy.spec.ts). Execution goes through the injected + * verdict host, asserted against a stub. + */ +import { describe, expect, test } from "bun:test"; +import { z } from "zod"; +import { createOpportunityVerdictTools } from "../opportunity.verdict.tools.js"; +import { bindOwnerApprovalProvenance } from "../opportunity.owner-provenance.js"; +import type { ToolDeps, ResolvedToolContext } from "../../shared/agent/tool.helpers.js"; +import type { NegotiatorVerdictInput, NegotiatorVerdictResult } from "../../shared/interfaces/negotiator-verdict.interface.js"; + +const OWNER = "user-owner"; +const INTENT_ID = "intent-1"; + +function makeContext(opts: { sessionAuthenticated?: boolean; agentId?: string; bindProvenance?: boolean } = {}): ResolvedToolContext { + const context = { + userId: OWNER, + user: { id: OWNER, name: "Alice", email: "a@test" }, + userProfile: null, + userNetworks: [], + isMcp: true, + ...(opts.agentId ? { agentId: opts.agentId } : {}), + } as unknown as ResolvedToolContext; + if (opts.bindProvenance !== false) { + bindOwnerApprovalProvenance(context, { + surface: "mcp", + sessionAuthenticated: opts.sessionAuthenticated ?? true, + }); + } + return context; +} + +function makeHost(result: NegotiatorVerdictResult = { status: "executed", counterparty: "Basak" }) { + const calls: Array<{ verdict: "rejected" | "accepted"; userId: string; input: NegotiatorVerdictInput }> = []; + return { + calls, + host: { + rejectOpportunity: async (userId: string, input: NegotiatorVerdictInput) => { + calls.push({ verdict: "rejected", userId, input }); + return result; + }, + acceptOpportunity: async (userId: string, input: NegotiatorVerdictInput) => { + calls.push({ verdict: "accepted", userId, input }); + return result; + }, + }, + }; +} + +function captureTools(deps: Record) { + const captured = new Map Promise; querySchema: z.ZodType }>(); + const defineTool = (def: { name: string; handler: never; querySchema: z.ZodType }) => { + captured.set(def.name, def as never); + return def; + }; + createOpportunityVerdictTools(defineTool as never, deps as unknown as ToolDeps); + return captured; +} + +describe("MCP owner verdict tools", () => { + test("a session-authenticated owner's reject executes through the host and names who it landed on", async () => { + const { host, calls } = makeHost(); + const tools = captureTools({ negotiatorVerdictTools: host }); + const result = JSON.parse(await tools.get("reject_opportunity")!.handler({ + context: makeContext(), + query: { intentId: INTENT_ID, counterparty: 2, reason: "not this one" }, + })); + + expect(result.success).toBe(true); + expect(result.data.status).toBe("executed"); + expect(result.data.counterparty).toBe("Basak"); + expect(result.data.message).toContain("Basak"); + expect(calls).toEqual([{ + verdict: "rejected", + userId: OWNER, + input: { intentId: INTENT_ID, counterparty: 2, reason: "not this one" }, + }]); + }); + + test("a session-authenticated owner's accept is one side of two", async () => { + const { host, calls } = makeHost(); + const tools = captureTools({ negotiatorVerdictTools: host }); + const result = JSON.parse(await tools.get("accept_opportunity")!.handler({ + context: makeContext(), + query: { intentId: INTENT_ID, counterparty: 1 }, + })); + + expect(result.success).toBe(true); + expect(result.data.message).toContain("when Basak accepts too"); + expect(calls[0]!.verdict).toBe("accepted"); + expect(calls[0]!.input.reason).toBeUndefined(); + }); + + test("an API-key agent principal is refused before the host runs", async () => { + const { host, calls } = makeHost(); + const tools = captureTools({ negotiatorVerdictTools: host }); + const result = JSON.parse(await tools.get("reject_opportunity")!.handler({ + context: makeContext({ sessionAuthenticated: false, agentId: "agent-1" }), + query: { intentId: INTENT_ID, counterparty: 1 }, + })); + + expect(result.success).toBe(false); + expect(result.error).toContain("authenticated session"); + expect(calls).toEqual([]); + }); + + test("a context without host-bound provenance is refused — a caller cannot forge admission", async () => { + const { host, calls } = makeHost(); + const tools = captureTools({ negotiatorVerdictTools: host }); + const result = JSON.parse(await tools.get("accept_opportunity")!.handler({ + context: makeContext({ bindProvenance: false }), + query: { intentId: INTENT_ID, counterparty: 1 }, + })); + + expect(result.success).toBe(false); + expect(calls).toEqual([]); + }); + + test("an unknown counterparty re-lists instead of deciding", async () => { + const { host } = makeHost({ status: "unknown_counterparty", count: 2, actionable: ["Basak — waiting on your decision", "Deren — parked, waiting on you"] }); + const tools = captureTools({ negotiatorVerdictTools: host }); + const result = JSON.parse(await tools.get("reject_opportunity")!.handler({ + context: makeContext(), + query: { intentId: INTENT_ID, counterparty: 7 }, + })); + + expect(result.success).toBe(false); + expect(result.error).toContain("nothing was decided"); + expect(result.error).toContain("1. Basak — waiting on your decision"); + expect(result.error).toContain("2. Deren — parked, waiting on you"); + }); + + test("a missing host is an honest unavailability error", async () => { + const tools = captureTools({}); + const result = JSON.parse(await tools.get("reject_opportunity")!.handler({ + context: makeContext(), + query: { intentId: INTENT_ID, counterparty: 1 }, + })); + expect(result.success).toBe(false); + expect(result.error).toContain("not available"); + }); +}); diff --git a/packages/protocol/src/shared/agent/canonical-guidance.ts b/packages/protocol/src/shared/agent/canonical-guidance.ts index da3fe9bc4..2ff145d8c 100644 --- a/packages/protocol/src/shared/agent/canonical-guidance.ts +++ b/packages/protocol/src/shared/agent/canonical-guidance.ts @@ -49,7 +49,7 @@ Index Network is a private, intent-driven discovery protocol. Users express sign **Opportunities** — Discovered matches between users. Lifecycle: draft → pending → accepted/rejected/expired. -**Negotiations** — Agents coordinate, users approve. **A2A acceptance is not owner approval.** These are separate gates. +**Negotiations** — Agents coordinate, users approve. **A2A acceptance is not owner approval.** These are separate gates. A negotiation can PARK waiting on a principal's answer; the park names the open question, and answering it is the only thing that resumes the negotiation. **Workflows** — H2A (users express signals → agents discover) and A2A (agents coordinate) over MCP. Further escalation via native surfaces (human-to-human threads do not cross MCP). @@ -212,16 +212,27 @@ These gates are independent. Do not conflate them. ### Negotiation Workflow 1. Discovery creates draft opportunity 2. A2A coordination (agents evaluate viability) -3. A2A acceptance (agents agree to propose) -4. Opportunity sent to recipient (pending) -5. Owner review (human reads match reasoning) -6. Owner approval (human confirms) -7. Escalation (via native surfaces, not MCP) +3. Park (when needed): a negotiating agent needs a fact only its own principal can supply, so the negotiation PAUSES on that principal's answer +4. Answer: the principal answers the open question; the negotiation resumes (nothing else resumes it) +5. A2A acceptance (agents agree to propose) +6. Opportunity sent to recipient (pending) +7. Owner review (human reads match reasoning) +8. Owner approval (human confirms) +9. Escalation (via native surfaces, not MCP) + +### Parked Negotiations (Open Questions) +A negotiation can pause waiting on a person, in two shapes: a mid-flight consult (its task sits \`input_required\` on the recipient's side) or a post-stall park (the agents stalled and the recipient's agent left a question standing). While parked, the agents are NOT exchanging turns — and the opportunity legitimately still reads \`negotiating\`, so opportunity status alone never answers "is anything waiting on the user?". + +- \`list_negotiations\` and \`get_negotiation\` annotate a parked negotiation with \`park\`: \`waitingOn: "you" | "counterparty"\`, and for the user's own side the open question's \`question\` number and \`questionLabel\`. +- \`park.waitingOn = "you"\` means the user has something to answer RIGHT NOW; relay the question and route their answer with \`answer_pending_question\` (negotiationId + the shown question number). The number shown and the number the answer routes against come from the same record. +- A park on the counterparty's side names no question content; that question is not this user's to read. +- One answer can resume several negotiations parked on the same fact. ### Rules - Track A2A and owner approval separately - Never accept without explicit user approval - Always surface reasoning to owner +- A parked negotiation is waiting on a person, not negotiating — say so, and route the answer instead of editing the signal - Human-to-human messaging is not MCP`, workflows: `## Common Tool Workflows @@ -240,10 +251,11 @@ Two agents coordinate on behalf of users to identify, vet, and propose matches. 1. Approved signals for User A are evaluated in the background 2. Agent B vets match from User B side (A2A negotiation) -3. Agents reach agreement (A2A acceptance) -4. Both agents present to users with shared reasoning -5. Both users approve (owner approval required) -6. Escalation via native surfaces +3. The negotiation may PARK on a principal's answer (\`input_required\`); the principal answers via answer_pending_question and it resumes +4. Agents reach agreement (A2A acceptance) +5. Both agents present to users with shared reasoning +6. Both users approve (owner approval required) +7. Escalation via native surfaces ### MCP Scope The MCP protocol carries H2A and A2A workflows only. Escalation to direct messaging (web, Telegram, native surfaces) is outside MCP. diff --git a/packages/protocol/src/shared/agent/tool.helpers.ts b/packages/protocol/src/shared/agent/tool.helpers.ts index b30fdac09..79217461e 100644 --- a/packages/protocol/src/shared/agent/tool.helpers.ts +++ b/packages/protocol/src/shared/agent/tool.helpers.ts @@ -487,6 +487,19 @@ interface ToolDepsBindings { * whose side, it just cannot name the question's number. */ negotiationListingPark?: NegotiationListingParkHost; + /** + * Host bridge behind the MCP-surface `answer_pending_question` tool — the + * same host, numbering, and consumption path the negotiator persona's chat + * tool uses (#1466). Consumed only by the MCP tool registry surface. + */ + negotiatorAnswerTools?: NegotiatorAnswerToolsHost; + /** + * Host bridge behind the MCP-surface `reject_opportunity` / + * `accept_opportunity` owner-verdict tools (#1471, one surface over). + * Consumed only by the MCP tool registry surface, and only for + * session-authenticated owners (capability matrix + provenance re-check). + */ + negotiatorVerdictTools?: NegotiatorVerdictToolsHost; /** Chat session reader for exposing the caller's past conversations as MCP tools. */ chatSession?: ChatSessionReader; /** Read-through chat-session digest. Optional; consumers fall back to undefined `chatContext`. */ diff --git a/packages/protocol/src/shared/agent/tool.registry.ts b/packages/protocol/src/shared/agent/tool.registry.ts index 3eacd54f1..564dd83d1 100644 --- a/packages/protocol/src/shared/agent/tool.registry.ts +++ b/packages/protocol/src/shared/agent/tool.registry.ts @@ -5,12 +5,12 @@ import { error, redactSensitiveFields } from './tool.helpers.js'; import { createEnrichmentTools } from '../../enrichment/enrichment.tools.js'; import { Intents } from '../../intents/intent.module.js'; import { Networks } from '../../networks/network.module.js'; -import { createOpportunityTools } from '../../opportunities/opportunity.module.js'; +import { createOpportunityTools, createOpportunityVerdictTools } from '../../opportunities/opportunity.module.js'; import { createUtilityTools } from './utility.tools.js'; import type { ToolSurface } from './utility.tools.js'; import { createContactTools } from '../../contacts/contact.module.js'; import { createAgentTools } from '../../agents/agent.module.js'; -import { createNegotiationTools } from '../../negotiations/negotiation.module.js'; +import { createNegotiationAnswerTools, createNegotiationTools } from '../../negotiations/negotiation.module.js'; import { createChatTools } from '../../chat/chat.tools.js'; import { createPremiseTools } from '../../premises/premise.tools.js'; import type { OpportunityOwnerApprovalDeps } from '../../opportunities/opportunity.tools.port.js'; @@ -105,6 +105,16 @@ export function createToolRegistry(deps: ToolRegistryDeps, options: CreateToolRe } createAgentTools(dt, deps); createNegotiationTools(dt, deps); + // The MCP question flow: the answer lane for parked negotiations and the + // owner's verdict tools. MCP-only, deliberately — the chat lane is the + // negotiator persona's appended toolset, and the REST Tool API's API-key + // principals must never gain an owner-verdict lever (IND-593: the + // capability matrix admits verdicts for session humans only, and the + // verdict handler re-checks the host-bound provenance). + if (isMcpSurface) { + createNegotiationAnswerTools(dt, deps); + createOpportunityVerdictTools(dt, deps); + } createPremiseTools(dt, deps); if (deps.chatSession) { createChatTools(dt, deps); diff --git a/services/api/src/adapters/chat.database.adapter.ts b/services/api/src/adapters/chat.database.adapter.ts index 39781607c..f9266911b 100644 --- a/services/api/src/adapters/chat.database.adapter.ts +++ b/services/api/src/adapters/chat.database.adapter.ts @@ -1,6 +1,8 @@ import { readUserContext, readPremisesForUser, upsertIntentNetworkAssignment, schema, ActiveIntentRow, ArchiveResultShape, CreateIntentInput, CreateOpportunityInput, CreatedIntentRow, HydeDocumentRow, Id, NetworkMembershipEvents, NetworkMembershipRow, OnboardingState, OpportunityRow, SaveHydeDocumentInput, UpdateIntentInput, UserIdentity, activeIntentLifecycleWhere, activeOwnIntentsWhere, and, buildProfileFromUser, buildProfileWithIdFromUser, count, db, desc, ensurePersonalNetwork, eq, getPersonalIndexId, gt, gte, ilike, inArray, intentNetworks, intents, isNotNull, isNull, logger, networkMembers, networks, notInArray, or, persistProfileIdentityToUser, sql, traceAppOperation, userContexts, users } from './database.shared'; import { tasks } from '../schemas/conversation.schema'; +import { notArchivedNegotiationTaskWhere } from './negotiation-attempt.atomic'; +import { NEGOTIATION_PARK_REASONING } from './parked-negotiation.reader.adapter'; import { EnrichmentDatabaseAdapter } from './enrichment.database.adapter'; import { IntentDatabaseAdapter } from './intent.database.adapter'; @@ -105,6 +107,11 @@ export class ChatDatabaseAdapter { * negotiation totals) are narrowed to opportunities where the owner's * actor belongs to that community; own-signal and question aggregates are * meta-network and stay global. + * + * Pending question counts are read from the PARKED NEGOTIATIONS (the durable + * open-question record since the conversational-questions plan), never from + * the retired `questions` table's leftover pending rows. Answered counts + * still read the table — answered history legitimately lives there. * @returns Reproducible owner-scoped activity totals. */ async getAgentActivitySummary( @@ -144,7 +151,7 @@ export class ChatDatabaseAdapter { )`; const ownQuestion = sql`${schema.questions.actors}::jsonb @> ${JSON.stringify([{ userId }])}::jsonb`; - const [liveRows, surfacedRows, bySignalRows, pendingRows, answeredRows, negotiationRows] = await Promise.all([ + const [liveRows, surfacedRows, bySignalRows, inflightParkRows, postStallParkRows, answeredRows, negotiationRows] = await Promise.all([ db.select({ count: count() }) .from(schema.intents) .where(activeOwnIntentsWhere(userId)), @@ -170,17 +177,49 @@ export class ChatDatabaseAdapter { .where(eq(schema.intents.userId, userId)) .groupBy(schema.intents.id, schema.intents.summary, schema.intents.payload) .orderBy(desc(count(schema.opportunities.id))), - db.select({ - mode: sql`${schema.questions.detection}->>'mode'`, - count: count(), - }) - .from(schema.questions) + // Pending questions are the PARKED NEGOTIATIONS, not the retired + // `questions` table: since the conversational-questions plan, openness + // means "a negotiation is parked on this user's side", and the card rows + // are leftovers that must never make "does my owner have pending + // questions?" answer 0 beside a live park. Two shapes, mirroring the + // parked-negotiation reader set-wise: + // mid-flight consult — the exact task sits `input_required` with an + // ask-user binding naming this user as the recipient. + db.select({ count: count() }) + .from(tasks) .where(and( - eq(schema.questions.status, 'pending'), - ownQuestion, - or(isNull(schema.questions.expiresAt), gt(schema.questions.expiresAt, new Date())), - )) - .groupBy(sql`${schema.questions.detection}->>'mode'`), + eq(tasks.state, 'input_required'), + sql`${tasks.metadata}->>'type' = 'negotiation'`, + notArchivedNegotiationTaskWhere(), + sql`${tasks.metadata}->'turnContext'->'askUserBinding'->>'recipientUserId' = ${userId}`, + )), + // post-stall park — a stalled opportunity of this user whose + // negotiation's newest message is the authored ask_user gap from this + // user's own agent. Anything else on a stalled opportunity is a + // terminal stall, never an open question. + db.select({ count: count() }) + .from(schema.opportunities) + .where(and( + eq(schema.opportunities.status, 'stalled'), + ownActor, + sql`COALESCE(( + SELECT last_message.sender_id = ${`agent:${userId}`} + AND EXISTS ( + SELECT 1 + FROM jsonb_array_elements(last_message.parts) AS park_part + WHERE park_part->>'kind' = 'data' + AND park_part->'data'->>'action' = 'ask_user' + AND park_part->'data'->'assessment'->>'reasoning' = ${NEGOTIATION_PARK_REASONING} + ) + FROM ${schema.messages} AS last_message + JOIN ${tasks} AS park_task ON park_task.id = last_message.task_id + WHERE park_task.metadata->>'type' = 'negotiation' + AND park_task.metadata->>'opportunityId' = ${schema.opportunities.id} + AND park_task.metadata->>'archivedAt' IS NULL + ORDER BY last_message.created_at DESC, last_message.id DESC + LIMIT 1 + ), FALSE)`, + )), db.select({ mode: sql`${schema.questions.detection}->>'mode'`, count: count(), @@ -222,7 +261,18 @@ export class ChatDatabaseAdapter { title: row.title, count: Number(row.count), })), - pendingQuestionsByMode: toModeCounts(pendingRows), + // Keyed by the modes the projection already maps to the negotiations + // domain, so a mid-flight consult and a post-stall park inherit the + // exact permission gates the card-era rows carried. Retired-table + // pending rows deliberately contribute nothing. + pendingQuestionsByMode: { + ...(Number(inflightParkRows[0]?.count ?? 0) > 0 + ? { negotiation_inflight: Number(inflightParkRows[0]!.count) } + : {}), + ...(Number(postStallParkRows[0]?.count ?? 0) > 0 + ? { negotiation: Number(postStallParkRows[0]!.count) } + : {}), + }, answeredQuestionsByMode: toModeCounts(answeredRows), negotiationsStarted: Number(negotiationRows[0]?.started ?? 0), negotiationsCompleted: Number(negotiationRows[0]?.completed ?? 0), diff --git a/services/api/src/adapters/tests/agent-activity.database.adapter.isolated.ts b/services/api/src/adapters/tests/agent-activity.database.adapter.isolated.ts index c8a9a05fd..03dba8d79 100644 --- a/services/api/src/adapters/tests/agent-activity.database.adapter.isolated.ts +++ b/services/api/src/adapters/tests/agent-activity.database.adapter.isolated.ts @@ -4,7 +4,8 @@ import { v4 as uuidv4 } from 'uuid'; import db from '../../lib/drizzle/drizzle'; import { ChatDatabaseAdapter } from '../database.adapter'; -import { conversations, tasks } from '../../schemas/conversation.schema'; +import { conversations, messages, tasks } from '../../schemas/conversation.schema'; +import { NEGOTIATION_PARK_REASONING } from '../parked-negotiation.reader.adapter'; import { intents, opportunities, questions, users } from '../../schemas/database.schema'; const TEST_PREFIX = `agent_activity_${Date.now()}_`; @@ -23,6 +24,15 @@ const foreignNetworkOpportunityId = uuidv4(); const conversationId = uuidv4(); const questionIds = [uuidv4(), uuidv4(), uuidv4(), uuidv4(), uuidv4()]; const taskIds = [uuidv4(), uuidv4(), uuidv4(), uuidv4(), uuidv4()]; +// Parked-negotiation fixtures (pending counts read the parks, not the retired +// questions table). All stamped `old` so the negotiation started/completed +// windows never see them. +const inflightParkOpportunityId = uuidv4(); +const counterpartyParkOpportunityId = uuidv4(); +const postStallOpportunityId = uuidv4(); +const terminalStallOpportunityId = uuidv4(); +const parkTaskIds = [uuidv4(), uuidv4(), uuidv4(), uuidv4()]; +const parkMessageIds = [uuidv4(), uuidv4()]; const now = new Date(); const recent = new Date(now.getTime() - 2 * 60 * 60 * 1000); @@ -30,7 +40,14 @@ const old = new Date(now.getTime() - 48 * 60 * 60 * 1000); const boundNetworkId = uuidv4(); const foreignNetworkId = uuidv4(); -function opportunity(id: string, createdAt: Date, actorUserId: string, intentId?: string, networkId?: string) { +function opportunity( + id: string, + createdAt: Date, + actorUserId: string, + intentId?: string, + networkId?: string, + status: 'pending' | 'stalled' = 'pending', +) { return { id, detection: { source: 'opportunity_graph' as const, timestamp: createdAt.toISOString() }, @@ -41,7 +58,7 @@ function opportunity(id: string, createdAt: Date, actorUserId: string, intentId? interpretation: { category: 'test', reasoning: 'test-only', confidence: 0.8 }, context: {}, confidence: '0.8', - status: 'pending' as const, + status, createdAt, updatedAt: createdAt, expiresAt: null, @@ -69,6 +86,10 @@ beforeAll(async () => { // community — this is what proves query-layer network filtering (an // unrelated-user row could never distinguish it). opportunity(foreignNetworkOpportunityId, recent, userId, activeIntentId, foreignNetworkId), + // Post-stall park material: a stalled pairing whose negotiation ends in + // the authored gap (counts), and a terminal stall without one (does not). + opportunity(postStallOpportunityId, old, userId, activeIntentId, boundNetworkId, 'stalled'), + opportunity(terminalStallOpportunityId, old, userId, activeIntentId, boundNetworkId, 'stalled'), ]); await db.insert(questions).values([ { @@ -140,14 +161,62 @@ beforeAll(async () => { id: taskIds[4], conversationId, state: 'working', metadata: { type: 'negotiation', opportunityId: foreignNetworkOpportunityId }, createdAt: recent, updatedAt: recent, }, + // ── Parked negotiations: the record pending question counts read. ── + // Mid-flight consult parked on the OWNER — counts as negotiation_inflight. + { + id: parkTaskIds[0], conversationId, state: 'input_required', + metadata: { + type: 'negotiation', + opportunityId: inflightParkOpportunityId, + turnContext: { askUserBinding: { recipientUserId: userId, recipientIntentId: activeIntentId, opportunityId: inflightParkOpportunityId, networkId: boundNetworkId } }, + }, + createdAt: old, updatedAt: old, + }, + // Mid-flight consult parked on the COUNTERPARTY — never this owner's question. + { + id: parkTaskIds[1], conversationId, state: 'input_required', + metadata: { + type: 'negotiation', + opportunityId: counterpartyParkOpportunityId, + turnContext: { askUserBinding: { recipientUserId: otherUserId, recipientIntentId: uuidv4(), opportunityId: counterpartyParkOpportunityId, networkId: boundNetworkId } }, + }, + createdAt: old, updatedAt: old, + }, + // Post-stall park (trailing authored gap below) and a terminal stall. + { + id: parkTaskIds[2], conversationId, state: 'completed', + metadata: { type: 'negotiation', opportunityId: postStallOpportunityId }, createdAt: old, updatedAt: old, + }, + { + id: parkTaskIds[3], conversationId, state: 'completed', + metadata: { type: 'negotiation', opportunityId: terminalStallOpportunityId }, createdAt: old, updatedAt: old, + }, + ]); + await db.insert(messages).values([ + // The authored post-stall gap: newest message of the stalled pairing's + // negotiation, ask_user with the park reasoning, from the owner's agent. + { + id: parkMessageIds[0], conversationId, taskId: parkTaskIds[2], + senderId: `agent:${userId}`, role: 'agent', + parts: [{ kind: 'data', data: { action: 'ask_user', assessment: { reasoning: NEGOTIATION_PARK_REASONING, suggestedRoles: { ownUser: 'peer', otherUser: 'peer' } }, message: null } }], + createdAt: old, + }, + // A terminal stall trails an ordinary turn — not an open question. + { + id: parkMessageIds[1], conversationId, taskId: parkTaskIds[3], + senderId: `agent:${userId}`, role: 'agent', + parts: [{ kind: 'data', data: { action: 'counter', assessment: { reasoning: 'nope', suggestedRoles: { ownUser: 'peer', otherUser: 'peer' } }, message: 'counter' } }], + createdAt: old, + }, ]); }); afterAll(async () => { - await db.delete(tasks).where(inArray(tasks.id, taskIds)); + await db.delete(messages).where(inArray(messages.id, parkMessageIds)); + await db.delete(tasks).where(inArray(tasks.id, [...taskIds, ...parkTaskIds])); await db.delete(conversations).where(eq(conversations.id, conversationId)); await db.delete(questions).where(inArray(questions.id, questionIds)); - await db.delete(opportunities).where(inArray(opportunities.id, [recentOpportunityId, secondRecentOpportunityId, oldOpportunityId, unrelatedOpportunityId, foreignNetworkOpportunityId])); + await db.delete(opportunities).where(inArray(opportunities.id, [recentOpportunityId, secondRecentOpportunityId, oldOpportunityId, unrelatedOpportunityId, foreignNetworkOpportunityId, postStallOpportunityId, terminalStallOpportunityId])); await db.delete(intents).where(inArray(intents.id, [activeIntentId, secondActiveIntentId, pausedIntentId, archivedIntentId])); await db.delete(users).where(inArray(users.id, [userId, otherUserId, unrelatedUserId])); }); @@ -163,7 +232,7 @@ describe('ChatDatabaseAdapter.getAgentActivitySummary', () => { opportunitiesBySignal: [ { intentId: activeIntentId, title: 'Climate founders', count: 3 }, ], - pendingQuestionsByMode: { intent: 1 }, + pendingQuestionsByMode: { negotiation_inflight: 1, negotiation: 1 }, answeredQuestionsByMode: { negotiation: 1 }, negotiationsStarted: 2, negotiationsCompleted: 1, @@ -187,7 +256,7 @@ describe('ChatDatabaseAdapter.getAgentActivitySummary', () => { opportunitiesBySignal: [ { intentId: activeIntentId, title: 'Climate founders', count: 2 }, ], - pendingQuestionsByMode: { intent: 1 }, + pendingQuestionsByMode: { negotiation_inflight: 1, negotiation: 1 }, answeredQuestionsByMode: { negotiation: 1 }, negotiationsStarted: 1, negotiationsCompleted: 1, @@ -207,7 +276,7 @@ describe('ChatDatabaseAdapter.getAgentActivitySummary', () => { opportunitiesBySignal: [ { intentId: activeIntentId, title: 'Climate founders', count: 1 }, ], - pendingQuestionsByMode: { intent: 1 }, + pendingQuestionsByMode: { negotiation_inflight: 1, negotiation: 1 }, answeredQuestionsByMode: { negotiation: 1 }, negotiationsStarted: 1, negotiationsCompleted: 0, diff --git a/services/api/src/controllers/mcp.controller.ts b/services/api/src/controllers/mcp.controller.ts index 85fe23084..c4082380d 100644 --- a/services/api/src/controllers/mcp.controller.ts +++ b/services/api/src/controllers/mcp.controller.ts @@ -671,6 +671,13 @@ function createMcpServerInstance(): McpServer { // #1472: same park annotations on the MCP surface — an external agent // reading this listing must not be told "still negotiating" either. negotiationListingPark: protocolDeps.negotiationListingPark, + // MCP question flow: the answer lane for parked negotiations (the same + // #1466 host, numbering, and consumption queue the chat lane uses) and the + // owner-verdict host behind reject/accept_opportunity (the same #1471 + // Radar Skip/Start-Chat path). The tools register on the MCP surface only; + // the capability matrix confines verdicts to session-authenticated owners. + negotiatorAnswerTools: protocolDeps.negotiatorAnswerTools, + negotiatorVerdictTools: protocolDeps.negotiatorVerdictTools, agentDatabase: protocolDeps.agentDatabase, grantDefaultSystemPermissions: protocolDeps.grantDefaultSystemPermissions, chatSession: protocolDeps.chatSession, diff --git a/services/api/tests/mcp.spec.ts b/services/api/tests/mcp.spec.ts index 9fd5044a1..f57f16aaf 100644 --- a/services/api/tests/mcp.spec.ts +++ b/services/api/tests/mcp.spec.ts @@ -1286,14 +1286,61 @@ describe('MCP Server Factory', () => { expect(resourceCalls.get).toBe(3); }); - it('does not list the retired question tools for any principal', async () => { - // read_pending_questions / answer_pending_question retired with the card - // question surface (conversational-questions plan, "Retirements"). + it('keeps the retired CARD question tools retired; the conversational answer lane is a different contract', async () => { + // read_pending_questions retired with the card question surface + // (conversational-questions plan, "Retirements") and stays retired. + // answer_pending_question RETURNS as a deliberately different contract + // (MCP question flow): the conversational answer lane over the #1466 host + // — (negotiationId, question, answer), the same numbering the listing's + // park annotations print — not the card-era read/answer pair. This pin + // was updated consciously with that registration. const names = await listToolNamesFor({ identity: { userId: 'test-user-id', isSessionAuth: true }, }); expect(names).not.toContain('read_pending_questions'); - expect(names).not.toContain('answer_pending_question'); + expect(names).toContain('answer_pending_question'); + }); + + it('lists the owner verdict tools for session humans only; agent principals never see or reach them', async () => { + // MCP question flow: reject/accept_opportunity are the owner's verdict + // lane — human_only, exactly the session-authenticated class the IND-593 + // owner-provenance binding admits. An API-key agent principal (even one + // holding manage:negotiations + manage:opportunities) is hidden in + // tools/list and capability-denied on tools/call before any DB work. + const humanNames = await listToolNamesFor({ + identity: { userId: 'test-user-id', isSessionAuth: true }, + }); + expect(humanNames).toContain('reject_opportunity'); + expect(humanNames).toContain('accept_opportunity'); + expect(humanNames).toContain('answer_pending_question'); + + const agentDatabase = agentDbWith({ + agentId: 'agent-verdict', + scope: 'global', + scopeId: null, + actions: ['manage:negotiations', 'manage:opportunities'], + }); + const agentNames = await listToolNamesFor({ + identity: { userId: 'test-user-id', agentId: 'agent-verdict' }, + agentDatabase, + }); + expect(agentNames).not.toContain('reject_opportunity'); + expect(agentNames).not.toContain('accept_opportunity'); + // The answer lane IS the agent's — it holds manage:negotiations. + expect(agentNames).toContain('answer_pending_question'); + + for (const toolName of ['reject_opportunity', 'accept_opportunity']) { + const denied = await callTool({ + identity: { userId: 'test-user-id', agentId: 'agent-verdict' }, + agentDatabase, + scopedThrows: true, + toolName, + arguments: { intentId: 'intent-1', counterparty: 1 }, + }); + expect(denied.isError, `${toolName}: agent principal denied`).toBe(true); + expect(denied.code, `${toolName}: capability denial`).toBe('MCP_CAPABILITY_DENIED'); + expect(denied.scopedCreateArgs, `${toolName}: no scoped DB`).toEqual([]); + } }); it('omits complete_onboarding from tools/list for every principal', async () => {