diff --git a/actions/setup/js/mcp_cli_bridge.cjs b/actions/setup/js/mcp_cli_bridge.cjs index 4c96401d929..11c0a7ef0fa 100644 --- a/actions/setup/js/mcp_cli_bridge.cjs +++ b/actions/setup/js/mcp_cli_bridge.cjs @@ -622,6 +622,63 @@ function readStdinSync() { return Buffer.concat(chunks).toString("utf8"); } +/** + * Try to extract a specific field value from a JSON object string. + * + * When per-field stdin mode (`--key .`) is used and stdin contains a JSON + * object, this function checks whether that object has the target key. If so, + * it returns the field value, allowing agents to pipe a full JSON payload and + * extract individual fields without the entire JSON string ending up as the + * field value. + * + * Example: an agent may construct a full payload JSON and still pass individual + * fields via `--key .`: + * printf '{"title":"Fix bug","body":"Details here"}' \ + * | safeoutputs create_pull_request --title "Fix bug" --body . + * Without this helper the body would be the raw JSON string. With it, the + * body is correctly extracted as "Details here". + * + * Returns `undefined` when stdin is not a JSON object, when the key is absent + * from the object, or when JSON parsing fails — in all those cases the caller + * falls back to using the raw stdin content. + * + * @param {string} trimmedStdin - Pre-trimmed stdin content. Must have no + * leading whitespace — the function uses a `startsWith('{')` fast-path to + * avoid parsing non-JSON input, so leading whitespace will cause a JSON + * object to be treated as non-JSON and return `undefined`. All callers + * must pass `stdinContent.trim()` before invoking this function. + * @param {string} canonicalKey - Canonical schema key to look up + * @param {Record} schemaProperties - Tool input schema properties + * @param {Map} normalizedSchemaKeyMap - Map from normalized key to canonical schema key + * @param {Set} ambiguousNormalizedSchemaKeys - Normalized keys that map to multiple schema keys + * @returns {unknown} The extracted field value, or `undefined` if not found + */ +function tryExtractJsonFieldFromStdin(trimmedStdin, canonicalKey, schemaProperties, normalizedSchemaKeyMap, ambiguousNormalizedSchemaKeys) { + if (!trimmedStdin.startsWith("{")) { + return undefined; + } + try { + const parsed = JSON.parse(trimmedStdin); + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) { + return undefined; + } + // Try the canonical key first, then any schema-aliased form. + if (Object.prototype.hasOwnProperty.call(parsed, canonicalKey)) { + return parsed[canonicalKey]; + } + // Search for a key in the JSON that resolves to the same canonical key. + for (const jsonKey of Object.keys(parsed)) { + const resolved = resolveSchemaPropertyKey(jsonKey, schemaProperties, normalizedSchemaKeyMap, ambiguousNormalizedSchemaKeys); + if (resolved === canonicalKey) { + return parsed[jsonKey]; + } + } + return undefined; + } catch { + return undefined; + } +} + /** * Parse user-provided --key value pairs into a tool arguments object. * Supports both --key value and --key=value styles. @@ -634,10 +691,15 @@ function readStdinSync() { * printf '{"issue_number":42,"body":"hello"}' | safeoutputs add_comment . * * When `stdinContent` is provided and non-empty, any '--key .' or '--key=.' - * pair substitutes that field's value with the raw stdin text (per-field - * stdin mode). This enables agents to pipe large text into a single field: - * printf 'Long issue body...' | safeoutputs create_issue --title "Bug" --body . - * When stdin is empty, the '.' is passed through as a literal value. + * pair substitutes that field's value with stdin. If stdin is a JSON object + * that contains the target key, the matching field value is extracted from the + * JSON instead of using the entire stdin string. This prevents a common agent + * mistake where the whole JSON payload ends up as a string field value: + * printf '{"title":"Fix bug","body":"Details"}' \ + * | safeoutputs create_pull_request --title "Fix bug" --body . + * When stdin is empty, the '.' is passed through as a literal value. When + * stdin is non-empty but the key is absent from the JSON (or stdin is not a + * JSON object), the entire trimmed stdin string is used as the field value. * * @param {string[]} args - User arguments after the tool name * @param {Record} [schemaProperties] - Tool input schema properties @@ -686,7 +748,8 @@ function parseToolArgs(args, schemaProperties = {}, stdinContent = null) { const canonicalKey = resolveSchemaPropertyKey(key, schemaProperties, normalizedSchemaKeyMap, ambiguousNormalizedSchemaKeys); const rawValue = raw.slice(eqIdx + 1); if (rawValue === "." && trimmedStdin) { - result[canonicalKey] = trimmedStdin; + const extracted = tryExtractJsonFieldFromStdin(trimmedStdin, canonicalKey, schemaProperties, normalizedSchemaKeyMap, ambiguousNormalizedSchemaKeys); + result[canonicalKey] = extracted !== undefined ? extracted : trimmedStdin; } else { result[canonicalKey] = coerceToolArgValue(canonicalKey, rawValue, schemaProperties[canonicalKey], result[canonicalKey], !hasSchemaProperties); } @@ -697,7 +760,8 @@ function parseToolArgs(args, schemaProperties = {}, stdinContent = null) { const canonicalKey = resolveSchemaPropertyKey(raw, schemaProperties, normalizedSchemaKeyMap, ambiguousNormalizedSchemaKeys); const rawValue = args[i + 1]; if (rawValue === "." && trimmedStdin) { - result[canonicalKey] = trimmedStdin; + const extracted = tryExtractJsonFieldFromStdin(trimmedStdin, canonicalKey, schemaProperties, normalizedSchemaKeyMap, ambiguousNormalizedSchemaKeys); + result[canonicalKey] = extracted !== undefined ? extracted : trimmedStdin; } else { result[canonicalKey] = coerceToolArgValue(canonicalKey, rawValue, schemaProperties[canonicalKey], result[canonicalKey], !hasSchemaProperties); } @@ -1441,6 +1505,7 @@ module.exports = { parseToolArgs, coerceToolArgValue, unescapeCliStringArg, + tryExtractJsonFieldFromStdin, extractJSONRPCMessages, renderProgressMessages, formatResponse, diff --git a/actions/setup/js/mcp_cli_bridge.test.cjs b/actions/setup/js/mcp_cli_bridge.test.cjs index de91e64368a..5e682df02a0 100644 --- a/actions/setup/js/mcp_cli_bridge.test.cjs +++ b/actions/setup/js/mcp_cli_bridge.test.cjs @@ -13,6 +13,7 @@ import { shouldShowToolHelpForEmptyArgs, showHelp, showToolHelp, + tryExtractJsonFieldFromStdin, unescapeCliStringArg, writeStdoutAndFlush, } from "./mcp_cli_bridge.cjs"; @@ -764,6 +765,162 @@ describe("mcp_cli_bridge.cjs", () => { }); }); + describe("per-field stdin mode with JSON stdin — field extraction", () => { + it("extracts matching field from JSON stdin when --body . is used (space-separated)", () => { + // Root cause of gh-aw-workshop#2118: agent piped JSON payload and used --body . + // expecting the body field to be extracted, but the entire JSON string ended up as body. + const schemaProperties = { title: { type: "string" }, body: { type: "string" } }; + const stdinContent = '{"title":"Fix bug","body":"This PR fixes the issue."}'; + + const { args } = parseToolArgs(["--title", "Fix bug", "--body", "."], schemaProperties, stdinContent); + + expect(args).toEqual({ title: "Fix bug", body: "This PR fixes the issue." }); + }); + + it("extracts matching field from JSON stdin when --body=. is used (equals-separated)", () => { + const schemaProperties = { title: { type: "string" }, body: { type: "string" } }; + const stdinContent = '{"title":"Fix bug","body":"Details here."}'; + + const { args } = parseToolArgs(["--title", "Fix bug", "--body=."], schemaProperties, stdinContent); + + expect(args).toEqual({ title: "Fix bug", body: "Details here." }); + }); + + it("extracts both title and body when --title . --body . used with JSON stdin", () => { + const schemaProperties = { title: { type: "string" }, body: { type: "string" } }; + const stdinContent = '{"title":"Fix: Bug #123","body":"This PR fixes bug #123."}'; + + const { args } = parseToolArgs(["--title", ".", "--body", "."], schemaProperties, stdinContent); + + expect(args).toEqual({ title: "Fix: Bug #123", body: "This PR fixes bug #123." }); + }); + + it("falls back to raw stdin when JSON does not contain the target key", () => { + const schemaProperties = { body: { type: "string" } }; + const stdinContent = '{"other_field":"value"}'; + + const { args } = parseToolArgs(["--body", "."], schemaProperties, stdinContent); + + // No 'body' key in JSON → use raw stdin content + expect(args).toEqual({ body: stdinContent }); + }); + + it("falls back to raw stdin when stdin is not a JSON object", () => { + const schemaProperties = { body: { type: "string" } }; + const stdinContent = "This is a long body from stdin."; + + const { args } = parseToolArgs(["--body", "."], schemaProperties, stdinContent); + + expect(args).toEqual({ body: stdinContent }); + }); + + it("falls back to raw stdin when stdin is a JSON array", () => { + const schemaProperties = { body: { type: "string" } }; + const stdinContent = '["item1","item2"]'; + + const { args } = parseToolArgs(["--body", "."], schemaProperties, stdinContent); + + expect(args).toEqual({ body: stdinContent }); + }); + + it("falls back to raw stdin when stdin is invalid JSON", () => { + const schemaProperties = { body: { type: "string" } }; + const stdinContent = '{"body": not-valid-json}'; + + const { args } = parseToolArgs(["--body", "."], schemaProperties, stdinContent); + + expect(args).toEqual({ body: stdinContent }); + }); + + it("resolves dash/underscore aliased JSON key to canonical schema key", () => { + const schemaProperties = { issue_number: { type: "integer" } }; + const stdinContent = '{"issue-number":42}'; + + const { args } = parseToolArgs(["--issue_number", "."], schemaProperties, stdinContent); + + expect(args).toEqual({ issue_number: 42 }); + }); + + it("preserves non-string JSON values extracted from stdin (e.g. boolean, number)", () => { + const schemaProperties = { draft: { type: "boolean" }, count: { type: "integer" } }; + const stdinContent = '{"draft":true,"count":5}'; + + const { args: draftArgs } = parseToolArgs(["--draft", "."], schemaProperties, stdinContent); + const { args: countArgs } = parseToolArgs(["--count", "."], schemaProperties, stdinContent); + + expect(draftArgs).toEqual({ draft: true }); + expect(countArgs).toEqual({ count: 5 }); + }); + + it("handles multiline JSON payload with --body . correctly", () => { + const schemaProperties = { title: { type: "string" }, body: { type: "string" } }; + const stdinContent = `{ + "title": "Fix bug", + "body": "### Summary\\n\\nDetails here." +}`; + + const { args } = parseToolArgs(["--title", "Fix bug", "--body", "."], schemaProperties, stdinContent); + + expect(args).toEqual({ title: "Fix bug", body: "### Summary\n\nDetails here." }); + }); + }); + + describe("tryExtractJsonFieldFromStdin", () => { + it("extracts a field value from a JSON object string", () => { + const schemaProperties = { body: { type: "string" } }; + const result = tryExtractJsonFieldFromStdin( + '{"title":"Fix","body":"PR description"}', + "body", + schemaProperties, + new Map([ + ["title", "title"], + ["body", "body"], + ]), + new Set() + ); + expect(result).toBe("PR description"); + }); + + it("returns undefined when the key is absent from the JSON", () => { + const schemaProperties = { body: { type: "string" } }; + const result = tryExtractJsonFieldFromStdin('{"title":"Fix"}', "body", schemaProperties, new Map([["title", "title"]]), new Set()); + expect(result).toBeUndefined(); + }); + + it("returns undefined for non-object JSON (array)", () => { + const result = tryExtractJsonFieldFromStdin('["a","b"]', "body", {}, new Map(), new Set()); + expect(result).toBeUndefined(); + }); + + it("returns undefined for invalid JSON", () => { + const result = tryExtractJsonFieldFromStdin("{not valid}", "body", {}, new Map(), new Set()); + expect(result).toBeUndefined(); + }); + + it("returns undefined for plain text (not starting with {)", () => { + const result = tryExtractJsonFieldFromStdin("plain text", "body", {}, new Map(), new Set()); + expect(result).toBeUndefined(); + }); + + it("preserves null value extracted from JSON stdin (does not fall back to raw stdin)", () => { + const schemaProperties = { body: { type: "string" } }; + const result = tryExtractJsonFieldFromStdin('{"body":null}', "body", schemaProperties, new Map([["body", "body"]]), new Set()); + expect(result).toBeNull(); + }); + + it("preserves false boolean extracted from JSON stdin", () => { + const schemaProperties = { draft: { type: "boolean" } }; + const result = tryExtractJsonFieldFromStdin('{"draft":false}', "draft", schemaProperties, new Map([["draft", "draft"]]), new Set()); + expect(result).toBe(false); + }); + + it("preserves 0 number extracted from JSON stdin", () => { + const schemaProperties = { count: { type: "number" } }; + const result = tryExtractJsonFieldFromStdin('{"count":0}', "count", schemaProperties, new Map([["count", "count"]]), new Set()); + expect(result).toBe(0); + }); + }); + describe("unescapeCliStringArg", () => { it("converts \\n to an actual newline", () => { expect(unescapeCliStringArg("Hello\\nWorld")).toBe("Hello\nWorld");