fix(cli): report stash plan's real outcome — no 'Plan drafted' without the file (rc.3 M2)

.changeset/plan-honest-outcome.md

@@ -0,0 +1,5 @@
+---
+'stash': patch
+---
+
+`stash plan` now reports the outcome that actually occurred instead of unconditionally printing `Plan drafted at .cipherstash/plan.md` and exiting 0 (#738). The plan file is written by the handed-off agent, so the command verifies it on disk after the handoff: "Plan drafted" appears only when the file exists; if a launched agent (Claude Code, Codex, or the wizard) exits without writing it, `plan` errors and exits non-zero so automation never proceeds against a plan that was never created; deferred handoffs (`--target agents-md`, or a CLI target that isn't installed) end with an honest "No plan drafted yet" hint; and a pre-existing plan the run didn't modify is reported as left unchanged rather than drafted. An unexpected filesystem error while reading the plan path (a locked or malformed `.cipherstash/`) now exits non-zero with a clear message rather than an opaque crash.

packages/cli/src/commands/impl/steps/handoff-claude.ts

@@ -85,6 +85,6 @@ export const handoffClaudeStep: HandoffStep = {
       )
     }
 
-    return state
+    return { ...state, agentLaunched: true }
   },
 }

packages/cli/src/commands/impl/steps/handoff-codex.ts

@@ -130,6 +130,6 @@ export const handoffCodexStep: HandoffStep = {
       )
     }
 
-    return state
+    return { ...state, agentLaunched: true }
   },
 }

packages/cli/src/commands/impl/steps/handoff-wizard.ts

@@ -44,6 +44,6 @@ export const handoffWizardStep: HandoffStep = {
       )
     }
 
-    return state
+    return { ...state, agentLaunched: true }
   },
 }

packages/cli/src/commands/init/types.ts

@@ -90,6 +90,13 @@ export interface InitState {
   agents?: AgentEnvironment
   /** What the user picked at the "how to proceed" step. */
   handoff?: HandoffChoice
+  /** True when the handoff step actually launched an agent process
+   *  (`claude` / `codex` / the wizard), regardless of its exit code.
+   *  Deferred handoffs — AGENTS.md, or a CLI target that isn't installed —
+   *  leave it unset. `stash plan` uses this to tell "the agent ran but
+   *  wrote no plan" (an error) from "the plan is written later, when the
+   *  user drives their agent" (#738). */
+  agentLaunched?: boolean
   /** Whether the handoff is producing a plan or executing one. Set by the
    *  command itself: `stash plan` always sets `'plan'`, `stash impl` always
    *  sets `'implement'`. */

packages/cli/src/commands/plan/index.ts

@@ -1,4 +1,4 @@
-import { existsSync } from 'node:fs'
+import { type Stats, statSync } from 'node:fs'
 import { resolve } from 'node:path'
 import { readManifest } from '@cipherstash/migrate'
 import * as p from '@clack/prompts'
@@ -25,6 +25,33 @@ import {
 import { CancelledError, type InitState } from '../init/types.js'
 import { detectPackageManager, runnerCommand } from '../init/utils.js'
 
+/**
+ * Stat the plan file, returning its `Stats` only when it's a regular FILE and
+ * treating "absent or not a usable plan" as `undefined`.
+ *
+ * `throwIfNoEntry: false` turns the common ENOENT into `undefined`. A non-file
+ * — most realistically a DIRECTORY at `.cipherstash/plan.md` — also maps to
+ * `undefined`: `statSync` succeeds for a directory, but the agent cannot have
+ * written a plan there, so without the `isFile()` gate it would read as a
+ * pre-existing/unchanged plan and let the command exit 0 against something no
+ * agent can consume. Any OTHER fs error — ENOTDIR if `.cipherstash` is somehow
+ * a file, EACCES on a locked path, an ELOOP symlink — is converted into a
+ * controlled `CliExit(1)` with an actionable message instead of unwinding as a
+ * generic "Fatal error". This command exists to give automation a reliable
+ * signal about the plan's state (#738), so a filesystem hiccup must not become
+ * an opaque crash, and a non-file must not read as success.
+ */
+function statPlan(planAbs: string): Stats | undefined {
+  try {
+    const stats = statSync(planAbs, { throwIfNoEntry: false })
+    return stats?.isFile() ? stats : undefined
+  } catch (err) {
+    const message = err instanceof Error ? err.message : String(err)
+    p.log.error(`Could not read \`${PLAN_REL_PATH}\`: ${message}`)
+    throw new CliExit(1)
+  }
+}
+
 function buildStateFromContext(
   ctx: ContextFile,
   agents: AgentEnvironment,
@@ -195,8 +222,16 @@ export async function planCommand(
   // because the complete-rollout confirmation needs it too.
   const isInteractive = isInteractiveTty()
 
+  const planAbs = resolve(cwd, PLAN_REL_PATH)
+
   try {
-    if (existsSync(resolve(cwd, PLAN_REL_PATH))) {
+    // Stat (not just existence) so the post-handoff check can tell a revised
+    // plan from a pre-existing one the run never touched (#738). Inside the
+    // try so a filesystem error routes through the same controlled exit as the
+    // rest of the command rather than bypassing it.
+    const planBefore = statPlan(planAbs)
+
+    if (planBefore) {
       p.log.warn(
         `Plan already exists at \`${PLAN_REL_PATH}\`. The agent will be told to revise it; delete the file first if you want to start fresh.`,
       )
@@ -235,11 +270,58 @@ export async function planCommand(
       return
     }
 
-    await howToProceedStep.run(target ? { ...state, handoff: target } : state)
+    const handedOff = await howToProceedStep.run(
+      target ? { ...state, handoff: target } : state,
+    )
+
+    // The plan file is written by the handed-off agent, not by this command,
+    // so the outcome is only knowable from the disk. Verify before claiming
+    // anything — the unconditional "Plan drafted" this replaces let a failed
+    // handoff exit 0 and send `stash impl` after a file that never existed
+    // (#738).
+    const planAfter = statPlan(planAbs)
+
+    if (!planAfter) {
+      if (handedOff.agentLaunched) {
+        // An agent ran and was told to write the plan, but didn't — the
+        // deliverable is missing. Non-zero, so automation never reads this
+        // as "a plan exists".
+        p.log.error(
+          `${messages.plan.notWritten} to \`${PLAN_REL_PATH}\`. The agent may have been interrupted before saving it — re-run \`${cli} plan\` to try again.`,
+        )
+        p.outro('No plan was drafted.')
+        throw new CliExit(1)
+      }
+      // Deferred handoff (AGENTS.md target, or a CLI target that isn't
+      // installed): the files-and-instructions contract was delivered and
+      // the plan is written later, when the user drives their agent. That's
+      // a success for what was runnable — but never claim the plan exists.
+      p.outro(
+        `${messages.plan.noPlanYet} — complete the handoff above, then review \`${PLAN_REL_PATH}\` and run \`${cli} impl\` to implement.`,
+      )
+      return
+    }
+
+    // A pre-existing plan the run didn't modify is still usable (the agent
+    // may have judged it current), but "drafted" would be a false claim —
+    // report which of the two happened.
+    //
+    // Heuristic, deliberately not a content hash: an in-place revision that
+    // preserves BOTH byte size and the mtime tick would misreport as
+    // "unchanged". Blast radius is a cosmetic wording error — the plan is
+    // usable either way, and a real agent write bumps mtime (and usually
+    // size) — so it isn't worth hashing a large file on every run.
+    const wrote =
+      !planBefore ||
+      planAfter.mtimeMs !== planBefore.mtimeMs ||
+      planAfter.size !== planBefore.size
+    const planLine = wrote
+      ? `${messages.plan.drafted} \`${PLAN_REL_PATH}\``
+      : `Plan at \`${PLAN_REL_PATH}\` ${messages.plan.unchanged}`
 
     if (isInteractive) {
       const proceed = await p.confirm({
-        message: `Plan drafted at \`${PLAN_REL_PATH}\`. Continue to \`${cli} impl\` now?`,
+        message: `${planLine}. Continue to \`${cli} impl\` now?`,
         initialValue: true,
       })
       if (!p.isCancel(proceed) && proceed) {
@@ -248,15 +330,13 @@ export async function planCommand(
         await implCommand({}, {})
         return
       }
-      p.outro(
-        `Plan drafted at \`${PLAN_REL_PATH}\`. Review it, then run \`${cli} impl\` to implement.`,
-      )
+      p.outro(`${planLine}. Review it, then run \`${cli} impl\` to implement.`)
     } else {
       // Mirror init's non-TTY hint: the next command will also hit the
       // agent-target picker, so name `--target` here rather than letting
       // the user re-discover the flag on the next exit-cleanly hint.
       p.outro(
-        `Plan drafted at \`${PLAN_REL_PATH}\`. Review it, then run \`${cli} impl --target <claude-code|codex|agents-md|wizard>\` to implement. The \`--target\` flag is required when running non-interactively.`,
+        `${planLine}. Review it, then run \`${cli} impl --target <claude-code|codex|agents-md|wizard>\` to implement. The \`--target\` flag is required when running non-interactively.`,
       )
     }
   } catch (err) {

packages/cli/src/messages.ts

@@ -155,6 +155,23 @@ export const messages = {
     /** Shown when `--yes` confirms the gate-skip (bypasses the prompt). */
     completeRolloutConfirmed:
       'Proceeding with --yes: the production-deploy gate is skipped',
+    /**
+     * Outcome honesty (#738): the plan file is written by the handed-off
+     * agent, not by the CLI, so `stash plan` verifies it on disk after the
+     * handoff and reports what actually happened. These are the stable
+     * leaders the e2e suite asserts on; the path and next-step hints are
+     * appended at the call sites.
+     */
+    drafted: 'Plan drafted at',
+    /** A pre-existing plan the run did not modify — usable, but not "drafted". */
+    unchanged: 'left unchanged by this run',
+    /** An agent was launched and told to write the plan, but the file is
+     *  absent. Exits non-zero — automation must not read this as success. */
+    notWritten: 'The agent handoff finished but no plan was written',
+    /** Deferred handoff (AGENTS.md target, or a CLI target that isn't
+     *  installed): the plan is written later, when the user drives their
+     *  agent. Exit 0, but never claim the plan exists. */
+    noPlanYet: 'No plan drafted yet',
   },
   init: {
     /**

packages/cli/tests/e2e/plan-outcome.e2e.test.ts

@@ -0,0 +1,181 @@
+import {
+  chmodSync,
+  mkdirSync,
+  mkdtempSync,
+  rmSync,
+  symlinkSync,
+  writeFileSync,
+} from 'node:fs'
+import { tmpdir } from 'node:os'
+import { delimiter, join } from 'node:path'
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { messages } from '../../src/messages.js'
+import { runPiped } from '../helpers/spawn-piped.js'
+
+/**
+ * #738: `stash plan` printed `Plan drafted at .cipherstash/plan.md` and exited
+ * 0 unconditionally. The plan file is written by the handed-off agent, not by
+ * the CLI, so a failed or deferred handoff produced a false success — and sent
+ * `stash impl` (and any automation reading the outro) after a file that never
+ * existed. The command now verifies the file on disk after the handoff and
+ * reports the outcome that actually occurred.
+ *
+ * A fake `claude` binary prepended to PATH drives the "agent launched" paths
+ * without the real agent — no DB, no network. (The e2e suite runs on POSIX
+ * only, so a /bin/sh script is fine.)
+ */
+describe('stash plan — outcome reflects the plan file on disk', () => {
+  let dir: string
+
+  beforeEach(() => {
+    dir = mkdtempSync(join(tmpdir(), 'plan-outcome-e2e-'))
+    mkdirSync(join(dir, '.cipherstash'), { recursive: true })
+    writeFileSync(
+      join(dir, '.cipherstash', 'context.json'),
+      JSON.stringify({
+        integration: 'postgresql',
+        packageManager: 'npm',
+        schemas: [],
+      }),
+    )
+  })
+  afterEach(() => {
+    rmSync(dir, { recursive: true, force: true })
+  })
+
+  /**
+   * Write an executable fake `claude` into a bin dir under the fixture and
+   * return a PATH that resolves it first. `spawnAgent` inherits the CLI's
+   * cwd, so the script's relative paths land inside the fixture project.
+   */
+  function fakeClaudePath(script: string): string {
+    const bin = join(dir, 'fake-bin')
+    mkdirSync(bin, { recursive: true })
+    const file = join(bin, 'claude')
+    writeFileSync(file, `#!/bin/sh\n${script}\n`)
+    chmodSync(file, 0o755)
+    return `${bin}${delimiter}${process.env.PATH ?? ''}`
+  }
+
+  it('exits 1 when the launched agent writes no plan (no false success)', async () => {
+    const r = await runPiped(['plan', '--target', 'claude-code'], {
+      cwd: dir,
+      env: { PATH: fakeClaudePath('exit 0') },
+      timeoutMs: 20000,
+    })
+    expect(r.timedOut).toBe(false)
+    expect(r.exitCode).toBe(1)
+    const out = r.stdout + r.stderr
+    expect(out).toContain(messages.plan.notWritten)
+    expect(out).not.toContain(messages.plan.drafted)
+  })
+
+  it('reports "Plan drafted" and exits 0 when the agent wrote the plan', async () => {
+    const r = await runPiped(['plan', '--target', 'claude-code'], {
+      cwd: dir,
+      env: {
+        PATH: fakeClaudePath('echo "# Plan" > .cipherstash/plan.md'),
+      },
+      timeoutMs: 20000,
+    })
+    expect(r.timedOut).toBe(false)
+    expect(r.exitCode).toBe(0)
+    const out = r.stdout + r.stderr
+    expect(out).toContain(`${messages.plan.drafted} \`.cipherstash/plan.md\``)
+  })
+
+  it('reports a pre-existing plan as unchanged, not drafted', async () => {
+    writeFileSync(join(dir, '.cipherstash', 'plan.md'), '# old plan\n')
+    const r = await runPiped(['plan', '--target', 'claude-code'], {
+      cwd: dir,
+      env: { PATH: fakeClaudePath('exit 0') },
+      timeoutMs: 20000,
+    })
+    expect(r.timedOut).toBe(false)
+    // A usable plan exists on disk, so this is not a failure — but the run
+    // must not claim it drafted anything.
+    expect(r.exitCode).toBe(0)
+    const out = r.stdout + r.stderr
+    expect(out).toContain(messages.plan.unchanged)
+    expect(out).not.toContain(messages.plan.drafted)
+  })
+
+  it('reports "drafted" when the agent revises a pre-existing plan', async () => {
+    writeFileSync(join(dir, '.cipherstash', 'plan.md'), '# old plan\n')
+    const r = await runPiped(['plan', '--target', 'claude-code'], {
+      cwd: dir,
+      // The agent appends — a real revision that changes size (and mtime), so
+      // the change-detection limb reports "drafted", not "unchanged".
+      env: { PATH: fakeClaudePath('echo "# revised" >> .cipherstash/plan.md') },
+      timeoutMs: 20000,
+    })
+    expect(r.timedOut).toBe(false)
+    expect(r.exitCode).toBe(0)
+    const out = r.stdout + r.stderr
+    expect(out).toContain(`${messages.plan.drafted} \`.cipherstash/plan.md\``)
+    expect(out).not.toContain(messages.plan.unchanged)
+  })
+
+  it('treats a plan.md directory as no plan, not a false "unchanged"', async () => {
+    // `statSync` succeeds for a directory, but no agent can write a plan there.
+    // Without the isFile() gate this would warn "already exists" and then, with
+    // an agent that writes nothing, report a false "unchanged" exit 0.
+    mkdirSync(join(dir, '.cipherstash', 'plan.md'))
+    const r = await runPiped(['plan', '--target', 'claude-code'], {
+      cwd: dir,
+      env: { PATH: fakeClaudePath('exit 0') },
+      timeoutMs: 20000,
+    })
+    expect(r.timedOut).toBe(false)
+    expect(r.exitCode).toBe(1)
+    const out = r.stdout + r.stderr
+    expect(out).toContain(messages.plan.notWritten)
+    expect(out).not.toContain(messages.plan.unchanged)
+    expect(out).not.toContain(messages.plan.drafted)
+  })
+
+  it('agents-md handoff says "No plan drafted yet" instead of claiming success', async () => {
+    const r = await runPiped(['plan', '--target', 'agents-md'], {
+      cwd: dir,
+      timeoutMs: 20000,
+    })
+    expect(r.timedOut).toBe(false)
+    // The deferred handoff delivered its files-and-instructions contract, so
+    // exit 0 — but the plan is written later, by the user's editor agent.
+    expect(r.exitCode).toBe(0)
+    const out = r.stdout + r.stderr
+    expect(out).toContain(messages.plan.noPlanYet)
+    expect(out).not.toContain(messages.plan.drafted)
+  })
+
+  it('claude-code target without claude on PATH defers honestly', async () => {
+    const r = await runPiped(['plan', '--target', 'claude-code'], {
+      cwd: dir,
+      // A PATH with no `claude` anywhere: the handoff writes files and
+      // prints install instructions instead of spawning.
+      env: { PATH: '/usr/bin:/bin' },
+      timeoutMs: 20000,
+    })
+    expect(r.timedOut).toBe(false)
+    expect(r.exitCode).toBe(0)
+    const out = r.stdout + r.stderr
+    expect(out).toContain(messages.plan.noPlanYet)
+    expect(out).not.toContain(messages.plan.drafted)
+  })
+
+  it('exits 1 with a clear message when the plan path cannot be statted', async () => {
+    // A self-referential symlink at plan.md makes `statSync` throw ELOOP, not
+    // ENOENT — a non-ENOENT fs error that must surface as a controlled exit
+    // (clear message + non-zero), never an opaque "Fatal error" crash.
+    symlinkSync('plan.md', join(dir, '.cipherstash', 'plan.md'))
+    const r = await runPiped(['plan', '--target', 'agents-md'], {
+      cwd: dir,
+      timeoutMs: 20000,
+    })
+    expect(r.timedOut).toBe(false)
+    expect(r.exitCode).toBe(1)
+    const out = r.stdout + r.stderr
+    expect(out).toContain('Could not read')
+    expect(out).not.toContain(messages.plan.drafted)
+  })
+})

skills/stash-cli/SKILL.md

@@ -263,6 +263,8 @@ Each column carries `path: "new" | "migrate"`. `stash impl` parses this to rende
 
 To re-plan, delete `.cipherstash/plan.md` first — otherwise the agent is told to revise it rather than start fresh. `--complete-rollout` is the escape hatch for databases with no deployed application (local dev, sandboxes, test DBs); it's only safe when nothing in production writes to that database.
 
+**The outro reports what actually happened.** The plan file is written by the handed-off agent, so `plan` verifies it on disk before claiming anything: `Plan drafted at .cipherstash/plan.md` appears only when the file exists after the handoff. If a launched agent exits without writing it, `plan` errors and **exits non-zero**. Deferred handoffs — `--target agents-md`, or a Claude Code / Codex target whose CLI isn't installed — end with `No plan drafted yet` and exit 0: the plan lands later, when you drive the agent yourself. A pre-existing plan the run didn't modify is reported as `left unchanged by this run`, not drafted. Either way, check that `.cipherstash/plan.md` exists before acting on it.
+
 ### `impl` — execute
 
 ```bash