Skip to content

feat(server): add --lazy-source-init to connect sources on first use - #3791

Open
akangsha7 wants to merge 5 commits into
mainfrom
feat/lazy-source-init
Open

feat(server): add --lazy-source-init to connect sources on first use#3791
akangsha7 wants to merge 5 commits into
mainfrom
feat/lazy-source-init

Conversation

@akangsha7

Copy link
Copy Markdown
Contributor

Description

Toolbox connects to every configured source at startup and aborts the process if any single connection fails. That is the right default for a small, always-used set of sources, but it means a tool catalog cannot be inspected without provisioning its databases, cold start pays for every source a config declares rather than every source it uses, and one bad credential produces an MCP server that silently never comes up.

This adds an opt-in --lazy-source-init flag. It is off by default and the eager path is untouched. When set, each source connects on the first tool call that needs it.

Where the laziness lives. The sources.Source interface carries no database-specific methods, so 271 of the 273 tool packages downcast the injected source to their own package-private interface, across 755 assertion sites. No wrapper type can satisfy 271 independently-declared private interfaces, so a proxy would fail every tool's assertion at runtime. Instead a SourceResolver sits between the invocation handlers and the primitive store and connects on demand; the concrete, fully-connected source still reaches the tool untouched and no tool code changes.

PrimitiveManager stays a plain get/set repository — it gains only SetSource. Keeping the fallible, I/O-performing lookup off the store is deliberate, so the store remains substitutable by a backend-backed implementation later.

Two lookup contracts. Listing paths (tools/list, /api/toolset) call the non-blocking GetSource and fall back to the tool's static manifest when a source has not been reached yet. Invocation paths (tools/call, /api/tool/invoke) call SourceResolver.Resolve. Only invocation handlers receive the resolver, so a listing handler cannot block on a connect. The rule does not branch on the flag: under eager init Resolve returns on its first line with no I/O.

Three properties worth calling out:

  • Concurrent first calls are coalesced. singleflight collapses N simultaneous first calls into one connection attempt, so an agent issuing parallel tool calls against a cold source does not open N pools.
  • The shared attempt is bounded and detached from its callers. It runs on context.WithoutCancel of the starting caller's context, capped at 60s. Each caller waits on its own context, so a caller returns on its own request deadline; a caller that gives up does not abort the attempt for everyone else.
  • Failures are not cached. A source that was down starts working on a later call without a restart.

Startup validation is split, not removed. A tool naming a source that does not exist is still a startup error. Tool/source type compatibility is deferred to first call, since asserting it requires a live source.

Connection failure is agent-visible. An unreachable source returns a tool result with isError: true rather than a JSON-RPC protocol error, which most client harnesses consume before the model ever sees it. Over REST the same failure is a 503.

Env var placeholders. Under the flag, an unset required ${VAR} resolves to its own name as a non-empty placeholder so required-string validation still passes, and startup logs a warning naming every substitution. Without the flag a missing variable remains a hard startup error. The end state is that ./toolbox --prebuilt alloydb-postgres --lazy-source-init serves the full catalog with no database, no credentials, and no environment variables set.

Known limitations

Raising these explicitly rather than leaving them to be discovered:

  • Placeholders only work for string-typed fields. Substitution is a text pass over the YAML before unmarshalling, so an unset ${VAR} on an int or bool field (for example database: ${REDIS_DB}) produces a bare scalar and the config fails to unmarshal — the server refuses to start. All 38 prebuilt configs are unaffected, so the documented flow works, but user-authored configs using ${VAR} on typed fields will hit this.
  • Config reload is not atomic. The source configs and the primitive store are updated under different locks. They are now installed in the order that makes the worst case a discarded connect rather than a stale source cached for the process lifetime, but a combined API would be needed to close the window properly.
  • Reload drops warm sources. Under lazy init every reload leaves the sources map empty, so the next call re-pays a cold connect even for a reload that changed nothing about sources. Related pre-existing issue: sources.Source has no Close(), so discarded pools are not torn down on either the eager or lazy reload path.
  • No notifications/tools/list_changed. A tool whose schema is derived from a live source advertises its static schema until that source connects. Clients that listed beforehand are not told when the resolved schema becomes available.

Non-goals

Partial-availability/health semantics, retry or backoff policy, negative caching of failed connections, adding Close() to the source interface, and making lazy the default.

Test plan

  • go test -race ./cmd/... ./internal/... — 358 packages, no failures
  • golangci-lint run --timeout 10m — 0 issues
  • go mod tidy && git diff --exit-code
  • Unit tests for the resolver: connect-when-cold, connect-when-warm, unknown source, concurrent first-call coalescing, caller-cancellation isolation, caller deadline, and no-caching-of-failures
  • Unit tests for the static manifest fallback, one per protocol version
  • Unit test that a connection failure yields an isError tool result rather than a protocol error
  • Regression: the existing suite passes unchanged with the flag unset
  • Manual: ./toolbox --prebuilt alloydb-postgres --lazy-source-init with no database, credentials, or env vars set; confirm the catalog lists and a tool call returns a readable error
  • Drive it from a real MCP client to confirm the agent receives readable isError content rather than a protocol-level failure

PR Checklist

  • Make sure to open an issue as a bug/issue before writing your code!
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)
  • Make sure to add ! if this involves a breaking change

Akangsha Goel added 5 commits July 31, 2026 12:58
… unavailable

Listing tools previously required every tool's source to be present in the
primitive manager, so a single unavailable source made tools/list and
/api/toolset fail outright. Fall back to the parameters and manifest baked at
tool initialization when the source is absent, and only resolve the dynamic,
source-derived manifest when the source is actually there.

This is a no-op today because sources are always initialized eagerly. It is a
prerequisite for lazy source initialization, where a source is connected on
first use and is legitimately absent until then.

Adds Tool.GetStaticParameters as the GetParameters counterpart to the existing
StaticManifest.
Toolbox connects to every configured source at startup and refuses to start if
any connection fails, so a tool catalog cannot be inspected without provisioning
its databases, cold start pays for sources that may never be called, and a
misconfigured source takes down the whole server instead of surfacing an error
the agent can act on.

--lazy-source-init defers each connection to the first tool call that needs it.
PrimitiveManager gains ResolveSource, which connects and caches on demand,
alongside the existing non-blocking GetSource that listing paths use. Concurrent
first calls are coalesced with singleflight; failures are deliberately not
cached, so a source that comes up later starts working without a restart.

Tools cannot be handed a lazy proxy because each asserts its source against its
own local interface, so laziness lives at the lookup layer and the concrete
source still reaches the tool untouched.

A connection failure on tools/call now returns a tool result with isError set
rather than a JSON-RPC protocol error, so the message reaches the agent instead
of being discarded by the harness.

Source name validation still happens at startup; only type compatibility and
source-derived tool schemas move to first call. Default behavior is unchanged.
Lazy source initialization parses configs it will never connect from, so an
unset required ${VAR} resolves to a placeholder rather than failing the parse.
Record every substitution so callers can warn about it, instead of letting a
placeholder stand in for real config silently.
…lver

PrimitiveManager is the primitive store, so putting a fallible, I/O-performing
lookup on it conflated the repository with connection management. Move the
deferral into a SourceResolver that owns the retained source configs, the
tracer and the singleflight group, and reduce the manager to GetSource and a
new SetSource.

Listing paths keep reading the store directly and fall back to static
manifests. Only invocation paths receive the resolver, so a listing handler
cannot block on a connect.

Also fixes problems found while reworking this:
- set the user agent on the REST invoke path; most source drivers refuse to
  build a client without one, so lazy connects failed there permanently
- install reloaded source configs before swapping primitives, so a call
  landing mid-reload cannot cache a source built from the pre-reload config
- drop a dead SkipSourceValidation write whose only reader is unreachable
  whenever it runs
- guard the singleflight result type assertion
- give the reload watcher test a real resolver; a zero-value Server panicked
  if the debounce fired before teardown
An unreachable source is reported to the agent as a tool result with IsError
set rather than as a JSON-RPC error, so nothing upstream marks the request as
failed. That left the entire class of failure invisible: during a total source
outage every tools/call returned success and the tool execution metric recorded
nothing at all.

Report it on the same metric a failing Invoke reports to, so an unreachable
source is as visible to operators as a tool that ran and errored.
@akangsha7
akangsha7 requested a review from a team as a code owner August 8, 2026 07:04

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a --lazy-source-init flag that defers connecting to configured sources until they are first used, allowing the server to start up and serve tool catalogs even when sources are unreachable or required environment variables are missing. To support this, a new SourceResolver is introduced to manage lazy connections, and tool listing and invocation paths are updated to fall back to static manifests when a source is not yet connected. The feedback suggests checking if the context is already canceled before initiating a connection attempt in the resolver to avoid spawning unnecessary goroutines.

Comment on lines +106 to +107
}

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.

medium

It is recommended to check if the caller's context (ctx) is already canceled or expired before initiating the connection attempt via singleflight. This prevents spawning unnecessary background connection goroutines when the request has already been abandoned by the client.

Suggested change
}
}
if err := ctx.Err(); err != nil {
return nil, fmt.Errorf("unable to initialize source %q: %w", sourceName, err)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants