morluto/rea

Fork 0

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

[GH-ISSUE #5849] [Bug] Decoder help reports an explicit unknown-ID refusal as execution_failure #3545

New issue

Closed

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

bsakaguchi commented

2026-10-31 00:41:42 +00:00

Owner

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

Originally assigned to: @kaoru0822-kitauji on GitHub.

Before submitting

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

Area

Provider or capability discovery

Observed behavior

Reproduced through the production in-memory MCP registration with a simulated sigrok command, not a real sigrok installation. For inspect_signal_decoders with {"decoder":"nosuchdecoder"}, a producer exit of 1 with both missing-decoder diagnostics below returns code: "execution_failure" and directs the caller to rea doctor. It provides no input issue identifying decoder as the rejected field.

Sending the same simulated refusal through decode_signal_recording returns invalid_request, identifies decoders, and directs the caller to decoder discovery. Both operations retain the producer's stderr. Catalog search and help for the fixture's installed uart decoder succeeded, and the owned workspaces were removed.

Reproduction

Executed once on the source revision below, with installed locked dependencies. Add the following temporary source test as src/repro.test.ts. It follows the existing command simulator in tests/boundary/signals/recordingProvider.test.ts and sends requests through the repository's MCP harness.

The producer emits the complete two-line refusal already recognized by the decode classifier. The test injects Linux admission and a limit reader, and launches a Node child in place of sigrok/prlimit. It exercises real process ownership, executable identity checks, service validation, MCP error projection, and cleanup. It does not exercise real Linux resource enforcement or establish real sigrok's --show diagnostic wording.

import { it, expect, onTestFinished } from "vitest";
import { chmod, readdir, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { SigrokRecordingProvider } from "./signals/SigrokRecordingProvider.js";
import { SignalRecordingService } from "./application/signals/SignalRecordingService.js";
import { PrivateRuntimeRoot } from "./process/PrivateRuntimeRoot.js";
import { spawnOwnedProviderProcess } from "./process/ProviderProcess.js";
import { readStableArtifact } from "./artifacts/readStableArtifact.js";
import { createTestTempDirectory } from "../tests/fixtures/temporaryDirectory.js";
import { createApplicationMcpHarness } from "../tests/fixtures/applicationMcpHarness.js";
import { parseMcpToolError } from "../tests/fixtures/mcpToolError.js";

it("compares the same producer refusal through discovery and decoding MCP calls", async () => {
  const directory = await createTestTempDirectory("rea-signal-repro-");
  const script = join(directory, "producer.mjs");
  const recording = join(directory, "recording.vcd");
  const refusal = "srd: ModuleNotFoundError: Failed to load decoder nosuchdecoder: import by name failed: No module named 'nosuchdecoder'\nFailed to load protocol decoder nosuchdecoder.\n";
  await writeFile(recording, "synthetic recording");
  await writeFile(script, `
if (process.argv.includes('--version')) console.log('sigrok-cli 0.7.2\\nlibsigrokdecode 0.5.3');
else if (process.argv.includes('--list-supported')) console.log('Supported protocol decoders:\\n  uart                 UART\\n');
else if (process.argv.includes('--protocol-decoders=nosuchdecoder')) { process.stderr.write(${JSON.stringify(refusal)}); process.exitCode = 1; }
else if (process.argv.includes('--show')) console.log('ID: uart\\nOptions:');
else throw Error('unexpected fixture command');
`);
  await chmod(script, 0o700);
  const requests: string[][] = [];
  const provider = new SigrokRecordingProvider(
    { ...process.env, REA_SIGROK_COMMAND: script, REA_SIGROK_PRLIMIT_COMMAND: script },
    async (input) => {
      requests.push([...input.arguments]);
      return spawnOwnedProviderProcess({ ...input, command: process.execPath,
        arguments: [script, ...input.arguments], expectedCommand: process.execPath });
    },
    () => PrivateRuntimeRoot.create({ parent: directory, prefix: "decoder-" }),
    "linux", readStableArtifact, async (limits) => limits,
  );
  onTestFinished(() => provider.close());
  const harness = await createApplicationMcpHarness({ serverOptions: {
    environment: {}, signalRecording: new SignalRecordingService(provider),
  } });
  onTestFinished(harness.close);
  for (const args of [{ query: "nosuchdecoder" }, { decoder: "uart" }]) {
    const response = await harness.client.callTool({ name: "inspect_signal_decoders", arguments: args });
    expect(response.isError).not.toBe(true);
  }
  const discovery = parseMcpToolError(await harness.client.callTool({
    name: "inspect_signal_decoders", arguments: { decoder: "nosuchdecoder" },
  })).error;
  const decode = parseMcpToolError(await harness.client.callTool({
    name: "decode_signal_recording", arguments: { path: recording, decoders: "nosuchdecoder" },
  })).error;
  expect(discovery).toMatchObject({ code: "execution_failure", details: {
    operation: "inspect_signal_decoders", diagnostics: { exit_code: 1, stderr: refusal },
  } });
  expect(discovery.message).toContain("Run `rea doctor`");
  expect(decode).toMatchObject({ code: "invalid_request", details: {
    operation: "decode_signal_recording", issues: [{ path: ["decoders"], reason: "invalid_value" }],
    captured_output: { exit_code: 1, stderr: refusal },
  } });
  expect(requests.some((args) => args.includes("--show") && args.includes("--protocol-decoders=nosuchdecoder"))).toBe(true);
  expect((await readdir(directory)).filter((name) => name.startsWith("decoder-"))).toEqual([]);
  console.log(JSON.stringify({ discovery, decode, controls: "catalog and installed help succeeded; workspaces removed" }, null, 2));
});

Run the test with isolated configuration and temporary-file directories, then delete the temporary test:

repro_root="$(mktemp -d)"
mkdir -p "$repro_root"/{home,config,data,cache,state,tmp}
env HOME="$repro_root/home" XDG_CONFIG_HOME="$repro_root/config" \
  XDG_DATA_HOME="$repro_root/data" XDG_CACHE_HOME="$repro_root/cache" \
  XDG_STATE_HOME="$repro_root/state" TMPDIR="$repro_root/tmp" \
  npm run test:local -- src/repro.test.ts
rm src/repro.test.ts

The recorded run used a verifier-specific temporary source filename; the body above is the executed test. No checkout implementation or test configuration changed.

Expected behavior

An explicit producer refusal establishing that the caller-selected decoder ID is absent should be a field-specific invalid_request, with a correction path to the installed catalog. An installed decoder failing because one of its own dependencies is absent should remain a provider failure.

This follows the repository requirement to model expected failures with the tagged error algebra and preserve meaningful reasons (AGENTS.md), the input-error and field-correction model, and the documented exact decoder selection and catalog lookup. The existing sigrok classifier establishes the narrow two-line diagnostic that distinguishes those cases; a single generic load-failure line is insufficient.

Impact and scope

P2: a bounded error-classification and recovery defect for rejected decoder-help selections. The observed MCP result misidentifies caller input as an execution failure and omits the rejected-field correction metadata. A practical recovery path exists: omit decoder to inspect the installed catalog and select an installed ID. The fixture's catalog search and installed-ID help succeeded.

No evidence corruption, recording mutation, leaked workspace, or security boundary crossing was observed. Successful discovery is unaffected in the tested controls. Real sigrok behavior, frequency across installations, and CLI projection were not executed; the demonstrated failure is REA's response to the simulated explicit refusal.

REA version

rea-agents 6.7.0; source revision a5f982de8d.

Runtime and platform

Node.js v26.12.0; macOS 27.0.1 arm64; production in-memory MCP harness with Linux platform/limit-reader injection and a Node command simulator.

Target and artifact context

No active binary target or hardware; synthetic recording bytes for the decode control and a fixture catalog containing uart; no real sigrok installation used.

Interface

MCP

Earliest failure stage

Transport or result projection

Relevant output or evidence

Trimmed from the executed test's logged MCP errors; omitted fields do not affect the comparison:

{
  "discovery": {
    "code": "execution_failure",
    "category": "execution_failure",
    "message": "Analysis could not complete. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.",
    "retryable": false,
    "details": {
      "provider_id": "sigrok-recording",
      "operation": "inspect_signal_decoders",
      "diagnostics": {
        "exit_code": 1,
        "stdout": "",
        "stderr": "srd: ModuleNotFoundError: Failed to load decoder nosuchdecoder: import by name failed: No module named 'nosuchdecoder'\nFailed to load protocol decoder nosuchdecoder.\n"
      }
    }
  },
  "decode": {
    "code": "invalid_request",
    "category": "invalid_input",
    "details": {
      "operation": "decode_signal_recording",
      "issues": [{ "path": ["decoders"], "reason": "invalid_value" }],
      "captured_output": { "exit_code": 1 }
    }
  },
  "controls": "catalog and installed help succeeded; workspaces removed"
}

Test Files  1 passed (1)
     Tests  1 passed (1)

Source trace at the tested revision:

The production MCP handler reaches the shared service and provider; a syntactically valid absent ID passes the input schema.
Discovery invokes execute without a rejection callback. The nonzero-exit branch consequently selects ProviderAdapterError.
Decoding supplies the rejection callback. The projection maps the two error classes to different codes, and the adapter-failure presentation selects the doctor advice.

Typed outcome or recovery expected

For the matched missing-ID refusal, invalid_request with category: "invalid_input", an issue on ["decoder"] with reason: "invalid_value", retained output, and guidance to omit decoder to list installed IDs or select an installed ID. Unrelated nonzero exits should retain their existing provider-failure classification.

Analysis or suspected cause

Source-derived: discovery and decoding share the command executor, but only decoding supplies the selector-refusal callback. The current classifier's input type contains decoders and annotations, so it cannot directly identify the singular discovery field. Recognizing the equivalent narrowly matched discovery refusal at that provider boundary would address the demonstrated case; real --show output should be captured before choosing production match strings.

Additional context

Searched open and closed issues and PRs with inspect_signal_decoders, sigrok decoder refusal, sigrok invalid_request, decoder discovery, and sigrok selector. The closest history is issue #5515, addressed by merged PR #5570. Both concern decoding; the merged fix explicitly runs recognition only for decode commands. No report for this discovery refusal was found.

No patch or restoration was tested. NOT_ENOUGH_INFO: first affected revision and real sigrok-cli --show refusal transcript are unavailable.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5849 Originally assigned to: @kaoru0822-kitauji on GitHub. ### 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 Provider or capability discovery ### Observed behavior Reproduced through the production in-memory MCP registration with a simulated sigrok command, not a real sigrok installation. For `inspect_signal_decoders` with `{"decoder":"nosuchdecoder"}`, a producer exit of 1 with both missing-decoder diagnostics below returns `code: "execution_failure"` and directs the caller to `rea doctor`. It provides no input issue identifying `decoder` as the rejected field. Sending the same simulated refusal through `decode_signal_recording` returns `invalid_request`, identifies `decoders`, and directs the caller to decoder discovery. Both operations retain the producer's stderr. Catalog search and help for the fixture's installed `uart` decoder succeeded, and the owned workspaces were removed. ### Reproduction Executed once on the source revision below, with installed locked dependencies. Add the following temporary source test as `src/repro.test.ts`. It follows the existing command simulator in `tests/boundary/signals/recordingProvider.test.ts` and sends requests through the repository's MCP harness. The producer emits the complete two-line refusal already recognized by the decode classifier. The test injects Linux admission and a limit reader, and launches a Node child in place of sigrok/prlimit. It exercises real process ownership, executable identity checks, service validation, MCP error projection, and cleanup. It does not exercise real Linux resource enforcement or establish real sigrok's `--show` diagnostic wording. ```ts import { it, expect, onTestFinished } from "vitest"; import { chmod, readdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { SigrokRecordingProvider } from "./signals/SigrokRecordingProvider.js"; import { SignalRecordingService } from "./application/signals/SignalRecordingService.js"; import { PrivateRuntimeRoot } from "./process/PrivateRuntimeRoot.js"; import { spawnOwnedProviderProcess } from "./process/ProviderProcess.js"; import { readStableArtifact } from "./artifacts/readStableArtifact.js"; import { createTestTempDirectory } from "../tests/fixtures/temporaryDirectory.js"; import { createApplicationMcpHarness } from "../tests/fixtures/applicationMcpHarness.js"; import { parseMcpToolError } from "../tests/fixtures/mcpToolError.js"; it("compares the same producer refusal through discovery and decoding MCP calls", async () => { const directory = await createTestTempDirectory("rea-signal-repro-"); const script = join(directory, "producer.mjs"); const recording = join(directory, "recording.vcd"); const refusal = "srd: ModuleNotFoundError: Failed to load decoder nosuchdecoder: import by name failed: No module named 'nosuchdecoder'\nFailed to load protocol decoder nosuchdecoder.\n"; await writeFile(recording, "synthetic recording"); await writeFile(script, ` if (process.argv.includes('--version')) console.log('sigrok-cli 0.7.2\\nlibsigrokdecode 0.5.3'); else if (process.argv.includes('--list-supported')) console.log('Supported protocol decoders:\\n uart UART\\n'); else if (process.argv.includes('--protocol-decoders=nosuchdecoder')) { process.stderr.write(${JSON.stringify(refusal)}); process.exitCode = 1; } else if (process.argv.includes('--show')) console.log('ID: uart\\nOptions:'); else throw Error('unexpected fixture command'); `); await chmod(script, 0o700); const requests: string[][] = []; const provider = new SigrokRecordingProvider( { ...process.env, REA_SIGROK_COMMAND: script, REA_SIGROK_PRLIMIT_COMMAND: script }, async (input) => { requests.push([...input.arguments]); return spawnOwnedProviderProcess({ ...input, command: process.execPath, arguments: [script, ...input.arguments], expectedCommand: process.execPath }); }, () => PrivateRuntimeRoot.create({ parent: directory, prefix: "decoder-" }), "linux", readStableArtifact, async (limits) => limits, ); onTestFinished(() => provider.close()); const harness = await createApplicationMcpHarness({ serverOptions: { environment: {}, signalRecording: new SignalRecordingService(provider), } }); onTestFinished(harness.close); for (const args of [{ query: "nosuchdecoder" }, { decoder: "uart" }]) { const response = await harness.client.callTool({ name: "inspect_signal_decoders", arguments: args }); expect(response.isError).not.toBe(true); } const discovery = parseMcpToolError(await harness.client.callTool({ name: "inspect_signal_decoders", arguments: { decoder: "nosuchdecoder" }, })).error; const decode = parseMcpToolError(await harness.client.callTool({ name: "decode_signal_recording", arguments: { path: recording, decoders: "nosuchdecoder" }, })).error; expect(discovery).toMatchObject({ code: "execution_failure", details: { operation: "inspect_signal_decoders", diagnostics: { exit_code: 1, stderr: refusal }, } }); expect(discovery.message).toContain("Run `rea doctor`"); expect(decode).toMatchObject({ code: "invalid_request", details: { operation: "decode_signal_recording", issues: [{ path: ["decoders"], reason: "invalid_value" }], captured_output: { exit_code: 1, stderr: refusal }, } }); expect(requests.some((args) => args.includes("--show") && args.includes("--protocol-decoders=nosuchdecoder"))).toBe(true); expect((await readdir(directory)).filter((name) => name.startsWith("decoder-"))).toEqual([]); console.log(JSON.stringify({ discovery, decode, controls: "catalog and installed help succeeded; workspaces removed" }, null, 2)); }); ``` Run the test with isolated configuration and temporary-file directories, then delete the temporary test: ```sh repro_root="$(mktemp -d)" mkdir -p "$repro_root"/{home,config,data,cache,state,tmp} env HOME="$repro_root/home" XDG_CONFIG_HOME="$repro_root/config" \ XDG_DATA_HOME="$repro_root/data" XDG_CACHE_HOME="$repro_root/cache" \ XDG_STATE_HOME="$repro_root/state" TMPDIR="$repro_root/tmp" \ npm run test:local -- src/repro.test.ts rm src/repro.test.ts ``` The recorded run used a verifier-specific temporary source filename; the body above is the executed test. No checkout implementation or test configuration changed. ### Expected behavior An explicit producer refusal establishing that the caller-selected decoder ID is absent should be a field-specific `invalid_request`, with a correction path to the installed catalog. An installed decoder failing because one of its own dependencies is absent should remain a provider failure. This follows the repository requirement to model expected failures with the tagged error algebra and preserve meaningful reasons ([AGENTS.md](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/AGENTS.md#L31-L31)), the [input-error and field-correction model](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/domain/analysisErrorCore.ts#L64-L94), and the documented [exact decoder selection and catalog lookup](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/signal-recordings.md#L121-L128). The [existing sigrok classifier](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/signals/sigrokSelectorRejection.ts#L17-L39) establishes the narrow two-line diagnostic that distinguishes those cases; a single generic load-failure line is insufficient. ### Impact and scope P2: a bounded error-classification and recovery defect for rejected decoder-help selections. The observed MCP result misidentifies caller input as an execution failure and omits the rejected-field correction metadata. A practical recovery path exists: omit `decoder` to inspect the installed catalog and select an installed ID. The fixture's catalog search and installed-ID help succeeded. No evidence corruption, recording mutation, leaked workspace, or security boundary crossing was observed. Successful discovery is unaffected in the tested controls. Real sigrok behavior, frequency across installations, and CLI projection were not executed; the demonstrated failure is REA's response to the simulated explicit refusal. ### REA version rea-agents 6.7.0; source revision a5f982de8dbe4b092cc20ab89d492b7e7028b758. ### Runtime and platform Node.js v26.12.0; macOS 27.0.1 arm64; production in-memory MCP harness with Linux platform/limit-reader injection and a Node command simulator. ### Target and artifact context No active binary target or hardware; synthetic recording bytes for the decode control and a fixture catalog containing uart; no real sigrok installation used. ### Interface MCP ### Earliest failure stage Transport or result projection ### Relevant output or evidence Trimmed from the executed test's logged MCP errors; omitted fields do not affect the comparison: ```json { "discovery": { "code": "execution_failure", "category": "execution_failure", "message": "Analysis could not complete. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.", "retryable": false, "details": { "provider_id": "sigrok-recording", "operation": "inspect_signal_decoders", "diagnostics": { "exit_code": 1, "stdout": "", "stderr": "srd: ModuleNotFoundError: Failed to load decoder nosuchdecoder: import by name failed: No module named 'nosuchdecoder'\nFailed to load protocol decoder nosuchdecoder.\n" } } }, "decode": { "code": "invalid_request", "category": "invalid_input", "details": { "operation": "decode_signal_recording", "issues": [{ "path": ["decoders"], "reason": "invalid_value" }], "captured_output": { "exit_code": 1 } } }, "controls": "catalog and installed help succeeded; workspaces removed" } ``` ```text Test Files 1 passed (1) Tests 1 passed (1) ``` Source trace at the tested revision: - The [production MCP handler](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/server/registerSignalTools.ts#L31-L43) reaches the shared service and provider; a syntactically valid absent ID passes the [input schema](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/domain/signals/signalDecoders.ts#L4-L13). - [Discovery invokes execute without a rejection callback](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/signals/SigrokRecordingProvider.ts#L286-L292). The [nonzero-exit branch](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/signals/SigrokRecordingProvider.ts#L234-L255) consequently selects `ProviderAdapterError`. - [Decoding supplies the rejection callback](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/signals/SigrokRecordingProvider.ts#L333-L335). The [projection maps the two error classes to different codes](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/domain/analysisErrorProjection.ts#L242-L262), and the [adapter-failure presentation selects the doctor advice](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/domain/analysisErrorPresentation.ts#L383-L384). ### Typed outcome or recovery expected For the matched missing-ID refusal, `invalid_request` with `category: "invalid_input"`, an issue on `["decoder"]` with `reason: "invalid_value"`, retained output, and guidance to omit `decoder` to list installed IDs or select an installed ID. Unrelated nonzero exits should retain their existing provider-failure classification. ### Analysis or suspected cause Source-derived: discovery and decoding share the command executor, but only decoding supplies the selector-refusal callback. The current classifier's input type contains `decoders` and `annotations`, so it cannot directly identify the singular discovery field. Recognizing the equivalent narrowly matched discovery refusal at that provider boundary would address the demonstrated case; real `--show` output should be captured before choosing production match strings. ### Additional context Searched open and closed issues and PRs with `inspect_signal_decoders`, `sigrok decoder refusal`, `sigrok invalid_request`, `decoder discovery`, and `sigrok selector`. The closest history is [issue #5515](https://github.com/morluto/rea/issues/5515), addressed by [merged PR #5570](https://github.com/morluto/rea/pull/5570). Both concern decoding; the merged fix explicitly runs recognition only for decode commands. No report for this discovery refusal was found. No patch or restoration was tested. NOT_ENOUGH_INFO: first affected revision and real sigrok-cli `--show` refusal transcript are unavailable.

bsakaguchi closed this issue

2026-10-31 00:41:42 +00:00

bsakaguchi commented

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

Author

Owner

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

Confirmed the inconsistency: the same producer refusal (unknown decoder nosuchdecoder) is classified as invalid_request with a field-level decoders hint through decode_signal_recording, but execution_failure + doctor advice through inspect_signal_decoders. Since the discovery path already has catalog lookup and help for installed decoders, routing an unknown-ID refusal to the input-error class (with decoder as the rejected field) would make the two operations consistent and give callers an actionable recovery path instead of a doctor hint. The provided repro test is a solid baseline for asserting this behavior.

@XinwangTao commented on GitHub (Oct 21, 2026): Confirmed the inconsistency: the same producer refusal (unknown decoder `nosuchdecoder`) is classified as `invalid_request` with a field-level `decoders` hint through decode_signal_recording, but `execution_failure` + doctor advice through inspect_signal_decoders. Since the discovery path already has catalog lookup and help for installed decoders, routing an unknown-ID refusal to the input-error class (with `decoder` as the rejected field) would make the two operations consistent and give callers an actionable recovery path instead of a doctor hint. The provided repro test is a solid baseline for asserting this behavior.

bsakaguchi referenced this issue

2026-10-31 01:09:38 +00:00

[PR #3545] [CLOSED] fix(javascript): explain parent budget exhaustion after a static checkpoint #5815

bsakaguchi referenced this issue

2026-10-31 01:09:39 +00:00

[PR #3550] [CLOSED] fix(javascript): keep parent budget exhaustion from advising more memory #5816