refactor(appkit): split the agents plugin into focused modules (#532) - #547
Open
MarioCadenas wants to merge 8 commits into
Open
refactor(appkit): split the agents plugin into focused modules (#532)#547MarioCadenas wants to merge 8 commits into
MarioCadenas wants to merge 8 commits into
Conversation
MarioCadenas
changed the title
This was referenced Aug 21, 2026
MarioCadenas
changed the title
…ules Step 1 of splitting the ~2.5k-line agents plugin. Moves module-scope pure functions/constants out of agents.ts verbatim (behavior-preserving): - approval.ts requiresApproval - prompt.ts composePromptForAgent - builtin-tools.ts LOAD_SKILL_TOOL_DEF, READ_SKILL_FILE_TOOL_DEF - adapter-extensions.ts buildAdapterExtensions, supervisorToolDescription, warnOnCapabilityMismatch agents.ts: 2512 -> 2331 lines. typecheck + 394 agent tests green. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
…ader Step 2 of splitting the agents plugin. Moves skill discovery, per-agent catalog resolution, and the load_skill/read_skill_file dispatch into skill-loader.ts as free functions; the class keeps thin delegators (call sites unchanged) and skillWorkspaceClient() as the OBO credential seam. agents.ts: 2331 -> 2152 lines. typecheck + 394 agent tests green. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
Step 3 of splitting the agents plugin. Moves the decoupled boot-time assembly helpers into registry.ts: loadCodeAgents, hasCodeAgentSources (now internal), resolveDefaultAgent, and the AgentSource type. buildAgentRegistry stays as the orchestrator that wires them. Also merges a duplicate import in skill-loader.ts. agents.ts: 2152 -> 2088 lines. typecheck + 394 agent tests green. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
…tch.ts Step 4 (final) of splitting the agents plugin. Moves dispatchToolCall + runSubAgent — the tool-call budget, approval gate, and sub-agent recursion — into tool-dispatch.ts as free functions over RunState + a ToolDispatchDeps object. The plugin builds deps via toolDispatchDeps(); the two executeTool closures call the free function. RunState moves with them. Tests updated to invoke the free functions (deps built from the plugin's own builder). agents.ts: 2088 -> 1839 lines (2512 -> 1839 across all four steps). typecheck + 394 agent tests green. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
…g.ts Moves resolvedApprovalPolicy / resolvedLimits defaulting into pure functions over AgentsPluginConfig. The getters keep the approval-policy memo cache and delegate. Config-only interface; both now unit-testable in isolation. agents.ts: 1839 -> 1812 lines. typecheck + agent tests green. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
Moves the active-stream map + per-user counter (the O(1) concurrency-limit check) into a StreamRegistry class. The plugin holds one instance and keeps trackStream/untrackStream/countUserStreams as delegators; cancel/approve read via streams.get(). Tests inject via trackStream and assert via the registry. agents.ts: 1812 -> 1784 lines. typecheck + 394 agent tests green. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
The previous commit named it StreamRegistry, colliding with the existing SSE-layer StreamRegistry in src/stream/ (connection/event-buffer tracking used by StreamManager). They're different concepts; renamed the agents-plugin one to ActiveStreamTracker (tracks active streams + per-user counts for the O(1) concurrency limit) to avoid the name clash. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
…ing) The prior commit's git add hit the already-deleted stream-registry.ts path, aborted, and recorded only the deletion — leaving the pushed tip non-compiling (agents.ts imported the removed file; active-stream-tracker.ts was uncommitted). This adds the new module and the agents.ts import/usage so the tree builds. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
MarioCadenas
force-pushed
the
refactor/agents-plugin
branch
from
August 21, 2026 17:15
9831cec to
a8f7cba
Compare
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
1 participant
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
Behavior-preserving refactor of the ~2,500-line
agents.tsgod-file into focused modules. Stacked on #543 (it only touchesagents.ts, which #543 owns) — independent of #544.No behavior changes: every commit keeps
pnpm --filter=@databricks/appkit typecheckand the agent test suite (394 tests) green.agents.ts: 2,512 → 1,784 lines (−728, ~29%). The plugin is now an orchestrator (registry wiring + route handlers + SSE plumbing) delegating cohesive concerns to focused sibling modules.What moved
approval.ts,prompt.ts,builtin-tools.ts,adapter-extensions.ts.skill-loader.ts(discovery, catalog resolution,load_skill/read_skill_file).registry.ts(loadCodeAgents,hasCodeAgentSources,resolveDefaultAgent,AgentSource).tool-dispatch.ts(RunState+dispatchToolCall+runSubAgent) as free functions over aToolDispatchDepsobject.resolve-config.ts(resolveApprovalPolicy/resolveLimitspure fns; getters keep the memo cache).ActiveStreamTracker(active-stream map + per-user O(1) concurrency counter). Named to avoid clashing with the unrelated SSE-layerStreamRegistryinsrc/stream/.Notes
skillWorkspaceClient()stays as the OBO credential seam.buildToolIndex(owns the mutablemcpClientlifecycle) and the HTTP handlers (thePluginroute contract) — both need 5-8 pieces of plugin state, so extraction would widen interfaces more than it'd clarify (fails the deep-module test). The remaining ~1,784 lines are the plugin genuinely being a plugin.StreamRegistry: that one is a global, transport-level store (event replay buffers, global cap) with no per-user dimension; this one counts streams per user formaxConcurrentStreamsPerUser+ ownership checks.dispatch-tool-call,dos-limits,approval-route).Merge order
Stacked on #543; auto-retargets to
mainonce #543 merges. Merge after #543.