fix(github): give internal-error PR comments a fixed sentence and an error reference

[aparajon]

Why

When a command fails because SchemaBot itself broke (storage read, GitHub API call, lock bookkeeping, transport failure, recovered panic), the raw Go error told the reader nothing they could act on, and it could carry hostnames, dial targets, DSN fragments, and driver internals onto a public PR.

PR comments are also a machine interface: most schema changes are driven by agents reading them. So internal failures now render one fixed, regex-stable sentence carrying a short error reference, and the raw error stays in the server log on the same line as error_ref.

What changes

A PR comment reports three kinds of failure. Only the third changes:

1. The schema change failed (engine plan/apply errors, rejected DDL, lock wait timeout): rendered in full, with the apply-logs fold. Unchanged.
2. SchemaBot refused the command (config and discovery outcomes, review gate, actor authorization, control guardrail rejections): typed messages authored for PR display, kept on a fail-closed allowlist and rendered verbatim. Unchanged.
3. SchemaBot itself broke: one of two fixed sentences plus a reference.

error at a comment-posting site
  |- engine error about the schema change ---> rendered in full (+ apply logs fold)
  |- typed, authored for PR display --------> rendered verbatim (fail-closed allowlist)
  |- known transport failure ---------------> fixed retry guidance
  '- anything else (storage/GitHub/untyped) -> "Internal SchemaBot error. Retry
                                               (error reference `ab12cd34`)."
                                               raw error only in the log, on the
                                               same line as error_ref

• Retryable: <summary> Internal SchemaBot error. Retry (error reference ab12cd34).
• Command already accepted, so retrying would be wrong: <summary> Internal SchemaBot error (error reference ab12cd34).

One regex extracts both facts an agent needs: whether a retry is worth attempting, and the reference to quote in a report.

Also here: the "no schema files found" discovery outcome becomes a typed error so it renders as its authored message instead of an internal-error sentence, and recovered webhook panics post a fixed message with a reference and count under their own containment boundary rather than the unknown operation label.

main independently sanitized most of the raw-error render sites while this branch was open, so what remains is the error-reference mechanism and the uniform sentence layered onto that work.

Rendering

Command failed inside SchemaBot (plan)

❌ Plan Failed

Environment: staging

Requested by @jackjackbits at 2026-01-15 14:30:00 UTC

Error

Failed to prepare the schema change request. Internal SchemaBot error. Retry (error reference ab12cd34).

Review gate could not be evaluated (fail-closed)

❌ Apply Failed

Environment: staging

Requested by @jackjackbits at 2026-01-15 14:30:00 UTC

Error

Review gate check failed. Internal SchemaBot error. Retry (error reference ab12cd34). If approval is granted through a GitHub team, verify the GitHub App can read organization members and team membership.

Prior-environment check could not be read (fail-closed)

❌ Apply Blocked

Could not verify staging status: failed to query check runs. Internal SchemaBot error. Retry (error reference ab12cd34).

Recovered panic while processing a command

Internal error while processing this command. This is a SchemaBot bug (error reference ab12cd34).

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR hardens SchemaBot’s GitHub PR-comment surfaces by preventing raw internal error strings (storage, GitHub API, transport, panic values) from being rendered verbatim, replacing them with fixed, user-safe summaries that point operators to server logs while preserving actionable, intentionally-authored user messages.

Changes:

