Repository navigation
fix(email): link the check page in incident emails instead of the down host - #274
Merged
Merged
Conversation
…ow-ups Work that was sitting uncommitted in the working tree, committed as-is on its own so the batch that follows starts from a clean base rather than carrying it silently into unrelated commits. * `ListOrgEntitlementAudits` returns an empty slice rather than nil on both drivers. bun leaves the destination nil when nothing matched, and the list is serialized straight onto the API, so an org with no history marshalled as `null` — which every caller had to guard before touching `.length`. * Follow-ups to the banner and rate-limit helpers, the superadmin entitlements editor, the availability strip, and their tests, plus the entitlements wiki page. Verified before committing, since none of this was written in this session: `go build ./...` clean, `tsc --noEmit` clean, dash0 unit tests 1053/1053, and the entitlements handler package green. `TestIncrementUsageCounter_Concurrent_Postgres` fails locally with "too many clients already", but it fails identically on a clean tree at f11d7b6 with none of this applied, and CI's backend job passed on #269/#270/#271 — pre-existing and environmental, not from these changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Track both specs so the batch starts from a recorded queue state. Spec 05 gains a Status section: its steps 1, 2 and 4 shipped in v0.19.0 (commit 1072984, via #269), leaving only the step 3 rename. Without this an implementer reading the file fresh would redo the E2E work that is already released. The rename was deferred because it collided with uncommitted work on check-rate-limit-banner.tsx; that work is now committed in e401b9e, so it is unblocked. Spec 06 (incident emails render the bare check name, so mail clients auto-link the target host instead of the check page) is queued as written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tifiers The link was retargeted from the Usage page to the check scheduling page in spec 2026-08-26-04, but the test id (check-rate-limit-usage-link) and the i18n key (checkRateLimit.viewUsage) still said "usage". Rename both to check-rate-limit-scheduling-link / checkRateLimit.reviewScheduling across the component, its unit test, the E2E spec, and the locale coverage map. The translated copy itself is unchanged in all four locales.
… the pre-2026-08-26-04 link target, and fails
…the pre-2026-08-26-04 link target, and fails
Bare CheckName text in the details-table row (and h1 headings, and the plain-text parts) let mail clients auto-linkify the hostname-looking check name into a link straight at the down target, instead of the dashboard check page. Wrap it in the same conditional CheckURL anchor the intro paragraphs already use, across all 8 incident templates, and add an explicit "Check URL:" line to the text parts (an anchor can't stop client-side auto-linking in plaintext).
checkDashURL (shared by Slack/Teams/Discord/PagerDuty via email.go/ slack.go) and escalationCheckURL (job_escalation_step.go) both linked by check.Slug and returned "" whenever it was nil, silently leaving the check unlinked. Build from check.UID instead: it's always present, survives a check rename between send and click, and never falls back to unlinked text.
Update the fixtures/tests that pinned the old slug-shaped check dashboard URL (emailpreview fixtures.go, formatter_test.go, email_test.go, slack_test.go) to a UID-shaped one, matching the checkDashURL/escalationCheckURL switch. Add regexp assertions on incident-created.html and escalation.html proving the details-table "Check" row renders an <a href> around the check name rather than bare text, plus a text-part assertion for the new "Check URL:" line.
…ame, so mail clients auto-link the hostname instead of the check page
…me, so mail clients auto-link the hostname instead of the check page
chromedp/headless-shell's version IS the Chrome version — the pin in docker-compose.yml is 151.0.7922.109 — so every ~4-week Chrome release reads to renovate as a *major* bump and gets held by the "Major updates: no automerge" rule, even though nothing breaking happened. #263 (v151 -> v152) is exactly that: a one-line docker tag change sitting open for manual review it does not need. Scoped narrowly to this image rather than opening up docker majors in general: a postgres or bun major genuinely can break, and should still be looked at. This one is pinned only in docker-compose.yml — local dev and the E2E browser, never a shipped artifact — and CI's E2E job exercises it on every PR, so a bad Chrome release fails the pipeline rather than reaching a user. Follows the existing goja exception, and is placed after the major rule for the same reason: renovate applies packageRules in order and the last match wins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…stored in plaintext The env provider lowercases and turns every underscore after SP_ into a dot, so SP_ENCRYPTION_MASTER_KEY arrived as "encryption.master.key" and never reached the koanf tag "master_key". Nothing bound it, so the credentials service stayed in disabled mode - and its own remediation message told the operator to set exactly the variable that did not work. Every other multi-word key already has a hand-written applyXxxEnv helper (ACME, rate limiting, PostHog, WhatsApp, scheduling...). Encryption never got one. This adds applyEncryptionEnv for MASTER_KEY, MASTER_KEY_FILE and AUTO_MIGRATE, and registers the three names in manualReaderEnvVars. Three assertions are inverted rather than deleted, because they were right when written: TestRecognizedEnvVars and the envcheck test both asserted the names were NOT recognized, with comments warning that registering them would paper over the bug. The bug is fixed at the source, so recognition now states a fact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fclairamb
enabled auto-merge (squash)
August 27, 2026 21:47
fclairamb
disabled auto-merge
August 27, 2026 21:47
fclairamb
enabled auto-merge (squash)
August 27, 2026 21:47
fclairamb
disabled auto-merge
August 27, 2026 21:47
fclairamb
enabled auto-merge (squash)
August 27, 2026 23:18
Merged
fclairamb
added a commit
that referenced
this pull request
Aug 28, 2026
🤖 I have created a release *beep* *boop* --- ## [0.19.1](v0.19.0...v0.19.1) (2026-08-28) ### Bug Fixes * **config:** bind SP_ENCRYPTION_MASTER_KEY so credentials stop being stored in plaintext ([#276](#276)) ([15615ad](15615ad)) * **deps:** update github.com/dop251/goja digest to 8f1c069 ([#275](#275)) ([550ed79](550ed79)) * **email:** link the check page in incident emails instead of the down host ([#274](#274)) ([0daaf40](0daaf40)) * name the license file LICENSE so pkg.go.dev detects it ([#272](#272)) ([f11d7b6](f11d7b6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This was referenced Sep 8, 2026
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batch branch for 2026-08-27. Two specs implemented end to end, plus the follow-ups that were sitting uncommitted in the working tree and a Renovate rule.
Incident emails link the check page (spec 06)
An incident email showed the check name as bare text in the details table. Checks are usually named after their target host, so Gmail and friends auto-linkified it into
https://<the-host-that-is-down>— a plausible-looking link strictly worse than the real one.checkDashURL,escalationCheckURL) build from the check UID instead of its slug. That survives a rename between sending and clicking, and drops the nil-slug early return that left the name unlinked. This also upgrades the Slack/Teams/Discord/PagerDuty notifiers, which sharecheckDashURL.Check URL:line — no anchor can stop auto-linking there, so the real link is made to win instead.<a href>around the name.Stale identifiers renamed (spec 05)
check-rate-limit-usage-link→check-rate-limit-scheduling-linkandorg:checkRateLimit.viewUsage→org:checkRateLimit.reviewScheduling. Both names still said "usage" after v0.19.0 retargeted that link to the scheduling page — which is what led a previous reader to conclude it was a self-link and write a now-wrong assertion.Translated copy is unchanged in all four locales; only the key moved. A different
viewUsagekey in thestatusPagesnamespace ("View plan & usage") is genuinely about usage and is deliberately untouched.Entitlements follow-ups
ListOrgEntitlementAuditsreturns an empty slice rather than nil on both drivers — bun leaves the destination nil when nothing matched, and the list is serialized straight onto the API, so an org with no history marshalled asnulland every caller had to guard before touching.length. Plus follow-ups to the banner and rate-limit helpers, the superadmin editor, the availability strip and their tests.Renovate: automerge chromedp/headless-shell
Its version is the Chrome version, so every ~4-week Chrome release reads as a major and gets held by the no-major-automerge rule — #263 was exactly that, a one-line docker tag change awaiting review it did not need. (#263 has since been merged by hand; this stops the next one from queueing up.)
Scoped narrowly to this image rather than opening up docker majors in general — a postgres or bun major genuinely can break. It is pinned only in
docker-compose.yml, and CI's E2E job exercises it on every PR. Follows the existinggojaexception and sits after the major rule, since Renovate's last match wins.Test plan
make build-backend lint-back test— greenmake build-dash0+ dash0 lint — green, no new errors against the pre-existing debtentitlements-usage.spec.tsupdated for the rename; authored-not-run locally (the:4000devloop is not inSP_RUNMODE=test)Each spec was independently audited against its requirements before archiving, not taken on the implementer's word.
🤖 Generated with Claude Code