diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index ac005e2c8d..1277c74ae5 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -397,6 +397,19 @@ Duplicate creation is a nonretryable `invalid_request`; use the existing head or update it with its current `expected_revision`. A stale update remains a retryable `revision_conflict`. +If automatic question publication fails after a workflow completes, +`details.workflow_unknown_publication` identifies the Evidence and states +whether its retention was confirmed. `created_questions` and +`existing_questions` count processed question occurrences; duplicate questions +may share an Unknown identity. `total_questions` counts string questions, and +`next_question_index` is the zero-based position in the workflow's original +`residual_unknowns` array where publication stopped. It is not a resume token. +For a retained record on the same MCP connection, retrieve the Evidence by ID +with `get_evidence_bundle` and inspect committed heads with `list_unknowns`. +Unprocessed questions remain in the completed Evidence; they were not checked +by this attempt and may already exist from earlier work. A one-shot invocation identifies session-only retention +as `invocation-only` and carries the completed Evidence in its failure when available. + When `record_unknown` or `update_unknown` commits but its complete reply exceeds the response budget, `details.reported_limits.committed_unknown` identifies the exact unknown ID, committed revision and mutation Evidence IDs. Its retained diff --git a/src/application/DirectAnalysis.ts b/src/application/DirectAnalysis.ts index 4e60e95348..5a5d7de710 100644 --- a/src/application/DirectAnalysis.ts +++ b/src/application/DirectAnalysis.ts @@ -514,6 +514,7 @@ const runAnalysis = async ( result: evidence.normalized_result, evidenceId: evidence.evidence_id, recordUnknown: (unknown) => session.recordUnknown(unknown), + evidenceRetained: true, }); if (!unknowns.ok) return err(unknowns.error.retainPartialObservation(evidence)); diff --git a/src/application/WorkflowEvidence.test.ts b/src/application/WorkflowEvidence.test.ts index 62699309a2..1f3d647cc7 100644 --- a/src/application/WorkflowEvidence.test.ts +++ b/src/application/WorkflowEvidence.test.ts @@ -13,7 +13,11 @@ import { createAnalysisExecution, type AnalysisOperationPort, } from "./AnalysisProvider.js"; -import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; +import { + AnalysisCancelledError, + AnalysisResourceConstraintError, +} from "../domain/analysisErrorCore.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { nativeApiBoundarySchema } from "../domain/native/nativeApiBoundary.js"; import { projectNativeApiInspection } from "./native/NativeApiInspection.js"; import { functionDossierSchema } from "../domain/native/functionDossier.js"; @@ -671,6 +675,71 @@ describe("workflow residual question retention", () => { }); }); +it("reports a retained workflow and the committed prefix when Unknown publication fails", () => { + const records = new InvestigationRecords(); + const result = { residual_unknowns: ["First?", "First?", "Second?"] }; + const evidence = createWorkflowEvidence({ + target: undefined, + operation: "trace_feature", + parameters: {}, + result, + upstreamProfile: undefined, + upstreamLimitations: [], + }); + expect(records.recordEvidence(evidence).ok).toBe(true); + const failure = new AnalysisResourceConstraintError( + "record_unknown", + "capacity", + "full", + null, + ); + const outcome = recordWorkflowUnknowns({ + name: "trace_feature", + result, + evidenceId: evidence.evidence_id, + evidenceRetained: true, + recordUnknown: (input) => + input.question.startsWith("Second?") + ? err(failure) + : records.recordUnknown(input), + }); + expect(outcome.ok).toBe(false); + if (outcome.ok) throw new Error("expected failure"); + const projected = projectAnalysisError(outcome.error); + expect(projected.details?.workflow_unknown_publication).toEqual({ + evidence_id: evidence.evidence_id, + evidence_retention: "retained", + total_questions: 3, + created_questions: 1, + existing_questions: 1, + next_question_index: 2, + }); + expect(projected.remediation.action).toContain("get_evidence_bundle"); + const invocation = projectAnalysisError( + outcome.error.retainPartialObservation(evidence), + "invocation", + ); + expect(invocation.details?.workflow_unknown_publication).toMatchObject({ + evidence_retention: "invocation-only", + }); + expect(invocation.details?.partial_observation).toEqual(evidence); + expect(invocation.remediation.action).not.toContain("get_evidence_bundle"); + expect(records.listUnknowns()).toHaveLength(1); + expect(records.evidenceById(evidence.evidence_id)?.normalized_result).toEqual( + result, + ); + expect( + recordWorkflowUnknowns({ + name: "trace_feature", + result, + evidenceId: evidence.evidence_id, + evidenceRetained: true, + recordUnknown: (input) => records.recordUnknown(input), + }).ok, + ).toBe(true); + expect(records.listUnknowns()).toHaveLength(2); +}); + it.each([ " Which boundary? ", "\nWhich boundary?\t", diff --git a/src/application/WorkflowEvidence.ts b/src/application/WorkflowEvidence.ts index dd3c952279..0d30237cbd 100644 --- a/src/application/WorkflowEvidence.ts +++ b/src/application/WorkflowEvidence.ts @@ -8,9 +8,10 @@ import type { AnalysisOperation, AnalysisOperationPort, } from "./AnalysisProvider.js"; -import { ok } from "../domain/result.js"; +import { err, ok } from "../domain/result.js"; import type { EnhancedResult } from "./EnhancedToolTypes.js"; import type { AnalysisProfileCommitment } from "../domain/analysisProfile.js"; +import type { AnalysisError } from "../domain/analysisErrorBase.js"; import type { WorkflowSnapshotRecordInput } from "./binary/BinarySessionRecords.js"; import { analysisProfileSchema, @@ -328,6 +329,8 @@ interface WorkflowUnknownInput { readonly result: JsonValue; readonly evidenceId: string; readonly recordUnknown: UnknownRegistryPort["recordUnknown"] | undefined; + /** The caller confirmed the completed Evidence is in its investigation. */ + readonly evidenceRetained?: boolean; } /** Retain residual workflow questions using the shared investigation policy. */ @@ -336,6 +339,7 @@ export const recordWorkflowUnknowns = ({ result, evidenceId, recordUnknown, + evidenceRetained, }: WorkflowUnknownInput): | ReturnType | { readonly ok: true; readonly value: null } => { @@ -350,7 +354,27 @@ export const recordWorkflowUnknowns = ({ !Array.isArray(result.residual_unknowns) ) return { ok: true, value: null }; - for (const question of result.residual_unknowns) { + const questions = result.residual_unknowns; + const total = questions.reduce( + (count, value) => count + (typeof value === "string" ? 1 : 0), + 0, + ); + let created = 0; + let existing = 0; + const stopped = (error: AnalysisError, index: number) => + err( + error.retainWorkflowUnknownPublication({ + evidence_id: evidenceId, + evidence_retention: + evidenceRetained === true ? "retained" : "unverified", + total_questions: total, + created_questions: created, + existing_questions: existing, + next_question_index: index, + }), + ); + for (let index = 0; index < questions.length; index += 1) { + const question = questions[index]; if (typeof question !== "string") continue; const input = workflowUnknownInput(evidenceId, { question, @@ -370,16 +394,15 @@ export const recordWorkflowUnknowns = ({ ], relationships: [], }); - if (!input.ok) return input; + if (!input.ok) return stopped(input.error, index); const recorded = recordUnknown(input.value); - if ( - !recorded.ok && - !( - recorded.error instanceof UnknownRegistryError && - recorded.error.reason === "already-exists" - ) + if (recorded.ok) created += 1; + else if ( + recorded.error instanceof UnknownRegistryError && + recorded.error.reason === "already-exists" ) - return recorded; + existing += 1; + else return stopped(recorded.error, index); } return { ok: true, value: null }; }; diff --git a/src/domain/analysisErrorBase.ts b/src/domain/analysisErrorBase.ts index cb0b73ba09..34b9c26f9c 100644 --- a/src/domain/analysisErrorBase.ts +++ b/src/domain/analysisErrorBase.ts @@ -54,6 +54,17 @@ export interface AnalysisCleanupObservation { readonly resources: readonly string[]; } +/** Publication stopped after a workflow produced its Evidence. Counts describe + * question occurrences, not distinct Unknown identities. */ +export interface WorkflowUnknownPublication { + readonly evidence_id: string; + readonly evidence_retention: "retained" | "unverified"; + readonly total_questions: number; + readonly created_questions: number; + readonly existing_questions: number; + readonly next_question_index: number; +} + /** * How long the adapter delivering a failure keeps REA's runtime and provider * session for its caller. An MCP connection continues: later calls reuse the @@ -162,6 +173,7 @@ export abstract class AnalysisError extends Error { readonly userCategory: "cancelled" | undefined = undefined; readonly executionFailure: string | undefined = undefined; partialObservation: AnalysisPartialObservation | undefined; + workflowUnknownPublication: WorkflowUnknownPublication | undefined; get cleanupPrimary(): AnalysisError | undefined { return this.#cleanupPrimary; @@ -306,6 +318,14 @@ export abstract class AnalysisError extends Error { this.partialObservation ??= observation; return this; } + + /** Preserve the exact committed prefix when automatic question publication stops. */ + retainWorkflowUnknownPublication( + publication: WorkflowUnknownPublication, + ): this { + this.workflowUnknownPublication ??= Object.freeze({ ...publication }); + return this; + } } /** Confirmed cleanup reveals the typed primary; unresolved ownership stays visible. */ diff --git a/src/domain/analysisErrorProjection.test.ts b/src/domain/analysisErrorProjection.test.ts index 921845a980..d85961b36e 100644 --- a/src/domain/analysisErrorProjection.test.ts +++ b/src/domain/analysisErrorProjection.test.ts @@ -33,7 +33,10 @@ import { } from "./hopperStartupFailure.js"; import { ProviderAdapterError } from "./providerAdapterError.js"; import { UnknownRegistryError } from "./unknownRegistryError.js"; -import { projectAnalysisError } from "./analysisErrorProjection.js"; +import { + childFailureDiagnostics, + projectAnalysisError, +} from "./analysisErrorProjection.js"; import { analysisErrorRemediationAction, analysisErrorUserMessage, @@ -48,6 +51,55 @@ import { analysisErrorWithCleanupFailure } from "./analysisErrorCleanup.js"; import type { AnalysisCallerLifetime } from "./analysisErrorBase.js"; import { ProviderCleanupError } from "./providerCleanupError.js"; +it("projects nested workflow-publication recovery for the actual caller lifetime", () => { + const child = new UnknownRegistryError( + "limit", + ).retainWorkflowUnknownPublication({ + evidence_id: `ev_${"a".repeat(64)}`, + evidence_retention: "retained", + total_questions: 3, + created_questions: 1, + existing_questions: 0, + next_question_index: 1, + }); + const error = new AnalysisCancelledError( + "trace_feature", + childFailureDiagnostics({ child: { error: child } }), + ); + const continuing = projectAnalysisError(error); + const invocation = projectAnalysisError(error, "invocation"); + const childOf = (value: typeof continuing) => { + const diagnostics = value.details?.diagnostics; + if ( + typeof diagnostics !== "object" || + diagnostics === null || + Array.isArray(diagnostics) + ) + throw new Error("Expected child diagnostics"); + return analysisErrorProjectionSchema.parse(diagnostics.child); + }; + expect( + childOf(continuing).details?.workflow_unknown_publication, + ).toMatchObject({ + evidence_retention: "retained", + created_questions: 1, + next_question_index: 1, + }); + expect(childOf(continuing).remediation.action).toContain( + "get_evidence_bundle", + ); + expect( + childOf(invocation).details?.workflow_unknown_publication, + ).toMatchObject({ + evidence_retention: "invocation-only", + created_questions: 1, + next_question_index: 1, + }); + expect(childOf(invocation).remediation.action).not.toContain( + "get_evidence_bundle", + ); +}); + describe("agent recovery metadata", () => { it("does not mark deterministic input validation as retryable", () => { const projected = projectAnalysisError( diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index b01c4a5893..e00d7c486e 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -128,7 +128,8 @@ const projectionDependsOnCallerLifetime = (error: AnalysisError): boolean => { if ( node.limit !== undefined || node.continuingCallerGuidance !== undefined || - node.invocationDiagnostics !== undefined + node.invocationDiagnostics !== undefined || + node.workflowUnknownPublication !== undefined ) return true; return false; @@ -168,8 +169,22 @@ const projectError = ( const underlyingCode = underlyingErrorCode(primary); const code = error.cleanupIncomplete ? "cleanup_incomplete" : underlyingCode; const primaryDetails = errorDetails(primary); + const publication = + composed.workflowUnknownPublication ?? primary.workflowUnknownPublication; const details = { ...primaryDetails, + ...(publication === undefined + ? {} + : { + workflow_unknown_publication: { + ...publication, + evidence_retention: + callerLifetime === "invocation" && + publication.evidence_retention === "retained" + ? "invocation-only" + : publication.evidence_retention, + }, + }), ...(primary.limit === undefined ? {} : { @@ -225,6 +240,11 @@ const projectError = ( ? errorRecovery(error) : cleanupRetryRecovery(cleanupRetry); const remediation = analysisErrorRemediationAction(error, callerLifetime); + const publicationRemediation = + callerLifetime === "continuing" && + publication?.evidence_retention === "retained" + ? " Use get_evidence_bundle with the Evidence ID in details.workflow_unknown_publication to retrieve the completed result; use list_unknowns to inspect recorded questions. Publication stopped at the reported index; later questions were not checked in this attempt." + : ""; return { code, category: analysisErrorCategory(error), @@ -239,8 +259,8 @@ const projectError = ( remediation: { action: cleanupRetry === undefined - ? remediation - : `${remediation} ${cleanupRetryRemediation(cleanupRetry)}`, + ? `${remediation}${publicationRemediation}` + : `${remediation} ${cleanupRetryRemediation(cleanupRetry)}${publicationRemediation}`, }, ...(recovery === undefined ? {} : { recovery }), ...(Object.keys(details).length === 0 ? {} : { details }), diff --git a/src/server/registerEnhancedTools.ts b/src/server/registerEnhancedTools.ts index 92c73bd2c5..99f0c0209c 100644 --- a/src/server/registerEnhancedTools.ts +++ b/src/server/registerEnhancedTools.ts @@ -292,7 +292,12 @@ const executeEnhancedTool = async ( registration.recordEvidence, result, ); - await reportOperationEnd(progress, name, result.ok); + const finish = async ( + response: CallToolResult, + ): Promise => { + await reportOperationEnd(progress, name, response.isError !== true); + return response; + }; if (result.ok) { const evidence = createWorkflowEvidence({ target: registration.activeTarget?.(), @@ -307,7 +312,9 @@ const executeEnhancedTool = async ( }); const recorded = registration.recordEvidence?.(evidence); if (recorded !== undefined && !recorded.ok) - return registration.delivery.toCallToolResult(recorded, contract); + return finish( + registration.delivery.toCallToolResult(recorded, contract), + ); if ( name !== "trace_native_ui_action" && registration.allowsSnapshotReplay?.(name) === true @@ -322,7 +329,9 @@ const executeEnhancedTool = async ( const snapshot = registration.recordWorkflowSnapshot?.(workflowRecord); if (snapshot !== undefined && !snapshot.ok) - return registration.delivery.toCallToolResult(snapshot, contract); + return finish( + registration.delivery.toCallToolResult(snapshot, contract), + ); } } const unknowns = recordWorkflowUnknowns({ @@ -330,9 +339,12 @@ const executeEnhancedTool = async ( result: result.value.result, evidenceId: evidence.evidence_id, recordUnknown: registration.recordUnknown, + evidenceRetained: recorded?.ok === true, }); if (!unknowns.ok) - return registration.delivery.toCallToolResult(unknowns, contract); + return finish( + registration.delivery.toCallToolResult(unknowns, contract), + ); const delivered = request.detail === "summary" ? summarizeRetainedAnalysis(evidence, { @@ -341,16 +353,22 @@ const executeEnhancedTool = async ( }) : ok(evidence); if (!delivered.ok) - return registration.delivery.toCallToolResult(delivered, contract); - return registration.delivery.toEvidenceToolResult( - delivered.value, - contract, - request.detail === "summary" - ? registration.recordEvidence?.(delivered.value) - : recorded, + return finish( + registration.delivery.toCallToolResult(delivered, contract), + ); + return finish( + registration.delivery.toEvidenceToolResult( + delivered.value, + contract, + request.detail === "summary" + ? registration.recordEvidence?.(delivered.value) + : recorded, + ), ); } - return deliverEnhancedFailure(registration, contract, result.error); + return finish( + deliverEnhancedFailure(registration, contract, result.error), + ); }, ); return admitted.ok diff --git a/tests/boundary/mcp/workflowUnknownPublication.test.ts b/tests/boundary/mcp/workflowUnknownPublication.test.ts new file mode 100644 index 0000000000..03721ff389 --- /dev/null +++ b/tests/boundary/mcp/workflowUnknownPublication.test.ts @@ -0,0 +1,120 @@ +import { join } from "node:path"; +import { writeFile } from "node:fs/promises"; + +import { expect, it, vi } from "vitest"; + +import { BinarySession } from "../../../src/application/binary/BinarySession.js"; +import { InvestigationRecords } from "../../../src/application/investigation/InvestigationRecords.js"; +import { + AnalysisCapabilityUnavailableError, + AnalysisResourceConstraintError, +} from "../../../src/domain/analysisErrorCore.js"; +import { err } from "../../../src/domain/result.js"; +import { observed } from "../../fixtures/analysisExecution.js"; +import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js"; +import { createTestProviderRouter } from "../../fixtures/binarySession.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +it.each(["compact", "full"] as const)( + "reports the committed Unknown prefix and retained trace through MCP (%s)", + async (profile) => { + const directory = await createTestTempDirectory("rea-unknown-publication-"); + const target = join(directory, "fixture.com"); + await writeFile(target, Buffer.from([0xb8, 0x00, 0x4c, 0xcd, 0x21])); + const session = new BinarySession( + createTestProviderRouter(() => ({ + execute: async (name) => { + switch (name) { + case "health": + return observed(null); + case "search_strings": + return observed([]); + case "search_procedures": + return observed([{ address: "0x10", value: "goal" }]); + case "xrefs": + return observed(["0x20", "0x30", "0x40"]); + case "resolve_containing_procedure": + return err( + new AnalysisCapabilityUnavailableError( + "fixture", + name, + "not supported", + ), + ); + default: + throw new Error(`Unexpected operation ${name}`); + } + }, + close: async () => ({ ok: true, value: null }), + })), + new InvestigationRecords(), + ); + const original = session.recordUnknown.bind(session); + let attempts = 0; + vi.spyOn(session, "recordUnknown").mockImplementation((input) => { + attempts += 1; + return attempts === 2 + ? err( + new AnalysisResourceConstraintError( + "record_unknown", + "capacity", + "fixture limit", + null, + ), + ) + : original(input); + }); + const { client } = await createApplicationMcpHarness({ + session, + serverOptions: { + environment: { + ...process.env, + REA_MCP_INPUT_SCHEMA_PROFILE: profile, + REA_MCP_OUTPUT_SCHEMA_PROFILE: profile, + }, + }, + }); + const opened = await client.callTool({ + name: "open_binary", + arguments: { path: target, format: "dos-com" }, + }); + expect(opened.isError, JSON.stringify(opened)).not.toBe(true); + const progress: string[] = []; + const reply = await client.callTool( + { name: "trace_feature", arguments: { query: "goal" } }, + { + onprogress: (update) => { + if (update.message) progress.push(update.message); + }, + }, + ); + const projected = parseMcpToolError(reply).error; + expect(projected.code).toBe("resource_constraint"); + expect(projected.details?.workflow_unknown_publication).toMatchObject({ + evidence_retention: "retained", + total_questions: 3, + created_questions: 1, + existing_questions: 0, + next_question_index: 1, + }); + expect(projected.remediation.action).toContain("get_evidence_bundle"); + expect(progress.at(-1)).toContain("failed"); + const retained = session.exportEvidenceBundle(); + const trace = retained.records.find( + (record) => record.operation === "trace_feature", + ); + expect(trace).toBeDefined(); + expect(projected.details?.workflow_unknown_publication).toMatchObject({ + evidence_id: trace?.evidence_id, + }); + expect(trace?.normalized_result).toMatchObject({ + references: expect.arrayContaining([ + expect.objectContaining({ source_address: "0x20" }), + expect.objectContaining({ source_address: "0x30" }), + expect.objectContaining({ source_address: "0x40" }), + ]), + }); + expect(session.listUnknowns()).toHaveLength(1); + }, +);