Skip to content

Commit 0c989dc

Browse files
committed
Address PR #101 review feedback from copilot-pull-request-reviewer
This commit addresses all 4 review comments from #101 (review) --- ## .github/workflows/codegen-check.yml ### Review comment (line 128): --ref "$BRANCH" will fail for PRs that don't contain the workflow file Copilot said: `gh workflow run` with `--ref "$BRANCH"` requires `codegen-agentic-fix.lock.yml` to exist on the PR branch. Dependabot or codegen-only PRs branched before this workflow was added won't have it, causing "workflow not found". Fix applied: Removed `--ref "$BRANCH"` from the `gh workflow run` call. GitHub now resolves the workflow file from main (default branch). The target branch is already passed via `-f branch="$BRANCH"` input. ### Review comment (line 95): Downstream steps run even when regen push fails Copilot said: `continue-on-error: true` on the push step means downstream steps (mvn verify, agentic fix trigger) run against a branch that doesn't have the regenerated code, so the agent can't reproduce the failure. Fix applied: Added a new step "Fail if regenerated files could not be pushed" that exits with a clear error message and manual fix instructions when the push is rejected (Dependabot read-only token, fork PRs). Gated Java setup, mvn verify, and agentic fix trigger on `steps.push-regen.outcome == 'success'`. ## .github/workflows/update-copilot-dependency.yml ### Review comment (line 211): Same --ref "$BRANCH" issue as codegen-check.yml Copilot said: Same dispatch-from-PR-branch problem. Fix applied: Removed `--ref "$BRANCH"` from the `gh workflow run` call, identical to the codegen-check.yml fix. ## .github/workflows/codegen-agentic-fix.md ### Review comment (line 85): mvn verify output truncated by tail -100 Copilot said: `mvn verify 2>&1 | tail -100` hides root compilation errors that appear early in the log. The agent may loop unproductively without seeing the actual cause. Fix applied: Changed both `mvn verify` invocations (Step 1 reproduce and Step 2 verify) from `tail -100` to `tee /tmp/mvn-verify.log`. Added guidance telling the agent to check the full log file for root causes. Recompiled the lockfile with `gh aw compile`. ## .github/workflows/codegen-agentic-fix.lock.yml Recompiled via `gh aw compile` to reflect the updated .md instructions. Signed-off-by: Ed Burns <edburns@microsoft.com>
1 parent d5721ae commit 0c989dc

3 files changed

Lines changed: 16 additions & 7 deletions

File tree

.github/workflows/codegen-agentic-fix.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,11 @@ mvn --version
8282
Run `mvn verify` to see the current errors:
8383

8484
```bash
85-
mvn verify 2>&1 | tail -100
85+
mvn verify 2>&1 | tee /tmp/mvn-verify.log
8686
```
8787

88+
Review the full log at `/tmp/mvn-verify.log` if the tail output is insufficient. The earliest errors are often the root cause.
89+
8890
If `mvn verify` succeeds (exit code 0), there is nothing to fix. Call the `noop` safe-output with message "mvn verify already passes on branch ${{ inputs.branch }}. No fixes needed." and stop.
8991

9092
### Step 2: Analyze and fix (up to 3 attempts)
@@ -111,9 +113,11 @@ For each attempt:
111113

112114
4. **Verify the fix:**
113115
```bash
114-
mvn verify 2>&1 | tail -100
116+
mvn verify 2>&1 | tee /tmp/mvn-verify.log
115117
```
116118

119+
If the output is long, check `/tmp/mvn-verify.log` for the full error details — root causes often appear early in the log.
120+
117121
5. If `mvn verify` passes, proceed to Step 3.
118122
If it fails and you have attempts remaining, go back to sub-step 1.
119123

.github/workflows/codegen-check.yml

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,23 @@ jobs:
8585
Auto-committed by codegen-check workflow."
8686
git push origin "HEAD:$HEAD_REF"
8787
88+
- name: Fail if regenerated files could not be pushed
89+
if: steps.push-regen.outcome == 'failure'
90+
run: |
91+
echo "::error::Could not push regenerated files to the PR branch. This is expected for Dependabot PRs (read-only token) and fork PRs."
92+
echo "To fix: check out this PR branch locally, run 'cd scripts/codegen && npm run generate', commit, and push."
93+
exit 1
94+
8895
- uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5
89-
if: steps.check-changes.outputs.changed == 'true' && github.event_name == 'pull_request'
96+
if: steps.push-regen.outcome == 'success'
9097
with:
9198
java-version: "17"
9299
distribution: "microsoft"
93100
cache: "maven"
94101

95102
- name: Run mvn verify
96103
id: mvn-verify
97-
if: steps.check-changes.outputs.changed == 'true' && github.event_name == 'pull_request'
104+
if: steps.push-regen.outcome == 'success'
98105
continue-on-error: true
99106
env:
100107
COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }}
@@ -113,7 +120,7 @@ jobs:
113120
114121
- name: Trigger agentic fix workflow
115122
id: trigger-fix
116-
if: steps.error-summary.outputs.has_errors == 'true'
123+
if: steps.error-summary.outputs.has_errors == 'true' && steps.push-regen.outcome == 'success'
117124
env:
118125
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
119126
PR_NUMBER: ${{ github.event.pull_request.number }}
@@ -122,7 +129,6 @@ jobs:
122129
ERROR_SUMMARY=$(cat /tmp/error-summary.txt)
123130
124131
gh workflow run codegen-agentic-fix.lock.yml \
125-
--ref "$BRANCH" \
126132
-f branch="$BRANCH" \
127133
-f pr_number="$PR_NUMBER" \
128134
-f error_summary="$ERROR_SUMMARY"

.github/workflows/update-copilot-dependency.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,6 @@ jobs:
205205
run: |
206206
ERROR_SUMMARY=$(cat /tmp/error-summary.txt)
207207
gh workflow run codegen-agentic-fix.lock.yml \
208-
--ref "$BRANCH" \
209208
-f branch="$BRANCH" \
210209
-f pr_number="$PR_NUMBER" \
211210
-f error_summary="$ERROR_SUMMARY"

0 commit comments

Comments
 (0)