Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/github-app-setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,18 @@ Under **Organization permissions**, grant:

| Event | Purpose |
|-------|---------|
| **Check run** | Receive `check_run.rerequested` so a human clicking **Re-run** on a SchemaBot check re-plans the PR head |
| **Check suite** | Receive `check_suite.requested` as a redundant auto-plan convergence signal: when the organic `pull_request` delivery for a push is lost upstream, SchemaBot recovers the missing auto-plan from the check suite instead of waiting for the next reconciler scan |
| **Issue comment** | Receive `schemabot plan`, `schemabot help`, etc. from PR comments |
| **Merge group** | Publish a passing SchemaBot check on a merge-queue commit so a required SchemaBot check does not block the merge queue (only needed if a repo uses a merge queue) |
| **Pull request** | Auto-plan each affected database on PR open/synchronize/reopen and clean up locks and stored check state on close |
| **Push** | Publish a passing SchemaBot check on default-branch commits so branch rulesets can select the App as a pinned required-check source (rulesets only index Apps whose check suites ran against the target branch) |

Subscribe to **Merge group**, not **Merge queue entry** — the two are distinct events, and SchemaBot handles only `merge_group`. The **Merge group** event requires the **Merge queues: Read** repository permission above. Because that is a new permission, adding it to an existing App marks the App as requesting new permissions, which an org or repository admin must approve on each installation before it takes effect.

The **Push** event needs no new permission — **Contents: Read** above already unlocks it — so subscribing an existing App takes effect immediately on every installation, with no admin re-approval. Expect webhook delivery volume to rise once subscribed: GitHub sends a `push` event for every branch and tag push on installed repositories, and SchemaBot discards everything but default-branch pushes.

Future phases will also use **Check run** (action buttons) and **Pull request** (auto-plan on open/sync).
The **Check suite** event likewise needs no new permission — **Checks: Read & Write** above already unlocks it — so subscribing takes effect immediately. It also raises delivery volume (GitHub sends `check_suite.requested` for every push to every branch), and SchemaBot acknowledges and drops every suite that has no open PR at its head. Without this subscription the recovery feature receives no deliveries and is silently inert: lost auto-plans then wait for the reconciler's next scan instead of converging within the recovery grace.

### Where Can This GitHub App Be Installed?

Expand Down
3 changes: 2 additions & 1 deletion pkg/metrics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ available, such as `repository`, `github_app`, and `installation_id`.
| `schemabot.webhook.inbox_oldest_claimable_age_seconds` | Gauge | environment | Age of the oldest ready-to-claim durable webhook inbox row |
| `schemabot.webhook.inbox_stuck_processing` | Gauge | environment | Durable webhook inbox rows stuck in processing past the attempt cap |
| `schemabot.webhook.inbox_stats_collection_failures` | Counter | environment | Failed durable webhook inbox metric snapshots (liveness signal for the inbox gauges) |
| `schemabot.webhook.check_suite_recovery_total` | Counter | environment, repository, outcome | Durable check_suite recovery outcomes (`covered`, `synthesized`, `resynthesized`, `already_queued` per candidate PR; `no_open_pr` per delivery) |
| `schemabot.webhook.inbox_dispatch_lag_seconds` | Histogram | environment, event_type, repository | Time from webhook receipt to the delivery's first dispatch claim |
| `schemabot.webhook.dispatch_duration_seconds` | Histogram | environment, event_type, outcome | Duration of one durable webhook dispatch claim by outcome (deliberately no repository label — see the ledger note below) |
| `schemabot.github.requests_total` | Counter | environment, operation, category, resource, status, repository, github_app, installation_id | GitHub API responses observed by SchemaBot |
Expand Down Expand Up @@ -102,7 +103,7 @@ available, such as `repository`, `github_app`, and `installation_id`.

**status** (locks): `success`, `conflict`, `not_found`, `not_owned`, `error`

**event_type** (webhooks): `create`, `issues`, `issue_comment`, `pull_request`, `pull_request_review`, `pull_request_review_comment`, `check_run`, `ping`, `push`
**event_type** (webhooks): `create`, `issues`, `issue_comment`, `pull_request`, `pull_request_review`, `pull_request_review_comment`, `check_run`, `check_suite`, `ping`, `push`

**action** (webhooks): common GitHub actions for the subscribed webhook events, such as `created`, `opened`, `synchronize`, `submitted`, `edited`, `closed`, `requested`, `completed` (omitted for events without actions like `ping` and `push`)

Expand Down
27 changes: 27 additions & 0 deletions pkg/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -1713,6 +1713,33 @@ func RecordWebhookReconcileSynthesizedEvent(ctx context.Context, repo string, re
attribute.String("outcome", outcome))
}

// RecordWebhookCheckSuiteRecovery counts outcomes of durable
// check_suite.requested recovery processing. "covered", "synthesized",
// "resynthesized", and "already_queued" increment once per candidate PR (a
// delivery can carry several), while "no_open_pr" increments once per
// delivery when resolution finds no candidate at all; a delivery retried
// under its attempt budget re-counts PRs it already observed on an earlier
// attempt, so outcomes are per observation, not per unique PR. "covered" is
// the healthy steady state — the organic pull_request delivery arrived
// during the recovery grace and planned the head, so the redundant signal
// no-oped. "synthesized" means the auto-plan delivery for an open PR head
// was genuinely lost and the check_suite signal recovered it — investigate
// the upstream loss (edge auth, GitHub send failures), not the recovery.
// "resynthesized" means the recovery reopened a terminally failed
// synthesized row, so a sustained rate is the same head failing repeatedly
// after recovery — investigate that head's processing failure.
// "already_queued" means another recovery producer (the reconciler or an
// earlier check_suite delivery) got there first. "no_open_pr" means no PR
// named by the delivery was still open at the suite head — its PRs closed or
// moved on during the grace, or a fork head matched no open PR.
func RecordWebhookCheckSuiteRecovery(ctx context.Context, repo string, outcome string) {
addCounter(ctx, "schemabot.webhook.check_suite_recovery_total",
"Total number of outcomes from durable check_suite recovery processing", "{event}",
EnvironmentAttribute(""),
attribute.String("repository", repo),
attribute.String("outcome", outcome))
}

// RecordWebhookReconcileStuckTerminated counts webhook inbox rows the
// reconciler terminated because they were parked in processing with an expired
// lease at the attempt cap — a driver hard-killed on its final attempt. A
Expand Down
28 changes: 28 additions & 0 deletions pkg/serve/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ func buildSingleAppWebhookRuntime(serverConfig *api.ServerConfig, svc *api.Servi
webhook.WithDurableWebhookDispatch(),
webhook.WithWebhookReconciler(),
}, webhookReconcileSynthesisOptions(logger)...)
handlerOpts = append(handlerOpts, checkSuiteRecoveryOptions(logger)...)
handler := webhook.NewHandler(svc, ghClient, []byte(ghWebhookSecret), logger, handlerOpts...)
svc.SetCheckRunBackfiller(handler)
logger.Info("GitHub webhook endpoint registered",
Expand Down Expand Up @@ -843,6 +844,7 @@ func buildMultiAppWebhookRuntime(serverConfig *api.ServerConfig, svc *api.Servic
webhook.WithDurableWebhookDispatch(),
webhook.WithWebhookReconciler(),
}, webhookReconcileSynthesisOptions(logger)...)
handlerOpts = append(handlerOpts, checkSuiteRecoveryOptions(logger)...)
handler := webhook.NewHandlerWithDispatch(
svc,
ghclient.NewClientSet(clients),
Expand Down Expand Up @@ -895,6 +897,32 @@ func webhookReconcileSynthesisOptions(logger *slog.Logger) []webhook.HandlerOpti
return []webhook.HandlerOption{webhook.WithWebhookReconcileSynthesis()}
}

// checkSuiteRecoveryOptions returns the handler option enabling durable
// check_suite.requested recovery, unless the operator disabled it with
// WEBHOOK_CHECK_SUITE_RECOVERY=false. The kill switch makes the webhook
// endpoint acknowledge and ignore check_suite deliveries with a restart
// instead of a code revert; the reconciler's missing-head scan remains the
// recovery backstop either way. An unparseable value also disables recovery:
// the only reason to set the variable is to turn recovery off, so a malformed
// value ("off", "disabled") is treated as intent to disable — an operator
// reaching for a kill switch mid-incident must get the switched-off behavior,
// not a warning in pod logs they are not watching.
func checkSuiteRecoveryOptions(logger *slog.Logger) []webhook.HandlerOption {
if value := os.Getenv("WEBHOOK_CHECK_SUITE_RECOVERY"); value != "" {
enabled, err := strconv.ParseBool(value)
if err != nil {
logger.Error("invalid WEBHOOK_CHECK_SUITE_RECOVERY value; check-suite recovery disabled (fail-safe) — lost auto-plan deliveries recover only via the reconciler",
"value", value, "error", err)
return nil
}
if !enabled {
logger.Info("check-suite recovery disabled by WEBHOOK_CHECK_SUITE_RECOVERY; lost auto-plan deliveries recover only via the reconciler")
return nil
}
}
return []webhook.HandlerOption{webhook.WithCheckSuiteRecovery()}
}

// buildServerAuthorizer constructs the API authorizer exactly as the server
// wires it: admin teams from PR command authorization, and the operator-group
// union that widens forward-auth write admission. Every server build and any
Expand Down
43 changes: 43 additions & 0 deletions pkg/serve/serve_check_suite_recovery_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package serve

import (
"io"
"log/slog"
"testing"

"github.com/stretchr/testify/require"
)

// TestCheckSuiteRecoveryOptions pins the kill-switch contract: durable
// check-suite recovery defaults on, WEBHOOK_CHECK_SUITE_RECOVERY set to false
// drops the webhook endpoint to acknowledge-and-ignore for check_suite
// deliveries, and an unparseable value fails safe to disabled — setting the
// variable at all signals intent to turn recovery off, so a malformed value
// must not leave it running.
func TestCheckSuiteRecoveryOptions(t *testing.T) {
logger := slog.New(slog.NewTextHandler(io.Discard, nil))

for _, tc := range []struct {
name string
value string
enabled bool
}{
{"default enabled when unset", "", true},
{"explicitly enabled", "true", true},
{"disabled", "false", false},
{"invalid value fails safe to disabled", "not-a-bool", false},
{"non-ParseBool disable spelling fails safe to disabled", "off", false},
} {
t.Run(tc.name, func(t *testing.T) {
t.Setenv("WEBHOOK_CHECK_SUITE_RECOVERY", tc.value)

opts := checkSuiteRecoveryOptions(logger)

if tc.enabled {
require.Len(t, opts, 1)
} else {
require.Empty(t, opts)
}
})
}
}
5 changes: 5 additions & 0 deletions pkg/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@ type WebhookEventStore interface {
// time passes. Producers of deferred work — a redundant convergence signal
// that should lose the race to the primary delivery — set it to schedule
// dispatch; nil means immediately claimable. A redelivery reopen clears it.
//
// Only a fresh insert populates event.ID; a reopen returns inserted=true
// with event.ID left zero, so callers whose behavior differs between the
// two — a deferred producer whose not-before time a reopen discards, for
// example — can tell them apart.
Create(ctx context.Context, event *WebhookEvent) (inserted bool, err error)

// GetByDeliveryID returns a webhook event by provider + delivery GUID, or nil if not found.
Expand Down
11 changes: 9 additions & 2 deletions pkg/webhook/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,16 @@ GitHub sends POST /webhook
| |
| +-- Release locks held by the PR and delete stored check state
|
+-- check_run
+-- check_run.rerequested
| |
| +-- Re-plan the PR head when a human clicks "Re-run" on a
| SchemaBot check
|
+-- check_suite.requested
|
+-- Accepted by the webhook but not implemented today
+-- Enqueue a grace-deferred recovery pass that synthesizes the
auto-plan delivery for open PR heads whose pull_request
delivery was lost
```

## GitHub App Authentication
Expand Down
Loading
Loading