Skip to content
Merged
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
22 changes: 15 additions & 7 deletions actions/setup/js/safe_outputs_handlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function hasExplicitTargetParameter(entry, fieldNames) {

/**
* @param {string} toolName
* @returns {{primary?: string, anyOf?: string[]} | null}
* @returns {{primary?: string, anyOf?: string[], allOf?: string[]} | null}
*/
function getWildcardTargetRequirement(toolName) {
return safeOutputsToolMap.get(toolName)?.["x-safe-outputs-target-requirements"]?.["*"] || null;
Expand Down Expand Up @@ -315,15 +315,23 @@ function createHandlers(server, appendSafeOutput, config = {}) {
return null;
}

const configKey = toolName.replace(/_/g, "-");

const anyOf = Array.isArray(requirement.anyOf) ? requirement.anyOf : [];
if (anyOf.length === 0 || hasExplicitTargetParameter(entry, anyOf)) {
return null;
if (anyOf.length > 0 && !hasExplicitTargetParameter(entry, anyOf)) {
const primary = requirement.primary || anyOf[0];
const guidance = anyOf.length === 1 ? primary : `one of: ${anyOf.join(", ")}`;
return buildIntentErrorResponse(`${toolName} requires ${primary} when safe-outputs.${configKey}.target is '*'. Provide ${guidance} and retry.`);
}

const configKey = toolName.replace(/_/g, "-");
const primary = requirement.primary || anyOf[0];
const guidance = anyOf.length === 1 ? primary : `one of: ${anyOf.join(", ")}`;
return buildIntentErrorResponse(`${toolName} requires ${primary} when safe-outputs.${configKey}.target is '*'. Provide ${guidance} and retry.`);
const allOf = Array.isArray(requirement.allOf) ? requirement.allOf : [];
for (const field of allOf) {
if (!hasExplicitTargetParameter(entry, [field])) {
return buildIntentErrorResponse(`${toolName} requires ${field} when safe-outputs.${configKey}.target is '*'. Provide ${field} and retry.`);
}
}

return null;
};

/**
Expand Down
45 changes: 43 additions & 2 deletions actions/setup/js/safe_outputs_handlers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1440,14 +1440,36 @@ describe("safe_outputs_handlers", () => {
}
});

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

const result = await wildcardHandlers.pushToPullRequestBranchHandler({ message: "Apply requested changes." });
const result = await wildcardHandlers.pushToPullRequestBranchHandler({
message: "Apply requested changes.",
pull_request_number: 123,
Comment thread
github-actions[bot] marked this conversation as resolved.
});

expect(result.isError).toBe(true);
const responseData = JSON.parse(result.content[0].text);
expect(responseData.result).toBe("error");
expect(responseData.error).toContain("requires repo");
Comment thread
github-actions[bot] marked this conversation as resolved.
expect(mockAppendSafeOutput).not.toHaveBeenCalled();
});

it("should require explicit pull_request_number when push_to_pull_request_branch target is '*' and only repo is supplied", async () => {
const wildcardHandlers = createHandlers(mockServer, mockAppendSafeOutput, {
push_to_pull_request_branch: {
target: "*",
},
});

const result = await wildcardHandlers.pushToPullRequestBranchHandler({
message: "Apply requested changes.",
repo: "owner/repo",
});

expect(result.isError).toBe(true);
const responseData = JSON.parse(result.content[0].text);
Expand All @@ -1456,6 +1478,25 @@ describe("safe_outputs_handlers", () => {
expect(mockAppendSafeOutput).not.toHaveBeenCalled();
});

it("should pass wildcard validation when push_to_pull_request_branch target is '*' and both repo and pull_request_number are supplied", async () => {
const wildcardHandlers = createHandlers(mockServer, mockAppendSafeOutput, {
push_to_pull_request_branch: {
target: "*",
},
});

const result = await wildcardHandlers.pushToPullRequestBranchHandler({
message: "Apply requested changes.",
repo: "owner/repo",
pull_request_number: 123,
});

// Wildcard validation passes; downstream failure (e.g. repo not found in workspace) is expected
const responseData = JSON.parse(result.content[0].text);
expect(responseData.error).not.toContain("requires repo");
expect(responseData.error).not.toContain("requires pull_request_number");
});

it("should reject obvious exploratory test payloads before recording a PR branch update intent", async () => {
// The agent can no longer supply `branch`; the handler derives it from
// the current working checkout. Model the failure mode where the
Expand Down
13 changes: 9 additions & 4 deletions actions/setup/js/safe_outputs_tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -1193,7 +1193,7 @@
},
{
"name": "push_to_pull_request_branch",
"description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. IMPORTANT: always supply the 'branch' argument with the local branch name you committed to. In batch workflows that process multiple PRs, this is required \u2014 if omitted, the branch is inferred from the current git HEAD, which will produce wrong results if the workspace has been checked out to a different branch between commit and tool-call time. IMPORTANT: do NOT use 'git merge' to update the branch against another branch \u2014 merge commits cannot be signed; the action will attempt to squash them into a single linear commit before pushing, but this rewrites history. Use 'git rebase' instead to avoid the rewrite. This tool auto-pins the PR branch HEAD before pushing, so it takes no concurrency-control / compare-and-swap parameter \u2014 do NOT pass expected_head_sha, head_sha, or base_sha; the only accepted fields are message, branch, and pull_request_number.",
"description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. IMPORTANT: always supply the 'branch' argument with the local branch name you committed to. In batch workflows that process multiple PRs, this is required \u2014 if omitted, the branch is inferred from the current git HEAD, which will produce wrong results if the workspace has been checked out to a different branch between commit and tool-call time. IMPORTANT: do NOT use 'git merge' to update the branch against another branch \u2014 merge commits cannot be signed; the action will attempt to squash them into a single linear commit before pushing, but this rewrites history. Use 'git rebase' instead to avoid the rewrite. This tool auto-pins the PR branch HEAD before pushing, so it takes no concurrency-control / compare-and-swap parameter \u2014 do NOT pass expected_head_sha, head_sha, or base_sha; the only accepted fields are message, branch, pull_request_number, and repo.",
"inputSchema": {
"type": "object",
"required": ["message"],
Expand All @@ -1210,9 +1210,13 @@
},
"pull_request_number": {
"type": ["number", "string"],
"description": "Pull request number to push changes to. This is the numeric ID from the GitHub URL (e.g., 654 in github.com/owner/repo/pull/654). Required when the workflow target is '*' (any PR).",
"description": "Pull request number to push changes to. This is the numeric ID from the GitHub URL (e.g., 654 in github.com/owner/repo/pull/654). Required when the workflow target is '*' (any PR) — both pull_request_number and repo must be supplied together for wildcard targets.",
"x-synonyms": ["pullRequestNumber"]
},
"repo": {
"type": "string",
"description": "Target repository in 'owner/repo' format. For multi-repo workflows where the pull request may live in a side checkout, provide this explicitly so the correct repository checkout is selected."
},
"secrecy": {
"type": "string",
"description": "Confidentiality level of the message content (e.g., \"public\", \"internal\", \"private\")."
Expand All @@ -1226,8 +1230,9 @@
},
"x-safe-outputs-target-requirements": {
"*": {
"primary": "pull_request_number",
"anyOf": ["pull_request_number"]
"primary": "repo",
Comment thread
github-actions[bot] marked this conversation as resolved.
"anyOf": ["repo"],
"allOf": ["pull_request_number"]
}
}
},
Expand Down
Loading