Skip to content

perf(app): activate plugin styles only while mounted - #2002

Merged
ymichael merged 1 commit into
mainfrom
bb/pr-3-on-demand-plugin-css-thr_zbbse558ev
Aug 20, 2026
Merged

perf(app): activate plugin styles only while mounted#2002
ymichael merged 1 commit into
mainfrom
bb/pr-3-on-demand-plugin-css-thr_zbbse558ev

Conversation

@ymichael

Copy link
Copy Markdown
Collaborator

What was wrong

Frontend activation installed every compatible plugin's generated app.css as a permanent stylesheet, even when the plugin rendered nothing on the current route. Seven installed frontend bundles therefore stayed active on representative pages and WebKit kept all seven rule sets in every document-wide style recalculation. The activation order also published CSS after content-script mount and slot registration, so a cold plugin deep link could render plugin DOM before its CSS URL was known; panel header contributions implicitly depended on a body co-mount for styling.

What changed

  • Added a reference-counted plugin CSS manager. Inactive bundles may issue a low-priority style preload and remove it when it settles; a mounted slot, portal owner, header accessory, or content-script generation retains one real stylesheet; the final release removes it.
  • Made live CSS replacement transactional: the previous active sheet remains until the replacement loads, and a failed replacement removes only the candidate sheet. Registration replacement keeps the old UI consumer alive through the handoff.
  • Published each generation's CSS URL synchronously before content scripts mount and before slot-store publication can render plugin code. Content scripts retain CSS through their complete mount/dispose lifetime.
  • Gave navPanel.headerContent independent CSS ownership, so split/header-only rendering does not rely on a panel body being mounted.
  • Documented the compatibility contract in the SDK, bundled plugin-authoring skill, and content-script example: frontend app.css follows UI/content-script lifetime; app-wide selectable palette CSS uses bb.themes; styling existing app-shell DOM belongs in a cleanup-safe content script.
  • Added integration coverage for a cold deep-link registration, content-script mount and cleanup, header-only content, concurrent mounts and a portal, route unmount, and the real generation reload/store/React path. Reload tests exercise both successful and failed stylesheet replacement. A theme test proves the independent bb.themes application style survives with zero plugin UI mounts.

There is no server/host-daemon wire change, public plugin API addition, CLI change, or HOST_DAEMON_PROTOCOL_VERSION bump.

Compatibility audit

