From 33d8de9e4526ff60d2db634d7e03c95c5e050b75 Mon Sep 17 00:00:00 2001 From: Yanek Yuk Date: Thu, 20 Aug 2026 17:26:35 -0500 Subject: [PATCH] fix(negotiations): the listing must say the park, from the same record the question came from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A client asked "do we have a question?" while a negotiation had sat parked `input_required` on her side for two hours with the open question "Timing: This week". Every #1470 surface was correct — the precedence gate found the question, the prompt's open-questions section named it at position 1. Then the model called `list_negotiations`, which renders lifecycle from OPPORTUNITY STATUS, where the pairing legitimately reads `negotiating`, and which said nothing about the park at all. Holding a static context line saying one thing and a just-executed tool saying another, it went with the tool: "there are currently no open questions… nothing for you to decide." The listing was the last surface deriving "what is happening" from a source other than the shared resolver, so the tool and the context could disagree — and the tool wins the model's trust every time. It now carries a `park` on any negotiation holding one. The question's number and label come from `NegotiationListingParkHost`, whose api-side host resolves them through `readOpenQuestionsForIntent` — the same call the prompt section and `answer_pending_question` make — so the number the client is shown is the number that routes their answer. Whose side a park is on comes from the canonical predicate: `classifyParkedNegotiation` is split into `classifyInflightPark`/`classifyPostStallPark`, pure over the task and messages the listing already holds, so classification costs no extra query and cannot drift. A park on the counterparty is narrated, never quoted. `lifecycleLabel` states the park, superseding the status label — the persona is told to narrate from it, and "the agents are still negotiating" is the sentence that became a false close-out. `opportunityStatus` still reports the truth. A negotiation with no park renders byte-for-byte as before, pinned by a fixture. Claude-Session: https://claude.ai/code/session_01D46BEDXrtVKWUy9LNJ4JNR --- bun.lock | 2 +- packages/protocol/CHANGELOG.md | 66 +++++ packages/protocol/package.json | 2 +- .../protocol/src/chat/negotiator.prompt.ts | 3 +- packages/protocol/src/index.ts | 4 + .../negotiation.answer-consumption.ts | 94 ++++-- .../negotiation.lifecycle-narration.ts | 57 ++++ .../src/negotiations/negotiation.module.ts | 6 + .../negotiations/negotiation.tools.port.ts | 8 + .../src/negotiations/negotiation.tools.ts | 138 ++++++++- .../tests/negotiation.tools.park.spec.ts | 278 ++++++++++++++++++ .../protocol/src/shared/agent/tool.factory.ts | 2 + .../protocol/src/shared/agent/tool.helpers.ts | 19 ++ .../negotiation-listing-park.interface.ts | 57 ++++ .../api/src/controllers/mcp.controller.ts | 9 + .../question/negotiation-listing-park.host.ts | 88 ++++++ .../negotiation-listing-park.host.spec.ts | 143 +++++++++ 17 files changed, 944 insertions(+), 32 deletions(-) create mode 100644 packages/protocol/src/negotiations/tests/negotiation.tools.park.spec.ts create mode 100644 packages/protocol/src/shared/interfaces/negotiation-listing-park.interface.ts create mode 100644 services/api/src/lib/question/negotiation-listing-park.host.ts create mode 100644 services/api/src/lib/question/tests/negotiation-listing-park.host.spec.ts diff --git a/bun.lock b/bun.lock index 9d205e94d..c5fe688b4 100644 --- a/bun.lock +++ b/bun.lock @@ -89,7 +89,7 @@ }, "packages/protocol": { "name": "@indexnetwork/protocol", - "version": "23.5.0", + "version": "23.5.1", "dependencies": { "@langchain/core": "1.1.48", "@langchain/langgraph": "1.3.2", diff --git a/packages/protocol/CHANGELOG.md b/packages/protocol/CHANGELOG.md index 7e49aa751..d47c07150 100644 --- a/packages/protocol/CHANGELOG.md +++ b/packages/protocol/CHANGELOG.md @@ -20,6 +20,72 @@ 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.5.1 - 2026-08-20 + +### Fixed + +- **The negotiation listing says the park, from the same record every other + answerability surface reads.** Observed live: a client asked her agent "do we + have a question?" while a negotiation had sat parked `input_required` on her + side for two hours with the open question "Timing: This week". Every 23.5.0 + surface was correct — the precedence gate found the question, and the + prompt's open-questions section named it at position 1. Then the model called + `list_negotiations`, which renders lifecycle from OPPORTUNITY STATUS, where + the pairing legitimately reads `negotiating`, and which said nothing at all + about the park. Holding a static context line saying one thing and a + just-executed tool result saying another, it went with the tool: "there are + currently no open questions… I am still negotiating with the other agent… + nothing for you to decide." Both clauses false at the task level; both + faithful to what the tool rendered. + + The listing was the last surface still deriving "what is happening" from a + source other than the shared resolver, so the tool and the context could + disagree — and the tool wins the model's trust every time. It now carries a + `park` on any negotiation that holds one: `waitingOn: "you" | "counterparty"`, + and for a park on the client's own side the open question's number and label. + +- **One call, every surface — the answer-openness rule extended to its last + holdout.** The question's NUMBER and LABEL come from + `NegotiationListingParkHost`, whose host implementation resolves them through + the same call the open-questions prompt section and `answer_pending_question` + make. The listing does not enumerate anything of its own, so the number the + client is shown is the number that routes their answer. A question's + `alsoUnblocks` refs carry that same number, since one answer resumes them all. + +- **Whose side a park is on is the canonical predicate, not a second one.** + `classifyParkedNegotiation` is split into `classifyInflightPark` and + `classifyPostStallPark`, pure over the task and messages a caller already + holds; the async function is now the reading half around them and keeps its + lazy messages read. The listing calls them with material it already loaded, so + park classification costs no extra query and cannot drift from the predicate + answer routing uses. Parked-ness is never re-derived from opportunity status. + +- **A park on the counterparty is narrated, never quoted.** It renders as + waiting on their side with no question content: that question is not this + client's to read. + +### Changed + +- **`lifecycleLabel` states the park, superseding the status label.** The + persona is told to take `lifecycleLabel` as its user-facing wording, and "the + agents are still negotiating" is exactly the sentence that became a false + "nothing for you to decide". `opportunityStatus` still reports the true + status; what it may no longer do is supply the sentence. + `connectionState` gains `parked_awaiting_your_answer` and + `parked_awaiting_counterparty`, and `buildLifecycleNarration` takes an + optional third argument. A negotiation with no park renders byte-for-byte as + it did in 23.5.0, and is pinned by a fixture spec. + +- **The tool description and the negotiator prompt both say the two agree.** + `list_negotiations` documents `input_required` as a status, documents `park`, + and states that the numbers come from the same record the open-questions + context section does — so the model has no basis to rank one over the other. + It also states that `negotiating` alone never means "nothing is waiting on + you". The persona gains one grounding rule saying the same thing. + `NegotiationListingParkHost` is optional: without it the listing still says + whether a pairing is parked and on whose side, it just cannot name the + question's number. + ## 23.5.0 - 2026-08-20 ### Added diff --git a/packages/protocol/package.json b/packages/protocol/package.json index ea6887ccf..feefc048a 100644 --- a/packages/protocol/package.json +++ b/packages/protocol/package.json @@ -1,6 +1,6 @@ { "name": "@indexnetwork/protocol", - "version": "23.5.0", + "version": "23.5.1", "type": "module", "main": "./dist/index.js", "types": "./dist/index.d.ts", diff --git a/packages/protocol/src/chat/negotiator.prompt.ts b/packages/protocol/src/chat/negotiator.prompt.ts index eca060541..8cb931047 100644 --- a/packages/protocol/src/chat/negotiator.prompt.ts +++ b/packages/protocol/src/chat/negotiator.prompt.ts @@ -277,7 +277,7 @@ ${profileContext} | Tool | Params | What it does | |------|--------|-------------| -| **list_negotiations** | status?, scope?, limit?, detail? | List agent negotiations with lifecycle-explicit opportunity and owner-action labels (clamped to the pinned signal when one is set; pass scope:'all' for full history) | +| **list_negotiations** | status?, scope?, limit?, detail? | List agent negotiations with lifecycle-explicit opportunity and owner-action labels, and a \`park\` on any negotiation waiting on a person (clamped to the pinned signal when one is set; pass scope:'all' for full history) | | **get_negotiation** | negotiationId | Full negotiation record: messages, outcome, reasoning | | **respond_to_negotiation** | negotiationId, ... | Act on a negotiation — ONLY on explicit client instruction |${opportunityListingToolRow} | **update_opportunity** | opportunityId, status | Accept/pass an opportunity — ONLY on explicit client instruction | @@ -296,6 +296,7 @@ ${profileContext} - **Never fabricate.** Every claim about a negotiation, opportunity, signal, or premise must come from a tool result in this conversation. If you have not looked it up this turn, look it up before answering. Only the client's identity and profile above are preloaded. - **Check tool results before confirming.** Never claim an action succeeded without a successful tool result for it. - **Be honest about your own actions.** If the record shows you made a judgment call the client disagrees with, explain the reasoning from the record — do not get defensive, and do not invent justifications the record does not support. +- **A parked negotiation is waiting on the client, whatever its status says.** \`list_negotiations\` marks any negotiation that is parked with a \`park\` object, and for a park on the client’s own side it names the open question by the SAME number the open-questions section of this prompt shows — both come from one record, so neither can override the other and there is nothing to rank. Opportunity status does not answer this question: a parked pairing still reads \`negotiating\`, so never take \`negotiating\`, or the absence of an item you were looking for, as “nothing is waiting on you”. When the client asks whether anything needs them, \`park.waitingOn="you"\` is a yes — name the question. A park on the counterparty’s side means the opposite: say it is waiting on their side, and never quote a question that is not the client’s to read. - **Keep lifecycle states distinct.** A negotiation task with status \`completed\` means only that the agents concluded. Use the tool's \`lifecycle\` object and \`lifecycleLabel\` for user-facing wording. If the opportunity is \`pending\`, say the agents concluded with a potential match awaiting the owner's review. Agent-turn \`accept\`, \`latestAction=accept\`, and \`outcome.hasOpportunity=true\` are agent-side judgments: never translate them into “I accepted”, “you accepted”, “connected”, “completed connection”, or equivalent. Describe rejected, stalled, draft, expired, pending, and accepted opportunities separately; never aggregate them as completed connections. - **Owner actions require explicit evidence.** Say the owner accepted only when \`lifecycle.ownerAction=accepted\`. This reporting contract does not prove an owner pass, so a rejected opportunity must not be narrated as “you passed” unless a separate current-turn tool result explicitly establishes that owner action. Reporting and history narration are read-only; call \`update_opportunity\` only for the client's explicit current instruction. - **Never infer a direct chat.** Negotiation completion and every opportunity status, including \`accepted\`, are insufficient evidence that an H2H conversation or message thread exists. A \`conversationId\` with \`conversationType=agent_negotiation\` identifies only the A2A agent transcript. \`lifecycle.directConversationEvidence=not_provided\` means do not mention messages. Mention a direct conversation only when a current-turn tool result independently and explicitly supplies H2H conversation evidence.${proposalCardGuidance} diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index 9a903b8b8..1f8b99bbb 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -348,7 +348,9 @@ export { // ─── Negotiation answer consumption (conversational questions) ────────────── export { + classifyInflightPark, classifyParkedNegotiation, + classifyPostStallPark, consumeQuestionBlockAnswers, negotiationParkAnswerId, resumeParkedNegotiation, @@ -362,6 +364,8 @@ export type { NegotiationAnswerInput, NegotiationAnswerResumeOutcome, ParkClassification, + ParkClassificationMessage, + ParkClassificationTask, QuestionBlockAnswerConsumptionInput, QuestionBlockAnswerConsumptionResult, RoutedAnswer, diff --git a/packages/protocol/src/negotiations/negotiation.answer-consumption.ts b/packages/protocol/src/negotiations/negotiation.answer-consumption.ts index ebb0ed3ca..2a762c726 100644 --- a/packages/protocol/src/negotiations/negotiation.answer-consumption.ts +++ b/packages/protocol/src/negotiations/negotiation.answer-consumption.ts @@ -141,12 +141,80 @@ export type ParkClassification = /** Parked, but awaiting the OTHER side's client — this user's answer must not resume it. */ | { kind: "wrong_recipient" }; +/** A negotiation task, reduced to what park classification reads off it. */ +export interface ParkClassificationTask { + id: string; + state: string; + metadata: Record | null; +} + +/** A negotiation's own turn messages, reduced to what post-stall classification reads. */ +export interface ParkClassificationMessage { + senderId: string; + parts: unknown[]; + taskId?: string | null; +} + +/** + * The park an `input_required` negotiation task holds, classified from the + * task itself. Split out of {@link classifyParkedNegotiation} so a caller that + * ALREADY holds the task — the negotiation listing does — asks the same + * question without re-reading it. One predicate, several callers; never the + * same rule written twice. + * + * Callers must only reach here for `state === "input_required"`. + */ +export function classifyInflightPark( + task: ParkClassificationTask, + input: { opportunityId: string; userId: string }, +): Extract | { kind: "not_parked" } | { kind: "wrong_recipient" } { + const binding = readAskUserResumeBinding(task.metadata); + if ( + !binding + || binding.opportunityId !== input.opportunityId + || binding.settlementId !== negotiationQuestionSettlementId(task.id) + ) { + answerLog.warn("input_required negotiation task carries no coherent ask-user binding; answer cannot resume it", { + taskId: task.id, + opportunityId: input.opportunityId, + }); + return { kind: "not_parked" }; + } + if (binding.recipientUserId !== input.userId) return { kind: "wrong_recipient" }; + return { kind: "inflight", taskId: task.id, binding }; +} + +/** + * The park a `completed` negotiation task holds, classified from the + * negotiation's own messages. Same split, same reason as + * {@link classifyInflightPark}: the listing has already read these messages. + * + * Callers must only reach here for `state === "completed"`. + */ +export function classifyPostStallPark( + task: Pick, + messages: ParkClassificationMessage[], + input: { userId: string }, +): Extract | { kind: "not_parked" } | { kind: "wrong_recipient" } { + const park = trailingParkMessage(messages); + if (!park) return { kind: "not_parked" }; + // The gap was written by the finalizing session's task — the most recent + // one. A trailing park from an older task means state has moved on. + if (park.taskId != null && park.taskId !== task.id) return { kind: "not_parked" }; + if (park.senderId !== `agent:${input.userId}`) return { kind: "wrong_recipient" }; + return { kind: "post_stall", taskId: task.id }; +} + /** * Re-resolve a negotiation ref to its current park. This is the exact task * re-resolution the graph itself uses (`getNegotiationTaskForOpportunity`), * never a snapshot: answer routing branches on what the negotiation is NOW, * so a park that was answered, expired, or superseded since the block was * authored classifies as `not_parked` and the answer no-ops. + * + * The two live-park branches are {@link classifyInflightPark} and + * {@link classifyPostStallPark}; this function is the reading half around + * them, and the messages read stays lazy — only a `completed` task pays it. */ export async function classifyParkedNegotiation( database: Pick, @@ -155,32 +223,10 @@ export async function classifyParkedNegotiation( const task = await database.getNegotiationTaskForOpportunity(input.opportunityId); if (!task) return { kind: "no_negotiation" }; - if (task.state === "input_required") { - const binding = readAskUserResumeBinding(task.metadata); - if ( - !binding - || binding.opportunityId !== input.opportunityId - || binding.settlementId !== negotiationQuestionSettlementId(task.id) - ) { - answerLog.warn("input_required negotiation task carries no coherent ask-user binding; answer cannot resume it", { - taskId: task.id, - opportunityId: input.opportunityId, - }); - return { kind: "not_parked" }; - } - if (binding.recipientUserId !== input.userId) return { kind: "wrong_recipient" }; - return { kind: "inflight", taskId: task.id, binding }; - } + if (task.state === "input_required") return classifyInflightPark(task, input); if (task.state === "completed") { - const messages = await database.getNegotiationMessages(input.opportunityId); - const park = trailingParkMessage(messages); - if (!park) return { kind: "not_parked" }; - // The gap was written by the finalizing session's task — the most recent - // one. A trailing park from an older task means state has moved on. - if (park.taskId != null && park.taskId !== task.id) return { kind: "not_parked" }; - if (park.senderId !== `agent:${input.userId}`) return { kind: "wrong_recipient" }; - return { kind: "post_stall", taskId: task.id }; + return classifyPostStallPark(task, await database.getNegotiationMessages(input.opportunityId), input); } // submitted/working/waiting_for_agent: a session is live (possibly the very diff --git a/packages/protocol/src/negotiations/negotiation.lifecycle-narration.ts b/packages/protocol/src/negotiations/negotiation.lifecycle-narration.ts index e891bbe93..4f2718d4c 100644 --- a/packages/protocol/src/negotiations/negotiation.lifecycle-narration.ts +++ b/packages/protocol/src/negotiations/negotiation.lifecycle-narration.ts @@ -12,8 +12,29 @@ type NegotiationConnectionState = | 'expired' | 'agents_negotiating' | 'latent' + /** Parked on THIS user: the negotiation is waiting on their answer. */ + | 'parked_awaiting_your_answer' + /** Parked on the other side: nothing here is waiting on this user. */ + | 'parked_awaiting_counterparty' | 'unknown'; +/** + * A live park on a listed negotiation, as the listing must narrate it (#1472). + * + * `question`/`questionLabel` are present only for a park on THIS user's side + * whose question the host could name, and the number is the one the + * open-questions prompt section shows and `answer_pending_question` takes — + * they come from the same record, never from a second enumeration. A park on + * the counterparty's side carries no question content: it is not this client's + * to read. + */ +export interface NegotiationParkNarration { + waitingOn: 'you' | 'counterparty'; + kind: 'mid_flight' | 'post_stall'; + question?: number; + questionLabel?: string; +} + export interface NegotiationLifecycleNarration { agentNegotiation: 'concluded' | 'in_progress' | 'awaiting_agent' | 'unknown'; opportunityStatus: OpportunityStatus | null; @@ -21,6 +42,26 @@ export interface NegotiationLifecycleNarration { ownerAction: 'accepted' | 'not_recorded'; directConversationEvidence: typeof DIRECT_CONVERSATION_EVIDENCE_UNAVAILABLE; lifecycleLabel: string; + /** Present only while the negotiation actually holds a park. */ + park?: NegotiationParkNarration; +} + +/** + * The one line a park is allowed to render as. Opportunity status is the wrong + * question here — a parked pairing is legitimately `negotiating` — so the park + * SUPERSEDES the status label rather than sitting beside it: the persona is + * told to take `lifecycleLabel` as its user-facing wording, and "the agents + * are still negotiating" is exactly the sentence that became a false "nothing + * for you to decide". + */ +export function parkLifecycleLabel(park: NegotiationParkNarration): string { + if (park.waitingOn === 'counterparty') { + return 'PARKED — waiting on the counterparty’s side. Nothing on this pairing is waiting on the client, and the agents are not exchanging turns until the other side answers.'; + } + const named = park.question !== undefined && park.questionLabel + ? ` waiting on YOUR client’s answer to open question ${park.question}, “${park.questionLabel}”` + : " waiting on YOUR client’s answer"; + return `PARKED —${named}. The agents are NOT exchanging turns and will not until the client answers; this is something for them to decide.`; } /** @@ -31,6 +72,7 @@ export interface NegotiationLifecycleNarration { export function buildLifecycleNarration( negotiationStatus: string, opportunity?: NegotiationOpportunityLifecycle, + park?: NegotiationParkNarration, ): NegotiationLifecycleNarration { const agentNegotiation: NegotiationLifecycleNarration['agentNegotiation'] = negotiationStatus === 'completed' ? 'concluded' @@ -46,6 +88,21 @@ export function buildLifecycleNarration( directConversationEvidence: DIRECT_CONVERSATION_EVIDENCE_UNAVAILABLE, }; + // A live park outranks the opportunity status for narration. The status is + // still reported in `opportunityStatus`; what it may not do is supply the + // sentence, because `negotiating` is a true status and "still negotiating" + // is a false answer to "is anything waiting on me?". + if (park) { + return { + ...common, + connectionState: park.waitingOn === 'you' + ? 'parked_awaiting_your_answer' + : 'parked_awaiting_counterparty', + lifecycleLabel: parkLifecycleLabel(park), + park, + }; + } + switch (opportunity?.status) { case 'pending': return { diff --git a/packages/protocol/src/negotiations/negotiation.module.ts b/packages/protocol/src/negotiations/negotiation.module.ts index 05307751f..a55fced6b 100644 --- a/packages/protocol/src/negotiations/negotiation.module.ts +++ b/packages/protocol/src/negotiations/negotiation.module.ts @@ -4,6 +4,8 @@ * Anything outside this capability imports from here and nowhere else. */ export { AMBIENT_PARK_WINDOW_MS, createNegotiationTools } from "./negotiation.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"; export { IndexNegotiator } from "./negotiation.agent.js"; export { negotiateCandidates, NegotiationGraphFactory } from "./negotiation.graph.js"; @@ -61,7 +63,9 @@ export type { export { NEGOTIATION_PARK_REASONING, NegotiationStallGapAuthor } from "./negotiation.stall-gap.js"; export type { NegotiationStallGap, NegotiationStallReason, StallGapAuthorInput } from "./negotiation.stall-gap.js"; export { + classifyInflightPark, classifyParkedNegotiation, + classifyPostStallPark, consumeQuestionBlockAnswers, negotiationParkAnswerId, resumeParkedNegotiation, @@ -75,6 +79,8 @@ export type { NegotiationAnswerInput, NegotiationAnswerResumeOutcome, ParkClassification, + ParkClassificationMessage, + ParkClassificationTask, QuestionBlockAnswerConsumptionInput, QuestionBlockAnswerConsumptionResult, RoutedAnswer, diff --git a/packages/protocol/src/negotiations/negotiation.tools.port.ts b/packages/protocol/src/negotiations/negotiation.tools.port.ts index fd2e3cbd7..848cdb800 100644 --- a/packages/protocol/src/negotiations/negotiation.tools.port.ts +++ b/packages/protocol/src/negotiations/negotiation.tools.port.ts @@ -1,6 +1,7 @@ import type { NegotiationGraphDatabase } from "../shared/interfaces/database.interface.js"; 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"; /** * Host capabilities consumed by the negotiation tool factory. @@ -15,4 +16,11 @@ export interface NegotiationToolDeps { negotiationDatabase: NegotiationGraphDatabase; agentDispatcher?: AgentDispatcher; negotiationTimeoutQueue?: NegotiationTimeoutQueue; + /** + * Resolves a signal's open questions for the listing's park annotations + * (#1472) — the same record the prompt's open-questions section is built + * from. Optional: without it the listing still says whether a pairing is + * parked and on whose side, it just cannot name the question's number. + */ + negotiationListingPark?: NegotiationListingParkHost; } diff --git a/packages/protocol/src/negotiations/negotiation.tools.ts b/packages/protocol/src/negotiations/negotiation.tools.ts index 1e39fb9dd..aa0d84a00 100644 --- a/packages/protocol/src/negotiations/negotiation.tools.ts +++ b/packages/protocol/src/negotiations/negotiation.tools.ts @@ -12,12 +12,15 @@ import type { NegotiationTurnPayload } from '../shared/interfaces/agent-dispatch import { protocolLogger } from '../shared/observability/protocol.logger.js'; import { focusedIntentId, focusedNetworkId } from '../shared/agent/tool.scope.js'; import { readAuthorizedNegotiationDetail } from './negotiation.detail-reader.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 { classifyInflightPark, classifyPostStallPark } from './negotiation.answer-consumption.js'; +import type { ListingOpenQuestion, NegotiationListingParkHost } from '../shared/interfaces/negotiation-listing-park.interface.js'; import { isNegotiationTurnCapReached } from './negotiation.turn-cap.js'; import { expectedNegotiationSpeaker } from './negotiation.expected-speaker.js'; import { readNegotiationMessages } from './negotiation.scope.js'; -export { buildLifecycleNarration } from './negotiation.lifecycle-narration.js'; +export { buildLifecycleNarration, parkLifecycleLabel } from './negotiation.lifecycle-narration.js'; const logger = protocolLogger('ChatTools:Negotiation'); @@ -139,6 +142,90 @@ function turnsFromMessages(messages: Array<{ parts: unknown[] }>): NegotiationTu .filter(Boolean); } +// ─── Park annotations for the listing (#1472) ─────────────────────────────── +// +// The listing renders lifecycle from OPPORTUNITY STATUS, where a pairing whose +// negotiation is parked on the client legitimately reads `negotiating`. On +// 2026-08-20 that is exactly what it read, while a task had sat +// `input_required` on the client's side for two hours with the open question +// "Timing: This week" — and the model, holding a context line saying one thing +// and a just-executed tool saying another, went with the tool and told her +// there were no open questions and nothing for her to decide. +// +// So the listing says the park, and out of the same record every other +// answerability surface reads. Whose side a park is on is the CANONICAL +// predicate (`classifyInflightPark` / `classifyPostStallPark`), run over the +// task and messages the listing already holds — no second predicate, no extra +// read. The question's NUMBER and LABEL come from the host, which resolves +// them through `readOpenQuestionsForIntent`: the same call the open-questions +// prompt section and `answer_pending_question` make, so the number the client +// is shown here is the number that routes their answer. + +/** + * Every open question of the given signals, keyed by the negotiation it + * unparks. Read per signal — in the pinned case that is a single call — and + * never allowed to fail the listing: an unreadable signal loses its numbers, + * not its parks. + */ +async function readListingOpenQuestions( + host: NegotiationListingParkHost | undefined, + userId: string, + intentIds: string[], +): Promise> { + const byOpportunity = new Map(); + if (!host || intentIds.length === 0) return byOpportunity; + for (const intentId of intentIds) { + try { + for (const question of await host.readOpenQuestions(userId, intentId)) { + if (!byOpportunity.has(question.opportunityId)) byOpportunity.set(question.opportunityId, question); + } + } catch (err) { + logger.warn('Failed to read open questions for negotiation listing', { intentId, err }); + } + } + return byOpportunity; +} + +/** + * The park this listed negotiation currently holds, or null. + * + * The host's open-question set is authoritative for a park on THIS user's side + * — it is the record the answer lands against. The classifier is what tells + * the two remaining cases apart: a park on this user whose question the host + * could not name (no host wired, or a park the block does not carry), and a + * park on the counterparty's side, which is narrated but never quoted. + */ +function listingPark(input: { + userId: string; + opportunityId: string; + task: { id: string; state: string; metadata: Record | null }; + messages: Array<{ senderId: string; parts: unknown[]; taskId?: string | null }>; + openQuestion: ListingOpenQuestion | undefined; +}): NegotiationParkNarration | null { + const { userId, opportunityId, task, messages, openQuestion } = input; + if (task.state === 'input_required') { + const classification = classifyInflightPark(task, { opportunityId, userId }); + if (classification.kind === 'wrong_recipient') return { waitingOn: 'counterparty', kind: 'mid_flight' }; + if (classification.kind !== 'inflight') return null; + return { + waitingOn: 'you', + kind: 'mid_flight', + ...(openQuestion ? { question: openQuestion.question, questionLabel: openQuestion.label } : {}), + }; + } + if (task.state === 'completed') { + const classification = classifyPostStallPark(task, messages, { userId }); + if (classification.kind === 'wrong_recipient') return { waitingOn: 'counterparty', kind: 'post_stall' }; + if (classification.kind !== 'post_stall') return null; + return { + waitingOn: 'you', + kind: 'post_stall', + ...(openQuestion ? { question: openQuestion.question, questionLabel: openQuestion.label } : {}), + }; + } + return null; +} + /** * Creates negotiation MCP tools for external agent access. * Exposes negotiation state for listing, reading, and responding to bilateral negotiations. @@ -155,10 +242,19 @@ export function createNegotiationTools(defineTool: DefineTool, deps: Negotiation '**Statuses:**\n' + '- `active` — Negotiation is in progress, agents are exchanging turns.\n' + '- `waiting_for_agent` — The graph has yielded and is waiting for an agent response (e.g. from the user via respond_to_negotiation) or a timeout.\n' + + '- `input_required` — The negotiation is PARKED on a person’s answer. Read `park` for whose.\n' + '- `completed` — The agent negotiation has concluded (agent-side accept/reject, or turn cap). This is not a completed connection or an owner decision.\n\n' + + '**Parked negotiations:** A negotiation that is 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`). It comes from the SAME open-question record that the ' + + 'open-questions section of your context is built from and that `answer_pending_question` routes against, so the numbers are the same numbers ' + + 'and neither surface overrides the other. `park.waitingOn="you"` means the user has something to answer RIGHT NOW — say so, whatever the ' + + 'opportunity status reads. Opportunity status does not settle this: a parked pairing is still `negotiating`, so `negotiating` alone never means ' + + '"nothing is waiting on you". A `park` on the counterparty’s side names no question content; that question is not this user’s to read.\n\n' + '**Lifecycle narration:** Every result includes additive `lifecycle` fields that distinguish the agent-negotiation state, ' + 'current opportunity status, and persisted owner acceptance. Agent-side `accept` means only that agents found a potential match; ' + - '`pending` still awaits owner review. `directConversationEvidence` is `not_provided`, so this tool never establishes that an H2H message thread exists.\n\n' + + '`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' + '**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({ @@ -209,13 +305,27 @@ export function createNegotiationTools(defineTool: DefineTool, deps: Negotiation return error('Signal scope requires a pinned intent.'); } - const signalIntentIdsByOpportunity = effectiveScope === 'signal' + // Resolved for the signal-scope filter, and — whenever the park host is + // wired — for park annotation in every scope: a pairing parked on the + // client is just as misread in the full-history view. + const parkHost = deps.negotiationListingPark; + const signalIntentIdsByOpportunity = effectiveScope === 'signal' || parkHost ? await negotiationDatabase.getIntentIdsForOpportunities(opportunityIds, context.userId) : null; const scopeMetadata = effectiveScope === 'signal' ? { scope: 'signal' as const, intentId: pinnedIntentId! } : { scope: 'all' as const }; + const annotatedIntentIds = effectiveScope === 'signal' + ? [pinnedIntentId!] + : [...new Set(Object.values(signalIntentIdsByOpportunity ?? {}) + .filter((intentId): intentId is string => typeof intentId === 'string' && intentId.length > 0))]; + const openQuestionsByOpportunity = await readListingOpenQuestions( + parkHost, + context.userId, + annotatedIntentIds, + ); + const negotiations = await Promise.all(tasks.map(async (task) => { const meta = task.metadata as { sourceUserId?: string; @@ -273,6 +383,20 @@ export function createNegotiationTools(defineTool: DefineTool, deps: Negotiation const isUsersTurn = status !== 'completed' && expectedSpeaker === context.userId; + // #1472: whether this pairing is parked, and on whose side. Derived + // from the task and messages already in hand through the canonical + // park predicate; the question's number and label come from the + // question record via the host, never from a second enumeration. + const park = opportunityId + ? listingPark({ + userId: context.userId, + opportunityId, + task, + messages, + openQuestion: openQuestionsByOpportunity.get(opportunityId), + }) + : null; + const base = { id: task.id, counterpartyId: counterpartyId ?? 'unknown', @@ -285,7 +409,11 @@ export function createNegotiationTools(defineTool: DefineTool, deps: Negotiation latestAction: lastTurnData?.action ?? null, latestActionActor: 'agent' as const, latestMessagePreview: lastTurnData?.message ?? null, - lifecycle: buildLifecycleNarration(status, opportunityLifecycles[opportunityId]), + // Top-level and inside `lifecycle` both, deliberately: a park is + // the first thing that must be true about a pairing that has one, + // and it must not depend on the reader opening a nested object. + ...(park ? { park: { ...park, label: parkLifecycleLabel(park) } } : {}), + lifecycle: buildLifecycleNarration(status, opportunityLifecycles[opportunityId], park ?? undefined), createdAt: task.createdAt, updatedAt: task.updatedAt, }; diff --git a/packages/protocol/src/negotiations/tests/negotiation.tools.park.spec.ts b/packages/protocol/src/negotiations/tests/negotiation.tools.park.spec.ts new file mode 100644 index 000000000..55273a075 --- /dev/null +++ b/packages/protocol/src/negotiations/tests/negotiation.tools.park.spec.ts @@ -0,0 +1,278 @@ +/** + * `list_negotiations` and the park (#1472). + * + * The incident: a negotiation sat `input_required` on the client's side for + * two hours with the open question "Timing: This week". Every #1470 surface + * was correct — the precedence gate found the question, the prompt's + * open-questions section named it — and then the model called this tool, + * which renders lifecycle from OPPORTUNITY STATUS, where the pairing + * legitimately reads `negotiating`. Holding a static context line saying one + * thing and a just-executed tool saying another, it went with the tool: + * "there are currently no open questions… nothing for you to decide". + * + * So these specs pin three things: a park on the viewer is named WITH its + * question number (the same number the open-questions enumeration assigns), a + * park on the counterparty is named WITHOUT its content, and a negotiation + * with no park renders byte-for-byte as it did 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, intentId?: string): ResolvedToolContext { + return { + userId, + user: { id: userId, name: "Alice", email: "a@test" }, + userProfile: null, + userNetworks: [], + isMcp: true, + ...(intentId ? { scopeType: "intent" as const, scopeId: intentId } : {}), + } as unknown as ResolvedToolContext; +} + +function captureListing(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 === "list_negotiations") captured = def as typeof captured; + return def; + }; + createNegotiationTools(defineTool as never, deps as unknown as ToolDeps); + return captured!; +} + +/** The parked task as the graph writes it: `input_required` + a coherent binding. */ +function parkedTask(recipientUserId: string) { + return { + id: TASK_ID, + conversationId: "conv-1", + state: "input_required", + metadata: { + type: "negotiation", + sourceUserId: VIEWER, + candidateUserId: COUNTERPARTY, + maxTurns: 6, + opportunityId: OPPORTUNITY_ID, + 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(senderUserId: string) { + return { + senderId: `agent:${senderUserId}`, + taskId: TASK_ID, + parts: [{ kind: "data", data: { action: "ask_user", assessment: { reasoning: NEGOTIATION_PARK_REASONING }, message: null } }], + }; +} + +/** + * The incident's own state: opportunity `negotiating` (which it legitimately + * is), task `input_required` on the viewer, one open question numbered 1. + */ +function incidentDeps(overrides: { + recipientUserId?: string; + openQuestions?: Array<{ opportunityId: string; question: number; label: string }>; + withHost?: boolean; +} = {}) { + const recipientUserId = overrides.recipientUserId ?? VIEWER; + const task = parkedTask(recipientUserId); + return { + negotiationDatabase: { + getTasksForUser: async () => [task], + getNegotiationMessages: async () => [askUserMessage(recipientUserId)], + getMessagesForConversation: async () => [askUserMessage(recipientUserId)], + getIntentIdsForOpportunities: async () => ({ [OPPORTUNITY_ID]: INTENT_ID }), + getOpportunityLifecyclesForNegotiations: async () => ({ + [OPPORTUNITY_ID]: { status: "negotiating", acceptedByOwner: false }, + }), + }, + ...(overrides.withHost === false ? {} : { + negotiationListingPark: { + readOpenQuestions: async () => overrides.openQuestions + ?? [{ opportunityId: OPPORTUNITY_ID, question: 1, label: "Timing: This week" }], + }, + }), + }; +} + +async function listOne(deps: Record, context = makeContext(VIEWER, INTENT_ID)) { + const tool = captureListing(deps); + const result = JSON.parse(await tool.handler({ context, query: {} })); + expect(result.success).toBe(true); + return result.data.negotiations[0]; +} + +describe("list_negotiations — a park on the viewer", () => { + test("names the open question with the number the enumeration assigned", async () => { + const negotiation = await listOne(incidentDeps()); + + expect(negotiation.park).toMatchObject({ + waitingOn: "you", + kind: "mid_flight", + question: 1, + questionLabel: "Timing: This week", + }); + expect(negotiation.park.label).toContain("open question 1"); + expect(negotiation.park.label).toContain("Timing: This week"); + }); + + test("carries whatever number the question record assigned, never a fresh one", async () => { + // The listing enumerates ONE negotiation; the block enumerates three. If + // the listing counted for itself this would read 1. + const negotiation = await listOne(incidentDeps({ + openQuestions: [{ opportunityId: OPPORTUNITY_ID, question: 3, label: "Budget" }], + })); + + expect(negotiation.park.question).toBe(3); + expect(negotiation.park.label).toContain("open question 3"); + }); + + test("the incident: `negotiating` cannot be rendered without the park", async () => { + const negotiation = await listOne(incidentDeps()); + + // The opportunity status is still reported truthfully... + expect(negotiation.lifecycle.opportunityStatus).toBe("negotiating"); + // ...but it may no longer supply the sentence the persona narrates from. + expect(negotiation.lifecycle.connectionState).toBe("parked_awaiting_your_answer"); + expect(negotiation.lifecycle.lifecycleLabel).not.toContain("still negotiating"); + expect(negotiation.lifecycle.lifecycleLabel).toContain("PARKED"); + expect(negotiation.lifecycle.lifecycleLabel).toContain("Timing: This week"); + expect(negotiation.lifecycle.park).toMatchObject({ waitingOn: "you", question: 1 }); + }); + + test("still says the park when no host is wired, just without the number", async () => { + const negotiation = await listOne(incidentDeps({ withHost: false })); + + expect(negotiation.park).toEqual({ + waitingOn: "you", + kind: "mid_flight", + label: negotiation.park.label, + }); + expect(negotiation.park.question).toBeUndefined(); + expect(negotiation.park.label).toContain("PARKED"); + expect(negotiation.lifecycle.connectionState).toBe("parked_awaiting_your_answer"); + }); + + test("a post-stall park on the viewer is a park too", async () => { + const task = { ...parkedTask(VIEWER), state: "completed" }; + const negotiation = await listOne({ + negotiationDatabase: { + getTasksForUser: async () => [task], + getNegotiationMessages: async () => [askUserMessage(VIEWER)], + getMessagesForConversation: async () => [askUserMessage(VIEWER)], + getIntentIdsForOpportunities: async () => ({ [OPPORTUNITY_ID]: INTENT_ID }), + getOpportunityLifecyclesForNegotiations: async () => ({ + [OPPORTUNITY_ID]: { status: "stalled", acceptedByOwner: false }, + }), + }, + negotiationListingPark: { + readOpenQuestions: async () => [{ opportunityId: OPPORTUNITY_ID, question: 2, label: "Rate" }], + }, + }); + + expect(negotiation.park).toMatchObject({ waitingOn: "you", kind: "post_stall", question: 2, questionLabel: "Rate" }); + }); +}); + +describe("list_negotiations — a park on the counterparty", () => { + test("is named, and its content is not", async () => { + const negotiation = await listOne(incidentDeps({ + recipientUserId: COUNTERPARTY, + // Even if the host somehow offered a question for this negotiation, the + // park is not on this client and its content is not theirs to read. + openQuestions: [{ opportunityId: OPPORTUNITY_ID, question: 1, label: "Their private ask" }], + })); + + expect(negotiation.park).toEqual({ + waitingOn: "counterparty", + kind: "mid_flight", + label: negotiation.park.label, + }); + expect(negotiation.park.question).toBeUndefined(); + expect(negotiation.park.questionLabel).toBeUndefined(); + expect(JSON.stringify(negotiation)).not.toContain("Their private ask"); + expect(negotiation.lifecycle.connectionState).toBe("parked_awaiting_counterparty"); + expect(negotiation.lifecycle.lifecycleLabel).toContain("counterparty"); + }); +}); + +describe("list_negotiations — no park", () => { + test("renders exactly as it did before the park annotations 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 = { + senderId: `agent:${VIEWER}`, + taskId: TASK_ID, + parts: [{ kind: "data", data: { action: "propose", assessment: { reasoning: "why" }, message: "hello" } }], + }; + const negotiation = await listOne({ + negotiationDatabase: { + getTasksForUser: async () => [task], + getNegotiationMessages: async () => [message], + getMessagesForConversation: async () => [message], + getIntentIdsForOpportunities: async () => ({ [OPPORTUNITY_ID]: INTENT_ID }), + getOpportunityLifecyclesForNegotiations: async () => ({ + [OPPORTUNITY_ID]: { status: "negotiating", acceptedByOwner: false }, + }), + }, + negotiationListingPark: { readOpenQuestions: async () => [] }, + }); + + // The fixture pin: field-for-field what this tool returned before #1472. + expect(negotiation).toEqual({ + id: TASK_ID, + counterpartyId: COUNTERPARTY, + role: "source", + turnCount: 1, + status: "active", + isUsersTurn: false, + isContinuation: false, + priorTurnCount: 0, + latestAction: "propose", + latestActionActor: "agent", + latestMessagePreview: "hello", + 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.", + }, + createdAt: task.createdAt.toISOString(), + updatedAt: task.updatedAt.toISOString(), + }); + expect("park" in negotiation).toBe(false); + }); +}); diff --git a/packages/protocol/src/shared/agent/tool.factory.ts b/packages/protocol/src/shared/agent/tool.factory.ts index 323574868..610538dbf 100644 --- a/packages/protocol/src/shared/agent/tool.factory.ts +++ b/packages/protocol/src/shared/agent/tool.factory.ts @@ -210,6 +210,8 @@ export async function createChatTools( enricher: deps.enricher, negotiationDatabase: deps.negotiationDatabase, negotiationTimeoutQueue: deps.negotiationTimeoutQueue, + // #1472: the open-question record behind the listing's park annotations. + ...(deps.negotiationListingPark && { negotiationListingPark: deps.negotiationListingPark }), agentDatabase: deps.agentDatabase, grantDefaultSystemPermissions: deps.grantDefaultSystemPermissions, agentDispatcher: deps.agentDispatcher, diff --git a/packages/protocol/src/shared/agent/tool.helpers.ts b/packages/protocol/src/shared/agent/tool.helpers.ts index da76bb281..b30fdac09 100644 --- a/packages/protocol/src/shared/agent/tool.helpers.ts +++ b/packages/protocol/src/shared/agent/tool.helpers.ts @@ -24,6 +24,7 @@ import type { DeliveryLedger } from "../interfaces/delivery-ledger.interface.js" import type { NegotiatorMemoryToolsHost } from "../interfaces/negotiator-memory.interface.js"; import type { NegotiatorAnswerToolsHost } from "../interfaces/negotiator-answer.interface.js"; import type { NegotiatorVerdictToolsHost } from "../interfaces/negotiator-verdict.interface.js"; +import type { NegotiationListingParkHost } from "../interfaces/negotiation-listing-park.interface.js"; import type { QuestionerEnqueueFn } from "../../questions/question.input.js"; import type { EnrichmentRunQueue, EnrichmentRunStore } from "../interfaces/enrichment-run.interface.js"; import type { McpActivityCaller } from "./activity-projection.js"; @@ -192,6 +193,15 @@ interface ToolContextBindings { * intent-scoped session (the counterparties are one signal's). */ negotiatorVerdictTools?: NegotiatorVerdictToolsHost; + /** + * Host bridge for the park annotations `list_negotiations` renders (#1472). + * Resolves a signal's open questions out of the same record the prompt's + * open-questions section is built from, so the listing and the context + * cannot disagree about whether a negotiation is waiting on the client. + * Absent → the listing still says whether a pairing is parked and on whose + * side, but cannot name the question's number. + */ + negotiationListingPark?: NegotiationListingParkHost; /** * Resolve a user's global user_context paragraph (profile-replacing identity * text), generating it on demand when absent. Mirrors `ToolDeps.getUserContextText` @@ -468,6 +478,15 @@ interface ToolDepsBindings { enricher: ProfileEnricher; /** Database adapter for negotiations/conversation operations. */ negotiationDatabase: NegotiationGraphDatabase; + /** + * Host bridge for the park annotations `list_negotiations` renders (#1472). + * Resolves a signal's open questions out of the same record the persona's + * open-questions context section is built from, so the listing and the + * context cannot disagree about whether a negotiation is waiting on the + * client. Absent → the listing still says whether a pairing is parked and on + * whose side, it just cannot name the question's number. + */ + negotiationListingPark?: NegotiationListingParkHost; /** 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/interfaces/negotiation-listing-park.interface.ts b/packages/protocol/src/shared/interfaces/negotiation-listing-park.interface.ts new file mode 100644 index 000000000..a4b22c359 --- /dev/null +++ b/packages/protocol/src/shared/interfaces/negotiation-listing-park.interface.ts @@ -0,0 +1,57 @@ +/** + * Host bridge for the park annotations the negotiation LISTING renders (#1472). + * + * The listing was the last surface still answering "what is happening on this + * pairing?" from a source of its own. It renders lifecycle from OPPORTUNITY + * STATUS, and a pairing whose negotiation is parked on the client legitimately + * reads `negotiating` there. On 2026-08-20 a client asked "do we have a + * question?" while a negotiation had sat `input_required` on her side for two + * hours: every #1470 surface was correct — the precedence gate found the + * question and the prompt's open-questions section named it — and then the + * model called `list_negotiations`, read "still negotiating", and told her + * there were no open questions and nothing for her to decide. + * + * A model holding a static context line and a just-executed tool result will + * take the tool. So the listing must say the park, out of the SAME record the + * open-questions section is built from — `readOpenQuestionsForIntent` on the + * host side. Not a second predicate, not a re-derivation from opportunity + * status: the same call, so the number the listing prints and the number + * `answer_pending_question` takes cannot drift apart. + * + * The host is optional. Without it the listing still says WHETHER a pairing is + * parked and on whose side (classified from the task the listing already + * holds); what it loses is the question's number and label, which only the + * question record can supply. + */ + +/** One open question of a signal, as the listing must name it. */ +export interface ListingOpenQuestion { + /** + * The negotiation this question unparks. Includes refs a question carries + * through `alsoUnblocks` — one answer resumes them all, so each of them is + * annotated with that question's number. + */ + opportunityId: string; + /** + * 1-based position in the signal's question block — the SAME number the + * open-questions prompt section shows and `answer_pending_question` takes. + */ + question: number; + /** The step's label: its checklist dimension, else a short form of the prompt. */ + label: string; +} + +export interface NegotiationListingParkHost { + /** + * Every question currently open on this user's side for one signal, resolved + * through the question record itself. + * + * Must never throw and must never widen: an unreadable signal resolves to + * `[]`, and a park on the counterparty's side is not this user's question to + * read and must not appear here. + * + * @param userId - The acting client; the host scopes every read to them. + * @param intentId - The signal whose parked negotiations are being listed. + */ + readOpenQuestions(userId: string, intentId: string): Promise>; +} diff --git a/services/api/src/controllers/mcp.controller.ts b/services/api/src/controllers/mcp.controller.ts index fd7d9525b..85fe23084 100644 --- a/services/api/src/controllers/mcp.controller.ts +++ b/services/api/src/controllers/mcp.controller.ts @@ -41,6 +41,7 @@ import { negotiatorClientDmRetrieve } from '../adapters/negotiator-client-dm.ret import { negotiatorMemoryWriteService } from '../services/negotiator-memory.service'; import { isNegotiatorMemoryWriteEnabled } from '../lib/negotiator-feature'; import { negotiatorAnswerToolsHost } from '../lib/question/negotiator-answer.host'; +import { negotiationListingParkHost } from '../lib/question/negotiation-listing-park.host'; import { negotiatorVerdictToolsHost } from '../lib/agent/negotiator-verdict.host'; import { resolveProtocolBaseUrl } from '../lib/protocol-url'; import { isHermesNegotiatorAudience } from '../lib/agent/hermes-credential'; @@ -126,6 +127,11 @@ const protocolDeps = { // in chat before. Registered only in intent-pinned negotiator sessions; the // orchestrator registry never sees it. negotiatorVerdictTools: negotiatorVerdictToolsHost, + // #1472: the open-question record behind `list_negotiations`' park + // annotations. The listing was the last surface deriving "what is happening" + // from a source other than the shared resolver, so the tool and the context + // could disagree — and the tool wins the model's trust every time. + negotiationListingPark: negotiationListingParkHost, ...(isNegotiatorMemoryWriteEnabled() && { negotiatorMemoryTools: { remember: async (userId: string, input: { kind: 'disclosure_rule' | 'playbook' | 'threshold'; content: string; sessionId?: string }) => @@ -662,6 +668,9 @@ function createMcpServerInstance(): McpServer { negotiationDatabase: protocolDeps.negotiationDatabase, agentDispatcher: protocolDeps.agentDispatcher, negotiationTimeoutQueue: protocolDeps.negotiationTimeoutQueue, + // #1472: same park annotations on the MCP surface — an external agent + // reading this listing must not be told "still negotiating" either. + negotiationListingPark: protocolDeps.negotiationListingPark, agentDatabase: protocolDeps.agentDatabase, grantDefaultSystemPermissions: protocolDeps.grantDefaultSystemPermissions, chatSession: protocolDeps.chatSession, diff --git a/services/api/src/lib/question/negotiation-listing-park.host.ts b/services/api/src/lib/question/negotiation-listing-park.host.ts new file mode 100644 index 000000000..cf6af5ca5 --- /dev/null +++ b/services/api/src/lib/question/negotiation-listing-park.host.ts @@ -0,0 +1,88 @@ +/** + * Host bridge behind the park annotations `list_negotiations` renders (#1472). + * + * The listing was the last surface still answering "what is happening on this + * pairing?" from a source of its own. It renders lifecycle from OPPORTUNITY + * STATUS, and a pairing whose negotiation is parked on the client legitimately + * reads `negotiating` there. On 2026-08-20, in the DM of a signal whose + * negotiation had sat `input_required` on the client's side for two hours with + * the open question "Timing: This week", every #1470 surface was correct — the + * precedence gate found the question, the prompt's open-questions section + * named it — and then the model called `list_negotiations`, read "still + * negotiating", and told her there were no open questions and nothing for her + * to decide. Both clauses false at the task level; both faithful to what the + * tool rendered. A model holding a static context line and a just-executed + * tool result will take the tool, every time. + * + * So this exists, and it re-implements nothing: it resolves the signal's open + * questions through the SAME call the precedence gate, the prompt section and + * `answer_pending_question` make (`readOpenQuestionsForIntent`) and hands the + * listing back the enumeration it already produced. The number the listing + * prints IS the number the answer routes against, because there is one call + * and one enumeration — not the same rule written twice. That is the #1470 + * rule, extended to its last holdout. + * + * The parked-ness itself is NOT resolved here. The listing already holds the + * task and the negotiation's messages and classifies whose side a park is on + * through the canonical protocol predicate; what only the question record can + * supply is the question's number and label, and that is all this returns. + */ +import { readOpenQuestionsForIntent } from './open-question-message'; +import type { OpenQuestionsForIntentDeps } from './open-question-message'; +import { log } from '../log'; + +const logger = log.lib.from('negotiation-listing-park.host'); + +/** Mirrors the protocol's `ListingOpenQuestion`; structural by design. */ +export interface ListingOpenQuestion { + opportunityId: string; + question: number; + label: string; +} + +/** + * Every question currently open on this user's side for one signal, expanded + * to every negotiation each one unparks. + * + * `alsoUnblocks` refs are annotated with their question's number too: one + * answer resumes them all, and the block guarantees a ref appears exactly once + * across the whole block, so the mapping is unambiguous. A pairing unparked + * through an `alsoUnblocks` ref would otherwise render as parked-with-no-number + * while its sibling rendered the number — the same divergence, one level down. + * + * Never throws: this feeds a tool result, and the reader below it already + * swallows its own failures. `[]` means nothing is parked on this user's side + * for this signal — the only state in which the listing may stay silent. + */ +export async function readListingOpenQuestions( + userId: string, + intentId: string, + deps?: OpenQuestionsForIntentDeps, +): Promise { + try { + const open = await readOpenQuestionsForIntent(userId, intentId, deps); + if (!open) return []; + return open.questions.flatMap((question) => { + // Same index by construction: `enumerateQuestions` maps the block's + // questions 1:1, in order, into `questions`. + const alsoUnblocks = open.block.questions[question.position - 1]?.alsoUnblocks ?? []; + return [question.opportunityId, ...alsoUnblocks].map((opportunityId) => ({ + opportunityId, + question: question.position, + label: question.label, + })); + }); + } catch (err) { + logger.warn('listing_open_questions_read_failed', { + userId, + intentId, + error: err instanceof Error ? err.message : String(err), + }); + return []; + } +} + +/** The host object the composition root injects into the negotiation toolset. */ +export const negotiationListingParkHost = { + readOpenQuestions: (userId: string, intentId: string) => readListingOpenQuestions(userId, intentId), +}; diff --git a/services/api/src/lib/question/tests/negotiation-listing-park.host.spec.ts b/services/api/src/lib/question/tests/negotiation-listing-park.host.spec.ts new file mode 100644 index 000000000..654c2ab1d --- /dev/null +++ b/services/api/src/lib/question/tests/negotiation-listing-park.host.spec.ts @@ -0,0 +1,143 @@ +/** + * The host behind `list_negotiations`' park annotations (#1472), and the + * anti-divergence contract it exists to hold. + * + * The listing was the last surface answering "what is happening on this + * pairing?" from a source of its own — opportunity status, where a parked + * pairing legitimately reads `negotiating`. On 2026-08-20 the model held a + * correct open-questions context section AND a listing that said "still + * negotiating", and it went with the tool: "there are currently no open + * questions… nothing for you to decide", while a task had sat + * `input_required` on the client's side for two hours. + * + * So the number the listing prints and the number `answer_pending_question` + * routes against must come from ONE call. These specs drive both from a single + * fixture and assert they are equal — the #1470 pattern, one surface further. + */ +import { describe, expect, it } from 'bun:test'; + +import { serializeQuestionMessage } from '@indexnetwork/protocol'; +import type { QuestionBlock } from '@indexnetwork/protocol'; + +import type { ParkedNegotiation } from '../../../adapters/parked-negotiation.reader.adapter'; +import { readListingOpenQuestions } from '../negotiation-listing-park.host'; +import { readOpenQuestionsForIntent } from '../open-question-message'; + +const USER_ID = '6c17f313-0000-4000-8000-000000000001'; +const INTENT_ID = '34fa30bc-0000-4000-8000-000000000002'; +const TIMING_OPPORTUNITY = '6d8b07ef-7fa8-4968-80d9-6af0ce364d27'; +const BUDGET_OPPORTUNITY = '7f3d2c1b-8a90-4e5f-b6c7-d8e9f0a1b2c3'; +const SIBLING_OPPORTUNITY = '1a2b3c4d-5e6f-4a8b-9c0d-1e2f3a4b5c6d'; + +/** The incident's own block: the buried question is question 1. */ +const BLOCK: QuestionBlock = { + version: 1, + questions: [ + { prompt: 'When could you meet?', opportunityId: TIMING_OPPORTUNITY, dimension: 'Timing: This week' }, + { prompt: 'What budget range works?', opportunityId: BUDGET_OPPORTUNITY, dimension: 'Budget' }, + ], +}; +const BODY = serializeQuestionMessage('Two conversations are waiting on you.', BLOCK); + +function park(opportunityId: string, dimension: string): ParkedNegotiation { + return { + opportunityId, + kind: 'mid_flight', + dimension, + dimensionKind: 'hard_constraint', + transcript: [], + parkedAt: new Date('2026-08-20T20:20:00Z'), + }; +} + +function deps(overrides: Record = {}) { + return { + findSession: async () => ({ id: 'session-1' }), + getSessionMessages: async () => [ + { id: 'm1', role: 'assistant', content: BODY }, + // The message that buried the question without settling it. + { id: 'm2', role: 'assistant', content: 'Updated your signal: timing is now open to this month.' }, + ], + readParkedNegotiations: async () => [ + park(TIMING_OPPORTUNITY, 'Timing: This week'), + park(BUDGET_OPPORTUNITY, 'Budget'), + ], + ...overrides, + }; +} + +describe('readListingOpenQuestions', () => { + it('numbers each negotiation exactly as the open-questions enumeration does', async () => { + const harness = deps(); + + const listing = await readListingOpenQuestions(USER_ID, INTENT_ID, harness); + const enumeration = await readOpenQuestionsForIntent(USER_ID, INTENT_ID, harness); + + // The anti-divergence assertion: same fixture, one call underneath, so the + // listing's numbers ARE the enumeration's numbers — and the enumeration's + // numbers are what the prompt section renders and the answer host resolves. + expect(enumeration).not.toBeNull(); + for (const question of enumeration!.questions) { + const annotation = listing.find((entry) => entry.opportunityId === question.opportunityId); + expect(annotation).toBeDefined(); + expect(annotation!.question).toBe(question.position); + expect(annotation!.label).toBe(question.label); + } + expect(listing).toHaveLength(enumeration!.questions.length); + }); + + it("names the incident's buried question as question 1", async () => { + const listing = await readListingOpenQuestions(USER_ID, INTENT_ID, deps()); + + expect(listing.find((entry) => entry.opportunityId === TIMING_OPPORTUNITY)).toEqual({ + opportunityId: TIMING_OPPORTUNITY, + question: 1, + label: 'Timing: This week', + }); + }); + + it('annotates every negotiation one question unblocks with that question\'s number', async () => { + const block: QuestionBlock = { + version: 1, + questions: [{ + prompt: 'When could you meet?', + opportunityId: TIMING_OPPORTUNITY, + dimension: 'Timing: This week', + alsoUnblocks: [SIBLING_OPPORTUNITY], + }], + }; + const harness = deps({ + getSessionMessages: async () => [{ id: 'm1', role: 'assistant', content: serializeQuestionMessage('Waiting on you.', block) }], + readParkedNegotiations: async () => [ + park(TIMING_OPPORTUNITY, 'Timing: This week'), + park(SIBLING_OPPORTUNITY, 'Timing: This week'), + ], + }); + + const listing = await readListingOpenQuestions(USER_ID, INTENT_ID, harness); + + // One answer resumes both, so both pairings carry the same number — the + // alternative is one of them rendering as parked-with-no-number beside its + // sibling, which is the same divergence one level down. + expect(listing).toEqual([ + { opportunityId: TIMING_OPPORTUNITY, question: 1, label: 'Timing: This week' }, + { opportunityId: SIBLING_OPPORTUNITY, question: 1, label: 'Timing: This week' }, + ]); + }); + + it('returns nothing when nothing is parked on this user for this signal', async () => { + const listing = await readListingOpenQuestions(USER_ID, INTENT_ID, deps({ + readParkedNegotiations: async () => [], + })); + + expect(listing).toEqual([]); + }); + + it('degrades to no annotations rather than failing the listing', async () => { + const listing = await readListingOpenQuestions(USER_ID, INTENT_ID, deps({ + readParkedNegotiations: async () => { throw new Error('db down'); }, + })); + + expect(listing).toEqual([]); + }); +});