fix(#3302): return per-agent worktree metadata from workflow scripts (#3450)

* fix(#3302): return per-agent worktree metadata from workflow scripts

* fix(#3302): link changeset fragment to pr 3450

---------

Co-authored-by: sim <sim@local>

.changeset/sunny-badgers-howl.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3450
+---
+Workflow-backend waves (claude-orchestration, BETA) no longer strand executor commits on worktree-wf_* branches: the emitted Workflow script now returns each agent's worktree metadata, and the orchestrator records it into the wave manifest so the existing merge-and-cleanup step lands every plan's commits. Missing metadata now halts the wave loudly instead of reporting success with an empty worklist.

capabilities/claude-orchestration/fragments/execute-wave-pre.md

@@ -168,10 +168,79 @@ worktree isolation applied PER PLAN from the manifest's `use_worktree` field
   Omitting it from the tool invocation silently regresses phase-resume to a
   no-op: an interrupted phase re-runs completed plans.
 
-The orchestrator still runs steps 4–5.8 (wait for completion, worktree cleanup,
-post-merge gate, tracking update) exactly as it does for inline dispatch — the
-Workflow backend only replaces HOW agents are spawned for this wave, not what
-happens after they return.
+### After the run: manifest bridge into the merge chain (#3302)
+
+The single Workflow tool call replaces step 3's per-plan `Agent()` loop — which also
+means step 3's manifest bookkeeping (creation + per-agent recording) does NOT happen on
+this path. The orchestrator MUST bridge the run's per-agent results into the SAME
+manifest-scoped merge chain inline dispatch uses, before steps 4–5.8, which then run
+unchanged:
+
+1. **Create the manifest BEFORE invoking the tool** (this is step 3's creation block,
+   which this path skips). When ANY plan in the wave has `use_worktree` not `false`:
+
+   ```bash
+   if [ -z "${WAVE_WORKTREE_MANIFEST:-}" ]; then
+     M=$(mktemp "${TMPDIR:-/tmp}/gsd-worktree-wave-XXXXXX") && mv "$M" "$M.json" && WAVE_WORKTREE_MANIFEST="$M.json" || exit 1  # XXXXXX must be path-final on BSD/macOS (#1520)
+     # Persist the dispatch-time orchestrator worktree root so wave-cleanup pins back
+     # to the orchestrator's OWN worktree (#630), exactly as inline dispatch does.
+     ORCH_ROOT=$(git rev-parse --show-toplevel)
+     ORCH_ROOT="$ORCH_ROOT" MANIFEST="$WAVE_WORKTREE_MANIFEST" node -e 'const fs=require("fs");fs.writeFileSync(process.env.MANIFEST,JSON.stringify({orchestrator_root:process.env.ORCH_ROOT||null,worktrees:[]})+"\n")'
+     export WAVE_WORKTREE_MANIFEST
+   fi
+   ```
+
+2. **Invoke the Workflow tool with the emitted script and
+   `resumeFromRunId: summary.resumeRunId`.** The script top-level `return`s one entry
+   per dispatched plan: `{ plan, expects_worktree, metadata }`. `metadata` is that
+   plan's executor `<worktree_metadata>` JSON (`{agent_id, worktree_path, branch,
+   expected_base}` — captured by the executor itself per
+   `agents/gsd-executor.md`), or `null` when the agent's result carried none
+   (interrupted agent, resumed-from-cache plan, or a non-worktree plan).
+
+3. **Record every worktree plan** exactly as inline dispatch does at step 3's
+   "After each `Agent()` returns" — one `worktree.record-agent` per returned entry
+   with `expects_worktree: true` and complete metadata:
+
+   ```bash
+   gsd_run query worktree.record-agent --manifest "$WAVE_WORKTREE_MANIFEST" \
+     --agent-id "<metadata.agent_id>" --path "<metadata.worktree_path>" \
+     --branch "<metadata.branch>" --base "<metadata.expected_base>" \
+     --files "<plan files_modified, space-separated>"
+   ```
+
+   The verb's write-strict validation applies as inline: on a non-zero exit or any
+   missing field, stop and ask for recovery — do not append an under-populated entry.
+
+4. **HALT on uncapturable metadata — never a silently-empty manifest (#3302).**
+   After recording, the manifest must hold one entry per `expects_worktree: true`
+   outcome (`summary.worktreePlans` from `resolve-wave-dispatch` is the expected
+   count). Any shortfall — a `null` `metadata`, a missing/empty field, or a count
+   mismatch — means commits are stranded on their `worktree-wf_*` branches and
+   `worktree.cleanup-wave` would merge nothing while the phase looks green. STOP the
+   phase with the failing plan id and the recovery hint below; do NOT run
+   `worktree.cleanup-wave` and do NOT proceed to step 4.
+
+   **Recovery hint:** the unmerged `worktree-wf_*` branch still holds the work. Recover
+   the missing metadata from the run's per-agent result journal (`journal.jsonl` — one
+   `{"type":"result",…}` line per agent — in the Workflow run's transcript dir), re-run
+   `worktree.record-agent` by hand, then re-run cleanup. If the journal cannot be
+   recovered either, merge the branch manually after review — never discard it.
+
+5. **Resume (`resumeFromRunId`).** Cached/resumed agents do not re-emit their final
+   messages, so a previously-completed plan can return with `metadata: null`. Recover
+   that plan's metadata from the ORIGINAL run's journal (same hint as above). If it
+   cannot be recovered, fail loudly per rule 4 — a resumed run must never report
+   success over silently-dropped agent work.
+
+6. **Non-worktree plans** (`expects_worktree: false` — `use_worktree: false` in the
+   manifest): they ran without isolation; their commits are already on the main working
+   tree. No record-agent entry, no manifest write.
+
+With the manifest populated, steps 4–5.8 (wait/completion bookkeeping, step 5.5's
+manifest-scoped `worktree.cleanup-wave`, post-merge gate, tracking update) run
+UNCHANGED — the Workflow backend replaces HOW agents are spawned and returns their
+metadata; the merge chain itself is the inline path's own, now with real input.
 
 **If `backend == "inline"`** (any gate miss, or `resolve-wave-dispatch` itself
 unavailable/erroring): proceed to step 3's standard per-message `Agent()`

src/claude-orchestration.cts

@@ -299,6 +299,13 @@ interface EmitOk {
   summary: {
     waves: number;
     plans: number;
+    /**
+     * #3302 — the number of record-agent entries the orchestrator must end up
+     * with in WAVE_WORKTREE_MANIFEST after the run (plans with `use_worktree`
+     * not `false`). The loud count check: fewer entries than this after the
+     * Workflow run means metadata capture failed and the wave must HALT.
+     */
+    worktreePlans: number;
     stagesByWave: string[][][]; // wave → stage → planId[]
     resumeRunId: string;
     budgetTokens: number | null;
@@ -564,14 +571,44 @@ function emitWorkflowScript(input: EmitInput | null | undefined): EmitOk | EmitE
   }
   lines.push('');
 
+  // #3302: the generated script must hand the per-agent executor results back
+  // to the orchestrator so it can feed the wave merge chain. Emitted right
+  // after the header comments (meta stays the first statement): a helper that
+  // extracts the executor's <worktree_metadata> JSON (agents/gsd-executor.md
+  // <worktree_metadata_capture>) from one agent() result, plus the outcomes
+  // accumulator the stage barriers below push into. `metadata` is null when
+  // the result carried no parseable block — the LOUD-failure input the
+  // orchestrator halts on for expects_worktree plans (never a silent skip).
+  lines.push('// #3302: extract the executor-returned <worktree_metadata> JSON so the');
+  lines.push('// orchestrator can record it into WAVE_WORKTREE_MANIFEST after the run');
+  lines.push('// (worktree.record-agent -> worktree.cleanup-wave, the same manifest-scoped');
+  lines.push('// merge chain inline dispatch feeds). null = absent/unparseable/interrupted.');
+  lines.push('function gsdWorktreeMetadata(agentResult) {');
+  lines.push('  if (typeof agentResult !== \'string\') return null;');
+  lines.push('  const m = agentResult.match(/<worktree_metadata>([\\s\\S]*?)<\\/worktree_metadata>/);');
+  lines.push('  if (m === null) return null;');
+  lines.push('  try {');
+  lines.push('    const parsed = JSON.parse(m[1]);');
+  lines.push('    return (parsed !== null && typeof parsed === \'object\') ? parsed : null;');
+  lines.push('  } catch (e) {');
+  lines.push('    return null;');
+  lines.push('  }');
+  lines.push('}');
+  lines.push('const gsdAgentOutcomes = [];');
+  lines.push('');
+
   const stagesByWave: string[][][] = [];
   let totalPlans = 0;
+  let worktreePlans = 0;
 
   for (let wi = 0; wi < waves.length; wi++) {
     const wave = waves[wi];
     const stages = partitionStages(wave.plans);
     stagesByWave.push(stages);
     totalPlans += wave.plans.length;
+    for (const p of wave.plans) {
+      if (p.use_worktree !== false) worktreePlans += 1;
+    }
 
     lines.push('// Wave ' + wave.id);
     // Title must match this wave's meta.phases entry EXACTLY.
@@ -587,17 +624,37 @@ function emitWorkflowScript(input: EmitInput | null | undefined): EmitOk | EmitE
       // threw "parallel() expects an array of functions" (#2590). Passing
       // agent() results directly would also start every agent eagerly, before
       // parallel() could bound concurrency.
-      lines.push('await parallel([');
+      // #3302: capture the barrier's resolved results (one per thunk, in thunk
+      // order — the documented parallel() contract) so each plan's outcome can
+      // be tagged and returned below. Discarding them stranded every
+      // worktree-wf_* branch: the merge chain had no input (#3302).
+      lines.push('const gsdStage_' + wi + '_' + si + ' = await parallel([');
       for (const p of stagePlans) {
         lines.push('  () => agent(' + quoteString(p.brief) + ', ' + agentOptions(p, executorModel) + '),');
       }
       lines.push('])');
+      // Positional tagging is decided at EMIT time from the validated manifest,
+      // so attribution survives out-of-order completion and needs no runtime
+      // introspection. expects_worktree mirrors agentOptions' own per-plan
+      // decision (use_worktree !== false).
+      lines.push('gsdAgentOutcomes.push(');
+      for (let pi = 0; pi < stagePlans.length; pi++) {
+        const p = stagePlans[pi];
+        const tail = pi < stagePlans.length - 1 ? ',' : '';
+        lines.push('  { plan: ' + quoteString(p.id) + ', expects_worktree: ' + (p.use_worktree !== false) + ', metadata: gsdWorktreeMetadata(gsdStage_' + wi + '_' + si + '[' + pi + ']) }' + tail);
+      }
+      lines.push(')');
     }
     if (wi < waves.length - 1) lines.push('');
   }
 
-  lines.push('// Each agent writes SUMMARY.md on its worktree branch; commits land there');
-  lines.push('// and are merged by the orchestrator exactly as in inline wave dispatch.');
+  // #3302: the script's top-level return value is what the Workflow tool hands
+  // back to the orchestrator. One { plan, expects_worktree, metadata } entry
+  // per dispatched plan; the orchestrator records every worktree entry via
+  // `gsd_run query worktree.record-agent` and HALTS on a null metadata entry
+  // for an expects_worktree plan (see the execute:wave:pre fragment) — a
+  // silently-empty manifest is the exact #3302 failure mode.
+  lines.push('return gsdAgentOutcomes');
 
   const script = lines.join('\n');
 
@@ -607,6 +664,9 @@ function emitWorkflowScript(input: EmitInput | null | undefined): EmitOk | EmitE
     summary: {
       waves: waves.length,
       plans: totalPlans,
+      // #3302: the number of record-agent entries the orchestrator must end up
+      // with in WAVE_WORKTREE_MANIFEST after the run — the loud count check.
+      worktreePlans,
       stagesByWave,
       resumeRunId: runId,
       budgetTokens,

tests/claude-orchestration.test.cjs

@@ -1693,9 +1693,14 @@ function firstStatement(script) {
 }
 
 describe('#2590: emitted Workflow scripts satisfy the Workflow tool contract', () => {
-  test('the emitted script is syntactically valid as an ES module', (t) => {
-    // `export const meta` + top-level `await` only parse in module context —
-    // which is exactly the context the Workflow tool runs the script in.
+  test('the emitted script parses in the Workflow tool\'s evaluation context', (t) => {
+    // #3302: the script now carries a top-level `return` (the documented way a
+    // Workflow script hands its result to the invoking model — see
+    // code.claude.com/docs/en/workflows), which plain ESM parsing rejects.
+    // The tool lifts the `export const meta` block out and evaluates the rest
+    // as an async function body — top-level `await` AND `return` are both
+    // valid there. Emulate that context: strip the export keyword and wrap the
+    // body in an async arrow, still parsed as ESM so everything else surfaces.
     const { script } = emit({
       waves: [
         { id: 'w1', plans: [
@@ -1710,9 +1715,14 @@ describe('#2590: emitted Workflow scripts satisfy the Workflow tool contract', (
     const dir = createTempDir('gsd-2590-parse-');
     t.after(() => cleanup(dir));
     const f = path.join(dir, 'emitted.mjs');
-    fs.writeFileSync(f, script);
+    fs.writeFileSync(
+      f,
+      'const gsdWorkflowScript = async () => {\n'
+        + script.replace(/^export /m, '')
+        + '\n};\nexport const __parsed = gsdWorkflowScript;\n',
+    );
     const result = runNode(['--check', f], { timeoutMs: PROBE_TIMEOUT_MS });
-    throwIfFailed(result, `node --check ${f} (emitted script must parse)`);
+    throwIfFailed(result, `node --check ${f} (emitted script must parse in the tool's async-body context)`);
   });
 
   test('1. `export const meta` is the first statement', () => {
@@ -1898,3 +1908,211 @@ describe('#2590: the backend is reachable without hand-passed flags', () => {
     assert.equal(JSON.parse(result.stdout).backend, 'workflow');
   });
 });
+
+// ─── #3302 — Workflow-backend manifest bridge (emit returns per-agent outcomes) ──
+//
+// The Workflow backend wrapped a whole wave in ONE tool call and discarded the
+// per-agent results, so nothing ever fed WAVE_WORKTREE_MANIFEST and executor
+// commits stayed stranded on worktree-wf_* branches while the phase looked
+// green. The fix has two halves, both pinned here:
+//
+//   code        — emitWorkflowScript captures each parallel() barrier's results
+//                 and top-level `return`s one { plan, expects_worktree, metadata }
+//                 outcome per dispatched plan (metadata extracted in-script from
+//                 the executor's <worktree_metadata> block, or null);
+//   instructions — the execute:wave:pre fragment bridges those outcomes into the
+//                 SAME record-agent -> cleanup-wave merge chain inline dispatch
+//                 uses, halting loudly when metadata cannot be captured.
+//
+// The execution tests below run the EMITTED script the way the Workflow tool
+// does (meta lifted out, body evaluated as an async function with phase()/
+// parallel()/agent() supplied), asserting runtime behavior, not script text.
+
+const ASYNC_FUNCTION_3302 = Object.getPrototypeOf(async function () {}).constructor;
+
+/**
+ * Execute an emitted Workflow script in a stub runtime mirroring the documented
+ * tool contract (code.claude.com/docs/en/workflows): phase() groups progress,
+ * parallel() takes an array of thunks and resolves to their results in thunk
+ * order, agent() resolves to the agent's final message (or null when
+ * interrupted), and the script's top-level `return` value is what the invoking
+ * model receives.
+ */
+async function executeEmittedScript3302(script, agentStub) {
+  const body = script.replace(/^export /m, '');
+  const harness = [
+    'const phase = () => {};',
+    'const parallel = async (thunks) => Promise.all(thunks.map((t) => Promise.resolve().then(t)));',
+    'const agent = async (brief, opts) => agentStub(brief, opts);',
+    body,
+  ].join('\n');
+  const fn = new ASYNC_FUNCTION_3302('agentStub', harness);
+  return await fn(agentStub);
+}
+
+/** An executor final message carrying a valid <worktree_metadata> block. */
+function agentResultWithMetadata3302(fields) {
+  return [
+    '## PLAN COMPLETE',
+    '',
+    '<worktree_metadata>',
+    JSON.stringify(fields),
+    '</worktree_metadata>',
+    '',
+    '**Commits:**',
+    '- abc1234: fix(#000): example',
+  ].join('\n');
+}
+
+describe('#3302: emitted Workflow script returns per-agent outcomes for the manifest bridge', () => {
+  test('A1. the script returns one outcome entry per dispatched plan (today: undefined)', async () => {
+    const r = emitWorkflowScript(nonOverlappingManifest());
+    assert.strictEqual(r.ok, true);
+    const ret = await executeEmittedScript3302(r.script, () => agentResultWithMetadata3302({}));
+    assert.ok(Array.isArray(ret), 'the top-level return value must be the outcomes array');
+    assert.strictEqual(ret.length, 2, 'one entry per dispatched plan');
+  });
+
+  test('A2. entries are { plan, expects_worktree, metadata } with correct attribution', async () => {
+    const r = emitWorkflowScript(nonOverlappingManifest());
+    // Distinct branch per brief proves the positional mapping through the
+    // parallel() barrier attributes each agent's OWN metadata to its plan.
+    const stub = (brief) => agentResultWithMetadata3302({
+      agent_id: 'id-' + brief,
+      worktree_path: '/wt/' + brief,
+      branch: 'worktree-wf-run-' + brief,
+      expected_base: 'deadbee',
+    });
+    const ret = await executeEmittedScript3302(r.script, stub);
+    assert.deepEqual(
+      ret.map((e) => [e.plan, e.metadata && e.metadata.branch]),
+      [['p1', 'worktree-wf-run-Plan A'], ['p2', 'worktree-wf-run-Plan B']],
+      'plan ids in dispatch order, each with its own agent\'s metadata',
+    );
+    for (const e of ret) {
+      assert.strictEqual(e.expects_worktree, true, 'worktree plans expect metadata');
+      assert.deepEqual(
+        Object.keys(e.metadata).sort(),
+        ['agent_id', 'branch', 'expected_base', 'worktree_path'],
+        'metadata is the parsed <worktree_metadata> JSON object',
+      );
+    }
+  });
+
+  test('A3. expects_worktree mirrors use_worktree per plan (mixed wave)', async () => {
+    const r = emitWorkflowScript({
+      phaseDir: '.planning/phases/01-foo',
+      runId: 'run-abc-1143',
+      waves: [{
+        id: 'w1',
+        plans: [
+          { id: 'p1', brief: 'In worktree', files_modified: ['src/a.cts'] },
+          { id: 'p2', brief: 'No worktree', files_modified: ['src/b.cts'], use_worktree: false },
+        ],
+      }],
+    });
+    const ret = await executeEmittedScript3302(r.script, (brief) => (
+      brief === 'No worktree'
+        ? '## PLAN COMPLETE (no metadata — ran on the main tree)'
+        : agentResultWithMetadata3302({ agent_id: 'p1', worktree_path: '/wt/p1', branch: 'worktree-wf-run-1', expected_base: 'aa' })
+    ));
+    assert.strictEqual(ret[0].expects_worktree, true);
+    assert.strictEqual(ret[0].metadata.branch, 'worktree-wf-run-1');
+    assert.strictEqual(ret[1].expects_worktree, false, 'use_worktree:false -> expects_worktree:false');
+    assert.strictEqual(ret[1].metadata, null, 'non-worktree plans carry no metadata — not an error');
+    assert.strictEqual(r.summary.worktreePlans, 1, 'summary counts only worktree plans');
+  });
+
+  test('A4. null metadata is the loud-failure input: interrupted agent, missing block, bad JSON', async () => {
+    const cases = [
+      ['interrupted agent (agent() resolves to null)', null],
+      ['result without a <worktree_metadata> block', '## PLAN COMPLETE\n\n**Commits:** - x'],
+      ['unparseable JSON inside the block', '<worktree_metadata>{not json</worktree_metadata>'],
+      ['non-object JSON inside the block', '<worktree_metadata>"just a string"</worktree_metadata>'],
+    ];
+    for (const [label, rawResult] of cases) {
+      const r = emitWorkflowScript(singleWaveManifest());
+      const ret = await executeEmittedScript3302(r.script, () => rawResult);
+      assert.strictEqual(ret.length, 1, label);
+      assert.strictEqual(ret[0].expects_worktree, true, label);
+      assert.strictEqual(ret[0].metadata, null, `${label} -> metadata null (orchestrator must HALT, #3302)`);
+    }
+  });
+
+  test('A5. multi-wave and multi-stage manifests: every plan appears exactly once, in dispatch order', async () => {
+    const r = emitWorkflowScript({
+      phaseDir: '.planning/phases/01-foo',
+      runId: 'run-3302',
+      waves: [
+        { id: 'w1', plans: [
+          { id: 'a1', brief: 'A1', files_modified: ['src/shared.cts'] },
+          { id: 'a2', brief: 'A2', files_modified: ['src/shared.cts', 'src/b.cts'] }, // overlap -> stage split
+          { id: 'a3', brief: 'A3', files_modified: ['src/c.cts'] },                    // coalesces with a1
+        ] },
+        { id: 'w2', plans: [{ id: 'b1', brief: 'B1', files_modified: ['src/d.cts'] }] },
+      ],
+    });
+    const ret = await executeEmittedScript3302(r.script, (brief) => agentResultWithMetadata3302({ agent_id: brief, worktree_path: '/wt/' + brief, branch: 'br-' + brief, expected_base: 'ff' }));
+    // w1 stages: [a1, a3] then [a2] (greedy first-fit); then w2: [b1].
+    assert.deepEqual(ret.map((e) => e.plan), ['a1', 'a3', 'a2', 'b1']);
+    assert.strictEqual(ret.length, r.summary.plans);
+    assert.strictEqual(r.summary.worktreePlans, 4);
+  });
+
+  test('B1. the emitted script no longer claims an unbacked inline merge', () => {
+    const r = emitWorkflowScript(nonOverlappingManifest());
+    assert.ok(
+      !r.script.includes('exactly as in inline wave dispatch'),
+      'the pre-#3302 tail comment asserted a merge that no code performed',
+    );
+  });
+});
+
+describe('#3302: the execute:wave:pre fragment bridges Workflow results into the manifest merge chain', () => {
+  const FRAG_3302 = path.join(ROOT, 'capabilities', 'claude-orchestration', 'fragments', 'execute-wave-pre.md');
+
+  function fragContent() {
+    return fs.readFileSync(FRAG_3302, 'utf8');
+  }
+
+  test('C1. instructs recording outcomes via worktree.record-agent after the run', () => {
+    const c = fragContent();
+    assert.match(c, /worktree\.record-agent/, 'the record verb inline dispatch uses must be named');
+    assert.match(c, /WAVE_WORKTREE_MANIFEST/, 'against the wave manifest');
+  });
+
+  test('C2. instructs creating WAVE_WORKTREE_MANIFEST before invoking the tool (step 3 is skipped)', () => {
+    const c = fragContent();
+    assert.match(c, /orchestrator_root/, 'creation block must persist the orchestrator root (#630)');
+    assert.match(c, /mktemp/, 'same mktemp pattern as inline step 3');
+    assert.match(c, /worktrees:\[\]/, 'initialised empty — then populated from outcomes');
+  });
+
+  test('C3. mandates a HALT on uncapturable metadata — never a silently-empty manifest', () => {
+    const c = fragContent();
+    assert.match(c, /HALT on uncapturable metadata/);
+    assert.match(c, /worktreePlans/, 'count check against summary.worktreePlans');
+    assert.match(c, /do NOT run\s*\n?\s*`?worktree\.cleanup-wave/, 'cleanup must not run on a short manifest');
+  });
+
+  test('C4. covers resume: recover from the original run\'s journal or fail loudly', () => {
+    const c = fragContent();
+    assert.match(c, /journal\.jsonl/, 'the documented recovery source for per-agent results');
+    assert.match(c, /[Rr]esume/, 'resume path addressed');
+    assert.match(c, /never report\s*\n?\s*success over silently-dropped/, 'fail loudly, not silently');
+  });
+
+  test('C5. the false "exactly as inline dispatch" claim is gone', () => {
+    const c = fragContent();
+    assert.ok(
+      !c.includes('exactly as it does for inline dispatch'),
+      'the pre-#3302 fragment asserted steps 4-5.8 ran exactly as inline with no bridge',
+    );
+  });
+
+  test('C6. still continues into the unchanged cleanup-wave merge chain', () => {
+    const c = fragContent();
+    assert.match(c, /worktree\.cleanup-wave/, 'step 5.5 merge chain referenced');
+    assert.match(c, /UNCHANGED/, 'the chain itself is unchanged — only fed');
+  });
+});