Address PR feedback on external-session titling

- `generateExternalSessionTitle` returned a promise that resolved before
  generation had run, because `_generateTitleSoon` starts the work
  fire-and-forget. The awaited loop in `_titleUntitledExternalSessions`
  therefore launched both model calls concurrently and
  `whenDeferredWorkSettled()` reported completion while generation and
  persistence were still in flight, breaking the lane's serialization
  contract. Split out `_startTitleGeneration`, which returns the tracked
  promise, and await it on the external-session path.

- `listSessions` marked the first listing as served from its rejection
  handler too, so deferred maintenance could start after a failed listing
  and compete with the retry the gate exists to protect. Keep in-flight
  cleanup on both paths but only set the flag on fulfillment.

- `agentHostMain` marked startup complete while the configured WebSocket
  server was still being created and wired, so deferred work could begin
  concurrently with that remaining startup. Mark completion once the
  optional startup promise settles, keeping its non-fatal error handling.

Adds a regression test for the failed-listing gate, and tightens the
existing titling tests to assert the awaited-generation contract without
polling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: benibenj

src/vs/platform/agentHost/node/agentHostMain.ts

@@ -457,14 +457,15 @@ async function startAgentHost(): Promise<void> {
 		handler => protocolHandlers.push(handler),
 	);
 	configuredWebSocketServer.settleWith(configuredWebSocketServerStart);
+	// Startup is complete once the last ingress has settled — successfully or
+	// not, since a failed WebSocket server is non-fatal. Deferred maintenance
+	// then runs after a client has also been served its first session listing.
 	void configuredWebSocketServerStart.catch(err => {
 		logService.error('Failed to start WebSocket server', err);
+	}).finally(() => {
+		agentService.markStartupComplete();
 	});
 
