Analyze: update_pull_request with empty args silently drops safe outputs

actions/setup/js/safe_outputs_handlers.cjs

@@ -43,6 +43,23 @@ function buildIntentErrorResponse(error) {
   };
 }
 
+/**
+ * Returns true if `args` contains at least one meaningful field for update_pull_request:
+ * a string `title`, a string `body`, or `update_branch === true`.
+ * Mirrors the downstream requiresOneOf:title,body,update_branch validation in
+ * safe_output_type_validator.cjs (which also excludes field === false from the count).
+ * @param {Record<string, any> | null | undefined} args
+ * @returns {boolean}
+ */
+function hasUpdatePullRequestFields(args) {
+  const safeArgs = args || {};
+  return (
+    typeof safeArgs.title === "string" ||
+    typeof safeArgs.body === "string" ||
+    safeArgs.update_branch === true
+  );
+}
+
 /**
  * Create handlers for safe output tools
  * @param {Object} server - The MCP server instance for logging
@@ -1354,6 +1371,25 @@ function createHandlers(server, appendSafeOutput, config = {}) {
     };
   };
 
+  /**
+   * Handler for update_pull_request tool
+   * Spec cross-reference: Safe Output Outcome Evaluation §update_pull_request.
+   * Per Safe Outputs Specification MCE1: Enforces constraints during tool invocation
+   * to provide immediate feedback to the LLM before recording to NDJSON.
+   * Uses hasUpdatePullRequestFields to validate that at least one of 'title', 'body',
+   * or 'update_branch' is provided before recording to NDJSON.
+   */
+  const updatePullRequestHandler = args => {
+    if (!hasUpdatePullRequestFields(args)) {
+      throw {
+        code: -32602,
+        message: `${ERR_VALIDATION}: update_pull_request requires at least one of: 'title', 'body', 'update_branch' fields`,
+      };
+    }
+
+    return defaultHandler("update_pull_request")(args || {});
+  };
+
   return {
     defaultHandler,
     uploadAssetHandler,
@@ -1366,10 +1402,12 @@ function createHandlers(server, appendSafeOutput, config = {}) {
     addCommentHandler,
     createPullRequestReviewCommentHandler,
     submitPullRequestReviewHandler,
+    updatePullRequestHandler,
   };
 }
 
 module.exports = {
   buildIntentErrorResponse,
   createHandlers,
+  hasUpdatePullRequestFields,
 };

actions/setup/js/safe_outputs_handlers.test.cjs

@@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
 import fs from "fs";
 import path from "path";
 import { execSync } from "child_process";
-import { createHandlers } from "./safe_outputs_handlers.cjs";
+import { createHandlers, hasUpdatePullRequestFields } from "./safe_outputs_handlers.cjs";
 import {
   looksLikeExploratoryBranch,
   normalizeProbeValue,
@@ -1764,4 +1764,149 @@ describe("safe_outputs_handlers", () => {
       expect(() => handlers.submitPullRequestReviewHandler({ event: "COMMENT" })).toThrow(expect.objectContaining({ code: -32602, message: expect.stringContaining("review body is empty") }));
     });
   });
