feat: builtin tools redesign — factories + ctx + setTools push#58
Merged
feat: builtin tools redesign — factories + ctx + setTools push#58
Conversation
…mpl accepts entries
- Add unionByName helper for merging tool lists by name (override wins) - StelloEngineImpl.pushToolsToSession() builds session-compatible tool list from this.tools and unions with session.tools, then forwards to setTools - Constructor pushes after this.tools is assigned (initial sync to session) - forkSession captures child runtime from session.fork() and pushes to it (so newly forked sessions advertise engine-level tools to the LLM) - Add unit tests for unionByName + integration tests for engine push behavior - Update existing session stubs in agent / fork-compress / engine-factory tests to provide setTools (now required by EngineRuntimeSession contract)
Task 10 of builtin-tools redesign. The factory returns a ToolRegistryEntry whose execute closure resolves agent.forkSession at call time and reads agent.profiles dynamically for profile validation — no external state captured, so the factory takes no parameters. Framework adjustments: - ForkProfileRegistry: add has(name) for cheap profile membership check (used by the new tool's runtime profile validation). - core/index.ts: export createSessionTool from ./builtin-tools and remove the legacy re-export from @stello-ai/session (name collision; the session-package version is superseded by this factory).
…reateSessionTool - Delete packages/core/src/engine/builtin-tools.ts (schema gen moved to factory) - Delete createBuiltinToolEntries and CompositeToolRuntime (engine no longer composites) - Delete packages/session/src/tools/create-session-tool.ts (bypassed Engine editing) - Delete obsolete tests; un-skip and rewrite buildSessionToolList tests - Drop activate_skill auto-injection skill-filter tests (no longer applicable)
…ssion The Session and MainSession interfaces gained tools getter + setTools mutator (required, not optional). The demo's local wrappers must forward both members so the Engine's pushToolsToSession can update the underlying real Session at runtime. Fixes runtime error: 'session.setTools is not a function' in devtools demo.
- Replace 'as any' test stubs with 'as never' (no-explicit-any rule) - Drop unused 'vi' / 'ForkProfileRegistryImpl' imports - Drop unused '_ctx' param in activateSkillTool
CI runs strict tsc --noEmit; vitest/tsup were lenient and missed: - builtin-tools-llm-exposure.test.ts: cast Session→SessionCompatible at adaptSessionToEngineRuntime boundary (Session.fork's context union excludes 'compress'; applyCompressContext resolves it before reaching session.fork, so the cast is runtime-safe). - session-runtime.test.ts: add setTools stub to 4 SessionCompatible mocks. - fork-compress.test.ts + default-engine-factory.test.ts: add agent stub to StelloEngineOptions / DefaultEngineFactoryOptions test fixtures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Redesign of how built-in tools (
stello_create_session,activate_skill) are embedded, and a real bug fix: their descriptions never reached the LLM under the old auto-injection model.The bug. Engine internally built a
CompositeToolRuntimecontaining built-ins + user tools, but only exposed it viaengine.executeTool/engine.getToolDefinitions.Session.sendinjected tools to the LLM from a list captured atloadSessiontime, and the engine never pushed its composite into the session. Net effect: in any production config the LLM never sawstello_create_sessionoractivate_skilland so never called them.The redesign. Built-in and user tools are now isomorphic
ToolRegistryEntryobjects. Users register exactly what they want exposed:All tool execute receives
ctx: { agent, sessionId, toolCallId?, toolName }. The Engine pushesunion(session.tools, capabilities.tools)to the session at construction and after everyforkSession()via a newsetToolsmutator. The bug fix flows naturally from the new model — single source of truth for "what the LLM sees" is the registry.Breaking changes
ToolRegistryEntry.executenow requires actx: ToolExecutionContextparametercreateBuiltinToolEntries,CompositeToolRuntime,engine/builtin-tools.ts,Engine.executeCreateSessioncreateSessionToolfrom@stello-ai/session(legacy duplicate that bypassed Engine editing/profile/SplitGuard); the newcreateSessionToolfrom@stello-ai/coreis the replacementSession/MainSession/EngineRuntimeSessiongained requiredtoolsgetter +setToolsmutator (additive but type-required — custom Session-shaped wrappers must forward both members)Versions:
@stello-ai/core0.6.1 → 0.7.0,@stello-ai/session0.5.1 → 0.6.0.Test plan
pnpm -r test: 270 core / 127 session (6 env-gated skipped) / 15 devtools — all greenpnpm -r build: all packages compilepackages/core/src/__tests__/builtin-tools-llm-exposure.test.tsasserts LLMcompletereceives tool descriptions through the full chain (registry → engine push → session.setTools → session.send → llm.complete)demo/stello-agent-chat/chat-devtools.tsmigrated to new factory API; wrapper objects forwardsetTools