From 7c5133488a1fb8ca403796f86595fbcb50a3da70 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 9 Jul 2026 13:05:44 +0800 Subject: [PATCH] refactor(core): every registry register-method returns the exact effect disposer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exact-disposer fix (5fbac8be B1) repaired agents.register but left the same wrapper (return () => void dispose()) at seven sibling sites: tools.register, tools.restrict, systemPrompt.section/tools/variable, agents.setFactory, and subagents.registerProvider. A wrapper makes correct composite usage unrepresentable — the exact disposer cannot be recovered, so a generator effect yielding it leaves the inner effect disposing as a CONCURRENT SIBLING on owner unload, silently reproducing B1's ordering corruption. The exact disposer serves both usages (composite-nestable AND fire-and-forget callable); all seven now return it, typed () => Promise | void, with the convention pinned by a discriminating test: an async-link composite probe that passes with the exact disposer and observes the sibling unregistration firing mid-drain with a wrapper. Re-auditing also surfaced that B1 itself SHIPPED a full-lint failure: it changed register()'s return type without updating cross-file consumers (agent.spec.ts dispose() statements, tool-bash's disposer list), which the staged-scoped pre-commit lint never saw — pnpm run lint was red at HEAD. Those three sites and this change's own fallout are fixed together: tests now await disposers (stronger — they observe the full unwind), sync paths void them, and the two annotation sites carry the honest union type. agents.register's README line had drifted the same way (B1 updated the JSDoc, not the README) — all seven README signatures now match; services catalog regenerated. --- docs/cordis-catalog/services.md | 14 +++--- packages/bash/tool-bash/tests/tools.spec.ts | 4 +- packages/core/agent/README.md | 4 +- packages/core/agent/src/index.ts | 12 +++-- packages/core/agent/tests/agent.spec.ts | 6 +-- packages/core/system-prompt/README.md | 6 +-- packages/core/system-prompt/src/index.ts | 48 +++++++++++++------ .../core/system-prompt/tests/scoped.spec.ts | 2 +- .../system-prompt/tests/system-prompt.spec.ts | 8 ++-- packages/core/tools/README.md | 4 +- packages/core/tools/src/index.ts | 32 +++++++++---- packages/core/tools/tests/scoped.spec.ts | 2 +- packages/core/tools/tests/tools.spec.ts | 33 ++++++++++++- .../tests/structured.spec.ts | 2 +- packages/subagent/subagent/src/index.ts | 16 +++++-- .../subagent/subagent/tests/service.spec.ts | 8 ++-- packages/subagent/tool-subagent/src/index.ts | 4 +- 17 files changed, 138 insertions(+), 67 deletions(-) diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index 3dc09d3fa3..783ef7c83e 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -28,7 +28,7 @@ Source: [`packages/core/agent-loop/src/index.ts:70`](../../packages/core/agent-l Agent registry (`ctx.agents`): tracks live agents so UI, hook, and orchestrator plugins can find them without depending on the concrete loop package. Agent *creation* is provided by whichever plugin implements the AgentFactory (phase 1: `@deepseek-ai/dsh-agent-loop`), registered via setFactory. ```ts cordis-catalog -setFactory(factory: AgentFactory): () => void +setFactory(factory: AgentFactory): () => Promise | void create(options: CreateAgentOptions): AgentHandle async resume(options: ResumeAgentOptions): Promise register(agent: Agent): () => Promise | void @@ -191,7 +191,7 @@ Source: [`packages/core/session/src/index.ts:427`](../../packages/core/session/s The `subagents` service: a registry of named SubagentProviders and a capability-checked start surface. ```ts cordis-catalog -registerProvider(provider: SubagentProvider): () => void +registerProvider(provider: SubagentProvider): () => Promise | void getProvider(name: string): SubagentProvider | undefined list(): string[] start(name: string, request: SubagentStartRequest): SubagentRun @@ -204,9 +204,9 @@ Source: [`packages/subagent/subagent/src/index.ts:153`](../../packages/subagent/ Registry service (`ctx.systemPrompt`): plugins contribute ordered text sections, tool-schema providers, and named prompt variables; the agent loop calls `assemble(context)` once per step. Registers the harness-owned `harness:identity` and `deployment:persona` sections itself (see Config.persona). ```ts cordis-catalog -section(section: PromptSection): () => void -tools(provider: (context: AssembleContext) => ToolProviderResult): () => void -variable(name: string, provider: (context: AssembleContext) => string | undefined): () => void +section(section: PromptSection): () => Promise | void +tools(provider: (context: AssembleContext) => ToolProviderResult): () => Promise | void +variable(name: string, provider: (context: AssembleContext) => string | undefined): () => Promise | void async assemble(context: AssembleContext = {}): Promise ``` @@ -219,8 +219,8 @@ Tool registry (`ctx.tools`): tool plugins register definitions; the agent loop e Two registration layers (`@deepseek-ai/dsh-scope`): a registration through a plain plugin context is GLOBAL (visible to every agent); one through a scoped context (`agent.ctx`) is filed in that scope's layer — visible to that agent alone, disposed with the scope, and SHADOWING a global tool of the same name for that agent (most-specific-wins; within one layer a duplicate name still throws). restrict masks the global layer per scope. One visibility function (visible) feeds prompt assembly, get, and execute, so what the model is shown, what a presenter renders, and what dispatches can never disagree. ```ts cordis-catalog -register(definition: ToolDefinition): () => void -restrict(filter: ToolRestriction): () => void +register(definition: ToolDefinition): () => Promise | void +restrict(filter: ToolRestriction): () => Promise | void visible(scope?: ScopeKey): ToolDefinition[] get(name: string, scope?: ScopeKey): ToolDefinition | undefined schemas(scope?: ScopeKey): ToolSchema[] diff --git a/packages/bash/tool-bash/tests/tools.spec.ts b/packages/bash/tool-bash/tests/tools.spec.ts index 7d4b34f74f..6299aea7d3 100644 --- a/packages/bash/tool-bash/tests/tools.spec.ts +++ b/packages/bash/tool-bash/tests/tools.spec.ts @@ -35,7 +35,7 @@ async function setup() { * The registration disposer is tracked so {@link unregisterFakeAgents} can drop * it (simulating the owning session disconnecting before a task completes). */ -const fakeAgentDisposers = new Map void)[]>() +const fakeAgentDisposers = new Map Promise | void)[]>() function registerFakeAgent(ctx: Context, sessionId: string, inject: (...args: unknown[]) => void): Agent { // The registry KEY (agent.id) is deliberately DIFFERENT from the session // token (session.header.id) — a config agent has `agentId !== sessionId`. The @@ -53,7 +53,7 @@ function registerFakeAgent(ctx: Context, sessionId: string, inject: (...args: un /** Unregister every fake agent in this ctx (simulate the owning session disconnecting). */ function unregisterFakeAgents(ctx: Context): void { - for (const dispose of fakeAgentDisposers.get(ctx) ?? []) dispose() + for (const dispose of fakeAgentDisposers.get(ctx) ?? []) void dispose() fakeAgentDisposers.delete(ctx) } diff --git a/packages/core/agent/README.md b/packages/core/agent/README.md index e0668ef761..5c7b723739 100644 --- a/packages/core/agent/README.md +++ b/packages/core/agent/README.md @@ -10,7 +10,7 @@ Tracks live agents so UI, hook, and orchestrator plugins can find them without i The scoped-registration surface: `Agent.ctx` is the agent's scope context (`dsh-scope`, key = the agent) — register tools/sections/variables/listeners through it for that agent alone, all unwound on disposal. `agentEvents(ctx, agent)` is the fused dispatcher every agent-subject event goes through (carrier + injected subject in one move); `assembleContextFor(agent)` builds the per-agent assembly context (`agent` + `scope` together). `CreateAgentOptions.setup(agentCtx)` composes a child's scoped world at creation — setup registers, it never drives. -- `ctx.agents.register(agent: Agent): () => void` — record an **already-constructed** agent. Disposed with the calling fiber. +- `ctx.agents.register(agent: Agent): () => Promise | void` — record an **already-constructed** agent. Disposed with the calling fiber. - `ctx.agents.get(id: AgentId): Agent | undefined` - `ctx.agents.list(): Agent[]` @@ -18,7 +18,7 @@ The scoped-registration surface: `Agent.ctx` is the agent's scope context (`dsh- Agent *creation* is provided by whichever plugin implements `AgentFactory` (phase 1: `dsh-agent-loop`), registered via `setFactory`. This keeps creation on the `dsh-agent` interface so consumers (UI, the ACP bridge) program against `ctx.agents` without depending on the concrete loop package. -- `ctx.agents.setFactory(factory: AgentFactory): () => void` — register the creation factory (the loop calls this on construction). Throws on a second factory; the slot clears on dispose. +- `ctx.agents.setFactory(factory: AgentFactory): () => Promise | void` — register the creation factory (the loop calls this on construction). Throws on a second factory; the slot clears on dispose. - `ctx.agents.create(options: CreateAgentOptions): AgentHandle` — construct, start, AND register a new agent on a caller-supplied `sessionId` (with optional `meta.cwd`/`meta.parentSession`/`meta.seedLength` and optional `seed` events for forked children). Distinct from `register` (which only records). Throws if no factory is registered. - `ctx.agents.resume(options: ResumeAgentOptions): Promise` — load a persisted session ([session persistence](../../../docs/rfc/implemented/architecture/2026-06-14-session-persistence.md)) and resume an agent on it. Async; rejects if no factory is registered, or if the factory finds session persistence unconfigured. diff --git a/packages/core/agent/src/index.ts b/packages/core/agent/src/index.ts index 3821acd442..588a3c8ee1 100644 --- a/packages/core/agent/src/index.ts +++ b/packages/core/agent/src/index.ts @@ -162,15 +162,21 @@ export class AgentRegistry extends Service { * effect-scoped). Throws if a factory is already registered. Returns the * disposer; on dispose the factory slot is cleared. * @param factory - the loop-owned factory {@link create}/{@link resume} delegate to. - * @returns the disposer that clears the factory slot. + * @returns the disposer that clears the factory slot. The exact + * Cordis effect disposer (single-shot): composite (generator) effects may + * yield it directly — exact identity nests the teardown in order. */ - setFactory(factory: AgentFactory): () => void { + setFactory(factory: AgentFactory): () => Promise | void { const dispose = this.ctx.effect(() => { if (this.factory !== undefined) throw new Error('an agent factory is already registered') this.factory = factory return () => { this.factory = undefined } }, 'agents.setFactory()') - return () => void dispose() + // The exact cordis effect disposer (the agents.register() convention): a + // caller's composite effect can yield it for in-order teardown; the + // loop's constructor effect returns it directly, identity-nesting the + // registration under that effect. + return dispose } /** diff --git a/packages/core/agent/tests/agent.spec.ts b/packages/core/agent/tests/agent.spec.ts index 43b4752f1b..08623549ec 100644 --- a/packages/core/agent/tests/agent.spec.ts +++ b/packages/core/agent/tests/agent.spec.ts @@ -37,7 +37,7 @@ describe('AgentRegistry', () => { expect(ctx.agents.get(AgentId('a1'))).toBe(agent) expect(ctx.agents.list()).toEqual([agent]) - dispose() + await dispose() expect(disposed).toEqual(['a1']) expect(ctx.agents.get(AgentId('a1'))).toBeUndefined() }) @@ -74,7 +74,7 @@ describe('AgentRegistry', () => { // tracked exactly once (the duplicate-id check is not wedged). const dispose = ctx.agents.register(stubAgent('main')) expect(ctx.agents.list().map(a => a.id)).toEqual(['main']) - dispose() + await dispose() expect(ctx.agents.get(AgentId('main'))).toBeUndefined() }) }) @@ -128,7 +128,7 @@ describe('AgentRegistry factory seam', () => { it('disposing the setFactory fiber clears the factory (HMR safety)', async () => { const ctx = new Context() await ctx.plugin(AgentRegistry) - let dispose!: () => void + let dispose!: () => Promise | void const fiber = await ctx.plugin(Object.assign((inner: Context) => { dispose = inner.agents.setFactory(stubFactory().factory) }, { inject: ['agents'] })) diff --git a/packages/core/system-prompt/README.md b/packages/core/system-prompt/README.md index b890ad3cd5..bf8c8bc4cc 100644 --- a/packages/core/system-prompt/README.md +++ b/packages/core/system-prompt/README.md @@ -13,9 +13,9 @@ System prompt assembly registry. Plugins contribute ordered text sections, tool- ### Public API -- `ctx.systemPrompt.section(section: PromptSection): () => void` Contribute a section. The layer is the CALLING context's scope: `agent.ctx` contributes to that agent alone, SHADOWING a same-named global section there (the per-agent persona mechanism — a scoped `deployment:persona`). Duplicate names within one layer throw. Disposed with the calling fiber. -- `ctx.systemPrompt.tools(provider: (context: AssembleContext) => ToolProviderResult): () => void` Contribute tool schemas, evaluated at each assembly with that assembly's context. `ToolProviderResult` = `{ schemas, knownNames? }`: `schemas` is the post-restriction visible set for `context.scope`; `knownNames` (defaulting to the schemas' names) is the pre-restriction universe `toolOrder` validates against. A provider must not return a schema named `TOOL_ORDER_REST`. Scoped providers are consulted only for their scope's assemblies. Disposed with the calling fiber. -- `ctx.systemPrompt.variable(name: string, provider: (context) => string | undefined): () => void` Contribute a prompt variable, referenced from section text as `{{name}}`. Scoped variables (via `agent.ctx`) shadow a same-named global for that agent. Duplicate-in-layer or unreferenceable names throw; `undefined` means "no value for this assembly". Disposed with the calling fiber. +- `ctx.systemPrompt.section(section: PromptSection): () => Promise | void` Contribute a section. The layer is the CALLING context's scope: `agent.ctx` contributes to that agent alone, SHADOWING a same-named global section there (the per-agent persona mechanism — a scoped `deployment:persona`). Duplicate names within one layer throw. Disposed with the calling fiber. +- `ctx.systemPrompt.tools(provider: (context: AssembleContext) => ToolProviderResult): () => Promise | void` Contribute tool schemas, evaluated at each assembly with that assembly's context. `ToolProviderResult` = `{ schemas, knownNames? }`: `schemas` is the post-restriction visible set for `context.scope`; `knownNames` (defaulting to the schemas' names) is the pre-restriction universe `toolOrder` validates against. A provider must not return a schema named `TOOL_ORDER_REST`. Scoped providers are consulted only for their scope's assemblies. Disposed with the calling fiber. +- `ctx.systemPrompt.variable(name: string, provider: (context) => string | undefined): () => Promise | void` Contribute a prompt variable, referenced from section text as `{{name}}`. Scoped variables (via `agent.ctx`) shadow a same-named global for that agent. Duplicate-in-layer or unreferenceable names throw; `undefined` means "no value for this assembly". Disposed with the calling fiber. - `ctx.systemPrompt.assemble(context?: AssembleContext): Promise` Assemble the prompt for one caller: the global layer merged with `context.scope`'s layer (scoped shadows global). Runs through the `system-prompt/assemble` waterfall (scope-filtered by `context.scope`). Rejects when a configured `toolOrder` names a tool outside the providers' `knownNames` universe (a restricted-away KNOWN tool is a normal absence), or when a provider returns the reserved rest-entry name. ### Events diff --git a/packages/core/system-prompt/src/index.ts b/packages/core/system-prompt/src/index.ts index 8fa2daf3b0..f89370447f 100644 --- a/packages/core/system-prompt/src/index.ts +++ b/packages/core/system-prompt/src/index.ts @@ -389,9 +389,11 @@ export class SystemPrompt extends Service { * alternative). Removed when the calling fiber is disposed. Emits * `system-prompt/change` on register/unregister. * @param section - the section to contribute (name, order, text or provider). - * @returns the disposer that removes the section. + * @returns the disposer that removes the section. The exact + * Cordis effect disposer (single-shot): composite (generator) effects may + * yield it directly — exact identity nests the teardown in order. */ - section(section: PromptSection): () => void { + section(section: PromptSection): () => Promise | void { const scope = scopeOf(this.ctx) const dispose = this.ctx.effect(function* (this: SystemPrompt) { const layer = scope === undefined @@ -420,9 +422,13 @@ export class SystemPrompt extends Service { } this.ctx.emit('system-prompt/change') }.bind(this), 'systemPrompt.section()') - // ctx.effect's disposer returns Promise; our disposer API is - // synchronous fire-and-forget — discard the (always-resolved) promise. - return () => void dispose() + // The EXACT cordis effect disposer, not a wrapper: a composite (generator) + // effect that owns a teardown ORDER must be able to yield THIS function — + // cordis nests a disposer out of the fiber's concurrent sibling list by + // exact function identity, so a wrapper would silently break the nesting + // (the agents.register() lesson). Fire-and-forget callers may still + // discard the (always-resolved) promise. + return dispose } /** @@ -437,9 +443,11 @@ export class SystemPrompt extends Service { * {@link Config.toolOrder}'s rest entry and rejects the assembly. Emits * `system-prompt/change`. * @param provider - evaluated at every {@link assemble} for fresh schemas. - * @returns the disposer that removes the provider. + * @returns the disposer that removes the provider. The exact + * Cordis effect disposer (single-shot): composite (generator) effects may + * yield it directly — exact identity nests the teardown in order. */ - tools(provider: (context: AssembleContext) => ToolProviderResult): () => void { + tools(provider: (context: AssembleContext) => ToolProviderResult): () => Promise | void { const scope = scopeOf(this.ctx) const dispose = this.ctx.effect(function* (this: SystemPrompt) { const layer = scope === undefined @@ -460,9 +468,13 @@ export class SystemPrompt extends Service { } this.ctx.emit('system-prompt/change') }.bind(this), 'systemPrompt.tools()') - // ctx.effect's disposer returns Promise; our disposer API is - // synchronous fire-and-forget — discard the (always-resolved) promise. - return () => void dispose() + // The EXACT cordis effect disposer, not a wrapper: a composite (generator) + // effect that owns a teardown ORDER must be able to yield THIS function — + // cordis nests a disposer out of the fiber's concurrent sibling list by + // exact function identity, so a wrapper would silently break the nesting + // (the agents.register() lesson). Fire-and-forget callers may still + // discard the (always-resolved) promise. + return dispose } /** @@ -479,9 +491,11 @@ export class SystemPrompt extends Service { * emits `system-prompt/change` on register/unregister. * @param name - the reference name (matches `[a-z][a-z0-9_]*`). * @param provider - evaluated at every {@link assemble} for the value. - * @returns the disposer that removes the variable. + * @returns the disposer that removes the variable. The exact + * Cordis effect disposer (single-shot): composite (generator) effects may + * yield it directly — exact identity nests the teardown in order. */ - variable(name: string, provider: (context: AssembleContext) => string | undefined): () => void { + variable(name: string, provider: (context: AssembleContext) => string | undefined): () => Promise | void { const scope = scopeOf(this.ctx) const dispose = this.ctx.effect(function* (this: SystemPrompt) { if (!VARIABLE_NAME.test(name)) { @@ -508,9 +522,13 @@ export class SystemPrompt extends Service { } this.ctx.emit('system-prompt/change') }.bind(this), 'systemPrompt.variable()') - // ctx.effect's disposer returns Promise; our disposer API is - // synchronous fire-and-forget — discard the (always-resolved) promise. - return () => void dispose() + // The EXACT cordis effect disposer, not a wrapper: a composite (generator) + // effect that owns a teardown ORDER must be able to yield THIS function — + // cordis nests a disposer out of the fiber's concurrent sibling list by + // exact function identity, so a wrapper would silently break the nesting + // (the agents.register() lesson). Fire-and-forget callers may still + // discard the (always-resolved) promise. + return dispose } /** diff --git a/packages/core/system-prompt/tests/scoped.spec.ts b/packages/core/system-prompt/tests/scoped.spec.ts index 99e6b1c113..729ec52b1c 100644 --- a/packages/core/system-prompt/tests/scoped.spec.ts +++ b/packages/core/system-prompt/tests/scoped.spec.ts @@ -104,7 +104,7 @@ describe('scoped tool providers and toolOrder × restriction', () => { const ctx = await mount() const scope = await mintScope(ctx, 'child') const dispose = scope.ctx.systemPrompt.tools(() => ({ schemas: [schema('scoped_tool')] })) - dispose() + await dispose() const after = await ctx.systemPrompt.assemble({ scope: scopeKeyOf(scope) }) expect(after.tools.map(t => t.name)).toEqual([]) // Re-registering through the same scope starts a fresh layer. diff --git a/packages/core/system-prompt/tests/system-prompt.spec.ts b/packages/core/system-prompt/tests/system-prompt.spec.ts index 560a640643..3d4cb9d02c 100644 --- a/packages/core/system-prompt/tests/system-prompt.spec.ts +++ b/packages/core/system-prompt/tests/system-prompt.spec.ts @@ -247,7 +247,7 @@ describe('SystemPrompt', () => { // registration emits change expect(changeCount).toBe(1) - dispose() + await dispose() // disposal emits change again expect(changeCount).toBe(2) }) @@ -272,7 +272,7 @@ describe('SystemPrompt', () => { const dispose = ctx.systemPrompt.section({ name: 'direct', order: 0, text: 'direct section' }) expect(contributed(await ctx.systemPrompt.assemble())).toHaveLength(1) - dispose() + await dispose() expect(contributed(await ctx.systemPrompt.assemble())).toHaveLength(0) }) @@ -283,7 +283,7 @@ describe('SystemPrompt', () => { const dispose = ctx.systemPrompt.tools(() => ({ schemas: [{ name: 'direct-tool', description: '', parameters: {} }] })) expect((await ctx.systemPrompt.assemble()).tools).toHaveLength(1) - dispose() + await dispose() expect((await ctx.systemPrompt.assemble()).tools).toHaveLength(0) }) @@ -301,7 +301,7 @@ describe('SystemPrompt', () => { // A provider returning undefined records "registered but no value here". expect((await ctx.systemPrompt.assemble()).variables).toEqual({ who: undefined }) - dispose() + await dispose() expect(changeCount).toBe(2) expect((await ctx.systemPrompt.assemble()).variables).toEqual({}) }) diff --git a/packages/core/tools/README.md b/packages/core/tools/README.md index b42087b5d6..e5e953bc84 100644 --- a/packages/core/tools/README.md +++ b/packages/core/tools/README.md @@ -6,8 +6,8 @@ Tool registry and execution pipeline. Tool plugins register their schemas and ex ### Public API -- `ctx.tools.register(definition: ToolDefinition): () => void` Register a tool. The layer is the CALLING context's scope (`dsh-scope`): a plain plugin context registers globally; an agent's `agent.ctx` registers for that agent alone, SHADOWING a same-named global tool there (per-agent tool variants). Duplicate names within one layer throw. Disposed with the calling fiber (= the agent, for scoped registrations). -- `ctx.tools.restrict(filter: ToolRestriction): () => void` Scoped-only (throws on a plain context): mask the GLOBAL tool surface for the calling agent — `allow` keeps only the listed tools, `deny` removes them; multiple restrictions intersect; scoped registrations bypass restriction as explicit grants. Snapshot-at-registration, loud unknown-name validation, `restrict({})` rejects (the materialized-empty-config trap). +- `ctx.tools.register(definition: ToolDefinition): () => Promise | void` Register a tool. The layer is the CALLING context's scope (`dsh-scope`): a plain plugin context registers globally; an agent's `agent.ctx` registers for that agent alone, SHADOWING a same-named global tool there (per-agent tool variants). Duplicate names within one layer throw. Disposed with the calling fiber (= the agent, for scoped registrations). +- `ctx.tools.restrict(filter: ToolRestriction): () => Promise | void` Scoped-only (throws on a plain context): mask the GLOBAL tool surface for the calling agent — `allow` keeps only the listed tools, `deny` removes them; multiple restrictions intersect; scoped registrations bypass restriction as explicit grants. Snapshot-at-registration, loud unknown-name validation, `restrict({})` rejects (the materialized-empty-config trap). - `ctx.tools.get(name: string, scope?: ScopeKey): ToolDefinition | undefined` Resolution as one scope sees it (shadowing applied; a restricted-away global reads as absent) — presenters pass the calling agent so the card matches what executed. - `ctx.tools.visible(scope?: ScopeKey): ToolDefinition[]` THE visibility function — restricted global layer ∪ the scope's own layer — feeding prompt assembly, `get`, and `execute`, so what the model sees and what dispatches can never disagree. - `ctx.tools.knownNames(scope?: ScopeKey): string[]` The PRE-restriction name universe configuration (`toolOrder`, `restrict`) validates against: a typo fails loud while a restricted-away tool stays a normal absence. diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index cfbeffbd57..d449fb7801 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -344,9 +344,11 @@ export class ToolRegistry extends Service { * Emits `tools/change` on register/unregister. * @param definition - the tool's schema plus its execute (and optional * presentation) functions. - * @returns the disposer that unregisters the tool. + * @returns the disposer that unregisters the tool. The exact + * Cordis effect disposer (single-shot): composite (generator) effects may + * yield it directly — exact identity nests the teardown in order. */ - register(definition: ToolDefinition): () => void { + register(definition: ToolDefinition): () => Promise | void { const scope = scopeOf(this.ctx) const dispose = this.ctx.effect(function* (this: ToolRegistry) { const layer = scope === undefined ? this.global : this.layerFor(scope) @@ -370,9 +372,13 @@ export class ToolRegistry extends Service { } this.ctx.emit('tools/change') }.bind(this), 'tools.register()') - // ctx.effect's disposer returns Promise; our disposer API is - // synchronous fire-and-forget — discard the (always-resolved) promise. - return () => void dispose() + // The EXACT cordis effect disposer, not a wrapper: a composite (generator) + // effect that owns a teardown ORDER must be able to yield THIS function — + // cordis nests a disposer out of the fiber's concurrent sibling list by + // exact function identity, so a wrapper would silently break the nesting + // (the agents.register() lesson). Fire-and-forget callers may still + // discard the (always-resolved) promise. + return dispose } /** @@ -389,9 +395,11 @@ export class ToolRegistry extends Service { * Scoped registrations bypass restrictions (explicit grants win). Disposed * with the calling fiber (revocable independently); emits `tools/change`. * @param filter - global-surface mask: `allow` (keep only) and/or `deny` (remove). - * @returns the disposer that lifts this restriction. + * @returns the disposer that lifts this restriction. The exact + * Cordis effect disposer (single-shot): composite (generator) effects may + * yield it directly — exact identity nests the teardown in order. */ - restrict(filter: ToolRestriction): () => void { + restrict(filter: ToolRestriction): () => Promise | void { const scope = scopeOf(this.ctx) if (scope === undefined) { throw new Error('tools.restrict() requires a scoped context (agent.ctx): a context-global restriction would mask every agent — deny the tool for the intended agent instead') @@ -422,9 +430,13 @@ export class ToolRegistry extends Service { } this.ctx.emit('tools/change') }.bind(this), 'tools.restrict()') - // ctx.effect's disposer returns Promise; our disposer API is - // synchronous fire-and-forget — discard the (always-resolved) promise. - return () => void dispose() + // The EXACT cordis effect disposer, not a wrapper: a composite (generator) + // effect that owns a teardown ORDER must be able to yield THIS function — + // cordis nests a disposer out of the fiber's concurrent sibling list by + // exact function identity, so a wrapper would silently break the nesting + // (the agents.register() lesson). Fire-and-forget callers may still + // discard the (always-resolved) promise. + return dispose } /** The (created-on-demand) scoped layer for `scope`. */ diff --git a/packages/core/tools/tests/scoped.spec.ts b/packages/core/tools/tests/scoped.spec.ts index d7c0f020d0..86bdd1414a 100644 --- a/packages/core/tools/tests/scoped.spec.ts +++ b/packages/core/tools/tests/scoped.spec.ts @@ -125,7 +125,7 @@ describe('restrict()', () => { const liftAllow = scope.ctx.tools.restrict({ allow: ['a', 'b'] }) scope.ctx.tools.restrict({ deny: ['b'] }) expect(ctx.tools.schemas(key).map(t => t.name)).toEqual(['a']) - liftAllow() + await liftAllow() // The deny remains after the allow-list is lifted. expect(ctx.tools.schemas(key).map(t => t.name).sort()).toEqual(['a', 'c']) }) diff --git a/packages/core/tools/tests/tools.spec.ts b/packages/core/tools/tests/tools.spec.ts index 09158b8398..8892cadd1a 100644 --- a/packages/core/tools/tests/tools.spec.ts +++ b/packages/core/tools/tests/tools.spec.ts @@ -358,7 +358,7 @@ describe('ToolRegistry', () => { const dispose = ctx.tools.register({ ...echoTool, name: 'disposable' }) expect(ctx.tools.schemas().map(t => t.name)).toEqual(['echo', 'disposable']) - dispose() + await dispose() expect(ctx.tools.schemas().map(t => t.name)).toEqual(['echo']) }) @@ -379,9 +379,38 @@ describe('ToolRegistry', () => { // exposed exactly once (the duplicate-name check is not wedged). const dispose = ctx.tools.register(echoTool) expect(ctx.tools.schemas().map(t => t.name)).toEqual(['echo']) - dispose() + await dispose() expect(ctx.tools.get('echo')).toBeUndefined() }) + + it('register() returns the EXACT effect disposer: a composite yield nests the teardown in order', async () => { + // The registry-disposer convention (set by agents.register): the returned + // function IS the cordis effect disposer, so a composite (generator) + // effect that yields it has the unregistration run at that yield's LIFO + // position on owner unload. A wrapper would leave the inner effect + // disposing as a CONCURRENT SIBLING of the composite; the async probe + // below (disposed first, LIFO) yields the event loop exactly like the + // agent factory's stop-and-drain link, and a sibling unregistration fires + // in that window — the probe would observe the tool already gone. Pins + // the convention for the whole register-method family (system-prompt + // registrars, registerProvider, setFactory share the same return). + const ctx = await setup() + const order: string[] = [] + const fiber = await ctx.plugin(Object.assign((inner: Context) => { + inner.effect(function* () { + yield () => { order.push('disposed-last') } + yield inner.tools.register({ ...echoTool, name: 'nested' }) + order.push('registered') + yield async () => { + await new Promise(resolve => setTimeout(resolve, 0)) + order.push(inner.tools.get('nested') ? 'first: still registered' : 'first: already gone') + } + }) + }, { inject: ['tools'] })) + await fiber.dispose() + expect(order).toEqual(['registered', 'first: still registered', 'disposed-last']) + expect(ctx.tools.get('nested')).toBeUndefined() + }) }) describe('defineTool / schema DSL', () => { diff --git a/packages/subagent/subagent-inprocess/tests/structured.spec.ts b/packages/subagent/subagent-inprocess/tests/structured.spec.ts index e93faed4f1..3a024c7e66 100644 --- a/packages/subagent/subagent-inprocess/tests/structured.spec.ts +++ b/packages/subagent/subagent-inprocess/tests/structured.spec.ts @@ -544,7 +544,7 @@ describe('in-process structured output', () => { const run = ctx.subagents.start('spawn', structuredRequest(parent)) // A backend hot-reload mid-run must not unregister the capture tool out // from under the live child: the registration rides the CHILD's fiber. - disposeProvider() + await disposeProvider() const result = await run.result expect(result.structured).toEqual({ answer: 4 }) const child = ctx.agents.get(run.id)! diff --git a/packages/subagent/subagent/src/index.ts b/packages/subagent/subagent/src/index.ts index 850958048a..0a7cea32e1 100644 --- a/packages/subagent/subagent/src/index.ts +++ b/packages/subagent/subagent/src/index.ts @@ -164,9 +164,11 @@ export class SubagentService extends Service { * the registration and `subagent/provider-removed` on unregistration, so * consumers can mirror provider lifecycle instead of assuming load order. * @param provider - the provider; its `name` is the registry key. - * @returns the disposer that unregisters the provider. + * @returns the disposer that unregisters the provider. The exact + * Cordis effect disposer (single-shot): composite (generator) effects may + * yield it directly — exact identity nests the teardown in order. */ - registerProvider(provider: SubagentProvider): () => void { + registerProvider(provider: SubagentProvider): () => Promise | void { const dispose = this.ctx.effect(function* (this: SubagentService) { if (this.providers.has(provider.name)) { throw new SubagentError(`a subagent provider named "${provider.name}" is already registered`, 'DUPLICATE_PROVIDER') @@ -184,9 +186,13 @@ export class SubagentService extends Service { } this.ctx.emit('subagent/provider-added', provider) }.bind(this), 'subagents.registerProvider()') - // ctx.effect's disposer returns Promise; our disposer API is - // synchronous fire-and-forget — discard the (always-resolved) promise. - return () => void dispose() + // The EXACT cordis effect disposer, not a wrapper: a composite (generator) + // effect that owns a teardown ORDER must be able to yield THIS function — + // cordis nests a disposer out of the fiber's concurrent sibling list by + // exact function identity, so a wrapper would silently break the nesting + // (the agents.register() lesson). Fire-and-forget callers may still + // discard the (always-resolved) promise. + return dispose } /** diff --git a/packages/subagent/subagent/tests/service.spec.ts b/packages/subagent/subagent/tests/service.spec.ts index 66c3e80042..afacd48a9b 100644 --- a/packages/subagent/subagent/tests/service.spec.ts +++ b/packages/subagent/subagent/tests/service.spec.ts @@ -57,7 +57,7 @@ describe('SubagentService', () => { expect(added).toEqual(['alpha']) expect(removed).toEqual([]) - dispose() + await dispose() expect(removed).toEqual(['alpha']) }) @@ -92,7 +92,7 @@ describe('SubagentService', () => { ctx.on('subagent/provider-removed', name => void heard.push(name)) const dispose = ctx.subagents.registerProvider(new StubProvider('alpha')) - expect(() => { dispose() }).not.toThrow() + expect(() => void dispose()).not.toThrow() expect(heard).toEqual(['alpha']) // the listener AFTER the thrower still ran expect(ctx.subagents.getProvider('alpha')).toBeUndefined() // teardown reached quiescence expect(warnings.some(w => w.includes('boom removed listener'))).toBe(true) @@ -167,12 +167,12 @@ describe('SubagentService', () => { const dispose = ctx.subagents.registerProvider(new StubProvider('reuse')) expect(ctx.subagents.list()).toEqual(['reuse']) - dispose() + await dispose() expect(ctx.subagents.list()).toEqual([]) const disposeAgain = ctx.subagents.registerProvider(new StubProvider('reuse')) expect(ctx.subagents.list()).toEqual(['reuse']) - disposeAgain() + await disposeAgain() expect(ctx.subagents.list()).toEqual([]) }) diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index 31d6dce113..4f2a1bc33d 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -202,7 +202,7 @@ export function apply(ctx: Context, config: Config): void { // available — deriving the wording from THAT provider — and unregister it // when the provider goes away, so the description can never outlive or // predate the provider it describes. - let disposeTool: (() => void) | undefined + let disposeTool: (() => Promise | void) | undefined const mount = (provider: SubagentProvider): void => { const wording = providerWording(provider.inheritsParentContext) disposeTool = ctx.tools.register(defineTool({ @@ -284,7 +284,7 @@ export function apply(ctx: Context, config: Config): void { }) ctx.on('subagent/provider-removed', (name) => { if (name !== config.provider || disposeTool === undefined) return - disposeTool() + void disposeTool() disposeTool = undefined }) const present = ctx.subagents.getProvider(config.provider)