• Introduces shared helpers to map errors into either (a) fixed “see server logs” PR-safe text for internal failures or (b) whitelisted user-authored messages for actionable cases.
• Updates webhook command paths (plan/apply/unlock/control/rollback/review gate/prior-env checks/panic recovery) to use the new rendering behavior.
• Adjusts and expands tests to assert that raw error text no longer appears in PR comments while preserving expected guidance and retry UX.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/webhook/unlock_test.go	Updates unlock comment assertions to require log-pointer wording and forbid raw storage error text.
pkg/webhook/templates/apply_test.go	Updates template tests to ensure GitHub API errors don’t render raw and prior-env blocks point to logs.
pkg/webhook/templates/apply_commands.go	Stops rendering raw GitHub error details in apply-blocked templates; replaces with log-pointer text.
pkg/webhook/schema_reconciliation.go	Adds typed closed-PR rejection for check-refresh flows and maps other failures to internal log-pointer text.
pkg/webhook/rollback.go	Converts multiple rollback failure comment paths to PR-safe internal error summaries while keeping guardrail rejections user-visible.
pkg/webhook/review_gate.go	Replaces raw review-gate error rendering with internal log-pointer text plus actionable permission guidance where applicable.
pkg/webhook/review_gate_test.go	Adjusts review gate tests to assert raw errors aren’t rendered and log-pointer wording is present.
pkg/webhook/plan.go	Converts internal failures to PR-safe summaries; uses stricter schema-request error rendering in multi-env plan flow and avoids silent failures.
pkg/webhook/plan_test.go	Updates tests for remote-unavailable guidance to ensure raw gRPC text isn’t rendered.
pkg/webhook/handler.go	Changes panic recovery comments to avoid printing panic values; records recovered panic metric.
pkg/webhook/handler_test.go	Updates panic recovery test to ensure panic value isn’t rendered and log-pointer wording is present.
pkg/webhook/error_comment.go	Adds internalErrorDetail and error-mapping helpers; removes raw remote transport error rendering from PR surfaces.
pkg/webhook/error_comment_test.go	Adds/updates tests for internal error detail, mapped user-facing errors, schema-request mapping, and control-command error rendering.
pkg/webhook/control.go	Routes control-command failures through a detail-mapper to avoid rendering raw internal failures on PRs while preserving user-actionable rejections.
pkg/webhook/check_prior_env.go	Updates prior-environment apply-blocked comments to avoid rendering raw storage/GitHub errors.
pkg/webhook/check_prior_env_test.go	Updates fail-closed prior-env tests to assert log-pointer wording and forbid raw storage error text.
pkg/webhook/check_aggregate.go	Introduces environmentConfigError to mark safe-to-render configuration/discovery messages while preserving typed causes in the error chain.
pkg/webhook/apply_handlers.go	Converts many apply/apply-confirm/unlock internal failure comment paths to PR-safe log-pointer summaries; adds unlock database inference error mapping.
pkg/webhook/apply_execute.go	Converts apply confirm-time plan failure rendering to user-facing mapping and hides raw status-check update errors.

---

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

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for pull/768, d0228cf.

Verdict: 8 findings — 4 blocking (an allowlist regression, a raw-error leak on the new control path, merge conflicts with main, a mislabeled metric), 4 non-blocking. The core sanitization sweep is real and well-tested; the blockers are all small fixes plus one careful rebase.

Blocking

