fix(evals): use merge commit parent for skill discovery diff base

[mrizzi]

Summary

• Replace pr.base.sha (GitHub API baseRefOid) with HEAD^1 in the eval-pr-run workflow's skill discovery step
• Remove stale base_sha output from the PR identity step
• Add reproducer test that simulates the merge-commit topology and verifies correct skill detection

Bug

Fixes TC-5101 — when baseline commits landed on main after the PR branch forked, the three-dot diff included those files and falsely detected all 8 skills as changed.

Implements TC-5102

Test plan

• Reproducer test (tests/test-skill-discovery.sh) passes — verifies old approach over-detects and new approach detects only PR-changed skills
• CI eval run on a future PR that changes a single skill should detect only that skill

Summary by Sourcery

Adjust eval PR workflow skill discovery to diff against the merge commit’s first parent and add coverage for the refs/pull/N/merge topology.

Bug Fixes:

• Ensure skill discovery ignores baseline commits landed on main after the PR branch fork by using HEAD^1 as the diff base.

Tests:

• Add a shell-based reproducer test that simulates a merge-commit PR topology and verifies only PR-changed skills are detected.

[sourcery-ai]

Reviewer's Guide

This PR changes the eval-pr-run workflow to base skill discovery diffs on the merge commit’s first parent (HEAD^1) instead of the PR’s base SHA from GitHub, and adds a reproducer shell test that simulates the refs/pull/N/merge topology to verify correct skill detection.

Sequence diagram for updated skill discovery diff base in eval-pr-run workflow

sequenceDiagram
  actor GithubActions
  participant PRIdentityStep
  participant SkillDiscoveryStep
  participant Git

  GithubActions->>PRIdentityStep: actions/github-script
  PRIdentityStep->>GithubActions: setOutput pr_number
  PRIdentityStep->>GithubActions: setOutput author

  GithubActions->>SkillDiscoveryStep: run Discover changed skills
  SkillDiscoveryStep->>Git: git rev-parse HEAD^1
  Git-->>SkillDiscoveryStep: BASE_SHA
  SkillDiscoveryStep->>Git: git diff --name-only BASE_SHA...HEAD
  Git-->>SkillDiscoveryStep: changed_files
  SkillDiscoveryStep->>GithubActions: echo changed_files and detect skills

Loading

File-Level Changes

Change	Details	Files
Update skill discovery diff base in eval-pr-run workflow to use merge commit parent instead of GitHub PR base SHA.	Remove the base_sha output from the PR identity step in the eval-pr-run GitHub Actions workflow. Change the skill discovery step to compute BASE_SHA via git rev-parse HEAD^1 instead of consuming a PR base SHA from workflow outputs. Keep the diff calculation as git diff --name-only "$BASE_SHA"...HEAD while changing only how BASE_SHA is derived.	.github/workflows/eval-pr-run.yml
Add an end-to-end shell test that reproduces the merge-commit topology and validates skill discovery behavior.	Create a temporary git repository that mimics refs/pull/N/merge: initial main, PR branch, baseline commits on main, and merge commits. Embed a Bash implementation of the skill discovery logic using git diff and regex extraction of skills from plugins/ and evals/ paths. Add assertions that the old base SHA approach over-detects skills while the new HEAD^1 approach detects only PR-touched skills, and that both evals/ and plugins/ paths are correctly recognized.	tests/test-skill-discovery.sh

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• Using BASE_SHA=$(git rev-parse HEAD^1) assumes the workflow always runs on a merge commit; consider falling back to the previous behavior or a more defensive strategy (e.g., detect when HEAD is not a merge commit and choose an appropriate base) to avoid failures or incorrect diffs in other trigger contexts.
• The skill discovery logic is now duplicated between the GitHub workflow and tests/test-skill-discovery.sh; consider extracting this logic into a shared script that both the workflow and the test invoke to reduce drift and keep future changes centralized.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Using `BASE_SHA=$(git rev-parse HEAD^1)` assumes the workflow always runs on a merge commit; consider falling back to the previous behavior or a more defensive strategy (e.g., detect when `HEAD` is not a merge commit and choose an appropriate base) to avoid failures or incorrect diffs in other trigger contexts.
- The skill discovery logic is now duplicated between the GitHub workflow and `tests/test-skill-discovery.sh`; consider extracting this logic into a shared script that both the workflow and the test invoke to reduce drift and keep future changes centralized.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[mrizzi]

[sdlc-workflow/verify-pr] Re: @sourcery-ai[bot] review — Classified as suggestion (2 items):

1. Defensive HEAD^1 fallback — this proposes adding fallback logic for non-merge-commit contexts. However, the workflow exclusively runs on refs/pull/N/merge checkouts (guaranteed by GitHub to be merge commits), so HEAD^1 is always valid. No documented convention or codebase pattern mandates defensive git strategies. No sub-task created.

2. Extract discovery logic to shared script — this proposes reducing duplication between the workflow and test. CONVENTIONS.md documents no established pattern for shared CI scripts, and the test intentionally duplicates the logic to serve as an independent reproducer. No sub-task created.

[mrizzi]

Verification Report for TC-5102 (commit 290ea54)

Check	Result	Details
Review Feedback	PASS	2 suggestions from sourcery-ai — no code change requests
Root-Cause Investigation	N/A	No sub-tasks created
Scope Containment	WARN	Out-of-scope tests/test-skill-discovery.sh added (reproducer test, justified by acceptance criteria)
Diff Size	PASS	2 files, proportionate to bug fix + reproducer test
Commit Traceability	PASS	Commit 290ea54 references "Implements TC-5102"
Sensitive Patterns	PASS	No secrets or credentials detected
CI Status	PASS	All 3 checks pass (Plugin Validation, Skill Lint, Sourcery)
Acceptance Criteria	PASS	5 of 5 criteria met
Test Quality	PASS	Eval Quality: N/A
Test Change Classification	ADDITIVE	New test file only, no existing tests modified
Verification Commands	N/A	CI-only command (git diff HEAD^1...HEAD), covered by reproducer test

Overall: PASS

All acceptance criteria are satisfied. The workflow fix correctly replaces the stale pr.base.sha with HEAD^1, and the reproducer test verifies the fix against the exact merge-commit topology that triggered the bug. The two sourcery-ai suggestions (defensive fallback, shared script extraction) remain classified as suggestions — neither matches a documented convention.

---

This comment was AI-generated by sdlc-workflow/verify-pr v0.12.2.