Compare commits

..
Author SHA1 Message Date
Vasanthdev2004 6d19afa3c4 test(sessions): name the expected context block instead of the first render
Comparing later renders against the first only asks whether the renderer is
stable, and a renderer that is stably wrong passes it: any fixed field order
satisfies it, including the unsorted one this change replaces.

It also hid something. The events were tool calls, which
promptContextEvents filters out of the resume context on this branch, so
the block being compared was a single message line and nothing said so. The
test now names the block it expects, on events that survive the filter, and
both failures are visible.
2026-09-11 10:20:23 +05:30
Vasanthdev2004 b45bb2ece8 fix(sessions): render resume context fields in key order
extractText walked a map[string]any directly and Go randomizes map
iteration, so the same session put the same fields in a different order in
every process: one run rendered a tool call as c1 read_file {"path":...},
the next as {"path":...} c1 read_file. That block is part of the user
prompt, so the prompt itself changed run to run, which leaves a provider
nothing stable to prefix-cache for it and makes two resumes of one session
impossible to diff while debugging.

Only the ordering is decided here. The summary short-circuit is untouched,
a list keeps the order it was written in, and which fields survive at all
stays the caller's projection.

Pinned as exact rendered strings rather than as two renders agreeing: a
same-process repeat passes whenever the random order happens to repeat,
which for a small map is often. The raw-JSON case proved it while being
written, matching on the first five attempts before diverging on the sixth.

Closes #1020
2026-09-11 10:20:22 +05:30
Vasanth Tandbaoyu0 1e4db7c664 fix: let the configured forward proxy through the loopback guard (#1025)
* fix(providerhealth,tools): let the configured forward proxy through the loopback guard

With HTTPS_PROXY pointing at a local forward proxy, the transport dials the
proxy first and tunnels the request through it. The SSRF-guarded dialer in
providerhealth refused that dial as a loopback address, so
`zero doctor --connectivity` failed with "proxyconnect tcp: provider
connectivity URL is unsafe: loopback hosts are blocked" for a public
provider that had already been validated, and web_fetch had no proxy
support at all (#569).

Each dialer is now handed its transport's own Proxy function and lets a dial
through unpinned only when the address is exactly the proxy endpoint that
function names for the request scheme: same host, same port, with a missing
proxy port taken as the scheme default. A sibling loopback port is still
refused. The shared check lives in internal/proxydial since providerhealth
and tools otherwise share nothing.

web_fetch honours http.ProxyFromEnvironment again. The guarded fetch had set
Proxy to nil as part of its SSRF hardening; the target is still validated
before the request, and what a forward proxy removes is only the dial-time
rebind pin, which is inherent to using any forward proxy. The pre-permission
URL validation stays in place.

Started from #594 by baoyu0. This version keeps the pre-permission check,
matches the proxy port exactly instead of any port when the proxy URL has
none, and injects the transport's Proxy function instead of reading the
environment a second time.

Closes #569

Co-authored-by: baoyu0 <[email protected]>

* fix(proxydial): have the dialer ask its transport for the proxy at dial time

Copying transport.Proxy into the dialer at construction left the wiring
between the two unpinned: a dialer handed nil would pass every unit test
while a proxied request still died as loopback. The dialer now consults
the transport it is installed on, and each package drives a real request
through a loopback listener acting as the proxy and checks that the
CONNECT arrives. The proxy is substituted on the transport rather than
through HTTPS_PROXY, since http.ProxyFromEnvironment reads the
environment once per process and an env-based test would depend on
test order.

* fix(proxydial): bind the proxy exemption to the route the transport chose

Asking the Proxy function a question of our own invention answered for a
request the transport never made. With NO_PROXY excluding a host, the real
request to that host goes direct while a synthetic probe still comes back
proxied, so a direct dial whose address happened to equal the configured
proxy endpoint skipped the guard and reached whatever the name resolved to
at connect time. A probe on the previous head dials rebind.example:80 with
zero safety lookups; the guard now runs and refuses its private answer.
Host-dependent proxy callbacks make the mismatch general, and no spelling
of the probe host closes it, because the two calls are about different
requests.

The decision is now recorded where it is made. proxydial.Transport wraps
the transport, asks its Proxy function about the actual request, and puts
the answer in that request's context; the transport derives the dial
context from the request context, so the dialer reads back the route this
very connection is being opened for. Per-request, so concurrent requests
cannot overwrite each other, and the context key is unexported so nothing
outside the package can plant one.

Address matching is gone with it. A transport opens one connection dial,
and when a proxy is selected that dial goes to the proxy, so the recorded
route already identifies the dial. Comparing the address on top could only
disagree with the transport, which is what a Unicode proxy hostname did:
Go dials xn--bro-hoa.example while the URL still spells it as configured,
and the proxy was refused as an ordinary private address. Both spellings
are now one endpoint with no comparison to get wrong.

Documents where destination enforcement ends: with a proxy configured, the
target hostname is sent to the proxy and the proxy resolves it, so the
local pre-request checks no longer bind the address that is reached. A
forward proxy already sees and can rewrite every request through it, so
that trust comes with configuring one; the alternative is refusing to use
proxies at all, which is the outage in #569.

---------

Co-authored-by: baoyu0 <[email protected]>
2026-09-10 21:50:26 +05:30
Vasanth T f9092668a8 fix(agent): stop the tool-schema ratchet meaning a different thing per host (#1017)
* fix(agent): stop the tool-schema ratchet meaning a different thing per host

TestEagerToolSchemaTokenBudget counted exec_command shell guidance along with
the schemas, and that guidance is host-specific. The same tree measured 3498 on
Linux, about 3600 on a Windows host with pwsh 7, and 3653 on Windows PowerShell
5.1, because shellGuidanceForRuntime appends a longer block for 5.1 (it has to
explain that && and || do not work there).

5.1 is stock Windows, and that case has been over the 3650 ceiling since
view_image landed. So `go test ./...` failed for anyone on a stock Windows box
while CI stayed green: the runners have pwsh 7 and take the cheaper branch. The
workflow does run the tests on windows-latest, so this was never a coverage gap,
just a ceiling sized against whichever shell the runner happened to have.

The two halves are now charged separately. The schema ceiling measures the
schemas alone and reads 3498 on Linux and on Windows PowerShell 5.1, so it
ratchets the same thing everywhere. The host guidance gets its own ceiling at
200, above the worst case rather than above this machine: 5.1 is 155 tokens,
pwsh 7 is 110, cmd.exe is 87, POSIX is 0.

No guidance text was trimmed. It is what stops the model writing bash on Windows,
and the 5.1 note is the reason a chained command does not silently do half its
work. Paying 155 tokens for it is the right trade; measuring it as if it were a
schema was not.

HostExecCommandShellGuidance is exported for the budget, which is production
surface added for a test. It is one accessor over existing logic, and the
alternative is a ratchet that cannot tell the two costs apart. A guard in
internal/tools pins that the guidance really is the tail of the description, so
the subtraction cannot silently start measuring the wrong thing.

* test(tools,agent): make the guidance contract checkable off Windows

The ratchet separates the fixed tool schemas from this host's exec_command
shell guidance, and both halves of that separation were only being checked on
Windows.

The description test returned early off Windows once the accessor reported no
guidance, so it never looked at the description. Dropping the GOOS condition in
the constructor would have a POSIX host append its own guidance (about 150
characters on darwin) with the accessor still reporting none and the ratchet
charging every byte of it to the schemas, and nothing would fail. Both the
description and the accessor now take goos, so the contract is checked for all
three platforms from any one of them.

The budget also went from subtracting a separately rounded guidance estimate to
removing the guidance from the description before estimating. ApproxTextTokens
is an integer division, so floor((schema+guidance)/4) - floor(guidance/4) is one
token high whenever the two remainders add past four, and which side of that a
host lands on depends on the length of its guidance. That is the same
host-dependence the ratchet was being fixed for, one token smaller.

No behaviour change: exec_command's description is byte for byte what it was on
every platform.
2026-09-10 21:43:58 +05:30
KRATOS 7f5e5af7bb fix(acp): make configured MCP tools callable (#1003)
* fix(acp): register configured MCP tools per turn

* fix(acp): address MCP review findings
2026-09-10 20:17:26 +05:30
14 changed files with 848 additions and 50 deletions

No files matched your search

+52 -6
View File
@@ -14,6 +14,7 @@ import (
"github.com/Gitlawb/zero/internal/providercatalog"
"github.com/Gitlawb/zero/internal/providermodelcatalog"
"github.com/Gitlawb/zero/internal/providermodeldiscovery"
"github.com/Gitlawb/zero/internal/redaction"
"github.com/Gitlawb/zero/internal/sandbox"
"github.com/Gitlawb/zero/internal/sessions"
"github.com/Gitlawb/zero/internal/tools"
@@ -30,10 +31,12 @@ type Deps struct {
DiscoverModels func(context.Context, config.ProviderProfile) ([]providermodeldiscovery.Model, error)
NewProvider func(profile config.ProviderProfile) (zeroruntime.Provider, error)
RunAgent func(ctx context.Context, prompt string, provider zeroruntime.Provider, opts agent.Options) (agent.Result, error)
// BuildWorkspace builds the SCOPED tool registry and the sandbox engine for a
// validated workspace root, so ACP shell tools (bash/exec_command) are confined
// exactly like the exec surface — never run unconfined on the host.
BuildWorkspace func(workspaceRoot string, resolved config.ResolvedConfig) (*tools.Registry, *sandbox.Engine, error)
// BuildWorkspace creates the per-turn scoped tool workspace for a validated
// workspace root. Its Close method releases any resources the registry owns
// (notably MCP server connections) after the turn completes or is cancelled.
// Shell tools remain confined exactly like the exec surface — never unconfined
// on the host.
BuildWorkspace func(ctx context.Context, workspaceRoot string, resolved config.ResolvedConfig, mode agent.PermissionMode) (*Workspace, error)
// ResolveWorkspaceRoot validates + normalizes a client-supplied cwd (must be an
// existing directory; never the bare root). It is the file-tool confinement root.
ResolveWorkspaceRoot func(cwd string) (string, error)
@@ -41,6 +44,27 @@ type Deps struct {
AgentInfo Implementation
}
// Workspace is the per-turn execution environment passed to the agent. ACP does
// not retain it across turns because MCP connections belong to the registry that
// advertised their tools; keeping a stale registry after a cancelled turn would
// leak its server process and its permission state into the next turn.
type Workspace struct {
Registry *tools.Registry
Sandbox *sandbox.Engine
DeferThreshold int
Notices []string
Cleanup func() error
}
// Close releases resources created with the workspace. A nil cleanup is valid
// for core-only workspaces.
func (w *Workspace) Close() error {
if w == nil || w.Cleanup == nil {
return nil
}
return w.Cleanup()
}
// Agent is the ACP agent server bound to one JSON-RPC connection (one editor).
type Agent struct {
conn *Conn
@@ -241,13 +265,34 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
if err != nil {
return "", RPCError(codeInternalError, "provider: "+err.Error())
}
mode := sess.currentMode()
// Build the SCOPED registry + sandbox engine for this session's workspace so
// shell/file tools are confined to the workspace exactly like the exec surface.
registry, sandboxEngine, err := a.deps.BuildWorkspace(sess.cwd, resolved)
// The workspace can also own MCP connections, which must be closed after every
// turn even if the agent run returns an error or the client cancels it.
workspace, err := a.deps.BuildWorkspace(ctx, sess.cwd, resolved, mode)
if err != nil {
return "", RPCError(codeInternalError, "workspace: "+err.Error())
}
if workspace == nil || workspace.Registry == nil {
if workspace != nil {
_ = workspace.Close()
}
return "", RPCError(codeInternalError, "workspace: missing tool registry")
}
defer func() {
if err := workspace.Close(); err != nil {
log.Printf("acp: close workspace: %v", err)
}
}()
registry := workspace.Registry
sandboxEngine := workspace.Sandbox
note := &notifier{conn: a.conn, sessionID: sess.id}
for _, notice := range workspace.Notices {
if text := strings.TrimSpace(redaction.RedactString(notice, redaction.Options{})); text != "" {
note.text("\n\n[zero warning] " + text + "\n")
}
}
opts := agent.Options{
Cwd: sess.cwd,
@@ -256,7 +301,8 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
Model: resolved.Provider.Model,
Registry: registry,
Sandbox: sandboxEngine,
PermissionMode: sess.currentMode(),
PermissionMode: mode,
DeferThreshold: workspace.DeferThreshold,
MaxTurns: resolved.MaxTurns,
Images: images,
OnText: note.text,
+152 -4
View File
@@ -8,6 +8,7 @@ import (
"io"
"os"
"path/filepath"
"slices"
"strings"
"testing"
"time"
@@ -35,6 +36,35 @@ func (f fakeProvider) StreamCompletion(_ context.Context, _ zeroruntime.Completi
return ch, nil
}
type acpDeferredPromptTool struct{ name string }
func (t acpDeferredPromptTool) Name() string { return t.name }
func (acpDeferredPromptTool) Description() string { return "deferred MCP test tool" }
func (acpDeferredPromptTool) Parameters() tools.Schema { return tools.Schema{Type: "object"} }
func (acpDeferredPromptTool) Deferred() bool { return true }
func (acpDeferredPromptTool) Run(context.Context, map[string]any) tools.Result {
return tools.Result{Status: tools.StatusOK, Output: "ok"}
}
func (acpDeferredPromptTool) Safety() tools.Safety {
return tools.Safety{
SideEffect: tools.SideEffectNetwork, Permission: tools.PermissionPrompt,
AdvertiseInAuto: true,
}
}
type captureToolsProvider struct {
requests chan zeroruntime.CompletionRequest
}
func (p captureToolsProvider) StreamCompletion(_ context.Context, request zeroruntime.CompletionRequest) (<-chan zeroruntime.StreamEvent, error) {
p.requests <- request
ch := make(chan zeroruntime.StreamEvent, 2)
ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventText, Content: "done"}
ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventDone}
close(ch)
return ch, nil
}
func testDeps(t *testing.T) Deps {
t.Helper()
store := sessions.NewStore(sessions.StoreOptions{RootDir: t.TempDir()})
@@ -53,10 +83,10 @@ func testDeps(t *testing.T) Deps {
return fakeProvider{text: "Hello from ZERO"}, nil
},
RunAgent: agent.Run,
BuildWorkspace: func(string, config.ResolvedConfig) (*tools.Registry, *sandbox.Engine, error) {
BuildWorkspace: func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
r := tools.NewRegistry()
r.Register(tools.NewUpdatePlanTool())
return r, nil, nil
return &Workspace{Registry: r}, nil
},
ResolveWorkspaceRoot: func(cwd string) (string, error) { return cwd, nil },
Store: store,
@@ -468,8 +498,8 @@ func TestACPRunTurnWiresSandboxAndScopedRegistry(t *testing.T) {
reg := tools.NewRegistry()
reg.Register(tools.NewUpdatePlanTool())
engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: t.TempDir()})
deps.BuildWorkspace = func(string, config.ResolvedConfig) (*tools.Registry, *sandbox.Engine, error) {
return reg, engine, nil
deps.BuildWorkspace = func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
return &Workspace{Registry: reg, Sandbox: engine}, nil
}
var captured agent.Options
deps.RunAgent = func(_ context.Context, _ string, _ zeroruntime.Provider, opts agent.Options) (agent.Result, error) {
@@ -497,6 +527,124 @@ func TestACPRunTurnWiresSandboxAndScopedRegistry(t *testing.T) {
}
}
func TestACPRunTurnUsesWorkspaceDeferThresholdAtProviderBoundary(t *testing.T) {
for _, tc := range []struct {
name string
threshold int
want []string
}{
{name: "explicit zero disables", threshold: 0, want: []string{"mcp_docs_alpha", "mcp_docs_beta"}},
{name: "below threshold stays eager", threshold: 3, want: []string{"mcp_docs_alpha", "mcp_docs_beta"}},
{name: "at threshold defers", threshold: 2, want: []string{tools.ToolSearchToolName}},
} {
t.Run(tc.name, func(t *testing.T) {
requests := make(chan zeroruntime.CompletionRequest, 4)
deps := testDeps(t)
deps.NewProvider = func(config.ProviderProfile) (zeroruntime.Provider, error) {
return captureToolsProvider{requests: requests}, nil
}
deps.BuildWorkspace = func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
registry := tools.NewRegistry()
registry.Register(acpDeferredPromptTool{name: "mcp_docs_alpha"})
registry.Register(acpDeferredPromptTool{name: "mcp_docs_beta"})
registry.Register(tools.NewToolSearchTool(registry))
return &Workspace{Registry: registry, DeferThreshold: tc.threshold}, nil
}
h := newHarness(t, deps)
defer h.stop()
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
var created NewSessionResult
if err := h.client.Call(ctx, MethodSessionNew, NewSessionParams{Cwd: t.TempDir()}, &created); err != nil {
t.Fatal(err)
}
if err := h.client.Call(ctx, MethodSessionPrompt, PromptParams{SessionID: created.SessionID, Prompt: []ContentBlock{TextBlock("hi")}}, &PromptResult{}); err != nil {
t.Fatal(err)
}
request := <-requests
got := make([]string, 0, len(request.Tools))
for _, definition := range request.Tools {
got = append(got, definition.Name)
}
if !slices.Equal(got, tc.want) {
t.Fatalf("provider tools = %#v, want %#v", got, tc.want)
}
})
}
}
func TestACPRunTurnEmitsWorkspaceSetupNotices(t *testing.T) {
deps := testDeps(t)
deps.BuildWorkspace = func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
registry := tools.NewRegistry()
registry.Register(tools.NewUpdatePlanTool())
return &Workspace{Registry: registry, Notices: []string{"MCP server docs unavailable, skipped: timeout"}}, nil
}
h := newHarness(t, deps)
defer h.stop()
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
var created NewSessionResult
if err := h.client.Call(ctx, MethodSessionNew, NewSessionParams{Cwd: t.TempDir()}, &created); err != nil {
t.Fatal(err)
}
if err := h.client.Call(ctx, MethodSessionPrompt, PromptParams{SessionID: created.SessionID, Prompt: []ContentBlock{TextBlock("hi")}}, &PromptResult{}); err != nil {
t.Fatal(err)
}
got := drainText(t, h.updates)
if !strings.Contains(got, "[zero warning] MCP server docs unavailable, skipped: timeout") || !strings.Contains(got, "Hello from ZERO") {
t.Fatalf("streamed text = %q", got)
}
}
// TestACPRunTurnClosesWorkspace proves a per-turn workspace never outlives the
// agent run. This is particularly important for MCP: its registry owns live
// client connections and, for stdio servers, child processes.
func TestACPRunTurnClosesWorkspaceAfterSuccessAndFailure(t *testing.T) {
for _, tc := range []struct {
name string
runErr error
}{
{name: "success"},
{name: "agent failure", runErr: errors.New("provider interrupted")},
} {
t.Run(tc.name, func(t *testing.T) {
deps := testDeps(t)
closed := 0
deps.BuildWorkspace = func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
registry := tools.NewRegistry()
registry.Register(tools.NewUpdatePlanTool())
return &Workspace{Registry: registry, Cleanup: func() error {
closed++
return nil
}}, nil
}
deps.RunAgent = func(context.Context, string, zeroruntime.Provider, agent.Options) (agent.Result, error) {
return agent.Result{FinalAnswer: "ok"}, tc.runErr
}
h := newHarness(t, deps)
defer h.stop()
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
var created NewSessionResult
if err := h.client.Call(ctx, MethodSessionNew, NewSessionParams{Cwd: t.TempDir()}, &created); err != nil {
t.Fatalf("session/new: %v", err)
}
err := h.client.Call(ctx, MethodSessionPrompt, PromptParams{SessionID: created.SessionID, Prompt: []ContentBlock{TextBlock("hello")}}, &PromptResult{})
if tc.runErr == nil && err != nil {
t.Fatalf("session/prompt: %v", err)
}
if tc.runErr != nil && err == nil {
t.Fatal("session/prompt succeeded after agent failure")
}
if closed != 1 {
t.Fatalf("workspace cleanup calls = %d, want 1", closed)
}
})
}
}
// TestACPRejectsInvalidCwd confirms session/new fails when the workspace root
// resolver rejects the client cwd (e.g. filesystem root).
func TestACPRejectsInvalidCwd(t *testing.T) {
+51 -2
View File
@@ -1,9 +1,11 @@
package agent
import (
"strings"
"testing"
"github.com/Gitlawb/zero/internal/tools"
"github.com/Gitlawb/zero/internal/zeroruntime"
)
// These ceilings are a deliberate ratchet on Zero's fixed per-turn overhead — the
@@ -33,7 +35,24 @@ const (
// The ratchet did its job: it caught the creep and forced a decision instead
// of a drift. That is the point of it, so keep raising it deliberately rather
// than reflexively.
//
// THIS COUNTS THE SCHEMAS ONLY, NOT THE HOST'S SHELL GUIDANCE. Until
// 2026-09-07 it counted both, and that made it a different measurement on
// every machine: exec_command appends shell guidance whose size depends on
// which shell was detected, so the same tree measured 3498 on Linux, ~3600 on
// a Windows host with pwsh 7, and 3653 on Windows PowerShell 5.1, which is
// stock Windows. The 5.1 case has been over this ceiling since view_image
// landed, so `go test ./...` failed for anyone on a stock Windows box while
// CI stayed green, because the runners have pwsh 7 and take the cheaper
// branch. A budget that passes or fails on which PowerShell happens to be
// installed is not ratcheting anything.
maxEagerToolSchemaTokens = 3650
// The host-specific half, ratcheted on its own so it cannot creep either.
// Measured 2026-09-07: Windows PowerShell 5.1 is the most expensive host at
// 155 tokens (722 chars), pwsh 7 is 110, cmd.exe is 87, and every POSIX host
// is 0 because no guidance is appended there. Set above the worst case, not
// above whatever this machine happens to be.
maxExecCommandShellGuidanceTokens = 200
)
func TestSystemPromptTokenBudget(t *testing.T) {
@@ -56,8 +75,35 @@ func TestEagerToolSchemaTokenBudget(t *testing.T) {
// Options{} keeps DeferThreshold at 0, so deferral is inactive and every core
// tool is exposed eagerly — exactly what a plugin-free session sends each turn.
exposed, _ := partitionTools(registry, PermissionModeAuto, Options{}, map[string]bool{})
got := estimateToolDefTokens(exposed)
t.Logf("eager core tool schemas: %d tokens across %d tools", got, len(exposed))
total := estimateToolDefTokens(exposed)
// exec_command's description carries this host's shell guidance, and that
// guidance is the one part of the eager set that is not the same everywhere.
// Charged separately below rather than folded in here, or this ceiling would
// mean a different thing on every machine. See the constants above.
//
// Removed from the description BEFORE estimating rather than subtracted from
// the total afterwards. ApproxTextTokens is an integer division, so
// floor((schema+guidance)/4) - floor(guidance/4) comes out one token high
// whenever the two remainders add past four, and which side of that line a
// host lands on depends on the length of its guidance: exactly the
// host-dependence this number is supposed to be rid of.
guidance := tools.HostExecCommandShellGuidance()
schemas := make([]zeroruntime.ToolDefinition, len(exposed))
copy(schemas, exposed)
stripped := 0
for index := range schemas {
without := strings.TrimSuffix(schemas[index].Description, guidance)
if without != schemas[index].Description {
stripped++
}
schemas[index].Description = without
}
if guidance != "" && stripped != 1 {
t.Fatalf("SETUP INVALID: this host's shell guidance came off %d descriptions, want exactly exec_command's; internal/tools pins it as that description's tail", stripped)
}
got := estimateToolDefTokens(schemas)
guidanceTokens := ApproxTextTokens(guidance)
t.Logf("eager core tool schemas: %d tokens across %d tools (%d total, %d host shell guidance)", got, len(exposed), total, guidanceTokens)
if got > maxEagerToolSchemaTokens {
// NOT "defer a tool": this test pins DeferThreshold at 0, so marking a
// tool deferred leaves it exposed here and changes nothing. Deferral is
@@ -66,6 +112,9 @@ func TestEagerToolSchemaTokenBudget(t *testing.T) {
// deliberate raise.
t.Fatalf("eager tool schemas are %d tokens, over the %d ceiling — trim a schema, drop a core tool, or raise the ceiling deliberately (deferring will NOT help: this test disables deferral)", got, maxEagerToolSchemaTokens)
}
if guidanceTokens > maxExecCommandShellGuidanceTokens {
t.Fatalf("this host's exec_command shell guidance is %d tokens, over the %d ceiling — trim the guidance in internal/tools/shell_runtime.go or raise the ceiling deliberately", guidanceTokens, maxExecCommandShellGuidanceTokens)
}
}
func TestAgentAdvertisesBothEditTools(t *testing.T) {
+75 -15
View File
@@ -10,9 +10,11 @@ import (
"github.com/Gitlawb/zero/internal/acp"
"github.com/Gitlawb/zero/internal/agent"
"github.com/Gitlawb/zero/internal/config"
"github.com/Gitlawb/zero/internal/execution"
"github.com/Gitlawb/zero/internal/mcp"
"github.com/Gitlawb/zero/internal/providermodeldiscovery"
"github.com/Gitlawb/zero/internal/redaction"
"github.com/Gitlawb/zero/internal/sandbox"
"github.com/Gitlawb/zero/internal/tools"
)
const acpUsage = `zero acp — serve the Agent Client Protocol (ACP) over stdio
@@ -57,20 +59,8 @@ func runACP(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int
// surface — no ACP-specific credential handling needed.
NewProvider: deps.newProvider,
RunAgent: agent.Run,
// Build the SCOPED registry + sandbox engine per workspace, exactly like the
// exec surface, so ACP shell/file tools are confined — never run unconfined.
BuildWorkspace: func(workspaceRoot string, resolved config.ResolvedConfig) (*tools.Registry, *sandbox.Engine, error) {
scope, err := sandbox.NewScope(workspaceRoot, resolved.Sandbox.AdditionalWriteRoots)
if err != nil {
return nil, nil, err
}
engine, err := buildExecSandboxEngine(workspaceRoot, resolved, deps, scope)
if err != nil {
return nil, nil, err
}
registry := newCoreRegistryScoped(workspaceRoot, scope)
registerLocalControlTools(registry, workspaceRoot, resolved.LocalControl)
return registry, engine, nil
BuildWorkspace: func(ctx context.Context, workspaceRoot string, resolved config.ResolvedConfig, mode agent.PermissionMode) (*acp.Workspace, error) {
return buildACPWorkspace(ctx, workspaceRoot, resolved, mode, deps)
},
ResolveWorkspaceRoot: acpWorkspaceRootResolver(deps),
Store: deps.newSessionStore(),
@@ -85,6 +75,76 @@ func runACP(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int
return exitSuccess
}
// buildACPWorkspace matches exec's registry construction for one ACP turn. MCP
// servers use the same sandbox-prepared execution runner, project configuration is
// gated by the validated workspace's trust state, and their runtime is released
// when acp.Agent completes the turn. ACP deliberately stays at low autonomy: an
// editor connection never upgrades MCP permissions beyond an interactive prompt.
func buildACPWorkspace(ctx context.Context, workspaceRoot string, resolved config.ResolvedConfig, mode agent.PermissionMode, deps appDeps) (*acp.Workspace, error) {
scope, err := sandbox.NewScope(workspaceRoot, resolved.Sandbox.AdditionalWriteRoots)
if err != nil {
return nil, err
}
engine, err := buildExecSandboxEngine(workspaceRoot, resolved, deps, scope)
if err != nil {
return nil, err
}
registry := newCoreRegistryScoped(workspaceRoot, scope)
workspace := &acp.Workspace{
Registry: registry, Sandbox: engine,
DeferThreshold: resolved.Tools.DeferThreshold,
}
if mode != agent.PermissionModePlan {
// MCP stdio servers are subprocesses. Passing the engine to their runner
// keeps them inside the exact sandbox / lifecycle path used by zero exec.
runtime, trustSkip, err := registerMCPToolsForWorkspaceWithOptions(
ctx, workspaceRoot, registry, deps, mcp.AutonomyLow, workspaceRoot,
mcp.RegisterOptions{Execution: execution.NewRunner(engine), AdvertiseInAuto: true},
)
if err != nil {
// RegisterTools may have connected an earlier server before reporting a
// later failure, so do not orphan a partial runtime on this error path.
if runtime != nil {
_ = runtime.Close()
}
return nil, err
}
workspace.Cleanup = runtime.Close
workspace.Notices = acpMCPSetupNotices(trustSkip, runtime)
}
registerLocalControlTools(registry, workspaceRoot, resolved.LocalControl)
// MCP tools are deferred-eligible. Register their loader only after every
// ACP-visible tool is present, using the same mode the agent receives.
registerToolSearchIfEligible(registry, resolved.Tools.DeferThreshold, mode, nil, nil)
return workspace, nil
}
func acpMCPSetupNotices(skip trustSkip, runtime mcpToolRuntime) []string {
var notices []string
if skip.excludedProjectConfig {
if skip.trustCheckErrored {
notices = append(notices, "The workspace-trust store could not be read; project MCP servers were ignored (fail-closed). Run 'zero trust' to enable them.")
} else {
notices = append(notices, "Project MCP servers were ignored in this untrusted workspace. Run 'zero trust' to enable them.")
}
}
if runtime == nil {
return notices
}
for _, skipped := range runtime.Skipped() {
if skipped.UnconfiguredDefault {
continue
}
message := fmt.Sprintf("MCP server %s unavailable, skipped", skipped.Name)
if skipped.Err != nil {
message += ": " + redaction.ErrorMessage(skipped.Err, redaction.Options{})
}
notices = append(notices, redaction.RedactString(message, redaction.Options{}))
}
return notices
}
// acpWorkspaceRootResolver validates a client-supplied cwd into a confinement
// root. It reuses exec's resolveWorkspaceRoot (abs+clean, must be an existing
// dir) and additionally rejects the filesystem root and the home directory — an
+201
View File
@@ -5,10 +5,19 @@ import (
"context"
"errors"
"io"
"os"
"path/filepath"
"strings"
"sync"
"testing"
"time"
"github.com/Gitlawb/zero/internal/agent"
"github.com/Gitlawb/zero/internal/config"
"github.com/Gitlawb/zero/internal/mcp"
"github.com/Gitlawb/zero/internal/sandbox"
"github.com/Gitlawb/zero/internal/tools"
"github.com/Gitlawb/zero/internal/workspacetrust"
)
type acpTestReader func([]byte) (int, error)
@@ -108,6 +117,198 @@ func TestRunACPIdleCancellationExitsCleanly(t *testing.T) {
}
}
func TestBuildACPWorkspaceRegistersSandboxedMCPToolsAndClosesThem(t *testing.T) {
setTrustConfigRoot(t)
workspaceRoot := t.TempDir()
if err := workspacetrust.Trust(workspaceRoot); err != nil {
t.Fatalf("trust workspace: %v", err)
}
grantStore, err := sandbox.NewGrantStore(sandbox.StoreOptions{FilePath: t.TempDir() + "/grants.json"})
if err != nil {
t.Fatalf("new grant store: %v", err)
}
var gotExclude, registered, closed bool
deps := fillAppDeps(appDeps{
resolveMCPConfig: func(root string, excludeProject bool) (config.MCPConfig, error) {
if root != workspaceRoot {
t.Fatalf("MCP workspace root = %q, want %q", root, workspaceRoot)
}
gotExclude = excludeProject
return config.MCPConfig{Servers: map[string]config.MCPServerConfig{
"docs": {Type: "stdio", Command: "fake-docs"},
}}, nil
},
newMCPStore: func() (*mcp.PermissionStore, error) { return nil, nil },
newSandboxStore: func() (*sandbox.GrantStore, error) { return grantStore, nil },
registerMCPTools: func(ctx context.Context, registry *tools.Registry, cfg config.MCPConfig, options mcp.RegisterOptions) (mcpToolRuntime, error) {
if ctx == nil {
t.Fatal("MCP registration received nil context")
}
if len(cfg.Servers) != 1 || cfg.Servers["docs"].Command != "fake-docs" {
t.Fatalf("MCP config = %+v", cfg)
}
if options.Autonomy != mcp.AutonomyLow {
t.Fatalf("MCP autonomy = %q, want low", options.Autonomy)
}
if options.Execution == nil {
t.Fatal("MCP stdio server was not given the sandbox execution runner")
}
if options.WorkspaceRoot != workspaceRoot {
t.Fatalf("MCP execution workspace = %q, want %q", options.WorkspaceRoot, workspaceRoot)
}
if !options.AdvertiseInAuto {
t.Fatal("ACP MCP tools must be advertised in auto mode without being pre-approved")
}
registered = true
registry.Register(cliFakeDeferredTool{
name: "mcp_docs_search", permission: tools.PermissionPrompt,
advertiseInAuto: options.AdvertiseInAuto,
})
return closeFunc(func() error {
closed = true
return nil
}), nil
},
})
workspace, err := buildACPWorkspace(context.Background(), workspaceRoot, config.ResolvedConfig{
Tools: config.ToolsConfig{DeferThreshold: 1},
}, agent.PermissionModeAuto, deps)
if err != nil {
t.Fatalf("buildACPWorkspace: %v", err)
}
if gotExclude {
t.Fatal("trusted ACP workspace excluded project MCP configuration")
}
if !registered {
t.Fatal("ACP workspace did not register configured MCP tools")
}
if _, ok := workspace.Registry.Get("mcp_docs_search"); !ok {
t.Fatal("MCP tool is absent from ACP agent registry")
}
if _, ok := workspace.Registry.Get(tools.ToolSearchToolName); !ok {
t.Fatal("ACP registry is missing tool_search for deferred MCP tools")
}
if workspace.DeferThreshold != 1 {
t.Fatalf("workspace defer threshold = %d, want 1", workspace.DeferThreshold)
}
if err := workspace.Close(); err != nil {
t.Fatalf("close ACP workspace: %v", err)
}
if !closed {
t.Fatal("ACP workspace did not close its MCP runtime")
}
}
func TestBuildACPWorkspacePreservesRedactedMCPSetupNotices(t *testing.T) {
setTrustConfigRoot(t)
workspaceRoot := t.TempDir()
if err := os.MkdirAll(filepath.Join(workspaceRoot, ".zero"), 0o755); err != nil {
t.Fatal(err)
}
projectConfig := `{"mcp":{"servers":{"project-docs":{"type":"stdio","command":"project-docs"}}}}`
if err := os.WriteFile(filepath.Join(workspaceRoot, ".zero", "config.json"), []byte(projectConfig), 0o600); err != nil {
t.Fatal(err)
}
secret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
grantStore, err := sandbox.NewGrantStore(sandbox.StoreOptions{FilePath: filepath.Join(t.TempDir(), "grants.json")})
if err != nil {
t.Fatal(err)
}
deps := fillAppDeps(appDeps{
resolveMCPConfig: func(string, bool) (config.MCPConfig, error) {
return config.MCPConfig{Servers: map[string]config.MCPServerConfig{
"docs": {Type: "stdio", Command: "fake-docs"},
}}, nil
},
newMCPStore: func() (*mcp.PermissionStore, error) { return nil, nil },
newSandboxStore: func() (*sandbox.GrantStore, error) { return grantStore, nil },
registerMCPTools: func(context.Context, *tools.Registry, config.MCPConfig, mcp.RegisterOptions) (mcpToolRuntime, error) {
return fakeMCPRuntimeWithSkips{skipped: []mcp.SkippedServer{{
Name: "docs", Err: errors.New("token=" + secret),
}}}, nil
},
})
workspace, err := buildACPWorkspace(context.Background(), workspaceRoot, config.ResolvedConfig{}, agent.PermissionModeAuto, deps)
if err != nil {
t.Fatal(err)
}
defer workspace.Close()
joined := strings.Join(workspace.Notices, "\n")
if !strings.Contains(joined, "untrusted workspace") || !strings.Contains(joined, "MCP server docs unavailable") {
t.Fatalf("workspace notices = %#v", workspace.Notices)
}
if strings.Contains(joined, secret) || !strings.Contains(joined, "[REDACTED]") {
t.Fatalf("workspace notices leaked secret: %q", joined)
}
}
func TestBuildACPWorkspaceSkipsMCPInPlanMode(t *testing.T) {
grantStore, err := sandbox.NewGrantStore(sandbox.StoreOptions{FilePath: t.TempDir() + "/grants.json"})
if err != nil {
t.Fatalf("new grant store: %v", err)
}
called := false
deps := fillAppDeps(appDeps{
resolveMCPConfig: func(string, bool) (config.MCPConfig, error) {
called = true
return config.MCPConfig{}, nil
},
newSandboxStore: func() (*sandbox.GrantStore, error) { return grantStore, nil },
registerMCPTools: func(context.Context, *tools.Registry, config.MCPConfig, mcp.RegisterOptions) (mcpToolRuntime, error) {
called = true
return nil, nil
},
})
workspace, err := buildACPWorkspace(context.Background(), t.TempDir(), config.ResolvedConfig{}, agent.PermissionModePlan, deps)
if err != nil {
t.Fatalf("buildACPWorkspace: %v", err)
}
if called {
t.Fatal("plan-mode ACP workspace must not resolve or start MCP servers")
}
if err := workspace.Close(); err != nil {
t.Fatalf("close plan workspace: %v", err)
}
}
func TestBuildACPWorkspaceClosesPartialMCPRuntimeOnRegistrationError(t *testing.T) {
grantStore, err := sandbox.NewGrantStore(sandbox.StoreOptions{FilePath: t.TempDir() + "/grants.json"})
if err != nil {
t.Fatalf("new grant store: %v", err)
}
closed := false
deps := fillAppDeps(appDeps{
resolveMCPConfig: func(string, bool) (config.MCPConfig, error) {
return config.MCPConfig{Servers: map[string]config.MCPServerConfig{
"partial": {Type: "stdio", Command: "fake-partial"},
}}, nil
},
newMCPStore: func() (*mcp.PermissionStore, error) { return nil, nil },
newSandboxStore: func() (*sandbox.GrantStore, error) { return grantStore, nil },
registerMCPTools: func(context.Context, *tools.Registry, config.MCPConfig, mcp.RegisterOptions) (mcpToolRuntime, error) {
return closeFunc(func() error {
closed = true
return nil
}), errors.New("second MCP server failed")
},
})
workspace, err := buildACPWorkspace(context.Background(), t.TempDir(), config.ResolvedConfig{}, agent.PermissionModeAuto, deps)
if err == nil {
t.Fatal("buildACPWorkspace succeeded after MCP registration error")
}
if workspace != nil {
t.Fatal("buildACPWorkspace returned a workspace after MCP registration error")
}
if !closed {
t.Fatal("partial MCP runtime was not closed after registration error")
}
}
type acpNotifyingReadCloser struct {
io.ReadCloser
readStarted chan<- struct{}
+8 -2
View File
@@ -18,14 +18,20 @@ import (
// cliFakeDeferredTool is deferred-eligible (implements Deferred() bool), mirroring
// an MCP registry tool, so it counts toward the deferral threshold.
type cliFakeDeferredTool struct {
name string
name string
permission tools.Permission
advertiseInAuto bool
}
func (t cliFakeDeferredTool) Name() string { return t.name }
func (t cliFakeDeferredTool) Description() string { return "fake deferred tool" }
func (t cliFakeDeferredTool) Parameters() tools.Schema { return tools.Schema{Type: "object"} }
func (t cliFakeDeferredTool) Safety() tools.Safety {
return tools.Safety{SideEffect: tools.SideEffectNetwork, Permission: tools.PermissionAllow}
permission := t.permission
if permission == "" {
permission = tools.PermissionAllow
}
return tools.Safety{SideEffect: tools.SideEffectNetwork, Permission: permission, AdvertiseInAuto: t.advertiseInAuto}
}
func (t cliFakeDeferredTool) Run(context.Context, map[string]any) tools.Result {
return tools.Result{Status: tools.StatusOK, Output: "ok"}
+12 -10
View File
@@ -31,6 +31,14 @@ type mcpToolListItem struct {
// into the one-line trust notice (mirroring the hooks and plugins chokepoints);
// otherwise a workspace whose only project config is MCP would be gated silently.
func registerMCPToolsForWorkspace(ctx context.Context, workspaceRoot string, registry *tools.Registry, deps appDeps, autonomy mcp.PermissionAutonomy, trustRoot string, runners ...*execution.Runner) (mcpToolRuntime, trustSkip, error) {
options := mcp.RegisterOptions{}
if len(runners) > 0 {
options.Execution = runners[0]
}
return registerMCPToolsForWorkspaceWithOptions(ctx, workspaceRoot, registry, deps, autonomy, trustRoot, options)
}
func registerMCPToolsForWorkspaceWithOptions(ctx context.Context, workspaceRoot string, registry *tools.Registry, deps appDeps, autonomy mcp.PermissionAutonomy, trustRoot string, options mcp.RegisterOptions) (mcpToolRuntime, trustSkip, error) {
excludeProject, trustCheckErrored := resolveTrust(trustRoot)
skip := trustSkip{
excludedProjectConfig: excludeProject && projectMCPConfigExists(workspaceRoot),
@@ -47,16 +55,10 @@ func registerMCPToolsForWorkspace(ctx context.Context, workspaceRoot string, reg
if err != nil {
return nil, skip, err
}
var runner *execution.Runner
if len(runners) > 0 {
runner = runners[0]
}
runtime, err := deps.registerMCPTools(ctx, registry, cfg, mcp.RegisterOptions{
PermissionStore: store,
Autonomy: autonomy,
Execution: runner,
WorkspaceRoot: workspaceRoot,
})
options.PermissionStore = store
options.Autonomy = autonomy
options.WorkspaceRoot = workspaceRoot
runtime, err := deps.registerMCPTools(ctx, registry, cfg, options)
return runtime, skip, err
}
+7 -3
View File
@@ -23,6 +23,9 @@ const defaultConnectTimeout = 8 * time.Second
type RegisterOptions struct {
PermissionStore *PermissionStore
Autonomy PermissionAutonomy
// AdvertiseInAuto exposes prompt-gated MCP tools to model surfaces whose
// default mode is auto while preserving the execution-time prompt.
AdvertiseInAuto bool
ClientFactory func(context.Context, Server) (ToolClient, error)
// ConnectTimeout bounds the per-server connect+list at startup. Zero uses
// defaultConnectTimeout.
@@ -271,9 +274,10 @@ func newRegistryTool(server Server, remote RemoteTool, client ToolClient, option
client: client,
parameters: SchemaFromMCP(remote.InputSchema),
safety: tools.Safety{
SideEffect: tools.SideEffectNetwork,
Permission: permission,
Reason: fmt.Sprintf("MCP tool %s/%s runs through the configured %s server.", server.Name, remote.Name, server.Type),
SideEffect: tools.SideEffectNetwork,
Permission: permission,
Reason: fmt.Sprintf("MCP tool %s/%s runs through the configured %s server.", server.Name, remote.Name, server.Type),
AdvertiseInAuto: options.AdvertiseInAuto,
},
}
}
+13
View File
@@ -342,6 +342,19 @@ func TestRegistryToolIsDeferredEligible(t *testing.T) {
}
}
func TestRegistryToolCanAdvertiseInAutoWithoutPreapproval(t *testing.T) {
tool := newRegistryTool(
Server{Name: "docs", Type: "stdio"},
RemoteTool{Name: "search"},
&fakeToolClient{},
RegisterOptions{Autonomy: AutonomyLow, AdvertiseInAuto: true},
)
safety := tool.Safety()
if safety.Permission != tools.PermissionPrompt || !safety.AdvertiseInAuto {
t.Fatalf("MCP safety = %#v", safety)
}
}
// TestRegistryToolReportsMCPServerName verifies the registryTool reports its true
// configured server name (not the sanitized tool-name token) so the deferred-tools
// discovery label names a multi-token server correctly via tools.DeferredSource.
+136
View File
@@ -0,0 +1,136 @@
package sessions
import (
"encoding/json"
"strings"
"testing"
)
// ONE SESSION HAS TO RENDER ONE PROMPT.
//
// extractText walked a map[string]any directly, and Go randomizes map
// iteration, so the resume context put the same fields in a different order in
// every process: `c1 read_file {"path":...}` one run, `{"path":...} c1
// read_file` the next. That block is part of the user prompt, so the prompt
// itself changed run to run, which leaves a provider nothing stable to
// prefix-cache and makes two resumes of one session impossible to diff.
//
// PINNED AS AN EXACT STRING, NOT AS TWO RUNS AGREEING. A test that renders the
// same payload twice in one process and compares the results passes whenever
// the randomized order happens to repeat, which for a small map is often. The
// only assertion that cannot pass by luck names the order it expects.
func TestSummarizePayloadRendersFieldsInKeyOrder(t *testing.T) {
payload := map[string]any{
"id": "c1",
"name": "read_file",
"arguments": `{"path":"deploy/prod.env"}`,
"status": "ok",
}
// Keys sorted: arguments, id, name, status.
const want = `{"path":"deploy/prod.env"} c1 read_file ok`
for attempt := range 50 {
if got := summarizePayload(payload); got != want {
t.Fatalf("attempt %d rendered %q, want %q", attempt, got, want)
}
}
}
// The same for a payload that arrives as raw JSON, which is the shape the store
// hands back, since that decodes into a map and takes the same path.
func TestSummarizePayloadRendersRawJSONInKeyOrder(t *testing.T) {
raw, err := json.Marshal(map[string]any{
"zulu": "last",
"alpha": "first",
"mike": "middle",
})
if err != nil {
t.Fatal(err)
}
const want = "first middle last"
for attempt := range 50 {
if got := summarizePayload(json.RawMessage(raw)); got != want {
t.Fatalf("attempt %d rendered %q, want %q", attempt, got, want)
}
}
}
// The summary short-circuit is untouched: a payload carrying one still answers
// with it alone, rather than with every field in key order.
func TestSummarizePayloadStillPrefersASummary(t *testing.T) {
payload := map[string]any{
"alpha": "not this",
"summary": "this one",
"zulu": "nor this",
}
if got := summarizePayload(payload); got != "this one" {
t.Fatalf("rendered %q, want the summary alone", got)
}
}
// And a list keeps the order it was written in, which is the caller's and not
// this function's to sort.
func TestSummarizePayloadKeepsListOrder(t *testing.T) {
payload := map[string]any{"items": []any{"third", "first", "second"}}
if got := summarizePayload(payload); got != "third first second" {
t.Fatalf("rendered %q, want the list in its written order", got)
}
}
// THE WHOLE BLOCK IS STABLE, not just one payload. Rendered through the
// prompt-context path a resume actually uses, so a future renderer that walks a
// map of its own is caught here too.
//
// THE EXPECTED BLOCK IS WRITTEN OUT, NOT TAKEN FROM THE FIRST RENDER. Comparing
// later renders against the first only asks whether the renderer is stable, and
// a renderer that is stably wrong passes: any fixed field order satisfies it,
// including the one this change replaces. It also hid something. The first
// version of this test used tool events, which promptContextEvents filters out
// of the resume context on this branch, so the block being compared was a
// single line and nobody could tell. Naming the expected block makes both
// failures visible.
func TestResumeContextBlockIsIdenticalAcrossRenders(t *testing.T) {
events := []Event{
{Sequence: 1, Type: EventMessage, Payload: mustOrderPayload(t, map[string]any{"role": "user", "content": "rotate the deploy key"})},
{Sequence: 2, Type: EventMessage, Payload: mustOrderPayload(t, map[string]any{"role": "assistant", "content": "reading the env file"})},
{Sequence: 3, Type: EventMessage, Payload: mustOrderPayload(t, map[string]any{"role": "user", "content": "and the hooks"})},
}
// Keys sorted, so content comes before role in every line.
want := strings.Join([]string{
"message: rotate the deploy key user",
"message: reading the env file assistant",
"message: and the hooks user",
}, "\n")
for attempt := range 50 {
if got := renderOrderContext(t, events); got != want {
t.Fatalf("attempt %d rendered:\n%s\nwant:\n%s", attempt, got, want)
}
}
}
func renderOrderContext(t *testing.T, events []Event) string {
t.Helper()
lines := []string{}
for _, event := range promptContextEvents(events) {
var decoded any
if len(event.Payload) > 0 {
if err := json.Unmarshal(event.Payload, &decoded); err != nil {
t.Fatal(err)
}
}
lines = append(lines, string(event.Type)+": "+summarizePayload(decoded))
}
return strings.Join(lines, "\n")
}
func mustOrderPayload(t *testing.T, payload map[string]any) json.RawMessage {
t.Helper()
raw, err := json.Marshal(payload)
if err != nil {
t.Fatal(err)
}
return raw
}
+19 -3
View File
@@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"log"
"sort"
"strings"
"unicode/utf8"
)
@@ -267,9 +268,24 @@ func extractText(value any) string {
if summary, ok := typed["summary"].(string); ok && strings.TrimSpace(summary) != "" {
return summary
}
parts := []string{}
for _, item := range typed {
if text := extractText(item); text != "" {
// BY KEY, BECAUSE THIS TEXT BECOMES PART OF A PROMPT. Go randomizes map
// iteration, so the same session rendered its resume context in a
// different field order in every process: one run said
// `c1 read_file {"path":...}` and the next `{"path":...} c1 read_file`.
// The block goes into the user prompt, so the prompt itself changed run
// to run, which gives a provider nothing stable to prefix-cache and
// leaves two resumes of one session impossible to diff while debugging.
//
// Only the ordering is decided here. Which fields survive is the
// caller's projection, and a slice keeps the order it was written in.
keys := make([]string, 0, len(typed))
for key := range typed {
keys = append(keys, key)
}
sort.Strings(keys)
parts := make([]string, 0, len(keys))
for _, key := range keys {
if text := extractText(typed[key]); text != "" {
parts = append(parts, text)
}
}
+17 -5
View File
@@ -75,14 +75,10 @@ func NewScopedExecCommandTool(workspaceRoot string, scope PathScope, manager *ex
if manager == nil {
manager = defaultExecSessionManager
}
description := "Runs a command in a PTY, returning output or a session ID for ongoing interaction."
if runtimeGOOS() == "windows" {
description += "\n\n" + shellGuidanceForGOOS(runtimeGOOS())
}
return execCommandTool{
baseTool: baseTool{
name: ExecCommandToolName,
description: description,
description: execCommandDescription(runtimeGOOS()),
parameters: Schema{
Type: "object",
Properties: map[string]PropertySchema{
@@ -674,3 +670,19 @@ func execDisplaySummary(commandText string, sessionID int, exited bool, exitCode
func runtimeGOOS() string {
return runtime.GOOS
}
// execCommandDescription is exec_command's description as a host running goos
// would see it. The shell guidance is appended on Windows only.
//
// Taking goos as an argument rather than reading it is what lets the guidance
// contract be checked for every platform from any one of them. internal/agent's
// token ratchet removes exactly this appended text before charging the schemas,
// so the condition here and hostExecCommandShellGuidance have to agree on all
// three platforms, not only on whichever one the test happens to run on.
func execCommandDescription(goos string) string {
description := "Runs a command in a PTY, returning output or a session ID for ongoing interaction."
if goos == "windows" {
description += "\n\n" + shellGuidanceForGOOS(goos)
}
return description
}
@@ -0,0 +1,83 @@
package tools
import (
"runtime"
"strings"
"testing"
)
// THE BUDGET TEST REMOVES THIS TEXT, SO IT HAS TO BE EXACTLY WHAT IS ADDED.
//
// internal/agent's eager-schema ratchet charges the tool schemas and this
// host's shell guidance separately, and it separates them by stripping
// HostExecCommandShellGuidance off exec_command's description before
// estimating. That is only valid while the guidance is appended verbatim as the
// tail of that description, and only while a platform that reports no guidance
// really appends none. If the two drift apart the ratchet keeps reporting a
// number, just not the one it claims.
//
// Checked for all three platforms rather than for the running one: the
// appending is itself conditional on GOOS, so a host-only check is the shape of
// bug this pins. Off Windows it would pass while reporting nothing at all.
func TestExecCommandDescriptionMatchesReportedGuidance(t *testing.T) {
for _, goos := range []string{"windows", "linux", "darwin"} {
t.Run(goos, func(t *testing.T) {
description := execCommandDescription(goos)
guidance := hostExecCommandShellGuidance(goos)
if goos == "windows" {
if guidance == "" {
t.Fatal("exec_command appends shell guidance on Windows, but the accessor reports none; the budget would charge it to the schemas")
}
if !strings.HasSuffix(description, guidance) {
t.Fatalf("the guidance is not the tail of the description, so removing it from the schema total is wrong.\nguidance: %q\ndescription tail: %q",
guidance, description[max(0, len(description)-len(guidance)-40):])
}
} else if guidance != "" {
t.Fatalf("no shell guidance is appended off Windows, but the accessor reported %q", guidance)
}
// Whatever guidance THIS platform would produce, none of it may be
// left in the description once the reported guidance is removed.
// Off Windows the accessor reports nothing, so this is the check
// that the description really carries nothing: drop the GOOS
// condition in execCommandDescription and a POSIX host starts
// appending its own guidance (about 150 characters on darwin) with
// the ratchet still charging every byte of it to the schemas.
platformGuidance := shellGuidanceForGOOS(goos)
if platformGuidance == "" {
t.Fatalf("SETUP INVALID: %s produces no shell guidance at all, so the check below cannot fail", goos)
}
if remainder := strings.TrimSuffix(description, guidance); strings.Contains(remainder, platformGuidance) {
t.Fatalf("exec_command's description still carries %s shell guidance after removing the %d characters the accessor reports, so the ratchet charges the rest to the tool schemas.\nremainder: %q",
goos, len(guidance), remainder)
}
})
}
}
// And the running host agrees with the table above, so the exported accessor
// and the real constructor cannot drift away from the platform contract.
func TestHostExecCommandShellGuidanceMatchesTheDescription(t *testing.T) {
tool := NewExecCommandTool(t.TempDir(), nil)
if got, want := tool.Description(), execCommandDescription(runtime.GOOS); got != want {
t.Fatalf("the constructed tool's description is not what the platform contract says.\ngot: %q\nwant: %q", got, want)
}
if got, want := HostExecCommandShellGuidance(), hostExecCommandShellGuidance(runtime.GOOS); got != want {
t.Fatalf("the exported accessor disagrees with the platform contract.\ngot: %q\nwant: %q", got, want)
}
}
// And the guidance a Windows host gets is never empty, whichever shell was
// detected, since an empty one would mean the model gets no syntax rule at all.
func TestWindowsShellGuidanceIsNeverEmpty(t *testing.T) {
for _, shell := range []shellRuntime{
{GOOS: "windows", Executable: `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`, Kind: shellKindPowerShell, Syntax: "PowerShell"},
{GOOS: "windows", Executable: `C:\Program Files\PowerShell\7\pwsh.exe`, Kind: shellKindPowerShell, Syntax: "PowerShell"},
{GOOS: "windows", Executable: `C:\Windows\System32\cmd.exe`, Kind: shellKindCmd, Syntax: "cmd.exe"},
} {
if guidance := shellGuidanceForRuntime(shell); strings.TrimSpace(guidance) == "" {
t.Errorf("%s produced no shell guidance", shell.Executable)
}
}
}
+22
View File
@@ -181,6 +181,28 @@ func shellGuidanceForRuntime(shell shellRuntime) string {
return guidance
}
// HostExecCommandShellGuidance returns the shell guidance appended to
// exec_command's description on this host, or "" where none is appended.
//
// Exported for the per-turn token ratchet in internal/agent. That budget has to
// separate what every host pays for the tool schemas from what THIS host pays
// for its shell guidance, because the guidance varies by shell and by
// PowerShell version, and a single number covering both silently means a
// different thing on each machine.
func HostExecCommandShellGuidance() string {
return hostExecCommandShellGuidance(runtimeGOOS())
}
// hostExecCommandShellGuidance is the guidance a host running goos appends. It
// is the pair of execCommandDescription, so the two can be checked against each
// other for every platform rather than only for the running one.
func hostExecCommandShellGuidance(goos string) string {
if goos != "windows" {
return ""
}
return shellGuidanceForGOOS(goos)
}
// HostShellEnvironmentGuidance returns the concise, model-facing shell rule
// for the current host.
func HostShellEnvironmentGuidance() string {