fix(dispatch): resolve PR head SHA for issue_comment events - #5274
Conversation
|
🤖 Finished Review · ✅ Success · Started 10:30 PM UTC · Completed 10:44 PM UTC |
PR Summary by QodoFix dispatch: resolve PR head SHA for issue_comment-triggered agents
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Site previewPreview: https://01a693a4-site.fullsend-ai.workers.dev Commit: |
Code Review by Qodo
Context used✅ Tickets:
🎫 feat(repos): glob patterns in repos.yaml exclude private repositories 🎫 Story 3: Triage Agent✅ Compliance rules (platform):
61 rules✅ Skills:
writing-user-docs, writing-adrs 1.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
Previous runReviewFindingsHigh
Low
Previous run (3)ReviewFindingsMedium
Low
Previous run (4)ReviewFindingsHigh
Low
|
|
/fs-fix fix all of the review issues uncovered, both by qodo and the fullsend-ai-bot. Squash the commit and rebase. |
d8bd4fc to
750b81c
Compare
|
🤖 Finished Review · ✅ Success · Started 11:04 PM UTC · Completed 11:20 PM UTC |
Superseded by updated review
waynesun09
left a comment
There was a problem hiding this comment.
Multi-agent review squad (Claude, Grok, Codex — 4 agents, findings cross-verified against the head branch). Posting the deduplicated MEDIUM+ findings as inline comments below; low/info items omitted.
Also verified clean, for the record: the fork-block consolidation does not regress security — the fix-stage check runs in every case the old step ran, stays fail-closed on API errors, the GITHUB_OUTPUT heredoc delimiter is injection-safe, and untrusted comment input never reaches shell interpolation.
750b81c to
a6ca0c0
Compare
|
🤖 Finished Review · ✅ Success · Started 3:42 PM UTC · Completed 3:58 PM UTC |
The dispatch jq expression reads .pull_request from the raw GitHub event, but issue_comment events have no top-level pull_request object (GitHub puts it under issue.pull_request with URL-only references). This caused "pull_request": null in the dispatched event_payload, so all downstream SHA resolution fell back to GITHUB_SHA — the config repo's checkout SHA, not the PR head. Add a "Resolve PR head" step that fetches the PR info from the API for issue_comment events and merges it into the event_payload. Also fix the action.yml reconcile step to extract the SHA from the event payload instead of using github.sha directly. The fork-PR blocking check for fix stage is consolidated into the same step to eliminate a duplicate API call and keep the scaffold dispatch.yml lint-workflow-size increase modest (475→495). Both scaffold dispatch.yml (per-org) and reusable-dispatch.yml (per-repo) are updated in sync per AGENTS.md requirements. Closes fullsend-ai/.fullsend#126 Signed-off-by: Greg Allen <gallen@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
a6ca0c0 to
f9af4d5
Compare
|
🤖 Finished Review · ✅ Success · Started 4:02 PM UTC · Completed 4:18 PM UTC |
waynesun09
left a comment
There was a problem hiding this comment.
Ran 3 independent reviews (Claude x2, Grok) plus my own verification, focused on whether the current state (after 4 rounds of fixes) is genuinely correct rather than just re-litigating resolved threads. One new HIGH finding below, confirmed by tracing the actual code myself. Also two minor LOW notes: the PR body cites #5271 as a "code-agent example" of this bug, but #5271 is unrelated (a repos.yaml glob-pattern feature) — the actual documented repro is fullsend-ai/.fullsend#126, already cited correctly elsewhere in the body, likely a copy-paste slip. And TestReusableDispatchPRHeadSHAPassthrough/the scaffold-alignment test check the two dispatch YAML files' "Resolve PR head" step against a fixed checklist independently rather than diffing them against each other — currently identical, but nothing would catch future one-sided drift in this specific step. Everything else (stage-aware error handling, fork-PR safety, injection surface, token scope, the two new prHeadSHAFromEventPath tests) checks out clean across all three reviews. Not blocking this PR given the maintainer approval and green CI, but worth a fast follow-up.
There was a problem hiding this comment.
Approve. Review-squad pass (3 agents) plus my own independent verification found one real HIGH gap (harness-run job doesn't receive the fix — filed as #5329, out of scope for this PR since it's a pre-existing bug in a different code path this PR doesn't touch) and two minor LOW notes (wrong issue reference in the PR body, missing cross-file test for the new step) — see inline review for details. Nothing here blocks this PR: the five stage jobs it targets are fixed correctly, verified end-to-end against the linked repro (fullsend-ai/.fullsend#126), and the fork-PR/security handling is sound.
|
🤖 Retro · ❌ Terminated · Started 7:08 PM UTC · Ended 7:25 PM UTC |
Retro: PR #5274 — fix(dispatch): resolve PR head SHA for issue_comment eventsPR #5274 is a human-authored infrastructure fix by ggallen, discovered while observing the code agent working on issue #5271. It fixes Workflow overview
Key quality gapThe review agent validated the fix for one deployment mode ( Proposals filed
Proposals filed
|
The harness-run job — which runs custom third-party agents registered via .fullsend/config.yaml — was not included in the #5274 fix that threaded pr-head-sha through to the five stage jobs (triage, code, review, fix, retro). Without it, comment-triggered custom harness agents fall through to the GITHUB_SHA fallback and operate on the wrong commit. Extract .pull_request.head.sha from matrix.event_payload (which the Go harnessdispatch package already populates correctly via a real API lookup) and pass it as pr-head-sha to the composite action, mirroring the existing stage jobs. Extend TestReusableDispatchPRHeadSHAPassthrough to cover harness-run alongside the five stage jobs. Closes #5329 Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Summary
/fs-review,/fs-code,/fs-fix) using the.fullsendconfig repo's checkout SHA instead of the target PR's head SHAdispatch.ymlthat fetches PR info from the API for issue_comment events and merges it into the event_payloadaction.yml's reconcile step to extract the SHA from the dispatched event payload instead of usinggithub.shadirectlyRoot cause
Comment-triggered agents (
/fs-review,/fs-code,/fs-fix) buildevent_payloadwith a jq filter over$GITHUB_EVENT_PATHthat only copies a top-level.pull_requestobject.issue_commentevents do not have that field — GitHub puts PR linkage under.issue.pull_request(URL-only) — so the payload gets"pull_request": null. Downstream steps then treat the config-repo checkout SHA (GITHUB_SHA) as the PR head: status comments show the wrong commit, andfullsend post-reviewsubmits reviews with acommit_idthat does not exist in the target repo (GitHub 422).Affects all comment-triggered classic-stage agents. See #5271 for a code-agent example.
Changes
internal/scaffold/fullsend-repo/.github/workflows/dispatch.ymlissue_commentevents; merge into event_payloadaction.ymlGITHUB_SHAinternal/cli/run_test.gopull_requestand resolved PR in event payloadTest plan
TestPRHeadSHAFromEventPath_NullPullRequest— documents the broken case (returns empty, doesn't panic)TestPRHeadSHAFromEventPath_IssueCommentWithResolvedPR— verifies SHA extraction after the dispatch fixTestPRHeadSHAFromEventPath_*andTestSetupStatusNotifier_*tests pass/fs-reviewon a PR shows the correct commit SHA in the status comment and submits the review successfullyCloses fullsend-ai/.fullsend#126