morluto/rea

Fork 0

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

[GH-ISSUE #2516] fix(mcp): navigation query returns invalid success for malformed selection #833

New issue

Closed

opened 2026-10-15 18:28:50 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:28:50 +00:00

Owner

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

Summary

When a provider reports a null current document or current address, the navigation query reports success with that null value even though successful navigation requires both fields to be strings. Through MCP, the SDK rejects that success and returns a generic output-validation error. The query should classify the unreadable provider observation as a typed failure before success delivery. Both variants reproduced with a simulated provider; no live provider returning null was observed.

Runtime or environment

Failure was verified at a2d8b94dd5 and is still present at 8b181716ac by source recheck, Node.js v26.10.0, npm 11.19.1, Vitest 5.0.3, and repository-locked MCP client/server 2.3.1. The reproduction uses a provider fixture and does not require Hopper, Ghidra, a binary, or a live target.

Steps to reproduce

At the stated revision, place the following test in src/application/navigationOutputContract.test.ts.
Run npm run test:focused -- src/application/navigationOutputContract.test.ts.
Observe that both cases return application success and fail the assertion that the declared navigation output schema accepts that success. Delete the temporary test after recording the result.

import { expect, it } from "vitest";
import { getNavigationContext } from "./AnalysisContextQueries.js";
import {
  createAnalysisExecution,
  type AnalysisOperationPort,
} from "./AnalysisProvider.js";
import { sessionOutputSchemas } from "../contracts/toolOutputSchemaGroups.js";
import { ok } from "../domain/result.js";

const provider = { id: "fixture", name: "Fixture", version: "1" };

it.each([
  [null, "0x401000"],
  ["fixture", null],
] as const)("validates navigation for %s / %s", async (document, address) => {
  const analysis: AnalysisOperationPort = {
    execute: async (operation) => {
      const value = operation === "current_document" ? document
        : operation === "current_address" ? address
        : { found: false, procedure: null };
      return ok(createAnalysisExecution(value, provider));
    },
  };
  const result = await getNavigationContext(analysis, {});
  expect(result.ok).toBe(true);
  if (!result.ok) throw new Error("Unexpected application failure");
  const parsed = sessionOutputSchemas.get_navigation_context.safeParse({
    result: result.value,
  });
  console.log(JSON.stringify({ result, valid: parsed.success }));
  expect(parsed.success).toBe(true);
});

Expected behavior

Successful navigation returns a string document, a string current address, and a procedure name or null. A provider observation that cannot supply the required string should produce a typed provider-output failure, consistent with the existing document-resolution behavior. A null observation must not become a success that the tool's own output contract rejects.

Actual behavior

With no explicit document argument, null current-document output and a string current address produce ok({document:null,address:"0x401000",procedure:null}). A string document and null current address produce ok({document:"fixture",address:null,procedure:null}). The focused run exited 1 with two schema assertions failing at result.document and result.address, each reporting that a string was expected but null was received.

A separate in-memory MCP client/server check used the same canonical tool registration options and production result delivery. The SDK blocked the invalid success and returned the following text error without structured content. This check used a simulated provider and did not exercise a complete target lifecycle.

{"content":[{"type":"text","text":"Output validation error: Invalid structured content for tool get_navigation_context: result.document: Invalid input: expected string, received null"}],"isError":true}

Evidence

Expected source: src/contracts/toolOutputSchemaGroups.ts:516-522 requires string navigation fields; src/contracts/sessionToolSchemas.ts:18-20 permits empty arguments; AGENTS.md, Architecture and Cleanup, requires parsing unknown boundary values and tagged expected failures; src/application/AnalysisDocument.ts:63-71 returns a typed error for a null document observation.
Failure source: src/application/AnalysisContextQueries.ts:39-78 forwards non-string observations into success. The two-case fixture above reproduced the schema rejection. src/server/registerSessionTools.ts:247-264 binds this query to the canonical MCP output contract, and src/server/toolResult.ts:57-67 wraps a successful application result for SDK delivery.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

Six focused checks ran in 1.51 seconds. The two desired schema assertions failed. Four controls passed: an explicit document with a string address produces valid success; an explicit document with null address still produces invalid success; the shared resolver returns AnalysisOutputError for null document output; and the MCP SDK converts invalid success into a generic validation error. The reproduction establishes handling of malformed provider observations, not the frequency of malformed observations from real providers.

Restoration check

For each null variant, the application returns a typed provider-output failure that identifies the unreadable observation, and MCP delivers the corresponding typed error rather than an SDK output-validation message. Successful string observations remain accepted by the existing output schema. An explicit document continues to skip current-document observation, and a cursor outside a procedure still returns procedure:null. Keep the advertised string contract unchanged and do not replace malformed identities with invented strings.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Malformed provider selection produces an invalid application success and loses typed failure diagnostics when the SDK rejects it. The reproduced scope is two simulated null observations; live-provider frequency and broader workflow effects are unknown.
Workaround status: unverified
Workaround basis: An explicit document was verified to restore the null-document case when the address is a string, but it does not restore a null current address. No practical replacement that preserves current navigation discovery for both cases was established.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2516 ## Summary When a provider reports a null current document or current address, the navigation query reports success with that null value even though successful navigation requires both fields to be strings. Through MCP, the SDK rejects that success and returns a generic output-validation error. The query should classify the unreadable provider observation as a typed failure before success delivery. Both variants reproduced with a simulated provider; no live provider returning null was observed. ## Runtime or environment Failure was verified at a2d8b94dd50dab8d966239dd63ab84718aa8b4b4 and is still present at 8b181716aca2cb467be499cba226abfb5ae19141 by source recheck, Node.js v26.10.0, npm 11.19.1, Vitest 5.0.3, and repository-locked MCP client/server 2.3.1. The reproduction uses a provider fixture and does not require Hopper, Ghidra, a binary, or a live target. ## Steps to reproduce 1. At the stated revision, place the following test in `src/application/navigationOutputContract.test.ts`. 2. Run `npm run test:focused -- src/application/navigationOutputContract.test.ts`. 3. Observe that both cases return application success and fail the assertion that the declared navigation output schema accepts that success. Delete the temporary test after recording the result. ```ts import { expect, it } from "vitest"; import { getNavigationContext } from "./AnalysisContextQueries.js"; import { createAnalysisExecution, type AnalysisOperationPort, } from "./AnalysisProvider.js"; import { sessionOutputSchemas } from "../contracts/toolOutputSchemaGroups.js"; import { ok } from "../domain/result.js"; const provider = { id: "fixture", name: "Fixture", version: "1" }; it.each([ [null, "0x401000"], ["fixture", null], ] as const)("validates navigation for %s / %s", async (document, address) => { const analysis: AnalysisOperationPort = { execute: async (operation) => { const value = operation === "current_document" ? document : operation === "current_address" ? address : { found: false, procedure: null }; return ok(createAnalysisExecution(value, provider)); }, }; const result = await getNavigationContext(analysis, {}); expect(result.ok).toBe(true); if (!result.ok) throw new Error("Unexpected application failure"); const parsed = sessionOutputSchemas.get_navigation_context.safeParse({ result: result.value, }); console.log(JSON.stringify({ result, valid: parsed.success })); expect(parsed.success).toBe(true); }); ``` ## Expected behavior Successful navigation returns a string document, a string current address, and a procedure name or null. A provider observation that cannot supply the required string should produce a typed provider-output failure, consistent with the existing document-resolution behavior. A null observation must not become a success that the tool's own output contract rejects. ## Actual behavior With no explicit document argument, null current-document output and a string current address produce `ok({document:null,address:"0x401000",procedure:null})`. A string document and null current address produce `ok({document:"fixture",address:null,procedure:null})`. The focused run exited 1 with two schema assertions failing at `result.document` and `result.address`, each reporting that a string was expected but null was received. A separate in-memory MCP client/server check used the same canonical tool registration options and production result delivery. The SDK blocked the invalid success and returned the following text error without structured content. This check used a simulated provider and did not exercise a complete target lifecycle. ```json {"content":[{"type":"text","text":"Output validation error: Invalid structured content for tool get_navigation_context: result.document: Invalid input: expected string, received null"}],"isError":true} ``` ## Evidence - Expected source: `src/contracts/toolOutputSchemaGroups.ts:516-522` requires string navigation fields; `src/contracts/sessionToolSchemas.ts:18-20` permits empty arguments; `AGENTS.md`, Architecture and Cleanup, requires parsing unknown boundary values and tagged expected failures; `src/application/AnalysisDocument.ts:63-71` returns a typed error for a null document observation. - Failure source: `src/application/AnalysisContextQueries.ts:39-78` forwards non-string observations into success. The two-case fixture above reproduced the schema rejection. `src/server/registerSessionTools.ts:247-264` binds this query to the canonical MCP output contract, and `src/server/toolResult.ts:57-67` wraps a successful application result for SDK delivery. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete Six focused checks ran in 1.51 seconds. The two desired schema assertions failed. Four controls passed: an explicit document with a string address produces valid success; an explicit document with null address still produces invalid success; the shared resolver returns AnalysisOutputError for null document output; and the MCP SDK converts invalid success into a generic validation error. The reproduction establishes handling of malformed provider observations, not the frequency of malformed observations from real providers. ## Restoration check For each null variant, the application returns a typed provider-output failure that identifies the unreadable observation, and MCP delivers the corresponding typed error rather than an SDK output-validation message. Successful string observations remain accepted by the existing output schema. An explicit document continues to skip current-document observation, and a cursor outside a procedure still returns `procedure:null`. Keep the advertised string contract unchanged and do not replace malformed identities with invented strings. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Malformed provider selection produces an invalid application success and loses typed failure diagnostics when the SDK rejects it. The reproduced scope is two simulated null observations; live-provider frequency and broader workflow effects are unknown. - Workaround status: unverified - Workaround basis: An explicit document was verified to restore the null-document case when the address is a string, but it does not restore a null current address. No practical replacement that preserves current navigation discovery for both cases was established.

bsakaguchi

2026-10-15 18:28:50 +00:00

closed this issue
added the
area: mcp

bug
labels

bsakaguchi commented

2026-10-15 18:28:51 +00:00

Author

Owner

@morluto commented on GitHub (Oct 14, 2026):

Fixed on main in 80de3145e4. The navigation query validates both current-document and current-address observations before publishing success and returns a typed AnalysisOutputError for null/non-string output.

Validation: src/application/AnalysisContextQueries.test.ts covers both malformed variants, valid navigation, empty strings allowed by the contract, and provider failures; this suite passed in CI run 38514789942 at 500e32399. This matches the simulated-provider reproduction in this issue.

@morluto commented on GitHub (Oct 14, 2026): Fixed on main in 80de3145e458474319f160d2d2c09586e7a282a4. The navigation query validates both current-document and current-address observations before publishing success and returns a typed AnalysisOutputError for null/non-string output. Validation: `src/application/AnalysisContextQueries.test.ts` covers both malformed variants, valid navigation, empty strings allowed by the contract, and provider failures; this suite passed in CI run 38514789942 at 500e32399. This matches the simulated-provider reproduction in this issue.

bsakaguchi referenced this issue

2026-10-16 21:16:34 +00:00

[PR #833] [MERGED] refactor(domain): map validation taxonomies and add inventory Result seam #2271

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#833

No description provided.

Rows
Columns