fix(setup): preserve interactive init and cancellation#42
Conversation
Zsh startup benchmarkUbuntu 24.04 with Linuxbrew, atuin, fnm, fzf, zoxide, zinit, and the shipped plugins. Lower is better. Warm startup (daily use)
Cache-miss startupCompletion and tool-init caches are removed before every run; installed tools and plugins remain.
|
There was a problem hiding this comment.
Code Review
This pull request updates the init command to run an interactive guided initialization instead of a non-interactive setup with defaults, updates documentation and help text accordingly, and improves error handling by propagating Ctrl-C from Homebrew installations and handling .zshrc overwrite cancellations. The review feedback highlights that mapping init to runSetup() without arguments makes it identical to setup, creating redundant options in the CLI and installer. Additionally, the test assertion in tests/shell.test.js using expect.objectContaining inside toThrow is not supported in Vitest/Jest and should be refactored to catch and assert the error properties directly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| case "init": | ||
| await runSetup({ defaults: true }); | ||
| await runSetup(); | ||
| break; |
There was a problem hiding this comment.
With this change, the init command now calls runSetup() with no arguments, making it identical in behavior to the setup command. Both commands will run the full interactive setup flow.
This redundancy makes the distinction between init and setup in install.sh, README.md, and the help text confusing for users, as they are presented as two different options (e.g., 'interactive, choose each step yourself' vs 'interactive guided initialization') but result in the exact same experience.
If init and setup are intended to be identical, consider simplifying the CLI and installer by removing the redundant option or clearly documenting init as an alias. If they are intended to be different, init should pass specific options to runSetup to differentiate its behavior.
| echo 'How would you like to run suitup?' >&2 | ||
| echo ' 1) setup — interactive, choose each step yourself' >&2 | ||
| echo ' 2) init — non-interactive, install everything with recommended defaults' >&2 | ||
| echo ' 2) init — interactive guided initialization' >&2 |
There was a problem hiding this comment.
Since the init command has been updated in src/cli.js to call runSetup() without any arguments, it now behaves identically to the setup command.
Presenting these as two distinct options here (with different descriptions) is misleading to the user, as both choices will launch the exact same interactive setup flow. Consider removing this prompt and defaulting to a single setup flow, or updating the descriptions to reflect that they are now identical.
| throw Object.assign(new Error("interrupted"), { signal: "SIGINT", status: null }); | ||
| }); | ||
|
|
||
| expect(() => brewInstall("jq")).toThrow(expect.objectContaining({ interrupted: true })); |
There was a problem hiding this comment.
In Vitest (and Jest), passing an asymmetric matcher like expect.objectContaining directly to toThrow (or toThrowError) is not natively supported and may not assert the properties of the thrown error as expected. It often defaults to checking if any error is thrown, which can lead to silent test passes even if the error properties are incorrect.
To reliably assert custom properties on a thrown error, you should catch the error and assert its properties directly.
let thrownError;
try {
brewInstall("jq");
} catch (error) {
thrownError = error;
}
expect(thrownError).toBeDefined();
expect(thrownError.interrupted).toBe(true);## Background PR #42 changed the `init` route from `runSetup({ defaults: true })` to `runSetup()`. That made `init` behave exactly like interactive `setup`, even though the two commands are intended to serve different use cases. The same path could also launch `p10k configure`, introducing a secondary interactive wizard into an otherwise unattended initialization flow. `setup` should collect user choices. `init` should pass an explicit option to the shared setup engine so recommended steps, tools, apps, and presets are selected without prompts. ## Root cause The CLI stopped passing the recommended-defaults option for `init`. Without that parameter, the shared setup engine entered its normal prompt-driven path. In addition, the defaults branch explicitly launched the Powerlevel10k configuration wizard when no existing configuration was found. ## Changes - restore `init` routing through `runSetup({ defaults: true })` - keep Powerlevel10k personalization out of the non-interactive flow - document the permanent `setup`/`init` design boundary in `AGENTS.md` - add `CLAUDE.md` as a symlink to `AGENTS.md` so coding agents share the same project guidance - synchronize CLI help, installer messaging, and both README translations - update regression tests to assert the recommended-defaults parameter and non-interactive messaging ## Design contract - `setup` remains interactive and asks for steps and content - `init` remains non-interactive and selects recommended content through setup-engine parameters and recommended-value lists - package installation stays non-interactive after selections are resolved - secondary personalization tools such as `p10k configure` require a later explicit user action ## Verification - `npm test` - 234 Vitest tests passed - 12 Zsh performance tests passed - `bash -n install.sh` - `git diff --check`
问题
init错误地把整个初始化流程设为非交互,替用户选择步骤和软件.zshrc覆盖确认处按Ctrl-C会被当作拒绝覆盖,后续安装仍继续SIGINT会被brewInstall吞掉修复
init恢复步骤、工具和应用的交互式问答,只有底层包安装保持非交互SIGINT验证
npm test(231 tests passed)git diff --check