Skip to content

Let plugin pages declare their own icon - #512

Merged
wesbillman merged 4 commits into
block:mainfrom
bostonaholic:bostonaholic--plugin-page-icon
Oct 5, 2026
Merged

wesbillman merged 4 commits into
block:mainfrom
bostonaholic:bostonaholic--plugin-page-icon

Conversation

@bostonaholic

@bostonaholic bostonaholic commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Plugin pages can now declare their own icon. ctx.pages.register accepts an optional icon, a data:image/... URL, and Search Buzz and the primary page navigation show it beside the page title. Today every page outside the host's hardcoded bundledIcons map in presentation.ts gets the generic browser-window icon, so external plugins such as PR Beacon cannot be told apart in Search Buzz.

A missing, invalid, or unloadable icon falls back to the generic icon. Bundled pages keep their host icons.

Design Decisions

  • Data URL only. External plugins ship only manifest.json and a self-contained plugin.js (docs/plugin-architecture.md), the renderer CSP already allows img-src data:, and a data URL makes no network request. Named host icons (a plugin picks from the design-system icon gateway) were the runner-up: they keep design-system control but give plugins no brand mark. Remote URLs and manifest fields were rejected because they add a fetch or a Rust manifest-schema change.
  • Invalid icons warn, not throw. Other optional page fields throw on bad values. An icon is decoration, so a bad value is dropped with one console.warn that names the plugin and page id, and the page still registers and opens.
  • Bundled icons win. When a page's key is in bundledIcons, its declared icon is ignored.
  • Messages is matched by full key. pagePresentation now checks page.key === "buzz.channels/channels" instead of page.id === "channels". Before, any external page with local id channels took the Messages label and icon.
  • FOUNDATION edit. src/features/pages/service.ts gains the icon field and its validation. src/plugins/author.ts already re-exports Page, so it is unchanged. This edit was requested by the human author.

Changes

src/features/pages/service.ts   Page.icon?: string; register() keeps a valid data:image/ URL, drops anything else with a warning
src/app/shell/PageIcon.tsx      new: img element for a declared icon, falls back to the component icon on load error (retries once per new source)
src/app/shell/presentation.ts   adds image to the presentation of non-bundled pages; Messages matched by full key
src/app/shell/SearchChoices.tsx search rows render through PageIcon
src/app/shell/AppShell.tsx      primary nav rows render through PageIcon
docs/plugin-architecture.md     documents icon

Screenshots

All shots come from buzz-app's browser test fixture with synthetic data, with a page registered using PR Beacon's real Owner Owl icon value.

Search Buzz, before and after

Search Buzz filtered to "PR". The fixture and the query are the same in both shots.

Before (main) After (this PR)
Before: PR Beacon shows the generic browser-window icon After: PR Beacon shows its Owner Owl icon

Other icon states

Dark theme Primary page navigation Image fails to load
Search Buzz in dark theme with the Owner Owl beside PR Beacon Sidebar page navigation with the Owner Owl beside a primary PR Beacon page; other plugin pages keep the generic icon A page whose icon is an undecodable PNG data URL falls back to the generic icon

