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
70 changes: 58 additions & 12 deletions actions/setup/js/submit_pr_review.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction
*/

const { resolveTarget } = require("./safe_output_helpers.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");

/** @type {string} Safe output type handled by this module */
const HANDLER_TYPE = "submit_pull_request_review";

Expand All @@ -23,6 +26,7 @@ const VALID_EVENTS = new Set(["APPROVE", "REQUEST_CHANGES", "COMMENT"]);
*/
async function main(config = {}) {
const maxCount = config.max || 1;
const targetConfig = config.target || "triggering";
const buffer = config._prReviewBuffer;

if (!buffer) {
Expand All @@ -32,7 +36,7 @@ async function main(config = {}) {
};
}

core.info(`Submit PR review handler initialized: max=${maxCount}`);
core.info(`Submit PR review handler initialized: max=${maxCount}, target=${targetConfig}`);

let processedCount = 0;

Expand Down Expand Up @@ -82,17 +86,59 @@ async function main(config = {}) {

// Ensure review context is set for body-only reviews (no inline comments).
// If create_pull_request_review_comment already set context, this is a no-op.
if (!buffer.getReviewContext() && typeof context !== "undefined" && context && context.payload) {
const pr = context.payload.pull_request;
if (pr && pr.head && pr.head.sha) {
const repo = `${context.repo.owner}/${context.repo.repo}`;
buffer.setReviewContext({
repo,
repoParts: { owner: context.repo.owner, repo: context.repo.repo },
pullRequestNumber: pr.number,
pullRequest: pr,
});
core.info(`Set review context from triggering PR: ${repo}#${pr.number}`);
// Use target config as single source of truth (same as add_comment): resolveTarget first, then use payload PR only when it matches.
if (!buffer.getReviewContext()) {
const repo = `${context.repo.owner}/${context.repo.repo}`;
const repoParts = { owner: context.repo.owner, repo: context.repo.repo };

const targetResult = resolveTarget({
targetConfig,
item: message,
context,
itemType: "PR review",
supportsPR: false,
supportsIssue: false,
});

if (!targetResult.success) {
if (targetResult.shouldFail) {
core.warning(`Could not resolve PR for review context: ${targetResult.error}`);
}
} else if (targetResult.number) {
const prNum = targetResult.number;
const payloadPR = context.payload?.pull_request;
const usePayloadPR = payloadPR && payloadPR.number === prNum && payloadPR.head?.sha;

if (usePayloadPR) {
buffer.setReviewContext({
repo,
repoParts,
pullRequestNumber: payloadPR.number,
pullRequest: payloadPR,
});
core.info(`Set review context from triggering PR: ${repo}#${payloadPR.number}`);
} else {
try {
const { data: fetchedPR } = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: prNum,
});
if (fetchedPR?.head?.sha) {
buffer.setReviewContext({
repo,
repoParts,
pullRequestNumber: fetchedPR.number,
pullRequest: fetchedPR,
});
core.info(`Set review context from target: ${repo}#${fetchedPR.number}`);
} else {
core.warning("Fetched PR missing head.sha - cannot set review context");
}
} catch (fetchErr) {
core.warning(`Could not fetch PR #${prNum} for review context: ${getErrorMessage(fetchErr)}`);
}
}
}
}

Expand Down
124 changes: 122 additions & 2 deletions actions/setup/js/submit_pr_review.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
event: "APPROVE",
};

const result = await handler(message, {});

Check failure on line 56 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should set review metadata for APPROVE event

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:56:26

expect(result.success).toBe(true);
expect(result.event).toBe("APPROVE");
Expand All @@ -68,7 +68,7 @@
event: "REQUEST_CHANGES",
};

const result = await handler(message, {});

Check failure on line 71 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should set review metadata for REQUEST_CHANGES event

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:71:26

expect(result.success).toBe(true);
expect(result.event).toBe("REQUEST_CHANGES");
Expand All @@ -81,7 +81,7 @@
event: "COMMENT",
};

const result = await handler(message, {});

Check failure on line 84 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should set review metadata for COMMENT event

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:84:26

