Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions notifiarr-branch-builder.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
#!/bin/bash

# Extend the PATH to include the go binary directory
export PATH=$PATH:/usr/local/go/bin
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Quote PATH expansion for robustness.

While PATH rarely contains spaces, quoting variable expansions is a shell scripting best practice.

♻️ Proposed fix
-export PATH=$PATH:/usr/local/go/bin
+export PATH="$PATH:/usr/local/go/bin"
📝 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.

Suggested change
export PATH=$PATH:/usr/local/go/bin
export PATH="$PATH:/usr/local/go/bin"
🤖 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 `@notifiarr-branch-builder.sh` at line 4, The PATH export should quote the
expansion to be robust against spaces; update the export command from export
PATH=$PATH:/usr/local/go/bin to export PATH="$PATH:/usr/local/go/bin" so the
PATH variable expansion is quoted (affecting the export of PATH).


# Function to display error messages and exit with status 1
handle_error() {
echo "Error: $1" >&2
exit 1
}

# Function to display usage information
display_help() {
echo "Usage: $0 [options]"
echo "Options:"
echo " -h, --help Display this help message"
echo " --repo-url URL Set the repository URL (default: https://github.com/Notifiarr/notifiarr.git)"
echo " --repo-dir DIR Set the repository directory (default: /opt/notifiarr-repo)"
echo " --bin-path PATH Set the binary path (default: /usr/bin/notifiarr)"
echo " --branch BRANCH Set the branch (default: master)"
echo " --reinstall-apt Reinstall Notifiarr using apt without prompting."
exit 0
}

# Function to check and prompt for installation of a required tool
ensure_tool_installed() {
local tool=$1
local install_cmd=$2
if ! command -v "$tool" &>/dev/null; then
read -p "$tool is not installed. Do you want to install it? [Y/n] " response
if [[ "$response" =~ ^[Yy] ]]; then
Comment on lines +30 to +31
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix default behavior to match prompt convention.

The prompt uses [Y/n] notation suggesting "Y" is the default (accepted on Enter), but the regex ^[Yy] only matches explicit Y/y input. An empty response (Enter key) won't match and will not install the tool.

🔧 Proposed fix to accept empty input as yes
-    read -p "$tool is not installed. Do you want to install it? [Y/n] " response
-    if [[ "$response" =~ ^[Yy] ]]; then
+    read -p "$tool is not installed. Do you want to install it? [Y/n] " response
+    if [[ -z "$response" || "$response" =~ ^[Yy] ]]; then
       eval "$install_cmd" || handle_error "Failed to install $tool."
📝 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.

Suggested change
read -p "$tool is not installed. Do you want to install it? [Y/n] " response
if [[ "$response" =~ ^[Yy] ]]; then
read -p "$tool is not installed. Do you want to install it? [Y/n] " response
if [[ -z "$response" || "$response" =~ ^[Yy] ]]; then
eval "$install_cmd" || handle_error "Failed to install $tool."
🤖 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 `@notifiarr-branch-builder.sh` around lines 30 - 31, The prompt shows "[Y/n]"
(Y default) but the check uses if [[ "$response" =~ ^[Yy] ]]; then which ignores
empty input; update the conditional around the read prompt (the read -p ...
response and the if test) to treat an empty response as yes by checking for
either empty response or a Y/y match (e.g., test -z "$response" or "$response"
=~ ^[Yy]) so pressing Enter selects the default "Y" behavior.

eval "$install_cmd" || handle_error "Failed to install $tool."
else
echo "$tool is required for this script. Exiting."
exit 1
fi
fi
}

# Default parameters
repo_url="https://github.com/Notifiarr/notifiarr.git"
repo_dir="/opt/notifiarr-repo"
bin_path="/usr/bin/notifiarr"
branch="master"
apt_reinstall=false

# Parse command line options
while [[ $# -gt 0 ]]; do
case "$1" in
-h | --help)
display_help
;;
--repo-url)
repo_url="$2"
shift
;;
--repo-dir)
repo_dir="$2"
shift
;;
--bin-path)
bin_path="$2"
shift
;;
--branch)
branch="$2"
shift
;;
--reinstall-apt)
apt_reinstall=true
;;
*)
echo "Invalid option: $1. Use -h for help."
exit 1
;;
esac
shift
done

