fix: Ctrl+C should terminate the ArkEnv CLI#1019
Conversation
|
no API key found — Pullfrog needs at least one LLM provider API key (e.g. Open repo secrets → · Configure model → · Setup docs → · Ask in Discord →
|
🦋 Changeset detectedLatest commit: 14a2df7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughCLI gracefully handles Ctrl+C by setting up centralized SIGINT/SIGTERM handlers at startup that flush logs, disable interactive output, and exit with proper codes. Reporter ChangesGraceful SIGINT shutdown with deferred process exit
Sequence DiagramsequenceDiagram
participant CLI as CLI Process
participant Handler as Signal Handler
participant Logger as Logger Instance
participant Output as stdout/stderr
CLI->>Handler: setupGracefulShutdown() at startup
Note over Handler: Registers SIGINT/SIGTERM listeners
User->>Handler: Ctrl+C (SIGINT)
Handler->>Handler: Check isShuttingDown guard
Handler->>Logger: cancel() to flush pending output
Handler->>Logger: flush() to write buffered logs
Handler->>Output: disable interactiveStdout if enabled
Handler->>Handler: setTimeout(2s) for forced exit
Handler->>CLI: process.exit(130)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
🤖 Hi @yamcodes, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
arkenv
@arkenv/bun-plugin
@arkenv/cli
@arkenv/fumadocs-ui
@arkenv/vite-plugin
commit: |
📦 Bundle Size Report✅ All size limits passed! |
|
🤖 I'm sorry @yamcodes, but I was unable to process your request. Please see the logs for more details. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/graceful-shutdown-fix.md:
- Line 7: Update the sentence "The CLI now terminates immediately upon receiving
a `SIGINT` (Ctrl+C)." to reflect the graceful shutdown window: replace it with
wording that says the CLI initiates shutdown on SIGINT, attempts to flush logs
and JSON data, and will terminate after that attempt with a 2‑second timeout (or
similar phrasing matching the 2-second behavior described in the following
lines) so the description aligns with the behavior.
In `@packages/cli/src/adapters/reporters/text.reporter.ts`:
- Around line 50-55: The change to text.reporter.ts removed termination from
fatal(), causing callers like InitUseCase.execute to log fatal errors but
continue and return success; restore termination semantics by either making
fatal() ensure process termination (e.g., call process.exit(1) after writing the
message) or change fatal() to throw an Error (i.e., have it be a never-returning
error path) and then update callers such as InitUseCase.execute to propagate
that exception; locate the fatal(message: string, error?: unknown)
implementation and either reintroduce a terminating call (process.exit/throw) or
update InitUseCase.execute to return an explicit failure status (non-zero) after
calling this.logger.fatal(...) so fatal paths cannot result in exit code 0.
In `@packages/cli/src/index.ts`:
- Around line 44-61: The shutdown function's graceful path currently calls
logger.cancel() and await logger.flush() directly, so if either throws the
shutdown promise can reject and cause a non-intended exit code; wrap the logging
calls in a try/catch and ensure process.exit(code) is invoked in a finally block
so logging is best-effort only. Specifically, update the shutdown function to
call logger.interactiveStdout(false) inside the normal flow, then try {
logger.cancel("Operation cancelled."); await logger.flush(); } catch (err) {
logger.error?.("Logger failed during shutdown", err); } finally { clear/leave
the 2s timeout as-is (it already unref()s) and call process.exit(code); } Ensure
you reference the existing symbols shutdown, isShuttingDown, logger.cancel,
logger.flush, timeout.unref, and process.exit when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f1a3f7d-327b-4862-a849-7f7816e3e545
📒 Files selected for processing (8)
.changeset/graceful-shutdown-fix.mdpackages/cli/src/adapters/reporters.test.tspackages/cli/src/adapters/reporters/json.reporter.tspackages/cli/src/adapters/reporters/silent.reporter.tspackages/cli/src/adapters/reporters/text.reporter.tspackages/cli/src/cli/commands/init.tspackages/cli/src/cli/ui/prompts/wizard.tspackages/cli/src/index.ts
💤 Files with no reviewable changes (1)
- packages/cli/src/adapters/reporters/json.reporter.ts
|
|
||
| #### Improve Ctrl+C handling and implement graceful shutdown | ||
|
|
||
| - The CLI now terminates immediately upon receiving a `SIGINT` (Ctrl+C). |
There was a problem hiding this comment.
Clarify "immediately" to reflect graceful shutdown window.
The wording "terminates immediately" on line 7 could mislead users, since lines 8-9 describe a graceful shutdown with a 2-second timeout window. The CLI doesn't exit instantly—it first attempts to flush logs and JSON data before terminating.
📝 Suggested wording
-The CLI now terminates immediately upon receiving a `SIGINT` (Ctrl+C).
+The CLI now terminates promptly upon receiving a `SIGINT` (Ctrl+C), stopping all wizard progression.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - The CLI now terminates immediately upon receiving a `SIGINT` (Ctrl+C). | |
| - The CLI now terminates promptly upon receiving a `SIGINT` (Ctrl+C), stopping all wizard progression. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.changeset/graceful-shutdown-fix.md at line 7, Update the sentence "The CLI
now terminates immediately upon receiving a `SIGINT` (Ctrl+C)." to reflect the
graceful shutdown window: replace it with wording that says the CLI initiates
shutdown on SIGINT, attempts to flush logs and JSON data, and will terminate
after that attempt with a 2‑second timeout (or similar phrasing matching the
2-second behavior described in the following lines) so the description aligns
with the behavior.
| fatal(message: string, error?: unknown) { | ||
| const msg = `${pc.red(`✘ ${message}`)}\n`; | ||
| process.stderr.write(`${pc.red(`✘ ${message}`)}\n`); | ||
| if (error) { | ||
| const detail = `${pc.red(error instanceof Error ? (error.stack ?? String(error)) : String(error))}\n`; | ||
| process.stderr.write(msg, () => { | ||
| process.stderr.write(detail, () => process.exit(1)); | ||
| }); | ||
| } else { | ||
| process.stderr.write(msg, () => process.exit(1)); | ||
| process.stderr.write(detail); | ||
| } |
There was a problem hiding this comment.
Don't remove fatal() termination semantics without updating callers.
InitUseCase.execute() still catches executor failures, calls this.logger.fatal(...), and then returns. After this contract change, arkenv init can print a fatal scaffolding error and still exit 0. Please make fatal paths rethrow or return an explicit failure status before dropping the old terminating behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/adapters/reporters/text.reporter.ts` around lines 50 - 55,
The change to text.reporter.ts removed termination from fatal(), causing callers
like InitUseCase.execute to log fatal errors but continue and return success;
restore termination semantics by either making fatal() ensure process
termination (e.g., call process.exit(1) after writing the message) or change
fatal() to throw an Error (i.e., have it be a never-returning error path) and
then update callers such as InitUseCase.execute to propagate that exception;
locate the fatal(message: string, error?: unknown) implementation and either
reintroduce a terminating call (process.exit/throw) or update
InitUseCase.execute to return an explicit failure status (non-zero) after
calling this.logger.fatal(...) so fatal paths cannot result in exit code 0.
| if (confirmStrict === null) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Propagate prompt cancellation as SIGINT, not a successful return.
These return null branches stop the flow, but InitUseCase.execute() treats null as a normal early return and main() then falls off the end with exit code 0. Please surface prompt cancellation as a dedicated error/result so the entrypoint can terminate with 130.
Also applies to: 119-120, 151-153
| const shutdown = async (code: number) => { | ||
| if (isShuttingDown) { | ||
| process.exit(code); | ||
| } | ||
| isShuttingDown = true; | ||
|
|
||
| // Force exit after 2 seconds if graceful shutdown hangs | ||
| const timeout = setTimeout(() => { | ||
| process.exit(code); | ||
| }, 2000); | ||
| timeout.unref(); | ||
|
|
||
| if (logger.interactiveStdout) { | ||
| logger.interactiveStdout(false); | ||
| } | ||
| logger.cancel("Operation cancelled."); | ||
| await logger.flush(); | ||
| process.exit(code); |
There was a problem hiding this comment.
Guard the shutdown path against logger failures.
If logger.cancel() or logger.flush() throws here, this promise rejects and the process falls through the global rejection/exception handlers, which turns a SIGINT/SIGTERM into exit code 1. The graceful-shutdown path should treat logging as best-effort and always process.exit(code) from a finally.
💡 Suggested hardening
const shutdown = async (code: number) => {
if (isShuttingDown) {
process.exit(code);
}
isShuttingDown = true;
@@
- if (logger.interactiveStdout) {
- logger.interactiveStdout(false);
- }
- logger.cancel("Operation cancelled.");
- await logger.flush();
- process.exit(code);
+ try {
+ if (logger.interactiveStdout) {
+ logger.interactiveStdout(false);
+ }
+ logger.cancel("Operation cancelled.");
+ await logger.flush();
+ } finally {
+ process.exit(code);
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const shutdown = async (code: number) => { | |
| if (isShuttingDown) { | |
| process.exit(code); | |
| } | |
| isShuttingDown = true; | |
| // Force exit after 2 seconds if graceful shutdown hangs | |
| const timeout = setTimeout(() => { | |
| process.exit(code); | |
| }, 2000); | |
| timeout.unref(); | |
| if (logger.interactiveStdout) { | |
| logger.interactiveStdout(false); | |
| } | |
| logger.cancel("Operation cancelled."); | |
| await logger.flush(); | |
| process.exit(code); | |
| const shutdown = async (code: number) => { | |
| if (isShuttingDown) { | |
| process.exit(code); | |
| } | |
| isShuttingDown = true; | |
| // Force exit after 2 seconds if graceful shutdown hangs | |
| const timeout = setTimeout(() => { | |
| process.exit(code); | |
| }, 2000); | |
| timeout.unref(); | |
| try { | |
| if (logger.interactiveStdout) { | |
| logger.interactiveStdout(false); | |
| } | |
| logger.cancel("Operation cancelled."); | |
| await logger.flush(); | |
| } finally { | |
| process.exit(code); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/index.ts` around lines 44 - 61, The shutdown function's
graceful path currently calls logger.cancel() and await logger.flush() directly,
so if either throws the shutdown promise can reject and cause a non-intended
exit code; wrap the logging calls in a try/catch and ensure process.exit(code)
is invoked in a finally block so logging is best-effort only. Specifically,
update the shutdown function to call logger.interactiveStdout(false) inside the
normal flow, then try { logger.cancel("Operation cancelled."); await
logger.flush(); } catch (err) { logger.error?.("Logger failed during shutdown",
err); } finally { clear/leave the 2s timeout as-is (it already unref()s) and
call process.exit(code); } Ensure you reference the existing symbols shutdown,
isShuttingDown, logger.cancel, logger.flush, timeout.unref, and process.exit
when making the change.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @arkenv/cli@0.0.9 ### Patch Changes - #### Refined setup experience in `arkenv init` _[`#1016`](#1016) [`d536ed7`](d536ed7) [@yamcodes](https://github.com/yamcodes)_ - **Clearer Framework Options**: Updated terminology to better distinguish between server-side runtime validation and client-side bundling integrations. - **Architecture Detection**: Improved detection logic recommends the most efficient configuration based on your project's features. - **Better In-File Guidance**: Generated templates now include comments clarifying validation behavior for your specific environment. - #### Add keyboard navigation hints _[`ac3adcc`](ac3adcc) [@yamcodes](https://github.com/yamcodes)_ - #### Improve Ctrl+C handling and implement graceful shutdown _[`#1019`](#1019) [`102ce4a`](102ce4a) [@yamcodes](https://github.com/yamcodes)_ - Implemented graceful shutdown for `SIGINT` (Ctrl+C) to flush logs and JSON data, with a 2-second safety timeout and support for immediate exit on a second Ctrl+C. - Corrected exit code (130) for `SIGINT` terminations. - Fixed a bug where the `init` wizard would continue after a prompt was cancelled. - #### Respect `tsconfig.json` for path resolution and scaffolding _[`#1013`](#1013) [`0a18edd`](0a18edd) [@yamcodes](https://github.com/yamcodes)_ The Arkenv CLI now dynamically resolves configuration paths and scans project files by respecting `tsconfig.json` settings (`rootDir`, `paths`, `baseUrl`). Key improvements include: - **Robust `tsconfig` Parser**: Added support for parsing `tsconfig.json` files with comments (`jsonc-parser`), handling `extends`, `rootDir`, `baseUrl`, and `paths` alias resolution. - **Dynamic Scaffolding Defaults**: Updated `init` scaffolding logic to suggest project-appropriate default paths based on `compilerOptions.rootDir` rather than hardcoding `./src/env.ts`. - **Advanced Environment Scanning**: Enhanced `getEnvExampleKeys` to scan project source files for `process.env` and `import.meta.env` usages, correctly resolving aliased imports (e.g. `@/env`). - **Framework & Package Manager Detection**: Leveraged parsed `tsconfig.json` context to accurately identify project frameworks and package managers. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fixes #1017. Implements graceful shutdown and improved SIGINT handling.
Summary by CodeRabbit
New Features
Bug Fixes