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
58 changes: 58 additions & 0 deletions mftool.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
#!/usr/bin/env bash

Comment on lines +1 to +2
Copy link
Copy Markdown
Contributor

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

Add error handling for Git command failures.

The script executes multiple git commands without checking their exit status. If any git command fails (e.g., network issues, invalid URL, permissions), the script continues silently, which can cause confusion.

🛡️ Proposed fix to add error handling

Add set -e after the shebang to exit immediately if any command fails:

 #!/usr/bin/env bash 
+set -e
 

Alternatively, for stricter error handling, use:

set -euo pipefail

This will exit on:

  • Any command failure (-e)
  • Undefined variable usage (-u)
  • Pipe failures (-o pipefail)
📝 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
#!/usr/bin/env bash
#!/usr/bin/env bash
set -e
🤖 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 `@mftool.sh` around lines 1 - 2, The script runs git commands without checking
failures; add robust shell error handling by inserting a strict set command
immediately after the shebang in mftool.sh (e.g., use set -e or, better, set
-euo pipefail) so the script exits on any failing command, undefined variable,
or pipe failure; update any places relying on unchecked globals or allow
exceptions where appropriate.


MF_ORIGIN_URL=https://github.com/musescore/muse_framework.git
MF_FORK_URL=""
TASK=""

while [[ "$#" -gt 0 ]]; do
case $1 in
-f|--fork) MF_FORK_URL="$2"; shift ;;
-t|--task) TASK="$2"; shift ;;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
*) echo "Unknown parameter passed: $1"; exit 1 ;;
esac
shift
done

if [ -z "$MF_FORK_URL" ]; then MF_FORK_URL=$(cat mf_fork_url.local); fi
Copy link
Copy Markdown
Contributor

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

Add error handling and whitespace trimming for file reading.

The cat command will output a confusing error message if mf_fork_url.local doesn't exist, and any whitespace in the file content is not trimmed, which could cause git command failures.

🛡️ Proposed fix to add file validation and whitespace trimming
-if [ -z "$MF_FORK_URL" ]; then MF_FORK_URL=$(cat mf_fork_url.local); fi
+if [ -z "$MF_FORK_URL" ]; then
+    if [ -f "mf_fork_url.local" ]; then
+        MF_FORK_URL=$(cat mf_fork_url.local | tr -d '[:space:]')
+    fi
+fi
📝 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
if [ -z "$MF_FORK_URL" ]; then MF_FORK_URL=$(cat mf_fork_url.local); fi
if [ -z "$MF_FORK_URL" ]; then
if [ -f "mf_fork_url.local" ]; then
MF_FORK_URL=$(cat mf_fork_url.local | tr -d '[:space:]')
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 `@mftool.sh` at line 17, Replace the fragile use of cat in the MF_FORK_URL
fallback with a safe check: test that the file mf_fork_url.local exists and is
readable before assigning to MF_FORK_URL, and if it’s missing or unreadable emit
a clear error and exit; when reading the file, trim surrounding
whitespace/newlines (e.g., via shell parameter expansion or a trimming command)
so MF_FORK_URL gets a clean value. Update the conditional that sets MF_FORK_URL
(the block referencing MF_FORK_URL and mf_fork_url.local) to perform these
checks and trimming and to fail fast with a descriptive message when the file
isn’t present.

if [ -z "$MF_FORK_URL" ]; then echo "error: not set MF_FORK_URL"; exit 1; fi

if [ -z "$TASK" ]; then echo "error: not set TASK"; exit 1; fi

echo "MF_FORK_URL: $MF_FORK_URL"
echo "TASK: $TASK"


function use_fork() {
git submodule set-url muse $MF_FORK_URL
cd muse
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add error handling for directory changes.

Both cd muse commands (lines 28 and 39) lack error handling. If the muse directory doesn't exist or isn't accessible, subsequent git commands will execute in the wrong directory, potentially causing unintended modifications.

🛡️ Proposed fix to add cd error handling
 function use_fork() {
     git submodule set-url muse "$MF_FORK_URL"
-    cd muse
+    cd muse || { echo "error: failed to cd into muse directory"; exit 1; }
     git remote set-url origin "$MF_FORK_URL"
     git remote remove upstream
     git remote add upstream "$MF_ORIGIN_URL"
     git remote -v
     cd ..
 }
 
 function use_origin() {
     git submodule set-url muse "$MF_ORIGIN_URL"
     git submodule update --init
-    cd muse
+    cd muse || { echo "error: failed to cd into muse directory"; exit 1; }
     git remote set-url origin "$MF_ORIGIN_URL"
     git remote remove upstream
     git remote -v
     cd ..
 }

Also applies to: 39-39

🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 28-28: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)