expect(result.success).toBe(true);
expect(result.event).toBe("COMMENT");
Expand All @@ -94,7 +94,7 @@
event: "approve",
};

const result = await handler(message, {});

Check failure on line 97 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should normalize event to uppercase

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:97:26

expect(result.success).toBe(true);
expect(result.event).toBe("APPROVE");
Expand All @@ -106,7 +106,7 @@
body: "Some feedback",
};

const result = await handler(message, {});

Check failure on line 109 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should default event to COMMENT when missing

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:109:26

expect(result.success).toBe(true);
expect(result.event).toBe("COMMENT");
Expand All @@ -132,7 +132,7 @@
event: "APPROVE",
};

const result = await handler(message, {});

Check failure on line 135 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should allow empty body for APPROVE event

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:135:26

expect(result.success).toBe(true);
expect(result.event).toBe("APPROVE");
Expand All @@ -158,7 +158,7 @@
event: "COMMENT",
};

const result = await handler(message, {});

Check failure on line 161 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should allow empty body for COMMENT event

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:161:26

expect(result.success).toBe(true);
expect(result.event).toBe("COMMENT");
Expand All @@ -169,7 +169,7 @@
type: "submit_pull_request_review",
};

const result = await handler(message, {});

Check failure on line 172 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should allow no event and no body (defaults to COMMENT)

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:172:26

expect(result.success).toBe(true);
expect(result.event).toBe("COMMENT");
Expand All @@ -190,7 +190,7 @@
};

// First call should succeed
const result1 = await handler(message1, {});

Check failure on line 193 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should respect max count configuration

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:193:27
expect(result1.success).toBe(true);

// Second call should fail (max=1)
Expand All @@ -217,7 +217,7 @@
event: "APPROVE",
};

const result2 = await handler(validMessage, {});

Check failure on line 220 in actions/setup/js/submit_pr_review.test.cjs

View workflow job for this annotation

GitHub Actions / js

submit_pr_review.test.cjs > submit_pr_review (Handler Factory Architecture) > should not consume max count slot on validation failure

ReferenceError: context is not defined ❯ handleSubmitPRReview submit_pr_review.cjs:91:20 ❯ submit_pr_review.test.cjs:220:27
expect(result2.success).toBe(true);
expect(result2.event).toBe("APPROVE");
});
Expand Down Expand Up @@ -245,8 +245,9 @@
});

it("should set review context from triggering PR for body-only reviews", async () => {
// Simulate a PR trigger context
// Simulate a PR trigger context (resolveTarget uses eventName)
global.context = {
eventName: "pull_request",
repo: { owner: "test-owner", repo: "test-repo" },
payload: {
pull_request: { number: 42, head: { sha: "abc123" } },
Expand Down Expand Up @@ -274,10 +275,129 @@
expect(ctx.repo).toBe("test-owner/test-repo");
expect(ctx.pullRequestNumber).toBe(42);

// Clean up
delete global.context;
});

it("should set review context from target config when explicit PR number (e.g. workflow_dispatch)", async () => {
const fetchedPR = {
number: 99,
head: { sha: "fetched-sha" },
user: { login: "author" },
};
global.context = {
eventName: "workflow_dispatch",
repo: { owner: "my-owner", repo: "my-repo" },
payload: {},
};
global.github = {
rest: {
pulls: {
get: vi.fn().mockResolvedValue({ data: fetchedPR }),
},
},
};

const localBuffer = createReviewBuffer();
const { main } = require("./submit_pr_review.cjs");
const localHandler = await main({ max: 1, target: "99", _prReviewBuffer: localBuffer });

const message = {
type: "submit_pull_request_review",
body: "Approved via target",
event: "APPROVE",
};

const result = await localHandler(message, {});

expect(result.success).toBe(true);
expect(localBuffer.hasReviewMetadata()).toBe(true);
const ctx = localBuffer.getReviewContext();
expect(ctx).not.toBeNull();
expect(ctx.repo).toBe("my-owner/my-repo");
expect(ctx.pullRequestNumber).toBe(99);
expect(ctx.pullRequest.head.sha).toBe("fetched-sha");
expect(global.github.rest.pulls.get).toHaveBeenCalledWith({
owner: "my-owner",
repo: "my-repo",
pull_number: 99,
});

delete global.context;
delete global.github;
});

it("should not set review context when target is triggering and not in PR context", async () => {
global.context = {
eventName: "workflow_dispatch",
repo: { owner: "o", repo: "r" },
payload: {},
};

const localBuffer = createReviewBuffer();
const { main } = require("./submit_pr_review.cjs");
const localHandler = await main({ max: 1, _prReviewBuffer: localBuffer });

const message = {
type: "submit_pull_request_review",
body: "Review",
event: "COMMENT",
};

const result = await localHandler(message, {});

expect(result.success).toBe(true);
expect(localBuffer.hasReviewMetadata()).toBe(true);
expect(localBuffer.getReviewContext()).toBeNull();

delete global.context;
});

it("should set review context from target '*' with message.pull_request_number", async () => {
const fetchedPR = {
number: 5,
head: { sha: "sha5" },
user: { login: "u" },
};
global.context = {
eventName: "workflow_dispatch",
repo: { owner: "o", repo: "r" },
payload: {},
};
global.github = {
rest: {
pulls: {
get: vi.fn().mockResolvedValue({ data: fetchedPR }),
},
},
};

const localBuffer = createReviewBuffer();
const { main } = require("./submit_pr_review.cjs");
const localHandler = await main({ max: 1, target: "*", _prReviewBuffer: localBuffer });

const message = {
type: "submit_pull_request_review",
body: "Review",
event: "APPROVE",
pull_request_number: 5,
};

const result = await localHandler(message, {});

expect(result.success).toBe(true);
const ctx = localBuffer.getReviewContext();
expect(ctx).not.toBeNull();
expect(ctx.pullRequestNumber).toBe(5);
expect(global.github.rest.pulls.get).toHaveBeenCalledWith({
owner: "o",
repo: "r",
pull_number: 5,
});

delete global.context;
delete global.github;
});

it("should not override existing review context from comments", async () => {
// Pre-set context as if a comment handler already set it
buffer.setReviewContext({
Expand Down
5 changes: 5 additions & 0 deletions docs/src/content/docs/reference/frontmatter-full.md
Original file line number Diff line number Diff line change
Expand Up @@ -2700,6 +2700,11 @@ safe-outputs:
# (optional)
max: 1

# Target PR for the review: 'triggering' (default), '*' (any PR), or explicit PR number.
# Required when workflow is not triggered by a pull request (e.g. workflow_dispatch).
# (optional)
target: "triggering"

# Controls whether AI-generated footer is added to the review body. When false,
# the footer is omitted. Defaults to true.
# (optional)
Expand Down
8 changes: 8 additions & 0 deletions docs/src/content/docs/reference/safe-outputs-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,13 @@ add-comment:
allowed-repos: [...]
```

**Submit PR Review Extensions**:
```yaml
submit-pull-request-review:
target: "triggering" | "*" | <PR number> # Required when not in pull_request trigger
footer: "always" | "none" | "if-body" # Footer on review body
```

**Pull Request Extensions**:
```yaml
create-pull-request:
Expand Down Expand Up @@ -2317,6 +2324,7 @@ This section provides complete definitions for all remaining safe output types.
**Notes**:
- Submits all buffered review comments from `create_pull_request_review_comment`
- Review status affects PR merge requirements
- **Target**: `target` accepts `"triggering"` (default), `"*"` (use `pull_request_number` from message), or an explicit PR number (e.g. `${{ github.event.inputs.pr_number }}`). Required when the workflow is not triggered by a pull request (e.g. `workflow_dispatch`).
- Footer control: `footer` accepts `"always"` (default), `"none"`, or `"if-body"` (only when review body has text); boolean `true`/`false` maps to `"always"`/`"none"`

---
Expand Down
3 changes: 3 additions & 0 deletions docs/src/content/docs/reference/safe-outputs.md
Original file line number Diff line number Diff line change
Expand Up @@ -736,12 +736,15 @@ If the agent calls `submit_pull_request_review`, it can specify a review `body`

If the agent does not call `submit_pull_request_review` at all, buffered comments are still submitted as a COMMENT review automatically.

When the workflow is not triggered by a pull request (e.g. `workflow_dispatch`), set `target` to the PR number (e.g. `${{ github.event.inputs.pr_number }}`) so the review can be submitted. Same semantics as [add-comment](#comment-creation-add-comment) `target`: `"triggering"` (default), `"*"` (use `pull_request_number` from the message), or an explicit number.

```yaml wrap
safe-outputs:
create-pull-request-review-comment:
max: 10
submit-pull-request-review:
max: 1 # max reviews to submit (default: 1)
target: "triggering" # or "*", or e.g. ${{ github.event.inputs.pr_number }} when not in pull_request trigger
footer: false # omit AI-generated footer from review body (default: true)
```

Expand Down
4 changes: 4 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -5039,6 +5039,10 @@
],
"description": "Controls when AI-generated footer is added to the review body. Accepts boolean (true/false) or string ('always', 'none', 'if-body'). The 'if-body' mode is useful for clean approval reviews without body text. Defaults to 'always'."
},
"target": {
"type": "string",
"description": "Target PR for the review: 'triggering' (default, current PR), '*' (any PR, requires pull_request_number in agent output), or explicit PR number (e.g. ${{ github.event.inputs.pr_number }}). Required when workflow is not triggered by a pull request (e.g. workflow_dispatch)."
},
"github-token": {
"$ref": "#/$defs/github_token",
"description": "GitHub token to use for this specific output type. Overrides global github-token if specified."
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/compiler_safe_outputs_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ var handlerRegistry = map[string]handlerBuilder{
c := cfg.SubmitPullRequestReview
return newHandlerConfigBuilder().
AddIfPositive("max", c.Max).
AddIfNotEmpty("target", c.Target).
AddStringPtr("footer", getEffectiveFooterString(c.Footer, cfg.Footer)).
Build()
},
Expand Down
3 changes: 3 additions & 0 deletions pkg/workflow/safe_outputs_target_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ func validateSafeOutputsTarget(config *SafeOutputsConfig) error {
if config.CreatePullRequestReviewComments != nil {
configs = append(configs, targetConfig{"create-pull-request-review-comment", config.CreatePullRequestReviewComments.Target})
}
if config.SubmitPullRequestReview != nil {
configs = append(configs, targetConfig{"submit-pull-request-review", config.SubmitPullRequestReview.Target})
}
if config.ReplyToPullRequestReviewComment != nil {
configs = append(configs, targetConfig{"reply-to-pull-request-review-comment", config.ReplyToPullRequestReviewComment.Target})
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/workflow/submit_pr_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ var submitPRReviewLog = logger.New("workflow:submit_pr_review")
// If this safe output type is not configured, review comments default to event: "COMMENT".
type SubmitPullRequestReviewConfig struct {
BaseSafeOutputConfig `yaml:",inline"`
Target string `yaml:"target,omitempty"` // Target PR: "triggering" (default), "*" (use message.pull_request_number), or explicit number e.g. ${{ github.event.inputs.pr_number }}
Footer *string `yaml:"footer,omitempty"` // Controls when to show footer in PR review body: "always" (default), "none", or "if-body" (only when review has body text)
}

Expand All @@ -31,6 +32,14 @@ func (c *Compiler) parseSubmitPullRequestReviewConfig(outputMap map[string]any)
// Parse common base fields with default max of 1
c.parseBaseSafeOutputConfig(configMap, &config.BaseSafeOutputConfig, 1)

// Parse target (same semantics as add-comment / create-pull-request-review-comment)
if target, exists := configMap["target"]; exists {
if targetStr, ok := target.(string); ok && targetStr != "" {
config.Target = targetStr
submitPRReviewLog.Printf("Target: %s", config.Target)
}
}

// Parse footer configuration (string: "always"/"none"/"if-body", or bool for backward compat)
if footer, exists := configMap["footer"]; exists {
switch f := footer.(type) {
Expand Down
Loading
Loading