notifications (5/5): notifications UI by illegalprime · Pull Request #456 · block/proto-fleet

illegalprime · 2026-06-15T15:53:52Z

Stack 5/5 — base: `eden/notifications-4-server`

Adds the notifications UI.

Notifications feature: channels, rules, silences, history — API hooks, Zustand store, modals, tables
Dashboard active-notifications card
Router, nav, route-prefetch, and permission-catalog wiring, gated on `notification:read`

Stack

proto + generated code
notification_history keyset listing
notification:read / notification:manage permissions
Grafana-backed service, handlers, server wiring
this PR — notifications UI

🤖 Generated with Claude Code

github-actions · 2026-06-15T15:56:47Z

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

Reviewed pull request diff only (75185f66396fe7581a62d34f4869e4be93b79392...466fdd686a2d59cc95337b9371d54768806868e1, exact PR three-dot diff)

Model: gpt-5.5

💡 Click "edited" above to see previous reviews for this PR.

Review Summary

Overall Risk: MEDIUM

Findings

[MEDIUM] Dashboard Active Notifications Can Be Stale Or Incomplete

Category: Reliability
Location: client/src/protoFleet/features/notifications/components/HistoryTable.tsx:74
Description: The new dashboard card derives “Active notifications” from the first paginated history page only, and HistoryTable refreshes that history only once on mount. The code even notes that alerts older than the loaded window will not surface.
Impact: Operators can see “No active notifications” while an alert is still firing if that firing row falls outside the first 50 history entries. The card can also remain stale while the dashboard stays open, missing newly firing alerts or showing alerts that have since resolved.
Recommendation: Back the active card with a dedicated active/firing notifications API or server-side query. At minimum, poll while mounted and avoid deriving active state from a bounded history page.

Notes

The scoped diff is client-only; I did not see backend, migration, plugin, infrastructure, or protobuf source changes in .git/codex-review.diff. I did not find evidence of XSS, secret persistence in browser storage, pool hijacking, command injection, or new auth bypasses in the changed client code.

_{Generated by Codex Security Review |

Triggered by: @illegalprime |

Review workflow run}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2ace81cb65

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-06-15T16:13:17Z

+        <Button
+          variant={variants.secondary}
+          size={sizes.compact}
+          text="Add channel"
+          onClick={() => setShowAddModal(true)}


Gate notification mutations on manage permission

For users who have notification:read but not notification:manage, the Notifications settings entry is visible, but this button and the row edit/test/delete actions still call RPCs that server/internal/handlers/middleware/rpc_permissions.go maps to notification:manage. Those read-only users will see mutation controls that only produce permission-denied errors, so the mutation buttons/actions should be hidden or disabled behind useHasPermission("notification:manage").

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-06-15T16:13:17Z

+      group_id: null,
+      site_id: null,
+      device_ids: [],


Preserve non-rule silence targets when editing

When the backend returns an existing group/site/device-scoped silence, the edit modal initializes scope to that kind, but saving always sends group_id, site_id, and device_ids as empty. The server's validateSilenceScope requires those targets for non-rule scopes, so any edit to such a silence fails with an invalid-argument error instead of preserving the current target; either preserve the existing scope fields or hide editing for unsupported scope kinds.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-06-15T16:13:17Z

+    <HistoryTable
+      activeOnly
+      noDataElement={<div className="py-6 text-center text-text-primary-50">No active notifications.</div>}


Don't derive active alerts from one history page

The dashboard card labels this as active notifications, but activeOnly uses refreshHistory(), which fetches only the first 50 history rows, and then hides pagination. In fleets with more than 50 newer notification events, a still-firing alert whose latest firing row is older than that first page will disappear from the active card and can incorrectly show “No active notifications”; the active view needs a complete/current active-alert source or must keep paging until it can safely determine active state.

Useful? React with 👍 / 👎.

…ation:manage Two client-side hardening fixes from review: - Channel destination secrets (webhook bearer header, Slack webhook URL, SMTP password) used the default text input, leaving capability secrets shoulder- surfable. Switch them to type=password (Input already ships a show/hide toggle). - The Rules/Channels/Silences sections rendered create/edit/test/delete/pause/ resume/silence controls to any notification:read user, though the server gates every mutation on notification:manage. Gate the add buttons, row actions, and inline-editable channel cells on useHasPermission("notification:manage") so read-only users see the lists without unusable controls. Addresses Codex security review findings (MEDIUM) on #456. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

illegalprime · 2026-06-15T17:39:08Z

Codex findings on this PR:

