fix(signals): reject missing sigrok channels

Recognize the exact selected-channel diagnostic even when sigrok-cli exits successfully, while preserving partial producer output in the invalid-input details. Cover both public JSON adapters and add the missing-channel case to the Linux real-tool verifier.

Fixes #6631
This commit is contained in:
morluto
2026-10-23 18:40:48 +08:00
parent 80d3fd40f1
commit 48cd33af18
5 changed files with 144 additions and 4 deletions
+6 -1
View File
@@ -31,6 +31,10 @@ The corresponding MCP request is:
`decoders` uses sigrok's decoder stack and option/channel syntax. Channel names
must match the recording; REA does not guess wiring, baud rate or protocol.
When sigrok reports that a selected channel name is missing, REA returns
`invalid_request` for `decoders` even if sigrok continues with fallback
annotations and exits successfully. The diagnostic and any partial stdout remain
in `captured_output`; unrelated producer warnings do not change request status.
Omitting `input_format` requests sigrok's autodetection. REA uses an immutable
private copy whose filename retains the selected recording's final suffix, so
filename-based format detection receives that hint without exposing the
@@ -95,7 +99,8 @@ REA_SIGROK_COMMAND=/absolute/path/to/sigrok-cli node scripts/verify-signal-recor
The verifier synthesizes a 9600-baud 8N1 UART VCD carrying `REA`, then checks
actual CLI/MCP parity, advertised schemas, stored Evidence, source hashes,
annotation filtering/autodetection, invalid decoder/input failures and cleanup.
annotation filtering/autodetection, invalid decoder/input failures, missing
channel rejection with retained partial output, and cleanup.
An optional positional REA entrypoint verifies an installed package.
The real-tool lane requires Linux, sigrok-cli 0.7.2 and libsigrokdecode. A separate
kernel-limit regression checks the inherited soft/hard limits and verifies an
+28 -2
View File
@@ -40,7 +40,12 @@ const environment = {
};
const client = new Client({ name: "signal-recording-verifier", version: "1" });
// Real sigrok refusals of an unknown decoder ID must name the caller's field.
const assertRejectedSelector = (response, operation, field) => {
const assertRejectedSelector = (
response,
operation,
field,
expectedExitCode = 1,
) => {
assert.equal(response.isError, true);
const text = response.content.find((item) => item.type === "text")?.text;
const { error } = JSON.parse(text ?? "{}");
@@ -50,7 +55,8 @@ const assertRejectedSelector = (response, operation, field) => {
error.details.issues.map((issue) => issue.path),
[[field]],
);
assert.equal(error.details.captured_output.exit_code, 1);
assert.equal(error.details.captured_output.exit_code, expectedExitCode);
return error;
};
const transport = new StdioClientTransport({
command: process.execPath,
@@ -174,6 +180,26 @@ try {
assert.equal(await readFile(path, "utf8"), source);
cases++;
}
const missingChannel = await client.callTool({
name: advertised.name,
arguments: {
path,
decoders: "uart:rx=REA_NO_SUCH_CHANNEL:baudrate=9600",
input_format: "vcd",
},
});
const channelError = assertRejectedSelector(
missingChannel,
advertised.name,
"decoders",
0,
);
assert.match(
channelError.details.captured_output.stderr,
/cli: No channel with name "REA_NO_SUCH_CHANNEL" found\./u,
);
assert.match(channelError.details.captured_output.stdout, /uart-1:/u);
cases++;
const pagedLines = lines.slice(0, 7);
let pagedTick = 1000;
for (const byte of Array.from({ length: 120 }, (_, i) => i)) {
+3 -1
View File
@@ -242,8 +242,10 @@ export class SigrokRecordingProvider implements SignalRecordingPort {
);
phase = previousPhase;
resourcePath = "path" in input ? input.path : command;
// Some selected-channel refusals are diagnostics, not process failures:
// sigrok can emit fallback annotations and still exit successfully.
const rejection =
output.exitCode === 0 || selection === undefined
selection === undefined
? undefined
: parseSigrokSelectorRejection(output.stderr.text, selection);
if (rejection !== undefined)
+32
View File
@@ -25,6 +25,8 @@ const DECODE_RECOVERY =
"Use inspect_signal_decoders to list installed decoder IDs, or pass a decoder ID to show its options and annotation classes.";
const HELP_RECOVERY =
"Omit decoder to list installed decoder IDs, optionally filtered by query, then request help for one of those IDs.";
const CHANNEL_RECOVERY =
"Choose a channel present in the recording; inspect_signal_decoders shows the selected decoder's channel IDs.";
/**
* sigrok-cli loads selected decoders before decoding or showing help, so an
@@ -46,6 +48,30 @@ const unknownDecoderRefusal = (
: undefined;
};
/**
* The sigrok-cli 0.7.2 channel mapper reports a missing target as a plain
* stderr line, even though it continues with the other channel mappings. It
* does not include the decoder channel key, so compare the reported target
* with exact values from the caller's assignments rather than searching the
* selector text for a substring.
*/
const missingSelectedChannelRefusal = (
lines: ReadonlySet<string>,
decoders: string,
): string | undefined => {
const selectedValues = new Set<string>();
for (const decoder of decoders.split(","))
for (const assignment of decoder.split(":").slice(1)) {
const separator = assignment.indexOf("=");
if (separator > 0) selectedValues.add(assignment.slice(separator + 1));
}
for (const channel of selectedValues) {
const producerMessage = `No channel with name "${channel}" found.`;
if (lines.has(`cli: ${producerMessage}`)) return producerMessage;
}
return undefined;
};
/**
* Recognize only observed sigrok-cli 0.7.2 selector refusals whose rejected
* name equals a caller-supplied selector; other failures stay unclassified.
@@ -70,6 +96,12 @@ export const parseSigrokSelectorRejection = (
if (refusal !== undefined)
return rejectedSelector("decoders", refusal, DECODE_RECOVERY);
}
const missingChannel = missingSelectedChannelRefusal(
lines,
selection.decoders,
);
if (missingChannel !== undefined)
return rejectedSelector("decoders", missingChannel, CHANNEL_RECOVERY);
// Annotation selections are "decoder=class:class,decoder".
for (const entry of selection.annotations?.split(",") ?? []) {
const [decoder = "", classes, ...rest] = entry.split("=");
@@ -45,6 +45,8 @@ else if (${JSON.stringify(mode)} === 'nonzero') { console.error('invalid decoder
else if (${JSON.stringify(mode)} === 'unknown-decoder') { console.error("srd: ModuleNotFoundError: Failed to load decoder nosuchdecoder: import by name failed: No module named 'nosuchdecoder'\\nFailed to load protocol decoder nosuchdecoder."); process.exitCode = 1; }
else if (${JSON.stringify(mode)} === 'broken-decoder') { console.error("srd: ModuleNotFoundError: Failed to load decoder uart: import by name failed: No module named 'serial'\\nFailed to load protocol decoder uart."); process.exitCode = 1; }
else if (${JSON.stringify(mode)} === 'unknown-annotation') { console.error("Annotation 'nosuchclass' not found for protocol decoder 'uart'."); process.exitCode = 1; }
else if (${JSON.stringify(mode)} === 'unknown-channel') { process.stdout.write('1000-1105 uart-1: Start bit\\n'); console.error('cli: No channel with name "NONEXISTENT" found.'); }
else if (${JSON.stringify(mode)} === 'other-channel-warning') { process.stdout.write('1000-1105 uart-1: Start bit\\n'); console.error('cli: No channel with name "OTHER" found.'); }
else if (${JSON.stringify(mode)} === 'malformed-copy') { chmodSync(${JSON.stringify(snapshotName)}, 0o600); writeFileSync(${JSON.stringify(snapshotName)}, 'changed'); }
else if (${JSON.stringify(mode)} === 'oversized-copy') { chmodSync(${JSON.stringify(snapshotName)}, 0o600); truncateSync(${JSON.stringify(snapshotName)}, 32 * 1024 * 1024 + 1); }
else if (${JSON.stringify(mode)} === 'line-budget') process.stdout.write('ID: uart' + '\\n'.repeat(65536));
@@ -334,6 +336,79 @@ it.each([
},
);
it("projects sigrok's zero-exit missing-channel refusal through CLI and MCP with partial output", async () => {
const f = await fixture("unknown-channel");
const refusal = 'No channel with name "NONEXISTENT" found.';
const message = `sigrok-cli rejected the decoders selector: ${refusal}`;
const harness = await createApplicationMcpHarness({
serverOptions: { environment: {}, signalRecording: f.service },
});
onTestFinished(harness.close);
const { error } = parseMcpToolError(
await harness.client.callTool({
name: "decode_signal_recording",
arguments: {
path: f.path,
decoders: "uart:rx=NONEXISTENT:baudrate=9600",
},
}),
);
expect(error).toMatchObject({
code: "invalid_request",
category: "invalid_input",
message: expect.stringContaining(message),
details: {
issues: [{ path: ["decoders"], reason: "invalid_value" }],
captured_output: {
exit_code: 0,
stdout: "1000-1105 uart-1: Start bit\n",
stderr: expect.stringContaining(refusal),
},
},
});
const cli = Cli.create("signal-channel-refusal-test", { sync: false });
registerSignalCommands(cli, silentLogger, {}, f.service);
let stdout = "";
await cli.serve(
[
"decode-signal-recording",
f.path,
"--decoders",
"uart:rx=NONEXISTENT:baudrate=9600",
"--format",
"json",
],
{
stdout: (text) => {
stdout += text;
},
exit: () => {},
},
);
expect(JSON.parse(stdout)).toMatchObject({
code: "invalid_request",
details: {
issues: [{ path: ["decoders"], reason: "invalid_value" }],
captured_output: {
exit_code: 0,
stdout: "1000-1105 uart-1: Start bit\n",
stderr: expect.stringContaining(refusal),
},
},
});
});
it("does not match channel refusals by substring", async () => {
const f = await fixture("other-channel-warning");
const result = await f.service.decode({
path: f.path,
decoders: "uart:rx=RX:baudrate=9600",
});
expect(result.ok).toBe(true);
});
it("reports an unknown decoder ID as the rejected field of each MCP operation", async () => {
const f = await fixture("unknown-decoder");
const harness = await createApplicationMcpHarness({