+
+  describe("updatePullRequestHandler", () => {
+    it("should throw MCP error when no fields are provided", () => {
+      expect(() => handlers.updatePullRequestHandler({})).toThrow(
+        expect.objectContaining({
+          code: -32602,
+          message: expect.stringContaining("requires at least one of"),
+        })
+      );
+    });
+
+    it("should throw MCP error when called with null/undefined args", () => {
+      expect(() => handlers.updatePullRequestHandler(null)).toThrow(
+        expect.objectContaining({ code: -32602 })
+      );
+      expect(() => handlers.updatePullRequestHandler(undefined)).toThrow(
+        expect.objectContaining({ code: -32602 })
+      );
+    });
+
+    it("should throw MCP error when update_branch is explicitly false and no other fields", () => {
+      expect(() => handlers.updatePullRequestHandler({ update_branch: false })).toThrow(
+        expect.objectContaining({ code: -32602 })
+      );
+    });
+
+    it("should throw MCP error when title is null", () => {
+      expect(() => handlers.updatePullRequestHandler({ title: null })).toThrow(
+        expect.objectContaining({ code: -32602 })
+      );
+    });
+
+    it("should throw MCP error when body is null", () => {
+      expect(() => handlers.updatePullRequestHandler({ body: null })).toThrow(
+        expect.objectContaining({ code: -32602 })
+      );
+    });
+
+    it("should throw MCP error when update_branch is null", () => {
+      expect(() => handlers.updatePullRequestHandler({ update_branch: null })).toThrow(
+        expect.objectContaining({ code: -32602 })
+      );
+    });
+
+    it("should write entry and return success when title is provided", () => {
+      const result = handlers.updatePullRequestHandler({ title: "New Title" });
+      expect(result).toHaveProperty("content");
+      const data = JSON.parse(result.content[0].text);
+      expect(data.result).toBe("success");
+      expect(mockAppendSafeOutput).toHaveBeenCalledWith(
+        expect.objectContaining({ type: "update_pull_request", title: "New Title" })
+      );
+    });
+
+    it("should write entry and return success when body is provided", () => {
+      const result = handlers.updatePullRequestHandler({ body: "Updated body" });
+      expect(result).toHaveProperty("content");
+      const data = JSON.parse(result.content[0].text);
+      expect(data.result).toBe("success");
+      expect(mockAppendSafeOutput).toHaveBeenCalledWith(
+        expect.objectContaining({ type: "update_pull_request", body: "Updated body" })
+      );
+    });
+
+    it("should write entry and return success when update_branch is true", () => {
+      const result = handlers.updatePullRequestHandler({ update_branch: true });
+      expect(result).toHaveProperty("content");
+      const data = JSON.parse(result.content[0].text);
+      expect(data.result).toBe("success");
+      expect(mockAppendSafeOutput).toHaveBeenCalledWith(
+        expect.objectContaining({ type: "update_pull_request", update_branch: true })
+      );
+    });
+
+    it("should write entry and return success when both title and body are provided", () => {
+      const result = handlers.updatePullRequestHandler({ title: "New Title", body: "New body" });
+      expect(result).toHaveProperty("content");
+      const data = JSON.parse(result.content[0].text);
+      expect(data.result).toBe("success");
+      expect(mockAppendSafeOutput).toHaveBeenCalledWith(
+        expect.objectContaining({ type: "update_pull_request", title: "New Title", body: "New body" })
+      );
+    });
+
+    it("error message should mention all required fields", () => {
+      try {
+        handlers.updatePullRequestHandler({});
+        expect.fail("Should have thrown");
+      } catch (err) {
+        expect(err.message).toContain("'title'");
+        expect(err.message).toContain("'body'");
+        expect(err.message).toContain("'update_branch'");
+      }
+    });
+  });
+});
+
+describe("hasUpdatePullRequestFields", () => {
+  it("returns false for empty object", () => {
+    expect(hasUpdatePullRequestFields({})).toBe(false);
+  });
+
+  it("returns false for null", () => {
+    expect(hasUpdatePullRequestFields(null)).toBe(false);
+  });
+
+  it("returns false for undefined", () => {
+    expect(hasUpdatePullRequestFields(undefined)).toBe(false);
+  });
+
+  it("returns false when update_branch is false", () => {
+    expect(hasUpdatePullRequestFields({ update_branch: false })).toBe(false);
+  });
+
+  it("returns false when title is null", () => {
+    expect(hasUpdatePullRequestFields({ title: null })).toBe(false);
+  });
+
+  it("returns false when body is null", () => {
+    expect(hasUpdatePullRequestFields({ body: null })).toBe(false);
+  });
+
+  it("returns false when update_branch is null", () => {
+    expect(hasUpdatePullRequestFields({ update_branch: null })).toBe(false);
+  });
+
+  it("returns true when title is a string", () => {
+    expect(hasUpdatePullRequestFields({ title: "New Title" })).toBe(true);
+  });
+
+  it("returns true when body is a string", () => {
+    expect(hasUpdatePullRequestFields({ body: "Updated body" })).toBe(true);
+  });
+
+  it("returns true when update_branch is exactly true", () => {
+    expect(hasUpdatePullRequestFields({ update_branch: true })).toBe(true);
+  });
+
+  it("returns true when both title and body are provided", () => {
+    expect(hasUpdatePullRequestFields({ title: "t", body: "b" })).toBe(true);
+  });
+
+  it("returns true for empty string title (typeof === 'string')", () => {
+    expect(hasUpdatePullRequestFields({ title: "" })).toBe(true);
+  });
 });

