feat: make command explanation and confirmation opt-in via --explain flag - #146
feat: make command explanation and confirmation opt-in via --explain flag#146bernoussama wants to merge 3 commits into
Conversation
…flag Replace the --silent flag (which disabled explanation) with --explain/-e flag that enables both explanation and confirmation. By default, commands are now generated and executed immediately without explanation or confirmation prompt. - Add -e/--explain CLI option (off by default) - When --explain is passed: show explanation + interactive confirm/refine/cancel - When not passed: generate command, display it, and run immediately - Update generateCommandStruct default to explanation=false - Update README to document the new --explain flag
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the user interaction flow for the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly implements the --explain flag to make command explanation and confirmation optional, changing the default behavior to execute commands immediately. The implementation is solid, but I've suggested a small refactoring in src/index.ts to reduce code duplication and improve maintainability.
| if (explain) { | ||
| const action = await select({ | ||
| message: 'Run command?', | ||
| options: [ | ||
| { label: '✅ Yes', value: 'execute' }, | ||
| { label: '🔧 Refine', value: 'refine' }, | ||
| { label: '❌ Cancel', value: 'cancel' }, | ||
| ], | ||
| }); | ||
| if (isCancel(action)) { | ||
| cancel('Operation cancelled.'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| switch (action) { | ||
| case 'execute': | ||
| outro(`running command: ${chalk.green(command)}`); | ||
| runCommand(command); | ||
| shouldContinue = false; | ||
| break; | ||
| case 'refine': | ||
| currentPrompt = await refineCommand(currentPrompt, command); | ||
| break; | ||
| case 'cancel': | ||
| outro(chalk.yellow('Command cancelled.')); | ||
| return; | ||
| } | ||
| } else { | ||
| outro(`running command: ${chalk.green(command)}`); | ||
| runCommand(command); | ||
| shouldContinue = false; | ||
| } |
There was a problem hiding this comment.
There's some code duplication for executing the command. You can refactor this logic to avoid repeating the outro, runCommand, and shouldContinue = false lines. This will make the code more maintainable.
let execute = !explain;
if (explain) {
const action = await select({
message: 'Run command?',
options: [
{ label: '✅ Yes', value: 'execute' },
{ label: '🔧 Refine', value: 'refine' },
{ label: '❌ Cancel', value: 'cancel' },
],
});
if (isCancel(action)) {
cancel('Operation cancelled.');
process.exit(0);
}
switch (action) {
case 'execute':
execute = true;
break;
case 'refine':
currentPrompt = await refineCommand(currentPrompt, command);
break;
case 'cancel':
outro(chalk.yellow('Command cancelled.'));
return;
}
}
if (execute) {
outro(`running command: ${chalk.green(command)}`);
runCommand(command);
shouldContinue = false;
}Separate explanation and confirmation into independent flags: - -e/--explain: show AI-generated explanation of the command - -c/--confirm: prompt for confirmation before running (Yes/Refine/Cancel) Both are off by default. They can be used independently or together.
WalkthroughThe CLI replaces the previous silent mode with Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as src/index.ts
participant AI as generateCommandStruct
participant Prompt as Confirmation UI
participant Exec as runCommand
User->>CLI: Invoke with prompt (+ --explain / --confirm)
CLI->>AI: generateCommandStruct(prompt, config, explain)
AI-->>CLI: Return { command, explanation? }
alt explanation present
CLI-->>User: Display explanation
end
alt confirm enabled
CLI->>Prompt: Show "Run command?" (Yes / Refine / Cancel)
Prompt-->>User: Choose
alt User -> Yes
CLI->>Exec: await runCommand(command)
else User -> Refine
CLI-->>User: Return for refinement
else User -> Cancel
CLI-->>User: Abort
end
else
CLI->>Exec: await runCommand(command)
end
Exec-->>User: Streamed output / exit code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 138be3e734
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else { | ||
| outro(`running command: ${chalk.green(command)}`); | ||
| runCommand(command); |
There was a problem hiding this comment.
This new default removes the only runtime guard between the model output and exec(): runCommand() in src/utils.ts shells out to whatever string the model returned, while generateCommandStruct() in src/lib/ai.ts only asks the model to be safe and even allows warning strings like error: ambiguous request. For prompts that are ambiguous or mildly destructive (delete old log files, kill the node servers, etc.), a model misgeneration now becomes an immediate side effect instead of something the user can cancel or refine, which is a significant safety regression from the previous default.
Useful? React with 👍 / 👎.
| .option('-e, --explain', 'show explanation of the generated command') | ||
| .option('-c, --confirm', 'ask for confirmation before running the command') |
There was a problem hiding this comment.
--silent as a deprecated alias
Replacing -s/--silent outright is a breaking CLI change for existing aliases, scripts, and dotfiles that followed the previously documented flag. With commander 14 (package.json), unknown options abort parsing before .action() runs, so lsh --silent "..." now exits with an error instead of generating a command. Even if --explain is the new preferred flag, we should still accept --silent as a no-op/deprecated alias for compatibility.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR changes LazyShell’s CLI defaults so that AI-generated commands run immediately by default, and makes both the generated explanation and the interactive confirmation prompt opt-in via new flags.
Changes:
- Replaces
--silentwith--explainand--confirmflags, which independently toggle explanation output and pre-execution confirmation. - Updates
generateCommandStructto defaultexplanationtofalse. - Updates README usage/docs to match the new flags and default behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/lib/ai.ts |
Changes generateCommandStruct default so explanations are not generated unless explicitly requested. |
src/index.ts |
Adds --explain / --confirm flags and runs commands immediately when --confirm isn’t provided. |
README.md |
Documents the new default behavior and the new flags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if (confirm) { | ||
| const action = await select({ | ||
| message: 'Run command?', | ||
| options: [ | ||
| { label: '✅ Yes', value: 'execute' }, | ||
| { label: '🔧 Refine', value: 'refine' }, | ||
| { label: '❌ Cancel', value: 'cancel' }, | ||
| ], | ||
| }); | ||
| if (isCancel(action)) { | ||
| cancel('Operation cancelled.'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| switch (action) { | ||
| case 'execute': | ||
| outro(`running command: ${chalk.green(command)}`); | ||
| runCommand(command); | ||
| shouldContinue = false; | ||
| break; | ||
| case 'refine': | ||
| currentPrompt = await refineCommand(currentPrompt, command); | ||
| break; | ||
| case 'cancel': | ||
| outro(chalk.yellow('Command cancelled.')); | ||
| return; | ||
| } | ||
| } else { | ||
| outro(`running command: ${chalk.green(command)}`); | ||
| runCommand(command); | ||
| shouldContinue = false; | ||
| } |
| export async function generateCommandStruct( | ||
| prompt: string, | ||
| modelConfig?: ModelConfig, | ||
| explanation: boolean = true | ||
| explanation: boolean = false | ||
| ): Promise<Command | CommandWithExplanation> { | ||
| const modelConf = modelConfig || getDefaultModel(); | ||
| const schema = explanation ? zCmdExp : zCmd; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/ai.ts (1)
207-209:⚠️ Potential issue | 🟡 MinorError path returns inconsistent shape regardless of
explanationparameter.When
explanation=false, the function should return aCommand(only{ command }), but the error path always returns{ command, explanation: '' }. This breaks the type contract and makes type guards like'explanation' in resultunreliable.The current code in
src/index.tsworks around this by checking truthiness (result.explanation), but this should be fixed for type consistency.🔧 Proposed fix
} catch { const result = await generateCommand(prompt, modelConfig); - return { command: result, explanation: '' }; + return explanation ? { command: result, explanation: '' } : { command: result }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ai.ts` around lines 207 - 209, The catch block always returns { command, explanation: '' } which violates the expected shape when explanation is disabled; update the catch to return only { command } when modelConf.explanation is false and return { command, explanation: '' } otherwise—inspect modelConf.explanation in the catch, call generateCommand(prompt, modelConf) as now, then conditionally return either the Command-only shape or the CommandWithExplanation shape so type guards work correctly (refer to generateCommand and modelConf).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/lib/ai.ts`:
- Around line 207-209: The catch block always returns { command, explanation: ''
} which violates the expected shape when explanation is disabled; update the
catch to return only { command } when modelConf.explanation is false and return
{ command, explanation: '' } otherwise—inspect modelConf.explanation in the
catch, call generateCommand(prompt, modelConf) as now, then conditionally return
either the Command-only shape or the CommandWithExplanation shape so type guards
work correctly (refer to generateCommand and modelConf).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0144303-01c4-424f-a90f-e50ede6b5bf2
📒 Files selected for processing (3)
README.mdsrc/index.tssrc/lib/ai.ts
runCommand was using fire-and-forget exec() without awaiting the child process, so the Node.js process could exit before the command finished. This was masked when the interactive confirmation prompt was always shown. Changes: - Rewrite runCommand to use spawn with stdio:'inherit' and return a Promise<number> that resolves with the exit code - Await runCommand at both call sites in index.ts - Add tests for runCommand (exit codes, pipes, args) and CLI flag parsing - Add @types/bun and bun-types to tsconfig for test type support - Add 'test' script to package.json
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/index.ts (1)
90-121: Consider propagating the command exit code.The
runCommandnow returns aPromise<number>with the exit code, but it's currently ignored. For better CLI behavior, consider propagating a non-zero exit code to the process:♻️ Proposed enhancement
case 'execute': outro(`running command: ${chalk.green(command)}`); - await runCommand(command); + const exitCode = await runCommand(command); shouldContinue = false; + if (exitCode !== 0) process.exitCode = exitCode; break;Apply similar change to the non-confirm path at lines 118-120.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 90 - 121, runCommand now returns a Promise<number> (exit code) but the code ignores it; capture the returned exit code in both the 'execute' case inside the confirm flow and the non-confirm branch, and if the exit code is non-zero call process.exit(exitCode) to propagate failure; update the 'execute' branch (switch case for 'execute') and the else branch that calls runCommand(command) to await the result into a variable (e.g., exitCode) and call process.exit(exitCode) when exitCode !== 0, otherwise continue setting shouldContinue = false or return as appropriate.src/utils.test.ts (1)
4-28: Good test coverage for the happy path.The tests effectively verify exit code handling for various command scenarios. Consider adding a test for the error rejection path (e.g., when
spawnitself fails due to an invalid shell) to ensure thereject(err)branch inrunCommandis covered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils.test.ts` around lines 4 - 28, Add a unit test to cover the rejection/error branch in runCommand by simulating a spawn failure: in the test file, mock child_process.spawn (e.g., with jest.spyOn(require('child_process'), 'spawn').mockImplementationOnce(() => { throw new Error('spawn failed') }) or return a mock EventEmitter that emits an 'error' event) and assert that runCommand rejects with that error (use await expect(runCommand(...)).rejects.toThrow('spawn failed')). This ensures the reject(err) branch inside runCommand is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/index.ts`:
- Around line 90-121: runCommand now returns a Promise<number> (exit code) but
the code ignores it; capture the returned exit code in both the 'execute' case
inside the confirm flow and the non-confirm branch, and if the exit code is
non-zero call process.exit(exitCode) to propagate failure; update the 'execute'
branch (switch case for 'execute') and the else branch that calls
runCommand(command) to await the result into a variable (e.g., exitCode) and
call process.exit(exitCode) when exitCode !== 0, otherwise continue setting
shouldContinue = false or return as appropriate.
In `@src/utils.test.ts`:
- Around line 4-28: Add a unit test to cover the rejection/error branch in
runCommand by simulating a spawn failure: in the test file, mock
child_process.spawn (e.g., with jest.spyOn(require('child_process'),
'spawn').mockImplementationOnce(() => { throw new Error('spawn failed') }) or
return a mock EventEmitter that emits an 'error' event) and assert that
runCommand rejects with that error (use await
expect(runCommand(...)).rejects.toThrow('spawn failed')). This ensures the
reject(err) branch inside runCommand is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6aebdfe-4f52-437b-8981-28f8dde78a39
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
package.jsonsrc/cli.test.tssrc/index.tssrc/utils.test.tssrc/utils.tstsconfig.json
Summary
Makes the generated command explanation and confirmation prompt optional, independent, and off by default, each togglable via its own CLI flag. Also fixes broken command execution.
Changes
Before
--silent)runCommandused fire-and-forgetexec()— process could exit before command finishedAfter
-e/--explain: show the AI-generated explanation of the command-c/--confirm: present the interactive confirmation prompt (Yes/Refine/Cancel) before executingrunCommandnow usesspawnwithstdio: 'inherit'and returns aPromise<number>(exit code), properly awaited by the callerFiles changed
src/index.ts-s, --silentwith-e, --explainand-c, --confirm; awaitrunCommandsrc/lib/ai.tsgenerateCommandStructdefaultexplanationparam fromtruetofalsesrc/utils.tsrunCommandto usespawn+stdio: 'inherit', returnPromise<number>README.mdsrc/utils.test.tsrunCommand(exit codes, pipes, args, unknown commands)src/cli.test.ts-e,-c,--explain,--confirm, defaults)package.jsontestscript,@types/bundev dependencytsconfig.jsonbun-typesfor test type supportUsage
Summary by CodeRabbit