fix: scope branch-merge and proposed-change-review events to the default branch (closes #9761) - #10422
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ranch Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
No issues found across 6 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes webhook event scoping: branch-merge and proposed-change review events now carry the default branch as their scoping branch, with tests pinning the behavior and a changelog entry. No contract, schema, or operational tradeoffs exposed in the diff.
Re-trigger cubic
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
I'm not super confident about shipping this behavior change directly in the next minor release, since it changes the branch that some events are tied to. Maybe for this bug we should target develop branch and add a section in the 1.12 release note about this breaking change.
I'll target develop branch for now.
f43bffe to
9f0543a
Compare
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Not all branch-agnostic events are stamped to the default branch: created/deleted stay on the global branch and rebased/migrated inherit the caller's context. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| from infrahub.database import InfrahubDatabase | ||
|
|
||
|
|
||
| def _build_dispatcher( |
There was a problem hiding this comment.
extracted as this is now reused in other test classes
| - Proposed change merge and review events (merged, approved, rejected, and the approval/rejection revoke variants) are stamped to the default branch, so scoping is independent of the branch the mutation ran on. | ||
| - `branch.merged` is stamped to the default branch as well, since the merge lands there. Its payload still carries the merged branch in `branch_name` / `branch_id`; only the scoping branch is the default one. | ||
| - `branch.created` and `branch.deleted` are stamped to the global branch, pending a general rule for branch-agnostic node events. | ||
| - `branch.rebased` and `branch.migrated` inherit the caller's context branch; they are not overridden. |
There was a problem hiding this comment.
I wonder whether it would be relevant to assign the target branch for those operations
| branch=source_branch, | ||
| proposed_change_id=None, | ||
| node_events=[], | ||
| context=_context(default_branch), |
There was a problem hiding this comment.
Should this be context=_context(source_branch) to validate that the fix works properly?
Run dispatch_events with the source-branch context so the test proves the event is deliberately scoped to the default branch, not merely inheriting it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Webhook branch scoping change alters which branch-scoped webhooks fire for merge/review events (backward-incompatible), and the delta since the prior decline only tweaks a test, not the substantive behavior.
Re-trigger cubic
Why
Webhook branch scoping matches an event against the branch carried on its metadata. Two proposed-change / branch lifecycle events carried the wrong branch there, so branch-scoped webhooks matched them inconsistently:
infrahub.branch.mergedwas emitted on the global branch, so a Default Branch scoped webhook never matched it while an Other Branches one did, exactly the inverse ofinfrahub.proposed_change.merged, which is already default-scoped.infrahub.proposed_change.approved/rejected/approval_revoked/rejection_revokedinherited the branch of the review mutation. They reached the default branch only because the review UI happens to run in the default-branch context — a non-UI client (SDK/REST/automation) reviewing from another branch would emit them on that branch.Goal: emit both consistently on the default branch so webhook branch scoping is deterministic and client-independent.
Non-goals: the general rule for branch-agnostic node events (#9760) and webhook form validation (#9759).
branch.created/branch.deletedcarry the same pattern but are intentionally left for #9760.Future thoughts
Changes related to the global branch are matched by the "All other branches" filter and not by the "Default branch" filter. Events tied to the global branch can be misleading from a user's perspective, since this behavior relies on the fact that some objects are branch-agnostic and others are not. A dedicated "Global branch" filter could be a way to solve this.
Closes #9761
What changed
Behavioral changes:
infrahub.branch.mergedis now scoped to the default branch.Implementation notes:
PostMergeDispatchernow stamps the merge event withself.default_branchinstead ofself.global_branch. The event payload (branch_name,branch_id) still identifies the branch that was merged.EventMeta.from_context(...), so only the event's scoping branch is overridden.What stayed the same: no schema changes, no GraphQL/REST contract changes, event payloads unchanged.
How to review
backend/infrahub/core/merge/post_merge.py— merge event scoping (one-line change) + droppedglobal_branchparam.backend/infrahub/graphql/mutations/proposed_change.py— review event scoping override.backend/infrahub/core/merge/builder.py— wiring updated for the dropped param.How to test
uv run pytest \ backend/tests/component/core/merge/test_post_merge.py::TestPostMergeBranchMergedEvent \ "backend/tests/functional/proposed_change/test_review.py::TestProposedChangeReview::test_review_event_scoped_to_default_branch" \ -vImpact & rollout
infrahub.branch.mergedwebhook matching changes — a Default-Branch webhook starts matching it and an Other-Branches webhook stops. Documented in the changelog / release notes. Approve/reject scoping is unchanged for UI clients.Checklist