Skip to content

fix: store the FULL open-PR set in Status.SourceBranches, not just the delta - #1

Merged
ZacxDev merged 1 commit into
feat/label-retriggerfrom
zach/store-full-sourcebranches
Jul 6, 2026
Merged

fix: store the FULL open-PR set in Status.SourceBranches, not just the delta#1
ZacxDev merged 1 commit into
feat/label-retriggerfrom
zach/store-full-sourcebranches

Conversation

@ZacxDev

@ZacxDev ZacxDev commented Jul 6, 2026

Copy link
Copy Markdown

Problem (production incident 2026-07-06)

Reconcile polls GitHub for the current open-PR set (newBranches, the FULL list) and then stored only the delta into the authoritative state the downstream pipeline-trigger-operator reads:

pullrequest.Status.SourceBranches.Branches = setDifferences  // only the newly-added branches

Status.SourceBranches is supposed to be the complete set of open PRs. Storing only setDifferences truncates it:

  • PR opened (list grows) → stores just the 1 new PR → SourceBranches becomes a partial list of 1.
  • PR closed (list shrinks) → setDifferences is empty → SourceBranches becomes empty.

Either way the consumer sees a truncated branch set and rebuilds every open PR (~188 PipelineRuns) despite no new commits, and the next poll re-diffs against the truncated state → endless "New PR received" churn. This is the producer half of the incident; the consumer-side guard already shipped.

Fix

Store the FULL current list while still emitting New PR ... received events only for the genuinely-new PRs. The store-decision is extracted into a tiny pure helper:

func nextSourceBranches(current, polled Branches) (store Branches, newlyAdded []Branch) {
    newlyAdded = current.BranchSetDifference(polled) // events fire only for these
    store = Branches{Branches: polled.Branches}      // authoritative state = FULL polled list
    return
}

Orthogonal to this branch's label-retrigger customization: newBranches already carries the label-folded commit discriminators from the poller, so a label change still shows up as a new entry and still retriggers. No other logic changed.

Test

controllers/pullrequest_controller_helpers_test.goTestNextSourceBranches drives the helper the way Reconcile does across successive polls (initial population → add → remove) and asserts SourceBranches always equals the FULL polled list while events fire only for genuinely-new PRs.

Verified both directions:

  • Passes on the fix.
  • Fails on the original delta-only code (temporarily reintroduced): add case stores {c} (want {a,b,c}), remove case stores {} (want {a,b}).

go build ./... and go test ./pkg/... ./api/... ./controllers/... are green. The pre-existing controllers/suite_test.go Ginkgo/envtest suite (TestAPIs) has no specs, so it passes trivially without envtest binaries. CI (.github/workflows/test.yml) gains a targeted go test ./controllers/... -run TestNextSourceBranches step (the full envtest suite is still excluded, as before).

⚠️ Note on other branches

main and gh-conditional-requests carry the identical upstream bug (= setDifferences). This PR only fixes the deployed civitai branch feat/label-retrigger; those branches are not touched here.

🤖 Generated with Claude Code

…e delta

Reconcile stored `pullrequest.Status.SourceBranches.Branches = setDifferences`
— only the branches newly added since the last poll — into the authoritative
open-PR set that the downstream pipeline-trigger-operator reads. So:

- PR opened  → SourceBranches truncated to the 1 new PR
- PR closed  → setDifferences empty → SourceBranches blanked entirely

Either way the consumer saw a truncated branch set and rebuilt every open PR
(~188 PipelineRuns) with no new commits, and the next poll re-diffed against
the truncated state → endless "New PR received" churn (incident 2026-07-06).

Fix: store the full polled list while still emitting events only for the
genuinely-new PRs. Extracted the store-decision into a pure helper
`nextSourceBranches(current, polled)` returning (fullList, newlyAdded), and
added a unit test that drives it across initial/add/remove polls and asserts
the full list is always retained. The test fails against the old delta-only
code (add stores {c}, remove stores {}).

Orthogonal to the label-retrigger customization: newBranches already carries
the label-folded commit discriminators, so a label change still appears as a
new entry and still retriggers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ZacxDev
ZacxDev merged commit 59335b0 into feat/label-retrigger Jul 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant