feat(cli): add per-stage spinners to mgw:run pipeline steps

[snipcodeit]

Summary

• Adds lib/spinner.cjs — a zero-dependency spinner utility with createSpinner and withSpinner helpers that cycle |/-\ frames with elapsed time in TTY mode, falling back to plain log lines in non-TTY/CI environments
• Integrates spinners into bin/mgw.cjs so mgw run displays a named pipeline stage overview before handing off to claude, and a live spinner in --quiet mode that shows succeed/fail on completion
• Exports createSpinner, withSpinner, and SUPPORTS_SPINNER from lib/index.cjs barrel for use by any future pipeline step
• Adds 17 unit tests in test/spinner.test.cjs covering all spinner methods, non-TTY fallback behavior, and withSpinner error propagation

Closes #120

Milestone Context

• Milestone: v4 — Interactive CLI & TUI
• Phase: 28 — Pipeline Progress Indicators and Live Status
• Issue: 4 of 9 in milestone

Changes

lib/spinner.cjs (new)

• createSpinner(stage) — returns { start, succeed, fail, stop } object; animates at 100ms in TTY, plain lines in non-TTY
• withSpinner(stage, fn, opts) — wraps async fn with spinner lifecycle; rethrows on reject
• SUPPORTS_SPINNER — boolean export for TTY+non-CI detection

lib/index.cjs

• Added require('./spinner.cjs') to barrel export

bin/mgw.cjs

• Added STAGE_LABELS map for human-readable stage names per command
• Added RUN_PIPELINE_STAGES array for pipeline stage banner
• Updated runAiCommand() — shows spinner in --quiet mode wrapping the full claude invocation; shows brief stage indicator in streaming mode
• Updated run command action — prints pipeline stage overview (validate → triage → create-worktree → execute-gsd → create-pr) before invoking claude

test/mgw.test.cjs

• Added spinner.cjs to the lib modules existence check

test/spinner.test.cjs (new)

• 17 tests covering module exports, createSpinner methods, and withSpinner success/fail/rethrow paths

Test Plan

• All 127 tests pass (npm test — 110 existing + 17 new spinner tests)
• createSpinner start/succeed/fail/stop methods verified in non-TTY test environment
• withSpinner resolves return values, emits success output, rethrows on reject
• SUPPORTS_SPINNER is correctly false in piped/non-TTY contexts
• lib/index.cjs barrel exports createSpinner, withSpinner, SUPPORTS_SPINNER
• Manual: run mgw run <number> --quiet and verify spinner appears with elapsed time
• Manual: run mgw run <number> (streaming) and verify stage banner prints before claude output

[snipcodeit]

Review: PR #164 — per-stage spinners for mgw:run

This is clean, well-tested work. The spinner module is properly isolated, respects TTY/CI/NO_COLOR detection, and the 17 unit tests cover the important paths. Approving with a few notes.

---

🟡 80ms streaming mode spinner is a timing assumption

In bin/mgw.cjs, streaming mode does:

const spinner = createSpinner(`mgw:${stageLabel}`);
spinner.start();
await new Promise(r => setTimeout(r, 80));
spinner.stop();

This is essentially a 80ms flash to show something is happening before Claude's streaming output takes over. The 80ms assumption is fine for normal machines, but on a slow system (or if invokeClaude starts a subprocess with some latency), the spinner might stop before the first Claude output appears, leaving a gap.

Consider logging a non-animated stage header instead for streaming mode, since the user will see Claude's output anyway:

// Streaming: just print a stage header, no animation needed
if (!opts.json) {
  process.stdout.write(`mgw:${stageLabel} → starting...\n`);
}

This avoids the timing assumption entirely. Not blocking, just a suggestion.

---

🟡 stage parameter mutation in start()

function start(label) {
  if (label) stage = label;  // mutates the outer closure variable
  ...
}

This mutates the stage closed-over variable. It works, but it means calling spinner.start('override') after spinner.stop() and then spinner.start() again would use the overridden label. Minor, but worth documenting or removing the override parameter if it's not needed.

---

🟡 lib/index.cjs conflict risk with PR #165

Both this PR and PR #165 add a single line to lib/index.cjs:

• PR #164: adds ...require('./spinner.cjs')
• PR #165: adds ...require('./progress.cjs')

These will conflict if merged sequentially without resolving. Whoever merges second needs to include both lines.

---

✅ Strong points

• SUPPORTS_SPINNER = IS_TTY && !IS_CI is exactly right — no surprises in CI
• clearLine() using \r + spaces + \r is the correct portable approach
• withSpinner properly rethrows — error propagation is correct
• Non-TTY fallback prints plain [mgw] stage... lines — good for CI logs
• Test coverage is thorough: 17 tests including edge cases and error propagation
• All 127 existing tests still pass