# mcpagent public API simplification

**Status:** Public-surface cutover complete; builder single-construction lifecycle follow-up open
**Date:** 2026-08-01
**Last updated:** 2026-08-02
**Repositories:** `mcpagent`, `mcp-agent-builder-go`
**Related:** `docs/bugs/custom_tool_category_as_agent_addressing.md`,
`docs/bugs/mcp_startup_retry_and_double_construction.md`,
`docs/refactor/canonical_agent_definition_construction.md`

## Implementation status

Completed core boundary:

- `Agent` is opaque: it exposes exactly four methods and no exported fields.
- `Session` exposes exactly five methods; both lifecycle types use
  `Close() error`.
- Builder, workflow, chat, delegation, gRPC, Family Server, and test callers no
  longer read or write public `Agent` state.
- Construction-time runtime values use typed `RuntimeConfig`; request-specific
  streaming and tool policy use `Turn`.
- Read-only access uses definition, diagnostics, runtime-info, and opaque
  session-handle snapshots.
- `DefinitionAssembly`, `LegacyOptions`, all 57 exported `With*` functions, and
  the three legacy public constructors were removed from the supported API.
- Unsafe testing/mutation helpers, example-only conveniences, and dead recovery
  wrappers were removed. The package now has exactly 45 exported functions,
  down from 148 at the start of this pass.
- The unused example trees were deleted rather than allowing demonstrations to
  keep compatibility APIs public. Command tests call `Agent.Run` directly.
- The legacy Tool Search mode was removed completely. Provider-native Codex
  `tool_search` disabling remains a separate transport capability.
- Attached skills now have one transport-neutral access contract owned by
  `mcpagent`: attaching the first skill installs the reserved `read_skill` tool,
  exposes it through the coding-agent bridge, and keeps native CLI projection
  as an optional optimization rather than the only way to read a skill.
- Golden tests pin the exact 45-function inventory and the exact 14-function
  `Agent` facade, as well as the four/five method contracts, zero exported
  `Agent` fields, matching close contracts, and no exported `ForTesting` helper.

Open builder lifecycle follow-up (confirmed 2026-08-02):

- The public API reduction is complete, but `LLMAgentWrapper` still constructs
  a placeholder `Agent` before chat/delegation definition assembly and replaces
  it in `FinalizeDefinition`. Builder call sites use
  `GetUnderlyingAgent() != nil` as a pre-finalization readiness gate, so the
  intended single immutable construction boundary has not yet landed. With a
  failing MCP server this repeats the complete startup/retry sequence. See
  `docs/bugs/mcp_startup_retry_and_double_construction.md`.
- The public API golden tests do not cover construction count; a builder
  lifecycle regression test is required before this follow-up can be closed.
- The proposed builder-wide migration that closes this follow-up without
  inventing another public agent-spec type is documented in
  [canonical_agent_definition_construction.md](canonical_agent_definition_construction.md).

Verified on the current working tree (2026-08-02):

- `go test ./... -count=1` passes in `mcpagent`, including the exact API
  ratchets.
- `go test -race ./agent -count=1` and `go vet ./agent` pass in
  `mcpagent`.
- `go test ./... -count=1` passes in `mcp-agent-builder-go/agent_go`.
- That suite compiles and verifies the real `agent_go/cmd/family-server` target.
- `npm run build` passes in `frontend`, and the rebuilt static bundle contains
  no legacy structured-output event renderer.
- `npm test -- --run` passes all 54 frontend test files (335 tests).

### Surface audit (2026-08-02): exact contract after reduction

The final measured surface is:

```text
*Agent methods                                    4
Agent exported fields                             0
*Session methods                                  5
package functions                                45
  exported With* options                          0
  public constructors returning *Agent            1
  functions accepting or returning *Agent        14
```

The AST-based golden test pins the sorted names, not merely these counts. A
deleted export cannot be silently replaced by a different export, and moving a
method into a package function changes the reviewed 14-name facade list.

### Consumer boundary and Family Server

The supported local consumers are `mcpagent`'s gRPC/command packages and
`mcp-agent-builder-go/agent_go`, which uses the local module replacement. The
real Family Server is `agent_go/cmd/family-server`; it lives on
`mcp-agent-builder-go` main and is compiled by the full builder suite. Its small
refactor-branch delta uses the immutable session API.

The separately named `/Users/mipl/ai-work/mcpagent-family-server` folder is an
old worktree of the **same mcpagent module** on
`fix/stale-claude-native-resume`. It is not the Family Server application and is
not a downstream consumer to migrate. Its stale-resume commits should be merged
through normal branch integration if still wanted; its legacy public API does
not define the supported contract.

No third-party consumer was found in the local workspace. A breaking published
release still requires an explicit external-consumer policy.

### Completed reduction work

The five-step follow-up is complete on the supported code path:

1. The exact exported inventory and Agent-facade inventory are golden-tested.
2. The builder constructs `AgentDefinition` directly; `DefinitionAssembly` and
   all duplicate `AddDefinition*` wrappers are gone.
3. `RuntimeConfig` is grouped into generation, tools, context, coding, MCP,
   workspace, and observability values. `LegacyOptions`, exported `With*`
   options, and public legacy constructors are gone.
4. Package-only ask, retry, summarization-threshold, recovery, and definition
   lookup helpers are private. Dead constructors and one-shot structured/test
   conveniences were deleted. Unused example trees were deleted with user
   approval; command tests use `Agent.Run` and diagnostics.
5. The actual Family Server target is part of the active builder suite and
   passes. The similarly named old mcpagent worktree was correctly excluded as
   a consumer.

The legacy structured-output subsystem went further than originally planned.
The structured ask/conversion helpers and dynamically forced completion tool
were removed outright, together with their private implementation, bespoke
events, tracing spans, UI renderers, and obsolete command tests. A grep across
both supported consumers found no runtime caller. Builder-owned JSON results
continue to use explicit prompt contracts plus prevalidation, or ordinary
declared tools. Coding-agent `transport=structured` remains intact because it
is the CLI transport protocol, not this deleted response-conversion feature.

What remains public is context compaction/summarization, resume, steering,
diagnostics, and retirement, because active production code calls them. They
form the reviewed 14-name facade ratchet. Moving some of them into narrower
runtime services may be a future cleanup, but is not a compatibility shim or
blocker for this refactor.

## Decision

An agent definition has only three meaningful parts:

1. **Instructions** — what the agent should do and the constraints it follows.
2. **Skills** — reusable domain or procedural knowledge available to it.
3. **Tools** — capabilities it may discover and execute, whether implemented
   directly or supplied by MCP servers.

These three inputs are fixed when the agent is created. They are not mutable
lifecycle state.

Model selection, provider transport, workspace paths, permissions, session
handles, events, logging, usage accounting, caches, retries, and routing are
runtime infrastructure. They may configure or operate an agent, but they are
not additional agent-content concepts and should not expand the main `Agent`
API.

## Why this refactor is needed

Before this cutover, `mcpagent.Agent` exposed 70 methods and 64 exported mutable
fields. That large surface allowed callers to manage internal lifecycle order
themselves:

- overwrite, append, clear, inspect, and rebuild prompt state;
- register a tool and then explicitly refresh a second registry;
- narrow tool access after prompt construction;
- address custom tools through implementation categories;
- manipulate provider, bridge, event, virtual-tool, and continuation internals;
- choose between several overlapping conversation and delivery APIs.

This is how state became duplicated across prompt text, tool maps, allow lists,
execution registries, schema caches, and session handles. A caller can invoke
individually valid methods in an invalid order.

The first containment pass reduced the surface from 84 to 70 methods and
removed the worst prompt and registry lifecycle combinations. The completed
core cutover reduced the concrete `Agent` surface again—from 70 methods and 64
exported fields to four methods and zero exported fields. The old mutation
operations are no longer methods on a live agent; the remaining compatibility
work is confined to constructors and package-level bridge/runtime helpers.

Production evidence makes the cost concrete:

- 46 failed `get_api_spec` calls were recorded in one day, dominated by callers
  guessing a category or server even though the tool name already resolved.
- One background agent received a 10,476-character prompt with no tool inventory
  while `query_workflow_db` was registered and executable.
- A materialized prompt previously grew to 14 times its intended size across
  turns because clearing one prompt list did not rebuild the copied prompt text.

## Core invariant

```text
agent_definition = instructions + skills + tools

effective_tools = agent_definition.tools intersect runtime_permissions
```

The definition is immutable. Runtime permissions may deny an operation, but
they must not rewrite the definition, mutate prompt text, or create another
tool-addressing scheme.

If instructions, skills, or the available tool set need to change, create a new
agent definition. Do not reset an existing agent.

## Target public model

```go
type AgentDefinition struct {
    Instructions string
    Skills       []Skill
    Tools        ToolSet
}

type ToolSet struct {
    Direct []ToolDefinition
    MCP    []MCPToolSource
}

type ToolDefinition struct {
    Name        string
    Description string
    InputSchema map[string]any
    Execute     ToolExecutor
    Timeout     time.Duration
}

type MCPToolSource struct {
    ServerID string
    Config   MCPServerConfig
}

type RuntimeConfig struct {
    Model         ModelConfig
    Transport     TransportConfig
    Workspace     WorkspaceAccess
    Observability ObservabilityConfig
}
```

Construction becomes explicit:

```go
agent, err := mcpagent.NewAgent(ctx, AgentDefinition{
    Instructions: instructions,
    Skills:       skills,
    Tools: ToolSet{
        Direct: directTools,
        MCP:    mcpServers,
    },
}, runtimeConfig)
```

MCP is a tool source, not a second kind of agent. MCP tools and direct tools
enter one name-keyed registry and become indistinguishable to the model after
registration.

## Categories are not part of the public contract

The current `RegisterCustomTool(..., category)` signature gives `category`
several unrelated jobs:

- configuration bundle;
- display grouping;
- filtering key;
- schema-discovery address;
- routing hint.

Only the first two are legitimate long-term uses.

Categories may remain internal metadata for configuration expansion or UI
grouping. They must not appear in:

- `AgentDefinition` tool addresses;
- `get_api_spec` calls;
- execution URLs chosen by the model;
- authorization decisions after construction;
- cache identity.

The public and model-facing address is the globally unique tool name. Registry
construction fails if two sources claim the same name.

## Target runtime API

The ordinary API should have approximately 8–12 methods, not 70:

```go
type Agent interface {
    Start(ctx context.Context) (Session, error)
    Definition() AgentDefinitionView
    Close() error
}

type Session interface {
    Run(ctx context.Context, turn Turn) (Result, error)
    Send(ctx context.Context, input Input) (DeliveryResult, error)
    Snapshot() SessionHandle
    Events() <-chan Event
    Close() error
}

type Turn struct {
    Input      Input
    ToolPolicy ToolPolicy
}
```

`Run` starts or continues a model turn and applies that turn's runtime tool
policy. `Send` delivers steering or user input into an already-running turn; it
does not begin a second turn. The effective tool manifest is rendered at the
outbound request boundary from the immutable definition intersected with the
current `ToolPolicy`. It is never stored in the base instructions.

For simple one-turn callers, a convenience method may be provided:

```go
result, err := agent.Run(ctx, input)
```

The exact count is less important than the ownership boundary:

- `Agent` owns one immutable definition.
- `Session` owns conversation history, continuation, steering, and delivery.
- `Result` owns output, usage, and diagnostics.
- internal packages own bridge routing, virtual tools, caches, event emission,
  and provider transport.

## Current 70 methods and intended disposition

### Core execution

| Current method | Disposition |
|---|---|
| `Ask` | Replace with `Run` convenience method. |
| `AskWithHistory` | Move history ownership to `Session.Run`. |
| `Close` | Keep, returning an error consistently. |

### Instructions

| Current method | Disposition |
|---|---|
| `SetInstructions` | Remove; instructions are construction input. |
| `AddInstructions` | Remove; builder assembles final instructions before construction. |
| `ResetInstructions` | Remove; a phase change creates a new agent. |
| `Instructions` | Replace with read-only `Definition()` or diagnostic snapshot. |

### Skills

| Current method | Disposition |
|---|---|
| `AttachSkill` | Remove from runtime API; skills are construction input. |
| `AttachedSkills` | Expose only through read-only definition diagnostics if needed. |
| `DetachSkill` | Remove; create a new agent definition. |
| `ClearSkills` | Remove; create a new agent definition. |

### Tool definition, registration, and policy

| Current method | Disposition |
|---|---|
| `RegisterCustomTool` | Replace with `ToolDefinition` in `AgentDefinition.Tools.Direct`. |
| `RegisterCustomToolWithTimeout` | Fold timeout into `ToolDefinition`. |
| `ReplaceCustomToolExecutor` | Remove; registry is immutable after construction. |
| `GetCustomToolExecutor` | Internal registry operation. |
| `GetCustomTools` | Read-only definition diagnostics, not mutable map exposure. |
| `GetCustomToolsByCategory` | Remove; category is not runtime addressing. |
| `GetCustomToolCategories` | Move to configuration/UI metadata if still needed. |
| `SetToolAccess` | Replace mutable agent state with `Turn.ToolPolicy`; permissions remain runtime policy, not identity. |
| `SetToolArgTransformer` | Fold into the tool definition or internal adapter. |
| `GetToolOutputHandler` | Internal. |
| `SetToolOutputHandler` | Runtime configuration at construction. |
| `GetToolToServer` | Internal registry routing. |
| `GetSelectedTools` | Read-only definition diagnostics if needed. |

### Provider, MCP, workspace, and connection state

| Current method | Disposition |
|---|---|
| `GetProvider` | Read from definition/runtime diagnostic view. |
| `SetProvider` | Remove; provider is immutable runtime configuration. |
| `GetLLMModelConfig` | Read from runtime diagnostic view. |
| `GetPrompts` | Remove from `Agent`; MCP prompt resources are internal tool-source data. |
| `GetResources` | Remove from `Agent`; MCP resources are internal tool-source data. |
| `GetServerNames` | Read-only tool-source diagnostics if needed. |
| `GetConfiguredServerName` | Remove singular server abstraction; definition contains tool sources. |
| `GetMCPConfigJSON` | Internal bridge adapter operation. |
| `SetFolderGuardPaths` | Construction-time `WorkspaceAccess`. |
| `GetFolderGuardPaths` | Runtime diagnostic view only. |
| `GetContext` | Remove; callers pass contexts to operations. |
| `IsCancelled` | Remove; operation errors and context state are authoritative. |
| `CheckConnectionHealth` | Move to a diagnostics interface/service. |
| `GetConnectionStats` | Move to a diagnostics interface/service. |

### Events and usage

| Current method | Disposition |
|---|---|
| `AddEventListener` | Replace with construction-time observer or `Session.Events`. |
| `RemoveEventListener` | Eliminated by session/event-subscription lifetime. |
| `EmitTypedEvent` | Internal only. |
| `HandleEvent` | Internal adapter/interface only. |
| `HasStreamingCapability` | Runtime capability view, not a primary method. |
| `GetEventStream` | Merge into `Session.Events`. |
| `SubscribeToEvents` | Merge into `Session.Events` or a separate observer. |
| `GetTokenUsage` | Return structured usage in `Result`. |
| `GetTokenUsageWithPricing` | Return structured usage/cost in `Result`. |

### Conversation, continuation, and steering

| Current method | Disposition |
|---|---|
| `AddSteerMessage` | Internal queue operation behind `Session.Send`. |
| `DrainSteerMessages` | Internal only. |
| `ContinueConversation` | Merge into `Session.Run`/`Session.Send`. |
| `TurnInFlight` | Read-only session state if genuinely needed. |
| `SupportsSteering` | Session capability view. |
| `Deliver` | Merge into `Session.Send`. |
| `DeliverControlKey` | Optional specialized session-control API. |
| `DeliverUserMessage` | Merge into `Session.Send`. |
| `StartCodingAgentTransportSession` | Internal provider adapter. |
| `StartCodingAgentTmuxSession` | Remove legacy transport-specific entry point. |
| `CurrentAgentSessionHandle` | Replace with `Session.Snapshot`. |
| `ApplyAgentSessionHandle` | Handle is supplied to `ResumeSession` construction. |
| `ContinueAgentSession` | Merge into resumed `Session.Run`. |
| `ContinueAgentSessionWithHistory` | Merge into resumed `Session.Run`. |

### Bridge, virtual tools, and large-output internals

| Current method | Disposition |
|---|---|
| `BuildBridgeMCPConfig` | Internal transport adapter. |
| `CreateVirtualTools` | Internal registry construction. |
| `HandleVirtualTool` | Internal routing. |
| `CreateLargeOutputVirtualTools` | Internal registry construction. |
| `HandleLargeOutputVirtualTool` | Internal routing. |
| `BuildLargeOutputFilePath` | Internal storage service. |

### Removed legacy Tool Search diagnostics

| Current method | Disposition |
|---|---|
| `GetDiscoveredToolCount` | Removed with the legacy Tool Search mode. |
| `GetDeferredToolCount` | Removed with the legacy Tool Search mode. |

### Legacy prompt method

| Current method | Disposition |
|---|---|
| `RebuildSystemPromptWithFilteredServers` | Delete; contradicts immutable definitions and request-time rendering. |

## Required builder changes

`mcp-agent-builder-go` currently constructs an agent and then incrementally
changes it. The builder must instead complete definition assembly first.

### Before

```text
create agent
  -> register tools one at a time
  -> add instruction fragments
  -> clear/reset instructions for a phase
  -> attach skills
  -> apply tool allow list
  -> start or resume
```

### After

```text
resolve phase
  -> build final instructions
  -> resolve enabled skills
  -> expand configured tool bundles and MCP sources
  -> validate unique tool names
  -> create immutable agent
  -> start or resume session
  -> apply the current permissions to each turn
```

When the workflow phase changes identity inputs, the builder creates a new agent
with the new definition. A workshop-mode change that only changes authorization
uses a new per-turn `ToolPolicy`, not a new agent. Conversation continuity is
preserved through an opaque `SessionHandle`; it is not preserved by mutating the
old agent.

Dynamic values should be routed deliberately:

| Value | Destination |
|---|---|
| User request, run-specific evidence | Input message/context |
| Stable role and constraints | `Instructions` |
| Reusable procedural guidance | `Skills` |
| Executable capabilities | `Tools` |
| Secrets and environment values | Tool runtime environment, never prompt mutation |
| Folder permissions | `RuntimeConfig.Workspace` plus per-turn `ToolPolicy` |
| Continuation state | `SessionHandle` |
| Usage, cost, tool failures | `Result` and observability |

## Migration plan

Every stage is independently shippable. Each stage lands as its own commit in
both repositories, retains the previously working path until its replacement is
covered, and can be reverted without reverting later unrelated work. Production
diagnostics must identify whether the legacy or replacement path handled a run.

### Stage 1 — freeze the target API

- Add `AgentDefinition`, `ToolSet`, `ToolDefinition`, and `RuntimeConfig`.
- Add a public-API golden test that enumerates exported `Agent` and `Session`
  methods.
- Freeze the legacy `*Agent` baseline at exactly 70 exported methods and only
  lower that committed number during migration.
- Set the final target at exactly four `Agent` methods (`Start`, `Run`,
  `Definition`, `Close`) and five `Session` methods (`Run`, `Send`, `Snapshot`,
  `Events`, `Close`).
- Do not add compatibility aliases without an explicit migration deadline.

### Stage 2 — structured tool registry

- Replace positional `RegisterCustomTool` calls with `ToolDefinition` values.
- Reject duplicate names before mutating any registry, identifying both owners.
- Expand category-based configuration bundles before `NewAgent`.
- Load MCP tools into the same name-keyed registry.
- Fail construction on duplicate names.
- Remove category from schema discovery, execution authorization, and cache keys.

### Stage 3 — immutable construction in the builder

- Introduce one builder function that returns `AgentDefinition` and
  `RuntimeConfig`.
- Move all current `AddInstructions`, `AttachSkill`, and tool registration
  identity assembly before construction.
- Replace phase mutation with new-agent construction.
- Replace per-turn `SetToolAccess` mutation with `Turn.ToolPolicy`.
- Delete `SetInstructions`, `AddInstructions`, `ResetInstructions`, and skill
  mutation.

### Stage 4 — one session abstraction

- Introduce `Session` as the owner of history, streaming, continuation,
  steering, and delivery.
- Replace overlapping `Ask*`, `Continue*`, `Deliver*`, and transport-specific
  methods.
- Persist and restore only opaque `SessionHandle` values.

### Stage 5 — move infrastructure off Agent

- Move bridge configuration and virtual-tool handlers into internal packages.
- Move event emission into an observer/transport layer and migrate all 11
  downstream listener-registration call sites.
- Return usage and diagnostics in `Result`.
- Move connection health into a diagnostics service.
- Remove raw mutable map getters.

### Stage 6 — delete legacy surface

- Remove old methods rather than retaining permanent deprecated wrappers.
- Remove old lifecycle documentation and tests.
- Update examples and both repositories in the same change.
- Run the public-API golden test to prevent regrowth.

## Acceptance criteria

- `AgentDefinition` exposes only instructions, skills, and tools as agent
  identity.
- Instructions, skills, tools, model, provider, and workspace policy cannot be
  mutated after construction.
- A phase change that alters identity creates a new definition; a workshop-mode
  authorization change supplies a new per-turn policy.
- Direct and MCP tools share one unique name-keyed registry.
- Category is absent from the public tool registration and discovery APIs.
- Duplicate tool names fail construction with both sources identified.
- The builder performs no prompt rebuilding, registry refreshing, allow-list
  mutation, or manual tool-manifest assembly.
- The final `Agent` surface has exactly four methods and `Session` exactly five.
- Events emitted for a run contain the same instructions and effective tool
  view actually sent to the model.
- Usage, cost, tool failures, and diagnostics are returned structurally rather
  than queried through unrelated `Agent` getters.
- Existing API-provider and coding-agent continuation behavior remains covered
  by end-to-end tests.
- `go test ./...` passes in `mcpagent` and `go test ./agent_go/...` passes in
  `mcp-agent-builder-go`.

## Cutover progress (updated 2026-08-02)

The branch-level cutover now has a working end-to-end spine:

- the concrete `Agent` now exposes exactly four methods and zero exported
  fields; `Session` exposes exactly five methods, and both `Close` methods
  return `error`;
- package-level exported functions fell from 148 to 45. AST golden tests pin
  every exported name plus the reviewed 14-function Agent facade and reject
  `ForTesting` backdoors;
- the builder no longer writes runtime state into public `Agent` fields. It
  supplies provider keys, prompt labels, workspace paths, session handles, and
  per-turn streaming callbacks through typed construction/turn inputs;
- read-only state needed by callers is exposed through immutable definition,
  diagnostics, runtime-info, and opaque session-handle snapshots rather than
  mutable fields;
- the full Go suites pass in both repositories, including
  `agent_go/cmd/family-server`;
- direct and MCP tools enter one canonical name-keyed registry;
- request-time manifests and `get_api_spec` resolve from that registry;
- conflicting implementation owners fail before replacing registry state;
- `AgentDefinition` is validated and deeply cloned before runtime creation;
- the reusable `agentsession` path and the main orchestrator assemble direct
  tools and MCP sources before constructing the agent;
- attached skills are now readable through the intrinsic, reserved
  `read_skill` tool on API and coding-agent transports; CLI filesystem
  projection remains an optimization, so background/stage-agent isolation no
  longer changes the skill-reading contract;
- `BaseAgent` executes through `Run(Turn)` rather than choosing among ask and
  continuation methods itself;
- the reusable `agentsession` adapter and the legacy `agentwrapper` execution
  paths now also run through `Session.Run`/`Agent.Run`, with continuation,
  updated history, usage, and cancellation results returned together;
- `Turn.ToolPolicy` is request-scoped and controls the rendered manifest,
  schema discovery, and session HTTP execution guard without changing identity;
- `Session` is pinned to exactly five methods: `Run`, `Send`, `Snapshot`,
  `Events`, and `Close`; and
- structured `Result.Usage` now includes token counts, cost components, and
  context utilization, removing the wrapper's need to query pricing state; and
- provider-native continuation is now supplied through `RuntimeConfig`, and
  workflow recovery/identity changes rebuild an agent around the opaque handle
  rather than mutating the live instance;
- workflow supplementary skills and prompt sections are applied by creating a
  replacement immutable definition before the turn, while preserving the MCP
  session and provider continuation handle; and
- the main chat and delegation wrappers now finalize their incrementally
  assembled legacy drafts into one immutable definition before the first turn;
  the freeze includes static prompt supplements, cloned skill definitions,
  direct tool executors/schemas, and observers, while replacement retirement
  preserves shared tracers and MCP/provider state;
- chat and delegation prompt/skill/tool assembly builds a private
  `AgentDefinition` draft, but currently also constructs a placeholder Agent
  before assembly and replaces it at finalization. This is the remaining
  lifecycle violation tracked in
  `docs/bugs/mcp_startup_retry_and_double_construction.md`;
- the gRPC adapter now owns an explicit `Session` and returns response, history,
  usage, and costs from `Result` instead of calling legacy `Ask*`, token getters,
  or raw tool maps; and
- the concrete `Agent` surface is now the final four methods: `Start`, `Run`,
  `Definition`, and `Close`; the golden test pins that exact list;
- workflow, chat, delegation, and gRPC tool factories assemble direct
  `AgentDefinition` values before the first-turn boundary and reject later
  identity changes;
- workflow and chat permissions are passed as `Turn.ToolPolicy`, while folder
  guards live in `RuntimeConfig` rather than mutable agent identity;
- workflow steering now goes through the active `Session`, whose delivery path
  remains available while a serialized `Run` is in flight; and
- gRPC custom tools are constructed into the immutable definition. Their stable
  executors proxy to the currently bound stream callback, so a new conversation
  changes runtime routing without replacing tool schemas or mutating the Agent;
- cleanup ticker shutdown now detaches lifecycle state under a mutex and uses
  goroutine-local channels, removing the construction/close race exposed by the
  new definition tests.

The duplicate *public construction APIs* are gone: no `LegacyOptions`, exported
`With*`, `DefinitionAssembly`, public legacy constructor, or legacy Tool Search
mode remains. A separate duplicate runtime construction remains inside the
builder wrapper: it creates a placeholder through `NewAgentFromDefinition`,
then creates its final replacement through the same canonical constructor. The
remaining 14 Agent-related package functions are exact-listed because active
production code uses them for context maintenance, diagnostics, resume,
steering, transport startup, replacement, and the one manual virtual-tool test
boundary. They are no longer an unbounded public compatibility surface, but the
builder lifecycle follow-up must still remove the placeholder construction.

## Review (2026-08-01)

Endorsed in direction. The core diagnosis — *"a caller can invoke individually
valid methods in an invalid order"* — is exactly what produced every failure in
the related bug report, and `agent_definition = instructions + skills + tools`,
immutable, is the right generalisation. The 70-method count is accurate
(verified: 46 in `agent.go`, 24 across nine other files). The public-API golden
test in Stage 1 is the single most valuable item here, because it is the only
part that prevents regrowth.

Four things to resolve before this is actionable.

### 1. The invariant and the method disposition contradict each other

The invariant permits runtime permissions:

```text
effective_tools = agent_definition.tools intersect runtime_permissions
```

But the table disposes of `SetToolAccess` as *"Remove from normal runtime API;
construct the agent with its effective tool set."* Those cannot both hold. The
second says permissions are construction-time; the first says they are not.

This is not hypothetical. `cmd/server/workflow_phase_tools.go` applies the allow
list **per turn, deliberately**:

> The chat-history auto-restore path … passes `applyAllowList=false` so the
> restored CLI sees the superset; `/api/query` later narrows it via
> `SetToolAllowList` **when the first user turn arrives**.

The workshop mode is not known at construction, and a restored coding-agent CLI
must be created with the superset before it can be narrowed.

**Correction to an earlier draft of this review:** it claimed recreating the
agent per phase was impractical because it would re-run MCP setup and invalidate
the provider session. Both reasons are wrong. MCP connections are lazy and
session-keyed (`connection_session.go:404` — *"prefers the session registry
(lazy-connect, reuses existing connections)"*), so a new `Agent` on the same
`SessionID` reuses them. Continuation already survives recreation:
`AgentSessionHandle` carries `SessionID` plus `CodingProviderSessionHandle`, and
`ApplyAgentSessionHandle` restores it (`session_handle.go:63`, `:86`) — which is
the mechanism this document already names. Recreation on a **phase change** is
therefore viable, and the "new agent per phase" position stands.

What remains is narrower: the allow list is applied *per turn*, not per phase, so
"construct with the effective tool set" would mean a new agent every turn — a
heavier claim than the plan makes, and one the plan should either commit to
explicitly or drop.

The invariant is the part that is right. Keep `effective_tools` derived at
request time from the definition and the *current* permissions, and let a
permission-setting call remain in the runtime API — it changes policy, not
identity, which the invariant already allows. Then correct the `SetToolAccess`
row, and state explicitly that the tool manifest sent to the model is rendered
from `effective_tools` at request time rather than stored. Without that sentence,
a reader implements immutability by materialising the manifest at construction,
which is precisely the bug this refactor exists to remove.

### 2. There is no incremental verification or rollback story

Six stages across two repositories, ending with *"update examples and both
repositories in the same change"* and *"remove old methods rather than retaining
permanent deprecated wrappers"*. That is a big-bang cutover for the layer every
agent in the system runs on.

The evidence for caution is in this repository's own history: on 2026-08-01 two
separate, carefully-reasoned fixes to this exact area were wrong — one placed in
`SetSystemPrompt`, one in `rebuildSystemPromptWithUpdatedToolStructure` — and
both had passing tests that encoded the author's assumed lifecycle rather than
the production one. A rewrite of the whole surface by the same means needs an
answer to: what runs in production during Stages 2–5, and what is the revert if
Stage 4 is wrong?

Suggest making each stage independently shippable and observable, with Stage 2
(structured registry, duplicate-name failure, category out of discovery and cache
keys) first — it is the smallest, it is already half-done, and it carries most of
the correctness benefit.

### 3. Cite the production evidence

The "why" here is argued structurally. The concrete evidence is stronger and
lives in the sibling document: 46 failed `get_api_spec` calls in one day; a
background agent shipped a 10,476-character prompt with no tool inventory while
`query_workflow_db` sat registered; and the pre-existing **CLAUDE.md 14× bloat
bug**, recorded in `ClearAppendedSystemPrompts`, where a materialised prompt grew
14× across turns because its copy drifted from its inputs. That last one predates
all of this and shows the missing invariant has already been paid for once, and
was patched with a caller-ordering convention rather than a fix.

Anyone deciding whether to fund this work should see those three numbers.

### 4. Smaller notes

- `Session.Events() <-chan Event` replaces listener registration.
  `mcp-agent-builder-go` has 11 `AddEventListener`/`RemoveEventListener` call
  sites, so this is a bounded but real downstream migration and deserves naming
  in Stage 5 rather than the one-line "move event emission into an observer
  layer".
- `Ask` → `Run` and `Session.Run`/`Session.Send` overlap enough to be confused;
  worth one sentence on which is a turn and which is a delivery into an in-flight
  turn, since `Deliver`/`DeliverUserMessage`/`AddSteerMessage` all fold into
  `Send` and their current semantics differ.
- Acceptance criteria should include the golden test *count* as a committed
  number, not just "no more than 12". A count that can be edited is not a ratchet.

## Canonical registry follow-up: complete

**Status: implemented and verified (2026-08-02).** The audit found that the
observation was real architectural debt, even though no production divergence
had yet been captured. `Agent.customTools` contained no information absent from
`registeredTool`: definition, executor, display group, and timeout all existed
in both records.

The two stores could silently disagree because registration wrote them in
separate operations. The timeout path was especially weak: it updated
`customTools` first and discarded any error while updating `toolRegistry`.
Meanwhile discovery, bridge schema lookup, serial execution, parallel execution,
skill-name collision checks, and timeout selection mostly read the projection.
The structure therefore made the object documented as canonical the minority
source in practice.

The follow-up removed `CustomTool` and `Agent.customTools` completely. The
canonical registry is now the sole Agent-side record for direct-tool identity,
schema, executor, display metadata, and timeout. All of these consumers read it:

- prompt and OpenAPI discovery;
- bridge MCP schema construction;
- serial and parallel direct-tool execution;
- per-tool timeout selection;
- direct-tool category/display-group enumeration;
- attached-skill reserved-name checks; and
- construction of the code-execution executor projection.

Registration now writes one complete canonical record, including timeout, in one
operation. Re-registering the same direct tool in a different display group is
rejected inside the locked registry as well as at the caller boundary, so two
concurrent registrations cannot bypass the invariant.

`agent/codeexec.ToolRegistry` still contains executor maps. Those are deliberate
session-scoped runtime projections used by the HTTP bridge: they contain only
callable functions, are rebuilt from the canonical snapshot, and cannot answer
identity, schema, category, or timeout questions. They are not a second
Agent-side tool registry.

Regression coverage now asserts that a timeout-bearing direct tool produces one
complete canonical record and that the executor projection is derived from that
record. `go test ./...`, `go test -race ./agent -count=1`, and `go vet ./agent`
pass in `mcpagent`; `go test ./agent_go/... -count=1` passes in the builder. The
exported API ratchets are unchanged: four `Agent` methods, zero exported `Agent`
fields, and 45 package functions.

Two related comments remain intentionally out of scope because their state is
still live: the legacy per-provider session ID fields alongside
`codingProviderSessionHandle`, and supported same-display-group re-registration
used to refresh session-aware executors.

### Session-scoped permissions after the projection change: verified

Removing `Agent.customTools` changed where the code-execution executor map comes
from, so the session permission model was re-traced end to end rather than
assumed. It is intact. There are two enforcement surfaces and both receive the
same policy from one place:

```text
Turn.ToolPolicy
  → normalizeToolPolicy
  → ctx turnPolicyContextKey  → isToolAllowedForContext   (in-process calls)
  → codeexec.SetSessionToolAllowList(sessionID, allowed)  (HTTP bridge calls)
```

`Session.Run` (`turn_session.go:189`) writes both on every turn, so a
code-executing agent cannot escape a per-turn policy by reaching a tool over the
HTTP bridge instead of calling it directly.

Three properties were confirmed in code, not inferred:

1. **Executor refresh still works.** `canonicalToolRegistry.register` overwrites
   an existing record when kind, source, and display group match. The builder
   paths that re-register a tool to swap in a session-aware executor therefore
   still take effect; the projection is rebuilt from the updated record.
2. **Session registration happens on both paths** — at construction
   (`agent.go:1866`) and on re-registration (`agent.go:3391`), each guarded by a
   non-empty `sessionID`.
3. **A session registry does not fall through to global.** Once
   `sessionCustomTools[sessionID]` exists, a missing tool is an error rather than
   a global lookup, so one workflow cannot borrow another's executor. Only a
   session with no registry at all uses the legacy global path.

**One real gap was found and closed.** The HTTP-bridge allow-list gate in
`CallCustomToolWithSession` — the half of the model that enforces `ToolPolicy`
over the bridge — had no test anywhere in either repository. The in-process half
was covered (`agent/session_policy_test.go`, `skill_reader_test.go`) and
cross-session executor isolation was covered
(`TestCallCustomToolWithSessionDoesNotBorrowGlobalExecutor`), but nothing
exercised the allow list itself. It could have stopped enforcing while every
existing test stayed green.

Four tests were added to `agent/codeexec/registry_test.go`:

- a tool outside the allow list is rejected and its executor never runs;
- a tool inside the allow list executes;
- a nil allow list means unrestricted, not blocked — `Session.Run` passes nil
  whenever `ToolPolicy.AllowedTools` is empty, so inverting this would break
  every unrestricted turn rather than fail closed on one;
- one session's allow list does not gate another's, since concurrent workflows
  share one process-wide registry.

They were mutation-checked: disabling the gate in `CallCustomToolWithSession`
fails exactly the two enforcement tests and leaves the two permissive ones
passing, which is the expected signature. `go test ./...`,
`go test -race ./agent ./agent/codeexec`, and `go vet` pass in `mcpagent`, and
`go test ./... -count=1` passes in `agent_go`.

## Non-goals

- Replacing MCP itself.
- Putting full tool schemas into the system prompt.
- Removing runtime permission enforcement.
- Forcing API providers and coding-agent CLIs to use identical internal
  transports.
- Persisting an agent object. Persist only definitions and opaque session
  handles.

## Final target

The builder should be able to explain agent creation in one sentence:

> Resolve the instructions, skills, and tools; create the agent; run a session.

If builder code must refresh a registry, reset instructions, discover a tool
category, or manually synchronize prompt text with authorization state, the
ownership boundary is still wrong.