From 67a4ab78be8a4c57da71a8defc3a41cecb1fc4dd Mon Sep 17 00:00:00 2001 From: turtle1999 Date: Mon, 19 Oct 2026 16:59:18 +0800 Subject: [PATCH] fix(file-upload): release the Agent resolver when its owner reloads session-controller registers its resolver through `ctx.fileUploads`. Inside that call Cordis returns a new proxy for every function-valued property read from `this`, so the disposer's identity check never matched and the slot was never cleared. Any restart of session-controller, such as a profile patch that changes session-query-sqlite, then failed with "Agent resolver is already registered" and left every sessionController dependent pending. Store the resolver in a plain holder object, as the commands receipt resolver already does, and test registration and disposal through a plugin fiber. --- .../tests/commands-upload-file.host.spec.ts | 27 ++++++++++++++----- packages/client/file-upload/src/index.ts | 13 +++++---- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/api/session-controller/tests/commands-upload-file.host.spec.ts b/packages/api/session-controller/tests/commands-upload-file.host.spec.ts index aa7422918e..48ba896f01 100644 --- a/packages/api/session-controller/tests/commands-upload-file.host.spec.ts +++ b/packages/api/session-controller/tests/commands-upload-file.host.spec.ts @@ -1,4 +1,4 @@ -import { Context } from '@deepseek-ai/cordis' +import { Context, FiberState } from '@deepseek-ai/cordis' import AgentRegistry from '@deepseek-ai/dsh-agent' import SessionProjectionRegistry from '@deepseek-ai/dsh-session-projection' import { createInboxStub } from '@deepseek-ai/dsh-agent-loop-testkit' @@ -240,26 +240,39 @@ describe('Session file uploads', () => { await expect(uploading).rejects.toMatchObject({ code: 'session/not-found' }) }) - it('resolves a cold ordinary Agent and releases the resolver registration', async () => { + it('resolves a cold ordinary Agent and releases the resolver with its registering fiber', async () => { const { ctx, uploads, agent, disposeAgent } = await uploadHarness() await disposeAgent() const resolveAgent = vi.fn(async () => { await ctx.agents.register(agent) return agent }) - const disposeResolver = uploads.registerAgentResolver(resolveAgent) - expect(() => { uploads.registerAgentResolver(resolveAgent) }).toThrow('already registered') + const resolverOwner = { + name: 'resolver-owner', + inject: ['fileUploads'], + apply: (owner: Context) => { + owner.effect(() => owner.fileUploads.registerAgentResolver(resolveAgent), 'test: Agent resolver') + }, + } + const first = ctx.plugin(resolverOwner) + await first + expect(() => { ctx.fileUploads.registerAgentResolver(resolveAgent) }).toThrow('already registered') await expect(uploads.uploadStream({ sessionId: SESSION, data: (async function* (): AsyncIterable { yield Uint8Array.of(1) })(), })).resolves.toMatchObject({ file: { bytes: 1 } }) expect(resolveAgent).toHaveBeenCalledWith(SESSION) + await first.dispose() + const second = ctx.plugin(resolverOwner) + await second + expect(second.state).toBe(FiberState.ACTIVE) + await second.dispose() + const disposeResolver = ctx.fileUploads.registerAgentResolver(resolveAgent) disposeResolver() const replacement = vi.fn(async () => agent) - const disposeReplacement = uploads.registerAgentResolver(replacement) + const disposeReplacement = ctx.fileUploads.registerAgentResolver(replacement) disposeResolver() - expect(() => { uploads.registerAgentResolver(replacement) }).toThrow('already registered') - expect(disposeReplacement).toBeTypeOf('function') + expect(() => { ctx.fileUploads.registerAgentResolver(replacement) }).toThrow('already registered') disposeReplacement() }) diff --git a/packages/client/file-upload/src/index.ts b/packages/client/file-upload/src/index.ts index a784096461..acf99f22d2 100644 --- a/packages/client/file-upload/src/index.ts +++ b/packages/client/file-upload/src/index.ts @@ -57,7 +57,10 @@ export class FileUploads extends TypertRemoteService { static inject = ['agents', 'attachments', 'commands', 'connection'] private readonly stagedFiles = new WeakMap>() - private agentResolver: AgentResolver | undefined + // Inside a method called through `ctx.fileUploads`, Cordis returns a new proxy + // for every function-valued property read from `this`, so `===` against the + // registered resolver would always be false; a plain object is returned as is. + private readonly agentResolution: { resolver: AgentResolver | undefined } = { resolver: undefined } /** @param ctx - Host context carrying Agent, attachment, command, and Connection services. */ constructor(ctx: Context) { @@ -87,10 +90,10 @@ export class FileUploads extends TypertRemoteService { * @returns disposer removing this resolver. */ registerAgentResolver(resolve: AgentResolver): () => void { - if (this.agentResolver !== undefined) throw new Error('file-upload: Agent resolver is already registered') - this.agentResolver = resolve + if (this.agentResolution.resolver !== undefined) throw new Error('file-upload: Agent resolver is already registered') + this.agentResolution.resolver = resolve return () => { - if (this.agentResolver === resolve) this.agentResolver = undefined + if (this.agentResolution.resolver === resolve) this.agentResolution.resolver = undefined } } @@ -212,7 +215,7 @@ export class FileUploads extends TypertRemoteService { private async resolveAgent(sessionId: SessionId): Promise { const live = this.ctx.agents.get(sessionId) if (live !== undefined) return live - const resolver = this.agentResolver + const resolver = this.agentResolution.resolver if (resolver === undefined) { throw new RemoteError('session/not-found', `session "${sessionId}" is not attached`, { sessionId }) }