Skip to content

fix(github): aggregate leader answers unscoped usage errors despite respond_to_unscoped - #903

Draft
aparajon wants to merge 2 commits into
mainfrom
armand/leader-usage-error-replies
Draft

fix(github): aggregate leader answers unscoped usage errors despite respond_to_unscoped#903
aparajon wants to merge 2 commits into
mainfrom
armand/leader-usage-error-replies

Conversation

@aparajon

@aparajon aparajon commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Why this matters

Some repos are served by a fleet of SchemaBot deployments arranged in an aggregate: one leader plus several participants. When a comment command has a usage error that isn't scoped to any one deployment — a bare schemabot apply missing its -e environment flag, an -e value naming an unknown environment, a help request, or an unrecognized command — the participants deliberately stay silent and defer the reply to the leader, so the error is posted exactly once instead of once per deployment.

To reinforce that, fleets also set respond_to_unscoped: false on every deployment. The problem: the leader's own reply path consulted that same policy and silenced itself. So nobody replied. The operator's command died with no PR-visible feedback at all, and the skip was logged only at debug level — invisible in production logs — so there was nothing to explain why.

What it does

  • The leader now answers unscoped usage errors (missing/malformed/unknown -e), help, and invalid commands regardless of respond_to_unscoped — it is the designated responder by role, and the deployments that deferred to it must not find it silenced. Repos without an aggregate role keep the existing respond_to_unscoped single-responder behavior.
  • The policy skips now log at info instead of debug, naming the policy and the deployment expected to post the reply, so a missing reply is triageable from logs.
operator comments: "schemabot apply"        (missing -e)

BEFORE                                      AFTER
──────────────────────────────────────      ──────────────────────────────────────
participant A ── defers to leader           participant A ── defers to leader
participant B ── defers to leader           participant B ── defers to leader
leader ── respond_to_unscoped=false         leader ── designated responder by role
          → silent                                    → replies once with usage help

PR reply: (nothing)                         PR reply: ❌ missing environment: use -e

🤖 Generated with Claude Code

…espond_to_unscoped

Participants on an aggregate repo defer unscoped usage-error replies to
the leader, and fleets set respond_to_unscoped to false on every
deployment to prevent duplicate replies — but the leader's own reply
path also consulted that policy and silenced itself, so a bare apply or
an -e value naming an unknown environment got no PR reply from any
deployment, and the skip logged only at debug level.

The leader is the designated responder by role, so it now answers
unscoped usage errors and help regardless of respond_to_unscoped; that
policy alone still picks the single responder on repos without an
aggregate role. The policy skips now log at info with the deployment
that is expected to post the reply, so a missing reply is triageable
from logs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 1, 2026 17:12

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

Fixes a webhook reply gap in aggregate-repo setups where all deployments set respond_to_unscoped: false: aggregate participants defer unscoped usage/help/invalid-command replies to the leader, but the leader previously also consulted respond_to_unscoped and could silence itself—resulting in no PR-visible feedback for certain commands.

Changes:

  • Add an aggregate-aware responder predicate (answersUnscopedUsageErrors) so aggregate leaders reply even when respond_to_unscoped is false.
  • Route help, invalid env, missing env, unknown env, and invalid-command reply gating through that predicate, and promote “skip” logs from debug to info.
  • Add a regression test covering aggregate-leader replies under respond_to_unscoped: false.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
pkg/webhook/schema_source_policy.go Adds aggregate-aware logic to decide when this deployment should post unscoped usage/help replies.
pkg/webhook/issue_comment.go Uses the new aggregate-aware responder logic and upgrades skip logs to info for triageability.
pkg/webhook/check_runs_test.go Adds coverage to ensure the aggregate leader posts replies despite respond_to_unscoped: false.

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

Comment thread pkg/webhook/schema_source_policy.go Outdated
Comment thread pkg/webhook/check_runs_test.go Outdated
…tions

answersUnscopedGuidanceReplies names what the gate actually covers — help
requests and invalid commands as well as usage errors — and the leader test
now asserts a PR comment was actually posted for every case instead of only
checking the handler's JSON response.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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