Repository navigation
Conversation
|
🤖 Folded into #2761 (shared UI for installed plugins), which now includes this PR's commits unchanged. Closing to keep one app UI dependency. |
dd940d7 to
bd003ea
Compare
|
🤖 Review round 1: Princess Donut, reviewing exact head Verdict: 9/10, no blockers. The slice stands on its own. Scope matches the slice. Compared with the combined #2761 head Works without the other slices. The guide section on Markdown and workspaces is complete by itself. Its example uses only Verified:
Findings (all optional)
|
bd003ea to
2df7cd4
Compare
|
🤖 Conflict-only rebase onto main |
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Three contract issues to address before merging: isolate plugin split persistence, narrow the public workspace props, and preserve unavailable-link feedback for viewerless sessions.
… an update (#3024) ## TL;DR A `buzz://plugin-install` Git link pinned to both a `commit` and a `path`, for a plugin already installed from that same repository and folder, now opens the **Update** review that a plugin's Git `requestInstall` already gets. If the link names another branch or tag than the plugin tracks, the review says so. Before, it hit the Add path's **Already installed** refusal, so there was no reviewed way to update from such a link. Links without both pins, and links to a plugin installed from another source, are still refused as duplicates. ## What changed - When the installed plugin was imported (not bundled), `PluginImport` treats a link with both `commit` and `path` like a bound `requestInstall` request when deciding between update and duplicate (`src/app/PluginImport.tsx`). It reuses the existing same-source check: the installed `updateSource` must be a Git source whose repository matches the fetched preview's source and whose folder matches the candidate's folder, and the candidate must have the installed manifest ID. Native import saves the requested `ref`, and later update checks follow it. So when a link's or plugin request's `ref` differs from the installed Git source's ref, the review adds "Future updates will follow <new> instead of <old>." An empty ref reads as "the repository default". - Confirming a requested update (link or plugin request) re-reads the live catalog and refuses with "The installed plugin changed. Close this review and check again." if the installed revision, enabled state or saved source changed after the review opened, so the review's "stays disabled" line can't go stale. - A requested update of a disabled plugin now says "This plugin will stay disabled.", as the Details **Update** review already does. Native install already keeps the enabled state, and the review offers no enable switch for updates. - Docs: `docs/plugin-architecture.md`, `docs/deep-links.md` and `docs/buzz-help.md` describe the pinned-link update. Protections that still apply: the commit pin (native refuses a ref whose tip differs, and the review re-checks the fetched commit), the folder filter, the access review against the installed manifest, the publisher line and native publisher-continuity refusal, the unsigned-to-signed rollback warning, explicit **Update** confirmation, and preview discard on cancel. Links with only `commit`, only `path`, or neither, and pinned links to a bundled plugin, keep showing **Already installed** / **Open in Buzz**. Not in scope: update discovery, Update-all, managed policy feeds or checkers (#2504, #2621, #637), signature or publisher-trust policy (#2212, #2746), and the compact request dialog (#2625). This PR doesn't touch their hunks. #2983 and #2762 also edit `tests/browser/plugin-import.spec.mjs`; this PR leaves that file unchanged. ## Supported operation On a Buzz desktop build that includes this change, open `buzz://plugin-install?repository=<repo>&ref=<ref>&path=<folder>&commit=<40-or-64-hex>` for a plugin whose installed copy came from a Git import of that same repository and folder. The review shows **Update <name>**, with access changes listed against the installed version, and states any change to the branch or tag later updates follow. Nothing changes until you choose **Update**. Native import still refuses the update if the ref's tip isn't the pinned commit, or if the publisher changed or a signed plugin became unsigned. ## Tests New `src/app/PluginImport.test.tsx` cases, all with fictional `example.com` fixtures: - A pinned link to the installed source reviews as **Update**, installs the pinned folder, and doesn't call enable. This is the regression test for the original failure. - A disabled plugin stays disabled and the review says so. - A pinned link with another ref than the installed plugin tracks (a named ref, or the repository default) reviews as **Update**, says which ref future updates will follow, and installs on **Update**. A same-ref link shows no such line. - A link without both pins (none, path only, commit only), or a pinned link to a bundled plugin, still shows **Already installed** with **Open in Buzz**. - An installed source in another repository or folder, a local folder, or an unknown source is refused with the source-mismatch message and **Open in Buzz**. - When only another plugin ID is installed from that source, the link offers a new install. - A fetched commit that differs from the pin offers no **Update**. - Cancel discards the preview without installing. - If the installed revision, enabled state or source changes after the review opens, **Update** is refused, for both a link and a plugin's `requestInstall`. The request and link harnesses now return a ready catalog from `plugins.snapshot()`, which the new confirm-time check reads. Checked on a Linux box at `37cf73e1`: `biome check` on both files, `pnpm typecheck`, and `vitest run src/app/PluginImport.test.tsx` (100 passed). After the first review fixes, the same three checks passed locally (102 tests), and the bundled-plugin test fails on the earlier predicate. The later enabled-state drift case (two lines plus one test input) passed biome locally; host load refused a local test run, so PR CI covers it. With only the `requested` line reverted, 6 of the 8 `pinned link` tests fail. Against `main`'s `PluginImport.tsx`, the same 6 fail and the other 94 pass. The two that pass either way are the existing pin-retry test and the other-plugin-ID new-install case, which should behave the same before and after. After review feedback, the tracked-ref disclosure was verified on a Blox box at `e913727f`: `biome check`, `pnpm typecheck` and `vitest run src/app/PluginImport.test.tsx` (103 passed). With the review line disabled, the new tracked-ref test fails. The before/after screenshots predate this line and show a same-ref update. ### Before / after These screenshots come from a temporary Chromium journey using the existing `nativeImports` IPC fixture and fictional `example` data; the journey is not committed. It installs `example.linked` from `https://github.com/example/plugins` at `plugins/linked`, then delivers the same pinned link again with a newer candidate. | | Light | Dark | |---|---|---| | Before (`main`) |  |  | | After |  |  | ## Remaining gaps - No native (Tauri) run: the Playwright screenshots use the existing IPC fixture. Native import, publisher continuity and the deep-link handoff were not changed here. - No human has tried it in the app yet, so `buzz-review-completed` is not set. ## Initiative context Status: https://g2.sqprod.co/apps/kylez-buzz-improvements-status (buzz-improvements T027). Fixes NEXUS-4327 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Kyle Zemek <kylez@squareup.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
d5e4786 to
7082669
Compare
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Provide the shared panel UI field in new mainline fixtures without changing their behavior or assertions. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Load the Channels plugin and existing host conversation features required by the installed-consumer proofs, instead of transforming every bundled plugin during navigation. Preserve all assertions, timeouts and retries. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Namespace injected split keys separately from built-in layouts. Verify host and plugin widths remain independent through save, remount and reset. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Expose documented workspace and tab controls without Channels navigation, drag or menu plumbing. Verify the generated author package and injected component reject host-only props. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Keep unavailable-link feedback for entity, agent-card and removed-repository links when the session has no viewer. Use the session partition for scoped targets. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Use the shared code scroller in messages, documents and transcripts. Preserve authored whitespace, keep copy actions pinned, and show half a line at the height cap. Match the frame and copy-action backing to the inline snippet fill. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
7082669 to
9e00e26
Compare
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Four issues to address: required browser-test expectations, table wrapping, plugin table bounds, and keyboard access to capped code.
| overflow-x: auto; | ||
| /* Leave half a line below the cap as a cue that more code follows. */ | ||
| max-height: calc(var(--space-3) + 16.5lh); | ||
| overflow: auto; |
There was a problem hiding this comment.
🤖 [P1] Update the existing browser contract for wrapped code
This intentional pre-wrap change leaves tests/browser/messages.spec.mjs:555 expecting white-space: pre. Exact-head Chromium and WebKit CI fail with Expected pre / Received pre-wrap, and CI required is failing (https://github.com/block/buzz-app/actions/runs/43503648849/job/132156198351). Update that assertion and its horizontal-overflow intent to verify the intended wrapping/cap behavior.
| /* Inline snippets keep their filled, rounded ends on every wrapped fragment. */ | ||
| .inline code, | ||
| .document :not(pre) > code { | ||
| white-space: pre-wrap; |
There was a problem hiding this comment.
🤖 [P2] Preserve nonwrapping inline code inside tables
This selector also matches td > code, overriding the inherited table white-space: pre contract. Path tables now wrap code cells instead of scrolling horizontally. The existing WebKit messages.spec.mjs:861 scrollbar check fails at this head and passes when .document table code { white-space: pre; overflow-wrap: normal; } is added as a control. Exclude table descendants or restore their rule, and retain a table-cell regression alongside the new paragraph-code checks. CI evidence: https://github.com/block/buzz-app/actions/runs/43503648849/job/132156198556.
| }: MarkdownProps & { navigation?: Navigation | undefined }) { | ||
| const plain = useMemo( | ||
| () => | ||
| !guardDocument(content, DOCUMENT_PROFILE, { chars: MAX_MARKDOWN_LENGTH }), |
There was a problem hiding this comment.
🤖 [P2] Apply the shared document table-cell budget
Passing only chars skips guardDocument's expanded-cell budget, contrary to the documented Shared notes bounds. A trusted reader displaying untrusted Markdown renders sparse tables that Notes/Canvas reject: the real component emitted 12,200 cells from 1,964 characters, while the shared 10,000-cell bound rejected the same input. A public-component fallback assertion fails at this head. Pass cells: DOCUMENT_RENDER_CELLS as NotesPage and CanvasView do, and add sparse-table fallback coverage. This establishes missing output bounds; browser freeze thresholds were not measured.
| display: block; | ||
| overflow-x: auto; | ||
| /* Leave half a line below the cap as a cue that more code follows. */ | ||
| max-height: calc(var(--space-3) + 16.5lh); |
There was a problem hiding this comment.
🤖 [P2] Make the capped code scrollport keyboard reachable
The new cap hides long code behind an inner code scrollport with no tabindex. In the actual installed-document app, WebKit skips it with Tab; focus() fails and End leaves scrollTop=0 with clientHeight=276 and scrollHeight=4824. Adding only tabindex=0 in the test makes Tab/End work (scrollTop=3925); Chromium's automatic scroller focus already works. Explicitly make this shared scrollport keyboard reachable while preserving the sibling copy action. Add natural Tab/keyboard scrolling coverage: existing tests assign scrollTop in JavaScript and miss this defect.




Overview
Category: improvement
User Impact: Installed document plugins can reuse Buzz’s safe Markdown reader and tabbed workspaces, with consistent wrapping code in messages and documents.
Problem: Installed plugins cannot import app source, so document reading, links and tabbed workspace behavior otherwise need to be copied.
Solution: Expose the existing Markdown renderer through
conversation.uiand the workspace/split/title owners throughpanels.ui. The shared renderer owns typography and code styles: inline snippets wrap with rounded ends on every fragment; filled fenced blocks wrap and cap at 16½ visible lines, deliberately clipping the bottom line to signal vertical scrolling. Copy controls remain pinned.Plugins keep records, permissions, navigation and selection. The host keeps parsing bounds, URL/image trust rules, focus, tabs and isolated split persistence. Markdown panels default to message-scale headings; document consumers can request document scale. This PR is independent of #2761 and is the base for #2983.
Changes
File changes
docs/buzz-help.md
Describe wrapping inline code and vertically capped fenced blocks.
docs/plugin-authoring-guide.md
Document injected Markdown heading scales and workspace/split/title contracts, keeping records and navigation with the plugin.
src/app/shell/usePanelLauncher.test.tsx
Supply the required shared panel UI in the existing fixture.
src/bundled/channels/ChannelsPage.context.test.tsx
Supply the shared panel UI without changing the host assertions.
src/bundled/channels/ChannelsPage.conversion.test.tsx
Keep channel conversion fixtures compatible with the shared panel contract.
src/bundled/channels/ChannelsPage.solo.test.tsx
Keep Solo channel fixtures compatible with the shared panel contract.
src/bundled/channels/ChannelsPage.tabs.test.tsx
Keep channel-tab fixtures compatible with the shared panel contract.
src/bundled/channels/useChannelPanels.test.tsx
Provide shared UI in the existing channel-panel fixtures.
src/features/agents/ActivityTranscript.module.css
Use the shared Markdown code-block cap instead of a separate transcript rule.
src/features/conversation/service.tsx
Expose the bounded host Markdown renderer through the existing conversation UI injection owner.
src/features/markdown/Markdown.module.css
Wrap inline snippets with rounded fragments; wrap and fill fenced code, retain pinned copy actions, and cap tall blocks at a partial bottom line.
src/features/messages/PluginMarkdown.test.tsx
Verify parsing bounds, link/image trust rules and local/host navigation.
src/features/messages/PluginMarkdown.tsx
Reuse the shared Markdown renderer, message/document heading scales and community-aware navigation with bounded fallback source.
src/features/messages/attachment-panes.test.tsx
Supply the shared panel UI in existing attachment-pane fixtures.
src/features/panels/PanelWorkspace.tsx
Export the existing retained-tab workspace types without creating another tab owner.
src/features/panels/service.test.tsx
Check injected workspace identity and lifecycle.
src/features/panels/service.ts
Expose a narrow plugin workspace contract while retaining host-only opening, drag and menu lifetimes.
src/features/panels/usePanelSplit.test.tsx
Verify plugin split preferences remain isolated from built-in layouts.
src/features/panels/usePanelSplit.tsx
Reuse the existing split owner with a separate plugin persistence namespace.
src/plugins/author.test.mjs
Compile an independent consumer of the public contracts and reject host-only workspace options.
src/plugins/author.ts
Export the supported public UI contracts as types.
tests/browser/document-plugin.mjs
Supply the import-free installed module used to verify Markdown, retained document tabs and split persistence.
tests/browser/markdown-documents.fixture.tsx
Render real document/message hosts with long inline snippets and indented, unbroken fenced lines.
tests/browser/markdown-documents.spec.mjs
Check both engines across themes, widths and text scales, including the partial-line cap, scrolling, source preservation and copy actions.
tests/browser/plugin-import.spec.mjs
Exercise installed Markdown, safe links, retained tabs, keyboard focus and persisted splits.
tests/fixtures/conversation.tsx
Keep the conversation proof limited to required host features while retaining its existing assertions and timeouts.
Reproduction steps
docs/plugin-authoring-guide.mdor the module intests/browser/document-plugin.mjs.Validation and limits
Mandatory publication hooks pass on the full editor stack at
a88e640f1d: TypeScript and related unit tests, design type checks/guards, and publication policy. The parent9e00e26fd8was pushed in the same atomic update; its dedicated PR CI remains authoritative for that parent-only state. The Clippy lane correctly skipped because this stack changes no Rust inputs.The full shared-Markdown browser file passes 3 cases with 1 existing WebKit screenshot-only skip across Chromium/WebKit, covering both themes, three widths and normal/enlarged text. It checks wrapping, the partial bottom line, scrolling to the end, literal source text and the existing copy interactions. Three filled-code capture journeys pass, including frame/snippet color parity and wheel scrolling in the installed reader/editor. The full installed-plugin file last passed 40/40 before the color-only refinement; fresh PR CI covers the combined update.
Captures use the real built app components with disposable documents and modeled relay/native inventory. They establish browser behavior, not packaged-native, live-relay or operating-system clipboard acceptance. The editor clipboard journey uses page-local ClipboardEvent/DataTransfer. The most recent repository-wide unit run had a dialog Cancel-focus assertion that also failed on its untouched base; no assertion or timeout was changed to hide it. Required review approvals and fresh CI remain pending.
Screenshots
Headless browser fixtures with disposable content and modeled transport. The same shared code styles apply to both surfaces. No plugin typography overrides are used.