From fc1ed49d721a7cf6ae426251ce2728ca693c0f9f Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Sat, 2 May 2026 03:29:56 +0200 Subject: [PATCH] test+docs(sf): parent-trace test + dispatching-subagents skill doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows up the parent-trace dispatch wiring (bundled into bc9cf4fef + 2508822b8). Adds: - src/resources/extensions/subagent/tests/parent-trace.test.ts — 7 cases covering the composeTaskWithParentTrace helper: undefined/empty/whitespace pass-through, tag wrapping, task-after-trace ordering, content trimming, embedded verifier instructions ("hedge words", "tool errors"). - src/resources/extensions/subagent/index.ts — exports composeTaskWithParentTrace so the test can import it. - skills/dispatching-subagents — new "Parent trace (for verifier/review subagents)" subsection documents the field at TaskItem / ChainItem / batch level, the per-task override, and the chain (step 0 only) and debate (round 1 only) behaviour. PDD packet (inline; small follow-up to the architectural change): - Purpose: parent-trace plumbing has a falsifiable test and is documented in the canonical dispatching-subagents skill so callers know how to use it. - Consumer: the dispatching-subagents skill (loaded by every agent that calls the subagent tool); the test (covers regression). - Contract: 7 test cases pass; SKILL.md contains the documented field at three schema levels with the override and per-mode behaviour notes. - Evidence: - tests/parent-trace.test.ts → 7/7 pass via the SF resolve-ts loader - npm run typecheck:extensions exits 0 - All 35 subagent suite tests pass - Non-goals: not changing the dispatch wiring (already in); not adding parent-trace handling to background jobs (separate slice if needed). - Invariants (safety only — sync helper + pure prose docs): - composeTaskWithParentTrace returns task unchanged when trace is empty. - The original task always appears after the closing tag. - Trimmed content is what gets injected, not the raw padded input. - Assumptions: tests load TS via the resolve-ts.mjs hook (standard SF pattern); skills load SKILL.md from dist via copy-resources. --- .../sf/skills/dispatching-subagents/SKILL.md | 28 +++++++ src/resources/extensions/subagent/index.ts | 2 +- .../subagent/tests/parent-trace.test.ts | 78 +++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 src/resources/extensions/subagent/tests/parent-trace.test.ts diff --git a/src/resources/extensions/sf/skills/dispatching-subagents/SKILL.md b/src/resources/extensions/sf/skills/dispatching-subagents/SKILL.md index 4f75f6ee0..78fdb777d 100644 --- a/src/resources/extensions/sf/skills/dispatching-subagents/SKILL.md +++ b/src/resources/extensions/sf/skills/dispatching-subagents/SKILL.md @@ -121,6 +121,34 @@ Use debate mode for high-stakes decisions, plan review, architecture review, and migrations where the cost of a weak plan is high. Keep `rounds` between 2 and 3 by default; max is 5. +### Parent trace (for verifier/review subagents) + +Set `parentTrace` when dispatching a verifier or reviewer that should audit +what the parent **actually did**, not just what the parent's prose claims. The +dispatch tool wraps the value in a `...` block +prepended to the task, with embedded verifier instructions to look for hedge +words, glossed-over tool errors, and claims without Command/Output traces. The +parent assembles its own trace (recent tool-call summary); the dispatch tool +only plumbs it through. + +``` +subagent({ + parentTrace: "", + tasks: [ + { agent: "reviewer", task: "Audit the migration for correctness against the spec." }, + { agent: "tester", task: "Audit test coverage for the new code paths." } + ] +}) +``` + +Per-task `parentTrace` overrides the batch-level value — set it on a single +`TaskItem` (or `ChainItem`) when one subagent needs a different trace. + +For chain mode, only step 0 receives the trace; later steps see `{previous}`. +For debate mode, only round 1 receives the trace; later rounds have the debate +transcript. For parallel and single mode, the trace is always injected when +set. + ### Agent selection and model overrides sf routes subagents through agent definitions in `src/resources/agents/`, diff --git a/src/resources/extensions/subagent/index.ts b/src/resources/extensions/subagent/index.ts index a7770a1c9..65075f7fa 100644 --- a/src/resources/extensions/subagent/index.ts +++ b/src/resources/extensions/subagent/index.ts @@ -1034,7 +1034,7 @@ async function mapWithConcurrencyLimit( * words, glossed-over tool errors, untraced self-reports) so review subagent * prompts do not need to repeat them. */ -function composeTaskWithParentTrace( +export function composeTaskWithParentTrace( task: string, parentTrace: string | undefined, ): string { diff --git a/src/resources/extensions/subagent/tests/parent-trace.test.ts b/src/resources/extensions/subagent/tests/parent-trace.test.ts new file mode 100644 index 000000000..03037eb72 --- /dev/null +++ b/src/resources/extensions/subagent/tests/parent-trace.test.ts @@ -0,0 +1,78 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { composeTaskWithParentTrace } from "../index.js"; + +describe("composeTaskWithParentTrace", () => { + it("returns task unchanged when parentTrace is undefined", () => { + const task = "do the thing"; + assert.equal(composeTaskWithParentTrace(task, undefined), task); + }); + + it("returns task unchanged when parentTrace is an empty string", () => { + const task = "do the thing"; + assert.equal(composeTaskWithParentTrace(task, ""), task); + }); + + it("returns task unchanged when parentTrace is whitespace-only", () => { + const task = "do the thing"; + assert.equal(composeTaskWithParentTrace(task, " \n\t "), task); + }); + + it("wraps trace content in tags when content is provided", () => { + const task = "verify the parent's claim"; + const trace = "ran `npm test`\nexit 0"; + const result = composeTaskWithParentTrace(task, trace); + assert.ok( + result.includes(""), + "should include opening tag", + ); + assert.ok( + result.includes(""), + "should include closing tag", + ); + assert.ok(result.includes(trace), "should include trimmed trace content"); + }); + + it("places the original task after the closing tag", () => { + const task = "verify the parent's claim"; + const trace = "ran `npm test`\nexit 0"; + const result = composeTaskWithParentTrace(task, trace); + const closeIdx = result.indexOf(""); + const taskIdx = result.indexOf(task); + assert.ok(closeIdx >= 0, "closing tag must be present"); + assert.ok(taskIdx >= 0, "task must be present"); + assert.ok( + taskIdx > closeIdx, + "task must appear after the closing tag", + ); + }); + + it("trims whitespace from the parentTrace content before injection", () => { + const task = "audit"; + const trace = "the actual trace line"; + const padded = `\n\n ${trace} \n\n`; + const result = composeTaskWithParentTrace(task, padded); + assert.ok( + result.includes(trace), + "trimmed trace content should appear in result", + ); + assert.ok( + !result.includes(padded), + "untrimmed padded trace should not appear verbatim", + ); + }); + + it("includes verifier instruction phrases for hedge words and tool errors", () => { + const task = "audit"; + const trace = "some trace"; + const result = composeTaskWithParentTrace(task, trace); + assert.ok( + result.includes("hedge words"), + "should mention hedge words in verifier instructions", + ); + assert.ok( + result.includes("tool errors"), + "should mention tool errors in verifier instructions", + ); + }); +});