chore(ship): fix stale workflow test, document ad hoc ChatGPT consultation

skills/ship/SKILL.md

@@ -137,6 +137,10 @@ this default beyond which Workflow script gets invoked — both are described th
   dropped: the default/code loops return `accepted` and `rejected` lists to record; the
   ChatGPT-author loop's rejections land as `## Review responses` entries in the plan
   itself, and a `needs_human` outcome there carries the round's raw `findings`.
+- Beyond the two formal loops, the coordinator may consult ChatGPT ad hoc for a
+  genuinely hard judgment call — never pass-counted (it isn't a `chatgpt-review`
+  invocation, so it doesn't conflict with the single-permitted-invocation rule above),
+  never a substitute for internal review; see `references/review-loops.md`.
 
 ### Output capture
 

skills/ship/references/per-issue-cycle.md

@@ -175,7 +175,10 @@ Before handing the unit back, verify:
 - **Security-sensitive** (auth / OAuth / `config.json`): add one focused pass with the
   `security-review` skill.
 
-Do not run a generic code review plus a multi-agent review over the same diff.
+Do not run a generic code review plus a multi-agent review over the same diff. None of
+these tiers include asking ChatGPT ad hoc — that's a distinct, coordinator-only escalation
+for when internal review genuinely can't resolve a tradeoff (`references/review-loops.md`),
+not a substitute for running this budget first.
 
 Reviewer prompt:
 
@@ -220,7 +223,9 @@ When a finding exposes a **missing invariant** rather than an isolated bug:
 
 If two review rounds find variants of one root cause, revise the invariant map before
 writing another fix. Fixes that relocate a defect are how a one-pass review becomes
-four.
+four. If even this process doesn't settle which structural fix is right,
+`references/review-loops.md`'s ad hoc consultation procedure is for exactly this — a
+genuine tradeoff internal review can't resolve, not routine debugging.
 
 ## 4 — Reconcile (before the PR — and before certification)
 

skills/ship/references/review-loops.md

@@ -117,6 +117,33 @@ its task notification; do not poll and do not start other review work meanwhile.
   Only after ruling out both (or after they fail to actually resolve it) does
   `SKILL.md`'s FULL STOP apply.
 
+- **Ad hoc consultation for a genuinely hard judgment call.** The two formal loops certify
+  a complete artifact against a verdict protocol — not the right tool for a single mid-cycle
+  question. When internal review (the risk-based budget in `per-issue-cycle.md` step 3, or
+  the coordinator's own judgment) genuinely cannot resolve a concrete tradeoff between two or
+  more approaches with no clear repo precedent — not a routine implementation choice, and not
+  a substitute for running that internal review first — the coordinator may ask ChatGPT one
+  self-contained question outside any pass-counted protocol:
+
+  1. By step 2.3, a ChatGPT conversation already exists for this unit (at minimum the
+     plan-review/plan-author session from 2.2) — always continue THAT conversation (or a
+     further-along code-review session), never open a new one, per "one unit, one ChatGPT
+     conversation" above.
+  2. Drive the existing tab directly (the same technique as the pass-cap and
+     stalled-generation procedures above). Frame it explicitly as an ad hoc consultation, not
+     a formal review — no `VERDICT:` line, just the concrete question, the specific approaches
+     under consideration, and why internal review didn't settle it.
+  3. **Verify the answer yourself before acting on it** — the same standing principle as every
+     formal finding, applied directly by the coordinator (read the real code/tests) since
+     there is no separate fact-check pass for an ad hoc exchange.
+  4. Record the question, the answer, and what was decided in the unit's ship-log entry under
+     "Decisions taken" (`per-issue-cycle.md` already asks for decisions made under ambiguity
+     there) — cite the conversation URL.
+
+  Never touches a pass counter (it isn't a `chatgpt-review plan`/`pr` CLI invocation) and is
+  coordinator-only, same as every other `chatgpt-review` touchpoint. One self-contained
+  question at a time, not an open-ended back-and-forth.
+
 ## Default plan loop — `plan-review-loop.workflow.mjs`
 
 ```

skills/ship/tests/workflow-contract.test.mjs

@@ -32,20 +32,29 @@ test('the canonical plan path remains the authoring-session identity', async ()
   assert.match(workflow, /--output-file \$\{shellQuote\(runArgs\.planFile\)\}/);
 });
 
-test('the decompose-and-implement loop grounds decomposition in the real branch, runs sub-tasks sequentially, and propagates failure', async () => {
+test('the decompose-and-implement loop grounds decomposition in the real branch, runs waves in dependency order, parallelizing only within an explicitly-independent wave, and propagates failure', async () => {
   const source = await fs.readFile(path.join(root, 'references/decompose-and-implement-loop.workflow.mjs'), 'utf8');
   assert.doesNotThrow(() => new Function(`return async function workflowSyntaxCheck() {\n${source.replace('export const meta', 'const meta')}\n}`));
   // Decomposition must read what's ALREADY committed, not just the plan in the abstract.
   assert.match(source, /git log --oneline origin\/main\.\.HEAD/);
   assert.match(source, /model: 'fable', effort: 'high'/);
-  // Sequential, not parallel — a for-loop with one agent() awaited per iteration, no
-  // parallel()/pipeline() call fanning sub-tasks out concurrently.
-  assert.match(source, /for \(const \[index, task\] of decomposition\.subtasks\.entries\(\)\)/);
-  assert.doesNotMatch(source, /parallel\(/);
+  // Waves run strictly in dependency order; a solo wave (no independent sibling) runs its
+  // one sub-task directly, never via parallel() — only a wave the decomposition agent
+  // explicitly declared independent (size > 1) fans out concurrently, scoped to that one
+  // wave's own sub-tasks.
+  assert.match(source, /for \(const \[waveIndex, wave\] of waves\.entries\(\)\)/);
+  assert.match(source, /if \(wave\.length === 1\)/);
+  assert.match(source, /parallel\(wave\.map\(/);
   assert.doesNotMatch(source, /pipeline\(/);
-  assert.match(source, /model: 'sonnet'/);
-  // A dead sub-task agent must stop the loop, not silently skip the rest.
-  assert.match(source, /if \(!result\) return \{ status: 'error'/);
+  // Each sub-task is implemented by a fresh Sonnet coding agent, on both the solo path
+  // and the concurrent-wave path.
+  assert.match(source, /\{ label: task\.id, phase: 'Implement', model: 'sonnet' \}/);
+  assert.match(source, /\{ label: task\.id, phase: 'Implement', model: 'sonnet', isolation: 'worktree'/);
+  // A dead sub-task agent must stop the loop at every call site, not silently skip the
+  // rest: the solo path, the concurrent-wave path, and post-wave integration.
+  assert.match(source, /if \(!summary\) return \{ status: 'error'/);
+  assert.match(source, /const diedIndex = waveOutputs\.findIndex\(o => !o\)/);
+  assert.match(source, /if \(!integration\) return \{ status: 'error'/);
 });
 
 test('required args are validated and the issue tag is derived for commit messages', async () => {