actions/setup/js/safe_outputs_tools.json

@@ -799,7 +799,8 @@
   },
   {
     "name": "update_pull_request",
-    "description": "Update an existing GitHub pull request's title or body. Supports replacing, appending to, or prepending content to the body. Title is always replaced. Only the fields you specify will be updated; other fields remain unchanged.",
+
+    "description": "Update an existing GitHub pull request's title or body. Supports replacing, appending to, or prepending content to the body. Title is always replaced. Only the fields you specify will be updated; other fields remain unchanged. REQUIRED: You must provide at least one of: 'title' (a non-empty string), 'body' (a string), or 'update_branch' set to true (not false). Omitting all three, or passing only 'update_branch: false', will return a -32602 error.",
     "inputSchema": {
       "type": "object",
       "properties": {
@@ -818,7 +819,7 @@
         },
         "update_branch": {
           "type": "boolean",
-          "description": "When true, update the pull request branch with the latest base branch changes before applying other updates. Defaults to false."
+          "description": "When true, update the pull request branch with the latest base branch changes before applying other updates. Defaults to false. Note: only true counts as a meaningful update; passing false is the same as omitting this field."
         },
         "pull_request_number": {
           "type": ["number", "string"],
@@ -845,6 +846,14 @@
           "description": "Trustworthiness level of the message source (e.g., \"low\", \"medium\", \"high\")."
         }
       },
+      "anyOf": [
+        { "required": ["title"] },
+        { "required": ["body"] },
+        {
+          "required": ["update_branch"],
+          "properties": { "update_branch": { "const": true } }
+        }
+      ],
       "additionalProperties": false
     }
   },

actions/setup/js/safe_outputs_tools_loader.cjs

@@ -82,6 +82,7 @@ function attachHandlers(tools, handlers) {
     add_comment: handlers.addCommentHandler,
     create_pull_request_review_comment: handlers.createPullRequestReviewCommentHandler,
     submit_pull_request_review: handlers.submitPullRequestReviewHandler,
+    update_pull_request: handlers.updatePullRequestHandler,
   };
 
   tools.forEach(tool => {

pkg/workflow/js/safe_outputs_tools.json

@@ -943,7 +943,7 @@
   },
   {
     "name": "update_pull_request",
-    "description": "Update an existing GitHub pull request's title or body. Supports replacing, appending to, or prepending content to the body. Title is always replaced. Only the fields you specify will be updated; other fields remain unchanged.",
+    "description": "Update an existing GitHub pull request's title or body. Supports replacing, appending to, or prepending content to the body. Title is always replaced. Only the fields you specify will be updated; other fields remain unchanged. REQUIRED: You must provide at least one of: 'title' (a non-empty string), 'body' (a string), or 'update_branch' set to true (not false). Omitting all three, or passing only 'update_branch: false', will return a -32602 error.",
     "inputSchema": {
       "type": "object",
       "properties": {
@@ -966,7 +966,7 @@
         },
         "update_branch": {
           "type": "boolean",
-          "description": "When true, update the pull request branch with the latest base branch changes before applying other updates. Defaults to false."
+          "description": "When true, update the pull request branch with the latest base branch changes before applying other updates. Defaults to false. Note: only true counts as a meaningful update; passing false is the same as omitting this field."
         },
         "pull_request_number": {
           "type": [
@@ -1002,6 +1002,14 @@
           "description": "Trustworthiness level of the message source (e.g., \"low\", \"medium\", \"high\")."
         }
       },
+      "anyOf": [
+        { "required": ["title"] },
+        { "required": ["body"] },
+        {
+          "required": ["update_branch"],
+          "properties": { "update_branch": { "const": true } }
+        }
+      ],
       "additionalProperties": false
     }
   },