Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 9 additions & 2 deletions docs/invariants.md
Original file line number Diff line number Diff line change
Expand Up @@ -1363,8 +1363,15 @@ their own review requirement. *Enforced:* the review gate and actor authorizatio
An unscoped PR command resolves to exactly one unambiguous database or is rejected with guidance,
never resolved by an arbitrary pick. A malformed command is rejected rather than "helpfully"
corrected into something executable, especially one carrying `--allow-unsafe`. Every command
receives a response, and silence only ever means another instance owns the reply. *Enforced:*
command discovery and the unowned-command policy (`pkg/webhook/commands.go`).
receives a response, and silence only ever means another instance owns the reply.

Deferring to the owner is right whenever an owner exists. When a command names an apply no
deployment stores, every deployment defers to an owner that will never speak, so the aggregate
leader closes the gap: after a grace period it re-reads the command comment's acknowledgment
reaction — the one signal about the command that every deployment can read (UX-2) — and, if
nothing claimed it, answers once, marking the comment as it does so a redelivery stays quiet.
*Enforced:* command discovery and the unowned-command policy (`pkg/webhook/commands.go`), and the
leader's unclaimed-command follow-up (`pkg/webhook/unclaimed_command.go`).

## Structural enforcement

Expand Down
15 changes: 15 additions & 0 deletions pkg/github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,21 @@ func (ic *InstallationClient) AddReactionToComment(ctx context.Context, repo str
return nil
}

// CommentHasReaction reports whether any account has left the named reaction
// on a comment. On a repository several SchemaBot deployments serve, the
// acknowledgment reaction is the one signal every deployment can read about
// what the others decided, so this is how a deployment learns whether a
// command was claimed at all.
func (ic *InstallationClient) CommentHasReaction(ctx context.Context, repo string, commentID int64, reaction string) (bool, error) {
owner, repoName := splitRepo(repo)
opts := &gh.ListReactionOptions{Content: reaction, ListOptions: gh.ListOptions{PerPage: 1}}
reactions, _, err := ic.client.Reactions.ListIssueCommentReactions(ctx, owner, repoName, commentID, opts)
if err != nil {
return false, fmt.Errorf("list %q reactions on comment %d in %s: %w", reaction, commentID, repo, err)
}
return len(reactions) > 0, nil
}

// PullRequestInfo holds relevant PR metadata. The base branch is carried by
// ref only: GitHub's pull.base.sha is a snapshot from PR creation, not the
// branch tip, so callers that need the current base commit must resolve
Expand Down
5 changes: 5 additions & 0 deletions pkg/github/rate_limit_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,11 @@ var githubRoutePatterns = []githubRoutePattern{
path: "/repos/{owner}/{repo}/issues/comments/{comment_id}/reactions",
operation: metrics.GitHubOperationAddCommentReaction,
},
{
method: http.MethodGet,
path: "/repos/{owner}/{repo}/issues/comments/{comment_id}/reactions",
operation: metrics.GitHubOperationListCommentReactions,
},
{
method: http.MethodPatch,
path: "/repos/{owner}/{repo}/issues/comments/{comment_id}",
Expand Down
2 changes: 2 additions & 0 deletions pkg/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -1460,6 +1460,7 @@ const (
GitHubOperationGraphQLMinimizeComment = "graphql_minimize_comment"
GitHubOperationGraphQLStatusCheckRollup = "graphql_status_check_rollup"
GitHubOperationListCheckRunsForRef = "list_check_runs_for_ref"
GitHubOperationListCommentReactions = "list_comment_reactions"
GitHubOperationListPRFiles = "list_pr_files"
GitHubOperationListReviews = "list_reviews"
GitHubOperationListTeamMembers = "list_team_members"
Expand Down Expand Up @@ -1659,6 +1660,7 @@ func isKnownGitHubOperation(operation string) bool {
GitHubOperationGraphQLMinimizeComment,
GitHubOperationGraphQLStatusCheckRollup,
GitHubOperationListCheckRunsForRef,
GitHubOperationListCommentReactions,
GitHubOperationListPRFiles,
GitHubOperationListReviews,
GitHubOperationListTeamMembers,
Expand Down
7 changes: 5 additions & 2 deletions pkg/webhook/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,18 @@ func (h *Handler) loadApplyForPRControl(ctx context.Context, repo string, pr int
// On an aggregate repo an unscoped control command fans out to every
// deployment, but the apply lives in exactly one tenant's storage. A
// deployment that doesn't have it is not the owner and stays silent so
// only the owning deployment answers.
// only the owning deployment answers. When no deployment owns it, that
// leaves the command unanswered, so the leader follows up once it can
// see that nothing claimed the comment.
if h.silentOnUnscopedFanOut(repo, result.Tenant) {
h.logger.Info("unscoped fan-out control command targets an apply not stored on this deployment; staying silent so the owning deployment responds",
h.logger.Info("unscoped fan-out control command targets an apply not stored on this deployment; deferring to the deployment that owns it",
"command", command,
"repo", repo,
"pr", pr,
"apply_id", result.ApplyID,
"environment", result.Environment,
"requested_by", requestedBy)
h.answerUnclaimedControlCommand(repo, pr, installationID, requestedBy, command, result)
return nil, false
}
h.logger.Warn("PR control command rejected because apply was not found",
Expand Down
5 changes: 5 additions & 0 deletions pkg/webhook/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ type Handler struct {
// package default.
participantNudgeRefoldDelay time.Duration

// unclaimedCommandGraceOverride overrides how long the aggregate leader
// waits before deciding an apply-scoped control command went unclaimed.
// Zero means the package default.
unclaimedCommandGraceOverride time.Duration

// participantRefoldDelayOverride overrides the backoff before each
// self-scheduled aggregate re-fold armed while expected participants
// remain unresolved. Test-only: when set it applies to every attempt.
Expand Down
11 changes: 9 additions & 2 deletions pkg/webhook/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,14 @@ func (h *Handler) knownEnvironments() []string {
return h.service.Config().KnownEnvironments()
}

// acknowledgeCommand adds the eyes reaction to the command comment,
// commandAcknowledgmentReaction is the reaction a deployment leaves on a
// command comment to signal that the command is its work and it has committed
// to acting (UX-2). On a repository several deployments serve it is also the
// only signal about a command that every one of them can read, which is how a
// command no deployment claimed is distinguished from one a sibling owns.
const commandAcknowledgmentReaction = "eyes"

// acknowledgeCommand adds the acknowledgment reaction to the command comment,
// signalling "this deployment is acting on your command".
func (h *Handler) acknowledgeCommand(repo string, pr int, installationID int64, deliveryID string, commentID int64) {
if commentID <= 0 || h.ghClients.Len() == 0 {
Expand All @@ -832,7 +839,7 @@ func (h *Handler) acknowledgeCommand(repo string, pr int, installationID int64,
"repo", repo, "pr", pr, "error", err)
return
}
if err := client.AddReactionToComment(ctx, repo, commentID, "eyes"); err != nil {
if err := client.AddReactionToComment(ctx, repo, commentID, commandAcknowledgmentReaction); err != nil {
h.logger.Error("failed to add command acknowledgment reaction",
"repo", repo, "pr", pr, "error", err)
}
Expand Down
29 changes: 29 additions & 0 deletions pkg/webhook/templates/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/block/schemabot/pkg/apitypes"
"github.com/block/schemabot/pkg/caller"
"github.com/block/schemabot/pkg/glyph"
)

// RenderRollbackMissingArguments renders the message posted when `schemabot rollback`
Expand Down Expand Up @@ -101,6 +102,34 @@ func RenderControlMissingApplyID(command string) string {
"Use `schemabot status -e <environment>` to find the apply ID.", usage))
}

// UnclaimedControlCommandData describes a control command that named an apply
// no SchemaBot on the repository is driving.
type UnclaimedControlCommandData struct {
Command string
ApplyID string
Environment string
RequestedBy string
}

// RenderUnclaimedControlCommand renders the reply to an apply-scoped control
// command that no deployment claimed. Several SchemaBot deployments can serve
// one repository, and each stays quiet about an apply another one owns, so the
// operator's own view of an unrecognized apply ID is a command that produced
// nothing at all. This says so, and says where the identifiers that do resolve
// come from.
func RenderUnclaimedControlCommand(data UnclaimedControlCommandData) string {
body := "## " + glyph.Attention + " No Schema Change Matched This Command\n\n" +
fmt.Sprintf("**Apply**: %s | **Environment**: %s\n", markdownInlineCode(data.ApplyID), markdownInlineCode(data.Environment))
if data.RequestedBy != "" {
body += fmt.Sprintf("**Requested by**: @%s\n", data.RequestedBy)
}
body += fmt.Sprintf("\nNo SchemaBot on this repository is driving a schema change with this identifier, so `%s` acted on nothing.\n\n", data.Command) +
"Apply identifiers appear on the schema change comments SchemaBot posts to this pull request, and in " +
"`schemabot status -e " + data.Environment + "`. Identifiers reported by a database engine are its own and " +
"are not accepted here."
return offerSupportChannel(body)
}

// RenderStopCommandAccepted renders the acknowledgement posted when a PR
// comment stop command records durable stop intent.
func RenderStopCommandAccepted(data StopCommandAcceptedData) string {
Expand Down
20 changes: 20 additions & 0 deletions pkg/webhook/templates/issue_comment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,23 @@ func TestRenderRevertCommandAccepted(t *testing.T) {
assert.Contains(t, rendered, "@alice")
assert.Contains(t, rendered, "SchemaBot will undo this schema change")
}

// The reply to a command no deployment claimed has to do more than say the
// apply is unknown: the operator got here by pasting an identifier that looked
// authoritative, so the message names where identifiers that do resolve come
// from, and that an engine's own identifiers are not among them (UX-4).
func TestRenderUnclaimedControlCommand(t *testing.T) {
rendered := RenderUnclaimedControlCommand(UnclaimedControlCommandData{
Command: "start",
ApplyID: "apply-49ea5a453e9a4f18",
Environment: "production",
RequestedBy: "alice",
})
assert.Contains(t, rendered, "No Schema Change Matched This Command")
assert.Contains(t, rendered, "`apply-49ea5a453e9a4f18`")
assert.Contains(t, rendered, "`production`")
assert.Contains(t, rendered, "@alice")
assert.Contains(t, rendered, "`start` acted on nothing")
assert.Contains(t, rendered, "schemabot status -e production")
assert.Contains(t, rendered, "Identifiers reported by a database engine are its own")
}
122 changes: 122 additions & 0 deletions pkg/webhook/unclaimed_command.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package webhook

import (
"context"
"time"

"github.com/block/schemabot/pkg/webhook/templates"
)

// defaultUnclaimedCommandGrace is how long the aggregate leader waits before deciding
// that an apply-scoped control command went unclaimed. A deployment that owns
// the named apply acknowledges the comment at its act point, which is one
// storage read after the command arrives, so the grace only has to cover
// ordinary webhook and storage latency across sibling deployments.
const defaultUnclaimedCommandGrace = 30 * time.Second

func (h *Handler) unclaimedCommandGrace() time.Duration {
if h.unclaimedCommandGraceOverride > 0 {
return h.unclaimedCommandGraceOverride
}
return defaultUnclaimedCommandGrace
}

// answerUnclaimedControlCommand replies to an apply-scoped control command that
// named an apply this deployment does not have, once it can tell that no
// sibling deployment had it either.
//
// On a repository several SchemaBot deployments serve, an unscoped command
// reaches all of them and each one that does not own the named apply stays
// quiet so only the owner answers (AZ-5). That is right whenever an owner
// exists. When none does — a mistyped identifier, one from another repository,
// or one an engine reported rather than SchemaBot — every deployment defers to
// an owner that is never going to speak, and the operator is left with a
// command that produced nothing at all.
//
// The acknowledgment reaction is what resolves it. It is the one signal about
// this command that every deployment can read, and per UX-2 a deployment adds
// it only once it has decided the command is its work. So the leader waits for
// the grace period, re-reads the comment, and answers only if nothing claimed
// it. It marks the comment as it answers, both because answering is itself
// acting on the command and so that a redelivered webhook sees the claim and
// stays quiet rather than posting a second copy.
//
// Only the leader does this: participants hold a partial view of the fleet and
// several of them replying would be the duplicate noise fan-out exists to
// avoid. The reply is operator visibility, never a gate — every failure along
// the way leaves the command exactly as silent as it is today, and says so in
// the logs.
func (h *Handler) answerUnclaimedControlCommand(repo string, pr int, installationID int64, requestedBy, command string, result CommandResult) {
config, ok := h.serverConfig()
if !ok {
h.logger.Warn("cannot tell whether a control command went unclaimed because server config is unavailable",
"command", command, "repo", repo, "pr", pr,
"apply_id", result.ApplyID, "environment", result.Environment)
return
}
if !config.IsAggregateLeaderForRepo(repo) {
h.logger.Debug("leaving an unclaimed control command for the aggregate leader to answer",
"command", command, "repo", repo, "pr", pr,
"apply_id", result.ApplyID, "environment", result.Environment)
return
}
if result.CommentID <= 0 {
h.logger.Warn("cannot tell whether a control command went unclaimed because it carries no comment to read",
"command", command, "repo", repo, "pr", pr,
"apply_id", result.ApplyID, "environment", result.Environment)
return
}

h.goSafe(repo, pr, installationID, result.DeliveryID, func() {
grace := h.unclaimedCommandGrace()
ctx, cancel := context.WithTimeout(context.Background(), grace+commandTimeout)
defer cancel()

select {
case <-time.After(grace):
case <-ctx.Done():
return
}
Comment thread
aparajon marked this conversation as resolved.
Outdated

client, err := h.clientForRepo(repo, installationID)
if err != nil {
h.logger.Error("failed to create GitHub client to check whether a control command went unclaimed",
"command", command, "repo", repo, "pr", pr,
"apply_id", result.ApplyID, "environment", result.Environment, "error", err)
return
}
claimed, err := client.CommentHasReaction(ctx, repo, result.CommentID, commandAcknowledgmentReaction)
if err != nil {
h.logger.Error("failed to read command acknowledgment; leaving the command unanswered rather than answering one a sibling deployment may own",
"command", command, "repo", repo, "pr", pr,
"apply_id", result.ApplyID, "environment", result.Environment, "error", err)
return
}
if claimed {
h.logger.Info("control command naming an apply this deployment does not have was claimed elsewhere",
"command", command, "repo", repo, "pr", pr,
"apply_id", result.ApplyID, "environment", result.Environment)
return
}

// Mark before posting. A crash between the two leaves the command as
// silent as it would have been without this path, where posting first
// would let a redelivery add a second copy.
if err := client.AddReactionToComment(ctx, repo, result.CommentID, commandAcknowledgmentReaction); err != nil {
h.logger.Error("failed to mark an unclaimed control command as answered; leaving it unanswered rather than risking a duplicate reply",
"command", command, "repo", repo, "pr", pr,
"apply_id", result.ApplyID, "environment", result.Environment, "error", err)
return
}

h.logger.Warn("no deployment claimed a control command; answering it as the aggregate leader",
"command", command, "repo", repo, "pr", pr,
"apply_id", result.ApplyID, "environment", result.Environment, "requested_by", requestedBy)
h.postComment(repo, pr, installationID, templates.RenderUnclaimedControlCommand(templates.UnclaimedControlCommandData{
Command: command,
ApplyID: result.ApplyID,
Environment: result.Environment,
RequestedBy: requestedBy,
}))
})
}
Loading
Loading