Skip to content

Commit 43258e9

Browse files
Copilotpelikhan
andauthored
fix: fix PR Sous Chef safe output failures by using safeoutputs CLI examples
The agent was confused by contradictory instructions: rule 9 said "do NOT use safeoutputs shell wrappers" while the system prompt instructed it to run `safeoutputs --help`. The agent tried `safeoutputs --help | sed ...` (blocked — sed not in allow list), then fell back to a full binary path (blocked), and gave up without emitting any safe outputs. Changes: - Rule 9: Replaced "do not use safeoutputs shell wrappers" with explicit instruction to USE `safeoutputs <tool> --param value` CLI commands and NOT to pipe them or run `--help` - noop example: Changed from JSON `{"noop":{...}}` to CLI `safeoutputs noop --message "..."` - add_comment example: Changed from JSON to `safeoutputs add_comment --pr_number` - Added update_pull_request example to prevent `gh pr update-branch` confusion - Updated contract test to check `pr_number 12345` (CLI) instead of `"pr_number":12345` (JSON) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent ac585e1 commit 43258e9

3 files changed

Lines changed: 13 additions & 9 deletions

File tree

.github/workflows/pr-sous-chef.lock.yml

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/pr-sous-chef.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ Move open non-draft PRs toward a state where a maintainer can investigate quickl
179179
6. If a `pr-processor` call returns non-JSON or an error, record `{pr_number: <N>, skip_reason: "sub_agent_error"}` in the `skipped` array of the run-summary noop payload and move to the next PR without retrying.
180180
7. Do not fetch full PR diffs or large file lists unless absolutely required for a skip decision.
181181
8. **Never finish without at least one safe-output tool call.** If you have not called `add_comment` or `update_pull_request`, you must call the run-summary `noop` (see **Run summary** below) before finishing.
182-
9. Call safe-output MCP tools directly (`add_comment`, `update_pull_request`, `push_to_pull_request_branch`, `noop`, `report_incomplete`). Do **not** use `gh pr comment`, `gh api ... -X POST`, or `safeoutputs ...` shell wrappers for write actions.
182+
9. Use `safeoutputs <tool> --param value` shell commands for all safe-output operations (`add_comment`, `update_pull_request`, `push_to_pull_request_branch`, `noop`, `report_incomplete`). Do **not** use `gh pr comment`, `gh pr update-branch`, `gh api ... -X POST`, or any GitHub API write calls outside of `safeoutputs`. Do **not** pipe `safeoutputs` to other commands or run `safeoutputs --help` — the tool schemas are already provided; use the examples below directly.
183183

184184
## Required skip rules per PR
185185

@@ -211,6 +211,10 @@ For each PR that is not skipped:
211211
1. **Update branch if possible**
212212
- If the PR is behind its base branch (or otherwise indicates branch update needed), attempt `update_pull_request` with `update_branch: true`.
213213
- Use a minimal append body marker so maintainers can trace the action, including `pr-sous-chef` and the run URL.
214+
- Example (`update_pull_request` shell call):
215+
```bash
216+
safeoutputs update_pull_request --pull_request_number 12345 --update_branch true --body "<!-- pr-sous-chef branch update -->" --operation append
217+
```
214218

215219
2. **Nudge unresolved review feedback**
216220
- Check pull request review threads/comments.
@@ -220,9 +224,9 @@ For each PR that is not skipped:
220224
- a short sentence asking Copilot to address unresolved review feedback.
221225
- Every `add_comment` must include `pr_number` set to the current PR's numeric `number` from the loop item.
222226
- Never emit `add_comment` without a numeric target field (`pr_number`/`pull_request_number`/`issue_number`/`item_number`) when `target: "*"` is configured.
223-
- Example (`add_comment` tool call):
224-
```json
225-
{"add_comment":{"pr_number":12345,"body":"<!-- gh-aw-pr-sous-chef-nudge -->\n@copilot review all comments and address unresolved review feedback."}}
227+
- Example (`add_comment` shell call):
228+
```bash
229+
safeoutputs add_comment --pr_number 12345 --body $'<!-- gh-aw-pr-sous-chef-nudge -->\n@copilot review all comments and address unresolved review feedback.'
226230
```
227231
228232
3. **Apply one additional forward-progress nudge**
@@ -244,10 +248,10 @@ At the end, call **exactly one** `noop` with a compact summary including counts
244248
- branch_update_attempts
245249
- formatter_pushes (number of PRs that had formatting fixes committed and pushed)
246250
247-
Example (`noop` tool call):
251+
Example (`noop` shell call):
248252
249-
```json
250-
{"noop":{"message":"processed=4; skipped_checks_running=0; skipped_last_comment_from_sous_chef=2; nudged_review_comments=1; nudged_other=1; branch_update_attempts=0; formatter_pushes=0"}}
253+
```bash
254+
safeoutputs noop --message "processed=4; skipped_checks_running=0; skipped_last_comment_from_sous_chef=2; nudged_review_comments=1; nudged_other=1; branch_update_attempts=0; formatter_pushes=0"
251255
```
252256
253257
## Formatting Requirements

pkg/cli/pr_sous_chef_workflow_contract_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func TestPRSousChefWorkflowAddCommentTargetContract(t *testing.T) {
2525
text := string(content)
2626
assert.Contains(t, text, "Every `add_comment` must include `pr_number`", "Workflow must require explicit pr_number in add_comment")
2727
assert.Contains(t, text, "Never emit `add_comment` without a numeric target field", "Workflow must forbid targetless add_comment items")
28-
assert.Contains(t, text, "\"pr_number\":12345", "Workflow should include a concrete add_comment pr_number example")
28+
assert.Contains(t, text, "pr_number 12345", "Workflow should include a concrete add_comment pr_number example")
2929
assert.Contains(t, text, "Process at most **5 PRs** per run.", "Workflow should cap per-run PR processing to 5")
3030
assert.Contains(t, text, "Make at most 8 tool calls total.", "Sub-agent should have a hard tool-call budget")
3131
assert.Contains(t, text, "skip_reason: \"sub_agent_error\"", "Workflow should skip failed sub-agent responses without retry")

0 commit comments

Comments
 (0)