Skip to content

Harden sample replay JSON-RPC framing and move shim logs off stdout#37528

Merged
dsyme merged 4 commits into
mainfrom
copilot/fix-apply-samples-crash
Jun 7, 2026
Merged

Harden sample replay JSON-RPC framing and move shim logs off stdout#37528
dsyme merged 4 commits into
mainfrom
copilot/fix-apply-samples-crash

Conversation

Copilot AI commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

apply_samples.cjs could crash during sample replay when the safe-outputs MCP path emitted [debug] ... lines on stdout, causing sendJsonRpc to parse non-JSON text as a protocol frame. This blocked --use-samples replay for PR-creating tools.

  • Protocol-safe logging for shimmed actions context

    • Updated actions/setup/js/shim.cjs so shimmed core.* logging writes to stderr via process.stderr.write(...) instead of console stdout paths.
    • Keeps MCP server stdout reserved for JSON-RPC frames.
  • Defensive JSON-RPC parsing in replay driver

    • Updated actions/setup/js/apply_samples.cjs::sendJsonRpc to read line-by-line until a JSON object frame is found.
    • Non-JSON stdout lines are ignored; malformed JSON frames now raise a targeted parse error including request id/context.
  • Focused regression coverage

    • Added sendJsonRpc unit tests in actions/setup/js/apply_samples.test.cjs for:
      • skipping interleaved debug lines before a valid JSON-RPC response
      • clear failure on malformed JSON-looking protocol lines
// apply_samples.cjs
while (true) {
  const { value, done } = await responseIterator.next();
  if (done) throw new Error(`...request id=${request.id}`);
  const line = typeof value === "string" ? value.trim() : "";
  if (!line) continue;
  if (!line.startsWith("{")) continue; // ignore interleaved debug/status lines
  return JSON.parse(line);
}

Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix apply_samples.cjs crash when parsing stdout Harden sample replay JSON-RPC framing and move shim logs off stdout Jun 7, 2026
Copilot AI requested a review from dsyme June 7, 2026 10:48
Copilot finished work on behalf of dsyme June 7, 2026 10:48
@dsyme dsyme marked this pull request as ready for review June 7, 2026 11:30
Copilot AI review requested due to automatic review settings June 7, 2026 11:30
@dsyme dsyme merged commit 98fff44 into main Jun 7, 2026
7 checks passed
@dsyme dsyme deleted the copilot/fix-apply-samples-crash branch June 7, 2026 11:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the deterministic sample replay driver (apply_samples.cjs) against non-protocol stdout output during MCP JSON-RPC exchanges, and adjusts the GitHub Actions core.* shim so diagnostic logging won’t corrupt JSON-RPC framing when MCP servers speak over stdout.

Changes:

  • Redirected shimmed core.* logging in actions/setup/js/shim.cjs from console.* (stdout-mixed) to process.stderr.write(...) to keep stdout protocol-safe.
  • Made apply_samples.cjs::sendJsonRpc more defensive by skipping non-JSON lines and surfacing targeted parse errors.
  • Added focused unit tests for sendJsonRpc behavior around interleaved debug lines and malformed JSON frames.
Show a summary per file
File Description
actions/setup/js/shim.cjs Routes shim core.* logs to stderr to avoid stdout protocol corruption for JSON-RPC-speaking processes.
actions/setup/js/apply_samples.cjs Makes JSON-RPC response parsing more resilient to non-JSON stdout lines and improves parse error reporting.
actions/setup/js/apply_samples.test.cjs Adds unit tests covering the new sendJsonRpc framing/parse defenses.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment on lines +183 to +193
if (!line.startsWith("{")) {
core.debug(`apply_samples: ignoring non-JSON stdout line: ${line}`);
continue;
}
try {
return JSON.parse(line);
} catch (err) {
throw new Error(
`apply_samples: failed to parse MCP JSON-RPC response for request id=${request.id}: ${getErrorMessage(err)} (line: ${line})`
);
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

apply_samples.cjs crashes parsing stdout that contains '[debug] ...' lines (create_pull_request sample replay)

3 participants