I audited repository plugin sources and the locally installed/source marketplace examples, excluding generated bundles and dependencies:

  • The content-script reference intentionally uses authored global .content-script-example-active CSS on app-shell editables; it already owns the matching class/listeners in a content-script disposer, and now receives a generation-long CSS lease.
  • The composer-customization reference uses namespaced authored classes in mounted slot/composer UI. The built-in Tasks CSS is the normal generated Tailwind entry. Neither assumes an always-on app-shell stylesheet.
  • The local Outline source imports authored CSS whose selectors are consistently namespaced under .outline-* and whose consumers live in its nav panel.
  • The installed Usage marketplace source at 1a2a96f73dd95363d4d6d2f25dc145cff8859074 has no authored CSS import or app-shell DOM styling, but it does register headerContent; that case is covered by independent header ownership.
  • Built-in Docs and Tasks also create plugin-namespaced editor <style> elements programmatically after editor use. Those tags are not generated frontend app.css, so this lifecycle deliberately does not manage them. Their existing persistence is called out as a remaining follow-up risk below.
  • No audited plugin manifest currently contributes bb.themes; the supported palette path is nevertheless independent (#bb-app-theme) and has regression coverage proving plugin UI teardown does not touch it.

I found no audited always-on global app.css assumption beyond the content-script reference, which conforms to the new documented ownership model.

How you verified

Automated validation

  • pnpm exec turbo run typecheck --filter=@bb/app — passed (3/3 tasks).
  • pnpm exec turbo run test --filter=@bb/app --force — passed: 398 files, 3,045 tests; 3 skipped.
  • pnpm exec turbo run build --filter=@get-bb/plugin-sdk --filter=@bb/app --filter=@bb/server --filter=@bb/host-daemon --concurrency=2 — passed (8/8 tasks) for both baseline and branch production builds.
  • pnpm exec turbo run test typecheck build --filter=@get-bb/plugin-sdk --filter=@bb/plugin-build --force — passed (10/10 tasks); SDK 13 files/105 tests, plugin-build 7 files/41 tests with 1 skipped. The two printed private-workspace-import errors are expected assertions from rejection tests.
  • git diff --check origin/main...HEAD — passed.

The new ordering/lifecycle tests fail against the old always-on/late-publication behavior and pass with this change.

iOS Simulator Safari profile

I profiled this branch independently; no numbers were reused from another thread.

  • Baseline: latest origin/main a3a3a8aa3a5bf9fdbd554bebdb0be429e066b664.
  • After: 6b95eeb369730ce688068b94486dda36d3c22923.
  • Host: MacBook Pro Mac16,6, Apple M4 Max (16 cores), 48 GB RAM, macOS 26.5.1 (25F80), Xcode 26.6 (17F113).
  • Client: dedicated iPhone 16 Pro Simulator, iOS 18.4 / build 22E238, Mobile Safari 18.4. The simulator and WebDriver were deleted after profiling.
  • Builds: identical production Turbo command listed above; server bound to loopback with telemetry disabled.
  • Data: the same restored SQLite image for each revision: 519 threads, 3 projects, 638,596 events. Seven enabled frontend CSS bundles were present (custom-instructions, inline-vis, outline, provider-retry, secrets, side-chat, tasks). The normal route displayed a median 15 sidebar thread links (15–15 before, 15–17 after); the Tasks route displayed 14. Median DOM sizes were 1,706 → 1,701 elements on the normal route and 703 → 698 on Tasks.
  • Routes: a representative normal thread and the cold deep link /plugins/tasks/tasks.
  • Runs: 5 cold plus 5 warm per route per revision (20 navigations/revision). Every cold run deleted its WebDriver session, shut down and erased the simulator, rebooted it, and created a fresh Safari session. Warm runs retained the route's session/cache.
  • Forced recalculation: after route readiness plus 250 ms, each run invalidated a custom property on document.documentElement and synchronously read computed style on the final DOM element 9 times (45 samples/cell, 180 samples/revision). Reported values are the median of each run's 9-sample median, with the range across the 5 run medians.
  • First render/FOUC: an early head script observed the first thread composer or plugin panel DOM mutation and sampled the plugin root/link immediately plus the next 5 animation frames.
Route / cache Active plugin sheets, before → after Total sheets, before → after Forced recalc ms, before → after First render ms, before → after
Normal thread / cold 7 (7–7) → 1 (1–1) 13 (13–13) → 7 (7–7) 49 (47–56) → 48 (47–50) 905 (862–957) → 1,284 (1,052–2,840)
Normal thread / warm 7 (7–7) → 1 (1–1) 13 (13–13) → 7 (7–7) 50 (48–54) → 48 (39–49) 523 (453–639) → 556 (494–633)
Tasks plugin / cold 7 (7–7) → 1 (1–1) 12 (12–12) → 6 (6–6) 20 (19–22) → 14 (14–16) 715 (621–818) → 650 (575–866)
Tasks plugin / warm 7 (7–7) → 1 (1–1) 12 (12–12) → 6 (6–6) 20 (18–22) → 16 (14–17) 478 (452–517) → 446 (433–475)

The remaining normal-route sheet is provider-retry, whose registered composer banner is a live mounted consumer; the Tasks route retains only tasks. Inactive preload count after the 250 ms settle was median 0; one branch cold normal sample still had 3 in-flight low-priority preloads, which the load/error handlers remove when they settle.

The baseline exposed the ordering bug in 1/5 cold Tasks runs: the first plugin DOM mutation had a Tasks root but no Tasks CSS link, and the first animation-frame sample had an unloaded sheet. After the change, 0/10 cold+warm Tasks runs showed a plugin root without its link; the Tasks sheet was already present and link.sheet was ready at the first DOM mutation and all 5 sampled animation frames in every run.

The forced-recalculation result is strongest on the plugin route (30% lower cold median, 20% lower warm median) and the active-sheet count falls by 6 on both routes. First-render timings are noisy: Tasks improved in both cache modes, while the normal route's after run was slower, including one 2.84 s cold outlier. This change does not claim a normal-route first-render win; the measured result is stylesheet lifetime/style-recalculation reduction plus elimination of the observed cold deep-link CSS ordering gap.

Remaining risks

  • Programmatically injected, plugin-namespaced editor style tags in Docs and Tasks remain outside this generated-app.css lifecycle and persist after first editor use. Converting those to explicit owner lifetimes would be a separate change.
  • A mounted slot whose component returns null is still a live CSS consumer (for example, the provider-retry banner). Finer-grained conditional ownership would require a plugin/host contract change and is not attempted here.
  • WebKit timing remains noisy, as the first-render ranges show; the sheet counts and FOUC ordering observations were deterministic after the change.

Fixes: N/A — no linked issue.

AGENT GENERATED: by GPT-5

@bb-slop-cop

bb-slop-cop Bot commented Aug 20, 2026

Copy link
Copy Markdown

🚨 SLOP COP 🚨 · review

I am the Slop Cop. I am reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior.

// Publish the URL before either non-React scripts mount or slot-store
// notifications can render plugin code. Inactive plugins only preload;
// an already-mounted generation starts a safe side-by-side replacement.
deps.applyCss(pluginId, candidate.bundle.cssUrl);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚨 slopcop/review — Keep the old CSS until the UI generation changes

This call publishes the candidate URL while the old registrations stay mounted. A loaded candidate sheet can replace the old sheet during a slow content-script mount. A null URL removes the old sheet at once. This can show the old UI with new CSS or no CSS for the ten-second timeout.

Stage the candidate sheet without activation. Commit the sheet and registrations together after the script setup succeeds.

record.url !== url ||
record.consumers === 0
) {
link.remove();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚨 slopcop/review — Clear the superseded pending link

This branch removes a stale link but can leave record.pendingStylesheet on that removed element. I reproduced this with a loaded v1, a pending v2, and a return to v1. After the v2 load event, a later v2 request added no link. The URL check treated the removed element as active.

Clear pendingStylesheet when it still equals this link. Add a regression test for the v1, v2, v1, v2 sequence.

instanceId,
onCrash,
}: PluginSlotMountProps) {
usePluginCss(pluginId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚨 slopcop/review — Cover the provider icon slot

This hook covers normal plugin mounts. The experimental_providerIcon path renders directly through provider-icon.ts. It does not keep the stylesheet active or add a plugin scope root. An icon-only frontend can render while its imported CSS stays inactive.

Use a shared plugin render wrapper. It should own the CSS lease, scope attributes, context, and error boundary.

@bb-slop-cop bb-slop-cop Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚨 SLOP COP 🚨 · review

Plain English summary

This PR stops every plugin stylesheet from affecting every page. It keeps a sheet while that plugin shows UI or runs a content script. This should reduce WebKit style work.

Findings

  1. High: Candidate CSS can replace the old CSS while the old UI still renders. A slow content-script setup can keep this mismatch visible for ten seconds.
  2. Medium: A superseded stylesheet load can leave a stale pending element. A later request for that URL then adds no stylesheet.
  3. Medium: The provider icon slot bypasses the new CSS owner. An icon-only frontend can render without its imported CSS.

I left an inline comment for each finding.

Security and architecture

The security review found no issue. The server creates same-origin CSS URLs, and this change adds no new data or message boundary.

The codebase has no duplicate reference-counted stylesheet manager. The new plugin-css.ts module is the correct common location.

PluginSlotMount is the existing common render wrapper. The provider icon path should use that common ownership model.

Verification

  • The app type check passed.
  • The focused changed suites passed 77 tests.
  • The full app run passed 3,043 tests and failed two unrelated timing tests under heavy parallel load.
  • Both unrelated files passed all 16 tests in an isolated rerun.
  • The browser test observed active plugin sheets change from zero, to one, and back to zero after navigation.
  • GitHub CI passes at this head SHA.

@ymichael
ymichael force-pushed the bb/pr-3-on-demand-plugin-css-thr_zbbse558ev branch 3 times, most recently from 0a333ce to 4cb4094 Compare August 20, 2026 18:15
@ymichael
ymichael force-pushed the bb/pr-3-on-demand-plugin-css-thr_zbbse558ev branch from 4cb4094 to 42effc0 Compare August 20, 2026 18:26
@ymichael
ymichael merged commit 55f86b6 into main Aug 20, 2026
13 checks passed
@ymichael
ymichael deleted the bb/pr-3-on-demand-plugin-css-thr_zbbse558ev branch August 20, 2026 18:42
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.

1 participant