Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/design-decision-gate.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions actions/setup/js/safe_outputs_handlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ function createHandlers(server, appendSafeOutput, config = {}) {
const TOKEN_THRESHOLD = 16000;
const addCommentConfig = config.add_comment || config["add-comment"] || {};
const wildcardAddCommentTargetRequiresItemNumber = addCommentConfig.target === "*";
const reviewCommentConfig = config.create_pull_request_review_comment || config["create-pull-request-review-comment"] || {};
const wildcardReviewCommentTargetRequiresPRNumber = reviewCommentConfig.target === "*";

/**
* Detect and offload large string fields to files.
Expand Down Expand Up @@ -1615,6 +1617,13 @@ function createHandlers(server, appendSafeOutput, config = {}) {
* to provide immediate feedback to the LLM before recording to NDJSON.
*/
const createPullRequestReviewCommentHandler = args => {
if (wildcardReviewCommentTargetRequiresPRNumber) {
const prNumber = args && args.pull_request_number;
const hasPRNumber = prNumber !== undefined && prNumber !== null && String(prNumber).trim() !== "";
if (!hasPRNumber) {
return buildIntentErrorResponse("create_pull_request_review_comment requires pull_request_number when safe-outputs.create-pull-request-review-comment.target is '*'. Provide pull_request_number and retry.");
}
}
const result = defaultHandler("create_pull_request_review_comment")(args);
// Increment only after the default handler returns successfully; if it throws
// (e.g. due to large-content rejection or an append write error) the counter
Expand Down
46 changes: 46 additions & 0 deletions actions/setup/js/safe_outputs_handlers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,52 @@ describe("safe_outputs_handlers", () => {
// Counter was NOT incremented, so empty-body submit should still be rejected
expect(() => handlers.submitPullRequestReviewHandler({ event: "COMMENT" })).toThrow(expect.objectContaining({ code: -32602, message: expect.stringContaining("review body is empty") }));
});

it("should require explicit pull_request_number when target is '*'", () => {
const wildcardHandlers = createHandlers(mockServer, mockAppendSafeOutput, {
create_pull_request_review_comment: {
target: "*",
},
});

const result = wildcardHandlers.createPullRequestReviewCommentHandler({ path: "src/foo.js", line: 5, body: "Consider renaming." });

expect(result.isError).toBe(true);
const responseData = JSON.parse(result.content[0].text);
expect(responseData.result).toBe("error");
expect(responseData.error).toContain("pull_request_number");
expect(mockAppendSafeOutput).not.toHaveBeenCalled();
});

it("should accept a comment when target is '*' and pull_request_number is provided", () => {
const wildcardHandlers = createHandlers(mockServer, mockAppendSafeOutput, {
create_pull_request_review_comment: {
target: "*",
},
});

const result = wildcardHandlers.createPullRequestReviewCommentHandler({ pull_request_number: 42, path: "src/foo.js", line: 5, body: "Consider renaming." });

expect(result.isError).toBeUndefined();
const responseData = JSON.parse(result.content[0].text);
expect(responseData.result).toBe("success");
expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "create_pull_request_review_comment", pull_request_number: 42 }));
});

it("should not require pull_request_number when target is not '*'", () => {
const triggeringHandlers = createHandlers(mockServer, mockAppendSafeOutput, {
create_pull_request_review_comment: {
target: "triggering",
},
});

const result = triggeringHandlers.createPullRequestReviewCommentHandler({ path: "src/foo.js", line: 5, body: "Consider renaming." });

expect(result.isError).toBeUndefined();
const responseData = JSON.parse(result.content[0].text);
expect(responseData.result).toBe("success");
expect(mockAppendSafeOutput).toHaveBeenCalled();
});
});

describe("updatePullRequestHandler", () => {
Expand Down