Skip to content

fix(github): answer a control command the schema change already satisfies - #970

Draft
aparajon wants to merge 1 commit into
mainfrom
armand/repeat-cancel-already-settled
Draft

fix(github): answer a control command the schema change already satisfies#970
aparajon wants to merge 1 commit into
mainfrom
armand/repeat-cancel-already-settled

Conversation

@aparajon

@aparajon aparajon commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Why this matters

Reissuing a control command is how an operator confirms one landed. Today the second cancel on a cancelled schema change comes back as a failure comment carrying a support-channel link — routing someone who already got exactly what they asked for toward an incident that does not exist. The same rejection also ticks the metric operators alert on, so ordinary confirming traffic inflates the rejection rate.

What it does

  • Adds a satisfied classification to the shared control-error type, for a rejection whose cause is the operator's own intent already being the apply's state. It carries the same conflict status — the request still cannot proceed — but callers can tell it apart from a request the operator must reissue differently.
  • Every command routed through the shared control lifecycle is covered, not just cancel: a stop on a stopped apply answers the same way.
  • The reply carries no support-channel link and no failure framing, and logs at INFO rather than the WARN a rejection earns.
  • The control-operation metric gains its own satisfied value, so a repeat command no longer counts as a rejection.
  • Collapses seven copies of the duplicated metric-status block into one named helper.

🤖 Generated with Claude Code

…fies

Reissuing a control command is how an operator confirms the first one landed.
A cancel on an already-cancelled schema change, or a stop on an already-stopped
one, answered with a failure comment carrying a support-channel link and logged
a warning — sending someone who got exactly what they asked for to look for an
incident that does not exist.

Control errors now distinguish a request the apply's state already satisfies
from one the operator must reissue differently. A satisfied request gets an
informational reply with no support escalation, an INFO log, and its own
"satisfied" value on the control-operation metric so repeat commands do not
inflate the rejection rate operators alert on. Every control command routes
through the same path, so any operation that can be issued twice is covered.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 7, 2026 20:43

Copilot AI left a comment

Copy link
Copy Markdown

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 operator UX and telemetry for repeated control commands by distinguishing “already in the requested state” from genuine rejections/failures. It adds a “satisfied” classification for control-operation conflicts so the webhook can respond informationally (without support escalation language) and the control-operation metric can exclude this expected confirmation traffic from the rejection rate.

Changes:

  • Add a “satisfied” variant to the shared control-operation HTTP error type, plus helpers to detect it and to classify control errors for metrics.
  • Update the webhook control-command lifecycle to post an informational “Nothing to …” comment (and log at INFO) when a command is already satisfied.
  • Add integration and unit tests covering repeat stop/cancel on already-stopped/cancelled schema changes.

Reviewed changes

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

Show a summary per file
File Description
pkg/webhook/templates/issue_comment.go Adds a dedicated template for “control command already satisfied” replies without support-channel framing.
pkg/webhook/control.go Detects satisfied control-operation errors and posts the informational reply + INFO log instead of routing through rejection handling.
pkg/webhook/control_integration_test.go Adds an integration test ensuring repeat stop/cancel on settled applies replies informationally and does not create new control requests.
pkg/api/control_handlers.go Introduces satisfied control-error classification, a shared metric-status helper, and returns satisfied conflicts for already-cancelled/stopped states.
pkg/api/control_handlers_test.go Adds unit coverage for satisfied classification, wrapping behavior, HTTP status, and metric-status mapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants