Skip to content

refactor: cut heavy transitive deps (goja, expr, openai-go, go-git, sqlite) from the code-built embedder surface#3771

Merged
dgageot merged 10 commits into
docker:mainfrom
dgageot:refactor/lean-embedder-imports
Jul 21, 2026
Merged

refactor: cut heavy transitive deps (goja, expr, openai-go, go-git, sqlite) from the code-built embedder surface#3771
dgageot merged 10 commits into
docker:mainfrom
dgageot:refactor/lean-embedder-imports

Conversation

@dgageot

@dgageot dgageot commented Jul 21, 2026

Copy link
Copy Markdown
Member

Go developers who build agents and teams in code (rather than YAML) were pulling in 322 → 1,054 linked packages — a +37.6 MB binary — because pkg/runtime, pkg/agent, pkg/session, and the anthropic provider all hard-imported heavy runtimes (goja, expr, openai-go SDK, full go-git, modernc/sqlite) through their transitive dependency chains. None of that weight is needed just to wire up an agent.

This change cuts the code-built embedder surface from 1,054 down to 719 linked packages by surgically breaking each heavyweight import chain. DMR endpoint resolution and /models listing move to a new pkg/model/provider/dmr/dmrmodels leaf that uses only net/http, so pkg/runtime no longer links the openai-go SDK. The full go-git import in pkg/fsx is replaced by a lightweight .git stat/parse that preserves identical no-upward-search semantics, keeping only the go-billy and plumbing/format/gitignore plumbing. The file-backed SQLite session store moves to pkg/session/sqlitestore so callers that only need the in-memory store don't pay for modernc/sqlite. The goja JS command evaluator becomes pluggable via pkg/runtime/jscommands.Register(); YAML/CLI paths call it automatically through teamloader, pkg/cli.Run, cmd/root, and embeddedchat/defaults, so there is no behavior change there. Code-built embedders that need ${...} expansion opt in with one call. Finally, pkg/tools/mcp's interactive-prompt gating, OAuth sentinel errors, and the authorization-code flow are extracted into pkg/tools and a new pkg/tools/mcp/oauthflow leaf so neither pkg/runtime nor pkg/agent need to import the MCP toolset package (which drags in expr via toolinstall and goja via pkg/js).

The one intentional breaking change is session.NewSQLiteSessionStoresqlitestore.New. All other public APIs are preserved via forwarders or aliases. An e2e dependency-budget test (e2e/dependencies_test.go) now asserts via go list -deps that goja, expr, openai-go, full go-git, sqlite, pkg/js, pkg/tools/mcp, and toolinstall remain absent from the embedder surface, so the budget cannot silently regress.

One item from the original feedback — defining tools.ToolAnnotations locally — was not done because the type is also referenced by the runtime's elicitation/sampling handler signatures and pkg/tools' own handler types; removing the MCP go-sdk import there would require breaking those public handler types, which is out of scope here.

dgageot added 7 commits July 21, 2026 15:02
pkg/runtime wired dmr.ListModels unconditionally, dragging the whole
openai-go SDK (via the DMR provider client and oaistream) into every
embedder that links the runtime, even Anthropic-only ones.

Move endpoint resolution and /models listing - which only use net/http -
into pkg/model/provider/dmr/dmrmodels and point the runtime at it.
dmr.ListModels and dmr.ErrNotInstalled remain as thin forwarders so
existing callers keep compiling and errors.Is keeps matching.
NewVCSMatcher used git.PlainOpen only to locate the worktree root, but
importing the top-level go-git package links its whole object/transport
stack (ssh, packfile, merkletrie, ...) into anything that touches the
permission checker - i.e. every embedder of pkg/runtime.

Locate the root by statting basePath/.git directly (PlainOpen did not
search parent directories either; a gitfile still counts for linked
worktrees) and read patterns via the lightweight
plumbing/format/gitignore package over an osfs, exactly as before.
pkg/session mixed the in-memory store with the file-backed constructor,
so every embedder of pkg/runtime linked modernc.org/sqlite (large and
slow to compile) even when using purely in-memory sessions.

session.NewSQLiteSessionStore(ctx, path) becomes sqlitestore.New(ctx,
path); behavior (open, migrate, backup+retry recovery) is unchanged.
SQLiteSessionStore, NewSQLiteSessionStoreFromDB and the migrations stay
in pkg/session - they only use database/sql and don't drag the driver.

BREAKING CHANGE: callers of session.NewSQLiteSessionStore must import
github.com/docker/docker-agent/pkg/session/sqlitestore and call New.
…t package

pkg/runtime and pkg/agent imported pkg/tools/mcp for a handful of small
helpers, linking the entire MCP toolset - and through it expr-lang/expr
(via toolinstall) and goja (via pkg/js) - into every embedder, even ones
with zero MCP toolsets.

- Move the interactive-prompt gating helpers, OAuth sentinel errors and
  PromptInfo/PromptArgument to pkg/tools; pkg/tools/mcp keeps aliases
  and forwarders so its public API is unchanged.
- Move the OAuth authorization-code flow (PKCE/state, callback server,
  token exchange/refresh, dynamic client registration) to the new leaf
  pkg/tools/mcp/oauthflow, again with aliases and forwarders; only the
  upstream-header-scoping transport stays in pkg/tools/mcp.
- Runtime prompt discovery now goes through a small mcpPromptToolset
  interface (satisfied by *mcp.Toolset) instead of a concrete type
  assertion.
pkg/runtime imported pkg/js solely to expand ${...} expressions in
slash-command instructions, linking the goja JavaScript engine into
every embedder of the runtime.

ResolveCommand now goes through a registered CommandEvaluator factory.
The new pkg/runtime/jscommands package registers the goja-backed
evaluator on import and is blank-imported by teamloader, the CLI and
embeddedchat/defaults, so every YAML/CLI path behaves exactly as
before. Code-built embedders that want JS command expressions opt in
with the same blank import; without it, ${...} is left unexpanded with
a warning pointing at the import.
Assert via go list -deps that the packages a code-built embedder links
(runtime, agent, team, session, tools, anthropic provider) stay free of
goja, expr, the OpenAI SDK, full go-git, the SQLite driver, pkg/js,
pkg/tools/mcp and toolinstall, so the import-graph cuts don't silently
regress. Also migrate e2e tests to sqlitestore.New.
golangci-lint's nolintlint flags it as unused; json.Marshal of the
token cache no longer trips gosec.
@dgageot
dgageot requested a review from a team as a code owner July 21, 2026 13:04
docker-agent

This comment was marked as outdated.

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

This is a well-structured dependency-reduction refactor. All seven hypotheses raised during review were dismissed after verification:

  • pkg/fsx/vcs.go gitfile parsing: The new lightweight .git stat/parse correctly handles Windows CRLF line endings via strings.TrimSpace, and the no-upward-search semantics are preserved and tested.
  • pkg/session/sqlitestore db lifecycle: The open() helper correctly closes db exactly once on failure; no leak or double-close.
  • commandEvaluatorFactory test: Standard Go test pattern for swapping package-global atomic state; not parallel, so no data race.
  • ListModelsAt context use after timeout: The slog.DebugContext call runs before defer cancel() fires; context is valid. Even a cancelled context doesn't suppress slog output.
  • HTTPClientForAllowPrivateIPs type assertion: Pre-existing pattern moved verbatim; not introduced by this PR.
  • Cache TOCTOU in NewVCSMatcher: The TOCTOU window between read and write lock yields only harmless idempotent writes.
  • SetHTTPClientForTesting restore ignored in TestMain: Intentional — the process exits immediately via os.Exit, so no restore is needed.

The dependency-budget e2e test (e2e/dependencies_test.go) that locks the embedder surface against regression is a solid safeguard for the stated goals.

@aheritier aheritier added area/agent For work that has to do with the general agent loop/agentic features of the app area/deps Dependency updates and version bumps area/mcp MCP protocol, MCP tool servers, integration area/providers/docker-model-runner Docker Model Runner (DMR) local inference area/runtime Runtime engine, agent loop execution, tool dispatch, loop detection area/sessions For features/issues/fixes related to session lifecycle (resume, persistence, export) area/tools For features/issues/fixes related to the usage of built-in and MCP tools kind/refactor PR refactors code without behavior change labels Jul 21, 2026
dgageot added 3 commits July 21, 2026 16:17
GetSessionMCPPrompts decodes the server response into map[string]any,
so each value is a nested map, never a concrete tools.PromptInfo; the
type assertion always failed and RemoteRuntime.CurrentMCPPrompts
silently returned an empty map. Round-trip each value through JSON to
get typed prompts.

Found by review on the PR; the bug predates the PromptInfo move.
os.Rename replaces the destination, so a second migration failure
clobbered the .bak from the first one - the last known-good copy of the
user's sessions. When .bak already exists, move the broken database
aside under a timestamped name instead.
- Cap non-200 response body reads at 512 bytes before embedding them in
  errors: OAuth error bodies come from the remote endpoint and end up
  in logs, so a bound keeps a hostile or misconfigured server from
  stuffing megabytes (or sensitive fields) into them.
- Use a comma-ok assertion on http.DefaultTransport so a replaced
  transport (test helper, proxy shim) degrades to a fresh default
  instead of panicking.
@dgageot
dgageot merged commit d55fc6a into docker:main Jul 21, 2026
16 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/agent For work that has to do with the general agent loop/agentic features of the app area/deps Dependency updates and version bumps area/mcp MCP protocol, MCP tool servers, integration area/providers/docker-model-runner Docker Model Runner (DMR) local inference area/runtime Runtime engine, agent loop execution, tool dispatch, loop detection area/sessions For features/issues/fixes related to session lifecycle (resume, persistence, export) area/tools For features/issues/fixes related to the usage of built-in and MCP tools kind/refactor PR refactors code without behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants