From b270b3ef9f816c97facc5025bc535667ccf7e396 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:18:34 +0800 Subject: [PATCH] fix(web): run a trailing catalog refresh for coalesced membership changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `refreshSubagents` single-flights per catalog owner: a request arriving while a pull is in flight returns the in-flight promise and is silently coalesced into it. The in-flight response was requested before the triggering change, so it can never contain that change — a debounced membership refresh (50ms after `host/session-added`) firing during a slow pull therefore lost the new child, and the catalog stayed stale until an unrelated trigger (reselection, menu reopen, reconnect). Mark the owner stale on coalescing and re-arm one trailing pull in the settlement `finally`, so every membership change observed during a pull is carried by a follow-up refresh exactly once. Bounded: the trailing pull only runs when a refresh request was actually coalesced, and a new coalescing during the trailing pull re-marks the same set. Adds a fake-timer regression test: a `host/session-added` debounce firing mid-pull yields exactly two `subagent.list` calls and the catalog eventually contains the new child. --- .../runtime/src/client/sessions/manager.ts | 17 +++++- packages/client/runtime/tests/manager.spec.ts | 58 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/packages/client/runtime/src/client/sessions/manager.ts b/packages/client/runtime/src/client/sessions/manager.ts index 1019327df5..3b841ae0b2 100644 --- a/packages/client/runtime/src/client/sessions/manager.ts +++ b/packages/client/runtime/src/client/sessions/manager.ts @@ -101,6 +101,8 @@ export class SessionManager { private readonly addresses = new Map() private readonly catalogs = new Map() private readonly catalogInflight = new Map() + /** Catalog owners whose membership changed while a pull was in flight: one trailing refresh after it settles. */ + private readonly catalogStale = new Set() private readonly openCatalogs = new Set() private readonly catalogDebounce = new Map>() @@ -286,7 +288,16 @@ export class SessionManager { */ refreshSubagents(parentSessionId: SessionId): Promise { const existing = this.catalogInflight.get(parentSessionId) - if (existing !== undefined) return existing.promise + if (existing !== undefined) { + // A refresh requested while a pull is in flight must not be silently + // coalesced into it: the in-flight response was requested before the + // triggering change (a membership frame or an opened menu), so it can + // never contain that change. Queue one trailing refresh that runs after + // the pull settles; without it the change stays invisible until an + // unrelated later trigger (reselection, menu reopen, reconnect). + this.catalogStale.add(parentSessionId) + return existing.promise + } const previous = this.catalogs.get(parentSessionId) const expandableRows = new Set() const activityRows = new Map() @@ -333,6 +344,10 @@ export class SessionManager { }) } finally { this.catalogInflight.delete(parentSessionId) + // Re-arm the trailing pull before the dirty notify: the response the + // caller observed predates the stale-marking change, so the follow-up + // refresh is the only carrier of that change. + if (this.catalogStale.delete(parentSessionId)) void this.refreshSubagents(parentSessionId) this.notifier.markDirty() } })() diff --git a/packages/client/runtime/tests/manager.spec.ts b/packages/client/runtime/tests/manager.spec.ts index 6ada34de7f..2c1b8c259d 100644 --- a/packages/client/runtime/tests/manager.spec.ts +++ b/packages/client/runtime/tests/manager.spec.ts @@ -529,6 +529,64 @@ describe('subagent catalogs', () => { { kind: 'child', id: S2, activity: 'inactive' }, ]) }) + + it('runs one trailing catalog refresh for a membership change coalesced into an in-flight pull', async () => { + vi.useFakeTimers() + try { + const api = new FakeApiClient() + const root = 'fk-root' as SessionId + const first = deferred>>() + const second = deferred>>() + api.onSubagentList = () => first.promise + const manager = new SessionManager(api) + manager.setSubagentCatalogOpen(root, true) + const refresh = manager.refreshSubagents(root) + + // A membership frame arrives while the pull is in flight; the debounced + // refresh it schedules fires 50ms later and is coalesced into the pull — + // which was requested before the new child existed. The stale mark must + // queue one trailing pull carrying the change. + manager.handleHostEnvelope({ + rpcId: 'child-added' as never, + payload: { + type: 'host/session-added', sessionId: S2, parentSessionId: root, blank: false, + }, + }) + await vi.advanceTimersByTimeAsync(50) + api.onSubagentList = () => second.promise + first.resolve(ok({ + entries: [{ + kind: 'child', id: S1, mode: 'continuable', label: 'older', + activity: 'inactive', hasChildren: false, + }] as never[], + parentAvailable: true, + })) + await refresh + // The trailing pull is already in flight (kicked synchronously in finally). + second.resolve(ok({ + entries: [ + { + kind: 'child', id: S1, mode: 'continuable', label: 'older', + activity: 'inactive', hasChildren: false, + }, + { + kind: 'child', id: S2, mode: 'continuable', label: 'new child', + activity: 'inactive', hasChildren: false, + }, + ] as never[], + parentAvailable: true, + })) + await second.promise + + expect(api.callsOf('subagent.list')).toHaveLength(2) + expect(manager.getListSnapshot().subagentsByParent[root]?.entries).toMatchObject([ + { kind: 'child', id: S1, label: 'older' }, + { kind: 'child', id: S2, label: 'new child' }, + ]) + } finally { + vi.useRealTimers() + } + }) }) describe('remaining branches', () => {