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
57 changes: 40 additions & 17 deletions .github/workflows/agent-fix.lock.yml

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

20 changes: 15 additions & 5 deletions .github/workflows/agent-fix.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ tools:
safe-outputs:
create-pull-request:
auto-merge: false
draft: true
draft: false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MODERATE — Duplicate review runs

With draft: false, the PR is created as non-draft immediately. This fires the pull_request: [opened] event, which triggers review-on-open.agent (it matches opened + draft == false). Then in Step 8, agent-fix also explicitly dispatches review.agent via workflow_dispatch.

Both workflows share the concurrency group review-$\{\{ PR_NUMBER }} with cancel-in-progress: false, so they serialize rather than deduplicate — the second review queues behind the first and runs in full. This doubles compute cost (~90 min × 2) and produces two separate review comment threads on the same PR for the same diff.

Mitigation options (pick one):

  1. Keep draft: true and rely solely on the explicit review.agent dispatch (original approach minus the approval-gate problem).
  2. Keep draft: false but remove the explicit dispatch_workflow for review.agent in Step 8, relying on review-on-open.agent to auto-trigger.
  3. Add a conditional label or environment flag so review-on-open.agent skips PRs created by agent-fix (e.g., if: github.actor != 'copilot-agentic-workflow[bot]').

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MODERATE — Duplicate reviews on bot-created PRs (Flagged by: 3/3 reviewers)

With draft: false, a non-draft PR created by the agent will immediately trigger review-on-open.agent (via pull_request: opened / ready_for_review). Then Step 8 also explicitly dispatches review.agent via workflow_dispatch.

Both workflows share the concurrency group review-<PR#> with cancel-in-progress: false, so they serialize rather than cancel — both run to completion, posting independent review comment sets on the same PR and doubling compute cost.

Concrete scenario: Agent-fix creates PR #100 (non-draft) → review-on-open.agent triggers on opened → Step 8 dispatches review.agent → two full expert reviews post on the same PR.

Suggested fix: Add a bot-user guard to review-on-open.agent (e.g., && github.event.pull_request.user.type != 'Bot') so it skips bot-created PRs where the explicit dispatch is the intended path. Alternatively, remove the explicit dispatch from Step 8 and rely solely on review-on-open.agent.

preserve-branch-name: true
protected-files: fallback-to-issue
add-comment:
max: 3
target: "*"
dispatch-workflow:
workflows: [polypilot-integration, verify-build]
max: 2
workflows: [polypilot-integration, verify-build, review.agent]
max: 3

timeout-minutes: 90

Expand Down Expand Up @@ -171,9 +171,9 @@ For each finding from the self-review:

Repeat Steps 6-7 up to **3 times** (max 3 review rounds).

## Step 8: Dispatch Integration Tests
## Step 8: Dispatch Integration Tests and Review

After all fixes are committed, dispatch the integration test workflows.
After all fixes are committed, dispatch the integration test workflows AND the expert code review.

**Important:** Use the exact branch name from the PR. If you named your branch `fix/issue-N`, the safe-outputs job will use that name without modification (because `preserve-branch-name: true` is set). If you're unsure, use `get_pull_request` to read the PR and get the `headRefName` field.

Expand All @@ -194,8 +194,17 @@ dispatch_workflow({
"scenario": "smoke"
}
})

dispatch_workflow({
Comment on lines +197 to +198
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 MINOR — Review dispatched concurrently with build; may review stale code (Flagged by: 1/3 reviewers initially, confirmed 3/3 after follow-up)

All three dispatches (verify-build, polypilot-integration, review.agent) fire simultaneously. If verify-build fails, auto-fix-on-failure dispatches fix.agent which pushes new commits — but the review has already started (or completed) on the pre-fix code. The resulting review comments will refer to code that no longer exists.

This is correctly a minor concern: the happy path (build passes) is unaffected, and adding sequencing would increase latency for every pipeline run. But it may be worth documenting as a known trade-off with a comment here.

"workflow": "review.agent",
"inputs": {
"pr_number": "<PR number>"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MODERATE — pr_number placeholder will be interpreted as a string (Flagged by: 3/3 reviewers)

The placeholder "<PR number>" is enclosed in double-quotes, making it look like a JSON string. The compiled lock file's schema for review_agent declares "pr_number": { "type": "number" }.

An LLM following this template will likely substitute as "pr_number": "42" (string) instead of "pr_number": 42 (number), which may fail strict JSON Schema validation in the safe-outputs handler.

Suggested fix: Remove the quotes to signal numeric intent:

"pr_number": <PR number as integer, e.g. 42>

}
})
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MODERATE — pr_number type mismatch between prompt example and tool schema

The prompt example passes pr_number as a string ("<PR number>"), but the compiled tool schema in the lock file defines pr_number with "type": "number". If the agent sends a string (e.g. "42" instead of 42), schema validation may reject the dispatch call at runtime.

The review.agent.md frontmatter also declares pr_number as type: number, so the lock file schema is correct. However, the prompt example should make it clear this must be an integer, not a quoted string.

Suggested fix: Change the example to use an unquoted placeholder:

"pr_number": <PR number>

or add a note: "Pass as an integer, not a string."


The review workflow runs a multi-model expert code review on the PR. This is dispatched via `workflow_dispatch` to bypass the approval gate that blocks `pull_request`-triggered workflows for bot-created PRs.

## Step 9: Post Summary

Post an `add_comment` on issue #${{ github.event.issue.number || inputs.issue_number }} with:
Expand All @@ -204,6 +213,7 @@ Post an `add_comment` on issue #${{ github.event.issue.number || inputs.issue_nu
- Test results (unit tests passed/failed count)
- Review summary (findings found and fixed)
- Integration test dispatch status
- Expert review dispatch status
- **For visual changes:** note that screenshots are available in the integration test CI artifacts (link to the workflow run)

## Rules
Expand Down
Loading