🤖 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 `@mftool.sh` at line 28, The script runs two bare "cd muse" commands without
handling failures, so if the directory is missing subsequent git operations run
in the wrong location; update both occurrences of "cd muse" to check the
command's result and abort the script (or return a non-zero status) when the
chdir fails, ensuring the script does not continue executing git commands
outside the intended repository.

git remote set-url origin $MF_FORK_URL
git remote remove upstream
Copy link
Copy Markdown
Contributor

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

Handle missing remote gracefully.

The git remote remove upstream commands (lines 30 and 41) will fail if the upstream remote doesn't exist (e.g., on first run). This causes an error message and, if error handling is added per earlier suggestions, will abort the script.

🛡️ Proposed fix to handle missing remote

Option 1: Check if remote exists before removing:

 function use_fork() {
     git submodule set-url muse "$MF_FORK_URL"
     cd muse || exit 1
     git remote set-url origin "$MF_FORK_URL"
-    git remote remove upstream
+    git remote get-url upstream &>/dev/null && git remote remove upstream
     git remote add upstream "$MF_ORIGIN_URL"
     git remote -v
     cd ..
 }
 
 function use_origin() {
     git submodule set-url muse "$MF_ORIGIN_URL"
     git submodule update --init
     cd muse || exit 1
     git remote set-url origin "$MF_ORIGIN_URL"
-    git remote remove upstream
+    git remote get-url upstream &>/dev/null && git remote remove upstream
     git remote -v
     cd ..
 }

Option 2: Suppress error if remote doesn't exist:

-    git remote remove upstream
+    git remote remove upstream 2>/dev/null || true

Also applies to: 41-41

🤖 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 `@mftool.sh` at line 30, The script currently runs the literal command "git
remote remove upstream" (found twice) which fails if the upstream remote doesn't
exist; modify both occurrences to either first check for the remote (e.g., use
"git remote get-url upstream" or "git remote | grep -q upstream") and only run
"git remote remove upstream" when present, or suppress the non-fatal error by
appending a no-op on failure (e.g., redirect stderr or use "|| true") so the
script continues gracefully when upstream is missing.

git remote add upstream $MF_ORIGIN_URL
git remote -v
cd ..
}

function use_origin() {
git submodule set-url muse $MF_ORIGIN_URL
git submodule update --init
cd muse
git remote set-url origin $MF_ORIGIN_URL
git remote remove upstream
git remote -v
cd ..
}

case "$TASK" in
"use-fork")
use_fork
;;
Comment on lines +47 to +49
Copy link
Copy Markdown
Contributor

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

Consider adding submodule update for consistency.

The use-fork task only sets the URL but doesn't update the submodule, unlike use-origin and sync-origin which both set the URL and update. This inconsistency might confuse users who expect the fork to be checked out after running this command.

Is this intentional, or should use-fork also call git submodule update --init?

📝 Proposed fix if update should be included
     "use-fork")
         git submodule set-url muse "$MF_FORK_URL"
+        git submodule update --init
         ;;
📝 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
"use-fork")
git submodule set-url muse $MF_FORK_URL
;;
"use-fork")
git submodule set-url muse "$MF_FORK_URL"
git submodule update --init
;;
🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 27-27: Double quote to prevent globbing and word splitting.

(SC2086)

🤖 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 `@mftool.sh` around lines 26 - 28, The "use-fork" case in mftool.sh only
changes the submodule URL (the case label "use-fork") but doesn't update the
submodule, creating inconsistency with "use-origin" and "sync-origin"; modify
the "use-fork" branch to run a submodule update (e.g., run git submodule update
--init for the "muse" submodule) after setting the URL so the forked
commit/branch is checked out and initialized just like the other commands.

"use-origin")
use_origin
;;
"sync-origin")
use_origin
git submodule update --init --remote ./muse
;;
*) echo "Unknown task: $TASK"; exit 1 ;;
esac
2 changes: 1 addition & 1 deletion muse
Submodule muse updated 289 files
Loading