-	// Every ingress is wired: deferred maintenance may run once a client has
-	// also been served its first session listing.
-	agentService.markStartupComplete();
-
 	process.once('exit', () => {
 		agentService.dispose();
 		logService.dispose();

src/vs/platform/agentHost/node/agentHostSessionTitleController.ts

@@ -430,12 +430,15 @@ export class AgentHostSessionTitleController extends Disposable {
 	 * live state (it is materialized when opened), so the generated title is
 	 * persisted and pushed onto its surfaced summary. A session that already
 	 * carries a persisted title keeps it; a rename during generation cancels it.
+	 *
+	 * Unlike the other entry points this awaits generation, so the caller's
+	 * deferred-work lane stays serialized against it.
 	 */
 	async generateExternalSessionTitle(session: ProtocolURI, userPrompt: string): Promise<void> {
 		if (this._isEphemeralSession(session) || await this._readPersistedTitleMetadata(session, SESSION_CUSTOM_TITLE_KEY)) {
 			return;
 		}
-		this._generateTitleSoon(
+		await this._startTitleGeneration(
 			session,
 			{ content: userPrompt, isConversation: false, gitHubReferenceSource: userPrompt },
 			'',
@@ -517,10 +520,22 @@ export class AgentHostSessionTitleController extends Disposable {
 		currentTitleMatchesFallback: () => boolean,
 		persist: (title: string) => void,
 	): void {
+		void this._startTitleGeneration(key, prompt, fallbackTitle, apply, currentTitleMatchesFallback, persist);
+	}
+
+	/** Starts generation and resolves once the title has been applied and persisted. */
+	private _startTitleGeneration(
+		key: ProtocolURI,
+		prompt: ITitlePromptContext,
+		fallbackTitle: string,
+		apply: (title: string) => void,
+		currentTitleMatchesFallback: () => boolean,
+		persist: (title: string) => void,
+	): Promise<void> {
 		this._cancelTitleGeneration(key);
 		const source = new CancellationTokenSource();
 		this._titleGenerationCancellationSources.set(key, source);
-		void this._generateTitle(key, prompt, fallbackTitle, apply, currentTitleMatchesFallback, persist, source.token).catch(err => {
+		return this._generateTitle(key, prompt, fallbackTitle, apply, currentTitleMatchesFallback, persist, source.token).catch(err => {
 			if (!source.token.isCancellationRequested) {
 				this._logService.warn(`[AgentHostSessionTitleController] Failed to apply generated title for ${key}`, err);
 			}

src/vs/platform/agentHost/node/agentService.ts

@@ -1903,10 +1903,17 @@ export class AgentService extends Disposable implements IAgentService {
 			if (this._inFlightListSessions.get(mode) === entry) {
 				this._inFlightListSessions.delete(mode);
 			}
-			this._firstListingServed = true;
-			this._openStartupSettled();
 		};
-		void promise.then(clear, clear);
+		void promise.then(
+			() => {
+				clear();
+				// Only a served listing ends startup: a failed one is retried, and
+				// deferred work must not compete with that retry.
+				this._firstListingServed = true;
+				this._openStartupSettled();
+			},
+			clear,
+		);
 		return [...await promise];
 	}
 

src/vs/platform/agentHost/test/node/agentHostSessionTitleController.test.ts

@@ -1088,7 +1088,7 @@ suite('AgentHostSessionTitleController', () => {
 
 		stateManager.announceSurfacedSession(createSummary(external));
 		await controller.generateExternalSessionTitle(external.toString(), 'Fix the flaky renderer test');
-		await waitForCondition(async () => await db.getMetadata('customTitle') === 'Flaky renderer test', 'generated title should be persisted');
+		// No polling: awaiting the call must mean the title is applied and persisted.
 
 		assert.deepStrictEqual({
 			summaryTitles,
@@ -1111,11 +1111,12 @@ suite('AgentHostSessionTitleController', () => {
 		const external = URI.parse('agenthost-session://claude/external-session');
 
 		stateManager.announceSurfacedSession(createSummary(external));
-		await controller.generateExternalSessionTitle(external.toString(), 'Fix the flaky renderer test');
+		const generation = controller.generateExternalSessionTitle(external.toString(), 'Fix the flaky renderer test');
 		await waitForCondition(() => copilotApiService.utilityCalls.length === 1, 'title generation should start');
 		controller.markTitleRenamed(external.toString());
 		resolveTitle('Flaky renderer test');
-		await Promise.resolve();
+		// Also proves a cancelled generation settles rather than hanging its caller.
+		await generation;
 
 		assert.deepStrictEqual({
 			aborted: copilotApiService.utilityCalls[0].options?.signal?.aborted,

src/vs/platform/agentHost/test/node/agentService.test.ts

@@ -3007,13 +3007,6 @@ suite('AgentService (node dispatcher)', () => {
 			await (service as unknown as { _sessionListReconciliation: Promise<void> })._sessionListReconciliation;
 		}
 
-		async function waitForUtilityCalls(copilotApiService: TestCopilotApiService, count: number): Promise<void> {
-			for (let i = 0; i < 20 && copilotApiService.utilityCalls.length < count; i++) {
-				await new Promise(resolve => setTimeout(resolve, 5));
-			}
-			assert.strictEqual(copilotApiService.utilityCalls.length, count, 'expected exactly this many title generations');
-		}
-
 		function exposeListedSessions(service: AgentService, sessions: readonly IAgentSessionMetadata[]): void {
 			const summaries = sessions.map((session): SessionSummary => {
 				const provider = AgentSession.provider(session.session);
@@ -3145,13 +3138,18 @@ suite('AgentService (node dispatcher)', () => {
 			const callsBeforeStartupSettled = copilotApiService.utilityCalls.length;
 			await svc.listSessions();
 			svc.markStartupComplete();
+			// The lane is serialized, so settling implies generation finished: no polling.
 			await svc.whenDeferredWorkSettled();
-			await waitForUtilityCalls(copilotApiService, 2);
 
 			const titled = [oldest, middle, newest].filter(session => copilotApiService.utilityCalls.some(
 				call => call.request.messages.some(message => message.content.includes(`prompt of ${buildDefaultChatUri(session)}`))));
-			assert.deepStrictEqual({ callsBeforeStartupSettled, titled: titled.map(session => AgentSession.id(session)) }, {
+			assert.deepStrictEqual({
+				callsBeforeStartupSettled,
+				callsAfterSettled: copilotApiService.utilityCalls.length,
+				titled: titled.map(session => AgentSession.id(session)),
+			}, {
 				callsBeforeStartupSettled: 0,
+				callsAfterSettled: 2,
 				titled: ['middle', 'newest'],
 			});
 		});
@@ -4448,6 +4446,38 @@ suite('AgentService (node dispatcher)', () => {
 			});
 		});
 
+		test('a failed listing does not settle startup, so deferred work waits for a served one', async () => {
+			class UnavailableCatalogAgent extends MockAgent {
+				override readonly onDidDiscoverChats = Event.None;
+				enumerable = false;
+				override async listChatsToMigrate(): Promise<readonly IAgentChatMetadata[] | undefined> {
+					return this.enumerable ? [] : undefined;
+				}
+			}
+			const svc = createExternalSessionService();
+			const agent = disposables.add(new UnavailableCatalogAgent('copilot'));
+			svc.registerProvider(agent);
+			svc.markStartupComplete();
+
+			await assert.rejects(svc.listSessions());
+			let deferredWorkSettled = false;
+			void svc.whenDeferredWorkSettled().then(() => { deferredWorkSettled = true; });
+			// Ample turns for the gated maintenance to run if the gate were open.
+			for (let i = 0; i < 50; i++) {
+				await timeout(0);
+			}
+			const settledByFailedListing = deferredWorkSettled;
+
+			agent.enumerable = true;
+			await svc.listSessions();
+			await svc.whenDeferredWorkSettled();
+
+			assert.deepStrictEqual({ settledByFailedListing, settledAfterServedListing: deferredWorkSettled }, {
+				settledByFailedListing: false,
+				settledAfterServedListing: true,
+			});
+		});
+
 		test('overlapping mode computations share ownership of a replacement migration retry', async () => {
 			const retryGate = new DeferredPromise<void>();
 			class SingleFlightRetryAgent extends MockAgent {