Repository navigation
fix(ci): bind eval merge source to the live main tip - #358
Conversation
The Eval PR Run trust check compared the merge commit's first parent with the PR object's base.sha, which only refreshes on push/sync, while GitHub recomputes merge_commit_sha on the live main tip. Any main advance without a PR sync failed Discover PR Context and skipped all evals. Require parents[0] to equal the live main tip read from heads/main (stricter: older main commits are refused), with a bounded re-read (3 attempts, 2000 ms) on a tip mismatch only that re-runs every identity check. base_sha is now the verified tip. The native WIF recheck no longer compares the stale base.sha; merge_commit_sha still pins both parents. Implements TC-7209 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideThe workflow now securely binds eval execution to a merge commit whose first parent is the current live main tip and whose second parent is the event head, with bounded race recovery and complete identity revalidation. The native approval recheck no longer relies on stale pr.base.sha, and comprehensive tests cover valid, invalid, and concurrent-update scenarios. Sequence diagram for live main tip merge verificationsequenceDiagram
participant Workflow
participant GitHubAPI
participant Main
Workflow->>GitHubAPI: pulls.get(pr)
GitHubAPI-->>Workflow: PR identity and merge_commit_sha
Workflow->>GitHubAPI: git.getCommit(merge_commit_sha)
GitHubAPI-->>Workflow: merge.parents
Workflow->>GitHubAPI: git.getRef(ref: heads/main)
GitHubAPI-->>Workflow: live main tip
alt parents[1].sha == event head and parents[0].sha == live main tip
Workflow->>Workflow: set base_sha = live main tip
Workflow-->>Workflow: continue eval
else parent or tip mismatch
alt attempts remain
Workflow->>Workflow: wait 2000 ms
Workflow->>GitHubAPI: pulls.get(pr)
GitHubAPI-->>Workflow: revalidated PR identity
Workflow->>GitHubAPI: git.getCommit(merge_commit_sha)
Workflow->>GitHubAPI: git.getRef(ref: heads/main)
else retry limit reached or invalid data
Workflow-->>Workflow: fail closed
end
end
Sequence diagram for native approval revision rechecksequenceDiagram
participant Workflow
participant GitHubAPI
Workflow->>GitHubAPI: pulls.get(PR_NUMBER)
GitHubAPI-->>Workflow: PR state, branch, head, merge_commit_sha
alt identity and merge_commit_sha match recorded values
Workflow-->>Workflow: proceed to WIF and native eval
else mismatch or changed branch
Workflow-->>Workflow: fail closed
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="plugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py" line_range="96-98" />
<code_context>
createReview:async r=>{reviews.push(r);storedReviews.push({...r,id:storedReviews.length+1,user:{login:'github-actions[bot]'}});},
listFiles:async()=> (data.paths||[]).map(filename=>({filename}))},
- git:{getCommit:async()=>({data:{parents:(data.parents||['b'.repeat(40),'a'.repeat(40)]).map(sha=>({sha}))}})},
+ git:{getCommit:async()=>{
+ const parents=data.parentsSeq ? data.parentsSeq[Math.min(commitReads++,data.parentsSeq.length-1)] : data.parents;
+ return {data:{parents:(parents||['b'.repeat(40),'a'.repeat(40)]).map(sha=>({sha}))}};},
+ getRef:async()=>{if(data.refError) throw Error('ref unavailable');
+ const tip=data.mainTips ? data.mainTips[Math.min(refReads++,data.mainTips.length-1)] : data.mainTip;
</code_context>
<issue_to_address>
**Wrong Git requests pass tests**
When the workflow calls `getCommit` with a SHA other than the PR merge SHA, or `getRef` with a ref other than `heads/main`, `test_source_resolution_binds_merge_to_live_main_tip_despite_stale_base` (lines 142–143) and the retry/negative tests still pass if the workflow requests the wrong commit or ref: these stubs ignore the request arguments and return the configured parents and tip regardless.
Have the stubs capture and assert the requested commit SHA and ref so the tests fail when either resource selector is wrong.
Also at `plugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py:99-101`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the merge-parent/live-tip binding is wrong, a credentialed evaluation could run against a merge built from an unintended base. Reverting would stop future runs, but it cannot undo credentials issued or actions already taken by a run that passed the check.
Blocking findings: plugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py:98
| git:{getCommit:async()=>{ | ||
| const parents=data.parentsSeq ? data.parentsSeq[Math.min(commitReads++,data.parentsSeq.length-1)] : data.parents; | ||
| return {data:{parents:(parents||['b'.repeat(40),'a'.repeat(40)]).map(sha=>({sha}))}};}, |
There was a problem hiding this comment.
🟡 Medium · Wrong Git requests pass tests
When the workflow calls getCommit with a SHA other than the PR merge SHA, or getRef with a ref other than heads/main, test_source_resolution_binds_merge_to_live_main_tip_despite_stale_base (lines 142–143) and the retry/negative tests still pass if the workflow requests the wrong commit or ref: these stubs ignore the request arguments and return the configured parents and tip regardless.
Have the stubs capture and assert the requested commit SHA and ref so the tests fail when either resource selector is wrong.
Also at plugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py:99-101.
Prompt for AI agents
In `plugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py` at lines 96-98:
**Wrong Git requests pass tests**
When the workflow calls `getCommit` with a SHA other than the PR merge SHA, or `getRef` with a ref other than `heads/main`, `test_source_resolution_binds_merge_to_live_main_tip_despite_stale_base` (lines 142–143) and the retry/negative tests still pass if the workflow requests the wrong commit or ref: these stubs ignore the request arguments and return the configured parents and tip regardless.
Have the stubs capture and assert the requested commit SHA and ref so the tests fail when either resource selector is wrong.
Also at `plugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py:99-101`.
There was a problem hiding this comment.
[sdlc-workflow/verify-pr] Classified as suggestion - hardening the GitHub API test doubles to capture and assert the requested commit SHA / ref is a reasonable test-robustness improvement, but it is not required for correctness: the new tests already exercise the live-main-tip binding, race re-read, and fail-closed paths, and neither CONVENTIONS.md nor an established codebase stub pattern requires argument-asserting read stubs (the existing API doubles in this suite are all response-only). No sub-task created.
|
🤖 Finished Verify Pr · ✅ Success · Started 8:56 AM UTC · Completed 9:09 AM UTC Commit: Runtime: claude · Model: claude-opus-4-8 · Effort: high · Cost: $6.05 |
|
[sdlc-workflow/verify-pr] Re: @sourcery-ai[bot] review - Classified as suggestion - the review raises a single point: the GitHub API test doubles should assert the requested commit SHA / ref so wrong-resource requests fail. This is an optional test-hardening improvement, not required for correctness, and is not backed by CONVENTIONS.md or an established codebase stub pattern (existing API doubles in this suite are response-only). No sub-task created. |
Verification Report for TC-7209 (commit 8f8e212)
Overall: PASSAll verification checks pass. The PR cleanly implements the TC-7207 fix: the Eval PR Run trust check now binds the merge commit's first parent to the live One Sourcery review item (inline + review body, the same point) suggests the GitHub API test doubles assert their requested commit SHA/ref. It is classified as an optional suggestion - a reasonable test-hardening idea that is not required for correctness and is not backed by CONVENTIONS.md or an established codebase stub pattern (the existing API doubles in this suite are response-only). No sub-task was created. Note: the sandbox working tree was on This comment was AI-generated by sdlc-workflow/verify-pr v0.13.10. |
Summary
parents[0]must now equal the livemaintip (git.getRef heads/main) instead of the PR object'sbase.sha, which goes stale whenevermainadvances without a PR sync (TC-7207). This is stricter than before: an oldermaincommit is refused, even one inmain's history.parents[0]/tip mismatch triggers a bounded re-read (3 attempts, 2000 ms apart, same as the existingmerge_commit_sha === nullpoll). Each re-read re-runs every PR identity check.base_shaoutput = verified live tip (==merge.parents[0].sha), so the native report binding matches the merge commit thatrun-evals/run-native-evalscheck out.pr.base.shacomparison and the now-unusedBASE_SHAenv.merge_commit_shastill pins both parents, so the recheck still fails closed ifmainadvances during approval.Tests (
test_native_fullsend_eval_ci.py)run_jsdouble gainsgit.getRef(mainTip/mainTips/refError) and a per-readgetCommitsequence (parentsSeq).[tip, head]). It fails on currentmainand passes with the fix.python3 -m pytest plugins/sdlc-workflow/scripts/ -q→ 753 passed, 3 skipped.uvx skillsaw→ 0 errors.claude plugin validate→ passed.Implements TC-7209
🤖 Generated with Claude Code
Summary by Sourcery
Bind evaluation workflows to merges whose first parent is the verified live
maintip and fail closed when the merge source or pull request identity cannot be trusted.Bug Fixes:
maintip, preventing stale or unrelated base commits from reaching credentialed jobs.Enhancements:
maintip while reapplying pull request identity checks on each retry.Tests: