Repository navigation
fix: explicit non-TTY handling for add overwrite prompts (theme + component) - #51
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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. Comment |
Review: APPROVEAPPROVE — 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:
Standards:
Non-blocking findings (no change requested on this PR):
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. |
test: add theme existence-check rethrow coverage (PR #51 review)
What this does
Closes #49.
addwas the last command still relying on the incidental event-loop drain that issue #41 fixed forinit: the theme re-add path swallowedExitPromptErrorin an emptycatch {}(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):init's overwrite guard: when the destination file exists, stdin is non-interactive, and--forcewas 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.catch {}around the overwriteconfirm()is replaced:ExitPromptError→ "✖ Add cancelled." + clean return; any other error surfaces instead of being swallowed.isNotFoundErrorshape asinit); any otheraccesserror throws instead of silently skipping the guard.Component path (
add):confirm()is wrapped in an explicitExitPromptErrorcatch → "✖ Add cancelled." + clean return, mirroring howremove-projecthandles 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
/dev/null):add theme-minimalre-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 0add github-cardre-add → "✖ Add cancelled.", exit 0, no partial writes (before: "✖ Failed to add component — User force closed the prompt with 0 null", exit 1)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 tocopyFile) and the two characterization tests pass, pinning existing--force/no-answer behavior against refactor.pnpm install && pnpm build && pnpm typecheck && pnpm lint && pnpm test— 64 test files, 1051 tests passing.