Skip to content

fix: explicit non-TTY handling for add overwrite prompts (theme + component) - #51

Merged
ManningWorks merged 1 commit into
mainfrom
wt/t_08b16d83
Oct 14, 2026
Merged

ManningWorks merged 1 commit into
mainfrom
wt/t_08b16d83

Conversation

@ManningWorks

Copy link
Copy Markdown
Owner

What this does

Closes #49. add was the last command still relying on the incidental event-loop drain that issue #41 fixed for init: the theme re-add path swallowed ExitPromptError in an empty catch {} (silent exit 0 mid-promise — no message, nothing written), and the component re-add path let a prompt cancellation escape to the generic error handler ("✖ Failed to add component — User force closed the prompt with 0 null", exit 1).

Theme path (addTheme):

  • Explicit non-TTY guard mirroring init's overwrite guard: when the destination file exists, stdin is non-interactive, and --force was not passed → refuse up front (…already exists. / Refusing to overwrite in a non-interactive session. / Run in a TTY to confirm, or pass --force to overwrite.) and return — before the prompt is ever attempted.
  • The empty catch {} around the overwrite confirm() is replaced: ExitPromptError → "✖ Add cancelled." + clean return; any other error surfaces instead of being swallowed.
  • The destination existence check now handles only ENOENT (same isNotFoundError shape as init); any other access error throws instead of silently skipping the guard.

Component path (add):

  • The overwrite confirm() is wrapped in an explicit ExitPromptError catch → "✖ Add cancelled." + clean return, mirroring how remove-project handles it. The generic "Failed to add component" handler never sees a prompt cancellation again.

Untouched: edit-project / remove-project / remove-learning — the audit recorded on tracker issue #49 verified they already handle cancellation explicitly; no guards added there.

Verification

  • Live repro (built CLI, stdin from /dev/null):
    • add theme-minimal re-add, no --force → explicit refusal, exit 0, file byte-identical (before: prompt rendered, then silent exit 0 mid-promise)
    • add theme-minimal --force → ⚠ Overwriting existing file (--force), overwrites without prompting, exit 0
    • add github-card re-add → "✖ Add cancelled.", exit 0, no partial writes (before: "✖ Failed to add component — User force closed the prompt with 0 null", exit 1)
  • 6 new tests in add.test.ts: the three acceptance paths above, plus theme TTY prompt force-close → cancel, theme prompt unexpected error → surfaces (rejects), and TTY "no" answer → cancel. Mutation-checked: against the pre-fix code the four guard tests fail RED (exit(1) on cancellation, swallowed errors proceeding to copyFile) and the two characterization tests pass, pinning existing --force/no-answer behavior against refactor.
  • Full gate green: pnpm install && pnpm build && pnpm typecheck && pnpm lint && pnpm test — 64 test files, 1051 tests passing.

@vercel

vercel Bot commented Oct 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
projex-docs Ready Ready Preview Oct 14, 2026 7:50am UTC

@coderabbitai

coderabbitai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fe03e0a8-f788-472e-98c3-4adc633fc686
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ManningWorks

Copy link
Copy Markdown
Owner Author

Review: APPROVE

APPROVE — both axes pass. Full gate re-run on 25cac05 independently: pnpm install && pnpm build && pnpm typecheck && pnpm lint && pnpm test — all green, 64 files / 1051 tests, matching the coder's claim.

Spec (issue 49 brief) — all four acceptance criteria met:

  1. Theme re-add, /dev/null stdin, no --force → explicit refusal before the prompt (add.ts:318-328); no prompt, no write. Tested ("refuses to overwrite an existing theme in a non-interactive session without --force").
  2. Theme + --force → overwrites without prompting (add.ts:352-354). Tested.
  3. Component re-add under /dev/null → "✖ Add cancelled." (add.ts:134-143); the generic "Failed to add component" handler no longer sees a cancellation. Tested.
  4. 6 new tests covering all three paths plus TTY characterization; full gate green, verified by independent re-run.

Standards:

  • Theme guard is a faithful mirror of the approved init pattern (init.ts:54-99): same isNotFoundError shape (add.ts:275-281 ↔ init.ts:360), same three-line refusal message, same ExitPromptError catch shape.
  • error.name === 'ExitPromptError' detection is the established convention across all ten prompting commands.
  • "✖ Add cancelled." matches the existing sibling style in this file; the refusal message matches init's style.
  • Replacing the empty catch {} fixes a pre-existing silent-catch violation; remaining catches handle, rethrow, or exit, and the comments explain why (non-TTY drain rationale).
  • Scope guard holds: exactly 2 files, 1 commit; no changes to edit-project / remove-project / remove-learning, no docs/README, no getSourceFiles changes, no drive-by refactors.

Non-blocking findings (no change requested on this PR):

  • [MEDIUM, consider — follow-up] The new else { throw error } branch in the theme existence check (add.ts:310-315) is untested, while init's identical branch is tested: init.test.ts:181 ("surfaces a non-ENOENT existence-check error instead of writing" — access rejects with EACCES → init() rejects, config unchanged, no exit). A sibling test for addTheme (access rejects with EACCES → add('theme-minimal') rejects with { code: 'EACCES' }, copyFile never called) would complete parity with the mirrored pattern.
  • [LOW, observation] The theme path's rethrow surface is unhandled at CLI level: add() has no catch around the theme branch (add.ts:78-81) and cli.ts has no top-level handler, so a non-ENOENT access error or a non-ExitPromptError confirm failure surfaces as an unhandled-rejection stack + exit 1. This is identical to the surface the approved init guard already has on main (init.ts:54-99 rethrows outside the try/finally), so it is baseline-consistent rather than a defect of this PR — if a polished CLI-level handler is ever wanted, it is a cross-command (init + add) concern for a separate issue.

Adjacent / pre-existing (not from this diff): the misaligned indentation in the source-theme access catch (add.ts:283) and at the end of addTheme ("Import usage:" block) predates the merge base; no action requested here.

Clean, minimal, faithful to the approved pattern — good work.

@ManningWorks
ManningWorks merged commit 9f264e1 into main Oct 14, 2026
7 checks passed
ManningWorks added a commit that referenced this pull request Oct 14, 2026
test: add theme existence-check rethrow coverage (PR #51 review)

This branch was successfully deployed

1 active deployment
Preview — 25cac058 Deployed Oct 14, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-TTY guard from init (#41) should cover the other prompting commands

1 participant