Context
Landed in 5.8.4 (#80d3bb16): `ToolManagerAgent` now exposes `registerDynamicAgent` / `unregisterDynamicAgent` so app agents installed at runtime via `AppManager` appear in `getTools` discovery. `AgentRegistrationService` wraps `AppManager`'s register/unregister callbacks with a `syncToolManagerAgent` bridge that does a two-step dance: `AgentManager.registerAgent` + `ToolManagerAgent.registerDynamicAgent`.
This pattern works today because `AppManager` is the only dynamic registrar. It does not compose if a second one ever appears.
The problem
Every new dynamic registrar needs to:
- Be hand-wired at its construction site in `AgentRegistrationService`
- Duplicate the two-step `agentManager + syncToolManagerAgent` dance, or take the pair of closures
- Never forget the sync half, or `getTools` silently hides the agent
The coupling lives in the caller instead of the emitter. `AgentManager` has no way to say "I changed, tell anyone who cares."
Proposed fix
Add a small event surface to `AgentManager`:
```ts
onAgentRegistered(listener: (agent: IAgent) => void): Disposable
onAgentUnregistered(listener: (name: string) => void): Disposable
```
`ToolManagerAgent` subscribes once in its constructor. Every registrar (`AppManager`, future remote-MCP loader, future user-settings extension agent, etc.) just calls `AgentManager.registerAgent` — the sync becomes automatic.
Cleanups that fall out:
- Delete `syncToolManagerAgent` in `AgentRegistrationService`
- Delete the concrete `ToolManagerAgent` import + `instanceof` guard in `AgentRegistrationService`
- `registerDynamicAgent` / `unregisterDynamicAgent` on `ToolManagerAgent` become internal (or go away entirely once the event is the only surface)
When to do this
Not yet. YAGNI until a second dynamic registrar is actually on the roadmap — the event contract should be shaped by that consumer's needs, not designed in the abstract. When a remote MCP agent loader, plugin-extension agent, or similar lands, bundle this refactor with it.
Files likely involved
- `src/services/AgentManager.ts` — add event emitter surface (or reuse Obsidian `Events` that's already in scope)
- `src/agents/toolManager/toolManager.ts` — subscribe in constructor
- `src/services/agent/AgentRegistrationService.ts` — drop bridge + concrete import
- `tests/unit/ToolManagerDynamicRegistry.test.ts` — adapt
Labels
`refactor`, `tech-debt`, `deferred`
Context
Landed in 5.8.4 (#80d3bb16): `ToolManagerAgent` now exposes `registerDynamicAgent` / `unregisterDynamicAgent` so app agents installed at runtime via `AppManager` appear in `getTools` discovery. `AgentRegistrationService` wraps `AppManager`'s register/unregister callbacks with a `syncToolManagerAgent` bridge that does a two-step dance: `AgentManager.registerAgent` + `ToolManagerAgent.registerDynamicAgent`.
This pattern works today because `AppManager` is the only dynamic registrar. It does not compose if a second one ever appears.
The problem
Every new dynamic registrar needs to:
The coupling lives in the caller instead of the emitter. `AgentManager` has no way to say "I changed, tell anyone who cares."
Proposed fix
Add a small event surface to `AgentManager`:
```ts
onAgentRegistered(listener: (agent: IAgent) => void): Disposable
onAgentUnregistered(listener: (name: string) => void): Disposable
```
`ToolManagerAgent` subscribes once in its constructor. Every registrar (`AppManager`, future remote-MCP loader, future user-settings extension agent, etc.) just calls `AgentManager.registerAgent` — the sync becomes automatic.
Cleanups that fall out:
When to do this
Not yet. YAGNI until a second dynamic registrar is actually on the roadmap — the event contract should be shaped by that consumer's needs, not designed in the abstract. When a remote MCP agent loader, plugin-extension agent, or similar lands, bundle this refactor with it.
Files likely involved
Labels
`refactor`, `tech-debt`, `deferred`