fix(postmortem): a postmortem published while its incident is hidden is sent when the incident is made visible

[simlarsen]

The problem

Found in #4429: a postmortem published while its incident is hidden from status pages was never announced.

The status page shows a postmortem only on an incident it shows. So the send job settled the notification of a postmortem published on a hidden incident as Skipped ("Incident is not visible on status page. Skipping notifications to subscribers."). When someone later turned Visible on Status Page on, the postmortem appeared on the status page for the first time, but nothing queued its notification again. Only the API's Pending would ever send it.

What changes

• Making the incident visible sends the postmortem that was waiting for it, once. This covers every update that makes a hidden incident visible: the incident's Settings form, the API, Terraform and workflows. They all go through IncidentService.

  • The update queues the postmortem's notification only when it was skipped because the incident was hidden, and only when the postmortem is published once the update is written.
  • A notification that was sent, failed, is already on its way, or was skipped for another reason (Notify Subscribers off, no monitors, not published) stays as it is.
  • Hiding the incident and showing it again sends nothing more.

• A private incident counts as hidden. It is hidden from every status page, whatever its switch says, so its postmortem waits too. Making it visible means switching Visible on Status Page on and Private Incident off. That can happen in one save, or in two saves in either order. The job, the update and the Dashboard all use the same check (isIncidentShown). This matches how IncidentSubscriberAudienceBuilder already treats private incidents.

• The rule stays in one place. IncidentPostmortemPublication is the only place that decides. It gains:

  • isIncidentShown, mayShowIncident, isComparedBy and isShownByUpdate;
  • isHiddenIncidentSkip, isWaitingForIncidentToShow, isSentBySwitchingVisibilityOn and isDueOnceShown;
  • a new action from getNotificationAction, QueueIfSkippedAsHidden.

  The server, the send job and the Dashboard all ask it.

• No extra read before the write, and no postmortem body in it. The update reads isVisibleOnStatusPage, isPrivate and the notification's status and message in recordStoredValuesBeforeUpdate, the existing stored read in onBeforeUpdate. It reads them only when the update may show the incident: it switches visibility on, or writes Private Incident as off without switching visibility off. Saving a hidden incident's Settings reads nothing for this.

  • The postmortem note (VeryLongText) is not part of that read unless the update writes the postmortem itself.
  • Once the update is written, a single findOneById reads the note and the switches. This happens only when the notification stands skipped as hidden.

• The notification is queued once, even when two updates race. After the write, the hook queues the notification with a hook-free compare-and-set that expects both the status and the message (Skipped plus the hidden-incident message). Two updates that both show the incident cannot queue it twice, and neither can an update racing a send that settled in the meantime.

• The claim race is closed both ways, as Subscribers hear about a postmortem once, when it is published #4429 did for publishing.

  • The update looks again once it is written. That catches a run that skipped the notification as hidden before the write landed.
  • The job looks again after its own skip. That catches an update written after the skip.
  • The two skips the job looks again after (not shown, and hidden) now share one helper, requeueIfChangedSinceRead. It only moves the notification back while it still stands at the exact skip that run wrote.

• The skip says what happens next. The job's message is now "Incident is hidden from status pages. Subscribers will be sent the postmortem when the incident is made visible on status pages."

• The Dashboard explains it.

  • The Postmortem page labels the notification Not sent yet: incident hidden from status pages, with a clock icon, instead of "Notifications skipped." with a crossed-out circle. It does this only while the incident is hidden.
  • On the incident's Settings page, the Visible on Status Page switch says "This incident's postmortem was published while the incident was hidden, so subscribers have not been sent it. Turning this on sends it to them." It says this only while switching it on would send it, so never for a private incident.
  • The Settings card does not load the postmortem's body for this.
  • Both strings are translated in all 17 locales.

• Docs (English and Persian). Status pages → Subscribers → The postmortem gets a bullet for this case: private incidents, what happens to skips from earlier releases, and an API sentence naming isVisibleOnStatusPage and isPrivate. The incident page's Postmortem item and "Declaring hidden and publishing later" point to that section.

What changes for existing customers

• Hidden incidents with a published postmortem. If the postmortem was skipped because the incident was hidden (before or after the upgrade), its status page subscribers now get it, once, when someone makes the incident visible. That can be from the Dashboard, the API, Terraform or a workflow. Before, they never did.
• Postmortems an earlier release skipped this way, on an incident that is still hidden when you upgrade.
  • The migration MarkPostmortemsWaitingForHiddenIncidents1799100000000 gives them the new skip message. Making the incident visible then sends them, once.
  • The Postmortem page shows them as Not sent yet: incident hidden from status pages.
  • The migration changes only the message. It sends nothing, and the status stays Skipped.
• Postmortems an earlier release skipped this way, on an incident that is visible today. These incidents were made visible without anyone being told. They keep the old message, and nothing is ever sent for them, not even if the incident is hidden and shown again. Their postmortems have been on the status page for a while, and announcing them out of the blue would surprise people.
• One case the migration cannot tell apart. It cannot tell an incident that has stayed hidden since the skip from one that was shown and then hidden again before the upgrade, because nothing records it. Both are treated as waiting, so showing the latter sends its postmortem once.
• Through the API. A PUT that makes such an incident visible now moves subscriberNotificationStatusOnPostmortemPublished to Pending with its own guarded write. That means isVisibleOnStatusPage set to true, plus isPrivate set to false for a private incident. Nothing changes when the request sets that status itself.

Decisions

• It sends automatically; there is no checkbox. The incident-created notification asks before it publishes ("Notify subscribers that this incident was created") because announcing an old incident as new would mislead.
  • A postmortem is different. Whoever published it already chose Notify Subscribers.
  • Making the incident visible is the first time the status page shows the postmortem. That is the one rule: subscribers hear about a postmortem once, the first time the status page shows it.
  • The switch says so before you save.
  • Turning Notify Subscribers off on the postmortem still keeps it quiet, because the job reads that setting when it sends, as before.
• The skip is recognised by its message, as the created notification's is. The code review asked for a reason column instead. I kept the message, for consistency with IncidentCreatedRenotify.isHiddenFromStatusPagesSkip, which already works this way.
  • The text lives in one constant. The migration and the job, service and Dashboard tests pin it.
  • The guarded write expects both status and message, so a message changed since then is simply not queued.
  • A reason column would mean a schema change and a second migration for the same data.
• The old wording is no longer recognised in code. The migration moves the rows that are still hidden onto the new wording, and leaves the others alone (see above).
• "Status pages added" is not a trigger. I checked this in the code.
  • The job's hidden skip only happens while the incident is hidden.
  • Pages added to a hidden incident are covered when it is made visible: the postmortem goes to every page the incident reaches then.
  • Adding pages to an incident that is already visible does not send the postmortem. Either it was sent already, or it was skipped for another reason.
  • The postmortem has no per-page record like the created notification's statusPagesNotifiedOnCreation, so sending it again would reach the earlier pages twice.
• It reads as its own event. Showing the incident queues the notification with "Incident made visible on status pages. Subscribers will be sent its postmortem shortly."

Left out

• The Settings switch's note when the postmortem's note was emptied. The Settings page no longer loads the postmortem body, following the code review. So if someone empties the note of a waiting postmortem but leaves Publish on Status Page on, the switch still says it would send the postmortem.
  • The server reads the note after the write and sends nothing in that case.
  • The Postmortem page, which loads the note, reads "Notifications skipped.".
• A per-page record for the postmortem notification. It would let a page added to a visible incident's scope be told without telling the other pages twice. An incident limited to pages that do not show it is settled as "Not sent to any subscriber: no status page was sent this notification." (Success), and a page added later is not told. This is a follow-up candidate if it matters.
• Combining the created-notification read with this one. The created notification's "Notify subscribers that this incident was created" hook keeps its own read on publish, as before. The postmortem comparison adds no read before the write.
• Related weakness found, not fixed: the status page does not filter private incidents. StatusPageAPI filters incidents on isVisibleOnStatusPage only, never on isPrivate. Making an incident private switches its visibility off (IncidentService.onBeforeUpdate). But an API write of isVisibleOnStatusPage: true alone on an incident that is already private stores both as on, and the status page then shows it. Subscriber notifications, including this postmortem, treat it as hidden. This is pre-existing and worth its own fix.

Tests

• Regression test, which fails on master: App/Tests/Workers/Jobs/Incident/PostmortemPublishedNotifiesOnce.test.ts runs the real update hooks and the real send job against an in-memory incident. It covers:

  • hidden → publish → job skips → made visible → sent once;
  • hiding and showing it again sends nothing more;
  • a postmortem already sent is not sent again;
  • an unpublished postmortem, or one taken off while hidden, sends nothing;
  • a skip from an earlier release, for an incident still hidden and for one shown since;
  • private incidents: switched on and left private, then made not private; and both in one save;
  • a hand-written "true";
  • made private in the same save;
  • Notify Subscribers turned off since;
  • no monitors;
  • published and shown in one save;
  • shown with the created-notification box ticked;
  • the claim race both ways.

• Unit and service tests:

  • Common/Tests/Types/StatusPage/IncidentPostmortemPublication.test.ts covers the rule, private incidents included.
  • Common/Tests/Server/Services/PostmortemNotifyOnPublish.test.ts runs through the real onBeforeUpdate and onUpdateSuccess. It covers:
    • the one stored read and its exact select, without the note;
    • the read after the write;
    • the guarded write and its expectation;
    • every status that stays as it is;
    • the race;
    • bulk updates;
    • private incidents;
    • the caller's own status;
    • Notify Subscribers not being read.

