fix(branch merge): don't crash on the apply confirmation in non-interactive shells

[agent-zhang-beihai]

What was broken

insforge branch merge <name> called clack.confirm from @clack/prompts directly. @clack/core constructs new tty.WriteStream(0) unconditionally, so in a shell without a usable TTY (CI, agent sandboxes) the command printed the full reviewed SQL and the change summary, then died on the confirmation step with:

TTY initialization failed: uv_tty_init returned EBADF (bad file descriptor)

Nothing was applied, and the only way through was to re-run the identical, already-reviewed merge with -y.

What changed

src/commands/branch/merge.ts:

• Prompt goes through the TTY-safe wrapper src/lib/prompts.ts (prompts.confirm / prompts.isCancel) instead of @clack/prompts directly, so the confirmation can no longer blow up on TTY init.
• When stdin is not interactive and neither -y nor --json is set, throw CLIError('Applying a merge requires confirmation, but stdin is not interactive. Re-run with -y to apply, or with --dry-run to preview the SQL.', 1, 'MERGE_CONFIRMATION_REQUIRED'). This follows the existing convention in confirmPurchase (src/commands/domains/index.ts), and fires before the dry-run is computed and the plan printed — the feedback specifically asked to be told about -y before the full merge plan is dumped.

--dry-run, --json, and -y behavior are unchanged; the guard cannot apply a merge, it only fails fast.

How it was verified

• Added a regression test in src/commands/branch/merge.test.ts: merge feat-x with isInteractive: false and no -y exits 1, the error text contains Re-run with -y, the confirm prompt is never invoked, and mergeBranchExecuteApi is never called.
• The test file now mocks ../../lib/prompts.js instead of @clack/prompts, matching the module the command actually uses (same pattern as posthog/setup.test.ts and webscraper/apify/connect.test.ts).
• npm run lint (vitest + eslint): 782 passed / 13 skipped, eslint clean.

Skill sync

Per DEVELOPMENT.md §3: no InsForge/agent-skills update needed — no command, flag, default, or output shape changed; only the non-TTY failure mode did.

Addresses user feedback a6be7db1-7051-4d7d-99b4-a6933362bd2f (cli): Branch merge requires explicit -y in non-TTY Codex shell

🤖 Generated with Claude Code

---

Summary by cubic

Prevent insforge branch merge <name> from crashing on apply confirmation in non-interactive shells. Previously it printed the merge plan and exited with EBADF; now it fails fast with an actionable error unless -y or --json is set, and uses a TTY-safe prompt.

• Route confirmation through src/lib/prompts.ts (prompts.confirm/prompts.isCancel) instead of @clack/prompts to avoid @clack/core TTY initialization.
• Add early guard: when stdin is non-interactive and neither -y nor --json is provided (and not --dry-run), throw CLIError('MERGE_CONFIRMATION_REQUIRED') instructing to re-run with -y or --dry-run before computing/printing the plan.
• Keep --dry-run, --json, and -y behavior unchanged; the guard never applies changes and prints nothing on that path.
• Add a regression test in src/commands/branch/merge.test.ts that asserts the early error, no prompt, and no execute call; tests mock ../../lib/prompts.js rather than @clack/prompts.
• Addresses Linear feedback a6be7db1 requiring explicit -y in non-TTY environments.

^{Written for commit 1621f32. Summary will update on new commits.}

[greptile-apps]

Greptile Summary

The PR prevents branch merges from reaching TTY-dependent confirmation in non-interactive shells and routes interactive confirmation through the shared prompt wrapper.

• Adds an early actionable error when an apply requires confirmation but stdin is non-interactive.
• Preserves --dry-run, --json, and -y behavior.
• Adds regression coverage ensuring the non-interactive path neither prompts nor executes the merge.

Confidence Score: 5/5

The PR appears safe to merge, with no actionable changed-code defects identified.

The new guard fails before preview or execution when confirmation cannot be obtained, while dry-run and explicit non-confirming modes continue through their existing paths.

Important Files Changed

Filename	Overview
src/commands/branch/merge.ts	Adds a fail-closed non-interactive confirmation guard and replaces direct Clack calls with the shared TTY-safe prompt wrapper.
src/commands/branch/merge.test.ts	Updates the prompt mock to match the command dependency and verifies that non-interactive merges fail before prompting or execution.

_{Reviews (1): Last reviewed commit: "fix(branch merge): don't crash on confir..." | Re-trigger Greptile}

[jwfing]

Review — fix(branch merge): don't crash on the apply confirmation in non-interactive shells

Summary: Small, well-scoped fix that routes the merge confirmation through the TTY-safe prompts wrapper and fails fast with an actionable error in non-interactive shells before printing the merge plan — the change is correct, idiomatic, and covered by a regression test.

Requirements context

No /docs/superpowers/ in this repo. Specs live under docs/specs/ (per repo convention), and none there matches a branch-merge/TTY concern — this is a feedback-fix PR (feedback/a6be7db1), so I assessed against the PR description and the reported feedback item alone. The implementation faithfully addresses the reported symptom (uv_tty_init returned EBADF crash on the confirm step in non-TTY shells).

Findings

Critical

(none)

Suggestion

(none)

Information

• Functionality — piped-stdin confirmation is now foreclosed for merge (src/commands/branch/merge.ts:43-50). The guard fires whenever !prompts.isInteractive, i.e. any non-TTY stdin — including a genuine pipe (yes | insforge branch merge feat-x) that the prompts.confirm wrapper's nonTtyConfirm path (src/lib/prompts.ts:195-211) could otherwise have answered. This is not a regression (the old clack.confirm path crashed on any non-TTY stdin anyway) and it matches the established confirmPurchase precedent (src/commands/domains/index.ts:140-152), where an explicit flag is required rather than reading a piped "y" for a destructive/irreversible action. Requiring -y explicitly is the safer choice for an apply; noting only so the trade-off is on record.
• Scope — other direct clack.confirm/@clack/prompts callers remain (src/lib/credentials.ts, src/lib/auth.ts, src/lib/apify-bridge.ts, src/commands/webscraper/apify/*, etc.). They may share the same non-TTY crash mode, but fixing them is out of scope for this feedback item — leaving them untouched is correct scope discipline, flagged only for follow-up awareness.

Dimension coverage

• Software engineering: ✅ Regression test added (src/commands/branch/merge.test.ts:143-179) asserting exit code 1, error text contains Re-run with -y, confirm prompt never invoked, and mergeBranchExecuteApi never called. The test mock was correctly switched from @clack/prompts to ../../lib/prompts.js to match the module the command actually imports (same pattern as posthog/setup.test.ts). CLIError(msg, 1, 'MERGE_CONFIRMATION_REQUIRED') matches the repo's error-code convention.
• Functionality: ✅ The guard is placed before mergeBranchDryRunApi (line 53), so the plan is no longer printed before the failure — exactly what the feedback asked for. -y, --json, and --dry-run correctly bypass the guard; the confirm branch (line 102) is now only reachable when interactive, so prompts.confirm always takes the clack path there. No behavior change for existing paths.
• Security: ✅ No security-relevant changes — no new user input reaching SQL/shell/HTTP, no secrets/PII logged, no auth/authorization change. rendered_sql printing is unchanged.
• Performance: ✅ No new queries or loops; the early guard actually saves the dry-run network round-trip in the non-interactive-without--y case.

Verdict

approved (informational — human approval is a separate action). Zero Critical findings; the two notes are informational only and need not block merge.

[jwfing]

LGTM - approved.