Merge Hub organization-scoped discovery fix

2026-10-24 23:34:55 +08:00 · 2026-10-23 12:33:20 -07:00
parent 10f33c98b6 69e564efc8
commit e57b0572db
4 changed files with 124 additions and 22 deletions
@@ -138,7 +138,7 @@ Design rules:
 - `session/fork-metadata` owns fork ancestry and removal of inherited handoff markers; hosts retain title, transcript, and workspace-restore policies.
 - `@cline/core/cloud` owns remote cloud-session state and emits immutable snapshots and events. Viewers hydrating active runs with `readMessages` reconcile canonical history at completion even if they missed the run start. Desktop uses `HubCloudSessionClient` over its existing shared Hub connection. `HubCloudSessionService` in the daemon owns its account-scoped controller, refresh-aware credentials, organization lookup, provisioning, sandbox connections, queues, approvals, and transcript reconciliation. Desktop projects the snapshots into its UI; feature gating and host persistence remain in the app. Direct controller hosts (including handoff flows) can still supply authentication. Importing this subpath does not initialize a local agent.
 - `cloud/models` owns cloud model eligibility; `services/cloud-handoff` owns Git preflight, fingerprints, and transcript verification. Hosts own transfer orchestration, source locks, persistence, feature gating, and draft recovery.
- Cloud RPC validates an explicit operation allowlist and binds requests to the viewer’s observed hub instance and account-scope generation before dispatch. Handoff and prompt dispatch lifecycle callbacks stay with direct controller hosts and are excluded from the serializable RPC arguments. Viewer disconnects do not stop cloud work or close daemon-owned sandbox connections. Account changes retire the old controller and invalidate viewer caches; daemon shutdown disposes it. Cloud events carry a hub-instance id, account-scope generation, and revision; reconnecting subscriptions receive a complete snapshot set, including an empty set, and remove sessions and approvals absent from it. Scope checks run at command/auth boundaries and periodically, avoiding provider-settings reads per streamed event. Token refresh updates credential-based scope identity without retiring the same account’s controller. After an organization switch, desktop attempts a cloud-enabled organization refresh without making switch success or viewer cleanup depend on it. Fresh requests invalidate the old cache before fetching; a returning viewer also invalidates it to cover switches made while the Hub was unreachable. Cloud events are excluded from the durable local event log so old-account transcripts cannot be replayed after a scope change. Dispatched cloud mutations are never automatically resent after a lost reply.
+- Cloud RPC validates an explicit operation allowlist and binds requests to the viewer’s observed hub instance and account-scope generation before dispatch. Handoff and prompt dispatch lifecycle callbacks stay with direct controller hosts and are excluded from the serializable RPC arguments. Viewer disconnects do not stop cloud work or close daemon-owned sandbox connections. Account changes retire the old controller and invalidate viewer caches; daemon shutdown disposes it. Cloud events carry a hub-instance id, account-scope generation, and revision; reconnecting subscriptions receive a complete snapshot set, including an empty set, and remove sessions and approvals absent from it. Scope checks run at command/auth boundaries and periodically, avoiding provider-settings reads per streamed event. Token refresh updates credential-based scope identity without retiring the same account’s controller. Discovery caches, pending listing requests, and newly created fallback rows are scoped to their organization; switching the sidebar preserves other organizations’ task state without displaying their rows. After an organization switch, desktop attempts a cloud-enabled organization refresh without making switch success or viewer cleanup depend on it. Fresh requests invalidate the old cache before fetching; a returning viewer also invalidates it to cover switches made while the Hub was unreachable. Cloud events are excluded from the durable local event log so old-account transcripts cannot be replayed after a scope change. Dispatched cloud mutations are never automatically resent after a lost reply.
 - The cloud controller resumes suspended sessions and restores saved tasks.
 - Multi-step handoff callers can pin the existing Hub account scope across repository preparation, model lookup, creation, and sending. Send lifecycle callbacks stay inside the daemon; a cloud-specific failure payload preserves whether inner dispatch began without changing ordinary send success results. Missing replies remain uncertain.
 - The hub retains pending first-task creation options with its controller across desktop viewer disconnects and reconnects. The controller consumes that intent when an inner task exists or is created. Account changes dispose the old scope and its intent.
@@ -180,6 +180,10 @@ Handoff callbacks and the sixth `send` lifecycle argument are direct-controller
 features and cannot cross this RPC boundary. Hub-backed creation defaults to
 `"resumable"` sandboxes.

