shell: Add notification boards, replacing page_status by pfeifferj · Pull Request #23314 · cockpit-project/cockpit

pfeifferj · 2026-05-26T20:34:48Z

the hardcoded allowlist in PageStatusNotifications is replaced by a generic notification board mechanism that any page can post a single entry to and that the Shell aggregates

closes #23221 (alternative to #23127)

Venefilyn

Mostly went over the new notification structure in the first commit, WDYT?

Venefilyn · 2026-06-04T11:16:33Z

+ *  - page_status: legacy per-page status keyed by [host, page].  Used
+ *    for shell navigation icons.


If it's legacy and not recommended I'd say deprecated instead

Venefilyn · 2026-06-04T11:17:25Z

+ *
+ * The details are all still experimental and subject to change.


Once merged it shouldn't be experimental anymore IMO

Venefilyn · 2026-06-04T11:22:01Z

 export const page_status = new PageStatus();
+
+export const CHANNELS_KEY = "cockpit:notification-channels";
+export const OVERVIEW_HEALTH_CHANNEL = "overview:health";


Seems to only be used as a fallback/default. Can be renamed to reflect that. E.g. CHANNEL_DEFAULT

Venefilyn · 2026-06-04T14:02:03Z

+
+    publish(notification: ChannelNotification): void {
+        if (!notification || typeof notification.id !== "string" || notification.id === "")
+            throw new Error("NotificationChannel.publish: notification.id is required");


Could be its own error class

Venefilyn · 2026-06-04T14:04:54Z

+        if (dequal(prev, notification))
+            return;


Could be faster with quick undefined check

Suggested change

if (dequal(prev, notification))

return;

if (prev && dequal(prev, notification))

return;

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/get#return_value

Venefilyn · 2026-06-09T15:20:32Z

+    #init_notification_channels() {
+        sessionStorage.removeItem(CHANNELS_KEY);
+    }


Clearing notifications channel should be done with a function in notifications.ts I think. Or we move all sessionStorage stuff out of notifications

Venefilyn · 2026-06-09T15:22:22Z

+    #notify_channel(host: string, page: string, channel: string, id: string,
+        notification: ChannelNotification | null) {


This is enough params that I think we can use an object for it instead. {host, page, channel, id, notification}

Venefilyn · 2026-06-09T15:25:08Z

+    return ch;
+}
+
+/* - publish_page_health(page_id, status, opts?)


Can add in @deprecated. Though I wonder if we should even include it at all

Venefilyn · 2026-06-09T15:49:34Z

@mvollmer pinging you as well if you can do review

mvollmer · 2026-06-10T07:45:55Z

Nice!

I think the original idea to have some generic mechanism where any page can tell the Shell to put an icon and a tooltip into its navigation entry was good, and we should of course keep this functionality.

But driving the health card via that same mechanism was not a good idea, and we (I) probably only did that because it was convenient, implementation wise. You channel idea is much better.

Since the health card currently has a hard coded list of pages that it listens to, we can make some breaking changes to the existing API (since we know all places that will be affected).

So what about going further here in this PR already:

We don't ever mention "two mechanisms" in the documentation, we only document channels.
The shell nav icon stuff is documented to be done via the "shell:page-status" channel. It will not have a "details" field anymore.
When publishing to the "shell:page-status" channel, the code in pkg/lib/notifications will 'silently' also publish via the old mechanism, just in case a page runs in a old shell.
The old page_status.set_own API is just a wrapper around channel("shell:page-status").publish and it will ignore anything except type and title.
Or we might just outright remove it. Projects will have to adapt to this breaking change when they next update pkg/lib/. I think that is acceptable in this case.
Same for page_status.get.

I think we should make dedicated little APIs for specific channels, maybe even in their own files:

shell_set_page_status(type: string, title: string)
system_set_health_status(...)

How does that sound?

mvollmer · 2026-06-10T08:06:12Z

+
+    const t = status.type;
+    ch.publish({
+        id: page_id,


I'd say it should not be necessary for a page to know its own id. The old page_status.set_own doesn't need that and the Shell will figure it out because it knows where the message comes from. Why do we need an id for channels?

it was there so one publisher could hold multiple entries and clear them independently, but nothing uses that, every caller publishes one entry with id = page name

mvollmer · 2026-06-10T08:30:37Z

+ *  - page_status: legacy per-page status keyed by [host, page].  Used
+ *    for shell navigation icons.
+ *
+ *  - channel() / NotificationChannel: consumer-managed named channels


I think we need a different name here instead of "channel", or at least qualify it.

A "channel" is a very fundamental concept in the Cockpit architecture, see doc/protocol.md, and we need to make sure these notification channels are not confused with them in the mind of a code reader.

Hmm. What about "notification_board"? But if we export functions like "set_health_status" from pkg/lib, maybe the name "channel" here isn't actually part of the API that most people use.

Notification board

Notification box

Notification tap

tap sounds kinda cool :D

I vote for "board", to emphasize the permanent nature of the notifications, and de-emphasize the transport mechanism.

From Matrix

mvollmer: If we go with keeping notifications in _internal and nobody uses the "channel" name in client code, then I am fine with using "channel". Or just "notification_channel" to distinguish it enough from cockpit.channel.

mvollmer: the localStorage key name also doesn't matter a whole lot, but since it is the storage, it should probably not be called "channel", but rather "cockpit:notifications" (which then include a channel identifier as a attribute)

mvollmer: or something else that conveys "this is the current set of things received over the notification channels" as opposed to "this is a channel", which it isn't.

mvollmer: but if we want something else than "channel"... I vote for "notification board"

Which SGTM. Board works

mvollmer · 2026-06-10T08:31:24Z


 export const page_status = new PageStatus();
+
+export const CHANNELS_KEY = "cockpit:notification-channels";


"cockpit:notification-boards"? It kinda makes sense to me....

mvollmer · 2026-06-10T09:41:52Z

So, thinking a tiny bit more about this:

We should have a minimal public API for exactly the things that we need (page_status, health_status), and not commit to anything else for now.

Thus:

Move pkg/lib/notifications to pkg/lib/_internal/
Create pkg/lib/shell and export page_status = channel("shell:page-status") from it, with just type and title and no mention of details. Channel might need to become a generic type for that so that page_status.publish({ ...}) would only accept type and title.
Create pkg/lib/overview and export health_status = channel("overview:health") from it, with type, title, and link. "type" would accept anything previously allowed for details.icon or details.pficon.
Change all pages to use spage_status.publish() and health_status.publish() instead of page_status.set_own.
Change the Shell and the Overview to page_status.list etc.

That would keep the "channel" name out of the public API, which would make me happy. It would also make it clear that the Shell is the one defining what a "page status" is, and the Overview what a "health status" is,, and that there is currently nothing else.

(I can see how we add show_notification to pkg/lib/shell later for showing actual toast notifications, or other things related to controlling the Shell from a page, including moving cockpit.jump() there and other Shelly things currently part of the "cockpit" module.)

+    const handler = () => { fired++ };
+    ch.addEventListener("changed", handler);
+
+    window.dispatchEvent(new StorageEvent("storage", { key: "cockpit:page_status" }));


+    ch.addEventListener("changed", handler);
+
+    window.dispatchEvent(new StorageEvent("storage", { key: "cockpit:page_status" }));
+    window.dispatchEvent(new StorageEvent("storage", { key: "unrelated" }));


+
+    window.dispatchEvent(new StorageEvent("storage", { key: "cockpit:page_status" }));
+    window.dispatchEvent(new StorageEvent("storage", { key: "unrelated" }));
+    window.dispatchEvent(new StorageEvent("storage", { key: CHANNELS_KEY }));


+                if (typeof data.channel === "string" && typeof data.id === "string") {
+                    const n = data.notification;
+                    const valid = n === null || n === undefined ||
+                                  (typeof n === "object" && n !== null && !Array.isArray(n));


mvollmer · 2026-06-10T10:22:46Z

About compatibility:

We should keep the nav icons working with any combination of shell and page versions. But we can assume that the health card stuff is all updated at the same time.

The Shell should keep processing "notify" events with a "page_status" field and should decorate the navbar with icons and popups announced with them. A call to page_status.publish() should send a "notify" message with bothe "page_status" and the new "channel" stuff in it.

pfeifferj · 2026-06-16T09:11:41Z

Move pkg/lib/notifications to pkg/lib/_internal/
Create pkg/lib/shell and export page_status = channel("shell:page-status")
Create pkg/lib/overview and export health_status = channel("overview:health")
That would keep the "channel" name out of the public API

sounds good to me, I can make those changes

The old page_status.set_own API is just a wrapper [...] Or we might just outright remove it.

removing it is probably cleaner yea

Venefilyn · 2026-07-17T13:49:50Z

+export interface Notification {
+    type?: string | null;
+    title?: string;
+    [key: string]: JsonValue | undefined;
+}


Why is the notification data itself part of the notification message? I feel like this should be within a sub-field "details" like before. That way it is more flexible and easier to make changes in the future

I also think we should add a generic as mentioned in my previous review #23314 (comment)

Venefilyn · 2026-07-17T13:54:53Z

+export interface NotificationControlMessage {
+    page_status?: Notification | null;
+    board?: unknown;
+    notification?: unknown;
+}


What's the difference between page_status and notification? Why can page_status accept both null and undefined?

What would board and notification style be? Can we make this easier without unknown?

Venefilyn · 2026-07-17T14:03:30Z

+// Distinct from null so a fresh instance after a frame reload still sends its
+// first publish/clear, even if it matches the stale registry entry.
+const UNSENT = Symbol("unsent");


This is also mentioned in notifications.ts. Consolidation?

Venefilyn · 2026-07-17T14:24:22Z

+    // "type" carries "info"/"warning"/"error" or a PatternFly icon name
+    // accepted by get_pficon() in pkg/systemd/page-status.jsx (e.g.
+    // "security", "bug", "spinner", "check", "enhancement").
+


Venefilyn · 2026-07-17T14:24:46Z

@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2019 Red Hat, Inc.


Beep beep! 2026 please

Venefilyn · 2026-07-17T14:24:53Z

@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2019 Red Hat, Inc.


Beep beep! 2026 please

Venefilyn · 2026-07-17T14:32:05Z

Marius is out for 3 weeks so will try to go through this. Ping me on Matrix on Slack when you have time and we could go over it faster - either by chat or meeting

Introduce a generic notification board (pkg/lib/_internal/notifications) that any page can post a single entry to and that the Shell aggregates. Expose two small typed facades on top, page_status (pkg/lib/shell) and health_status (pkg/lib/overview), so the generic "board" name stays internal and is not confused with protocol channels (doc/protocol.md). Signed-off-by: Josephine Pfeiffer <hi@josie.lol>

Aggregate board posts by (board, host, page) and persist them under cockpit:notifications. Drive nav icons from the shell:page-status board, folding the legacy page_status notify field from older pages into it so icons keep working across shell and page versions. Signed-off-by: Josephine Pfeiffer <hi@josie.lol>

Publish updates and services health through the page_status and health_status facades. The Overview health card now lists the overview:health board instead of polling a hard-coded page list. Signed-off-by: Josephine Pfeiffer <hi@josie.lol>

Signed-off-by: Josephine Pfeiffer <hi@josie.lol>

Cover the board mechanism with qunit tests and remove the obsolete page_status module now that every consumer uses the facades. Signed-off-by: Josephine Pfeiffer <hi@josie.lol>

Venefilyn self-requested a review June 3, 2026 16:31

Venefilyn requested changes Jun 9, 2026

View reviewed changes

Venefilyn requested a review from mvollmer June 9, 2026 15:49

mvollmer reviewed Jun 10, 2026

View reviewed changes

github-advanced-security AI found potential problems Jun 10, 2026

View reviewed changes

pfeifferj force-pushed the feat/notification-channels branch from 85a8501 to 5471ee5 Compare June 21, 2026 14:50

pfeifferj changed the title ~~feat(notifications): add consumer-managed channels~~ shell: Add notification boards, replacing page_status Jun 22, 2026

pfeifferj mentioned this pull request Jun 22, 2026

shell: Add notification boards, replacing page_status #23127

Closed

Venefilyn reviewed Jul 17, 2026

View reviewed changes

pfeifferj added 5 commits July 19, 2026 22:32

playground: Demo notification board publish and subscribe

2e2d125

Signed-off-by: Josephine Pfeiffer <hi@josie.lol>

qunit: Test notification boards and drop the page_status module

fc47d09

Cover the board mechanism with qunit tests and remove the obsolete page_status module now that every consumer uses the facades. Signed-off-by: Josephine Pfeiffer <hi@josie.lol>

pfeifferj force-pushed the feat/notification-channels branch from 5471ee5 to fc47d09 Compare July 26, 2026 13:27

		* - page_status: legacy per-page status keyed by [host, page]. Used
		* for shell navigation icons.

		*
		* The details are all still experimental and subject to change.

		#notify_channel(host: string, page: string, channel: string, id: string,
		notification: ChannelNotification \| null) {


		export const page_status = new PageStatus();

		export const CHANNELS_KEY = "cockpit:notification-channels";

Uh oh!

Conversation

pfeifferj commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Venefilyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Venefilyn commented Jun 9, 2026

Uh oh!

mvollmer commented Jun 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Venefilyn Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvollmer commented Jun 10, 2026

Uh oh!

mvollmer commented Jun 10, 2026

Uh oh!

pfeifferj commented Jun 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Venefilyn commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pfeifferj commented May 26, 2026 •

edited

Loading

Venefilyn Jun 16, 2026 •

edited

Loading