# Ensure required tools are installed
ensure_tool_installed "make" "sudo apt update && sudo apt install -y make"

# Reinstallation condition handling
reinstall_notifiarr() {
sudo apt update && sudo apt install --reinstall notifiarr || handle_error "Failed to reinstall Notifiarr using apt."
}

[[ $apt_reinstall == true ]] && reinstall_notifiarr
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Quote variable in conditional for consistency.

While [[ ]] handles unquoted variables safely, quoting is still best practice.

♻️ Proposed fix
-[[ $apt_reinstall == true ]] && reinstall_notifiarr
+[[ "$apt_reinstall" == true ]] && reinstall_notifiarr
📝 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.

Suggested change
[[ $apt_reinstall == true ]] && reinstall_notifiarr
[[ "$apt_reinstall" == true ]] && reinstall_notifiarr
🤖 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 `@notifiarr-branch-builder.sh` at line 88, The conditional uses an unquoted
variable; update the check that references apt_reinstall (which triggers
reinstall_notifiarr) to quote the variable and the literal for consistency and
safety—i.e., change the condition that tests apt_reinstall to compare the quoted
variable to the quoted string "true" and keep the call to reinstall_notifiarr
unchanged.


# Repository management
if [[ ! -d "$repo_dir" ]]; then
git clone "$repo_url" "$repo_dir" || handle_error "Failed to clone repository."
else
git -C "$repo_dir" fetch --all --prune || handle_error "Failed to fetch updates from remote."
fi

# Branch handling and updating
current_branch=$(git -C "$repo_dir" rev-parse --abbrev-ref HEAD)
read -p "Do you want to use the current branch ($current_branch)? [Y/n] " choice
if [[ "$choice" =~ ^[Nn] ]]; then
branches=$(git -C "$repo_dir" branch -r | sed 's/origin\///;s/* //')
echo "Available branches:"
echo "$branches"
while true; do
read -p "Enter the branch name you want to use: " branch
if [[ $branches =~ $branch ]]; then
git -C "$repo_dir" checkout "$branch" || handle_error "Failed to checkout branch $branch."
break
else
echo "Invalid choice. Please select a valid branch."
fi
done
fi
Comment on lines +97 to +113
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix branch handling logic and --branch parameter usage.

Two issues with the branch selection:

  1. The --branch CLI parameter is accepted but completely ignored. Lines 98-113 always prompt interactively, overwriting any value set via --branch. This breaks expected CLI behavior.

  2. Line 106 uses substring matching which could select the wrong branch. For example, if branches include both "master" and "master-dev", entering "master" would match either.

🔧 Proposed fix to honor --branch parameter and improve validation
 # Branch handling and updating
 current_branch=$(git -C "$repo_dir" rev-parse --abbrev-ref HEAD)
-read -p "Do you want to use the current branch ($current_branch)? [Y/n] " choice
-if [[ "$choice" =~ ^[Nn] ]]; then
+
+# Honor --branch parameter if provided and different from current
+if [[ "$branch" != "master" && "$branch" != "$current_branch" ]]; then
+  # --branch was explicitly set via CLI
+  git -C "$repo_dir" checkout "$branch" || handle_error "Failed to checkout branch $branch."
+else
+  # Interactive branch selection
+  read -p "Do you want to use the current branch ($current_branch)? [Y/n] " choice
+  if [[ "$choice" =~ ^[Nn] ]]; then
   branches=$(git -C "$repo_dir" branch -r | sed 's/origin\///;s/* //')
   echo "Available branches:"
   echo "$branches"
   while true; do
     read -p "Enter the branch name you want to use: " branch
-    if [[ $branches =~ $branch ]]; then
+    # Use grep for exact line match instead of substring
+    if echo "$branches" | grep -qxF "$branch"; then
       git -C "$repo_dir" checkout "$branch" || handle_error "Failed to checkout branch $branch."
       break
     else
       echo "Invalid choice. Please select a valid branch."
     fi
   done
