diff --git a/.claude/skills/panel-review/SKILL.md b/.claude/skills/panel-review/SKILL.md index 8759dd831d..f012632556 100644 --- a/.claude/skills/panel-review/SKILL.md +++ b/.claude/skills/panel-review/SKILL.md @@ -81,6 +81,10 @@ Once pinned, capture once and reuse: - **Commit list:** `git log ..HEAD --oneline` (used to find issue/spec references). - **File list:** `git diff ...HEAD --name-only`. +- **Diff text:** run the diff command once and keep its output; this is + the `` in every Step 8 brief. Reviewers receive this; they + must not re-run `git diff`. For a diff over about 1000 lines, save it + once to `.scratch/panel-review/.diff` and pass that path instead. Print to the user: @@ -266,7 +270,7 @@ They run concurrently, separate contexts, no order dependencies. - `description`: "Spec adherence check for " - `prompt`: - > Diff to review: `git diff ...HEAD` + > Diff to review: > Commit list: > Spec source: > @@ -298,7 +302,7 @@ in final output. - `description`: "Standards conformance check for " - `prompt`: - > Diff to review: `git diff ...HEAD` + > Diff to review: > Standards source files (read these): > Tooling-enforced configs to SKIP (tooling already runs on > every commit; do not re-flag what they cover): @@ -317,7 +321,7 @@ in final output. - `description`: "Independent test-adequacy check for " - `prompt`: - > Diff to review: `git diff ...HEAD` + > Diff to review: > Spec source: > Acceptance plan / verify contract: > @@ -339,7 +343,7 @@ For each specialist in the panel: - `subagent_type`: the agent's `name` field. - `description`: " review of ". - `prompt`: - - The exact diff scope (`git diff ...HEAD -- `). + - The `` for this agent's paths. - A one-line statement of what the agent should focus on, derived from its own description. - The project-context items the agent's body needs. diff --git a/engine/agent/subagent.go b/engine/agent/subagent.go index 230c779c5f..92dfb6c2f7 100644 --- a/engine/agent/subagent.go +++ b/engine/agent/subagent.go @@ -608,7 +608,7 @@ var subagentSchema = json.RawMessage(`{ "mode": { "type": "string", "enum": ["read-only", "read-write"], - "description": "Workspace mode (default 'read-only'). 'read-write' gives the subagent the Edit and Write tools and lets it change files DIRECTLY in your workspace — exactly as you do — so its edits land immediately, with no copy or merge step. There is no isolation: a read-write subagent that crashes or goes wrong can leave partial edits in your working tree, the same as any interrupted edit; recover with git (git diff / git checkout / git stash) since your repository is the safety net. Use 'read-write' when you want a focused subagent to actually make and keep file changes (e.g. 'implement this fix and edit the files'); omit (or 'read-only') for an investigation that must not touch your files. Runs serially — never concurrently with your other tools — so it cannot race your own reads or writes. Cannot be combined with 'background' or with 'agent'+'model' together; 'read-write'+'agent' alone runs the named specialist writable (its scoped catalog + Edit/Write against your workspace); 'read-write'+'model' (no 'agent') runs the writable explorer on that model. Composes with 'fork', 'resume', 'output_schema'." + "description": "Workspace mode (default 'read-only'). 'read-write' gives the subagent the Edit and Write tools and lets it change files DIRECTLY in your workspace — exactly as you do — so its edits land immediately, with no copy or merge step. There is no isolation: a read-write subagent that crashes or goes wrong can leave partial edits in your working tree, the same as any interrupted edit; recover with git (git diff / git checkout / git stash) since your repository is the safety net. 'read-write' keeps the subagent's file changes; omit (or 'read-only') for an investigation that must not touch your files. Runs serially — never concurrently with your other tools — so it cannot race your own reads or writes. Cannot be combined with 'background' or with 'agent'+'model' together; 'read-write'+'agent' alone runs the named specialist writable (its scoped catalog + Edit/Write against your workspace); 'read-write'+'model' (no 'agent') runs the writable explorer on that model. Composes with 'fork', 'resume', 'output_schema'." } }, "required": ["prompt"] @@ -1425,25 +1425,25 @@ func (t *SubagentTool) Spec() tool.ToolSpec { shellClause = "By default the subagent is READ-ONLY — " + t.shellDisabledNote + " — with no Edit/Write" } desc := "Delegate ONE focused, self-contained task to a subagent with its own fresh context. " + + "Delegation has a real cost: the subagent starts empty, re-reads what you already have, and " + + "returns only its final message, so detail you did not ask for is lost. Do the work yourself " + + "when it is a handful of tool calls or depends on context you already hold. Delegate a " + + "read-only investigation whose intermediate output you do not need: a broad search ('search → " + + "summarize'), a code path traced across many files, or history work ('bisect the history'). " + "For two or more independent READ-ONLY tasks, prefer one Subagent call per task in the SAME " + "assistant turn; eligible calls execute concurrently and return separate results. Do not wait " + - "for one before launching the next unless a later task depends on an earlier result. Use it for " + - "a multi-step investigation ('search → summarize', 'trace this code path'), build/test/git " + - "work ('run the tests and report failures', 'bisect the history'), or an implementation task " + - "('implement this fix and edit the files'). " + + "for one before launching the next unless a later task depends on an earlier result. " + + "Run full test suites and repository-wide checks yourself; subagents run only focused checks. " + shellClause + ". " + - "Set mode:\"read-write\" to let it edit and write files DIRECTLY in your workspace — exactly " + - "as you do — so its edits land immediately, with no copy or merge step. This is how you " + - "delegate an implementation task and have the edits LAND. Serial execution applies only to a " + - "mode:\"read-write\" call (it never runs alongside your other tools), so it cannot race you; " + - "there is no isolation, so review its result with `git diff`/`git status` and undo with " + - "`git checkout`/`git stash` if needed. " + + "mode:\"read-write\" lets it edit and write files DIRECTLY in your workspace, with no copy or " + + "merge step. Serial execution applies only to a mode:\"read-write\" call (it never runs " + + "alongside your other tools), so it cannot race you; there is no isolation, so review its " + + "result with `git diff`/`git status` and undo with `git checkout`/`git stash` if needed. " + "The subagent cannot delegate further. " + "The subagent's FINAL MESSAGE is its deliverable — you receive only that " + "(see `prompt`; with `background: true` the call instead returns at once and you collect the " + "result later). Do NOT use it when you need the intermediate outputs in this conversation " + - "(do the work yourself) or when workers must coordinate (use Team) — and don't delegate a " + - "single quick read you can do with Read/Grep." + + "or when workers must coordinate (use Team)." + " Inline context you already hold (e.g. a diff, file contents, prior findings) directly in " + "`prompt` rather than making the subagent re-fetch it — that saves its limited turn/tool " + "budget for the actual task." + @@ -1453,7 +1453,8 @@ func (t *SubagentTool) Spec() tool.ToolSpec { "conversation survives)." + " Set `fork: true` to seed the subagent with a COPY of THIS conversation (your full context " + "so far) instead of a fresh, empty one — for a focused continuation that needs everything " + - "you have already gathered; it runs on your own model (no model/agent/resume)." + "you have already gathered; it runs on your own model (no model/agent/resume)." + + " When in doubt, don't spawn: do the work yourself." desc += t.agentEnumeration() return tool.ToolSpec{ Name: subagentToolName, diff --git a/engine/prompt/builder.go b/engine/prompt/builder.go index 704181c377..0d6ef4a289 100644 --- a/engine/prompt/builder.go +++ b/engine/prompt/builder.go @@ -288,7 +288,7 @@ func toolDisciplineHints(tools []tool.ToolSpec) string { writeSentence(&b, "Reserve Shell for real system/terminal commands.") } if present["Subagent"] { - writeSentence(&b, "Use Subagent for focused delegation. For multiple independent read-only tasks, issue one Subagent call per task in the same assistant turn so eligible calls run concurrently; wait between calls only when a later task depends on an earlier result.") + writeSentence(&b, "Use Subagent for focused delegation. A subagent starts from a fresh context, re-reads what you already have, and returns only its final message, so do a handful of tool calls, context-dependent work, and full verification yourself; when in doubt, don't spawn. For multiple independent read-only tasks, issue one Subagent call per task in the same assistant turn so eligible calls run concurrently; wait between calls only when a later task depends on an earlier result.") } if present["Parallel"] { writeSentence(&b, "Use Parallel only for isolated writable or competing branches that need built-in join or winner selection.") diff --git a/internal/app/build.go b/internal/app/build.go index 39c968fcce..5e5337c350 100644 --- a/internal/app/build.go +++ b/internal/app/build.go @@ -7150,6 +7150,9 @@ func childEngineDepsForProvider(cfg Config, role string, provider port.LLMProvid ) deps.Catalog = cat deps.PromptConfig = applyContextualGuardrailWorkerPosture(pc, guardrailsConfigured(cfg) && len(cat.Tools()) > 0) + if roleFamily(role) == "subagent" { + deps.PromptConfig = applySubagentVerificationScope(deps.PromptConfig) + } if cfg.harnessInstructions != nil { switch role { case "guardrail-reviewer", "ask-reviewer", "model-router", "usermodel-review", "parallel-judge": @@ -8788,6 +8791,27 @@ func applyContextualGuardrailWorkerPosture(pc prompt.Config, enabled bool) promp return pc } +// subagentVerificationScopeNote is the Role suffix of every Subagent child +// ("task" role family). Children inherit the full project instructions, whose +// repository-wide gates (full suites, lint, docs) are written for the session +// that finishes the change; without this note each child re-runs them, which +// multiplies the slowest step of a turn by the fan-out width. +const subagentVerificationScopeNote = "You are a subagent: the session that delegated this task owns " + + "repository-wide verification. Run only focused tests or checks that exercise your task. Do not run " + + "full test suites, lint, docs, or other repository-wide gates unless the delegating brief explicitly " + + "asks for it, even if project instructions require them; report what you ran and what still needs " + + "verifying instead." + +// applySubagentVerificationScope appends subagentVerificationScopeNote to the +// child's Role, seeding the default role first like the other posture notes. +func applySubagentVerificationScope(pc prompt.Config) prompt.Config { + if pc.Role == "" { + pc.Role = prompt.DefaultRole() + } + pc.Role += "\n\n" + subagentVerificationScopeNote + return pc +} + // noFSPostureNote is the MAIN-engine system-prompt suffix of a "no-fs" profile // session (issue #55, the #40 composition-append pattern): the model must learn // there is no filesystem from the prompt, not from a trail of unknown-tool diff --git a/internal/app/delegation_guidance_test.go b/internal/app/delegation_guidance_test.go index 1f457fdef6..16497a3084 100644 --- a/internal/app/delegation_guidance_test.go +++ b/internal/app/delegation_guidance_test.go @@ -2,12 +2,13 @@ package app import ( "context" + "encoding/json" "strings" + "sync" "testing" "github.com/stacklok/mecatl/engine/adapter/mockllm" "github.com/stacklok/mecatl/engine/port" - "github.com/stacklok/mecatl/engine/prompt" "github.com/stacklok/mecatl/engine/session" ) @@ -28,7 +29,7 @@ func TestDelegationGuidanceLandsInBuiltEnginePrompt(t *testing.T) { {"both", true, true}, } { t.Run(tc.name, func(t *testing.T) { - prefix := captureBuiltDelegationPrompt(t, tc.parallel, tc.teams).StablePrefix + prefix := captureBuiltPrefixes(t, tc.parallel, tc.teams, mockllm.TextTurn("done"))[0] if !strings.Contains(prefix, subagentGuidance) { t.Errorf("StablePrefix omits Subagent guidance") } @@ -47,16 +48,24 @@ func TestDelegationGuidanceLandsInBuiltEnginePrompt(t *testing.T) { } } -func captureBuiltDelegationPrompt(t *testing.T, parallel, teams bool) prompt.Layered { +// captureBuiltPrefixes runs one prompt through a built session driven by the +// scripted turns and returns the StablePrefix of every provider request, in order. +func captureBuiltPrefixes(t *testing.T, parallel, teams bool, turns ...mockllm.Turn) []string { t.Helper() - var captured prompt.Layered + var ( + mu sync.Mutex + prefixes []string + ) provider := mockllm.NewWith([]mockllm.Option{ - mockllm.WithRequestObserver(func(req port.LLMRequest) { captured = req.System }), - }, mockllm.TextTurn("done")) - workspace := t.TempDir() + mockllm.WithRequestObserver(func(req port.LLMRequest) { + mu.Lock() + prefixes = append(prefixes, req.System.StablePrefix) + mu.Unlock() + }), + }, turns...) built, err := buildIsolated(t, context.Background(), Config{ - Workspace: workspace, + Workspace: t.TempDir(), Model: "mock", MockProvider: provider, EnableParallel: parallel, @@ -78,8 +87,30 @@ func captureBuiltDelegationPrompt(t *testing.T, parallel, teams bool) prompt.Lay } for range run.Events() { } - if captured.StablePrefix == "" { + mu.Lock() + defer mu.Unlock() + if len(prefixes) == 0 || prefixes[0] == "" { t.Fatal("provider did not receive a built system prompt") } - return captured + return prefixes +} + +// TestSubagentChildPromptScopesVerification drives a real Subagent call through +// the built session: the child's stable system prefix carries the verification +// scope note, while the parent's does not. +func TestSubagentChildPromptScopesVerification(t *testing.T) { + prefixes := captureBuiltPrefixes(t, false, false, + mockllm.ToolCallTurn(session.ToolCall{ID: "s1", Name: "Subagent", Args: json.RawMessage(`{"prompt":"check one thing","description":"probe"}`)}), + mockllm.TextTurn("child done"), + mockllm.TextTurn("parent done"), + ) + if len(prefixes) != 3 { + t.Fatalf("provider saw %d requests, want 3 (parent, child, parent)", len(prefixes)) + } + if strings.Contains(prefixes[0], subagentVerificationScopeNote) { + t.Error("parent system prompt must not carry the subagent verification scope note") + } + if !strings.Contains(prefixes[1], subagentVerificationScopeNote) { + t.Errorf("Subagent child system prompt must carry the verification scope note, got:\n%s", prefixes[1]) + } }