feat(specialist): zeromaxing posture and orchestrate plan execution

internal/acp/agent.go

@@ -253,6 +253,7 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
 		Cwd:            sess.cwd,
 		SessionID:      sess.id,
 		ProviderName:   resolved.Provider.Name,
+		ModelFamily:    providercatalog.ModelFamilyFor(resolved.Provider.CatalogID),
 		Model:          resolved.Provider.Model,
 		Registry:       registry,
 		Sandbox:        sandboxEngine,

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/guardrails.go

@@ -168,7 +168,14 @@ var selfReportPhrases = []string{
 var inabilityStems = []string{
 	"i cannot ", "i can't ", "i can not ", "i could not ", "i couldn't ",
 	"i am unable to", "i'm unable to", "i was unable to", "i wasn't able to",
-	"i was not able to", "i do not have", "i don't have", "unable to ",
+	"i was not able to", "i do not have", "i don't have",
+	// "unable to " WITHOUT A SUBJECT WAS REMOVED. It is the only stem here that
+	// does not name who was unable, and it fired on a report's own section
+	// heading — "**Unable to verify (1):** - MCP #3 claim was truncated" — in a
+	// verification task that had completed and was categorising its findings.
+	// The first-person forms above still catch every genuine admission; a
+	// heading is not one.
+	"we are unable to", "we were unable to",
 	"without being able to",
 }
 
@@ -179,6 +186,19 @@ var inabilityStems = []string{
 var successNegationTails = []string{
 	"find any", "found any", "find a ", "see any", "detect any", "identify any",
 	"reproduce", "spot any", "locate any",
+	// A NEGATIVE SEARCH RESULT IS THE ANSWER, not a failure to produce one.
+	//
+	// The list above already encodes this — "I could not find any remaining
+	// issues" is success — but only for the "any" phrasings. A finder reporting
+	// "I could NOT find where AllowManifestToolAutoApproval is set to true in
+	// production code" was marked INCOMPLETE after 53 tool calls and a 19,145
+	// character audit, for doing precisely the job it was given: establishing
+	// that something is not there.
+	"find where", "find the", "find it", "find that", "find this",
+	"found where", "found the",
+	"locate where", "locate the", "locate it",
+	"determine where", "identify where", "see where",
+	"reproduce ", "confirm any", "observe any",
 }
 
 // narrativeMarkers flag a sentence as RETELLING a past exchange rather than
@@ -247,11 +267,71 @@ func admissionSentences(lower string) []string {
 // retells a past exchange (narrativeMarkers) is skipped entirely — an admission
 // must be the model's own report about the CURRENT objective, not general
 // language that merely resembles one.
+// toolGrantMarkers flag a sentence as reporting WHICH TOOLS this run was given,
+// not whether the work was done.
+//
+// A read-only plan task is SUPPOSED to say this. One wrote "I don't have an
+// update_plan tool available in this specialist context (only read-only
+// exploration tools were provided)" and then delivered the complete answer —
+// helper name, file, line 214, full source — and was marked INCOMPLETE on the
+// "i don't have" stem. The prompt asks tasks to name their limits plainly; a
+// detector that punishes exactly that teaches the opposite.
+//
+// NARROW ON PURPOSE: the sentence must name a TOOL or a GRANT. "I do not have
+// enough evidence" is still an admission and still fires.
+// NARROWED AFTER AN AUDIT OF THIS VERY FIX. The first version listed a bare
+// " tool", which exempts any inability sentence that merely mentions one —
+// measured at 5/5 on ordinary phrasings:
+//
+//	"I cannot run the build tool, so the change is unverified"
+//	"I could not use the migration tool and the data is untouched"
+//	"I was unable to invoke the formatting tool on the output"
+//
+// Those are genuine admissions, and silently exempting them is the WORSE
+// direction: a false positive costs a re-run, a false negative reports
+// unfinished work as done. The markers now have to be about what the run WAS
+// GIVEN, not about a tool being mentioned at all.
+var toolGrantMarkers = []string{
+	"tool available", "tools available", "no such tool", "not available in this",
+	"read-only tools", "read only tools", "only read-only", "only read only",
+	"tools were provided", "tools were given", "toolset provided",
+	"in this specialist context", "in this context only",
+	"is not in my toolset", "not in my toolset", "not in this toolset",
+}
+
+// objectiveFailureMarkers name the OBJECTIVE rather than a capability. A
+// sentence carrying one is about whether the job got done, so the tool-grant
+// exemption above does not apply to it however many tools it mentions.
+// VERB-ANCHORED, not bare nouns. "this task" alone was too crude: a task that
+// finished wrote "so i could not record a plan; the task is a single
+// read-and-report step and is now complete" — it names the task in order to
+// report SUCCESS, and a bare-noun override read that as failure. The marker has
+// to be the objective NOT BEING DONE, which needs the verb.
+var objectiveFailureMarkers = []string{
+	"complete this task", "complete the task", "completing this task", "completing the task",
+	"finish this task", "finish the task", "finishing this task",
+	"complete it", "completing it", "finish it", "finishing it",
+	"the objective", "the assignment", "as requested", "what was asked",
+	"do this task", "perform this task", "carry out this task",
+}
+
 func selfReportedIncompletion(text string) string {
 	for _, sentence := range admissionSentences(strings.ToLower(stripQuoted(text))) {
 		if containsAny(sentence, narrativeMarkers) {
 			continue
 		}
+		// A sentence about the tool grant is about CAPABILITY, not about the
+		// objective — UNLESS it also says the task itself could not be done.
+		//
+		// THE OVERRIDE IS NOT OPTIONAL. Without it the exemption swallowed a
+		// genuine failure: "I am unable to complete this task with the current
+		// tool set … Only write_file is enabled … so I cannot inspect the
+		// codebase." That task really did fail, and it mentions tools, so a bare
+		// tool-marker check waved it through. Naming the task is what separates
+		// "I lack a tool I did not need" from "I lack the tools this needed".
+		if containsAny(sentence, toolGrantMarkers) && !containsAny(sentence, objectiveFailureMarkers) {
+			continue
+		}
 		for _, phrase := range selfReportPhrases {
 			if strings.Contains(sentence, phrase) {
 				return selfReportReason(phrase)