Skip to content

added mf tool#33380

Merged
igorkorsukov merged 1 commit into
musescore:mainfrom
igorkorsukov:w/repo/mf_helpers
May 13, 2026
Merged

added mf tool#33380
igorkorsukov merged 1 commit into
musescore:mainfrom
igorkorsukov:w/repo/mf_helpers

Conversation

@igorkorsukov
Copy link
Copy Markdown
Member

@igorkorsukov igorkorsukov commented May 13, 2026

Added a simple script for switching MF to a fork

The fork URL can be specified using the -f option, or it is more convenient to add the mf_fork_url.local file with the URL.

mf_fork_url.local

git@github.com:igorkorsukov/muse_framework.git
image
  1. Switch to fork
bash ./mftool.sh -t use-fork
  1. Switch to origin
bash ./mftool.sh -t use-origin
  1. Sync origin
bash ./mftool.sh -t sync-origin

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This pull request adds mftool.sh, a Bash utility that parses --fork and --task, loads MF_FORK_URL from mf_fork_url.local if needed, validates inputs, prints resolved MF_FORK_URL and TASK, and dispatches tasks: use-fork (point muse submodule to a fork and update remotes), use-origin (reset muse to MF_ORIGIN_URL and update submodules/remotes), and sync-origin (use-origin then update ./muse with --remote). The muse submodule reference is updated to a new commit.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description explains the functionality and usage but does not follow the required template structure, missing sections like issue resolution, CLA confirmation, and verification checkboxes. Fill in the required template sections including the 'Resolves' issue reference and all verification checkboxes to comply with repository standards.
Title check ❓ Inconclusive The title 'added mf tool' is vague and generic, using non-descriptive terminology that doesn't convey meaningful information about the changeset's purpose or functionality. Make the title more specific and descriptive, such as 'Add mftool.sh script for switching between fork and origin repositories' to clearly communicate the primary change.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with 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.

Inline comments:
In `@mftool.sh`:
- Around line 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.
- Line 27: Quote the MF_FORK_URL variable to prevent globbing/word-splitting:
replace unquoted uses of $MF_FORK_URL in the git submodule set-url commands with
"$MF_FORK_URL" (the occurrences where the script runs git submodule set-url muse
$MF_FORK_URL and the other two similar set-url invocations) so the URL is
treated as a single argument even if it contains spaces or special characters.
- Around line 10-11: The -f|--fork and -t|--task case arms assign
MF_FORK_URL="$2" and TASK="$2" without validating that "$2" exists or isn't
another flag; update those branches in the option-parsing case (flags -f|--fork
and -t|--task) to check that "$2" is non-empty and does not start with '-'
before assignment, and if the check fails print a clear usage/error message and
exit nonzero; ensure you still shift the correct number of arguments only after
successful validation so MF_FORK_URL and TASK are never set to another flag or
empty.
- 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.
- Around line 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.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fb658605-ed25-4ce1-bf17-fe444d392746

📥 Commits

Reviewing files that changed from the base of the PR and between 4bf81c5 and e883ec6.

📒 Files selected for processing (2)
  • mftool.sh
  • muse

Comment thread mftool.sh
Comment on lines +1 to +2
#!/usr/bin/env bash

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.

Comment thread mftool.sh
Comment thread mftool.sh
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.

Comment thread mftool.sh
Comment on lines +26 to +28
"use-fork")
git submodule set-url muse $MF_FORK_URL
;;
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.

Comment thread mftool.sh Outdated

case "$TASK" in
"use-fork")
git submodule set-url muse $MF_FORK_URL
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

Quote the variable to prevent globbing and word splitting.

The $MF_FORK_URL variable should be quoted to prevent issues if the URL contains spaces or special characters. This applies to lines 27, 30, and 34.

🔧 Proposed fix to quote variables
-        git submodule set-url muse $MF_FORK_URL
+        git submodule set-url muse "$MF_FORK_URL"

Apply the same fix to lines 30 and 34:

-        git submodule set-url muse $MF_ORIGIN_URL
+        git submodule set-url muse "$MF_ORIGIN_URL"
📝 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
git submodule set-url muse $MF_FORK_URL
git submodule set-url muse "$MF_FORK_URL"
🧰 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` at line 27, Quote the MF_FORK_URL variable to prevent
globbing/word-splitting: replace unquoted uses of $MF_FORK_URL in the git
submodule set-url commands with "$MF_FORK_URL" (the occurrences where the script
runs git submodule set-url muse $MF_FORK_URL and the other two similar set-url
invocations) so the URL is treated as a single argument even if it contains
spaces or special characters.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (5)
mftool.sh (5)

17-17: ⚠️ 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
🤖 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, The current assignment to MF_FORK_URL uses cat
directly and doesn't handle missing file or trailing whitespace; change the
logic around MF_FORK_URL so that if MF_FORK_URL is empty you first verify
mf_fork_url.local exists and is readable, read its contents into MF_FORK_URL,
trim leading/trailing whitespace (e.g., with shell parameter expansion or a
trimming command), and if the file is missing or results in an empty value emit
a clear error via echo/processLogger and exit non‑zero; update the code
reference where MF_FORK_URL is set in mftool.sh to perform these checks and
trimming before continuing.

27-27: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Quote URL variables to prevent globbing and word splitting.

The $MF_FORK_URL and $MF_ORIGIN_URL variables should be quoted to prevent issues if the URL contains spaces or special characters. This applies to lines 27, 29, 31 (in use_fork), and lines 37, 40 (in use_origin).

🔧 Proposed fix to quote variables
 function use_fork() {
-    git submodule set-url muse $MF_FORK_URL
+    git submodule set-url muse "$MF_FORK_URL"
     cd muse
-    git remote set-url origin $MF_FORK_URL
+    git remote set-url origin "$MF_FORK_URL"
     git remote remove upstream
-    git remote add upstream $MF_ORIGIN_URL
+    git remote add upstream "$MF_ORIGIN_URL"
     git remote -v
     cd ..
 }
 
 function use_origin() {
-    git submodule set-url muse $MF_ORIGIN_URL
+    git submodule set-url muse "$MF_ORIGIN_URL"
     git submodule update --init
     cd muse
-    git remote set-url origin $MF_ORIGIN_URL
+    git remote set-url origin "$MF_ORIGIN_URL"
     git remote remove upstream
     git remote -v
     cd ..
 }

Also applies to: 29-29, 31-31, 37-37, 40-40

🤖 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 27, Quote the URL variables in the shell script to prevent
globbing/word-splitting: in the use_fork function update git submodule set-url
muse $MF_FORK_URL, git remote set-url --push origin $MF_FORK_URL, and git remote
set-url origin $MF_FORK_URL to use "$MF_FORK_URL"; in the use_origin function
update git submodule set-url muse $MF_ORIGIN_URL and git remote set-url origin
$MF_ORIGIN_URL to use "$MF_ORIGIN_URL". Ensure all occurrences of MF_FORK_URL
and MF_ORIGIN_URL are quoted consistently.

47-49: ⚠️ Potential issue | 🟠 Major | ⚡ 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.

📝 Proposed fix to add submodule update
     "use-fork")
-        use_fork
+        use_fork
+        git submodule update --init
         ;;

Alternatively, add the update inside the use_fork() function itself for consistency with use_origin().

🤖 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 47 - 49, The use-fork branch switch currently only
sets the submodule URL but doesn't update the working copy; make it consistent
with use_origin/sync_origin by also updating the submodule after changing the
URL. Either add a submodule update step where "use-fork" is handled (after
calling use_fork) or modify the use_fork() function to run the equivalent of a
git submodule update --init --recursive/--remote so the submodule is checked out
to the fork immediately; reference use_fork(), use_origin(), and sync_origin()
when locating where to add the update.

1-2: ⚠️ Potential issue | 🔴 Critical | ⚡ 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)
🤖 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, Add immediate-failure error handling to the
script by inserting a shell option line after the shebang (e.g., enable "set -e"
or the stricter "set -euo pipefail") so that any failing git command or other
error causes the script to exit instead of continuing silently; update the top
of the script (near the existing "#!/usr/bin/env bash" shebang) to include this
option and ensure callers are aware of the stricter behavior.

10-11: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Argument validation is still missing despite being marked as addressed.

The past review comment for these lines was marked as "✅ Addressed in commit 8e57709", but the current code still assigns $2 without validating that it exists or isn't another flag. If -f or -t is provided without a value (e.g., ./mftool.sh -f -t use-fork), MF_FORK_URL would be incorrectly set to -t.

🛡️ Proposed fix to validate argument values
-        -f|--fork) MF_FORK_URL="$2"; shift ;;
-        -t|--task) TASK="$2"; shift ;;
+        -f|--fork) 
+            if [[ -z "${2:-}" || "$2" == -* ]]; then 
+                echo "error: --fork requires a value"; exit 1
+            fi
+            MF_FORK_URL="$2"; shift ;;
+        -t|--task) 
+            if [[ -z "${2:-}" || "$2" == -* ]]; then 
+                echo "error: --task requires a value"; exit 1
+            fi
+            TASK="$2"; shift ;;
🤖 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 10 - 11, The option parsing for -f|--fork and
-t|--task currently assigns MF_FORK_URL="$2" and TASK="$2" without validating
the next token; update the handlers in the option parsing block so before
assigning MF_FORK_URL or TASK you check that "$2" is non-empty and does not
start with '-' (indicating another flag), otherwise emit a clear error and exit
(or show usage). Modify the branches handling -f|--fork and -t|--task to
validate the argument token and only shift/assign when valid, referencing the
MF_FORK_URL and TASK assignments and their option patterns in the parser.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@mftool.sh`:
- 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.
- 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.

---

Duplicate comments:
In `@mftool.sh`:
- Line 17: The current assignment to MF_FORK_URL uses cat directly and doesn't
handle missing file or trailing whitespace; change the logic around MF_FORK_URL
so that if MF_FORK_URL is empty you first verify mf_fork_url.local exists and is
readable, read its contents into MF_FORK_URL, trim leading/trailing whitespace
(e.g., with shell parameter expansion or a trimming command), and if the file is
missing or results in an empty value emit a clear error via echo/processLogger
and exit non‑zero; update the code reference where MF_FORK_URL is set in
mftool.sh to perform these checks and trimming before continuing.
- Line 27: Quote the URL variables in the shell script to prevent
globbing/word-splitting: in the use_fork function update git submodule set-url
muse $MF_FORK_URL, git remote set-url --push origin $MF_FORK_URL, and git remote
set-url origin $MF_FORK_URL to use "$MF_FORK_URL"; in the use_origin function
update git submodule set-url muse $MF_ORIGIN_URL and git remote set-url origin
$MF_ORIGIN_URL to use "$MF_ORIGIN_URL". Ensure all occurrences of MF_FORK_URL
and MF_ORIGIN_URL are quoted consistently.
- Around line 47-49: The use-fork branch switch currently only sets the
submodule URL but doesn't update the working copy; make it consistent with
use_origin/sync_origin by also updating the submodule after changing the URL.
Either add a submodule update step where "use-fork" is handled (after calling
use_fork) or modify the use_fork() function to run the equivalent of a git
submodule update --init --recursive/--remote so the submodule is checked out to
the fork immediately; reference use_fork(), use_origin(), and sync_origin() when
locating where to add the update.
- Around line 1-2: Add immediate-failure error handling to the script by
inserting a shell option line after the shebang (e.g., enable "set -e" or the
stricter "set -euo pipefail") so that any failing git command or other error
causes the script to exit instead of continuing silently; update the top of the
script (near the existing "#!/usr/bin/env bash" shebang) to include this option
and ensure callers are aware of the stricter behavior.
- Around line 10-11: The option parsing for -f|--fork and -t|--task currently
assigns MF_FORK_URL="$2" and TASK="$2" without validating the next token; update
the handlers in the option parsing block so before assigning MF_FORK_URL or TASK
you check that "$2" is non-empty and does not start with '-' (indicating another
flag), otherwise emit a clear error and exit (or show usage). Modify the
branches handling -f|--fork and -t|--task to validate the argument token and
only shift/assign when valid, referencing the MF_FORK_URL and TASK assignments
and their option patterns in the parser.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b33c86fb-dace-44ca-8c9f-bb778df0e989

📥 Commits

Reviewing files that changed from the base of the PR and between e883ec6 and 8e57709.

📒 Files selected for processing (2)
  • mftool.sh
  • muse
🚧 Files skipped from review as they are similar to previous changes (1)
  • muse

Comment thread mftool.sh

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.

Comment thread mftool.sh
git submodule set-url muse $MF_FORK_URL
cd muse
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.

@igorkorsukov igorkorsukov merged commit 534b474 into musescore:main May 13, 2026
9 of 14 checks passed
@cbjeukendrup
Copy link
Copy Markdown
Collaborator

Personally I have a simpler workflow: just git checkout <my-commit> in the submodule, and then commit the submodule change to the super repo. That way, I don't need to mess with setting the submodule URL or remote. And even if that commit is pushed only to a fork, GitHub will be able to find it alright, as seen at #33301. That might make a tool like this unnecessary.

@igorkorsukov
Copy link
Copy Markdown
Member Author

I don't have any experience, but I was told GitHub can switch a submodule to a fork using the SHA commit from the fork, without having to change the URL.
But I encountered this once when GitHub couldn't find it... I don't remember the details...

On the one hand, I understand that everyone has their own workflow. Some use only the SHA commit, some don't change the origin (we'd better change it internally team to avoid accidentally pushing it into upstream; there's already been a precedent), and some use GUI clients...

On the other hand, I thought it would be good to provide a typical tool for a typical workflow (we just need to decide what "typical" means). So that developers could choose whether to simply do this and that, or study, understand, and consciously implement what's most convenient for this developer.

Besides switching the fork, I think we can go further, automate the routine, create some kind of typical process.
For example, this is what my development looks like:

  • Switch to main
  • Update with upstream
  • Create a branch
  • Switch MF to fork
  • Create a branch in MF (preferably with the same name)

And so I think it’s possible to make a script for automation (and use it as a typical one)

./wf --make-feature some/name
./wf --make-feature-mf some/name

Something like that...

On the other hand, if something breaks, the developer won't know what to do if they didn't understand what was going on...

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.

3 participants