How to Verify

  • pnpm check and pnpm lint: pass (4 info-level Biome notes, all on base).
  • pnpm vitest run: 6344 tests pass. New coverage: icon validation (valid, invalid, case, absent; warning names the page and never echoes the value), PageIcon image, load-error fallback, retry on a new source, and no-image rendering, a declared icon on a Search Buzz row and a primary nav row, and bundled-key precedence including an external page with id channels.
  • src/plugins/author.test.mjs: the generated @buzz/author package accepts icon: string and rejects icon: 1.
  • node --test tests/integration/*.test.mjs: 182/182 pass and cargo test -p buzzodz-plugins passes, both run with Hermit active and after cargo fetch --locked. Without Hermit, plugin-cli and plugin-manager-lock fail to spawn cargo, and agent-build-config fails offline on uncached crates.
  • pnpm test:browser:ci: 1084 passed, 5 failed, 1 skipped. The 5 failures (profiles.spec.mjs:524 chromium and webkit, reactions-polish.spec.mjs:6 chromium and webkit, channel-completion.spec.mjs:7 webkit) fail the same way on base e5a7046 when run alone, so they are not caused by this change.
  • Browser fixture with mocked native IPC: a page registered with the PR Beacon owl data URL shows the image in Search Buzz and the primary nav, an https:// icon logs one warning and shows the generic icon, and an undecodable PNG falls back to the generic icon.
  • Not run: the Tauri desktop app. A human test there is still open (see Pre-merge).
  • The results above ran on e210633. Rebased onto main 539bb13 (985c049). feat(ui): Switch shared icons to Tabler #523 moved shared icons to Tabler, which drops the weight prop, so PageIcon now takes strokeWidth and the primary nav passes strokeWidth={2.5}, the same value feat(ui): Switch shared icons to Tabler #523 gave the other nav icons. On 985c049 with Hermit active: pnpm check, pnpm lint, pnpm build, vitest run, node --test tests/integration/*.test.mjs, and cargo test -p buzzodz-plugins pass. Browser tests were not rerun locally.
  • 1d451db (review fix): the icon <img> is no longer draggable. Pressing the icon and moving the pointer a few pixels started a native drag that swallowed the row's click in Chromium and WebKit. PageIcon.test.tsx asserts the attribute; the src/app/shell and src/features/pages tests, Biome, and pnpm typecheck pass.
  • Earlier CI on f6d320c: JavaScript passed. Browser journeys 1/6 failed on tests/browser/channel-tabs.spec.mjs:281 (crowded tab strip), which fails the same way on main since e697aa7 (Use top tabs in the new-tab picker #505).

Merge risk

Two-way door. The change adds an optional field and a render path; no data, storage, manifest, or Rust changes. Plugins that do not set icon render as before. Recovery is a revert of this PR; plugins that set icon then show the generic icon again and keep working, because older hosts already ignore the field.

Pre-merge

  • Human test in the desktop app (just desktop): import the PR Beacon plugin from its companion branch and confirm the owl shows in Search Buzz in light and dark themes, then add buzz-review-completed.

Review notes

  • [security review] LOW: withAcceptedIcon validates icon from one read, then createContributions copies the page with a second read, so a page object with a getter could pass the check and store a different value. Plugins are trusted same-process code and can already render any image element in their own component, so this grants no new capability. Fix by returning a copy holding the checked value.
  • [UX review] Checked in the browser fixture only, not the Tauri desktop app. The owl renders at about 14px in the sidebar; legibility at that size was not zoomed.
  • [verification] Integration tests that build with cargo need Hermit and fetched crates; five browser specs fail on base (listed under How to Verify).
  • [design-review-1] The design's literal types needed conditional spreads under exactOptionalPropertyTypes (done in the implementation). The validation rule now requires the comma, so data:image/png;base64 with no payload is dropped. Non-page search rows render through PageIcon with no image, covered by a test. The D4 rg enumeration and two D2 scoring criteria lacked recorded evidence; neither changes the outcome.
  • [cross-model-notes]

Design round 1

Cross-model disposition

codex (round 1)

  • Adopted as a suggestion (non-blocking): the validation rule accepts a ;-terminated value with no comma. I confirmed this at 6-design.md:40. The outcome is the PRD fallback, not a failure, so it does not block.
  • Adopted as a suggestion (non-blocking): search rows other than pages now render through PageIcon. I confirmed this at SearchResults.tsx:9,152. The file-level "no edit" at 6-design.md:176 is accurate, and the no-image path keeps the current behavior, so it does not block.
  • Adopted as a suggestion (non-blocking): the PR Beacon test cannot detect a wrong port. I confirmed this at 6-design.md:201. A weak test does not reject a correct implementation, so it does not block.
  • Refuted: the claim that animated GIF, APNG, or SVG content breaks scope. 3-prd.md Out of Scope lists animation as work this change does not build. No PRD constraint requires the host to reject animated plugin images, so this is a speculative requirement.
  • Refuted: the claim that the design needs encoded-byte and decoded-pixel caps. docs/plugin-architecture.md:645 and :766 say plugins are trusted same-process code. Plugin JavaScript can already allocate any amount of memory, so an icon cap adds no boundary. The design records "no per-icon cap" on purpose at 6-design.md:77 and :158.
  • Refuted: the claim that the shared image-with-fallback component should be extracted now. The design already defers this at 6-design.md:169 and :230. The reason is the PRD's exclusion of panel launcher changes (3-prd.md, Out of Scope).

agy (round 1)

  • Skipped: agy timed out after 600000 ms. It made no claims.

Cross-model disposition

codex

  • Adopted at non-blocking: the native-portrait test checks colors but not geometry, so the README's shape claim overstates it. I confirmed this at src/index.test.tsx:474 and README.md:292. It matches my own finding, so I report it once.
  • Refuted: none.
  • Unverifiable: none.

agy

  • skip: agy timed out after 600000 ms. The courier's first reply was commentary, not runner output. Its final reply was the runner's skip line, which I accepted.

Tree check: both worktrees are clean after the pass. The HEADs are unchanged (buzz-app e210633, pr-beacon 631829e). No vendor mutation.

References

  • docs/plugin-architecture.md → "Starting contracts" documents icon.
  • Panel launchers' existing image-with-fallback pattern: src/app/shell/PanelLaunchers.tsx (LauncherIcon).

Companion PRs

A Block-internal plugin, PR Beacon, adopts icon in a companion PR. It registers its Owner Owl as a data URL, which is what the screenshots above show.

@bostonaholic

Copy link
Copy Markdown
Contributor Author

@codex review

@bostonaholic
bostonaholic marked this pull request as ready for review October 1, 2026 20:39
@bostonaholic
bostonaholic requested review from a team, comp615 and wesbillman as code owners October 1, 2026 20:39
bostonaholic and others added 3 commits October 2, 2026 10:43
Plugin pages had no way to set their own mark, so every external page
showed the generic browser icon in Search Buzz. Page now takes an
optional icon that holds a data:image/ URL, and search rows render it.

register keeps a matching icon. Any other value is dropped with one
console warning that names the page, and the page still registers,
because an icon is decoration and must not fail plugin activation.
External plugins ship only plugin.js, so data URLs need no new asset
path, network load, or host-asset access.

PageIcon renders the image and falls back to the component icon when
it fails to load. A failed source does not retry; a new source gets
one attempt. Bundled pages keep their host icons.

This edits the FOUNDATION page contract in features/pages/service.ts.

Design: docs/plans/2026-10-01-plugin-page-icon/6-design.md (D1-D5)
Structure: docs/plans/2026-10-01-plugin-page-icon/7-structure.md
Part of slice 1: Search Buzz shows a plugin page's declared icon

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
…es by full key

A primary plugin page with a declared icon now shows it in the shell
page navigation, through the same PageIcon fallback that Search Buzz
uses. Nav keeps the bold weight for component icons only.

pagePresentation matched Messages by local page id, so an external
page with id "channels" took the Messages label and icon and would
also have hidden its declared icon. It now matches the full key
buzz.channels/channels, the same host policy bundledOrder follows.
The bundled Channels page still shows Messages, and bundled order is
unchanged.

pagePresentation now declares its return shape so every branch
carries the optional image field.

Design: docs/plans/2026-10-01-plugin-page-icon/6-design.md (D4, D5)
Structure: docs/plans/2026-10-01-plugin-page-icon/7-structure.md
Part of slice 2: Primary nav shows the icon, and host-owned keys keep
host marks

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
Plugin authors read the page contract in docs/plugin-architecture.md
and type against the generated @buzz/author package. The doc now names
the optional icon field, its data:image/ form, where it renders, the
generic-icon fallback, the registration warning, and that bundled pages
keep their host icons. External plugins ship only plugin.js, so the doc
points authors at an inline bundler import.

The author consumer test now registers a page with a string icon and
expects a type error for a numeric one, so the author.ts re-export
must keep carrying icon as an optional string.

Design: docs/plans/2026-10-01-plugin-page-icon/6-design.md (D9)
Structure: docs/plans/2026-10-01-plugin-page-icon/7-structure.md
Part of slice 3: PR Beacon shows the Owner Owl through the published
author contract

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
@bostonaholic
bostonaholic force-pushed the bostonaholic--plugin-page-icon branch from f6d320c to 985c049 Compare October 2, 2026 16:48

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No blocking findings; one P3 interaction defect inline. Reviewed head 985c049a293f7f146f6c2a12b96bd9cebbfa16f7 against base/merge-base 539bb13578d7c8c5a4d42002112857d66e20e19b. Meets the 9/10 bar for minimalness, elegance, and correctness, with the minor interaction fix noted below: one optional contract field, one shared renderer, and the existing contribution lifecycle.

  • Validation: traced registration → contribution → presentation → search/primary navigation, including invalid values, bundled precedence, and source replacement. Independent Chromium/WebKit component probes verified decoding, error fallback, decorative naming, and the reported pointer behavior. Existing CI is green for this head; broad suites were not duplicated locally.
  • Remaining acceptance: the PR’s human Tauri desktop test is still unchecked. Import the companion plugin into a desktop build of this head and confirm its icon in Search Buzz in light and dark themes. Our browser probes exercised the real icon/navigation components, not the full desktop workflow. No approval or readiness attestation is being given.
  • Optional public-PR hygiene: the Companion PRs section links to a private repository. Prefer a public-safe dependency description/reference, keeping private coordination links in the internal thread.

Comment thread src/app/shell/PageIcon.tsx

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I wanted to get to this but you beat me to it! Thanks!!

An <img> is draggable by default, so pressing a plugin page icon and moving
the pointer a few pixels started a drag and swallowed the row's click in
Chromium and WebKit. The SVG fallback never had this problem.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
@bostonaholic

Copy link
Copy Markdown
Contributor Author

🤖 Re: the review's public-PR hygiene note. The Companion PRs section no longer links to the private repository. It now describes the companion change in plain text: a Block-internal plugin, PR Beacon, adopts icon. The desktop test is still open and is tracked in Pre-merge.

@wesbillman
wesbillman merged commit be00aa0 into block:main Oct 5, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants