feat(specialist): zeromaxing posture and orchestrate plan execution

internal/agent/child_progress_test.go

@@ -0,0 +1,135 @@
+package agent
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Gitlawb/zero/internal/streamjson"
+	"github.com/Gitlawb/zero/internal/tools"
+)
+
+// progressProbeTool records whether the loop handed it a progress callback.
+// streams mirrors what a real tool declares via tools.ChildProgressStreamer.
+type progressProbeTool struct {
+	name    string
+	streams bool
+	got     *bool
+}
+
+func (t *progressProbeTool) Name() string        { return t.name }
+func (t *progressProbeTool) Description() string { return "probe" }
+func (t *progressProbeTool) Parameters() tools.Schema {
+	return tools.Schema{Type: "object", Properties: map[string]tools.PropertySchema{}}
+}
+func (t *progressProbeTool) Safety() tools.Safety {
+	return tools.Safety{SideEffect: tools.SideEffectRead, Permission: tools.PermissionAllow}
+}
+func (t *progressProbeTool) Run(context.Context, map[string]any) tools.Result {
+	*t.got = false
+	return tools.Result{Status: tools.StatusOK, Output: "ok"}
+}
+func (t *progressProbeTool) RunWithOptions(_ context.Context, _ map[string]any, options tools.RunOptions) tools.Result {
+	*t.got = options.Progress != nil
+	return tools.Result{Status: tools.StatusOK, Output: "ok"}
+}
+
+// StreamsChildProgress makes this type implement tools.ChildProgressStreamer;
+// the flag drives the answer. A tool that never declares at all is modelled by
+// silentProbeTool below, which genuinely does not implement the interface.
+func (t *progressProbeTool) StreamsChildProgress() bool { return t.streams }
+
+// silentProbeTool does NOT implement tools.ChildProgressStreamer at all — the
+// state every tool in the tree is in today except Task and orchestrate.
+type silentProbeTool struct {
+	name string
+	got  *bool
+}
+
+func (t *silentProbeTool) Name() string        { return t.name }
+func (t *silentProbeTool) Description() string { return "probe" }
+func (t *silentProbeTool) Parameters() tools.Schema {
+	return tools.Schema{Type: "object", Properties: map[string]tools.PropertySchema{}}
+}
+func (t *silentProbeTool) Safety() tools.Safety {
+	return tools.Safety{SideEffect: tools.SideEffectRead, Permission: tools.PermissionAllow}
+}
+func (t *silentProbeTool) Run(context.Context, map[string]any) tools.Result {
+	*t.got = false
+	return tools.Result{Status: tools.StatusOK, Output: "ok"}
+}
+func (t *silentProbeTool) RunWithOptions(_ context.Context, _ map[string]any, options tools.RunOptions) tools.Result {
+	*t.got = options.Progress != nil
+	return tools.Result{Status: tools.StatusOK, Output: "ok"}
+}
+
+func runProbe(t *testing.T, tool tools.Tool, got *bool, withSink bool) bool {
+	t.Helper()
+	registry := tools.NewRegistry()
+	registry.Register(tool)
+	options := Options{}
+	if withSink {
+		options.OnToolProgress = func(string, streamjson.Event) {}
+	}
+	result, err := executeToolCall(context.Background(),
+		registry,
+		ToolCall{ID: "call_1", Name: tool.Name(), Arguments: "{}"},
+		PermissionModeUnsafe,
+		options)
+	if err != nil {
+		t.Fatalf("executeToolCall: %v", err)
+	}
+	if result.Status == "" {
+		t.Fatalf("probe produced no result")
+	}
+	return *got
+}
+
+// THE PARITY OBLIGATION for un-gating the progress path.
+//
+// The relationship asserted here is EQUALITY, per RULES.md §3: a tool's
+// progress wiring before and after this change must be the same for every tool
+// that already had it and every tool that already lacked it. Only a tool that
+// newly DECLARES the interface may change.
+//
+// The name gate is gone, so the guarantee cannot be "Task still works" — it has
+// to be stated in terms of the declaration, which is what these cases do.
+func TestProgressCallbackFollowsTheDeclarationNotTheName(t *testing.T) {
+	t.Run("a declaring tool receives it (Task's behaviour, preserved)", func(t *testing.T) {
+		var got bool
+		// Named something other than "Task" ON PURPOSE: under the old name gate
+		// this case failed, which is the whole point of the change.
+		if !runProbe(t, &progressProbeTool{name: "spawner", streams: true, got: &got}, &got, true) {
+			t.Fatal("a tool declaring StreamsChildProgress must receive the callback")
+		}
+	})
+
+	t.Run("a tool named Task that does NOT declare gets nothing", func(t *testing.T) {
+		var got bool
+		// The inverse of the old behaviour, and the reason a name is the wrong
+		// key: identity is not capability.
+		if runProbe(t, &silentProbeTool{name: "Task", got: &got}, &got, true) {
+			t.Fatal("the callback must follow the declaration, not the name")
+		}
+	})
+
+	t.Run("a non-declaring tool receives nothing (every other tool, unchanged)", func(t *testing.T) {
+		var got bool
+		if runProbe(t, &silentProbeTool{name: "read_file", got: &got}, &got, true) {
+			t.Fatal("un-gating must not start handing a callback to tools that never had one")
+		}
+	})
+
+	t.Run("a declaring tool that answers false gets nothing", func(t *testing.T) {
+		var got bool
+		if runProbe(t, &progressProbeTool{name: "spawner", streams: false, got: &got}, &got, true) {
+			t.Fatal("StreamsChildProgress() == false must be honoured")
+		}
+	})
+
+	t.Run("no sink means no callback, whatever the tool declares", func(t *testing.T) {
+		var got bool
+		if runProbe(t, &progressProbeTool{name: "spawner", streams: true, got: &got}, &got, false) {
+			t.Fatal("without OnToolProgress there is nothing to forward to")
+		}
+	})
+}

internal/agent/export_test.go

@@ -2,6 +2,7 @@
 package agent
 
 import (
+	"github.com/Gitlawb/zero/internal/specialist"
 	"github.com/Gitlawb/zero/internal/tools"
 	"github.com/Gitlawb/zero/internal/zeroruntime"
 )
@@ -39,3 +40,15 @@ func parsePreservedState(summaryContent string) (string, []skillEntry) {
 func partitionTools(registry *tools.Registry, permissionMode PermissionMode, options Options, loaded map[string]bool) ([]zeroruntime.ToolDefinition, string) {
 	return partitionToolsCached(registry, permissionMode, options, loaded, nil)
 }
+
+// Phase 2 additivity-proof seam. The identity test uses the REAL orchestrate
+// tool rather than a stub, so it exercises the actual Deferred() contract that
+// enforces the posture-off constraint. internal/specialist does not import
+// internal/agent, so this direction creates no cycle.
+const phase2ToolName = specialist.OrchestrateToolName
+
+// registerPhase2ToolForTest registers the real tool with the posture OFF, which
+// is the condition the identity test is about.
+func registerPhase2ToolForTest(registry *tools.Registry) {
+	registry.Register(&specialist.OrchestrateTool{PostureActive: func() bool { return false }})
+}

internal/agent/loop.go

@@ -256,6 +256,19 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 	for turn := 0; turn < maxTurns; turn++ {
 		result.Turns = turn + 1
 
+		// The zeromaxing posture's reminders. Appended to the CONVERSATION tail
+		// as user-role messages — the same channel as the diagnostics nudge
+		// below and the failure/plan hints later in the turn — and never into
+		// the system prompt, which is built once per run and must stay
+		// byte-stable so the provider's cached prefix survives. See
+		// internal/agent/zeromaxing.go.
+		for _, reminder := range zeromaxingReminders(options.Zeromaxing, result.Turns, options.OrchestrateAvailable) {
+			messages = append(messages, zeroruntime.Message{
+				Role:    zeroruntime.MessageRoleUser,
+				Content: reminder,
+			})
+		}
+
 		// Deliver background post-edit diagnostics from the previous turn's edits
 		// BEFORE compaction so the nudge is part of the request being budgeted.
 		// A brief wait at most (asyncDiagnosticsDrainTimeout); an unfinished check
@@ -267,6 +280,19 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 			})
 		}
 
+		// A BACKGROUND PLAN that finished since the last turn. Same channel and
+		// the same point in the turn as the diagnostics nudge above: the model
+		// was told the plan was not finished and must not report it as done, so
+		// this is the message that makes that promise good.
+		if options.PlanCompletions != nil {
+			if finished := options.PlanCompletions(); finished != "" {
+				messages = append(messages, zeroruntime.Message{
+					Role:    zeroruntime.MessageRoleUser,
+					Content: finished,
+				})
+			}
+		}
+
 		// Build the per-turn tool list first so proactive compaction can include
 		// the tool-definition tokens (they ride on every request) in its estimate.
 		// partitionTools depends only on registry/permissions/options/loaded, not on
@@ -1333,10 +1359,17 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
 	}
 	args = shellExecutionArgsForApproval(call.Name, args, decisionAction, options)
 
-	// Task tool: wire progress callback so the TUI sees live tool-call events
-	// from the specialist child process.
+	// Wire the progress callback so the TUI sees live tool-call events from a
+	// child agent process.
+	//
+	// Keyed on the TOOL'S OWN DECLARATION (tools.ChildProgressStreamer), not on
+	// its name. This was `call.Name == "Task"`, which made the second
+	// sub-agent-spawning tool run invisibly; `|| call.Name == "orchestrate"`
+	// would have been the same defect one name later. A tool that spawns
+	// children declares it, and every tool that does not keeps the nil callback
+	// it has today.
 	var progressCallback func(streamjson.Event)
-	if call.Name == "Task" && options.OnToolProgress != nil {
+	if options.OnToolProgress != nil && tools.StreamsChildProgress(tool) {
 		toolCallID := call.ID
 		onProgress := options.OnToolProgress
 		progressCallback = func(event streamjson.Event) {
@@ -2710,7 +2743,7 @@ func availablePermissionDecisions(event PermissionEvent, args map[string]any, op
 				decisions = append(decisions, PermissionDecisionAlwaysAllowPrefix)
 			}
 		}
-		if options.Sandbox.CanPersistGrants() && permissionSupportsPersistentDecision(event.ToolName) && !filesystemSandboxPrompt(event) && !inlineAdditionalPermissions {
+		if options.Sandbox.CanPersistGrants() && permissionSupportsPersistentDecision(event.ToolName, options) && !filesystemSandboxPrompt(event) && !inlineAdditionalPermissions {
 			decisions = append(decisions, PermissionDecisionAlwaysAllow)
 		}
 	}
@@ -2881,7 +2914,17 @@ func grantFilesystemForSandboxPrompt(event PermissionEvent, scope sandbox.Permis
 	}, scope)
 }
 
-func permissionSupportsPersistentDecision(toolName string) bool {
+func permissionSupportsPersistentDecision(toolName string, options Options) bool {
+	// THE TOOL'S OWN DECLARATION FIRST. A name list cannot describe a tool whose
+	// reach depends on its arguments, and it cannot cover a tool that RUNS the
+	// ones already refused below — see tools.PersistentPermissionRefuser.
+	if options.Registry != nil {
+		if tool, found := options.Registry.Get(toolName); found {
+			if refuser, ok := tool.(tools.PersistentPermissionRefuser); ok && refuser.RefusesPersistentPermission() {
+				return false
+			}
+		}
+	}
 	switch toolName {
 	case "bash", "exec_command", "write_stdin", "apply_patch":
 		return false

internal/agent/loop_test.go

@@ -3756,3 +3756,137 @@ func TestRunNilTraceForwardsUsage(t *testing.T) {
 		t.Fatal("OnUsage not forwarded when Trace is nil")
 	}
 }
+
+// (h) The zeromaxing variant of TestRunPreservesRequestPrefixAcrossTurns.
+//
+// This is the test the whole reminder design exists to satisfy. The system
+// prompt and tool definitions are the provider's CACHED PREFIX; #760 made them
+// build once per run so they stay byte-identical across turns. The posture adds
+// per-turn text, so it is exactly the kind of change that silently destroys the
+// cache — roughly doubling input cost with nothing detecting it.
+//
+// The reminders are appended to the conversation TAIL as user-role messages, so
+// each turn's request must still be an exact prefix-extension of the previous
+// one, with identical tools and cache key.
+func TestRunPreservesRequestPrefixAcrossTurnsUnderZeromaxing(t *testing.T) {
+	root := t.TempDir()
+	writeAgentTestFile(t, filepath.Join(root, "notes.txt"), "alpha\n")
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewReadFileTool(root))
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		{
+			{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "read_file"},
+			{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"path":"notes.txt"}`},
+			{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"},
+			{Type: zeroruntime.StreamEventDone},
+		},
+		{
+			{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-2", ToolName: "read_file"},
+			{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-2", ArgumentsFragment: `{"path":"notes.txt"}`},
+			{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-2"},
+			{Type: zeroruntime.StreamEventDone},
+		},
+		{
+			{Type: zeroruntime.StreamEventText, Content: "done"},
+			{Type: zeroruntime.StreamEventDone},
+		},
+	}}
+
+	if _, err := Run(context.Background(), "read notes", provider, Options{
+		Cwd:        root,
+		Registry:   registry,
+		SessionID:  "session-stable-prefix-zeromaxing",
+		Zeromaxing: ZeromaxingEntering,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	if len(provider.requests) != 3 {
+		t.Fatalf("provider requests = %d, want 3", len(provider.requests))
+	}
+
+	// Every request must extend the previous one exactly — nothing rewritten
+	// above the append point, which is what keeps the cached prefix valid.
+	for i := 1; i < len(provider.requests); i++ {
+		prev, cur := provider.requests[i-1], provider.requests[i]
+		if len(cur.Messages) < len(prev.Messages) ||
+			!reflect.DeepEqual(cur.Messages[:len(prev.Messages)], prev.Messages) {
+			t.Fatalf("request %d is not an exact prefix-extension of request %d:\nprev=%#v\ncur=%#v",
+				i, i-1, prev.Messages, cur.Messages)
+		}
+		if !reflect.DeepEqual(prev.Tools, cur.Tools) {
+			t.Fatalf("tool definitions drifted between turns %d and %d", i-1, i)
+		}
+		if cur.PromptCacheKey != prev.PromptCacheKey {
+			t.Fatalf("prompt cache key drifted: %q -> %q", prev.PromptCacheKey, cur.PromptCacheKey)
+		}
+	}
+
+	// The system prompt — the cached prefix itself — must be byte-identical
+	// across turns, and must contain NONE of the posture text.
+	system := provider.requests[0].Messages[0]
+	for i, request := range provider.requests {
+		if !reflect.DeepEqual(request.Messages[0], system) {
+			t.Fatalf("system message changed on turn %d — the cached prefix is broken", i+1)
+		}
+	}
+	for _, forbidden := range []string{
+		ZeromaxingEnterNotice, ZeromaxingBudgetNotice, ZeromaxingStillOnNotice, ZeromaxingExitNotice,
+	} {
+		if strings.Contains(system.Content, forbidden) {
+			t.Fatalf("posture reminder leaked into the SYSTEM PROMPT (above the cache breakpoint): %q", forbidden)
+		}
+	}
+
+	// ...and the reminders really did arrive, on schedule. Without this the
+	// assertions above would pass just as happily if the feature did nothing.
+	firstTurn := renderZeromaxingMessages(provider.requests[0].Messages)
+	if !strings.Contains(firstTurn, ZeromaxingEnterNotice) || !strings.Contains(firstTurn, ZeromaxingBudgetNotice) {
+		t.Fatalf("turn 1 must carry the enter + budget notices:\n%s", firstTurn)
+	}
+	if strings.Contains(firstTurn, ZeromaxingStillOnNotice) {
+		t.Fatalf("turn 1 must NOT carry the still-on notice:\n%s", firstTurn)
+	}
+	secondOnly := renderZeromaxingMessages(provider.requests[1].Messages[len(provider.requests[0].Messages):])
+	if !strings.Contains(secondOnly, ZeromaxingStillOnNotice) {
+		t.Fatalf("turn 2 must carry the still-on notice:\n%s", secondOnly)
+	}
+	if strings.Contains(secondOnly, ZeromaxingEnterNotice) {
+		t.Fatalf("turn 2 must NOT re-announce entry:\n%s", secondOnly)
+	}
+}
+
+// A run with the posture OFF must be byte-identical to one that never heard of
+// it — the no-regression half of the feature.
+func TestRunWithoutZeromaxingCarriesNoPostureText(t *testing.T) {
+	root := t.TempDir()
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewReadFileTool(root))
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		{{Type: zeroruntime.StreamEventText, Content: "done"}, {Type: zeroruntime.StreamEventDone}},
+	}}
+	if _, err := Run(context.Background(), "hello", provider, Options{
+		Cwd: root, Registry: registry, SessionID: "session-no-zeromaxing",
+	}); err != nil {
+		t.Fatal(err)
+	}
+	rendered := renderZeromaxingMessages(provider.requests[0].Messages)
+	for _, forbidden := range []string{
+		ZeromaxingEnterNotice, ZeromaxingBudgetNotice, ZeromaxingStillOnNotice, ZeromaxingExitNotice,
+	} {
+		if strings.Contains(rendered, forbidden) {
+			t.Fatalf("a posture-free run must carry no posture text, found %q", forbidden)
+		}
+	}
+}
+
+// renderZeromaxingMessages flattens messages to one searchable string.
+func renderZeromaxingMessages(messages []zeroruntime.Message) string {
+	var b strings.Builder
+	for _, message := range messages {
+		b.WriteString(string(message.Role))
+		b.WriteString(": ")
+		b.WriteString(message.Content)
+		b.WriteString("\n")
+	}
+	return b.String()
+}