From c3a201bf914e6c8da7c1fd28b87656d6370724a8 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:23:28 -0700 Subject: [PATCH] fix(worktrees): keep the shared snapshot out of provider internals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The snapshot proxy passed itself as the Reflect.get receiver, so prototype methods invoked through it ran with `this` bound to the proxy. A provider whose own shutdown() re-read state via `this.listProcesses()` would then silently get this sweep's cached snapshot instead of the live host — batching leaking past the calls it was built for. Bind non-listProcesses members to the target so only the sweep's own calls share the snapshot. No shipped provider does this today; the point is that adding one must not quietly change teardown semantics. Raised by Greptile as an undocumented implicit constraint; closed structurally rather than by comment. Co-authored-by: Orca --- ...g-worktree-terminal-reconciliation.test.ts | 31 +++++++++++++++++++ ...issing-worktree-terminal-reconciliation.ts | 9 ++++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/main/runtime/missing-worktree-terminal-reconciliation.test.ts b/src/main/runtime/missing-worktree-terminal-reconciliation.test.ts index 1f9f74f4e18..f218f3a2c4f 100644 --- a/src/main/runtime/missing-worktree-terminal-reconciliation.test.ts +++ b/src/main/runtime/missing-worktree-terminal-reconciliation.test.ts @@ -154,6 +154,37 @@ describe('stopMissingWorktreeTerminals', () => { expect(provider.shutdownCalls).toHaveLength(ids.length) }) + // Why: the batching must not leak past the calls it was built for. If the proxy + // were the receiver, a provider whose own method called `this.listProcesses()` + // would silently read this sweep's cached snapshot instead of the live host. + it('does not serve the shared snapshot to provider-internal listProcesses', async () => { + class SelfListingProvider { + listCalls = 0 + constructor(private readonly sessions: { id: string; cwd: string; title: string }[]) {} + async listProcesses(): Promise<{ id: string; cwd: string; title: string }[]> { + this.listCalls += 1 + return this.sessions + } + async shutdown(): Promise { + // A provider that re-reads live state as part of stopping. + await this.listProcesses() + } + } + const ids = ['repo-1::/workspace/a', 'repo-1::/workspace/b'] + const provider = new SelfListingProvider( + ids.map((id) => ({ id: `${id}@@session`, cwd: '/workspace', title: 'shell' })) + ) + + await stopMissingWorktreeTerminals({ ...localRepo, connectionId: 'ssh-1' }, ids, [], { + runtime: createRuntime(), + getLocalProvider: () => null, + getSshProvider: () => provider as unknown as IPtyProvider + }) + + // One shared sweep scan, plus each shutdown's own live re-read. + expect(provider.listCalls).toBe(1 + ids.length) + }) + // Why: caching a rejected scan would let one transient failure suppress // teardown for every remaining worktree in the sweep. it('does not reuse a failed process scan', async () => { diff --git a/src/main/runtime/missing-worktree-terminal-reconciliation.ts b/src/main/runtime/missing-worktree-terminal-reconciliation.ts index d886b1cbcee..b25c4fc9f83 100644 --- a/src/main/runtime/missing-worktree-terminal-reconciliation.ts +++ b/src/main/runtime/missing-worktree-terminal-reconciliation.ts @@ -18,9 +18,14 @@ const MISSING_WORKTREE_TEARDOWN_CONCURRENCY = 4 function withSharedProcessSnapshot(provider: IPtyProvider): IPtyProvider { let snapshot: Promise>> | null = null return new Proxy(provider, { - get(target, property, receiver) { + get(target, property) { if (property !== 'listProcesses') { - return Reflect.get(target, property, receiver) + // Why: bind other members to `target`, not the proxy. With the proxy as + // receiver, a provider whose own method called `this.listProcesses()` + // would silently read this sweep's cached snapshot instead of the live + // host — the batching must not leak past the calls it was built for. + const member: unknown = Reflect.get(target, property) + return typeof member === 'function' ? member.bind(target) : member } return async (opts?: { deadlineMs?: number }) => { const pending = (snapshot ??= target.listProcesses(opts))