• API and job tests:

  • Common/Tests/Server/API/PostmortemNotifiesOnceThroughTheApi.test.ts exercises PUT /api/incident/:id with isVisibleOnStatusPage and isPrivate. It covers a viewer being refused, hiding and showing again, and an earlier release's skip. The stand-in database now keeps what each write sets.
  • App/Tests/Workers/Jobs/Incident/SendPostmortemNotificationToSubscribers.test.ts covers the job's hidden skip (private incidents included), which had no test before, and its look again.

• Migration: Common/Tests/Server/Infrastructure/Postgres/MarkPostmortemsWaitingForHiddenIncidentsMigration.test.ts checks the SQL contract, the texts and the registration. It also has an opt-in Postgres suite (RUN_POSTGRES_POSTMORTEM_WAITING_MIGRATION_TESTS) that covers:

  • hidden, never set and private rows;
  • shown rows;
  • other statuses and other reasons;
  • the created notification's columns;
  • running twice;
  • down().

  The flag is wired into the Common Test workflow, and Tests/Ops/PostgresSuitesRunInCi passes.

• Dashboard:

  • Common/Tests/App/Dashboard/IncidentPostmortemPage.test.tsx checks the label: hidden, private, earlier wording, visible, another reason, queued.
  • Common/Tests/App/Dashboard/SubscriberNotificationStatusWaiting.test.tsx checks the clock icon.
  • Common/Tests/App/Dashboard/IncidentSettingsRenotifyOnPublish.test.tsx checks the switch's description: when it appears and when it does not, private included. It also checks that the card does not load the note, and renders the description in the real ModelForm.

• Guards and docs:

  • App/Tests/Dashboard/PostmortemWaitsForIncidentI18n.test.ts checks the strings in all 17 locales.
  • App/Tests/PostmortemPublicationOneRule.test.ts is the source guard.
  • App/Tests/FeatureSet/Docs/PostmortemNotifiesOnceDocs.test.ts covers the English and Persian docs.

• Two existing tests were updated because showing an incident now also reads it once for its postmortem: IncidentCreatedRenotifyOnPublish and IncidentStatusPageScope count only the created notification's own reads now.

Verification

I ran the following locally, on the branch with the newest master merged in.

• The regression test fails before the fix and passes after. On the unfixed code, PostmortemPublishedNotifiesOnce reported "is announced once, when the incident is made visible" as failing, because nothing was sent. It passes now.

• The new tests catch regressions. I broke the code-review fixes one at a time and re-ran the postmortem suites. Every break made tests fail:

  Break	Failing tests
  Private incidents counted as shown	16
  The earlier wording recognised again	7
  The Settings switch's note shown for a private incident	2
  Making an incident not private not compared	7
  Queueing without reading the incident after the write	3
  The job's look again ignoring visibility	10
  The waiting badge keeping the crossed-out circle	1

• The migration on real Postgres. The opt-in suite passed, 24 tests, against a postgres:15 container of my own (postmortem-visible-later-pg), before and after the merge. Tests/Ops/PostgresSuitesRunInCi passes (65 tests) with the new flag wired into the Common Test workflow.

• Type checks:

  • npm run compile exits 0 in packages/Common and in packages/App. App's compile also type-checks App/Tests, and it ran again after the merge and after the last commit.
  • npm run compile-tests exits 0 in packages/Common (all three parts).
  • tsc --noEmit exits 0 in the Dashboard.

• jest, App:

  • Tests/Dashboard in full: 618 suites, 32,859 tests passed.
  • Tests/FeatureSet/Docs in full: 146 suites, 6,368 tests passed.
  • Tests/Workers/Jobs/Incident, Tests/Workers/Jobs/StatusPageSubscriber, Tests/Notification and the guards near this change: 73 suites, 3,191 tests passed.

• jest, Common:

  • Tests/UI in full: 821 suites passed.
  • Every suite that names IncidentService, plus the Postmortem and Settings page suites, the badge, the migration suite and Tests/Types/StatusPage. With CI's billing settings (BILLING_ENABLED=true and the SUBSCRIPTION_PLAN_* lines from test-setup.sh): 201 suites, 11,353 tests passed. With billing off: 198 suites, 11,272 tests passed. The 5 skipped suites are opt-in Postgres suites.

• jest, Home: 38 suites, 2,257 tests passed.

• i18n and lint:

  • npm run i18n:extract adds nothing.
  • npm run i18n:check passes ("Every locale file is sound and no translation was lost").
  • eslint --fix and prettier --check are clean on every changed file. I linted Common, App and Dashboard in separate runs.

I did not run the live stack, the Playwright E2E or offline suites, or the whole repository's tests. No offline fixture page sends a new request: the Postmortem and Settings cards only select more columns of the same incident read.

🤖 Generated with Claude Code

[simlarsen]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.