feat: publish extensions overhaul - #2098
Conversation
There was a problem hiding this comment.
Pull request overview
This PR overhauls extension publishing in the Web UI by replacing the per-extension publish dialog with a dedicated /publish page and a global drag-and-drop publishing flow backed by a persistent publish queue context.
Changes:
- Add a
/publishpage with a multi-file picker + drop area that immediately enqueues/uploads.vsixpackages and shows progress as an inline card strip. - Introduce an app-wide publish queue context with polling to reflect post-upload outcomes (review verdicts, icon availability).
- Turn the navbar “Publish” control into an exported
PublishButtonthat combines link +pshortcut + drop target.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| webui/test/unit/pages/publish/publish-page.spec.tsx | New unit tests covering /publish page upload/drop/login states and queue visibility. |
| webui/test/unit/context/publish-queue-context.spec.tsx | New unit tests for publish queue behavior, error handling, namespace creation, and polling. |
| webui/test/unit/components/publish/publish-queue-strip.spec.tsx | New unit tests for the inline queue strip rendering and states. |
| webui/test/unit/components/publish/publish-button.spec.tsx | New unit tests for the navbar publish button link/shortcut/drop behavior. |
| webui/src/utils.ts | Add formatFileSize() helper for human-readable byte formatting. |
| webui/src/pages/user/extensions/user-settings-extensions.tsx | Replace old publish dialog action with a link button to /publish. |
| webui/src/pages/user/extensions/publish-extension-dialog.tsx | Remove legacy one-file-at-a-time publish dialog implementation. |
| webui/src/pages/publish/publish-routes.ts | Introduce publish route constants (PublishRoutes.ROOT). |
| webui/src/pages/publish/publish-page.tsx | Implement new publish page UI (drop area, file picker, command-line alternative, inline queue). |
| webui/src/layout/app-layout.tsx | Register the new /publish route. |
| webui/src/index.ts | Export PublishButton for custom deployments/menus. |
| webui/src/extension-registry-service.ts | Make icon access resilient (files?.icon) in getExtensionIcon flow. |
| webui/src/default/menu-content.tsx | Replace old publish shortcut/button with PublishButton; mobile menu links to /publish. |
| webui/src/context/publish-queue-context.tsx | New publish queue provider: enqueue uploads, retry with namespace creation, poll for review/icon settling. |
| webui/src/components/publish/use-publish-drop.ts | New hook for window-level drag detection + drop target props + navigation to /publish. |
| webui/src/components/publish/publish-queue-strip.tsx | New horizontal card strip rendering queue items with status/clear UX and accept “flash”. |
| webui/src/components/publish/publish-button.tsx | New navbar publish button component (link + shortcut + drop target). |
| webui/src/components/extension/use-extension-icon.ts | Avoid crashing when files is missing (files?.icon) in query key. |
| webui/src/components/extension/manage-extension-card.tsx | Add support for iconPending and custom footer content (used by publish queue). |
| webui/src/components/extension/extension-icon.tsx | Add pending prop to keep skeleton visible until icon exists. |
| webui/src/components/extension-card.tsx | Thread through iconPending to ExtensionIcon. |
| webui/src/app-providers.tsx | Add PublishQueueProvider to the app provider stack so queue survives navigation. |
| webui/CHANGELOG.md | Document new publishing page/flow and PublishButton export. |
Suppressed comments (3)
webui/test/unit/context/publish-queue-context.spec.tsx:102
publishExtensionshould resolve to anExtension, not an array. Using[published()]here can cause the publish queue to poll for up to 60s (becausefiles.iconis missing on an array), making this test slow/flaky.
it('creates the namespace when the error comes back as a value instead of a throw', async () => {
const publishExtension = vi
.fn()
.mockResolvedValueOnce({ error: 'Unknown publisher: foo\nUse the CLI to create it' })
.mockResolvedValueOnce([published()]);
const createNamespace = vi.fn().mockResolvedValue({ success: 'ok' });
webui/test/unit/context/publish-queue-context.spec.tsx:116
- Same issue as above:
publishExtensionshould resolve to a singleExtension(or reject withErrorResult), not[Extension]. Returning an array here can trigger the queue’s polling loop and make the test hang/flap.
it('reads the namespace even when the message carries no second line', async () => {
const publishExtension = vi
.fn()
.mockRejectedValueOnce({ error: 'Unknown publisher: foo' })
.mockResolvedValueOnce([published()]);
const createNamespace = vi.fn().mockResolvedValue({ success: 'ok' });
webui/test/unit/context/publish-queue-context.spec.tsx:152
publishExtensionresolves to anExtension, not an array. Returning[published()]here can keep the item inawaitingIconpolling for up to 60s, making the test unnecessarily slow/flaky.
const publishExtension = vi
.fn()
.mockResolvedValueOnce([published()])
.mockReturnValueOnce(new Promise(() => {}));
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const publishExtension = vi | ||
| .fn() | ||
| .mockRejectedValueOnce({ | ||
| error: "Unknown publisher: foo\nUse the 'create-namespace' command to create a namespace." | ||
| }) | ||
| .mockResolvedValueOnce([published()]); | ||
| const createNamespace = vi.fn().mockResolvedValue({ success: 'ok' }); |
| if (!abortController.current.signal.aborted) { | ||
| update(id, { status: 'failed', error: errorMessage(err) }); | ||
| // The card keeps the record, but a rejected publish is worth interrupting for: | ||
| // this is the same dialog every other failed request in the app raises. | ||
| handleError(err as Error); | ||
| } |
There was a problem hiding this comment.
That safe handling should be moved to the handleError function itself, we should make it accept unknown, and assert the type internally.
| }, | ||
| [update, pollUntilSettled, publishOnce, hydrate, handleError] | ||
| ); |
There was a problem hiding this comment.
at the end of the day we can consider service as a constant, if we really wanna do exhaustive deps we should enable the eslint rule, otherwise it's hard to maintain
netomi
left a comment
There was a problem hiding this comment.
Nice feature — reviewed the queue/poll logic and the drop-target wiring in detail. Found one feature-breaking gap and a few real logic bugs in the poll/queue state machine, plus some lower-priority efficiency and duplication notes. All confirmed by reading the code directly (not just tool output). Inline comments below; a couple more that don't anchor to one line:
- No client-side file-size check before upload (a regression from the deleted
publish-extension-dialog.tsx, which used react-dropzone'smaxSizeto reject oversized files instantly).publish()(publish-queue-context.tsx:268) only filters onisVsixFile; an oversized file now fully uploads before the server rejects it. - Every concurrent upload polls independently — each
pollUntilSettledcall hits the full extensions-list endpoint on its own 5s timer, so N concurrent items means N× redundant full-list GETs every tick instead of one shared read. - No upload concurrency cap —
publish()'squeued.forEach(({ id, file }) => upload(id, file))(publish-queue-context.tsx:279) fires every queued file's upload simultaneously with no batching/throttling. - Smaller/lower-priority, not blocking:
statusOf()duplicatesgetExtensionStatus's precedence logic (risk of divergence);errorMessage()diverges fromutils.ts'shandleError(drops the.messagehalf of combined error objects); the hand-rolled drag-depth tracking inuse-publish-drop.tsreimplements whatreact-dropzonealready did — and that package is now an unused dependency left inpackage.json;dismiss()is exported/wired but never called from any UI; the newformatFileSizeutil has no test coverage; the newiconPendingprop onExtensionCardPropsisn't reflected in the existing unreleased CHANGELOG bullet for that interface.
| {loginProviders && !location.pathname.startsWith(UserSettingsRoutes.ROOT) && ( | ||
| <MenuItem component={RouteLink} to={UserSettingsRoutes.EXTENSIONS}> | ||
| {loginProviders && !location.pathname.startsWith(PublishRoutes.ROOT) && ( | ||
| <MenuItem component={RouteLink} to={PublishRoutes.ROOT}> |
There was a problem hiding this comment.
Drag-and-drop publishing doesn't work on mobile at all: usePublishDrop() — the hook that attaches the window-level drag listeners — is only called inside PublishButton (publish-button.tsx). MobileMenuContent renders a plain MenuItem here instead of <PublishButton />, so on any viewport below the lg breakpoint, dragging a .vsix file anywhere falls through to the browser's native handling — exactly the failure PublishButton's own doc comment warns must be avoided ("or publishing by drag and drop has nowhere to land"). Worth rendering PublishButton here too, or at least wiring usePublishDrop() into the mobile shell some other way.
| ))} | ||
| {loginProviders && !location.pathname.startsWith(UserSettingsRoutes.ROOT) && ( | ||
| <MenuItem component={RouteLink} to={UserSettingsRoutes.EXTENSIONS}> | ||
| {loginProviders && !location.pathname.startsWith(PublishRoutes.ROOT) && ( |
There was a problem hiding this comment.
Separately: this condition narrowed from !location.pathname.startsWith(UserSettingsRoutes.ROOT) (suppressed on all /user-settings/* pages) to !location.pathname.startsWith(PublishRoutes.ROOT) (suppressed only on /publish). Since /user-settings/extensions now has its own "Publish extension" entry point per this PR, mobile users visiting that page will see this menu item and that button — the old condition avoided exactly this duplication.
| export const PublishButton: FunctionComponent<PublishButtonProps> = ({ sx, className }) => { | ||
| const navigate = useNavigate(); | ||
| const { dragging, over, dropProps } = usePublishDrop(); | ||
| useShortcut({ key: 'p', label: 'Publish', order: 3, callback: () => navigate(PublishRoutes.ROOT) }); |
There was a problem hiding this comment.
The p shortcut lost its enabled guard here. The previous registration (in menu-content.tsx on main) had enabled: !!loginProviders. PublishButton is exported from webui/src/index.ts specifically for third-party deployments to embed directly ("render this rather than its own button"), so a consumer that mounts <PublishButton /> unconditionally now gets an always-active global p shortcut with no prop to disable it.
| const hydrate = useCallback( | ||
| async (extension: Readonly<Extension>): Promise<Readonly<Extension>> => { | ||
| try { | ||
| return (await readPublished(extension.namespace, extension.name)) ?? extension; |
There was a problem hiding this comment.
When this read fails/isn't found yet (a real eventual-consistency race right after publish), hydrate() falls back to the raw publish response, which per the comment above "carries none of that" — including reviewStatus. statusOf()'s switch (line ~79) defaults an undefined reviewStatus to 'published'. If pollUntilSettled's very first pending() check on this stale extension is already false (e.g. it already has an icon), the poll loop body never runs and nothing ever corrects the status — an extension actually stuck in review can permanently show as "Published".
| */ | ||
| const pollUntilSettled = useCallback( | ||
| async (id: number, initial: Readonly<Extension>) => { | ||
| const started = Date.now(); |
There was a problem hiding this comment.
This single started timestamp is reused for both the 5-minute review-wait budget and the 60-second icon-wait budget in pending() below. If a package sits under_review for more than 60s and then gets approved, the very next pending() check already has elapsed >= ASSET_POLL_TIMEOUT_MS, so the icon-wait branch is false immediately — the loop exits and awaitingIcon: false fires with no icon ever fetched. The two budgets need independent clocks (e.g. reset started once the item leaves under_review).
| status: 'uploading' as const, | ||
| file | ||
| })); | ||
| if (queued.length === 0) { |
There was a problem hiding this comment.
Dropping or selecting only non-.vsix files (or a folder of them) fails completely silently — queued.length === 0 just returns, no toast, no error, no visible effect at all. Worth at least a brief error via handleError or similar so the user knows the drop didn't register rather than wondering if it worked.
| [user, upload] | ||
| ); | ||
|
|
||
| const dismiss = useCallback((id: number) => setItems(current => current.filter(item => item.id !== id)), []); |
There was a problem hiding this comment.
dismiss() only filters the item out of React state — it never calls abortController.current.abort() (the controller is shared by the whole queue and only aborted on provider unmount, i.e. app close, per the useEffect above). So dismissing a stuck or unwanted in-flight upload/poll doesn't actually cancel it server-side; it just hides the card while the request keeps running, and there's no way to cancel one item without cancelling every other concurrent upload too.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jordi Gómez Hidalgo <31970428+gnugomez@users.noreply.github.com>
Replaces the one-file-at-a-time publish dialog with a /publish page and a drag-and-drop flow. Dragging a file anywhere turns the navbar's Publish button into a drop area; everything dropped on it is uploaded straight away, no confirmation. The queue lives in a context, so it survives navigation, and renders as a line of extension cards that poll until each package settles.
The button is exported as PublishButton — it carries the link, the p shortcut and the drop target together, so a deployment with its own menu content keeps all three.
Screen.Recording.2026-08-25.at.13.07.46.mov