+Discovery caches, pending listing requests, and newly created fallback rows are
+scoped to their organization; switching the sidebar preserves other organizations’
+task state without displaying their rows.
+
 After a successful organization switch, `refreshOrganization()` invalidates the
 Hub's cached organization before fetching the current one. A fetch failure leaves
 the old cache invalidated. Account-switch success and desktop cleanup must not
@@ -210,6 +210,78 @@ function resumableFixture(status = "ready") {
 }

 describe("CloudSessionController neutral host contract", () => {
+	it("does not reuse another organization's listing while the new listing times out", async () => {
+		let organizationId = "org-one";
+		const f = fixture({ getActiveOrganizationId: async () => organizationId });
+		try {
+			expect(await f.controller.listForDiscovery()).toHaveLength(1);
+			organizationId = "org-two";
+			let finish!: (rows: CloudSessionRecord[]) => void;
+			f.api.list.mockImplementationOnce(
+				() =>
+					new Promise((resolve) => {
+						finish = resolve;
+					}),
+			);
+			expect(await f.controller.listForDiscovery({ timeoutMs: 0 })).toEqual([]);
+			finish([]);
+			expect(await f.controller.listForDiscovery()).toEqual([]);
+		} finally {
+			await f.controller.dispose();
+		}
+	});
+	it("starts discovery in the new organization without joining an old organization's pending listing", async () => {
+		let organizationId = "org-one";
+		const f = fixture({ getActiveOrganizationId: async () => organizationId });
+		let finish!: (rows: CloudSessionRecord[]) => void;
+		f.api.list.mockImplementationOnce(
+			() =>
+				new Promise((resolve) => {
+					finish = resolve;
+				}),
+		);
+		const oldListing = f.controller.listForDiscovery();
+		try {
+			await vi.waitFor(() =>
+				expect(f.api.list).toHaveBeenCalledWith("org-one"),
+			);
+			organizationId = "org-two";
+			f.api.list.mockResolvedValue([]);
+			expect(await f.controller.listForDiscovery()).toEqual([]);
+		} finally {
+			finish([structuredClone(record)]);
+			await oldListing;
+			await f.controller.dispose();
+		}
+	});
+
+	it.each([
+		undefined,
+		"org-one",
+	])("keeps newly created fallback rows in their creation scope (%s)", async (initialOrganization) => {
+		let organizationId: string | undefined = initialOrganization;
+		const f = fixture({ getActiveOrganizationId: async () => organizationId });
+		f.api.list.mockResolvedValue([]);
+		try {
+			await f.controller.create({
+				modelId: "model",
+				repoUrl: "https://github.com/cline/test",
+			});
+			expect(await f.controller.listForDiscovery()).toHaveLength(1);
+			organizationId = initialOrganization ? undefined : "org-two";
+			await f.controller.refreshOrganization();
+			expect(await f.controller.listForDiscovery()).toEqual([]);
+			expect(await f.controller.listForDiscovery()).toEqual([]);
+			// The original task and first-task preferences survive a sidebar scope change.
+			expect(f.controller.getCachedDiscoveryRecord(record.id)).toBeDefined();
+			organizationId = initialOrganization;
+			await f.controller.refreshOrganization();
+			expect(await f.controller.listForDiscovery()).toHaveLength(1);
+		} finally {
+			await f.controller.dispose();
+		}
+	});
+
 	it("refreshes an attached session title after an external rename", async () => {
 		const f = await attached();
 		await f.controller.updateTitle(record.id, "Desktop title");
@@ -435,10 +435,17 @@ export class CloudSessionController {
 	>();
 	private readonly knownSessions = new Map<string, CloudSessionRecord>();
 	// Retain new sessions until discovery has observed them at least once.
-	private readonly unlistedSessions = new Map<string, CloudSessionRecord>();
+	private readonly unlistedSessions = new Map<
+		string,
+		{ record: CloudSessionRecord; organizationId: string | undefined }
+	>();
 	private readonly pendingInitialTasks: Map<string, CloudCreationOptions>;
 	private lastListedSessions: CloudSessionRecord[] = [];
-	private discoveryRefresh?: Promise<CloudSessionRecord[]>;
+	private lastListedOrganizationId?: string;
+	private discoveryRefresh?: {
+		organizationId: string | undefined;
+		promise: Promise<CloudSessionRecord[]>;
+	};
 	private readonly unconfirmedInnerCreates = new Set<string>();
 	private readonly createRequests = new Map<
 		string,
@@ -700,6 +707,14 @@ export class CloudSessionController {
 	}

 	async list(): Promise<CloudSessionRecord[]> {
+		return await this.listInOrganization(
+			await this.resolveActiveOrganizationId(),
+		);
+	}
+
+	private async listInOrganization(
+		organizationId: string | undefined,
+	): Promise<CloudSessionRecord[]> {
 		if (this.disposed) throw new Error("Cloud session controller was disposed");
 		const currentTitle = (id: string) =>
 			this.sessions.get(id)?.title ?? this.knownSessions.get(id)?.title;
@@ -719,12 +734,12 @@ export class CloudSessionController {
 				session.title = title ?? session.title;
 			}
 		};
-		const organizationId = await this.resolveActiveOrganizationId();
 		const listed = (await this.options.api.list(organizationId)).map(
 			(session) => this.preserveConnectedRuntimeModel(session),
 		);
 		if (this.disposed) throw new Error("Cloud session controller was disposed");
 		// Keep canonical rows available while their status checks run.
+		this.lastListedOrganizationId = organizationId;
 		this.lastListedSessions = listed;
 		for (const session of listed) {
 			preserveLocalTitle(session);
@@ -733,9 +748,9 @@ export class CloudSessionController {
 		}
 		// Omission can be a listing race; only a definitive status error drops a fallback.
 		await Promise.all(
-			Array.from(this.unlistedSessions.keys(), (sessionId) =>
-				this.getCrossScopeDiscoveryRecord(sessionId),
-			),
+			Array.from(this.unlistedSessions.entries())
+				.filter(([, pending]) => pending.organizationId === organizationId)
+				.map(([sessionId]) => this.getCrossScopeDiscoveryRecord(sessionId)),
 		);
 		const scoped = await Promise.all(
 			listed.map(async (session) => {
@@ -801,6 +816,7 @@ export class CloudSessionController {
 				}
 			}
 		}
+		this.lastListedOrganizationId = organizationId;
 		this.lastListedSessions = scoped;
 		for (const record of scoped)
 			if (this.sessions.has(record.id)) this.publishSnapshot(record.id);
@@ -913,16 +929,24 @@ export class CloudSessionController {
 	async listForDiscovery(
 		options: { timeoutMs?: number } = {},
 	): Promise<JsonRecord[]> {
-		const refresh =
-			this.discoveryRefresh ??
-			this.list().finally(() => {
-				if (this.discoveryRefresh === refresh) {
+		const organizationId = await this.resolveActiveOrganizationId();
+		let refresh =
+			this.discoveryRefresh?.organizationId === organizationId
+				? this.discoveryRefresh?.promise
+				: undefined;
+		if (!refresh) {
+			const promise = this.listInOrganization(organizationId).finally(() => {
+				if (this.discoveryRefresh?.promise === promise)
 					this.discoveryRefresh = undefined;
-				}
 			});
-		this.discoveryRefresh = refresh;
-
-		let records = this.lastListedSessions;
+			this.discoveryRefresh = { organizationId, promise };
+			refresh = promise;
+		}
+		const cachedRecords = () =>
+			this.lastListedOrganizationId === organizationId
+				? this.lastListedSessions
+				: [];
+		let records = cachedRecords();
 		if (options.timeoutMs === undefined) {
 			records = await refresh;
 		} else {
@@ -941,12 +965,12 @@ export class CloudSessionController {
 								error,
 							});
 						}
-						return { value: this.lastListedSessions };
+						return { value: cachedRecords() };
 					},
 				),
 				new Promise<{ value: CloudSessionRecord[] }>((resolve) => {
 					timeout = setTimeout(
-						() => resolve({ value: this.lastListedSessions }),
+						() => resolve({ value: cachedRecords() }),
 						Math.max(0, options.timeoutMs ?? 0),
 					);
 				}),
@@ -956,10 +980,12 @@ export class CloudSessionController {
 		}

 		const recordsById = new Map(
-			[...this.unlistedSessions.values(), ...records].map((record) => [
-				record.id,
-				record,
-			]),
+			[
+				...Array.from(this.unlistedSessions.values())
+					.filter((pending) => pending.organizationId === organizationId)
+					.map((pending) => pending.record),
+				...records,
+			].map((record) => [record.id, record]),
 		);
 		const listed = [...recordsById.values()].map((record) => {
 			const projected = cloudSessionToDiscoveryRecord(record);
@@ -1179,7 +1205,7 @@ export class CloudSessionController {
 			updatedAt: new Date().toISOString(),
 		};
 		this.knownSessions.set(record.id, record);
-		this.unlistedSessions.set(record.id, record);
+		this.unlistedSessions.set(record.id, { record, organizationId });
 		// REST does not round-trip these client-side first-task preferences.
 		const {
 			autoApproveTools,