-
-
Notifications
You must be signed in to change notification settings - Fork 776
.pr_agent_accepted_suggestions
| PR 758 (2026-04-30) |
[correctness] Reload spawns subshell
Reload spawns subshell
`_omb_cmd_reload` starts a new Bash process, so `omb reload` does not reload the current session; it drops the user into a nested shell and returns to the old environment on exit. This contradicts the CLI completion’s promise to “Reload the current bash session”.omb reload is implemented by starting a new Bash process ($BASH ...), which does not reload the current shell session and conflicts with the command description used in completion.
The repo already reloads the current shell after upgrades by unloading OMB hooks and sourcing ~/.bashrc in-place.
- lib/cli.bash[17-19]
- tools/upgrade.sh[54-59]
- lib/utils.sh[336-342]
Implement _omb_cmd_reload similarly to the upgrade flow:
- call
_omb_util_unloadif it exists -
source ~/.bashrc(in the current shell) Example:
[reliability] Unquoted BASH execution
Unquoted BASH execution
`_omb_cmd_reload` executes `$BASH` unquoted, which permits word-splitting/globbing and can fail if `BASH` is unset or contains spaces. If the implementation continues to exec/restart Bash, it should quote and default the executable path._omb_cmd_reload executes $BASH unquoted, which risks word-splitting/globbing and breaks if BASH is unset.
Even if you switch the implementation to exec a new shell, it should be done safely.
- lib/cli.bash[17-19]
Use a safe default and quote it, e.g.:
| PR 738 (2026-03-01) |
[reliability] Non-atomic sdirs rewrite
Non-atomic sdirs rewrite
Switching from `mv` to `cat >|` truncates the sdirs file before writing completes, so an interrupt or write failure can leave the bookmarks file partially written/empty. Because this file is frequently `source`d, transient corruption can cause runtime errors in other commands/shells./bin/cat "$t" >| "$1" overwrites the sdirs file in-place (truncate-then-write). If interrupted or if the write fails, the bookmarks file can become empty/partially-written, and other commands that source it may error.
We still need to preserve symlinks (the PR goal), but we should avoid in-place truncation. A safer approach is to update the symlink target (or the file itself if not a symlink) using a temp file in the same directory as the final target, then rename (mv) onto the final target.
- plugins/bashmarks/bashmarks.plugin.sh[215-229]
- plugins/bashmarks/bashmarks.plugin.sh[146-179]
| PR 737 (2026-02-26) |
[reliability] Non-zero return on missing tmux
Non-zero return on missing tmux
The plugin returns `1` when `tmux` is not installed. This is inconsistent with other plugins that only warn and continue, and the non-zero status can propagate to the module loader and potentially abort initialization for users running with `set -e`/errexit.plugins/tmux/tmux.plugin.sh returns exit code 1 when tmux is not installed. This non-zero status is propagated by the module loader and can disrupt initialization in strict shells.
Other plugins in this repo (e.g., fzf, virtualenvwrapper) warn but do not return non-zero when a dependency is missing.
- plugins/tmux/tmux.plugin.sh[5-8]
- lib/utils.sh[83-118]
- plugins/fzf/fzf.plugin.sh[3-9]
[reliability] `md5sum` dependency unchecked
`md5sum` dependency unchecked
The `tds` session naming logic calls `md5sum` without verifying it exists, which can break the plugin on systems where `md5sum` is unavailable. This is an unhandled failure point and prevents the alias from working reliably.The tmux tds helper uses md5sum unconditionally, which can fail in environments where md5sum is not installed (e.g., some macOS setups), causing the alias to break.
This plugin is intended to provide portable aliases; tds should degrade gracefully if hashing tools are missing.
- plugins/tmux/tmux.plugin.sh[23-26]