morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #6106] [Bug] Residual-unknown input validation is reported as changed Evidence state #3738

New issue

Closed

opened 2026-10-31 00:43:31 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:43:31 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 21, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/6106

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

MCP or agent integration

Observed behavior

The built stdio MCP server rejects a current-revision update_unknown request with status: "resolved" and resolution: null as execution_failure, telling the caller that Evidence state changed and to refresh and retry. The input is internally inconsistent; no concurrent mutation occurred. The diagnostic drops the validation issue naming resolution.

record_unknown with the same retained Evidence ID in both supporting and contradicting arrays produces the same diagnostic. Rejected writes left one revision-1 head intact. Supplying a consistent nonnull resolution succeeded at revision 2 with the original expected revision.

Reproduction

Place the following focused test under tests/boundary/mcp/inputDiagnostics.test.ts and run npx vitest run tests/boundary/mcp/inputDiagnostics.test.ts --maxWorkers 1 against the built checkout. This is the executed test extracted from a file that also contained two unrelated controls. It launches the production stdio server through the repository harness; no binary or native provider is required. Assertions describe the faulty current behavior.

import { expect, it } from "vitest";
import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js";
import { parseMcpToolError } from "../../fixtures/mcpToolError.js";

it("checks unknown input rejection through built MCP", async () => {
  const { call } = await connectLocalToolsMcp();
  const fields = {
    severity: "medium", supporting_evidence_ids: [], contradicting_evidence_ids: [],
    required_authority: "analyst-inference", required_confidence: "observed",
    required_environment: null, recommended_probes: [], relationships: [],
  };
  const created = await call("record_unknown", { question: "Does the result apply?", domain: "input-test", ...fields });
  expect(created.isError, JSON.stringify(created)).not.toBe(true);
  console.log("created", JSON.stringify(created.structuredContent));
  const evidence = (created.structuredContent as any).result.mutation_evidence_ids[0];
  const overlap = parseMcpToolError(await call("record_unknown", {
    question: "Can one observation both support and contradict?", domain: "input-test", ...fields,
    supporting_evidence_ids: [evidence], contradicting_evidence_ids: [evidence],
  })).error;
  expect(overlap).toMatchObject({ code: "execution_failure", details: { reason: "invalid-transition" } });
  console.log("record-overlap", JSON.stringify(overlap));
  const listed = await call("list_unknowns", {});
  const head = (listed.structuredContent as any).result.items[0];
  const args = { ...fields, unknown_id: head.unknown_id, expected_revision: head.revision, status: "resolved", resolution: null };
  for (let attempt = 0; attempt < 2; attempt++) {
    const invalid = parseMcpToolError(await call("update_unknown", args)).error;
    console.log("unknown-invalid", JSON.stringify(invalid));
    expect(invalid).toMatchObject({ code: "execution_failure", category: "execution_failure", retryable: false, details: { reason: "invalid-transition" } });
    expect(invalid.message).toContain("Evidence state changed");
    expect(invalid.details?.issues).toBeUndefined();
  }
  const unchanged = await call("list_unknowns", {});
  expect((unchanged.structuredContent as any).result.items).toHaveLength(1);
  expect((unchanged.structuredContent as any).result.items[0].revision).toBe(1);
  const corrected = await call("update_unknown", { ...args, resolution: { disposition: "withdrawn", rationale: "Question withdrawn", evidence_ids: [] } });
  expect(corrected.isError, JSON.stringify(corrected)).not.toBe(true);
  console.log("unknown-corrected", JSON.stringify(corrected.structuredContent));

}, 30000);

Both invalid updates and the overlap record failed in the final run; the corrected update succeeded. The combined focused run passed all three test cases.

Expected behavior

Report deterministic cross-field input failures as invalid_request / invalid_input, identify the rejected field or combination, and direct correction of the request. Preserve genuine optimistic revision conflicts and Evidence integrity failures as separate cases.

The status/resolution invariant already creates an issue at resolution. The documented recovery contract directs correction of invalid input instead of an unchanged retry.

Impact and scope

P2: the machine-readable category and recovery action misidentify deterministic malformed requests as an execution/state-change failure, while hiding the field needed for correction. Repeating the invalid update failed identically twice; no downstream autonomous retry loop was measured. A demonstrated workaround is to supply a consistent resolution, which succeeded using the unchanged expected revision. No loss of accepted records was observed. Scope directly executed: the two MCP inputs above; other cross-field checks and CLI delivery were not executed.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

macOS 27.0.1 arm64; Node.js v26.12.0; Vitest 5.0.3. The contributor Node 24.18.0 pin was not exercised.

Target and artifact context

Session-local residual unknowns and their mutation Evidence; no open binary or external provider.

Interface

MCP

Earliest failure stage

Request parsing or validation

Relevant output or evidence

Both invalid updates returned this complete diagnostic:

