Skip to content

Keep the Agents window responsive when marking many sessions as read - #342253

Draft
Karthik Nadig (karthiknadig) wants to merge 1 commit into
mainfrom
karthiknadig/bug-swarm/c9-f1-mark-all-read-freeze
Draft

Karthik Nadig (karthiknadig) wants to merge 1 commit into
mainfrom
karthiknadig/bug-swarm/c9-f1-mark-all-read-freeze

Conversation

@karthiknadig

@karthiknadig Karthik Nadig (karthiknadig) commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

Problem

In the Agents window, right-clicking a sessions section header and choosing Mark All as Read froze the whole window until every session had been processed. The freeze grows faster than the number of sessions. With 500 unread sessions in one section, the window stopped painting and ignored input for about 3.4 s (median of 3 runs). With 100 sessions, the freeze lasted about 0.4 s.

With this change, neither size produces a renderer task over 50 ms, and all visible rows show as read in about 0.2 s.

Unread sessions in the section Build Longest renderer task (3 runs) Median Time until all visible rows show read (median)
500 main 3455, 3399, 3415 ms 3415 ms 3506 ms
500 this PR no task over 50 ms in any run under 50 ms 190 ms
100 main 399, 376, 427 ms 399 ms 487 ms
100 this PR no task over 50 ms in any run under 50 ms 117 ms

How these numbers were measured:

  • Build and setup: Code OSS dev builds on Windows, from main at 1e81fc7 and from this PR, both with VSCODE_DEV_DISABLE_DISPOSABLE_TRACKING=1. Each build ran in an Agents window connected through chat.remoteAgentHosts to a local synthetic agent host (the test ScriptedMockAgent, script below).
  • Input: before every run, the agent host restarted from an empty profile, so each run started with N + 1 unread sessions in one workspace section: N generated sessions plus one pre-existing session.
  • Runs: each size and build had 1 warm-up run plus 3 measured runs. Only the measured runs are in the table. The warm-up results matched them: 3616 ms and 389 ms on main, and no long task with this PR.
  • Longest renderer task: the largest longtask entry recorded by a PerformanceObserver in the renderer. Recording ran from just before the click until 1.5 s after the rows turned read. The browser reports only tasks longer than 50 ms, so "no task over 50 ms" means it reported no long task at all.
  • Time until all visible rows show read: time from the menu click until no visible row's accessible label still said "unread". This was checked by polling the DOM over the DevTools protocol every 20 ms, so it includes the menu click and polling overhead.

Root cause

SessionsManagementService.markAllRead called setSessionReadState once for each session. For the agent host providers (local and remote), each call fired onDidChangeSessions for that one session: see setSessionReadState and the legacy-protocol path _setLegacySessionReadState.

Each of those events reached every listener of the session change event, and several listeners do work proportional to the total number of sessions. The sessions list, for example, responds to every event with a synchronous refresh(). That refresh reads all sessions again, rebuilds an autorun over every session, regroups the list, and calls tree.setChildren. Marking N sessions therefore did N refreshes of size N, so the work grew with N² inside one task.

A CPU profile of Mark All as Read with 500 sessions on main confirmed this:

  • 2.97 s was spent inside markAllRead.
  • 2.93 s of that was spent delivering the per-session change events.
  • The sessions list refresh accounted for 1.57 s. The agent host customization listeners took 0.46 s, and the sessions list model, groups service, and other observers took the rest.

