Resolve channel links before room mount

Bring the tested channel route fix from PR #391 into #344 so deep links mount once and keep the parent room through child navigation. Guard metadata callbacks by the route identity captured at render and renew the reviewed dynamic owner hash.

Author: MaggieAppleton

apps/web/src/channel-workspace.tsx

@@ -1,81 +1,52 @@
 import { useEffect, useState } from "react";
 
 import { readChannelRecovery, rememberChannel } from "./channel-recovery";
-import { newestDocument } from "./document-actions";
 import { documentRouteIdentity } from "./document-route-swap";
-import { useNavigationDocument } from "./navigation-shell";
 
-import type { ComponentType } from "react";
 import type * as Api from "./api";
 import type { DocumentRouteIdentity } from "./document-route-swap";
-import type {
-	ChannelSource,
-	DocumentRouteResolution,
-	HostedFailure,
-	HostedLoading,
-	HostedWorkspaceProps,
-} from "./hosted";
+import type { ChannelSource, HostedFailure, HostedLoading } from "./hosted";
 
+// A channel URL resolves to its document path before any workspace mounts, so the document
+// host mounts once under its own route identity instead of replacing a channel-keyed room.
 export default function ChannelWorkspace(
-	{ agent, Failure, Loading, onReady, retryable, source, user }: {
-		agent: boolean;
+	{ Failure, Loading, onReady, onResolved, retryable, source, user }: {
 		Failure: typeof HostedFailure;
 		Loading: typeof HostedLoading;
-		onReady?: (key: DocumentRouteIdentity, resolution?: DocumentRouteResolution) => void;
+		onReady: (key: DocumentRouteIdentity) => void;
+		onResolved: (key: DocumentRouteIdentity, pathname: string) => void;
 		retryable: (error: unknown) => boolean;
 		source: ChannelSource;
 		user: Api.User;
 	},
 ) {
-	type LoadedWorkspace = {
-		detail: Api.ChannelDetail;
-		Workspace: ComponentType<HostedWorkspaceProps>;
-	};
-	let [loaded, setLoaded] = useState<LoadedWorkspace>();
 	let [error, setError] = useState<unknown>();
 	let [retry, setRetry] = useState(0);
-	let { channel: navigationChannel } = useNavigationDocument();
 	let routeKey = documentRouteIdentity(source);
-	let recovery = readChannelRecovery(user.id, source.id);
+	let id = source.id;
 
 	useEffect(() => {
 		let active = true;
 		let controller = new AbortController();
-		setLoaded(undefined);
 		setError(undefined);
-		let prepared = import("./document-loader").then(module =>
-			module.prepareDocumentLoad({ id: source.id }, controller.signal)
-		).then(({ detail, pathname }) => ({ canonicalPath: pathname, detail }));
-		prepared = prepared.then(resolved => {
-			if (active) {
-				rememberChannel(user.id, resolved.detail.channel, resolved.detail.repository);
-			}
-			return resolved;
-		});
-		let workspace = import("./room-workspace").then(module => ({
-			Workspace: module.RoomWorkspace,
-		}));
-		Promise.all([prepared, workspace]).then(([resolved, selected]) => {
-			if (active) {
-				setLoaded({ detail: resolved.detail, ...selected });
-				onReady?.(routeKey, {
-					canonicalPath: resolved.canonicalPath,
-					channel: resolved.detail.channel,
-					routeKey,
-				});
-			}
+		import("./document-loader").then(module =>
+			module.prepareDocumentLoad({ id }, controller.signal)
+		).then(({ detail, pathname }) => {
+			if (!active) return;
+			rememberChannel(user.id, detail.channel, detail.repository);
+			onResolved(routeKey, pathname);
 		}, reason => {
-			if (active) {
-				setError(reason);
-				onReady?.(routeKey);
-			}
+			if (!active) return;
+			setError(reason);
+			onReady(routeKey);
 		});
 		return () => {
 			active = false;
 			controller.abort();
 		};
-	}, [onReady, retry, routeKey, source, user.id]);
+	}, [id, onReady, onResolved, retry, routeKey, user.id]);
 	if (error) {
+		let recovery = readChannelRecovery(user.id, id);
 		return (
 			<Failure
 				channel={recovery?.channel}
@@ -90,27 +61,5 @@ export default function ChannelWorkspace(
 			/>
 		);
 	}
-	if (!loaded) return <Loading label="Opening document…" />;
-	let { detail } = loaded;
-	let channel = navigationChannel?.id === detail.channel.id
-		? newestDocument(detail.channel, navigationChannel)
-		: detail.channel;
-	let props: HostedWorkspaceProps = {
-		agent,
-		archivedAt: channel.archivedAt,
-		canEdit: !channel.archivedAt && (detail.canEdit || detail.canManage),
-		canManage: detail.canManage,
-		description: channel.description,
-		descriptionRevision: channel.descriptionRevision,
-		handle: user.login,
-		label: channel.title,
-		presentation: { type: "document" },
-		slug: channel.slug,
-		updatedAt: channel.updatedAt,
-		repository: detail.repository,
-		room: detail.channel.id,
-		userId: user.id,
-	};
-	let Document = loaded.Workspace;
-	return <Document {...props} />;
+	return <Loading label="Opening document…" />;
 }

apps/web/src/document-workspace-host.tsx

@@ -315,6 +315,7 @@ export default function DocumentWorkspaceHost(
 	}, [onChildClose, presentation]);
 
 	let metadataChanged = useCallback((kind: "parent" | "child", metadata: Metadata) => {
+		if (documentRouteIdentity(routeRef.current) !== routeKey) return;
 		let current = stateRef.current;
 		if (current.status === "empty") return;
 		let target = kind === "parent" ? current.loaded.parent : current.loaded.child;
@@ -336,8 +337,8 @@ export default function DocumentWorkspaceHost(
 		if (kind === "parent" && routeRef.current.page === "child") {
 			history.replaceState(rebaseChildHistoryState(history.state, paths.parent), "");
 		}
-		onCanonicalPath(layerKey, documentRouteIdentity(routeRef.current), pathname);
-	}, [layerKey, onCanonicalPath, send]);
+		onCanonicalPath(layerKey, routeKey, pathname);
+	}, [layerKey, onCanonicalPath, routeKey, send]);
 	let parentMetadataChanged = useCallback(
 		(metadata: Metadata) => metadataChanged("parent", metadata),
 		[metadataChanged],

apps/web/src/hosted.tsx

@@ -275,6 +275,9 @@ function DocumentRouteSwap(
 			onDocumentRouteSettled(requestedRoute.current.routeKey);
 		}
 	}, [onDocumentRouteSettled]);
