fix(open-file): ask the host for a pending file once the listener is subscribed

[rahulkatiyar19955]

OpenFileListener now tells the host it is listening, and the host holds the file until it does.

The race

The ext_visualizer host streams open-file-chunk events as soon as createWebviewPanel resolves. This component subscribes to those events from a useEffect — and ExtensionRoot does not render it until two bridge round-trips have resolved:

if (orgLookup == undefined) return <></>;   // session.getOrgContext
if (deepLinks == undefined) return <></>;   // zenoh.pendingSource
// ...only then: <OpenFileListener bridge={bridge} />

Measured on a real open, that gate clears about 2.4 seconds after the panel is created. Every chunk sent in that window arrives with no subscriber and is dropped. OpenFileChunkAssembler only yields a File once all totalChunks have landed, so losing the head of the stream strands it permanently — no file opens, no error, nothing in the UI.

From the host log for one such open, all three of these completed successfully while the user saw an empty Visualizer:

15:07:32.462  opening visualizer panel      ← streaming starts here
15:07:33.332  config.getAll                 ← webview's first request, 870ms later
15:07:34.811  zenoh.pendingSource           ← the gate above, 2.3s later
15:07:38.839  streamed file to panel {"bytes":726849593}

The fix

Invert the direction. After subscribing, the listener calls a new openFile.pending bridge method; the host answers immediately and only then streams whatever it was holding. There is no window in which a chunk can be sent to a subscriber that does not exist yet.

This mirrors the pattern already used for zenoh.pendingSource — the webview pulls a one-shot value at mount rather than the host pushing it blind — so it fits the existing bridge design rather than adding a new one.

bridge.request is called after bridge.onEvent, and the ordering is asserted in the tests, because reversing those two lines silently reintroduces exactly the bug this removes. That assertion was verified by reversing them and watching it fail.

A host that does not implement the method rejects the request; the listener logs at debug and carries on, since drag-and-drop opening never depended on this handshake.

Tests

extension/src/OpenFileListener.test.tsx — 5 passing, two of them new:

• the handshake is sent exactly once, with the right method, and only after the chunk subscription exists
• a rejecting host leaves the listener working

The fake bridge in that file gains a request mock, so it now covers Pick<BridgeClient, "onEvent" | "request">.

Pairs with

bringup-labs/ext_visualizer#6, which adds the host half (openFile.pending) and stops streaming on panel creation. Neither half is useful alone: without this PR the host has nobody to answer, and without that one nothing sends the file.

[rahulkatiyar19955]

Handshake failure is logged at debug and always blamed on an old host.

A timeout or a transport failure lands in this same catch. If the paired host is running but wedged — or the paced outbox is saturated — BridgeClient rejects after its 15s timeout, this prints at console.debug (filtered out of the production webview console), and the message asserts a cause that was never checked. The user right-clicked a .mcap, sees an empty Visualizer, and there is nothing anywhere saying why. That is the same silent failure this PR exists to remove, moved one layer up.

Suggested change

	bridge.request(VIS_BRIDGE.openFilePending, {}).catch((err: unknown) => {
	// An older host does not know the method, and has nothing waiting either.
	console.debug("[open-file] host has no pending-file handshake", err);
	});
	bridge.request(VIS_BRIDGE.openFilePending, {}).catch((err: unknown) => {
	// An older host does not know the method and has nothing waiting either, so
	// this is harmless there. A timeout or a transport failure lands here too,
	// and that means a file the user asked to open never arrives - the exact
	// silence this handshake exists to end - so warn rather than bury it at
	// debug behind a cause we have not actually checked.
	console.warn(
	"[open-file] pending-file handshake failed - a file opened from the host will not appear " +
	"(harmless on an older host, which has nothing waiting)",
	err,
	);
	});

[rahulkatiyar19955]

createMockTransport was not given a case for this method. — extension/src/bridge/BridgeClient.ts:244

Every other VIS_BRIDGE key has a case in that switch. openFile.pending falls through to default: and answers { ok: false, error: "Unknown bridge method: openFile.pending" }, so yarn extension:serve logs a handshake failure on every reload for a transport that simply has nothing pending — and with the log level raised (see the other comment) that becomes a warning on every reload.

Suggested addition, right after the themeGet case:

      case VIS_BRIDGE.openFilePending:
        // Browser dev mode has no host process and so never has a file waiting.
        // Answering successfully says exactly that - a rejection here would look
        // like a broken handshake rather than an empty one.
        return { type: "response", id, ok: true, data: {} };

[rahulkatiyar19955]

subscribedAtRequest() stops measuring anything as soon as a test passes requestImpl.

subscribedWhenAsked is only ever assigned inside the default mock. Supply a custom requestImpl and the flag stays false forever — so expect(subscribedAtRequest()).toBe(true) fails even when the component did subscribe first, and expect(subscribedAtRequest()).toBe(false) passes vacuously and reads as proof the subscription was missing. The subscribe-before-request ordering is the whole point of this helper.

Recording it in a wrapper that always runs fixes it:

Suggested change

	const request =
	requestImpl ??
	jest.fn(async () => {
	subscribedWhenAsked = listeners.has(OPEN_FILE_CHUNK_EVENT);
	return {};
	});
	// The ordering is recorded around every implementation, custom ones included:
	// reading it off the default mock alone would silently report "not subscribed"
	// for any test that supplies its own `requestImpl`.
	const request = jest.fn(async (...args: unknown[]) => {
	subscribedWhenAsked = listeners.has(OPEN_FILE_CHUNK_EVENT);
	return await (requestImpl?.(...args) ?? {});
	});

Note this also makes the cast on line 43 unnecessary — @typescript-eslint/no-unnecessary-type-assertion fires under lint:ci, so that line needs to become plain request,.

[rahulkatiyar19955]

The fake unsubscribe never detaches, so no test can catch a listener that outlives unmount.

unsubscribe is a bare jest.fn(); nothing is removed from listeners. Delete the unsubscribe() call from the component's cleanup and every behavioural test still passes — only expect(unsubscribe).toHaveBeenCalledTimes(1) fails, and only because the function was called, not because anything detached. emit(...) after root.unmount() still reaches the handler, and listeners.has(OPEN_FILE_CHUNK_EVENT) stays true forever, so the mock cannot represent an unsubscribed bridge at all.

Suggested change

	onEvent: (event: string, cb: EventCallback) => {
	listeners.set(event, cb);
	return unsubscribe;
	},
	onEvent: (event: string, cb: EventCallback) => {
	listeners.set(event, cb);
	return () => {
	// Actually detach, so a listener that outlives unmount is observable
	// through `emit` rather than only through the call count below.
	listeners.delete(event);
	unsubscribe();
	};
	},

[rahulkatiyar19955]

Altitude: the handshake removes the dropped chunk, but not the gate that caused it. — extension/src/ExtensionRoot.tsx

This comment names the real problem exactly, and then works around it. OpenFileListener renders nothing and consumes no context, yet it still sits below both if (orgLookup == undefined) return <></> and if (deepLinks == undefined) return <></>. Your own measurement puts that at ~2.4s, and each gate can take up to BridgeClient's 15s timeout before its .catch resolves it — so a slow session.getOrgContext delays the file transfer by that long, even though nothing about opening a file depends on org context.

The deeper fix is to give this listener a tree position above both gates. It has to be a stable position though — a remount would re-issue the handshake after the host has already consumed its one-shot pendingOpenPath, and the second time nothing streams at all. So not return <OpenFileListener .../> in the early-return branches (different tree shape ⇒ unmount + remount when the gate clears), but something like always rendering <><OpenFileListener bridge={bridge} />{gatesCleared ? <SharedRoot>…</SharedRoot> : undefined}</>.

Left unapplied here since it is a behaviour change outside this diff — flagging it as the follow-up rather than something to fold into this PR.

[rahulkatiyar19955]

This JSDoc describes the opposite invariant from the one the helper measures.

"Events the host had already pushed" is not what the closure records — subscribedWhenAsked = listeners.has(OPEN_FILE_CHUNK_EVENT) is a subscription check, and no event is ever pushed in that test. A reader trusting the comment reads expect(subscribedAtRequest()).toBe(true) as asserting that chunks had already arrived, which is the inverse of what the test exists to pin down.

Suggested change

	/** Events the host had already pushed when `request` was called. */
	/** Whether the chunk listener was already subscribed when `request` ran. */

[rahulkatiyar19955]

This signature is not biome-formatted — yarn format:ci fails on this branch.

npx biome format extension/src/OpenFileListener.test.tsx reports "Formatter would have printed the following content" and collapses the parameter list to a single line. CI runs format:ci (biome format --max-diagnostics=none .), which exits non-zero on exactly this diagnostic.

Suggested change

	function makeFakeBridge(
	{ requestImpl }: { requestImpl?: jest.Mock } = {},
	): {
	function makeFakeBridge({ requestImpl }: { requestImpl?: jest.Mock } = {}): {