Skip to content

[pull] main from hookdeck:main#154

Merged
pull[bot] merged 2 commits into
erickirt:mainfrom
hookdeck:main
Jun 21, 2026
Merged

[pull] main from hookdeck:main#154
pull[bot] merged 2 commits into
erickirt:mainfrom
hookdeck:main

Conversation

@pull

@pull pull Bot commented Jun 21, 2026

Copy link
Copy Markdown

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

alexluong and others added 2 commits June 21, 2026 10:58
…usted-retries alerts (#964)

* feat(alert): add Settings + enable gates for consecutive/exhausted alerts

Introduce alert.Settings (the resolved, operational alert config) plus two
monitor gates: WithConsecutiveFailureEnabled and WithExhaustedRetriesEnabled.

Both default to true, so behavior is unchanged until a caller opts out. When
consecutive-failure alerting is gated off the monitor neither tracks failures
nor auto-disables; when exhausted-retries is gated off it never emits, even
with retries enabled. Extracts the consecutive-failure path into a helper to
keep the replay/ordering semantics identical on the enabled path.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* feat(config): resolve alert config to alert.Settings with unset/empty/value rule

AlertConfig.ConsecutiveFailureCount and ExhaustedRetriesWindowSeconds become
*string so the parse layer can tell three states apart: unset uses the default
(100 / 3600), an empty string disables that alert dimension, and any other value
must parse to a non-negative integer.

AlertConfig.ToConfig resolves the raw values into the operational alert.Settings
(domain-owned, so nothing downstream imports config). Validate rejects malformed
values at startup. builder wires the resolved gates into the monitor and only
builds the exhausted-retries suppression window when enabled with a positive
window (0 = alert on every exhaustion).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* docs(config): document alert default/disable behavior in config reference

Update the Alerts section of the self-hosting config reference for the new
unset/empty/value rule: ALERT_CONSECUTIVE_FAILURE_COUNT defaults to 100 (empty
disables), and document the previously-undocumented
ALERT_EXHAUSTED_RETRIES_WINDOW_SECONDS (default 3600, empty disables, 0 = no
suppression).

Also correct stale entries: drop the removed ALERT_CALLBACK_URL, fix the
ALERT_AUTO_DISABLE_DESTINATION default (false, not true), and fix the YAML
example key (alert, not alerts).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* docs(openapi): describe alert behavior in ManagedConfig

Document the unset/empty/value behavior for ALERT_CONSECUTIVE_FAILURE_COUNT,
ALERT_EXHAUSTED_RETRIES_WINDOW_SECONDS and ALERT_AUTO_DISABLE_DESTINATION in the
ManagedConfig schema. Descriptions only — the properties are already typed as
string. SDKs are regenerated from this schema at release time.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* fix(config): make empty-string alert disable work on the env-var surface

The *string representation had two problems found by manual QA:
1. caarlos0/env ignores a present-but-empty env var, so `ALERT_..._COUNT=`
   resolved to the default instead of disabling — the empty=off rule only
   worked via YAML, not env vars (the primary surface for the cloud product).
2. caarlos0/env crashes ("expected a pointer to a Struct") on any non-nil
   *string it walks, so setting these in a YAML config file would crash startup
   (env.Parse runs after the YAML load).

Replace *string with an OptionalString value type that implements both
TextUnmarshaler (bound by caarlos0/env as a scalar — no crash) and
yaml.Unmarshaler (so `key: ""` expresses the empty/off state). The one case
caarlos0/env cannot surface — a present-but-empty env var — is handled
explicitly via OSInterface.LookupEnv, which also gives env precedence over YAML.

Net: unset -> default, empty -> disabled, value -> value, identically on both
env and YAML, with env > yaml. Adds a full parse-path test covering the matrix
and precedence.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* fix(e2e): use OptionalString for ConsecutiveFailureCount in regression test

Missed call site when migrating AlertConfig fields to OptionalString;
the raw int assignment broke the cmd/e2e build.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@pull pull Bot locked and limited conversation to collaborators Jun 21, 2026
@pull pull Bot added the ⤵️ pull label Jun 21, 2026
@pull pull Bot merged commit 800c29d into erickirt:main Jun 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant