fix(wave_init): atomic kahuna bootstrap + plan persist (resequence)

handlers/wave_init.ts

@@ -1,6 +1,29 @@
 // Wave-init handler — platform-agnostic shell. Platform calls go through
 // `getAdapter()`; plan/state helpers live in `lib/wave_init_plan.ts`.
 // Story 2.22 (#316).
+//
+// Atomicity guarantee (#378). The handler interleaves the kahuna bootstrap
+// around the `wave-status init` plan-persist call so a kahuna failure can
+// never leave a half-state where the plan is on disk but the kahuna branch
+// is missing from remote. Sequence:
+//
+//   1. Validate input (zod) and run extend-mode pre-scan against existing
+//      state.json (read-only — no mutation).
+//   2. Pre-check kahuna state and CREATE the kahuna branch on remote (if
+//      `kahuna` arg provided). On failure → return error; nothing was
+//      persisted to disk.
+//   3. Run `wave-status init` to persist the plan to disk (creates
+//      state.json + phases-waves.json).
+//   4. Record the kahuna branch in state.json via `wave-status
+//      set-kahuna-branch` (must run after step 3 — set-kahuna-branch
+//      requires state.json to exist).
+//
+// Failure semantics:
+// - Step 2 fails → no plan persisted. Retry converges trivially.
+// - Step 3 fails → branch exists on remote, no state.json. Retry's step 2
+//   sees the existing branch and claims it as idempotent reuse (see the
+//   "recorded === null + branch present" path in `bootstrapKahunaBranchRemote`),
+//   then proceeds to retry step 3.
 import { execSync } from 'child_process';
 import { join } from 'path';
 import { z } from 'zod';
@@ -9,8 +32,9 @@ import { parseRepoSlug } from '../lib/shared/parse-repo-slug.js';
 import { getAdapter } from '../lib/adapters/index.js';
 import {
   projectDir, writePlanFile, statusDir, readJson, countIssuesFromPlan,
-  extendModePrescan, readPhasesWavesTotals, bootstrapKahunaBranch,
-  branchExistsOnRemote, type PlanData, type StateData,
+  extendModePrescan, readPhasesWavesTotals, bootstrapKahunaBranchRemote,
+  recordKahunaBranchInState, branchExistsOnRemote, fileExists,
+  type PlanData, type StateData,
 } from '../lib/wave_init_plan.js';
 import { repoOptionalSchema } from '../lib/schemas/repo.js';
 
@@ -29,12 +53,26 @@ const inputSchema = z.object({
 const envelope = (payload: unknown) => ({ content: [{ type: 'text' as const, text: JSON.stringify(payload) }] });
 const shellQuote = (s: string) => `'${s.replace(/'/g, `'\\''`)}'`;
 
+/**
+ * Read state.json, tolerating its absence (fresh-init mode where the file
+ * doesn't exist yet because `wave-status init` hasn't run). Returns the
+ * empty default `{kahuna_branch: null}` when missing — callers feed this
+ * into `bootstrapKahunaBranchRemote` which treats null-recorded as "fresh
+ * or recoverable claim" depending on remote state.
+ */
+async function readStateOrDefault(statePath: string): Promise<StateData> {
+  if (!(await fileExists(statePath))) return { kahuna_branch: null };
+  return (await readJson(statePath)) as StateData;
+}
+
 const waveInitHandler: HandlerDef = {
   name: 'wave_init',
   description:
     'Initialize a wave plan from structured JSON; supports --extend mode. ' +
     'Optional `kahuna` argument bootstraps a `kahuna/<plan_id>-<slug>` branch ' +
-    'off the plan\'s base_branch (default `main`) and records it in wave state.',
+    'off the plan\'s base_branch (default `main`) and records it in wave state. ' +
+    'Atomic across kahuna bootstrap + plan persist (#378): kahuna failure does ' +
+    'not persist the plan; plan-persist failure leaves the branch claimable on retry.',
   inputSchema,
   async execute(rawArgs: unknown) {
     let args: z.infer<typeof inputSchema>;
@@ -47,29 +85,51 @@ const waveInitHandler: HandlerDef = {
       if (!pre.ok) return envelope(pre);
     }
     try {
-      const planFile = writePlanFile(args.plan_json);
-      const extendFlag = args.extend ? ' --extend' : '';
-      const repoFlag = args.repo ? ` --repo ${shellQuote(args.repo)}` : '';
-      execSync(`wave-status init${extendFlag}${repoFlag} ${planFile}`, { cwd, encoding: 'utf8' });
-
       const plan = JSON.parse(args.plan_json) as PlanData;
-      const counts = countIssuesFromPlan(plan);
-      const totals = await readPhasesWavesTotals(cwd);
 
-      let kahuna: { kahuna_branch: string; kahuna_created: boolean } | undefined;
+      // ---- Step 2: kahuna bootstrap (REMOTE only — no state writes) -------
+      // Runs BEFORE plan persist so a kahuna failure doesn't strand state.
+      let kahunaBranch: string | undefined;
+      let kahunaCreated = false;
+      let kahunaPreviouslyRecorded = false;
       if (args.kahuna !== undefined) {
+        const slug = args.repo ?? parseRepoSlug() ?? undefined;
         const baseBranch = typeof plan.base_branch === 'string' && plan.base_branch.length > 0 ? plan.base_branch : 'main';
         const statePath = join(await statusDir(cwd), 'state.json');
-        const slug = args.repo ?? parseRepoSlug() ?? undefined;
-        const result = await bootstrapKahunaBranch(cwd, args.kahuna, baseBranch,
-          async () => (await readJson(statePath)) as StateData,
+        const remote = await bootstrapKahunaBranchRemote(cwd, args.kahuna, baseBranch,
+          () => readStateOrDefault(statePath),
           {
             adapter: getAdapter({ repo: slug }), slug,
-            recordKahunaBranch: (b) => { execSync(`wave-status set-kahuna-branch ${shellQuote(b)}`, { cwd, encoding: 'utf8' }); },
             branchPresentOnRemote: (b) => branchExistsOnRemote(cwd, b),
           });
-        if (!result.ok) return envelope({ ok: false, error: result.error });
-        kahuna = { kahuna_branch: result.kahuna_branch, kahuna_created: result.created };
+        if (!remote.ok) return envelope({ ok: false, error: remote.error });
+        kahunaBranch = remote.kahuna_branch;
+        kahunaCreated = remote.created;
+        kahunaPreviouslyRecorded = remote.previously_recorded;
+      }
+
+      // ---- Step 3: persist plan to disk -----------------------------------
+      const planFile = writePlanFile(args.plan_json);
+      const extendFlag = args.extend ? ' --extend' : '';
+      const repoFlag = args.repo ? ` --repo ${shellQuote(args.repo)}` : '';
+      execSync(`wave-status init${extendFlag}${repoFlag} ${planFile}`, { cwd, encoding: 'utf8' });
+
+      const counts = countIssuesFromPlan(plan);
+      const totals = await readPhasesWavesTotals(cwd);
+
+      // ---- Step 4: record kahuna branch in state.json ---------------------
+      // `set-kahuna-branch` requires state.json (created by step 3). Skipped
+      // when state.json already had the matching branch recorded — no work
+      // to do, and the `wave-status init` for a fresh init wouldn't have
+      // preserved it anyway (only relevant in extend mode).
+      let kahuna: { kahuna_branch: string; kahuna_created: boolean } | undefined;
+      if (kahunaBranch !== undefined) {
+        if (!kahunaPreviouslyRecorded) {
+          const recordResult = recordKahunaBranchInState(kahunaBranch,
+            (b) => { execSync(`wave-status set-kahuna-branch ${shellQuote(b)}`, { cwd, encoding: 'utf8' }); });
+          if (!recordResult.ok) return envelope({ ok: false, error: recordResult.error });
+        }
+        kahuna = { kahuna_branch: kahunaBranch, kahuna_created: kahunaCreated };
       }
 
       return envelope({ ok: true, mode: args.extend ? 'extend' : 'init', ...counts, ...totals, ...(kahuna ?? {}) });

lib/wave_init_plan.ts

@@ -159,33 +159,82 @@ export async function readPhasesWavesTotals(
 // ---------------------------------------------------------------------------
 // KAHUNA bootstrap — platform-facing calls go through the `PlatformAdapter`
 // so the handler remains free of platform branching (R-09).
+//
+// Atomicity (#378): the bootstrap is split into two phases so the handler
+// can interleave them around the `wave-status init` plan-persist step:
+//
+//   1. `bootstrapKahunaBranchRemote` — pre-check + create branch on remote.
+//      Reads state.json if present (extend mode), tolerates absence (fresh
+//      init). Returns `{ok, kahuna_branch, created}` or error. NO state.json
+//      writes — safe to run BEFORE `wave-status init` exists.
+//   2. Handler runs `wave-status init` to persist the plan to disk.
+//   3. `recordKahunaBranchInState` — writes the branch name into state.json
+//      via `wave-status set-kahuna-branch`. Must run AFTER step 2 because
+//      `set-kahuna-branch` requires state.json to exist.
+//
+// Failure modes after resequencing:
+// - Phase 1 fails → no plan persisted, no remote branch (createBranch is
+//   atomic at the platform API level). Retry converges trivially.
+// - Phase 2 fails → remote branch exists but state empty. Retry's phase 1
+//   sees `recorded === null` AND remote has desired → claims as idempotent
+//   reuse (NOT orphan-refused). This is the key behavior change vs the
+//   pre-#378 single-phase function: "orphan with matching name" is now a
+//   successful claim, not an error, because the branch name is fully
+//   determined by `kahuna/<plan_id>-<slug>` and a name collision across
+//   different plans is impossible (plan_id is the unique tracking-issue
+//   number for the master plan).
+// - Phase 3 fails → `wave-status set-kahuna-branch` is a local file write
+//   and rarely fails; if it does, the state has no kahuna_branch field but
+//   the remote branch exists. Retry would hit `wave-status init`'s "already
+//   initialized" guard. Out of scope for #378; tracked separately if it
+//   ever arises in practice.
 // ---------------------------------------------------------------------------
 
 export interface KahunaBootstrapResult {
   ok: true;
   kahuna_branch: string;
+  /** `true` if the adapter created the remote branch in this call; `false` if a
+   * matching branch already existed (claim/reuse). */
   created: boolean;
+  /** `true` if state.json's `kahuna_branch` already matched `desired` before
+   * this call (idempotent-reuse path). Handler uses this to skip the redundant
+   * `wave-status set-kahuna-branch` write. `false` when state was empty or the
+   * call created a new branch — in both cases the handler must record. */
+  previously_recorded: boolean;
 }
 export interface KahunaBootstrapError {
   ok: false;
   error: string;
 }
 
-export interface KahunaBootstrapDeps {
+export interface KahunaBootstrapRemoteDeps {
   adapter: Pick<PlatformAdapter, 'resolveBranchSha' | 'createBranch'>;
-  /** CLI shell-out — `wave-status set-kahuna-branch <name>`. Injectable for testing. */
-  recordKahunaBranch: (branch: string) => void;
   /** `git ls-remote` probe — local git, not a platform API. Injectable for testing. */
   branchPresentOnRemote: (branch: string) => boolean;
   slug: string | undefined;
 }
 
-export async function bootstrapKahunaBranch(
+/** Back-compat alias — pre-#378 callers passed a `recordKahunaBranch` callback. */
+export interface KahunaBootstrapDeps extends KahunaBootstrapRemoteDeps {
+  /** CLI shell-out — `wave-status set-kahuna-branch <name>`. Injectable for testing. */
+  recordKahunaBranch: (branch: string) => void;
+}
+
+/**
+ * Phase 1 of #378's atomic kahuna bootstrap: pre-check state + create the
+ * branch on remote, but do NOT write to state.json. Safe to call BEFORE
+ * `wave-status init` (state.json may not yet exist).
+ *
+ * `readState` should return `{kahuna_branch: null}` when state.json is
+ * absent (fresh init). Callers in the handler use a wrapper that swallows
+ * ENOENT and yields the empty default.
+ */
+export async function bootstrapKahunaBranchRemote(
   cwd: string,
   kahuna: { plan_id: number; slug: string },
   baseBranch: string,
   readState: () => Promise<{ kahuna_branch?: string | null }>,
-  deps: KahunaBootstrapDeps,
+  deps: KahunaBootstrapRemoteDeps,
 ): Promise<KahunaBootstrapResult | KahunaBootstrapError> {
   void cwd;
   const desired = `kahuna/${kahuna.plan_id}-${kahuna.slug}`;
@@ -194,7 +243,7 @@ export async function bootstrapKahunaBranch(
 
   if (recorded === desired) {
     if (deps.branchPresentOnRemote(desired)) {
-      return { ok: true, kahuna_branch: desired, created: false };
+      return { ok: true, kahuna_branch: desired, created: false, previously_recorded: true };
     }
     return {
       ok: false,
@@ -208,12 +257,18 @@ export async function bootstrapKahunaBranch(
     };
   }
 
-  // recorded === null: state unset. Check the remote for an orphan.
+  // recorded === null: state unset. Check the remote for an existing branch
+  // matching `desired`.
+  //
+  // #378: if the remote has the EXACT desired branch (`kahuna/<plan_id>-<slug>`),
+  // claim it as idempotent reuse rather than refusing as an orphan. The branch
+  // name is fully determined by request inputs; a true "orphan from a different
+  // plan" is impossible because plan_id is the unique tracking-issue number for
+  // the master plan, and (plan_id, slug) is a deterministic mapping. This makes
+  // wave_init retry-safe after a phase-2 (`wave-status init`) failure that left
+  // the branch on remote but no state.json.
   if (deps.branchPresentOnRemote(desired)) {
-    return {
-      ok: false,
-      error: `orphan kahuna branch ${desired} exists on remote but is not recorded in state — manual triage required`,
-    };
+    return { ok: true, kahuna_branch: desired, created: false, previously_recorded: false };
   }
 
   // Fresh creation path — adapter reads main HEAD SHA + creates the branch.
@@ -233,15 +288,52 @@ export async function bootstrapKahunaBranch(
     return { ok: false, error: createResult.error };
   }
 
+  return { ok: true, kahuna_branch: desired, created: true, previously_recorded: false };
+}
+
+/**
+ * Phase 3 of #378's atomic kahuna bootstrap: record the branch name in
+ * state.json via `wave-status set-kahuna-branch`. Must run AFTER
+ * `wave-status init` (which creates state.json). Idempotent — safe to call
+ * even when the branch was discovered as already-existing in phase 1.
+ */
+export function recordKahunaBranchInState(
+  branch: string,
+  recordKahunaBranch: (branch: string) => void,
+): { ok: true } | KahunaBootstrapError {
   try {
-    deps.recordKahunaBranch(desired);
+    recordKahunaBranch(branch);
+    return { ok: true };
   } catch (err) {
     return {
       ok: false,
       error: `wave-status set-kahuna-branch failed: ${err instanceof Error ? err.message : String(err)}`,
     };
   }
-  return { ok: true, kahuna_branch: desired, created: true };
+}
+
+/**
+ * Pre-#378 single-phase entry point — kept for back-compat with any caller
+ * outside the wave_init handler. New callers should use the resequenced
+ * pair (`bootstrapKahunaBranchRemote` + `recordKahunaBranchInState`) so
+ * plan-persist failures don't strand a branch on remote with no state.
+ *
+ * @deprecated Use the two-phase API for atomic bootstrap. See #378.
+ */
+export async function bootstrapKahunaBranch(
+  cwd: string,
+  kahuna: { plan_id: number; slug: string },
+  baseBranch: string,
+  readState: () => Promise<{ kahuna_branch?: string | null }>,
+  deps: KahunaBootstrapDeps,
+): Promise<KahunaBootstrapResult | KahunaBootstrapError> {
+  const remote = await bootstrapKahunaBranchRemote(cwd, kahuna, baseBranch, readState, deps);
+  if (!remote.ok) return remote;
+  if (!remote.previously_recorded) {
+    const recorded = recordKahunaBranchInState(remote.kahuna_branch, deps.recordKahunaBranch);
+    if (!recorded.ok) return recorded;
+  }
+  return remote;
 }
 
 function extractSha(