The per-session event pattern dates from moving read state into the providers in e2d7a76 (#325982).

Fix

  • Added an optional batch method, ISessionsProvider.setSessionsReadState(sessionIds, isRead). It matches the existing setChatReadStates batch method.
  • markAllRead now groups sessions by provider and calls each provider once. A provider without the batch method, such as the Copilot cloud provider, still receives one setSessionReadState call per session.
  • BaseAgentHostSessionsProvider, which covers both the local and remote agent hosts, implements the batch method by running its existing per-session setSessionReadState while its session change emitter is paused. The emitter is now a PauseableEmitter that merges queued events with the existing mergeSessionChangeEvents. Listeners therefore receive one change event that lists every affected session.

I fixed this in the provider rather than debouncing the sessions list, for two reasons:

  • About half of the profiled time was in other listeners of the same event.
  • A single event fixes every listener at once, without delaying updates to a single session.

What stays the same:

  • Each session's read state is still set optimistically and sent to the host one session at a time, using SessionIsReadChanged or ChatIsReadChanged. This includes the legacy-protocol path.
  • Mark as Read and Mark as Unread on a single session still fire their change immediately.
  • Ordering, grouping, and selection logic did not change.

Before / after

Mark All as Read on a section with 500 unread sessions. The caption at the top states the median result. The orange box is an on-screen timer driven by requestAnimationFrame, so it stops updating while the renderer is blocked. When the run ends, the box shows how long the rows took to turn read and the longest renderer task in that run.

Before

Before: on main, after Mark All as Read on 500 unread sessions the timer stops at 0.07 s for about 3.5 s, then every row turns read at once

After

After: with this PR, the same action turns every row read within about 0.2 s while the timer keeps running

How to test

  1. In a dev build, connect the Agents window to an agent host with a few hundred unread sessions in one workspace. The synthetic host below works: from the repo root, run node mock-host.mjs 8995 500. Then add "chat.remoteAgentHosts": [{ "address": "ws://127.0.0.1:8995", "name": "BS Mock Host" }] to your settings.
  2. In the Sessions list, right-click that host's section header and choose Mark All as Read. The window stays responsive, and every row turns read almost immediately. On main, the window freezes for several seconds.
  3. Right-click one session and choose Mark as Unread, then Mark as Read. The row updates immediately each time.
  4. Reload the window. The sessions are still read.
Automated repro scenario

Save as repro.cjs. Launch a Code OSS Agents window with remote debugging enabled, connected to the synthetic host below. Then run node <bug-swarm skill>/scripts/cdp-scenario.mjs --cdp <port> --scenario repro.cjs --out <dir>. The scenario fails on main and passes with this PR.

// "Mark All as Read" on an Agents window sessions section must not block the renderer.
//
// Precondition: a Code OSS Agents window (VSCODE_DEV_DISABLE_DISPOSABLE_TRACKING=1) connected via
// chat.remoteAgentHosts (name "BS Mock Host") to a fresh synthetic agent host with 500 unread sessions
// (mock-host.mjs below). No model traffic and no sign-in are involved.

const THRESHOLD_MS = 1000; // renderer may not be blocked longer than this by one bulk read-state update
const LIST = '.monaco-list[aria-label=Sessions]';

const OVERLAY = `(() => {
	let el = document.getElementById('bs-overlay');
	if (!el) {
		el = document.createElement('div');
		el.id = 'bs-overlay';
		el.style.cssText = 'position:fixed;right:16px;top:60px;z-index:100000;background:#000d;color:#fff;font:600 20px monospace;padding:10px 14px;border:2px solid #f80;border-radius:6px;white-space:pre;pointer-events:none';
		document.body.appendChild(el);
	}
	return true;
})()`;

async function setOverlay(page, text) {
	await page.evaluate(OVERLAY);
	await page.evaluate(t => { window.__bsTimerOn = false; document.getElementById('bs-overlay').textContent = t; }, text);
}

// Live timer driven by requestAnimationFrame: it freezes on screen while the renderer is blocked.
async function startTimer(page, caption) {
	await page.evaluate(OVERLAY);
	await page.evaluate(caption => {
		const el = document.getElementById('bs-overlay');
		const t0 = performance.now();
		window.__bsTimerOn = true;
		const tick = () => {
			if (!window.__bsTimerOn) { return; }
			el.textContent = `${caption}\nelapsed ${((performance.now() - t0) / 1000).toFixed(2)} s`;
			requestAnimationFrame(tick);
		};
		tick();
	}, caption);
}

const unreadVisible = page => page.evaluate(l => [...document.querySelectorAll(l + ' .monaco-list-row')]
	.map(r => r.getAttribute('aria-label') ?? '').filter(a => /unread/.test(a)).length, LIST);

module.exports = {
	id: 'agents-mark-all-read',
	title: 'Mark All as Read on a large sessions section keeps the Agents window responsive',
	stepPauseMs: 1500,
	steps: [
		{
			id: 'S1', title: 'Baseline: the host section lists 501 sessions and they are unread',
			async run({ page }) {
				const header = page.locator(`${LIST} .monaco-list-row`, { hasText: 'BS Mock Host' }).first();
				await header.waitFor({ state: 'visible', timeout: 20000 });
				const label = await header.getAttribute('aria-label');
				const count = Number(/, (\d+)$/.exec(label ?? '')?.[1]);
				if (!(count >= 500)) { throw new Error(`Expected >= 500 sessions in the host section, got "${label}"`); }
				const unread = await unreadVisible(page);
				if (unread < 3) { throw new Error(`Expected the visible sessions to be unread, found ${unread} unread rows`); }
				await setOverlay(page, `${count} sessions in this section\n${unread} visible rows are unread`);
				return `Section "${label}", ${unread} visible rows unread.`;
			}
		},
		{
			id: 'S2', title: 'Right-click the section header to open its context menu',
			async run({ page }) {
				await page.locator(`${LIST} .monaco-list-row`, { hasText: 'BS Mock Host' }).first().click({ button: 'right' });
				const item = page.locator('.monaco-menu .action-item', { hasText: 'Mark All as Read' }).first();
				await item.waitFor({ state: 'visible', timeout: 5000 });
				await page.evaluate(() => {
					window.__bsLong = [];
					new PerformanceObserver(l => window.__bsLong.push(...l.getEntries().map(e => Math.round(e.duration)))).observe({ type: 'longtask' });
				});
				return 'The context menu shows "Mark All as Read".';
			}
		},
		{
			id: 'S3', title: `Mark All as Read: no renderer block over ${THRESHOLD_MS} ms`,
			async run({ page }) {
				await startTimer(page, 'Mark All as Read clicked');
				const t0 = Date.now();
				await page.locator('.monaco-menu .action-item', { hasText: 'Mark All as Read' }).first().click();
				let doneMs;
				while (Date.now() - t0 < 30000) {
					if (await unreadVisible(page) === 0) { doneMs = Date.now() - t0; break; }
					await page.waitForTimeout(20);
				}
				await page.waitForTimeout(1500);
				const longs = await page.evaluate(() => window.__bsLong.slice());
				const worst = Math.max(0, ...longs);
				await setOverlay(page, `rows read after ${doneMs ?? '>30000'} ms\nlongest renderer block ${worst} ms\n(limit ${THRESHOLD_MS} ms)`);
				const detail = `Rows read after ${doneMs ?? '>30000'} ms; long tasks ${JSON.stringify(longs)}; longest ${worst} ms (limit ${THRESHOLD_MS} ms).`;
				if (doneMs === undefined) { throw new Error('Visible rows were still unread after 30 s. ' + detail); }
				if (worst > THRESHOLD_MS) { throw new Error('Renderer blocked too long. ' + detail); }
				return detail;
			}
		},
	]
};
Synthetic agent host used for the measurements (mock-host.mjs)
// Synthetic agent host seeding N unread sessions for sessions-list scale testing (uses the test ScriptedMockAgent).
// Usage, from the repo root after a compile: node mock-host.mjs <port> <N>  (stdin must stay open)
// Env: MOCK_HOST_OUT (out dir), MOCK_HOST_UDD (host user data dir).
import { pathToFileURL } from 'url';
import * as path from 'path';

const OUT = process.env.MOCK_HOST_OUT ?? path.resolve('out');
const port = process.argv[2] ?? '8991';
const N = Number(process.argv[3] ?? 100);
const imp = p => import(pathToFileURL(path.join(OUT, p)).href);
const { ScriptedMockAgent } = await imp('vs/platform/agentHost/test/node/mockAgent.js');
const { AgentSession } = await imp('vs/platform/agentHost/common/agent.js');
const { URI } = await imp('vs/base/common/uri.js');

const ts = () => new Date().toISOString().slice(11, 23);
const WORDS = ['Refactor', 'Fix', 'Investigate', 'Add', 'Remove', 'Update', 'Optimize', 'Document', 'Review', 'Migrate'];
const NOUNS = ['parser', 'tokenizer', 'session list', 'login flow', 'cache layer', 'settings editor', 'test suite', 'build script', 'telemetry', 'theme colors', 'terminal', 'debugger'];
const now = Date.now();
const meta = new Map();
for (let i = 0; i < N; i++) {
	const uri = URI.parse(`mock:/bs-s${String(i).padStart(5, '0')}`);
	const title = `${WORDS[i % WORDS.length]} ${NOUNS[(i * 7) % NOUNS.length]} #${i}`;
	// spread over ~120 days so date grouping has many buckets
	const modified = now - Math.floor(i * (120 * 24 * 3600 * 1000) / Math.max(1, N)) - 60000;
	meta.set(uri.toString(), { uri, title, modified });
}
const titleFor = s => meta.get(s.toString());
const withMeta = list => list.map(m => {
	const t = titleFor(m.session);
	return t ? { ...m, summary: t.title, startTime: t.modified - 3600000, modifiedTime: t.modified } : m;
});

const counters = {};
const count = k => { counters[k] = (counters[k] ?? 0) + 1; };
setInterval(() => process.stderr.write(`[mock ${ts()}] counters ${JSON.stringify(counters)}\n`), 5000).unref?.();

const proto = ScriptedMockAgent.prototype;
for (const k of ['listSessions', 'listExternalChats']) {
	const o = proto[k];
	proto[k] = async function (...a) { count(k); return withMeta(await o.apply(this, a)); };
}
const origMeta = proto.getSessionMetadata;
proto.getSessionMetadata = async function (session) {
	count('getSessionMetadata');
	const m = await origMeta.call(this, session);
	return m ? withMeta([m])[0] : m;
};
const origChatMeta = proto.getChatMetadata;
proto.getChatMetadata = async function (chat, ctx) {
	count('getChatMetadata');
	const r = await origChatMeta.call(this, chat, ctx);
	if (!r) { return r; }
	const owner = ctx && typeof ctx === 'object' && ctx.owner ? ctx.owner.get() : ctx;
	const t = owner ? titleFor(owner) : undefined;
	return t ? { ...r, summary: t.title, startTime: t.modified - 3600000, modifiedTime: t.modified } : r;
};
const origMsgs = proto.getSessionMessages;
proto.getSessionMessages = async function (session) { count('getSessionMessages'); return origMsgs.call(this, session); };

const origFireDisc = proto.fireDiscoveredChats;
proto.fireDiscoveredChats = function (chats) {
	count('fireDiscoveredChats');
	process.stderr.write(`[mock ${ts()}] fireDiscoveredChats ${chats.length} (forced non-external)\n`);
	return origFireDisc.call(this, chats.map(m => ({ ...m, external: false })));
};
const origDesc = proto.getDescriptor;
proto.getDescriptor = function () {
	if (!this.__bsSeeded) {
		this.__bsSeeded = true;
		for (const { uri } of meta.values()) {
			this._sessions.set(AgentSession.id(uri), uri);
		}
		process.stderr.write(`[mock ${ts()}] seeded ${N} sessions\n`);
		setTimeout(() => {
			if (counters.fireDiscoveredChats) { process.stderr.write(`[mock ${ts()}] skip refire\n`); return; }
			void this.listExternalChats().then(chats => {
				process.stderr.write(`[mock ${ts()}] firing discovery for ${chats.length} chats (non-external)\n`);
				this.fireDiscoveredChats(chats.map(m => ({ ...m, external: false })));
			});
		}, 1000);
	}
	return origDesc.call(this);
};

// Count protocol requests from clients.
const psh = await imp('vs/platform/agentHost/node/protocolServerHandler.js');
const origReq = psh.ProtocolServerHandler.prototype._handleRequest;
if (origReq) {
	psh.ProtocolServerHandler.prototype._handleRequest = function (client, method, params, id) {
		count('req:' + method);
		if (process.env.MOCK_HOST_TRACE === '1') { process.stderr.write(`[mock ${ts()}] req ${method} ${JSON.stringify(params ?? {}).slice(0, 120)}\n`); }
		return origReq.call(this, client, method, params, id);
	};
} else {
	process.stderr.write('[mock] no _handleRequest to wrap\n');
}

const udd = process.env.MOCK_HOST_UDD ?? path.join(process.env.TEMP ?? '.', `mock-agent-host-udd-${N}`);
process.argv = [process.argv[0], process.argv[1], '--port', String(port), '--host', '127.0.0.1', '--without-connection-token', '--enable-mock-agent', '--quiet', '--user-data-dir', udd];
await imp('vs/platform/agentHost/node/agentHostServerMain.js');

Validation

Check Command Result
Repro on main @ 1e81fc7, 500 sessions cdp-scenario.mjs --scenario repro.cjs (1 warm-up + 3 runs) ❌ Fails as expected in all 4 runs. The longest renderer task was 3399–3616 ms, over the 1000 ms limit, and rows turned read after 3502–3712 ms.
Repro with this PR, 500 sessions same ✅ Passes in all 4 runs. No renderer task was over 50 ms, and rows turned read after 183–214 ms.
Same scenario, 100 sessions (only the minimum section size changed to 100) same, 1 warm-up + 3 runs per build main: the longest task was 376–427 ms, under the 1000 ms limit but still a visible stall. This PR: no task over 50 ms, and rows turned read after 113–136 ms.
New tests against main sources scripts\test.bat --run <sessionsManagementService.test.ts> --run <localAgentHostSessionsProvider.test.ts> --grep "markAllRead hands each provider its sessions in one call|marking several sessions read publishes one session change" ❌ 3 failing as expected. markAllRead made 4 per-session calls instead of 1 batch call plus 2 fallback calls, and the provider had no batch method.
New tests with this PR same ✅ 3 passing. One change event covers 3 sessions on both the current and legacy protocols, each session is still dispatched to the host, and a follow-up single-session Mark as Unread fires its own event.
Targeted suites (sessions management, agent host provider, sessions list, sessions list context menu) scripts\test.bat --run src\vs\sessions\services\sessions\test\browser\sessionsManagementService.test.ts --run src\vs\sessions\contrib\providers\agentHost\test\browser\localAgentHostSessionsProvider.test.ts --run src\vs\sessions\contrib\sessions\test\browser\sessionsList.test.ts --run src\vs\sessions\contrib\sessions\test\browser\sessionsListContextMenu.test.ts ✅ 1081 passing and 14 pending. 1 test failed, and it is unrelated to this change: SessionsList comparison groups renders synthesis and Judge before connected compact attempts expects 1px connector strokes and gets 0.8px on this 125% display-scale machine. It fails the same way on main: scripts\test.bat --run src\vs\sessions\contrib\sessions\test\browser\sessionsList.test.ts --grep "renders synthesis and Judge before connected compact attempts".
Correctness controls in the fixed build (100 sessions) cdp-scenario.mjs --scenario control.cjs ✅ Mark All as Read turned every visible row read: 5 unread rows went to 0 in 46 ms. ✅ Mark as Unread on one session updated its row in 49 ms, and Mark as Read changed it back in 27 ms. ⚠️ I could not verify in the UI that read state survives a reload, because the page reload over the DevTools protocol never finished in the test harness. The unit tests do confirm that each session's read state is still sent to the host.
Type check npm run typecheck-client ✅ passed
Lint npm run eslint -- <5 changed files> ✅ passed

Found and fixed with automated exploratory testing. Draft until reviewed by a human.

Copilot AI balanced review requested due to automatic review settings October 17, 2026 08:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused change preserves existing read-state behavior, adds targeted regression coverage, and has no unresolved findings.

No open issues 🎉

What changed in this PR

Reduces Agents window freezes during Mark All as Read by batching provider notifications instead of refreshing listeners once per session.

Changes:

  • Adds optional provider-level batch read-state updates with a per-session fallback.
  • Merges agent-host session change events while preserving existing read-state operations.
  • Adds regression coverage for provider routing, current and legacy protocols, and immediate single-session updates.

No actionable findings remain. Tests were not rerun during this review.

File Description
src/​vs/​sessions/​services/​sessions/​test/​browser/​sessionsManagementService.test.ts Tests batch routing and fallback calls.
src/​vs/​sessions/​services/​sessions/​common/​sessionsProvider.ts Defines the optional batch capability.
src/​vs/​sessions/​services/​sessions/​browser/​sessionsManagementService.ts Groups read-state updates by provider.
src/​vs/​sessions/​contrib/​providers/​agentHost/​test/​browser/​localAgentHostSessionsProvider.test.ts Tests merged notifications and protocol compatibility.
src/​vs/​sessions/​contrib/​providers/​agentHost/​browser/​baseAgentHostSessionsProvider.ts Batches notifications using a pauseable emitter.

Review effort: Balanced

Mark All as Read set each session's read state separately, and the agent
host provider published one session change per session. The sessions list
and other listeners rebuild on every change, so marking N sessions did
O(N^2) work in one renderer task (about 3.4 s for 500 sessions).

markAllRead now hands each provider its sessions in one call. The agent
host provider applies them while its session change emitter is paused and
publishes one merged change. Providers without the batch method still get
one call per session.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@karthiknadig
Karthik Nadig (karthiknadig) force-pushed the karthiknadig/bug-swarm/c9-f1-mark-all-read-freeze branch from 7ffe52f to 47c9f4d Compare October 18, 2026 05:11

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants