Skip to content

feat: add shell command prompt (default ':') to execute arbitrary commands safely - #35

Merged
Blankeos merged 6 commits into
Blankeos:mainfrom
felipecm-br:feat/shell-prompt
Oct 7, 2026
Merged

Blankeos merged 6 commits into
Blankeos:mainfrom
felipecm-br:feat/shell-prompt

Conversation

@felipecm-br

@felipecm-br felipecm-br commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Adds a configurable shell command prompt (default :), inspired by lazygit, so commands can run from the repository root without suspending or exiting lazygitrs. The prompt is available in normal panels, diff focus, and comparison mode.

Behavior

  • keybinding.universal.customCommandPrompt defaults to ":", supports remapping, and can be disabled with "". Text entry and existing popups retain priority; footer hints and the command palette reflect the configured binding.
  • Literal command input preserves pasted newlines and quoted spacing. Enter executes; Esc dismisses without execution.
  • Commands run asynchronously in a detached Unix process group with stdin disconnected from the TUI. Esc or Ctrl-C cancels; a five-minute timeout bounds execution. Cleanup terminates the job group and reaps the shell, including when the app drops a running job.
  • The prompt uses $SHELL (fallback sh). Bash explicitly loads ~/.bash_aliases and ~/.bashrc with alias expansion; zsh loads ${ZDOTDIR:-$HOME}/.zshrc. Shell configuration cannot accidentally leave execution in a different working directory. Fish and other shells use their native -c syntax. No temporary execution scripts are needed.
  • Scrollable output shows exit status, stdout and stderr, including failure output even when stdout is nonempty. Capture is capped at 1 MiB per stream, terminal controls are stripped, and the command log keeps a bounded preview. Keyboard and mouse-wheel scrolling and copying are supported.
  • Completion refreshes repository/comparison state while retaining navigation, selected commit files, tree folding, search, and filters. Prompt/output popups consume mouse events instead of clicking through to underlying panels.
  • Queued AI generation and remote-operation results cannot overwrite shell input/output. Commit drafts are preserved when concurrent result popups need to take focus.
  • Existing configured customCommands retain sh -c execution semantics, but use the same asynchronous cancellation, timeout, output, and cleanup path.

Integration

Merged current main into the contributor branch without rewriting history. Preserves #33 tree navigation and #41's restored Ctrl-G commit generation; does not reintroduce the removed HEAD toggle.

Validation

Validated locally on macOS at fe49a49:

  • cargo test --locked: 360 passed, 0 failed, 1 ignored.
  • cargo fmt --check, git diff --check, and cargo build --locked: passed. Existing build warnings remain.
  • Real-terminal PTY smoke: configured footer, prompt opening, successful command/stdout, repository cwd, nonzero exit with stdout + stderr, responsive cancellation, prompt in diff focus, and clean quit all passed.
  • Regression coverage includes shell aliases/functions, native shell arguments, stdin EOF, launch errors, concurrent output capture and limits, timeout/cancellation/process-group cleanup, escaped-descendant pipe draining, remapped/disabled bindings, text-input priority, popup rendering/scrolling, comparison refresh preservation, modal mouse input, and concurrent Ctrl-G/remote-result draft preservation.

Limitations

This is noninteractive execution, not an embedded terminal: editors and password prompts are unsupported. Aliases/functions available only in an already-running interactive shell are not inherited; rc-file noninteractive guards may skip definitions. Background jobs in the command's process group are terminated on completion, so this is not for persistent services. Processes that explicitly create a separate process group/session can escape cleanup; this is not a sandbox. The runner currently requires Unix process-group support (macOS/Linux); non-Unix platforms receive an explicit unsupported error. Linux/cross-platform builds were not run locally.

Preserve tree navigation, commit filters, image previews, and branch checkout fixes while preparing the shell prompt integration.
Run prompt and configured commands off the GUI thread with detached noninteractive processes, five-minute deadlines, process-group cancellation/reaping, and bounded stdout/stderr capture. Replace temporary scripts with safe native shell arguments and bash/zsh rc wrappers while preserving sh semantics for configured commands.

Honor remapped and disabled prompt bindings in list, diff, and compare views without stealing search or popup input. Preserve literal pasted commands, report errors before stdout, expose scrollable/copyable results, bound command-log previews, and refresh repository/comparison data without losing filters or navigation. Add runner, UI, configuration, rendering, and output regressions; document limitations.
Bring PR Blankeos#35 up to date with Blankeos#41 without reintroducing the removed HEAD-toggle shortcut, footer hints, or configuration. Retain shell prompt binding and legacy-key regression coverage.
@Blankeos

Blankeos commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Updated this branch with a normal fast-forward push to fe49a4943235365bd2133289b765bc528c74b7f9; contributor history is preserved and the PR is left open/unmerged.

  • 4eef27c: merge main updates, including tree navigation.
  • 0a25695: asynchronous bounded shell execution, cancellation/timeout/process-group cleanup, correct output/failure reporting, literal input, shell compatibility, universal/remappable shortcuts, and refresh preservation.
  • 5db477a: merge latest main and retain fix: remove HEAD toggle to restore Ctrl-G commit generation #41's Ctrl-G generation behavior.
  • fe49a49: modal mouse handling, concurrent AI/remote-result draft preservation, and final regression tests.

Final local validation: 360 tests passed, 0 failed, 1 ignored; formatting, diff checks, build, and real-terminal PTY workflow all passed. The PR description now reflects the implementation and its noninteractive/Unix-only limitations. No merge or force-push was performed.

@felipecm-br

Copy link
Copy Markdown
Contributor Author

Thanks a lot for the thorough review and great improvements, @Blankeos! 🙌

I just pulled the branch locally and verified everything on Linux:

  • cargo test --locked: all 360 tests pass (and the ignored latency test passes cleanly as well).
  • cargo build --locked, cargo fmt --check, and git diff --check all pass.
  • Verified interactive shell command prompt (:) execution, output scrolling, and cancel handling in a real terminal on Linux.

Everything is running solidly and looks ready to merge whenever you'd like!

@felipecm-br

Copy link
Copy Markdown
Contributor Author

Tested interactively in my daily workflow as well and confirmed everything works as expected. All good to merge from my side! 🚀

@Blankeos
Blankeos merged commit 35752e7 into Blankeos:main Oct 7, 2026
6 checks passed
@Blankeos

Blankeos commented Oct 7, 2026

Copy link
Copy Markdown
Owner

PR review: feat/shell-prompt → main

Merge confidence: 4.8/5 for the fixed local worktree — all five findings addressed.

Important: The fixes are uncommitted and unpushed. GitHub PR #35 still points to fe49a49; the original 3/5 assessment applies to that remote version until these changes are published.

Fix and validation update — 2026-10-07 17:47 UTC

GitHub-ready follow-up

Addressed all five review findings with regressions: refresh notifications now wait behind shell input/execution/output; comparison refresh preserves paginated selection and handles removed commits/refs consistently; shell Escape no longer arms or cancels AI generation; Bash/zsh clear startup positional parameters; and Fish restores repository cwd after startup and clears $argv. Also fixed lost comparison/page-load errors, deferred path pickers, and misleading rebase-success wording. macOS and Linux each pass 370 tests, 0 failures, 1 ignored, with Bash, zsh, and Fish execution coverage. Builds, formatting, whitespace, and a fresh terminal smoke pass. No migration required. Local merge confidence: 4.8/5, pending publication and approving review.

Fixes completed

  • R1: Queue refresh/rebase notifications while shell UI owns the modal; coalesce adjacent duplicate failures. Model/view refresh still applies immediately. Deliver notices in order and defer competing AI/remote/filter results until dismissal.
  • R2: Retain previously loaded comparison pages, restore selection by hash, and page further only when a still-valid selected hash needs it. Use an explicit nearest-row fallback for removed hashes; do not restore stale commit files when a ref is invalid or the commit leaves the comparison.
  • R3: Route shell input/output and deferred-notification keys ahead of AI cancellation. Shell Escape neither arms nor cancels AI generation; terminal-command-modifier filtering remains intact.
  • R4: Clear Bash/zsh positional parameters after rc loading. zsh transports literal path/command values independently of $1/$2, preserving .zshenv/ZDOTDIR startup behavior even when startup replaces arguments.
  • R5: Use a native Fish wrapper to restore repository cwd after startup, clear $argv, and evaluate literal Fish syntax. Tests cover startup cd, argument mutation, functions, and quoted/metacharacter-containing values.
  • Preserve command result visibility when comparison reload fails; show the queued diff error afterward. Do not silently discard commit-page failures arriving during a deferred notification.
  • Replace refresh-derived “Successfully rebased” with “Rebase ended,” acknowledging that disk state may reflect either completion or abort.

Fresh validation of the fixes

Check Result
cargo fmt followed by cargo fmt --check Passed.
git diff --check Passed.
macOS cargo test --locked --offline with Bash/zsh/Fish available 370 passed, 0 failed, 1 ignored.
macOS cargo build --locked --offline Passed; 50 warnings remain.
Linux ARM64 cargo test --locked with Bash 5.2, zsh 5.9, Fish 4.0.2 installed 370 passed, 0 failed, 1 ignored.
Linux ARM64 cargo build --locked Passed; 50 warnings remain.
macOS Fish 4.9.3 focused execution suite 25 passed; Fish execution actually ran, not an early-return skip. Temporary verified Homebrew bottle outside the project; no system installation.
Real-terminal PTY smoke, isolated repos/config/state Passed: prompt, cwd, stdout/stderr, exit 7, cancellation, clean quit; actual Ctrl-G → arm Escape → open/close shell prompt leaves AI alive and restores draft; rebase abort output remains until dismissal before notification; commit selection beyond entry 300 and drilled files survive completion refresh.
Independent final static review No remaining actionable defect found.
Live main/PR check Main unchanged at 6271245; remote PR head remains fe49a49, OPEN/MERGEABLE, approval required.

Linux ran in an automatically removed disposable Docker container with the project mounted read-only, using Rust 1.99.0 and an isolated Cargo target directory. macOS also used Rust 1.99.0. The ignored test is the existing manual checkout-visibility timing benchmark. Test fixtures and helper dependencies stayed in temporary directories; no user Git config or installed shell was changed. Existing unrelated compiler warnings were not blindly auto-fixed.

Logs/harness:

  • /tmp/lazygitrs-pr35-fixes-tests.log
  • /tmp/lazygitrs-pr35-fixes-build.log
  • /tmp/lazygitrs-pr35-fixes-fish.log
  • /tmp/lazygitrs-pr35-fixes-linux-final.log
  • /tmp/lazygitrs-pr35-fixes-pty.py and /tmp/lazygitrs-pr35-fixes-pty.log

Remaining merge checklist and confidence bumps

  • All five findings fixed and regression-tested; dependent error/modal edge cases checked.
  • Revalidate full tests/build/formatting on macOS and Linux with installed shell execution coverage.
  • Fresh real-terminal smoke of rebase, comparison pagination, AI dismissal, and ordinary command behavior.
  • Inspect representative user-specific custom commands for persistent background children, execution longer than five minutes, output beyond the cap, unsupported platforms, and : collisions. Documented intentional runner constraints remain; repository tests cannot audit private user config.
  • Review and publish the local diff to the contributor branch when authorized; rerun checks on the exact published commit. No commit/push performed in this fix pass.
  • Obtain the required approving review and recheck main/head/mergeability before merging. PR deliberately left open and unmerged.

Confidence bump achieved: fixing R1–R5 plus fresh macOS/Linux/shell/PTY evidence raises the reviewed local implementation from 3/5 → 4.8/5. Compatibility confirmation for representative real configurations plus independent approval on the published SHA would support approximately 4.9/5. Approval/publication are procedural gates, not substitutes for correctness evidence; no score guarantees absence of bugs. Windows remains intentionally unsupported by the runner; Linux x86-64 was not separately executed.

Migrations remain unnecessary: no dependency, schema, persisted-state, or mandatory config migration. README documents shell startup behavior and the existing noninteractive/timeout/output/background-process limits.


Original pre-fix review (historical)

The sections below describe the fe49a49 version before the local fixes above; unresolved language and unchecked items are retained as review history, not current local findings.

Local comparison: fix/pr35-shell-prompt → origin/main (same commits as the PR head and live main).

Short GitHub-comment body

Adds a configurable shell prompt and an asynchronous, bounded, cancellable command runner. Local tests pass (360 passed, 1 ignored), as do formatting, whitespace, and build checks. However, refresh notifications can still overwrite shell popups, comparison refresh loses selections beyond the first commit page, and armed AI cancellation can intercept shell Escape. Bash/zsh also leak wrapper arguments into commands, while Fish startup can move execution outside the promised repository directory. Recommend addressing these edge cases with regression tests before merging. No schema/data migration is required; existing custom-command behavior needs a compatibility audit. Merge confidence: 3/5.

Findings

All five findings are P2 / medium: concrete, conditional correctness problems, not a claim that the ordinary execution path is broken. No P0/P1 issue was identified in this review. Passing tests do not cover the scenarios below.

R1 — Background refresh can overwrite the shell-owned popup

Locations: src/gui/mod.rs:2762–2765, 8781–8796; new prompt dispatch at 3643–3653; ownership predicate at 1720–1725.

AI and remote-result receivers respect shell ownership, but other asynchronous popup producers do not. A files-refresh error unconditionally calls show_error, replacing typed shell input or command output. Likewise, after_model_refresh replaces the popup with “Rebase complete.” The prompt is now accessible before rebase-mode dispatch, making this reachable from the rebase-progress view.

Scenario: Open the prompt during an in-progress rebase and execute git rebase --abort or a successful noninteractive git rebase --continue. Shell completion displays output and requests a refresh; the subsequent refresh replaces that output with the rebase notification. A competing refresh can also discard a partially typed command. The abort path additionally exposes the pre-existing misleading “Successfully rebased” notification; this PR does not introduce that wording.

Recommendation: Defer competing notifications while shell input, a job, or output owns the modal. Apply model updates separately from popup presentation. Test both files-refresh failure and rebase completion while typing, running, and viewing output.

Verification: Confirmed by tracing popup assignments and main-loop refresh ordering; this exact UI scenario was not executed locally.

R2 — Comparison refresh loses a selected commit beyond the first 300 entries

Locations: src/gui/controller/custom_commands.rs:174–201; reload at src/gui/controller/diff_mode.rs:817–832; reset at src/gui/modes/diff_mode.rs:155–164.

Completion saves the selected commit hash, but reload_diff_files clears all loaded commits and reloads only DEFAULT_COMMIT_LIMIT (300). Restoration searches that first page only. If the previously selected commit was on a subsequently loaded page, selection remains at index zero. Commit exploration then displays a different commit; drilled-down commit files can remain associated with the old hash while the comparison commit selection points elsewhere.

Scenario: Compare histories containing more than 300 commits, load another page, select a commit there, then run a no-op command such as true. The selected hash cannot be restored from the newly truncated list.

Recommendation: Reload at least the previous loaded count, or page until the selected hash is found. Use an explicit fallback only when the commit genuinely disappears. Test both full-commit and commit-file exploration beyond the first page.

Verification: Confirmed from the reload/reset/restore code. Existing shell-comparison preservation coverage uses a short history and does not exercise this case.

R3 — Shell Escape can cancel an unrelated AI job instead of dismissing the popup

Locations: src/gui/mod.rs:3618–3628, 3033–3057; shell dismissal at 4611–4624.

Running shell jobs receive explicit input priority, but shell input/output popups do not. The AI Escape handler runs before popup dispatch and consumes Escape when its cancellation is already armed.

Scenario: With AI generation active, press Escape once to arm cancellation, open : within the 900 ms window, then press Escape to close it. AI generation is cancelled, and the shell prompt remains open. Shell output has the same ordering problem if cancellation was armed earlier.

Recommendation: Route shell-popup keys before AI cancellation handling, or explicitly exclude shell-owned modals from the AI Escape handler. Assert both popup dismissal and preservation of the unrelated AI job.

Verification: Confirmed from input dispatch and the cancellation state machine; this timing-dependent UI sequence was not executed locally.

R4 — Bash/zsh expose internal wrapper arguments to the user's command

Locations: src/os/shell.rs:135–156, 179–187.

The wrappers save the repository path and command from $1/$2, but never clear positional parameters before eval. Commands therefore see the implementation arguments through $#, $1, and "$@"; an rc file can also leave its own parameters behind. This differs from native shell -c command without additional arguments and can silently change argument-forwarding snippets.

Verification: Executed the actual wrapper strings read from the source using isolated HOME/ZDOTDIR values and a print-only probe:

/bin/bash wrapper: argc=2 arg1=/Users/carlo/Desktop/Projects/lazygitrs
/bin/bash native:  argc=0 arg1=unset
/bin/zsh wrapper:  argc=2 arg1=/Users/carlo/Desktop/Projects/lazygitrs
/bin/zsh native:   argc=0 arg1=unset

Recommendation: Clear positional parameters after rc loading and before evaluating the saved command. Add tests for $#, $1, and "$@", including rc files that run set -- ....

R5 — Fish configuration can move execution outside the repository root

Location: src/os/shell.rs:189–193; repository-root promise in README.md:75–76.

Bash/zsh restore the repository directory after configuration loading. Other shells only receive an initial current_dir plus native -c. Fish executes configuration for noninteractive shells as well; an unconditional cd in config.fish changes the directory before the command runs. Relative-path commands may therefore affect a different directory or repository.

Scenario: With $SHELL=fish, configure an unconditional startup cd to another directory; execute pwd or a relative-path Git command in the prompt.

Recommendation: Restore cwd after startup using a Fish-native, safely parameterized wrapper, or explicitly narrow the documented guarantee. Test an isolated Fish config that changes directory.

Verification: Code inspection plus Fish's configuration-file documentation, which states configuration executes on every shell startup. Not locally reproduced: Fish is not installed. The existing optional Fish execution test returns early when Fish is unavailable, despite appearing as a passing test.

Regressions and compatibility

Yes — conditional regressions/new-feature correctness gaps are identified above. The highest-impact existing-feature interactions are modal replacement, comparison navigation, and AI cancellation. The wrapper-argument and Fish-cwd issues concern new prompt execution semantics.

Positive evidence:

  • main is already an ancestor of HEAD; no current integration conflict.
  • Tests cover tree navigation/footer behavior and restored Ctrl-G generation; no regression was identified in those covered paths. The removed HEAD toggle is not reintroduced.
  • The runner has strong coverage for cancellation, deadlines, output bounds, process-group cleanup, descendant-pipe draining, and stdin EOF.
  • Old configuration remains readable: UniversalKeybinding uses #[serde(default)], and omitted/remapped/disabled prompt-binding tests pass.

Intentional compatibility changes requiring awareness:

  • Existing configured customCommands still use sh -c, but now have a five-minute deadline, bounded output, cancellation, and termination of same-group background children even on successful completion.
  • Configured commands intended to launch persistent background services may stop working as intended. Commands expecting complete output larger than 1 MiB per stream will be truncated.
  • The shared runner rejects non-Unix platforms, including configured commands. Confirm the supported-platform policy if anyone relies on those commands outside macOS/Linux.
  • Process-group/session escape is documented; the runner is not a sandbox. No sandbox guarantee should be inferred from the PR title.

Checks performed for this review

Check Result
gh pr view --json number,title,headRefName No PR found for local fix/pr35-shell-prompt; explicit gh pr view 35 --json ... resolved the contributor PR.
Live PR head vs local HEAD Exact match at fe49a49.
git ls-remote origin refs/heads/main Live main matches local main/origin/main at 6271245. No fetch was needed.
git merge-base --is-ancestor origin/main HEAD Passed.
git diff --check origin/main...HEAD Passed.
cargo fmt --check Passed; formatting was checked, not rewritten.
cargo test --locked --offline 360 passed, 0 failed, 1 ignored.
cargo build --locked --offline Passed; reports 50 warnings.
Bash/zsh wrapper-vs-native argument probe Reproduced R4 on both installed shells.
GitHub checks for exact reviewed SHA Release plan succeeded; artifact build and publication stages skipped. Not Linux test/build evidence.
PR mergeability/protection OPEN, MERGEABLE, but BLOCKED / REVIEW_REQUIRED. One approving review required; no required status-check list configured.
Tracked worktree and index Unchanged after checks. Existing untracked PR_REVIEW_20261007_160335Z.md preserved.

Environment: macOS, Rust/Cargo 1.99.0. Tests/builds may update ignored Cargo artifacts and isolated temporary test fixtures; no application source, configuration, real repository history, branch refs, or PR state was changed. Only this requested review document was added to the worktree.

Logs: /tmp/lazygitrs-pr35-review-tests-20261008.log and /tmp/lazygitrs-pr35-review-build-20261008.log.

Not performed: Linux execution/build, Fish execution, new regression-test implementation, or a fresh PTY UI smoke run. The PR description reports a prior macOS PTY smoke run, but it is not counted as newly verified here. No cross-platform success is inferred from macOS tests or release planning.

Migrations

No schema, database, persisted-state, dependency, or mandatory configuration migration is required. Cargo.toml and Cargo.lock are unchanged. The new binding defaults to : when omitted and can be remapped or disabled with "".

A manual compatibility audit, not a data migration, is advisable for existing custom commands that rely on long execution, full unbounded output, persistent background children, or non-Unix execution. Also inspect any existing : shortcut for a collision: the universal prompt has priority over panel shortcuts, but not active text entry/popups.

Before-merge checklist

Completed during this review:

  • Confirm PR identity and exact head/base refs against GitHub.
  • Confirm latest main is integrated and the PR has no merge conflict.
  • Run formatting, whitespace, locked tests, and locked build checks.
  • Check configuration defaults and dependency/state migration scope.
  • Independently inspect worker lifecycle and GUI integration.

Still needed:

  • R1: Protect shell modal ownership from files-refresh errors and rebase-completion notifications; add regressions for each shell UI phase.
  • R2: Preserve paginated comparison selection and commit-file consistency; test a selected commit beyond entry 300.
  • R3: Prevent shell Escape from cancelling an armed AI job; test actual key-dispatch ordering.
  • R4: Clear Bash/zsh positional arguments before command evaluation; test both ordinary and rc-mutated parameters.
  • R5: Guarantee or explicitly qualify Fish post-startup cwd; test with Fish installed and a startup cd.
  • Rerun the full tests/build/formatting checks after fixes on the resulting head.
  • Run Linux tests/build and shell-cleanup checks with Bash, zsh, and Fish actually installed; avoid silently skipped shell coverage.
  • Run a fresh isolated PTY smoke covering prompt/output, cancellation, rebase notifications, comparison pagination, and concurrent AI cancellation.
  • Audit representative existing custom commands against the new timeout/output/background-process/platform constraints; check binding collisions.
  • Obtain the required approving review and recheck live main/head/mergeability immediately before merge.

Confidence bumps

These are conditional, cumulative estimates, not scores already earned. A green checklist alone does not prove the absence of bugs.

Accomplishment Why confidence improves New estimated score
Fix R1–R3 with focused regression tests Closes the identified existing-feature/modal/navigation regressions. 3.8/5
Also fix R4–R5 and rerun all local checks Aligns shell arguments and cwd with expected execution behavior, closing all five findings. 4.3/5
Also validate Linux, installed Fish/zsh/Bash, and a fresh PTY smoke on the fixed head Adds missing platform/shell and real input-loop evidence beyond unit tests. 4.8/5

Custom-command compatibility review removes rollout uncertainty; the approving review removes the current procedural merge block. Neither is a substitute for the correctness fixes. Recommendation remains request changes, with the PR left open and unmerged.

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.

2 participants