Issue #1152: fix: gate issue-sweep commit evidence on merge-base with main - #1153
Open
bjagg wants to merge 1 commit into
Open
Issue #1152: fix: gate issue-sweep commit evidence on merge-base with main#1153bjagg wants to merge 1 commit into
bjagg wants to merge 1 commit into
Conversation
…erge-base with main
The judge prompt accepted a commit SHA as proof an open issue was already
resolved, with no check that the commit is on `main`. A bare `git log --grep`
resolves from HEAD, so a sweep run from a feature branch — or an agent that
widens an empty search with `--all` — reads unmerged work as shipped. The
skill's stated evidence bar ("a merged PR #, a commit SHA, or a named shipped
file/symbol") is satisfied by that SHA, and the sweep's output is a close-list.
Verified live: issue LIF-Initiative#722 is open while two `Issue LIF-Initiative#722: …` commits sit on
`upstream/issue-722---Translator-performance`, neither on `main`. 190 commits
repo-wide are reachable but not on `main`, and the `Issue #NNN:` commit
convention means each matches a grep for its own issue number.
Two changes, both needed. The commands are now scoped (`git log main --grep`,
`git log main -S`), which fixes the root cause independent of checkout; and any
SHA must pass `git merge-base --is-ancestor <sha> main` before it can be cited,
which covers SHAs found in a PR body or issue comment. Widening an empty search
with `--all` is explicitly forbidden — an empty result from `main` is the answer.
A failed ancestor check returns open/uncertain with the branch named, rather
than dropping the finding: in-flight work is useful, it is just not a closure.
The arbiter previously said only "investigate the same way" and inherited the
gap, so it now re-runs the check on any commit either judge cited.
Documentation-only change to the skill file; no runtime code affected.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
Problem. The
issue-sweepskill accepted a commit SHA as proof that an open issue wasalready resolved, without checking the commit is on
main. Its judge prompt directs agents togit log --oneline --grep "#NNN"/git log -S "<symbol>"for commit evidence, and its statedevidence bar — "a merged PR #, a commit SHA, or a named shipped file/symbol" — is satisfied by a
SHA from an unmerged branch.
A bare
git log --grepresolves from HEAD, notmain, so the gap opens two ways: the sweepis run from a feature branch (HEAD carries that branch's unmerged commits), or the scoped search
comes back empty and the agent widens it with
--all, which is a natural move given the promptasks it to hunt for evidence. Because the sweep's output is a close-list, the failure mode is
proposing closure of an issue whose work is still on a branch.
Solution — two changes, both required:
git log main --oneline --grep "#N"andgit log main -S "<symbol>".This fixes the root cause independent of which branch the sweep runs from, and widening an
empty search with
--allis now explicitly forbidden ("an empty result frommainis theanswer").
git merge-base --is-ancestor <sha> mainbefore itcan be cited, wherever it was found — a PR body, an issue comment, a wider search.
A failed ancestor check returns
open/uncertainwith the branch named ("commit<sha>existson
<branch>, not merged to main") rather than dropping the finding — in-flight work is usefulinformation, it just isn't a closure. The arbiter previously said only "investigate the same
way" and inherited the gap, so it now re-runs the check on any commit either judge cited. The
#722case below is inline in the prompt as the worked example, and the rule is restated in theskill's Rules section.
Side effects / limitations. Documentation-only change to a
.claude/skill file; no runtimecode, no dependency, no schema. The
gh pr list --state merged --searchevidence path wasalready sound and is unchanged. This does not address two other known gaps in the same skill —
it runs two judges where overlap between two agents can't be measured, and its verdict enum has
no
PARTIAL— both left for separate PRs.How reviewers can test this. The bug is reproducible from a clean checkout, against a
currently-open issue:
Under the old prompt those SHAs are citable evidence that #722 is resolved. Under the new one the
ancestor check fails and the verdict stays open with the branch named. Repo-wide,
git log --all --oneline --not main | wc -lreports 190 such commits, and theIssue #NNN:commit convention means each matches a grep for its own issue number.
Related Issues
Closes #1152
Type of Change
Project Area(s) Affected
Checklist
uv run ruff check) — skipped by pre-commit, no Python filesuv run ruff format) — skipped by pre-commit, no Python filesuv run ty check)Testing
The changed text lives inside a JS template literal in a fenced code block, so escaping was the
real risk. Verification:
node --check— parses clean (once thetop-level
export/return, which are expected in a Workflow script, are accounted for).judgePrompt({number: 75, …})and printed the result, confirming the agent receivesreal backticks and a runnable
git merge-basecommand rather than escape artifacts.uv run pre-commit run --files .claude/skills/issue-sweep/SKILL.md— cspell, ty-check andpytest-test all pass; the two ruff hooks skip (no Python files in the change).
No automated tests added: the change is prompt text in a skill file, which the suite does not
cover.
Additional Notes
Found while auditing the LIF skills against a sibling project's more mature equivalents. This is
the same failure mode that project's backlog-sweep skill documented and fixed — its first
recorded wrong verdict was "an unmerged branch reported as shipped" — so the check is a known-good
one being brought over rather than a speculative hardening.