-
Notifications
You must be signed in to change notification settings - Fork 295
Description
Summary
Analysis of the last 24 hours of workflow runs identified 2 errors where the agent incorrectly used the create_pull_request_review_comment safe output tool. The workflow prompt is correct, but the tool schema is missing the pull_request_number field — which is required by the handler when the workflow uses target: "*" configuration.
Error Analysis
Error Pattern: Missing pull_request_number when target: "*" is configured
Occurrences: 2 times in 1 workflow run
What happened: The agent called create_pull_request_review_comment with path, line, and body fields — exactly as described in the tool schema. But the handler failed because it required pull_request_number when target: "*" is configured.
Example from workflow smoke-claude (Run §23090184937):
{
"type": "create_pull_request_review_comment",
"path": ".github/workflows/ace-editor.lock.yml",
"line": 357,
"body": "..."
}
```
**Handler error**:
```
##[warning]Target is "*" but no pull_request_number specified in comment item
##[error]✗ Message 4 (create_pull_request_review_comment) failed: Target is "*" but no pull_request_number specifiedWhy the agent couldn't fix it: The tool schema uses "additionalProperties": false, so even if the agent knew pull_request_number was needed, it could not add it — the MCP protocol would reject the unknown field.
Key Comparison with Other Tools
Other PR-targeting tools include pull_request_number in their schemas:
| Tool | Has pull_request_number? |
Notes |
|---|---|---|
update_pull_request |
✅ Yes | "Required when the workflow target is '*' (any PR)" — also auto-resolves from context |
push_to_pull_request_branch |
✅ Yes | "Required when the workflow target is '*' (any PR)" |
add_reviewer |
✅ Yes | Documented as required for non-PR triggers |
close_pull_request |
✅ Yes | |
create_pull_request_review_comment |
❌ Missing | Handler requires it for target: "*" but schema doesn't define it |
Current Tool Description
Current description from safe_outputs_tools.json
{
"name": "create_pull_request_review_comment",
"description": "Create a review comment on a specific line of code in a pull request. Use this for inline code review feedback, suggestions, or questions about specific code changes.",
"inputSchema": {
"type": "object",
"required": ["path", "line", "body"],
"properties": {
"path": { "type": "string", "description": "File path relative to the repository root..." },
"line": { "type": ["number", "string"], "description": "Line number for the comment..." },
"body": { "type": "string", "description": "Review comment content in Markdown..." },
"start_line": { ... },
"side": { ... },
"secrecy": { ... },
"integrity": { ... }
},
"additionalProperties": false
}
}Note: pull_request_number is completely absent from the schema.
Root Cause Analysis
- Missing field in schema:
pull_request_numberis not in the tool'sinputSchema.properties, so the MCP server rejects it if provided. additionalProperties: false: Agents literally cannot includepull_request_numbereven if they knew it was needed.- Handler requires it:
create_pr_review_comment.cjs(line 155–169) checks forcommentItem.pull_request_numberwhentarget: "*"and fails if absent — unlikeupdate_pull_requestwhich auto-resolves from context. - Inconsistency: All similar PR-targeting tools expose
pull_request_number; this tool is the only exception.
Recommended Improvements
1. Add pull_request_number to the tool schema
Add to pkg/workflow/js/safe_outputs_tools.json → create_pull_request_review_comment.inputSchema.properties:
"pull_request_number": {
"type": ["number", "string"],
"description": "Pull request number to add the review comment to. This is the numeric ID from the GitHub URL (e.g., 876 in github.com/owner/repo/pull/876). If omitted, adds the comment to the PR that triggered this workflow. Required when the workflow target is '*' (any PR) — omitting it will cause the comment to fail."
}2. Update the tool description to mention target: "*" behavior
Current description:
"Create a review comment on a specific line of code in a pull request."
Suggested:
"Create a review comment on a specific line of code in a pull request. When the workflow is configured with
target: \"*\", you must specifypull_request_numberto indicate which PR to target."
3. Also update safe_output_validation_config.go
Add to the create_pull_request_review_comment fields map:
"pull_request_number": {OptionalPositiveInteger: true},4. (Long-term) Auto-resolve PR from context
See issue #20814 for a complementary handler fix that auto-resolves the triggering PR when target: "*" and no pull_request_number is provided (matching update_pull_request behavior). This tool description fix is still needed for workflows that genuinely target arbitrary PRs.
Affected Workflows
smoke-claude— 2 errors (run §23090184937)
Implementation Checklist
- Add
pull_request_numberfield tocreate_pull_request_review_commentinpkg/workflow/js/safe_outputs_tools.json - Add
pull_request_numberto validation config inpkg/workflow/safe_output_validation_config.go - Update the tool description to mention
target: "*"requirement - Sync to
actions/setup/js/safe_outputs_tools.json(if separate copy) - Run
make buildto rebuild binary - Run
make recompileto update all workflows - Run
make testto ensure no regressions - Verify
smoke-claudeworks correctly after fix
References
- Tool schema:
pkg/workflow/js/safe_outputs_tools.json - Handler:
actions/setup/js/create_pr_review_comment.cjs(lines 155–169) - Validation config:
pkg/workflow/safe_output_validation_config.go - Related handler fix: [deep-report] Fix PR target auto-resolution for create_pull_request_review_comment #20814 (Fix PR target auto-resolution for
create_pull_request_review_comment)
Run IDs with errors: §23090184937
Generated by Daily Safe Output Tool Optimizer · ◷
- expires on Mar 16, 2026, 3:48 PM UTC