refactor(agent-core-v2): register tools as scoped services#2196
Conversation
…file-aware activation - replace module-level registerTool + Eager AgentBuiltinToolsRegistrar with registerAgentTool double registration (Agent-scope DI service + contribution table) - add toolActivation domain: AgentToolActivationService filters contributions by the bound Profile's tool policy using declared names, resolves instances lazily via accessor.get, and re-activates on agent.status.updated - rename BuiltinTool to AgentTool service interface; tools become Agent-scope services with decorator-injected dependencies (e.g. AgentTool -> SubagentTool/ISubagentTool) - AgentLifecycleService.create runs one activation pass after restore and profile binding so tools reflect the Profile before the first turn - update tool registrations, scripts, and tests; add toolActivationService tests
- move builtin tools from scattered domain folders (plan/tools, goal/tools, os/backends/node-local/tools, task/tools, etc.) into a unified agent/tools/ directory - split each tool into a kebab-case definition file (bash.ts) and a registration file (bashTool.ts) pairing with its prompt markdown - update imports across src, tests, kap-server, and TUI comments
Main's instantiateAll constructs every registered service at scope creation, but agent tool constructors may legitimately throw when their host capability is absent (e.g. WebSearchTool without a configured provider), and profile-aware activation must stay the only resolution path so the runtime registry holds real instances, never proxies. - add SyncDescriptor.instantiateWithScope (default true) and let registerScopedService opt registrations out of the instantiateAll sweep - registerAgentTool passes the opt-out, restoring lazy activation-driven construction on top of the eager-scope semantics - regenerate docs/state-manifest.d.ts
- add explicit OnScopeCreated and OnDemand activation modes - remove delayed proxy and idle initialization support - migrate service registrations, tests, and DI guidance
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b49838bcc1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| eventBus.subscribe('agent.status.updated', () => { | ||
| void this.activate(); |
There was a problem hiding this comment.
When a live agent changes profiles or calls update({ activeToolNames }), this subscription does not observe the new allowlist: useProfile() publishes agent.status.updated before setActiveTools(profile.tools), while an active-tools-only update publishes no status event at all. Consequently, a newly allowed contribution remains absent from the runtime registry for the next model request, so request-time policy cannot expose it; trigger activation after the active-tool state changes instead.
Useful? React with 👍 / 👎.
| // Activate the AgentTool contributions allowed by the bound Profile | ||
| // before the handle admits turns: restore and binding own the final | ||
| // `activeToolNames`, so this must run after both. |
There was a problem hiding this comment.
This newly added statement-level comment violates the package convention that comments may appear only in the top-of-file header; incorporate the sequencing responsibility into that header or make the ordering self-explanatory through code structure.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
| // The registry only holds tools the bound Profile activated; the | ||
| // contribution table is the full static universe, so a valid-but-inactive | ||
| // name never trips the unknown-tool warning. |
There was a problem hiding this comment.
This method-local explanatory comment is newly introduced despite the package rule requiring all comments to live solely in the top-of-file header; move the repository-versus-runtime-registry responsibility there or encode it in a named helper.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
1 participant
Summary
Register built-in agent tools as Agent-scoped DI services with profile-aware activation, centralize their contracts and implementations under
agent/tools, and replace legacy delayed DI proxies with explicit scope activation modes.Related Issue
No linked issue. The problem and rationale are described below.
1. Agent tool ownership and activation
Problem: Built-in tools were spread across feature domains and constructed through separate registrar paths, which made their ownership, profile filtering, and activation lifecycle inconsistent with the scoped DI model.
What was done:
agent/tools.2. Scoped service construction
Problem: The legacy eager/delayed descriptor flag combined scope activation with proxy-based lazy initialization, including an idle callback path that obscured when real service instances were created.
What was done:
OnScopeCreatedandOnDemandactivation modes.What changed
Checklist
gen-changesetsskill; this internal engine refactor needs no changeset.gen-docsskill, or this PR needs no user documentation update.make gen-changelogto update the changelog.make gen-docsto update the user documentation.