{"code":"execution_failure","category":"execution_failure","message":"Evidence state changed before the update completed. Refresh the current state and try again.","retryable":false,"remediation":{"action":"Evidence state changed before the update completed. Refresh the current state and try again."},"details":{"reason":"invalid-transition"}}

The overlap record returned the same diagnostic. list_unknowns then contained one item at revision 1. The corrected update returned revision 2, status resolved, and the supplied withdrawn resolution.

Typed outcome or recovery expected

A non-retryable invalid_request / invalid_input diagnostic, details.issues identifying resolution for the inconsistent update and the conflicting Evidence arrays for creation, and inspect_input_contract recovery naming the invoked operation. Preserve the rejection itself.

Analysis or suspected cause

Source-derived: the advertised update schema types status and resolution independently. Revision construction performs the cross-field refinement. The ledger catch replaces its input-validation failure with UnknownRegistryError("invalid-transition"); creation has the analogous catch. Projection maps that reason to execution failure and details retain only the reason. Input validation needs to preserve its cause as structured caller input, without changing actual CAS or integrity errors.

Additional context

Related #5396 concerns duplicate creation (already-exists) incorrectly advertising a retryable revision conflict; its supplementary invalid-transition example resembles this output, but changing duplicate mapping does not fix these input-validation producers. #3005 concerns valid-update integrity failures; #1723 concerns whitespace normalization before hashing; #1724 concerns mutation Evidence content; #3925 concerns workflow replay after a successful edit.

Fetched main d8a9673fc44fde4bd30ff98c241886e6683580ca retains the relevant validation, ledger and projection logic; that later revision was inspected, not executed.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6106 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area MCP or agent integration ### Observed behavior The built stdio MCP server rejects a current-revision `update_unknown` request with `status: "resolved"` and `resolution: null` as `execution_failure`, telling the caller that Evidence state changed and to refresh and retry. The input is internally inconsistent; no concurrent mutation occurred. The diagnostic drops the validation issue naming `resolution`. `record_unknown` with the same retained Evidence ID in both supporting and contradicting arrays produces the same diagnostic. Rejected writes left one revision-1 head intact. Supplying a consistent nonnull resolution succeeded at revision 2 with the original expected revision. ### Reproduction Place the following focused test under `tests/boundary/mcp/inputDiagnostics.test.ts` and run `npx vitest run tests/boundary/mcp/inputDiagnostics.test.ts --maxWorkers 1` against the built checkout. This is the executed test extracted from a file that also contained two unrelated controls. It launches the production stdio server through the repository harness; no binary or native provider is required. Assertions describe the faulty current behavior. ```ts import { expect, it } from "vitest"; import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; it("checks unknown input rejection through built MCP", async () => { const { call } = await connectLocalToolsMcp(); const fields = { severity: "medium", supporting_evidence_ids: [], contradicting_evidence_ids: [], required_authority: "analyst-inference", required_confidence: "observed", required_environment: null, recommended_probes: [], relationships: [], }; const created = await call("record_unknown", { question: "Does the result apply?", domain: "input-test", ...fields }); expect(created.isError, JSON.stringify(created)).not.toBe(true); console.log("created", JSON.stringify(created.structuredContent)); const evidence = (created.structuredContent as any).result.mutation_evidence_ids[0]; const overlap = parseMcpToolError(await call("record_unknown", { question: "Can one observation both support and contradict?", domain: "input-test", ...fields, supporting_evidence_ids: [evidence], contradicting_evidence_ids: [evidence], })).error; expect(overlap).toMatchObject({ code: "execution_failure", details: { reason: "invalid-transition" } }); console.log("record-overlap", JSON.stringify(overlap)); const listed = await call("list_unknowns", {}); const head = (listed.structuredContent as any).result.items[0]; const args = { ...fields, unknown_id: head.unknown_id, expected_revision: head.revision, status: "resolved", resolution: null }; for (let attempt = 0; attempt < 2; attempt++) { const invalid = parseMcpToolError(await call("update_unknown", args)).error; console.log("unknown-invalid", JSON.stringify(invalid)); expect(invalid).toMatchObject({ code: "execution_failure", category: "execution_failure", retryable: false, details: { reason: "invalid-transition" } }); expect(invalid.message).toContain("Evidence state changed"); expect(invalid.details?.issues).toBeUndefined(); } const unchanged = await call("list_unknowns", {}); expect((unchanged.structuredContent as any).result.items).toHaveLength(1); expect((unchanged.structuredContent as any).result.items[0].revision).toBe(1); const corrected = await call("update_unknown", { ...args, resolution: { disposition: "withdrawn", rationale: "Question withdrawn", evidence_ids: [] } }); expect(corrected.isError, JSON.stringify(corrected)).not.toBe(true); console.log("unknown-corrected", JSON.stringify(corrected.structuredContent)); }, 30000); ``` Both invalid updates and the overlap record failed in the final run; the corrected update succeeded. The combined focused run passed all three test cases. ### Expected behavior Report deterministic cross-field input failures as `invalid_request` / `invalid_input`, identify the rejected field or combination, and direct correction of the request. Preserve genuine optimistic revision conflicts and Evidence integrity failures as separate cases. The [status/resolution invariant](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/residualUnknown.ts#L151-L156) already creates an issue at `resolution`. The [documented recovery contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/mcp-contracts.md#L245-L247) directs correction of invalid input instead of an unchanged retry. ### Impact and scope P2: the machine-readable category and recovery action misidentify deterministic malformed requests as an execution/state-change failure, while hiding the field needed for correction. Repeating the invalid update failed identically twice; no downstream autonomous retry loop was measured. A demonstrated workaround is to supply a consistent resolution, which succeeded using the unchanged expected revision. No loss of accepted records was observed. Scope directly executed: the two MCP inputs above; other cross-field checks and CLI delivery were not executed. ### REA version rea-agents 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform macOS 27.0.1 arm64; Node.js v26.12.0; Vitest 5.0.3. The contributor Node 24.18.0 pin was not exercised. ### Target and artifact context Session-local residual unknowns and their mutation Evidence; no open binary or external provider. ### Interface MCP ### Earliest failure stage Request parsing or validation ### Relevant output or evidence Both invalid updates returned this complete diagnostic: ```json {"code":"execution_failure","category":"execution_failure","message":"Evidence state changed before the update completed. Refresh the current state and try again.","retryable":false,"remediation":{"action":"Evidence state changed before the update completed. Refresh the current state and try again."},"details":{"reason":"invalid-transition"}} ``` The overlap record returned the same diagnostic. `list_unknowns` then contained one item at revision 1. The corrected update returned revision 2, status `resolved`, and the supplied `withdrawn` resolution. ### Typed outcome or recovery expected A non-retryable `invalid_request` / `invalid_input` diagnostic, `details.issues` identifying `resolution` for the inconsistent update and the conflicting Evidence arrays for creation, and `inspect_input_contract` recovery naming the invoked operation. Preserve the rejection itself. ### Analysis or suspected cause Source-derived: [the advertised update schema](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/residualUnknown.ts#L267-L284) types status and resolution independently. [Revision construction](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/residualUnknown.ts#L322-L357) performs the cross-field refinement. [The ledger catch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/investigation/EvidenceLedger.ts#L302-L314) replaces its input-validation failure with `UnknownRegistryError("invalid-transition")`; creation has [the analogous catch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/investigation/EvidenceLedger.ts#L227-L236). [Projection](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorProjection.ts#L215-L223) maps that reason to execution failure and [details](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorProjection.ts#L368-L368) retain only the reason. Input validation needs to preserve its cause as structured caller input, without changing actual CAS or integrity errors. ### Additional context Related #5396 concerns duplicate creation (`already-exists`) incorrectly advertising a retryable revision conflict; its supplementary invalid-transition example resembles this output, but changing duplicate mapping does not fix these input-validation producers. #3005 concerns valid-update integrity failures; #1723 concerns whitespace normalization before hashing; #1724 concerns mutation Evidence content; #3925 concerns workflow replay after a successful edit. Fetched main `d8a9673fc44fde4bd30ff98c241886e6683580ca` retains the relevant validation, ledger and projection logic; that later revision was inspected, not executed.

bsakaguchi closed this issue

2026-10-31 00:43:31 +00:00

bsakaguchi commented

2026-10-31 00:43:32 +00:00

Author

Owner

@Sriram-PR commented on GitHub (Oct 22, 2026):

I'll take this one. Reproduced with your test on current main (002e1b68): both the overlapping supporting/contradicting record and the resolved-with-null-resolution update come back as execution_failure with "Evidence state changed before the update completed", and no issues in details. The ledger wraps the createResidualUnknown/updateResidualUnknown validation failure as UnknownRegistryError("invalid-transition"), and analysisErrorProjection.ts has no case for that reason, so it falls through to execution_failure and picks up the revision-conflict message. Plan: project invalid-transition as an invalid request that keeps the validation issues from the cause (so resolution is named) and tells the caller to fix the input, leaving revision-conflict and integrity as they are, plus an MCP boundary test for both cases. PR shortly.

@Sriram-PR commented on GitHub (Oct 22, 2026): I'll take this one. Reproduced with your test on current main (002e1b68): both the overlapping supporting/contradicting record and the resolved-with-null-resolution update come back as `execution_failure` with "Evidence state changed before the update completed", and no issues in `details`. The ledger wraps the `createResidualUnknown`/`updateResidualUnknown` validation failure as `UnknownRegistryError("invalid-transition")`, and `analysisErrorProjection.ts` has no case for that reason, so it falls through to `execution_failure` and picks up the revision-conflict message. Plan: project `invalid-transition` as an invalid request that keeps the validation issues from the cause (so `resolution` is named) and tells the caller to fix the input, leaving `revision-conflict` and `integrity` as they are, plus an MCP boundary test for both cases. PR shortly.

bsakaguchi referenced this issue

2026-11-01 21:10:58 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#3738

No description provided.

Rows
Columns