[MEDIUM] secrets in visible text fields — fixed (commit 76179b2): webhook bearer header, Slack webhook URL, and SMTP password inputs are now type=password (the shared Input ships a show/hide toggle).
[MEDIUM] read-only users shown manage controls — fixed (same commit): add buttons, row actions, and inline-editable channel cells are gated on useHasPermission("notification:manage"), so notification:read users see the lists without unusable controls. (The server already enforced this; this removes the misleading UI.)
[MEDIUM] active card can miss firing alerts — not fixed inline; deriving active alerts from the loaded history page is the wrong source, and the fix needs an authoritative active-alerts read. Tracked in Notifications dashboard: active-alerts card can miss alerts older than the loaded history page #460.

…ation:manage Two client-side hardening fixes from review: - Channel destination secrets (webhook bearer header, Slack webhook URL, SMTP password) used the default text input, leaving capability secrets shoulder- surfable. Switch them to type=password (Input already ships a show/hide toggle). - The Rules/Channels/Silences sections rendered create/edit/test/delete/pause/ resume/silence controls to any notification:read user, though the server gates every mutation on notification:manage. Gate the add buttons, row actions, and inline-editable channel cells on useHasPermission("notification:manage") so read-only users see the lists without unusable controls. Addresses Codex security review findings (MEDIUM) on #456. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Adds the notifications feature (channels, rules, silences, history) with its API hooks, Zustand store, modals, and tables, plus the dashboard active- notifications card. Wires the page into the router, nav, route prefetch, and permission catalog, gated on notification:read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ation:manage Two client-side hardening fixes from review: - Channel destination secrets (webhook bearer header, Slack webhook URL, SMTP password) used the default text input, leaving capability secrets shoulder- surfable. Switch them to type=password (Input already ships a show/hide toggle). - The Rules/Channels/Silences sections rendered create/edit/test/delete/pause/ resume/silence controls to any notification:read user, though the server gates every mutation on notification:manage. Gate the add buttons, row actions, and inline-editable channel cells on useHasPermission("notification:manage") so read-only users see the lists without unusable controls. Addresses Codex security review findings (MEDIUM) on #456. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 466fdd686a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-06-15T17:56:02Z

+  useEffect(() => {
+    void refresh().catch((error) => {


Gate the page before issuing read RPCs

When a user opens /settings/notifications directly without notification:read, the nav item is hidden but the route itself is not guarded, so this effect still calls the notification read RPCs (and HistoryTable does the same) and the server returns permission-denied toasts/empty sections. Match the other permission-gated settings pages by checking useHasPermission("notification:read") and redirecting or avoiding the fetch before mounting the sections.

Useful? React with 👍 / 👎.

illegalprime requested a review from a team as a code owner June 15, 2026 15:53

github-actions Bot added javascript Pull requests that update javascript code client labels Jun 15, 2026

github-actions Bot assigned illegalprime Jun 15, 2026

illegalprime force-pushed the eden/notifications-5-client branch from 2ace81c to d7bef30 Compare June 15, 2026 16:08

illegalprime mentioned this pull request Jun 15, 2026

notifications: preliminary UI work #306

Closed

chatgpt-codex-connector Bot reviewed Jun 15, 2026

View reviewed changes

illegalprime force-pushed the eden/notifications-4-server branch from 6cdd7d2 to 6655a15 Compare June 15, 2026 17:37

illegalprime force-pushed the eden/notifications-5-client branch from d7bef30 to 76179b2 Compare June 15, 2026 17:38

illegalprime mentioned this pull request Jun 15, 2026

Notifications dashboard: active-alerts card can miss alerts older than the loaded history page #460

Open

illegalprime force-pushed the eden/notifications-4-server branch from 6655a15 to 753a5b8 Compare June 15, 2026 17:44

illegalprime force-pushed the eden/notifications-5-client branch from 76179b2 to c069f13 Compare June 15, 2026 17:44

illegalprime and others added 2 commits June 15, 2026 13:47

illegalprime force-pushed the eden/notifications-5-client branch from c069f13 to 466fdd6 Compare June 15, 2026 17:48

chatgpt-codex-connector Bot reviewed Jun 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

notifications (5/5): notifications UI#456

notifications (5/5): notifications UI#456
illegalprime wants to merge 2 commits into
eden/notifications-4-serverfrom
eden/notifications-5-client

illegalprime commented Jun 15, 2026

Uh oh!

github-actions Bot commented Jun 15, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Uh oh!

illegalprime commented Jun 15, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

illegalprime commented Jun 15, 2026

Stack

Uh oh!

github-actions Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔐 Codex Security Review

Review Summary

Findings

[MEDIUM] Dashboard Active Notifications Can Be Stale Or Incomplete

Notes

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

illegalprime commented Jun 15, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Jun 15, 2026 •

edited

Loading