+	let channelResolved = useCallback((key: DocumentRouteIdentity, pathname: string) => {
+		if (requestedRoute.current.key === key) onCanonicalPath(pathname);
+	}, [onCanonicalPath]);
 	let metadataPath = useCallback((
 		key: DocumentRouteIdentity,
 		metadataRouteKey: DocumentRouteIdentity,
@@ -340,10 +343,10 @@ function DocumentRouteSwap(
 							: (
 								<Suspense fallback={<Loading label="Opening document…" />}>
 									<ChannelWorkspace
-										agent={agent}
 										Failure={Failure}
 										Loading={Loading}
 										onReady={ready}
+										onResolved={channelResolved}
 										retryable={retryableChannelFailure}
 										source={source}
 										user={user}

e2e/hosted.e2e.ts

@@ -1,6 +1,7 @@
 import { createChannel } from "./database";
 import { authenticate, expect, roomPath, test } from "./room";
 import { expectInsideViewport, expectNoHorizontalOverflow } from "./responsive";
+import { recordRoomMounts } from "./room-mounts";
 import { installVisualViewport } from "./visual-viewport";
 
 const score = {
@@ -84,6 +85,43 @@ test("an authenticated user adds a Project and creates its first document", asyn
 	await expect(activeRoute.locator("[data-plan-decisions-scroll]")).toBeAttached();
 });
 
+test("the Add project redirect through a channel link mounts its document once", async ({ baseURL, page, room }) => {
+	// The room fixture gives the project a saved document to land on.
+	void room;
+	let recorded = await recordRoomMounts(page);
+	let entered: string[] = [];
+	page.on("framenavigated", frame => {
+		let match = /^\/channels\/([^/]+)$/.exec(new URL(frame.url()).pathname);
+		if (frame === page.mainFrame() && match) entered.push(match[1]!);
+	});
+	// Holding the project's document list makes the landing redirect use the saved
+	// document's channel link, as it does when that list is slower than navigation.
+	await page.route("**/api/repositories/octo-org/score/channels", async route => {
+		await expect.poll(() => entered.length).toBeGreaterThan(0);
+		await route.continue();
+	});
+	await authenticate(page, `project-redirect-${crypto.randomUUID()}`, baseURL!);
+	await page.goto("/");
+	let documentRead = page.waitForResponse(response =>
+		new URL(response.url()).pathname.startsWith("/api/repositories/octo-org/score/documents/")
+	);
+	await repositoryOption(page, "score").click();
+
+	await expect(page).toHaveURL(url => url.pathname.startsWith("/documents/octo-org/score/"));
+	await documentRead;
+	let landed = entered[0]!;
+	// Wait until the route swap has settled on a single layer showing the synced document.
+	await page.waitForFunction(id => {
+		let layers = document.querySelectorAll(".document-route-swap > [data-content-swap-state]");
+		return layers.length === 1
+			&& !!layers[0]!.querySelector(`[data-workspace-room="${id}"] [data-plan-synced]`);
+	}, landed);
+	let mounts = await recorded.mounts();
+	expect(mounts.filter(entry => entry.startsWith("unmount "))).toEqual([]);
+	expect(mounts.filter(entry => entry === `mount ${landed}`)).toHaveLength(1);
+	expect(recorded.reads(landed)).toBe(1);
+});
+
 test("a signed-out document link returns to the document after OAuth", async ({ baseURL, page, room }) => {
 	let path = roomPath(room);
 	await page.goto(path);

e2e/research-child-surface.e2e.ts

@@ -1,6 +1,7 @@
 import { chatInput } from "./chat-input";
 import { seedChildChannel } from "./database";
 import { installPointerMedia } from "./pointer-media";
+import { recordRoomMounts } from "./room-mounts";
 import { authenticate, expect, test } from "./room";
 
 import type { Chat } from "../packages/protocol/index";
@@ -692,6 +693,42 @@ test("a recovered child id enters the canonical anchored child workspace", async
 	await expect(surface).toHaveCount(0);
 });
 
+test("a channel link opens one parent room that a child opens and closes over", async ({ baseURL, page, room, seed }) => {
+	await seed(PARENT_SOURCE);
+	let childTitle = `Channel link child ${room.slice(0, 8)}`;
+	let child = await seedChildChannel(
+		port(baseURL!),
+		room,
+		crypto.randomUUID(),
+		childTitle,
+		CHILD_SOURCE,
+	);
+	let recorded = await recordRoomMounts(page);
+	await authenticate(page, "ana", baseURL!);
+	await page.goto(`/channels/${room}`);
+
+	let parentPath = `/documents/octo-org/score/test-${room.slice(0, 8)}`;
+	await expect(page).toHaveURL(url => url.pathname === parentPath);
+	let parent = page.locator(`[data-workspace-room="${room}"]`);
+	await expect(parent.locator("[data-plan-synced]")).toBeAttached();
+	await parent.evaluate(element => element.setAttribute("data-mount-token", "preserved"));
+
+	let surface = page.getByRole("region", { name: `Child document: ${childTitle}` });
+	await page.getByRole("complementary", { name: "Projects" })
+		.getByRole("link", { name: childTitle, exact: true }).click();
+	await expect(page).toHaveURL(url => url.pathname === child.path);
+	await expect(surface.locator(`[data-workspace-room="${child.id}"]`)).toBeVisible();
+	await page.keyboard.press("Escape");
+	await expect(page).toHaveURL(url => url.pathname === parentPath);
+	await expect(surface).toHaveCount(0);
+
+	await expect(parent).toHaveAttribute("data-mount-token", "preserved");
+	expect((await recorded.mounts()).filter(entry => entry.endsWith(room))).toEqual([
+		`mount ${room}`,
+	]);
+	expect(recorded.reads(room)).toBe(1);
+});
+
 test("a child load failure preserves its mounted parent through back and retry", async ({ baseURL, join, page, room, seed }) => {
 	await seed(PARENT_SOURCE);
 	let childTitle = `Unreliable source ${room.slice(0, 8)}`;

e2e/room-mounts.ts

@@ -0,0 +1,37 @@
+import type { Page } from "@playwright/test";
+
+/** Record every workspace room element that enters or leaves the document. */
+export async function recordRoomMounts(page: Page): Promise<{
+	mounts: () => Promise<string[]>;
+	reads: (id: string) => number;
+}> {
+	await page.addInitScript(() => {
+		let log: string[] = [];
+		Object.defineProperty(window, "__roomMounts", { value: log });
+		let record = (nodes: NodeList, verb: string) => {
+			for (let node of nodes) {
+				if (!(node instanceof Element)) continue;
+				for (let element of [node, ...node.querySelectorAll("[data-workspace-room]")]) {
+					let id = element.getAttribute("data-workspace-room");
+					if (id) log.push(`${verb} ${id}`);
+				}
+			}
+		};
+		new MutationObserver(records => {
+			for (let item of records) {
+				record(item.addedNodes, "mount");
+				record(item.removedNodes, "unmount");
+			}
+		}).observe(document, { childList: true, subtree: true });
+	});
+	let channelReads: string[] = [];
+	page.on("request", request => {
+		let match = /^\/api\/channels\/([^/]+)$/.exec(new URL(request.url()).pathname);
+		if (match) channelReads.push(match[1]!);
+	});
+	return {
+		mounts: () =>
+			page.evaluate(() => [...(window as unknown as { __roomMounts: string[] }).__roomMounts]),
+		reads: id => channelReads.filter(value => value === id).length,
+	};
+}

scripts/design-contract/exceptions/dynamic-web.json

@@ -59,7 +59,7 @@
 				1
 			]
 		],
-		"sourceHash": "18d229959db00ac1a909e34270ef9519466fc48047bffbcf4a01786a7fdc7d40"
+		"sourceHash": "890443cedd0584c3e57e5295f4fafff7f0bc8231c818cb706c061d097991b49a"
 	},
 	{
 		"file": "apps/web/src/document-rename.tsx",