Skip to content

Commit ad5e9bd

Browse files
Copilotnhorton
andauthored
Address review feedback on auto-fix-ci workflow robustness and security (#30)
* Initial plan * Address review comments on auto-fix-ci workflow Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com> * Fix trailing spaces in workflow file Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com> * Fix claude_args quoting and improve null check Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com> * Use YAML folded scalar for claude_args to avoid quoting issues Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com> * Fix context object consistency and claude_args formatting Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com> * Add null safety check to PR validation step Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com> * Add JSON parsing safety and quote claude_args properly Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com> * Use validated PR number from pr_check step throughout workflow Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com>
1 parent 24d906e commit ad5e9bd

1 file changed

Lines changed: 69 additions & 40 deletions

File tree

.github/workflows/auto-fix-ci.yml

Lines changed: 69 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,18 @@ jobs:
3232
uses: actions/github-script@v7
3333
with:
3434
script: |
35+
const pullRequests = ${{ toJSON(github.event.workflow_run.pull_requests) }};
36+
if (!pullRequests || pullRequests.length === 0) {
37+
console.log('No pull requests found');
38+
return { isOpen: false, prNumber: null };
39+
}
40+
const prNumber = pullRequests[0].number;
3541
const pr = await github.rest.pulls.get({
3642
owner: context.repo.owner,
3743
repo: context.repo.repo,
38-
pull_number: ${{ github.event.workflow_run.pull_requests[0].number }}
44+
pull_number: prNumber
3945
});
40-
return { isOpen: pr.data.state === 'open' };
46+
return { isOpen: pr.data.state === 'open', prNumber: prNumber };
4147
4248
- name: Checkout code
4349
if: fromJSON(steps.pr_check.outputs.result).isOpen
@@ -51,7 +57,9 @@ jobs:
5157
if: fromJSON(steps.pr_check.outputs.result).isOpen
5258
id: check_loop
5359
run: |
54-
# Check last 3 commits for auto-fix tag to be more robust
60+
# Check last 3 commits for auto-fix tag to prevent infinite loops.
61+
# We look back 3 commits as a lightweight safeguard: this is enough to catch
62+
# any recent auto-fix pushes triggered by this workflow without scanning the full history.
5563
RECENT_COMMITS=$(git log -3 --pretty=%s)
5664
if echo "$RECENT_COMMITS" | grep -q "\[auto-fix\]"; then
5765
echo "skip=true" >> $GITHUB_OUTPUT
@@ -60,14 +68,6 @@ jobs:
6068
echo "skip=false" >> $GITHUB_OUTPUT
6169
fi
6270
63-
- name: Setup git identity
64-
if: |
65-
fromJSON(steps.pr_check.outputs.result).isOpen &&
66-
steps.check_loop.outputs.skip != 'true'
67-
run: |
68-
git config --global user.email "claude[bot]@users.noreply.github.com"
69-
git config --global user.name "claude[bot]"
70-
7171
- name: Get CI failure details
7272
if: |
7373
fromJSON(steps.pr_check.outputs.result).isOpen &&
@@ -107,6 +107,8 @@ jobs:
107107
repo: context.repo.repo,
108108
job_id: job.id
109109
});
110+
// Truncate logs to 50000 characters to stay within Claude's context window
111+
// and GitHub API response size limits
110112
errorLogs.push({
111113
jobName: job.name,
112114
logs: logs.data.substring(0, 50000)
@@ -141,7 +143,7 @@ jobs:
141143
## Failure Information
142144
- Failed CI Run: ${{ fromJSON(steps.failure_details.outputs.result).runUrl }}
143145
- Failed Jobs: ${{ join(fromJSON(steps.failure_details.outputs.result).failedJobs, ', ') }}
144-
- PR Number: ${{ github.event.workflow_run.pull_requests[0].number }}
146+
- PR Number: ${{ fromJSON(steps.pr_check.outputs.result).prNumber }}
145147
- Branch: ${{ github.event.workflow_run.head_branch }}
146148
- Repository: ${{ github.repository }}
147149
@@ -168,7 +170,7 @@ jobs:
168170
${{ toJSON(fromJSON(steps.failure_details.outputs.result).errorLogs) }}
169171
```
170172
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
171-
claude_args: "--allowedTools 'Edit,MultiEdit,Write,Read,Glob,Grep,LS,Bash(git:*),Bash(uv:*),Bash(python:*),Bash(pytest:*),Bash(ruff:*)'"
173+
claude_args: "--allowedTools Edit,MultiEdit,Write,Read,Glob,Grep,LS,Bash(git commit:*),Bash(git push:*),Bash(git status:*),Bash(git diff:*),Bash(uv:*),Bash(python:*),Bash(pytest:*),Bash(ruff:*)"
172174

173175
- name: Comment on PR with fix status
174176
if: |
@@ -179,40 +181,67 @@ jobs:
179181
uses: actions/github-script@v7
180182
with:
181183
script: |
182-
const prNumber = ${{ github.event.workflow_run.pull_requests[0].number }};
184+
// Safely access pull request number with null check
185+
const pullRequests = ${{ toJSON(github.event.workflow_run.pull_requests) }};
186+
if (!pullRequests || pullRequests.length === 0) {
187+
console.log('No pull request found, skipping comment');
188+
return;
189+
}
190+
const prNumber = pullRequests[0].number;
191+
192+
// Safely access failure details with null check and JSON validation
193+
const failureDetails = '${{ steps.failure_details.outputs.result }}';
194+
if (!failureDetails || failureDetails.trim() === '' || failureDetails === 'undefined' || failureDetails === 'null') {
195+
console.log('No failure details available, skipping comment');
196+
return;
197+
}
198+
let failureData;
199+
try {
200+
failureData = JSON.parse(failureDetails);
201+
} catch (e) {
202+
console.log('Failed to parse failure details:', e);
203+
return;
204+
}
205+
183206
const claudeOutcome = '${{ steps.claude.outcome }}';
184-
const hasFailedJobs = ${{ fromJSON(steps.failure_details.outputs.result).hasFailedJobs }};
207+
const hasFailedJobs = failureData.hasFailedJobs;
185208
const runUrl = '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}';
186-
const failureUrl = '${{ fromJSON(steps.failure_details.outputs.result).runUrl }}';
209+
const failureUrl = failureData.runUrl;
187210
188211
let body;
189212
if (!hasFailedJobs) {
190-
body = `## CI Auto-Fix Skipped
191-
192-
The CI workflow failed but no individual jobs failed (possibly a setup/infrastructure issue).
193-
194-
- **Fix workflow run**: ${runUrl}
195-
- **Original failure**: ${failureUrl}
196-
197-
Manual investigation may be required.`;
213+
body = [
214+
'## CI Auto-Fix Skipped',
215+
'',
216+
'The CI workflow failed but no individual jobs failed (possibly a setup/infrastructure issue).',
217+
'',
218+
`- **Fix workflow run**: ${runUrl}`,
219+
`- **Original failure**: ${failureUrl}`,
220+
'',
221+
'Manual investigation may be required.'
222+
].join('\n');
198223
} else if (claudeOutcome === 'success') {
199-
body = `## CI Auto-Fix Attempted
200-
201-
Claude has analyzed the CI failure and attempted to fix the issues.
202-
203-
- **Fix workflow run**: ${runUrl}
204-
- **Original failure**: ${failureUrl}
205-
206-
Please review the changes pushed to this branch.`;
224+
body = [
225+
'## CI Auto-Fix Attempted',
226+
'',
227+
'Claude has analyzed the CI failure and attempted to fix the issues.',
228+
'',
229+
`- **Fix workflow run**: ${runUrl}`,
230+
`- **Original failure**: ${failureUrl}`,
231+
'',
232+
'Please review the changes pushed to this branch.'
233+
].join('\n');
207234
} else {
208-
body = `## CI Auto-Fix Failed
209-
210-
Claude attempted to fix the CI failure but encountered issues.
211-
212-
- **Fix workflow run**: ${runUrl}
213-
- **Original failure**: ${failureUrl}
214-
215-
Manual intervention may be required.`;
235+
body = [
236+
'## CI Auto-Fix Failed',
237+
'',
238+
'Claude attempted to fix the CI failure but encountered issues.',
239+
'',
240+
`- **Fix workflow run**: ${runUrl}`,
241+
`- **Original failure**: ${failureUrl}`,
242+
'',
243+
'Manual intervention may be required.'
244+
].join('\n');
216245
}
217246
218247
await github.rest.issues.createComment({

0 commit comments

Comments
 (0)