morluto/rea

Fork 0

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

[GH-ISSUE #5100] [Bug] WASM summary requests enter the provider before refusing a fixed server without Evidence retention #3138

New issue

Closed

opened 2026-10-31 00:37:03 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-31 00:37:03 +00:00

Owner

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

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

A mocked boundary reproduction through the MCP SDK, production createServer, and WasmArtifactService shows that inspect_wasm_artifact with detail: "summary" invokes the WASM provider on a fixed server without Evidence retention. After a successful provider result it returns capability_unavailable because the server cannot retain Evidence. With a synthetic provider failure, invalid_request is returned instead of the retention diagnostic. Complete delivery succeeds.

This confirms provider entry and error precedence. No real WABT toolchain or WASM file was read or executed.

Reproduction

From a source checkout at the revision below with locked dependencies already installed, save this probe as tests/boundary/mcp/retention-preflight.test.ts. It uses the repository's synthetic WASM execution fixture and fixed-server test binding; the MCP registration and application service are production code.

import { Client, InMemoryTransport } from '@modelcontextprotocol/client';
import { expect, it, vi } from 'vitest';
import { WasmArtifactService } from '../../../src/application/wasm/WasmArtifactService.js';
import { AnalysisInputError } from '../../../src/domain/analysisErrorCore.js';
import { err, ok } from '../../../src/domain/result.js';
import { parseEvidence } from '../../../src/domain/evidence.js';
import { createServer } from '../../../src/server/createServer.js';
import { createTestBinarySession } from '../../fixtures/binarySession.js';
import { parseMcpToolError } from '../../fixtures/mcpToolError.js';
import { wasmArtifactFixture, wasmArtifactExecution } from '../../fixtures/wasm/artifact.js';

it('observes retention refusal only after WASM provider entry on a fixed MCP server', async () => {
 const value = wasmArtifactFixture();
 const inspect = vi.fn(() => Promise.resolve(ok(wasmArtifactExecution(value))));
 const service = new WasmArtifactService({close: () => Promise.resolve(), inspect});
 const session = createTestBinarySession(() => { throw new Error('No native provider expected'); });
 const server = createServer({kind:'fixed', analysis:session}, {wasmArtifact:service, environment:{}});
 const client = new Client({name:'retention-probe', version:'1'});
 const [a,b] = InMemoryTransport.createLinkedPair();
 try {
  await server.connect(b); await client.connect(a);
  const summary = await client.callTool({name:'inspect_wasm_artifact', arguments:{path:value.artifact.path,detail:'summary'}});
  const summaryError = parseMcpToolError(summary).error;
  expect(summaryError.code).toBe('capability_unavailable');
  expect(summaryError.message).toContain('does not retain Evidence');
  expect(inspect).toHaveBeenCalledTimes(1);
  console.log(JSON.stringify({case:'summary-successful-provider',provider_calls:inspect.mock.calls.length,code:summaryError.code,message:summaryError.message}));
  const complete = await client.callTool({name:'inspect_wasm_artifact', arguments:{path:value.artifact.path,detail:'complete'}});
  expect(complete.isError).not.toBe(true);
  expect(parseEvidence(complete.structuredContent).normalized_result).toEqual(value);
  expect(inspect).toHaveBeenCalledTimes(2);
  console.log(JSON.stringify({case:'complete-control',provider_calls:inspect.mock.calls.length,isError:complete.isError ?? false}));
  inspect.mockImplementationOnce(() => Promise.resolve(err(new AnalysisInputError('inspect_wasm_artifact','synthetic provider failure'))));
  const failed = await client.callTool({name:'inspect_wasm_artifact', arguments:{path:value.artifact.path,detail:'summary'}});
  const failure = parseMcpToolError(failed).error;
  expect(failure.code).toBe('invalid_request');
  expect(inspect).toHaveBeenCalledTimes(3);
  console.log(JSON.stringify({case:'summary-failing-provider',provider_calls:inspect.mock.calls.length,code:failure.code,message:failure.message}));
  await client.ping();
 } finally { await client.close(); await server.close(); await session.close(); }
});

Run from the checkout root with isolated configuration and one worker:

WORK=$(mktemp -d)
mkdir -p "$WORK/home" "$WORK/tmp" "$WORK/config"
env -i PATH="$PATH" HOME="$WORK/home" TMPDIR="$WORK/tmp" XDG_CONFIG_HOME="$WORK/config" node node_modules/vitest/vitest.mjs run --project mcp-boundary tests/boundary/mcp/retention-preflight.test.ts --maxWorkers=1 --no-file-parallelism

One run passed all three boundary observations. These assertions describe current faulty behavior, so a successful test run demonstrates the bug.

Expected behavior

Refuse summary delivery with capability_unavailable before entering the provider when the server cannot retain the original Evidence. The retention helper explicitly documents failure before analysis. The shared summary helper and EVM handler implement this order.

The server constructor explicitly supports the fixed-target seam for embedders as well as tests, and omits recorders without a session.

Impact and scope

Bounded to callers embedding a fixed MCP server without retention and selecting summary delivery. Provider work is unnecessarily entered, and its failure can obscure actionable delivery guidance. Complete delivery was observed to succeed on the same server. No Evidence corruption or security boundary crossing was observed.

NOT_ENOUGH_INFO: real WABT costs, affected deployment frequency, and other platform behavior were not measured. The ordinary session-backed stdio configuration was not shown to be affected.

REA version

Source main 3537e58c1a; package metadata 6.7.0. Relevant registration/composition/service source is unchanged from rea-agents-6.7.0; stable runtime was not tested.

Runtime and platform

Node.js v26.12.0; macOS 27.0.1; arm64; locked Vitest 5.0.3. Node 26 is supported by package engines. The pinned Node 24.18.0 development runtime was not exercised.

Target and artifact context

Synthetic source-owned WASM execution fixture with an eight-byte artifact identity and decoded empty-module WAT. Mocked provider only; fixture paths are labels, with no file acquisition or real provider process.

Interface

MCP

Earliest failure stage

Setup or provider admission

Relevant output or evidence

Observed through in-memory MCP calls:

summary-successful-provider: provider_calls=1, code=capability_unavailable
message: This server does not retain Evidence. Select detail complete to receive the complete analysis inline.
complete-control: provider_calls=2, isError=false
summary-failing-provider: provider_calls=3, code=invalid_request
message: Analysis input is invalid. Check the arguments and try again.
1 test passed

The call counts are cumulative. The first summary alone causes one provider invocation; the later summary invokes it again despite the unchanged missing retention capability.

Typed outcome or recovery expected

capability_unavailable with the existing guidance to select detail complete, and zero provider calls for either summary request. Preserve the successful complete-delivery behavior.

Analysis or suspected cause

SOURCE-ONLY explanation of the observed boundary behavior: the WASM handler awaits service.inspect before conditionally calling summarizeRetainedAnalysis. The summary function then performs the retention check. A failed inspection bypasses that function entirely. No earlier retention guard exists in this handler.

Additional context

Real WABT execution and its cost were not measured. The verified behavior is provider invocation and diagnostic precedence through the supported fixed-server composition. Source comparison with stable found only an unrelated immutable-JSON lookup change in the analysis-view service; release execution remains NOT_ENOUGH_INFO.

Originally created by @coygeek on GitHub (Oct 18, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5100 ## 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 A mocked boundary reproduction through the MCP SDK, production createServer, and WasmArtifactService shows that inspect_wasm_artifact with detail: "summary" invokes the WASM provider on a fixed server without Evidence retention. After a successful provider result it returns capability_unavailable because the server cannot retain Evidence. With a synthetic provider failure, invalid_request is returned instead of the retention diagnostic. Complete delivery succeeds. This confirms provider entry and error precedence. No real WABT toolchain or WASM file was read or executed. ## Reproduction From a source checkout at the revision below with locked dependencies already installed, save this probe as tests/boundary/mcp/retention-preflight.test.ts. It uses the repository's synthetic WASM execution fixture and fixed-server test binding; the MCP registration and application service are production code. ```ts import { Client, InMemoryTransport } from '@modelcontextprotocol/client'; import { expect, it, vi } from 'vitest'; import { WasmArtifactService } from '../../../src/application/wasm/WasmArtifactService.js'; import { AnalysisInputError } from '../../../src/domain/analysisErrorCore.js'; import { err, ok } from '../../../src/domain/result.js'; import { parseEvidence } from '../../../src/domain/evidence.js'; import { createServer } from '../../../src/server/createServer.js'; import { createTestBinarySession } from '../../fixtures/binarySession.js'; import { parseMcpToolError } from '../../fixtures/mcpToolError.js'; import { wasmArtifactFixture, wasmArtifactExecution } from '../../fixtures/wasm/artifact.js'; it('observes retention refusal only after WASM provider entry on a fixed MCP server', async () => { const value = wasmArtifactFixture(); const inspect = vi.fn(() => Promise.resolve(ok(wasmArtifactExecution(value)))); const service = new WasmArtifactService({close: () => Promise.resolve(), inspect}); const session = createTestBinarySession(() => { throw new Error('No native provider expected'); }); const server = createServer({kind:'fixed', analysis:session}, {wasmArtifact:service, environment:{}}); const client = new Client({name:'retention-probe', version:'1'}); const [a,b] = InMemoryTransport.createLinkedPair(); try { await server.connect(b); await client.connect(a); const summary = await client.callTool({name:'inspect_wasm_artifact', arguments:{path:value.artifact.path,detail:'summary'}}); const summaryError = parseMcpToolError(summary).error; expect(summaryError.code).toBe('capability_unavailable'); expect(summaryError.message).toContain('does not retain Evidence'); expect(inspect).toHaveBeenCalledTimes(1); console.log(JSON.stringify({case:'summary-successful-provider',provider_calls:inspect.mock.calls.length,code:summaryError.code,message:summaryError.message})); const complete = await client.callTool({name:'inspect_wasm_artifact', arguments:{path:value.artifact.path,detail:'complete'}}); expect(complete.isError).not.toBe(true); expect(parseEvidence(complete.structuredContent).normalized_result).toEqual(value); expect(inspect).toHaveBeenCalledTimes(2); console.log(JSON.stringify({case:'complete-control',provider_calls:inspect.mock.calls.length,isError:complete.isError ?? false})); inspect.mockImplementationOnce(() => Promise.resolve(err(new AnalysisInputError('inspect_wasm_artifact','synthetic provider failure')))); const failed = await client.callTool({name:'inspect_wasm_artifact', arguments:{path:value.artifact.path,detail:'summary'}}); const failure = parseMcpToolError(failed).error; expect(failure.code).toBe('invalid_request'); expect(inspect).toHaveBeenCalledTimes(3); console.log(JSON.stringify({case:'summary-failing-provider',provider_calls:inspect.mock.calls.length,code:failure.code,message:failure.message})); await client.ping(); } finally { await client.close(); await server.close(); await session.close(); } }); ``` Run from the checkout root with isolated configuration and one worker: ```sh WORK=$(mktemp -d) mkdir -p "$WORK/home" "$WORK/tmp" "$WORK/config" env -i PATH="$PATH" HOME="$WORK/home" TMPDIR="$WORK/tmp" XDG_CONFIG_HOME="$WORK/config" node node_modules/vitest/vitest.mjs run --project mcp-boundary tests/boundary/mcp/retention-preflight.test.ts --maxWorkers=1 --no-file-parallelism ``` One run passed all three boundary observations. These assertions describe current faulty behavior, so a successful test run demonstrates the bug. ## Expected behavior Refuse summary delivery with capability_unavailable before entering the provider when the server cannot retain the original Evidence. The [retention helper](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/application/analysisView/AnalysisViewService.ts#L114-L144) explicitly documents failure before analysis. The [shared summary helper](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/server/retainedSummaryDelivery.ts#L25-L45) and [EVM handler](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/server/registerEvmTools.ts#L41-L50) implement this order. The [server constructor](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/server/createServer.ts#L282-L293) explicitly supports the fixed-target seam for embedders as well as tests, and [omits recorders without a session](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/server/createServer.ts#L796-L812). ## Impact and scope Bounded to callers embedding a fixed MCP server without retention and selecting summary delivery. Provider work is unnecessarily entered, and its failure can obscure actionable delivery guidance. Complete delivery was observed to succeed on the same server. No Evidence corruption or security boundary crossing was observed. NOT_ENOUGH_INFO: real WABT costs, affected deployment frequency, and other platform behavior were not measured. The ordinary session-backed stdio configuration was not shown to be affected. ## REA version Source main 3537e58c1ab29309f06eabf50bc52adba9817a0c; package metadata 6.7.0. Relevant registration/composition/service source is unchanged from rea-agents-6.7.0; stable runtime was not tested. ## Runtime and platform Node.js v26.12.0; macOS 27.0.1; arm64; locked Vitest 5.0.3. Node 26 is supported by package engines. The pinned Node 24.18.0 development runtime was not exercised. ## Target and artifact context Synthetic source-owned WASM execution fixture with an eight-byte artifact identity and decoded empty-module WAT. Mocked provider only; fixture paths are labels, with no file acquisition or real provider process. ## Interface MCP ## Earliest failure stage Setup or provider admission ## Relevant output or evidence Observed through in-memory MCP calls: ```text summary-successful-provider: provider_calls=1, code=capability_unavailable message: This server does not retain Evidence. Select detail complete to receive the complete analysis inline. complete-control: provider_calls=2, isError=false summary-failing-provider: provider_calls=3, code=invalid_request message: Analysis input is invalid. Check the arguments and try again. 1 test passed ``` The call counts are cumulative. The first summary alone causes one provider invocation; the later summary invokes it again despite the unchanged missing retention capability. ## Typed outcome or recovery expected capability_unavailable with the existing guidance to select detail complete, and zero provider calls for either summary request. Preserve the successful complete-delivery behavior. ## Analysis or suspected cause SOURCE-ONLY explanation of the observed boundary behavior: the [WASM handler](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/server/registerWasmTools.ts#L60-L85) awaits service.inspect before conditionally calling summarizeRetainedAnalysis. The [summary function](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/application/analysisView/AnalysisViewService.ts#L151-L165) then performs the retention check. A failed inspection bypasses that function entirely. No earlier retention guard exists in this handler. ## Additional context Real WABT execution and its cost were not measured. The verified behavior is provider invocation and diagnostic precedence through the supported fixed-server composition. Source comparison with stable found only an unrelated immutable-JSON lookup change in the analysis-view service; release execution remains NOT_ENOUGH_INFO.

bsakaguchi closed this issue

2026-10-31 00:37:03 +00:00

bsakaguchi commented

2026-10-31 00:37:05 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 19, 2026):

Good catch — entering the WASM provider before confirming Evidence retention makes the error precedence misleading: a real capability problem surfaces as capability_unavailable only after a successful provider round-trip, and as invalid_request when the provider itself fails, which buries the actual retention diagnostic. Running the retention preflight before any provider call (as the complete-delivery path presumably does) would make both failure modes report the same root cause. The synthetic reproduction is a nice way to keep the test hermetic without a real WABT toolchain.

@XinwangTao commented on GitHub (Oct 19, 2026): Good catch — entering the WASM provider before confirming Evidence retention makes the error precedence misleading: a real capability problem surfaces as capability_unavailable only after a successful provider round-trip, and as invalid_request when the provider itself fails, which buries the actual retention diagnostic. Running the retention preflight before any provider call (as the complete-delivery path presumably does) would make both failure modes report the same root cause. The synthetic reproduction is a nice way to keep the test hermetic without a real WABT toolchain.

bsakaguchi commented

2026-10-31 00:37:05 +00:00

Author

Owner

@akushonkamen commented on GitHub (Oct 19, 2026):

Claiming this — I'll add the retention preflight before the provider call in the wasm tool handler, with a hermetic mocked-boundary regression test (summary+fixed → capability_unavailable with zero provider calls).

@akushonkamen commented on GitHub (Oct 19, 2026): Claiming this — I'll add the retention preflight before the provider call in the wasm tool handler, with a hermetic mocked-boundary regression test (summary+fixed → `capability_unavailable` with zero provider calls).

bsakaguchi commented

2026-10-31 00:37:05 +00:00

Author

Owner

@akushonkamen commented on GitHub (Oct 19, 2026):

Claimed and fixed on akushonkamen/rea branch fix/5100-wasm-retention-preflight (no PR opened yet — Author readiness fields are left for the branch owner to fill in).

Fix (src/server/registerWasmTools.ts): the inspect_wasm_artifact handler now runs requireAnalysisViewRetention before service.inspect when detail === "summary", matching the EVM handler order. Summary on a server without Evidence retention is refused with capability_unavailable and zero provider calls; complete delivery and post-preflight provider failures keep their original semantics.

Regression test (tests/boundary/mcp/retentionPreflightMcp.test.ts, mocked provider + production createServer, no WABT):

Red (before fix): expect(inspect).toHaveBeenCalledTimes(0) failed — the provider had already been invoked for the summary request.
Green (after fix): npx vitest run --project mcp-boundary tests/boundary/mcp/retentionPreflightMcp.test.ts — 1 passed (zero provider calls for summary, complete delivery succeeds, provider failure on complete still reports invalid_request).
Baselines: src/application/wasm/WasmArtifactService.test.ts 12 passed; wasmArtifactMcp.test.ts + analysisViewMcp.test.ts 31 passed. A full --project mcp-boundary run shows 63 pre-existing failures that reproduce identically on clean origin/main without this change (verified by stashing the fix), so they are unrelated environment issues.

@akushonkamen commented on GitHub (Oct 19, 2026): Claimed and fixed on `akushonkamen/rea` branch [`fix/5100-wasm-retention-preflight`](https://github.com/akushonkamen/rea/commits/fix/5100-wasm-retention-preflight) (no PR opened yet — Author readiness fields are left for the branch owner to fill in). **Fix** (`src/server/registerWasmTools.ts`): the `inspect_wasm_artifact` handler now runs `requireAnalysisViewRetention` before `service.inspect` when `detail === "summary"`, matching the EVM handler order. Summary on a server without Evidence retention is refused with `capability_unavailable` and zero provider calls; complete delivery and post-preflight provider failures keep their original semantics. **Regression test** (`tests/boundary/mcp/retentionPreflightMcp.test.ts`, mocked provider + production `createServer`, no WABT): - Red (before fix): `expect(inspect).toHaveBeenCalledTimes(0)` failed — the provider had already been invoked for the summary request. - Green (after fix): `npx vitest run --project mcp-boundary tests/boundary/mcp/retentionPreflightMcp.test.ts` — 1 passed (zero provider calls for summary, complete delivery succeeds, provider failure on complete still reports `invalid_request`). - Baselines: `src/application/wasm/WasmArtifactService.test.ts` 12 passed; `wasmArtifactMcp.test.ts` + `analysisViewMcp.test.ts` 31 passed. A full `--project mcp-boundary` run shows 63 pre-existing failures that reproduce identically on clean `origin/main` without this change (verified by stashing the fix), so they are unrelated environment issues.

bsakaguchi referenced this issue

2026-10-31 00:52:22 +00:00

[GH-ISSUE #8116] [Bug] Legacy Mach-O rebase decoder does not wrap its cursor at 64 bits, losing slots after a backward ULEB step #4679

bsakaguchi referenced this issue

2026-10-31 01:08:08 +00:00

[PR #3138] [CLOSED] fix(native): keep decoding legacy Mach-O binds past a negative skip #5635