Skip to content

coalesce stale auto-plan webhook claims via superseded terminal state - #1001

Open
Kiran01bm wants to merge 2 commits into
mainfrom
kiran01bm/wh-8b-superseded-coalescing
Open

coalesce stale auto-plan webhook claims via superseded terminal state#1001
Kiran01bm wants to merge 2 commits into
mainfrom
kiran01bm/wh-8b-superseded-coalescing

Conversation

@Kiran01bm

Copy link
Copy Markdown
Collaborator

Adds a superseded terminal state to the durable webhook inbox and coalesces stale auto-plan claims at claim time, so rapid pushes to the same PR no longer trigger one full plan run per delivery.

Why

Every retained webhook delivery becomes its own inbox row, and each auto-plannable pull_request row drives a full plan even when a newer push has already made its head stale. On busy PRs this wastes worker capacity and GitHub API budget replanning heads that will be immediately replaced, and it delays the plan for the head that actually matters.

What

  • New superseded terminal inbox state; redelivery reopens superseded rows, and the reconciler's HasEventForHead no longer counts them as coverage.
  • WebhookEventStore.SupersedeIfCovered: a lease-guarded, atomic conditional update that marks a claimed auto-plan pull_request row superseded only when a strictly newer covering successor exists for the same (provider, repository, pull_request).
  • Covering successors: newer auto-plan or closed PR events that are pending, live processing, expired-processing or retryable under the attempt budget, or completed. Terminally failed and superseded rows do not cover. closed covers older auto-plans but is never itself superseded.
  • Dispatcher checks coverage right after claiming; a superseded claim skips processing and records dispatch outcome superseded. Coalescing storage errors fail open and the row processes normally.
  • (provider, delivery_id) dedupe is unchanged.

Before:

push A ─▶ row A (pending)
push B ─▶ row B (pending)
push C ─▶ row C (pending)

claim A ─▶ full plan on stale head A
claim B ─▶ full plan on stale head B
claim C ─▶ full plan on current head C

After:

push A ─▶ row A (pending)
push B ─▶ row B (pending)
push C ─▶ row C (pending)

claim A ─▶ covered by B/C ─▶ superseded (no plan)
claim B ─▶ covered by C   ─▶ superseded (no plan)
claim C ─▶ no successor   ─▶ full plan on current head C

failure path: if C's successor check errors or C is terminally
failed, older rows are NOT superseded and process normally

Rapid pushes to the same PR enqueue one inbox row per delivery, and each
row runs a full plan even though only the newest head matters. At claim
time, an auto-plannable pull_request row is now atomically marked
superseded when a newer covering successor (auto-plan or closed) exists
for the same (provider, repository, PR), skipping redundant plan runs.
Terminally failed and superseded successors do not cover; closed rows
are never superseded; coalescing errors fail open to normal processing.
Copilot AI lite review requested due to automatic review settings August 11, 2026 05:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves SchemaBot’s durable webhook dispatcher efficiency by introducing a new terminal superseded inbox state and claim-time coalescing for stale auto-plan pull_request deliveries, preventing redundant plan runs during rapid push bursts on the same PR.

Changes:

  • Add superseded as a terminal durable webhook inbox state and exclude it from head-coverage checks.
  • Implement WebhookEventStore.SupersedeIfCovered and wire the dispatcher to supersede covered auto-plan claims immediately after claiming.
  • Extend metrics and tests to validate the new superseded dispatch outcome and storage coalescing behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/webhook/durable_dispatch.go Supersedes covered auto-plan pull_request deliveries at claim time to skip stale planning work.
pkg/webhook/durable_dispatch_test.go Updates test stores and adds driver tests for superseded / fail-open / lease-lost coalescing paths.
pkg/webhook/durable_dispatch_metrics_test.go Adds metrics assertion for the new superseded dispatch duration outcome.
pkg/storage/types.go Introduces WebhookEventSuperseded and PullRequestClosedAction, and adds superseded to the canonical state list.
pkg/storage/storage.go Extends the WebhookEventStore interface with SupersedeIfCovered and documents coalescing semantics.
pkg/storage/internal/sqlstore/webhook_events.go Implements SupersedeIfCovered via a single conditional UPDATE guarded by lease/state and successor coverage rules.
pkg/storage/internal/sqlstore/webhook_events_test.go Adds comprehensive storage-level tests covering successor states, scoping, timestamp semantics, and redelivery behavior.
pkg/metrics/metrics.go Allowlists the superseded dispatch outcome for the dispatch duration histogram.
pkg/api/webhook_inbox_metrics_test.go Updates inbox depth metrics expectations to include the new superseded state.
Suppressed comments (1)

pkg/webhook/durable_dispatch_test.go:63

  • recordingWebhookEventStore.Create claims to mirror the real SQL store’s duplicate-GUID reopen behavior, but it currently does not default Provider to GitHub and does not set/refresh ReceivedAt (the SQL store sets received_at to time.Now() when zero and refreshes it on reopen). This can cause test behavior to diverge from production for coalescing/newness logic based on received_at.
func (s *recordingWebhookEventStore) Create(_ context.Context, event *storage.WebhookEvent) (bool, error) {
	s.mu.Lock()
	defer s.mu.Unlock()

	key := event.Provider + ":" + event.DeliveryID
	if existing, ok := s.events[key]; ok {
		// Mirror the real store's duplicate-GUID branch: terminal rows are
		// reopened as fresh pending deliveries; live rows dedup.
		if existing.State == storage.WebhookEventFailed || existing.State == storage.WebhookEventCompleted ||
			existing.State == storage.WebhookEventSuperseded {
			existing.State = storage.WebhookEventPending
			existing.Attempts = 0
			existing.Payload = append([]byte(nil), event.Payload...)
			existing.LeaseOwner = ""
			existing.LeaseToken = ""
			existing.LeaseExpiresAt = nil
			existing.RetryAfter = nil
			existing.StartedAt = nil
			existing.CompletedAt = nil
			return true, nil

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Kiran01bm
Kiran01bm marked this pull request as ready for review August 11, 2026 10:17
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@Kiran01bm Kiran01bm changed the title Coalesce stale auto-plan webhook claims via superseded terminal state coalesce stale auto-plan webhook claims via superseded terminal state Aug 11, 2026
…claims

received_at is arrival order, not push order: a delayed delivery or an
operator Redeliver of an old-head row could supersede the delivery
carrying the PR's real current head, leaving it unplanned where
reconciler synthesis is disabled or unavailable. The driver now probes
for a covering successor, fetches the live PR uncached, and supersedes
only when GitHub confirms the claimed head is stale or the PR is
closed; every uncertain outcome fails open and processes the delivery.
Also documents the coverage-is-a-promise trade, catalogs the new
superseded metric values, and corrects reconciler triage docs.
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.

3 participants