1. NoSchemaFilesError regresses on the single-env path: a user-fixable outcome now renders as "Internal SchemaBot error — retry". The PR types this error precisely because its message is "safe to render on a PR" (schema.go#L36-L48) and allowlists it — but only in userFacingSchemaRequestError, which the multi-env loop uses. The single-env sink handleSchemaRequestError has typed branches for every other allowlisted class (DatabaseNotFound, config sentinels, environmentConfigError, truncated tree — plan.go#L568-L651) and none for NoSchemaFilesError, so it falls through to plan.go#L653. Scenario: schemabot plan -e production against a schema root with no files for that environment → the user gets "Internal SchemaBot error — retry (error reference …)" and retries forever, where the pre-PR fallthrough rendered the actionable "no schema files found under …". Also logs at Error and counts as metric reason "unexpected" for a config problem. Fix: add the typed branch (or route the fallthrough through userFacingSchemaRequestError).

2. The new control-command path still renders raw Tern-client-creation errors — the exact class this PR eliminates. controlTarget wraps client-creation failures as a 4xx typed error carrying the raw text: controlHTTPErrorf(http.StatusNotFound, "%s", err.Error()) (control_handlers.go#L433). Because the status is 4xx, IsInternalControlError is false and the PR's new controlCommandErrorDetail falls through to return err.Error() (control.go#L50). What that error carries: "resolve DSN for %s: %w" / "resolve token for %s: %w" from service.go#L498-L509 — secrets-backend paths and TLS wiring. Scenario: schemabot stop <apply-id> on a deployment with a broken DSN secret → the PR comment renders the secrets-file path. Every control verb routes through controlTarget, so this is ordinary-command reachable. Fix: wrap that one site as a 5xx/internal (its text was never authored for PR display).

3. The PR does not merge onto current main, and both conflicts sit on the PR's core mechanism. git merge-tree origin/main pr-768-review-tmp reports content conflicts in pkg/webhook/apply_handlers.go, pkg/webhook/handler.go, and pkg/webhook/handler_test.go. Two semantic overlaps make the rebase hazardous, not mechanical: (a) main's fix(github): post a fixed user-safe comment when a webhook goroutine panics #921 already shipped a competing recoverPanic fix — fixed message, no error reference, no metric (main handler.go#L943) — resolving toward main silently loses the reference and the panic metric this PR's body promises; (b) main's refactor(webhook): expose retry disposition for unlock commands #923 refactored handleUnlockCommand into unlockCommandCore with a typed retry-disposition contract (main apply_handlers.go#L651) while main's unlock path still posts raw err.Error() at multiple sites — a careless resolution either drops this PR's sanitization from the unlock path or breaks the retry contract pinned by main's new unlock-error-contract test. Rebase deliberately, keeping the PR side for the panic path and re-applying the ref plumbing inside the refactored unlock core.

4. The recovered-panic metric records under operation="unknown", not "webhook_command". The new call at handler.go#L941 passes "webhook_command", but knownRecoveredPanicOperations (metrics.go#L858-L866) doesn't contain it and RecordRecoveredPanic coerces unlisted operations to "unknown" — the PR doesn't touch pkg/metrics. Every recovered webhook panic is indistinguishable from an unregistered boundary. One-line fix: register the operation.

Non-blocking

5. Pre-engine internal failures still render raw through the "engine error" pipe. userFacingError maps only remote-unavailable and config-not-authorized; everything else renders err.Error() in full — intended for class-1 engine errors, but ExecutePlan/ExecuteApply wrap the same Tern-client DSN/token/TLS-resolution failures as "database %q (%s): %w" (plan_handlers.go#L495), and non-Unavailable gRPC codes (Internal, DeadlineExceeded) render their full rpc error: text. Reaches comments and failing aggregates at plan.go#L129, plan.go#L421 (multi-env, which never passes through userFacingErrorDetail), apply_handlers.go#L251, apply_execute.go#L49, rollback.go#L202. Needs a typed engine/pre-engine split at the api layer — reasonable as a tracked follow-up, but worth stating in the PR body since the class-1 carve-out ("engine plan/apply errors") doesn't describe these.

6. Rollback-confirm advises "— retry" where its apply twin deliberately doesn't, for the identical failure shape. rollback.go#L439 renders internalErrorDetail (retry) after ExecuteApply fails, but ExecuteApply can fail after the remote engine accepted work, and the rollback lock isn't released here — a retry re-executes the pinned rollback plan mid-rollback. The apply-side twin (apply_execute.go#L233) says "See SchemaBot server logs" with no retry advice for the same shape. One of the two is wrong; the no-retry variant looks right for both.

7. Three "see server logs" sites the body claims to have eliminated survive, with no error reference: apply_execute.go#L233, apply_execute.go#L244, and apply_gating.go#L41. No leak (fixed text), but users are pointed at logs they can't read, operators get no ref to grep, and the highest-stakes failure (apply execution itself) carries neither machine-matchable token.

8. "server config is unavailable" is reclassified from retryable-internal to terminal user-config rejection. environmentConfigErrorf("server config is unavailable") (check_aggregate.go#L131, #L143) makes a nil-service wiring condition take the config branch: rendered verbatim with no error reference, logged at Warn instead of Error, and handleSchemaRequestError now returns true (terminal) where the pre-PR fallthrough returned false (re-drivable). Latent today — the durability disposition is discarded for issue_comment commands — but it's a pre-wired wrong answer for the durable driver, and the Warn downgrade hides a process-level fault from Error alerting. Relatedly, the removed "resolve configured environments for database %q" wrapping means the multi-env cell log at plan.go#L370 carries no database attr for causes that don't self-identify.

General suggestions

• Make the machine pattern a tested contract, and conform the stragglers. The regex contract lives only in a doc comment; two live shapes already deviate: the no-allowed-environments comment (plan.go#L326) has a backticked reference but no "Internal SchemaBot error" anchor, and both checks-gate branches end with a colon instead of the documented period. Export a canonical pattern constant plus a test that runs it against every internal-error render.
• TEMPLATES.md silently drops the missing-permissions sample. The check-status preview now shows the generic branch, so the live "Grant or accept those permissions…" branch is documented nowhere. Add a second preview entry rather than swapping. (Verified TEMPLATES.md regenerates byte-identical via scripts/update-templates.sh — no generated-file drift.)
• Post-rebase defense-in-depth: main's fix(github): sanitize engine error text rendered into PR comments #891 added sanitizeCommentError but hooked only the engine-error writers; RenderGenericError — the sink for every postCommandError here — still renders ErrorDetail raw. Routing it through the sanitizer would backstop multi-line allowlisted messages (verified the fixed internal-error sentence passes that sanitizer untouched, so the regex contract survives). Also consider adding bare *api.DatabaseNotConfiguredError / *environmentNotConfiguredError to isUserMeaningfulSchemaRequestError — both are PR-authored but only reach the allowlist wrapped today.
• Centralize the three-line ritual. 45 non-test sites repeat newErrorReference() → logger.Error(..., "error_ref", ...) → postCommandError(..., internalErrorDetail(...)). A postInternalCommandError(summary, logMsg, err, attrs...) helper would make "ref rendered but never logged" unrepresentable and shrink ~35 sites to one line — relevant given the author's ~8 open PRs adding new comment sites.

The one thing that could have broken, verified

The fail-closed direction: a suppression allowlist this broad could easily swallow authored, user-actionable messages wholesale. Verified it mostly doesn't: every wrap layer of the allowlisted config sentinels composes SchemaBot-authored text plus identifiers only (no GitHub response bodies ahead of the sentinel); environmentConfigError.Unwrap preserves typed causes so the fan-out silent-skip and database-not-configured branches still fire (proven by TestEnvironmentConfigErrorPreservesCause and branch ordering at plan.go:611 before :629); and environmentNotConfiguredError cannot reach the multi-env allowlist at all, because the loop's environments are drawn from the same configured list that attachServerEnvironments membership-checks. The one true suppression gap that survived this audit is finding 1 — the single-env NoSchemaFilesError fallthrough.

Verified correct

• Every one of the ~40 newErrorReference() sites logs error_ref on the same log line as the raw error, and no site renders a reference it didn't log — the traceability contract holds diff-wide.
• The retained raw-looking site "Failed to acquire lock: "+err.Error() is gated by errors.Is(err, storage.ErrLockHeld), and every producer returns the bare authored sentinel — no driver text can flow through it.
• IsInternalControlError treats all untyped errors as internal (default 500), so the control path renders raw text only for typed 4xx guardrail messages — all authored, with the single exception in finding 2.
• The panic path keeps full server-side fidelity (value + stack logged); goSafe callers always pass real repo/PR coordinates; the comment no longer renders the panic value (test asserts NotContains "boom").
• No new raw-error comment sites landed on main in the 43-commit drift — every raw site on main today lives in a file this PR rewrites, so coverage is complete modulo the finding-3 rebase.
• Deleted symbols (remoteSchemaServiceUnavailableMessage, isUserFacingRemoteUnavailableError) leave no dangling references, and the new fingerprint logic can't double-wrap the formatted remote-unavailable text (retained test asserts byte-equal pass-through).
• CI is 32/32 green at head (same check set as fresh-base PRs — no missing suite), go test ./pkg/webhook/... passes in the worktree, TEMPLATES.md regenerates byte-identical, and the squash title (fix(github), 57 chars) matches repo conventions.

This review was generated by Claude Code (claude-fable-5).

[morgo]

🤖 Review from Morgan's AI agent. Holding — CI is red, and the failures look like the change rather than flakes.

Four checks are failing at 4982bb33: Integration Tests, Integration Tests (tern-webhook), E2E Tests, and E2E gRPC Multi-Deployment Tests (ordered). I pulled the logs, and the tern-webhook one is specific enough to be worth quoting:

pkg/webhook/plan_integration_test.go:1033
"## ❌ Plan Failed … Failed to prepare the schema change request.
 Internal SchemaBot error. Retry (error reference `844ff229`)."
   does not contain "after excluding ignored namespaces"

That reads as the PR working slightly too well. The goal — stop leaking raw internal error text onto a public PR surface and give the reader a reference code instead — is right, and it's the same instinct #1219 and #1224 apply to their operator-facing strings. But this particular error wasn't raw internal text; "no namespaces remain after excluding ignored namespaces" is a deliberate, actionable message, and it's now being replaced by an opaque reference. An author who misconfigures their ignore list used to be told what was wrong and could fix it themselves; after this they get error reference 844ff229 and have to ask someone to go read a log.

So the test isn't just pinning a string — it's catching a real regression in what an operator can self-serve. The E2E gRPC Multi-Deployment failure at multideploy_test.go:462 is a bare Should be true, so I can't attribute it as confidently, but it's plausibly the same class.

The fix is presumably a classification rather than a rework: errors that were authored for the reader keep their wording and skip the generic envelope; only genuinely internal errors (wrapped infra failures, dial errors, anything carrying hostnames or stack detail) get the reference-code treatment. Given the blast radius here — +775/-209 across 30 files, most of pkg/webhook — that boundary is worth drawing explicitly somewhere in the code, because every future error site has to land on one side of it.

Two notes beyond the CI:

• This is a large change to the webhook path even once green, so it stays human-reviewed under Morgan's standing bar regardless. I'm not going to stamp it.
• Worth a look at whether the error-comment envelope can lose the specific message entirely or merely wraps it. If the specific text is still recoverable and only the rendering drops it, this is a template fix; if the classification discards it upstream, the diagnostic is gone from the logs too, which is a bigger deal.

[morgo]

🤖 Automated review on Morgan's behalf — feedback, not an approval. Still holding on CI, but the hold may be cheaper to clear than it looks, and the review below is done so it's ready when CI is.

On the red checks — they're probably a stale flake, and a re-run would likely settle it. Two of the four are aggregates rolling up the other two, so the real failures are Integration Tests (tern-webhook) and E2E gRPC Multi-Deployment Tests (ordered), both from one run on 2026-09-01 at 12:13. That morning was rough for the integration shards generally:

• armand/observer-supersession-quiet failed the same tern-webhook shard at 11:53 and passed on re-run at 12:06.
• armand/subpercent-progress failed the same shard at 15:37 and passed on later runs.
• main itself failed Integration Tests (pkg/api) at 11:10.
• armand/spirit-failure-reason failed per-package and ./integration at 23:50.

So tern-webhook demonstrably flaked on unrelated branches either side of this run.

I'm still not stamping through it, and the reason is worth stating because it's the opposite of a case I did stamp through this week: there, a Postgres-engine PR was red only on k8s e2e jobs that couldn't execute its code at all. Here the failing shard is tern-webhook, and this PR changes pkg/webhook across twenty-plus files. The shard that failed is one that genuinely exercises the changed code, so "probably flaky" isn't good enough — and I can't attribute the E2E gRPC (ordered) failure to anything at all. A re-run of the two failed jobs would resolve it either way; the run is two days stale regardless.

The review itself came out clean. I went at the one question that decides whether this works: can raw error text still reach a PR comment?

• The allowlist is genuinely fail-closed. isUserMeaningfulSchemaRequestError enumerates typed errors and ends in return false, so anything unrecognized — a new storage error, a wrapped driver failure, an error type added next quarter — falls to the fixed sentence rather than rendering. That's the right default, and it's the one that has to be right for the PR's claim to hold over time rather than just today.
• No summary smuggles the error back in. The mechanism would be defeated by a single internalErrorDetail(fmt.Sprintf("...%v", err), ref), so I checked every call site the diff adds — around thirty — and they're all fixed literals. The one interpolation is a database name in the unlock path, which is already all over the PR surface and carries nothing from the error.
• newErrorReference fails toward posting, returning a fixed marker if crypto/rand fails rather than dropping the comment. Right trade: a comment with a useless reference beats silence.

The finding worth carrying forward: this PR's category-1 safety depends on #1246, and neither PR closes the seam alone.

userFacingError still ends in return err.Error(), which is correct — that's the "schema change failed" path, deliberately unchanged, because the engine's text is the actionable part (rejected DDL and so on). But that means plan.go's execution path renders whatever the engine hands it, and engine errors are exactly what carries hostnames, DSN fragments, and quoted failing statements. #1246 sanitizes that at the source. Together the two cover both halves — this PR the webhook's own internal failures, #1246 the engine's — but read separately, each looks like it has a hole the other one fills. Worth a line in the description pointing at #1246, since a reviewer checking only this PR will find err.Error() on a public comment path and reasonably wonder.

I did confirm the split is drawn in the right place: plan.go:380/386 route schema request errors through the fail-closed userFacingSchemaRequestError, while :437 routes plan execution errors through userFacingError. That's the intended line, not an inconsistency.

Nit: the reference is 4 bytes of crypto/rand — 8 hex chars, so a birthday collision lands around 65k references. The consequence is only that a quoted reference matches two log lines and you disambiguate by timestamp, so it doesn't matter much; 6 bytes would make it unambiguous if these are meant to be quoted in incident reports.

[morgo]

🤖 Automated review on Morgan's behalf — follow-up, not an approval.

Kiran's review agent left four blocking findings at d0228cf3 back on 2026-08-07. Since this PR has been sitting, I re-verified the three code findings against the current head 4982bb33 rather than assuming they were handled. One is fixed, one is materially reduced, and one is still live.

Finding 4 (metric operation) is fixed. knownRecoveredPanicOperations now contains "webhook_command", so recovered webhook panics no longer coerce to "unknown".

Finding 1 (NoSchemaFilesError) is still live, and it's an inconsistency inside this PR. The PR's own allowlist explicitly classifies it as user-meaningful — error_comment.go:150-153 has a typed errors.As branch for *ghclient.NoSchemaFilesError. But plan.go contains zero references to that type, and handleSchemaRequestError has typed branches for every other allowlisted class: DatabaseNotFoundError, ErrNoConfig, ErrInvalidConfig, ErrMultipleConfigs, environmentConfigError, ErrGitTreeTruncated. So on the single-env path it falls through to the internal-error tail and renders internalErrorDetail("Failed to prepare the schema change request.", errorRef), logs at Error, and counts under metric reason "unexpected".

The result is the same error type being user-actionable on the multi-env path and "Internal SchemaBot error — retry" on the single-env one. The user impact is the part that makes it worth fixing rather than tracking: a config problem is reported as an internal fault with retry advice that can never work, replacing a message that told them exactly what to do. Adding the typed branch — or routing the tail through userFacingSchemaRequestError — closes it.

Finding 2 (raw Tern-client errors on the control path) — the mechanism is still there, but the impact is much smaller than when it was written. Both halves still hold literally: control_handlers.go:459 returns controlHTTPErrorf(http.StatusNotFound, "%s", err.Error()) for a TernClient creation failure, and because 4xx means IsInternalControlError is false, control.go:50 falls through to return err.Error().

What's changed is what happens after. That string goes to postCommandError, and on this branch templates/errors.go:284 now runs quoteBlockLines(html.EscapeString(sanitizeCommentError(data.ErrorDetail))), with redactConnectionDetails making thirteen passes over it — DSN fragments, libpq password and parameters, absolute paths, host:port, hostnames, bare IPs. The specific scenario in the finding, a secrets-file path reaching the comment, is caught by the absolute-path pass.

So I'd downgrade this from a live leak to a defense-in-depth gap. It's still worth fixing, because regex redaction is the wrong thing to be the only barrier — a DSN shape nobody anticipated survives it, whereas not emitting the raw text in the first place has no such failure mode. Wrapping that one site as 5xx is a one-line change and makes the control path match the intent stated in controlCommandErrorDetail's own doc comment, which currently claims internal failures "never render raw."

Worth noting I made the inverse of this mistake myself. Reviewing #975's AV-8 entry, I concluded raw errors reached PR markdown because plan.go passes err.Error() to postCommandError — I checked the call sites and missed that the sanitizing happens at the rendering boundary. That's the same layer confusion, and it's why I traced finding 2 to the renderer before repeating it here.

On CI, one new data point. Integration Tests (tern-webhook) has now failed on a fourth independent branch in this window — armand/check-refusal-visibility on 2026-09-02 at 15:09, a day after this run. main itself failed twice on 2026-09-02, though on a different shard (pkg/api, which kiran01bm/deflake-pkg-api-shared-mysql is actively addressing). The job logs for this run have since expired, so the E2E gRPC Multi-Deployment Tests (ordered) failure remains unattributed — it's the one shard I can't place, and it hasn't recurred anywhere else in the window.

The run is now two days stale with no re-run and no new commits. Still holding — the failing tern-webhook shard exercises pkg/webhook, which this PR rewrites across twenty-plus files, so "flaky elsewhere" doesn't discharge it here. A re-run of the two failed jobs would settle it, and given finding 1 wants a commit anyway, pushing that fix would trigger a fresh run and resolve both at once.