+  fi
 fi
🤖 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 `@notifiarr-branch-builder.sh` around lines 97 - 113, The branch prompt
currently always runs and ignores any provided --branch option, and validation
uses substring matching which can pick partial names; update the logic so that
if a CLI variable (e.g., BRANCH or a parsed --branch) is set use that value and
skip the interactive prompt, otherwise prompt as before; replace the loose
substring check against branches with an exact-match check (e.g., compare
against the list elements or use anchored/line-exact matching) before calling
git -C "$repo_dir" checkout "$branch" (and still call handle_error on failure).
Ensure you reference the existing variables current_branch, choice, branches and
the git checkout invocation when implementing these changes.


git -C "$repo_dir" pull || handle_error "Failed to pull latest changes."
make --directory="$repo_dir" || handle_error "Failed to compile."

# Service management
echo "Stopping notifiarr..."
sudo systemctl stop notifiarr
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider adding error handling for service stop.

While systemctl stop typically succeeds even if the service is already stopped, adding error handling would be more consistent with the rest of the script's approach.

♻️ Proposed fix
 echo "Stopping notifiarr..."
-sudo systemctl stop notifiarr
+sudo systemctl stop notifiarr || echo "Warning: Failed to stop notifiarr service" >&2
📝 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.

Suggested change
sudo systemctl stop notifiarr
echo "Stopping notifiarr..."
sudo systemctl stop notifiarr || echo "Warning: Failed to stop notifiarr service" >&2
🤖 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 `@notifiarr-branch-builder.sh` at line 120, The call to "sudo systemctl stop
notifiarr" lacks error handling; update notifiarr-branch-builder.sh to check the
exit status of that command (or call "systemctl is-active notifiarr" first) and
handle failures consistently with the rest of the script: if stop returns
non-zero and the service remains active, log an error message and exit non‑zero,
otherwise log success or that the service was already stopped; reference the
"sudo systemctl stop notifiarr" invocation when adding the conditional check and
logging.


if [[ -f "$bin_path" ]]; then
sudo mv "$bin_path" "$repo_dir".old && echo "Old binary moved to $repo_dir.old"
fi

sudo mv "$repo_dir/notifiarr" "$bin_path" && echo "New binary moved to $bin_path"
sudo chown root:root "$bin_path"

echo "Starting Notifiarr..."
sudo systemctl start notifiarr

if sudo systemctl is-active --quiet notifiarr; then
echo "Notifiarr service started and is currently running"
else
handle_error "Failed to start Notifiarr service"
fi

exit 0
47 changes: 47 additions & 0 deletions qbm-qbit.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/bin/bash

# Check if lockfile command exists
if ! command -v lockfile &>/dev/null; then
echo "Error: lockfile command not found. Please install the procmail package." >&2
exit 1
fi

# Load environment variables from .env file if it exists
if [ -f ".env" ]; then
source ".env"
Comment on lines +10 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use script directory for .env path, not current working directory.

The relative path .env depends on the current working directory, not the script's location. If the script is invoked from a different directory (e.g., via cron with bash /path/to/qbm-qbit.sh), it will fail to find the .env file or load an unintended one from the working directory.

🔧 Proposed fix to make .env path relative to script directory
+# Get the script's directory
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+
 # Load environment variables from .env file if it exists
-if [ -f ".env" ]; then
-  source ".env"
+if [ -f "$SCRIPT_DIR/.env" ]; then
+  source "$SCRIPT_DIR/.env"
 fi
🤖 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 `@qbm-qbit.sh` around lines 10 - 11, The script currently sources ".env" from
the current working directory which can be wrong when the script is run from
elsewhere; change the .env lookup to be relative to the script directory by
resolving the script path (e.g., using BASH_SOURCE or $0 and dirname) and then
source the .env file from that directory instead of plain ".env" so functions
and variables load reliably regardless of CWD.

fi

# Use environment variables with descriptive default values
QBQBM_LOCK=${QBIT_MANAGE_LOCK_FILE_PATH:-/var/lock/qbm-qbit.lock}
QBQBM_PATH_QBM=${QBIT_MANAGE_PATH:-/opt/qbit-manage}
QBQBM_VENV_PATH=${QBIT_MANAGE_VENV_PATH:-/opt/qbit-manage/.venv}
QBQBM_CONFIG_PATH=${QBIT_MANAGE_CONFIG_PATH:-/opt/qbit-manage/config.yml}
QBQBM_QBIT_OPTIONS=${QBIT_MANAGE_OPTIONS:-"-cs -re -cu -tu -ru -sl -r"}
QBQBM_SLEEP_TIME=600

# Function to remove the lock file
remove_lock() {
rm -f "$QBQBM_LOCK"
}

# Function to handle detection of another running instance
another_instance() {
echo "There is another instance running, exiting."
exit 1
}

echo "Acquiring Lock"
# Acquire a lock to prevent concurrent execution, with a timeout and lease time
lockfile -r 0 -l "$QBQBM_SLEEP_TIME" "$QBQBM_LOCK" || another_instance

# Ensure the lock is removed when the script exits
trap remove_lock EXIT

echo "sleeping for $QBQBM_SLEEP_TIME"
# Pause the script to wait for any pending operations (i.e. Starr Imports)

sleep $QBQBM_SLEEP_TIME
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Quote variable expansion for consistency.

While numeric values don't require quoting for word-splitting safety, quoting all variable expansions is a shell scripting best practice.

♻️ Proposed fix
-sleep $QBQBM_SLEEP_TIME
+sleep "$QBQBM_SLEEP_TIME"
📝 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.

Suggested change
sleep $QBQBM_SLEEP_TIME
sleep "$QBQBM_SLEEP_TIME"
🤖 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 `@qbm-qbit.sh` at line 43, The sleep invocation uses unquoted variable
expansion (sleep $QBQBM_SLEEP_TIME); update it to quote the variable (sleep
"$QBQBM_SLEEP_TIME") to follow shell best-practices and avoid word-splitting or
unexpected behavior when QBQBM_SLEEP_TIME contains spaces or is empty.


# Execute qbit_manage with configurable options
echo "Executing Command"
"$QBQBM_VENV_PATH"/bin/python "$QBQBM_PATH_QBM"/qbit_manage.py "$QBQBM_QBIT_OPTIONS" --config-file "$QBQBM_CONFIG_PATH"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Consider using an array for options to handle edge cases.

The unquoted $QBQBM_QBIT_OPTIONS relies on word splitting to pass multiple arguments, which works for the current space-separated flags but breaks if any option value contains spaces.

♻️ More robust approach using arrays

Replace the variable definition (line 19) and usage (line 47):

-QBQBM_QBIT_OPTIONS=${QBIT_MANAGE_OPTIONS:-"-cs -re -cu -tu -ru -sl -r"}
+# shellcheck disable=SC2206
+QBQBM_QBIT_OPTIONS=(${QBIT_MANAGE_OPTIONS:-"-cs -re -cu -tu -ru -sl -r"})
-"$QBQBM_VENV_PATH"/bin/python "$QBQBM_PATH_QBM"/qbit_manage.py "$QBQBM_QBIT_OPTIONS" --config-file "$QBQBM_CONFIG_PATH"
+"$QBQBM_VENV_PATH"/bin/python "$QBQBM_PATH_QBM"/qbit_manage.py "${QBQBM_QBIT_OPTIONS[@]}" --config-file "$QBQBM_CONFIG_PATH"

Note: The shellcheck disable is needed because we intentionally want word splitting when initializing the array from the environment variable.

🤖 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 `@qbm-qbit.sh` at line 47, The QBQBM_QBIT_OPTIONS variable is unquoted and
relies on word-splitting, so change its declaration to build an array (e.g.,
qbit_opts=()) and initialize that array from the env value while intentionally
allowing splitting (use a shellcheck disable comment), then update the
invocation to use the array expansion ("${qbit_opts[@]}") instead of unquoted
$QBQBM_QBIT_OPTIONS; update references in qbm-qbit.sh (the variable
QBQBM_QBIT_OPTIONS and the command line that calls qbit_manage.py) to use the
new array name so option values containing spaces are passed correctly.

Loading