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.
This commit is contained in:
1 parent
ac71b42560
commit
67a4ab78be
2 files changed
+28
-12
No files matched your search
@@ -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<Uint8Array> { 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()
|
||||
})
|
||||
|
||||
|
||||
@@ -57,7 +57,10 @@ export class FileUploads extends TypertRemoteService {
|
||||
static inject = ['agents', 'attachments', 'commands', 'connection']
|
||||
|
||||
private readonly stagedFiles = new WeakMap<Session, Map<FileUploadReceiptId, StagedFileUpload>>()
|
||||
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<Agent> {
|
||||
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 })
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user