Fix/issues 355 357 361 362#453
Conversation
📝 WalkthroughWalkthroughAdds ChangesShared Soroban Error Taxonomy
Fix-submission automation scripts
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@Chinaza007 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@submit_fixes.bat`:
- Around line 68-98: The copy/stage flow in submit_fixes.bat assumes every
target file exists, so update each copy block to verify the copy command
succeeded before printing success and before staging that file. Use the existing
acbu_escrow, acbu_minting, and shared errors blocks to track which files were
actually copied, then gate the git add step so it only stages files that were
successfully copied instead of always adding all three paths. If a source is
missing or copy fails, fail that file’s step explicitly rather than proceeding
with stale or nonexistent targets.
- Around line 54-63: The branch-handling logic in submit_fixes.bat reuses an
existing local branch instead of recreating it from the freshly fetched base,
which can leave stale history on %BRANCH%. Update the branch setup flow around
the git show-ref / git checkout block so that an existing local %BRANCH% is
replaced or reset to %REMOTE%/%BASE_BRANCH% before continuing, ensuring the
script always starts from the latest remote base rather than an old local fix
branch.
- Around line 1-131: Normalize submit_fixes.bat to CRLF so cmd.exe can parse
labels and control flow correctly, and update .gitattributes to enforce *.bat
text eol=crlf for all batch files. Make the change in the batch script itself
and in the repo’s attributes config so the formatting stays consistent going
forward.
In `@submit_fixes.sh`:
- Around line 48-57: The staging step contradicts the missing-file skip behavior
in copy_if_exists, since it still adds all expected paths even when a source was
skipped. Update the git add logic in submit_fixes.sh so it only stages files
that were actually copied or already exist, and avoid failing when a destination
is absent after copy_if_exists warned and continued. Use the copy_if_exists
helper and the later staging block together to gate git add on file presence.
- Around line 37-43: The branch handling in the script currently reuses an
existing local branch via git checkout, which can leave stale commits behind.
Update the logic around the branch creation flow so that when "$BRANCH" already
exists locally it either aborts with a clear message or explicitly resets it to
"$REMOTE/$BASE_BRANCH" before proceeding, rather than silently checking it out.
Use the existing info/warn checkout block and the "$BRANCH", "$REMOTE", and
"$BASE_BRANCH" symbols to locate the fix.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d83ea695-c9a0-47c4-bebb-f456098db0f7
📒 Files selected for processing (3)
shared/src/errors.rssubmit_fixes.batsubmit_fixes.sh
| @echo off | ||
| :: ============================================================================= | ||
| :: submit_fixes.bat — ACBU smart contract fixes (#355, #357, #361, #362) | ||
| :: Usage: Double-click or run from a command prompt. | ||
| :: Run this from the ROOT of your local acbu-smart-contract clone. | ||
| :: ============================================================================= | ||
|
|
||
| setlocal enabledelayedexpansion | ||
|
|
||
| :: ── Config ──────────────────────────────────────────────────────────────────── | ||
| set BRANCH=fix/issues-355-357-361-362 | ||
| set REMOTE=origin | ||
| set BASE_BRANCH=dev | ||
| :: Change BASE_BRANCH to "main" above if your repo uses main | ||
|
|
||
| :: ── Helpers ─────────────────────────────────────────────────────────────────── | ||
| :: FIXES_DIR is relative to this script; adjust if you placed acbu-fixes elsewhere | ||
| set FIXES_DIR=%~dp0acbu-fixes | ||
|
|
||
| :: ── Pre-flight ──────────────────────────────────────────────────────────────── | ||
| echo [INFO] Checking prerequisites... | ||
|
|
||
| where git >nul 2>&1 | ||
| if errorlevel 1 ( | ||
| echo [ERROR] git not found. Install Git for Windows: https://git-scm.com/ | ||
| pause & exit /b 1 | ||
| ) | ||
|
|
||
| if not exist "Cargo.toml" ( | ||
| echo [ERROR] Cargo.toml not found. | ||
| echo Run this script from the root of the acbu-smart-contract repo. | ||
| pause & exit /b 1 | ||
| ) | ||
|
|
||
| :: Check for uncommitted changes | ||
| git diff --quiet | ||
| if errorlevel 1 ( | ||
| echo [ERROR] Working tree has uncommitted changes. | ||
| echo Stash or commit them before running this script. | ||
| pause & exit /b 1 | ||
| ) | ||
| git diff --cached --quiet | ||
| if errorlevel 1 ( | ||
| echo [ERROR] Staged changes detected. | ||
| echo Commit or reset them before running this script. | ||
| pause & exit /b 1 | ||
| ) | ||
|
|
||
| :: ── Fetch & branch ──────────────────────────────────────────────────────────── | ||
| echo [INFO] Fetching latest '%BASE_BRANCH%' from %REMOTE%... | ||
| git fetch %REMOTE% %BASE_BRANCH% | ||
| if errorlevel 1 ( echo [ERROR] git fetch failed & pause & exit /b 1 ) | ||
|
|
||
| :: Check if branch already exists locally | ||
| git show-ref --quiet "refs/heads/%BRANCH%" | ||
| if errorlevel 1 ( | ||
| echo [INFO] Creating branch '%BRANCH%' from %REMOTE%/%BASE_BRANCH%... | ||
| git checkout -b %BRANCH% %REMOTE%/%BASE_BRANCH% | ||
| ) else ( | ||
| echo [WARN] Branch '%BRANCH%' already exists — checking it out | ||
| git checkout %BRANCH% | ||
| ) | ||
| if errorlevel 1 ( echo [ERROR] git checkout failed & pause & exit /b 1 ) | ||
|
|
||
| :: ── Copy fixed files ────────────────────────────────────────────────────────── | ||
| echo [INFO] Copying fixed source files... | ||
|
|
||
| :: acbu_escrow | ||
| if exist "%FIXES_DIR%\acbu_escrow\src\lib.rs" ( | ||
| if not exist "acbu_escrow\src" mkdir "acbu_escrow\src" | ||
| copy /Y "%FIXES_DIR%\acbu_escrow\src\lib.rs" "acbu_escrow\src\lib.rs" >nul | ||
| echo [INFO] Copied: acbu_escrow\src\lib.rs | ||
| ) else ( | ||
| echo [WARN] Not found, skipping: %FIXES_DIR%\acbu_escrow\src\lib.rs | ||
| ) | ||
|
|
||
| :: acbu_minting | ||
| if exist "%FIXES_DIR%\acbu_minting\src\lib.rs" ( | ||
| if not exist "acbu_minting\src" mkdir "acbu_minting\src" | ||
| copy /Y "%FIXES_DIR%\acbu_minting\src\lib.rs" "acbu_minting\src\lib.rs" >nul | ||
| echo [INFO] Copied: acbu_minting\src\lib.rs | ||
| ) else ( | ||
| echo [WARN] Not found, skipping: %FIXES_DIR%\acbu_minting\src\lib.rs | ||
| ) | ||
|
|
||
| :: shared errors | ||
| if exist "%FIXES_DIR%\shared\src\errors.rs" ( | ||
| if not exist "shared\src" mkdir "shared\src" | ||
| copy /Y "%FIXES_DIR%\shared\src\errors.rs" "shared\src\errors.rs" >nul | ||
| echo [INFO] Copied: shared\src\errors.rs | ||
| ) else ( | ||
| echo [WARN] Not found, skipping: %FIXES_DIR%\shared\src\errors.rs | ||
| ) | ||
|
|
||
| :: ── Stage ───────────────────────────────────────────────────────────────────── | ||
| echo [INFO] Staging changes... | ||
| git add acbu_escrow\src\lib.rs acbu_minting\src\lib.rs shared\src\errors.rs | ||
| if errorlevel 1 ( echo [ERROR] git add failed & pause & exit /b 1 ) | ||
|
|
||
| :: Check if there is anything to commit | ||
| git diff --cached --quiet | ||
| if not errorlevel 1 ( | ||
| echo [WARN] Nothing new to commit — files may already be up to date | ||
| goto :push | ||
| ) | ||
|
|
||
| :: ── Commit ──────────────────────────────────────────────────────────────────── | ||
| echo [INFO] Committing... | ||
| git commit -m "fix: address issues #355 #357 #361 #362" -m "#357 — add AlreadyInitialized guard to initialize() in escrow & minting. Prevents re-initialisation from silently overwriting admin, fee rate, and token addresses." -m "#361 — add #[allow(dead_code)] to emit-only event structs. Suppresses spurious lint warnings so real dead-code issues are not masked." -m "#355 — differentiate token_client transfer errors. Switch from transfer() (panics) to try_transfer() (Result). TokenXferFailed for token-side rejections, TransferFailed for internal disbursements." -m "#362 — deduplicate require_auth when user == admin. New require_auth_dedup() helper skips the second auth charge when both addresses are identical." | ||
| if errorlevel 1 ( echo [ERROR] git commit failed & pause & exit /b 1 ) | ||
|
|
||
| :push | ||
| :: ── Push ────────────────────────────────────────────────────────────────────── | ||
| echo [INFO] Pushing branch to %REMOTE%... | ||
| git push -u %REMOTE% %BRANCH% | ||
| if errorlevel 1 ( echo [ERROR] git push failed & pause & exit /b 1 ) | ||
|
|
||
| :: ── Done ────────────────────────────────────────────────────────────────────── | ||
| echo. | ||
| echo [OK] Branch pushed successfully. | ||
| echo. | ||
| echo Next steps: | ||
| echo 1. Open a PR on GitHub: | ||
| echo https://github.com/Pi-Defi-world/acbu-smart-contract/compare/%BASE_BRANCH%...%BRANCH% | ||
| echo. | ||
| echo 2. Or, if you have the GitHub CLI installed: | ||
| echo gh pr create --base %BASE_BRANCH% --head %BRANCH% ^ | ||
| echo --title "fix: address issues #355 #357 #361 #362" ^ | ||
| echo --body-file acbu-fixes\FIX_SUMMARY.md | ||
| echo. | ||
| pause |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pathlib import Path
data = Path("submit_fixes.bat").read_bytes()
print("CRLF count:", data.count(b"\r\n"))
print("LF count:", data.count(b"\n"))
print("LF-only newlines present:", b"\n" in data.replace(b"\r\n", b""))
PYRepository: Pi-Defi-world/acbu-smart-contract
Length of output: 230
🌐 Web query:
cmd.exe batch file LF-only line endings CRLF required behavior
💡 Result:
Windows Batch files (.bat and.cmd) officially require CRLF (Carriage Return + Line Feed, \r\n) line endings to function reliably [1][2][3]. While scripts using only LF (Line Feed, \n) may appear to work in simple scenarios, they are prone to erratic behavior and critical failures [1][2]. The primary technical risks of using LF-only line endings include: 1. Label Lookup Failures: cmd.exe parses batch files in 512-byte chunks [2][4]. When a file uses LF-only, the parser can miscalculate the position of labels (e.g., :myLabel), leading to the error: "The system cannot find the batch label specified" [5][2][4]. This often occurs when a label crosses or aligns near a 512-byte boundary [5][4]. 2. Unpredictable Execution: Because the command interpreter expects CRLF to properly identify line boundaries, LF-only files may cause the interpreter to consume multiple lines as a single command, leading to syntax errors or unexpected execution logic [1][2]. 3. Parsing Anomalies: Complex batch scripts, particularly those involving loops, complex conditional logic, or multi-byte characters (like UTF-8), are highly susceptible to breakage when LF endings are used [2][3]. For cross-platform development (e.g., using Git), it is strongly recommended to enforce CRLF line endings for batch files to prevent these issues [6][7]. This can be accomplished by creating or updating a.gitattributes file in the repository root: *.bat text eol=crlf *.cmd text eol=crlf Adopting this practice ensures that files are converted to the required CRLF format automatically upon checkout on Windows, maintaining compatibility with cmd.exe [6][4].
Citations:
- 1: [JENKINS-7478] Wrong EOL (UNIX type: LF) in Windows batch files executed for build steps of type "Execute Windows batch command" jenkinsci/jenkins#19822
- 2: https://serverfault.com/questions/429594/is-it-safe-to-write-batch-files-with-unix-line-endings
- 3: [BUG] write tool produces LF-only .bat/.cmd files on Windows and doesn't handle non-UTF-8 code pages anomalyco/opencode#31276
- 4: https://stackoverflow.com/questions/232651/why-the-system-cannot-find-the-batch-label-specified-is-thrown-even-if-label-e
- 5: fix: Windows batch file line endings to avoid cmd parsing bug bazel-contrib/bazel-lib#1222
- 6: https://stackoverflow.com/questions/38836068/lf-versus-crlf-line-endings-in-windows-batch-files
- 7: https://bugs.python.org/issue17202
Normalize this batch file to CRLF, and enforce *.bat text eol=crlf in .gitattributes. submit_fixes.bat is LF-only, which can make cmd.exe misparse labels and control flow on Windows.
🧰 Tools
🪛 Blinter (1.0.112)
[error] 1-1: Unix line endings detected. Explanation: Batch file uses Unix line endings (LF-only) which can cause GOTO/CALL label parsing failures and script malfunction due to Windows batch parser 512-byte boundary bugs. Recommendation: Convert file to Windows line endings (CRLF). Use tools like dos2unix, notepad++, or configure git with 'git config core.autocrlf true'. Context: File uses Unix line endings (LF-only) - 131 LF sequences found
(E018)
[error] 11-11: Unsafe SET command usage. Explanation: SET commands without proper validation or quoting can cause security issues. Recommendation: Always quote SET values and validate input: SET "var=safe value". Context: SET command value should be quoted for safety
(SEC002)
🤖 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 `@submit_fixes.bat` around lines 1 - 131, Normalize submit_fixes.bat to CRLF so
cmd.exe can parse labels and control flow correctly, and update .gitattributes
to enforce *.bat text eol=crlf for all batch files. Make the change in the batch
script itself and in the repo’s attributes config so the formatting stays
consistent going forward.
Source: Linters/SAST tools
| :: Check if branch already exists locally | ||
| git show-ref --quiet "refs/heads/%BRANCH%" | ||
| if errorlevel 1 ( | ||
| echo [INFO] Creating branch '%BRANCH%' from %REMOTE%/%BASE_BRANCH%... | ||
| git checkout -b %BRANCH% %REMOTE%/%BASE_BRANCH% | ||
| ) else ( | ||
| echo [WARN] Branch '%BRANCH%' already exists — checking it out | ||
| git checkout %BRANCH% | ||
| ) | ||
| if errorlevel 1 ( echo [ERROR] git checkout failed & pause & exit /b 1 ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't keep working on an old local fix branch.
If %BRANCH% already exists locally, this path reuses whatever history is on it instead of recreating it from the freshly fetched %REMOTE%/%BASE_BRANCH%. That can push unrelated or stale commits into the PR.
Suggested fix
:: Check if branch already exists locally
git show-ref --quiet "refs/heads/%BRANCH%"
-if errorlevel 1 (
- echo [INFO] Creating branch '%BRANCH%' from %REMOTE%/%BASE_BRANCH%...
- git checkout -b %BRANCH% %REMOTE%/%BASE_BRANCH%
-) else (
- echo [WARN] Branch '%BRANCH%' already exists — checking it out
- git checkout %BRANCH%
+if not errorlevel 1 (
+ echo [ERROR] Local branch '%BRANCH%' already exists. Delete it or reset it to %REMOTE%/%BASE_BRANCH% before rerunning.
+ pause & exit /b 1
)
+echo [INFO] Creating branch '%BRANCH%' from %REMOTE%/%BASE_BRANCH%...
+git checkout -b %BRANCH% %REMOTE%/%BASE_BRANCH%📝 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.
| :: Check if branch already exists locally | |
| git show-ref --quiet "refs/heads/%BRANCH%" | |
| if errorlevel 1 ( | |
| echo [INFO] Creating branch '%BRANCH%' from %REMOTE%/%BASE_BRANCH%... | |
| git checkout -b %BRANCH% %REMOTE%/%BASE_BRANCH% | |
| ) else ( | |
| echo [WARN] Branch '%BRANCH%' already exists — checking it out | |
| git checkout %BRANCH% | |
| ) | |
| if errorlevel 1 ( echo [ERROR] git checkout failed & pause & exit /b 1 ) | |
| :: Check if branch already exists locally | |
| git show-ref --quiet "refs/heads/%BRANCH%" | |
| if not errorlevel 1 ( | |
| echo [ERROR] Local branch '%BRANCH%' already exists. Delete it or reset it to %REMOTE%/%BASE_BRANCH% before rerunning. | |
| pause & exit /b 1 | |
| ) | |
| echo [INFO] Creating branch '%BRANCH%' from %REMOTE%/%BASE_BRANCH%... | |
| git checkout -b %BRANCH% %REMOTE%/%BASE_BRANCH% | |
| if errorlevel 1 ( echo [ERROR] git checkout failed & pause & exit /b 1 ) |
🤖 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 `@submit_fixes.bat` around lines 54 - 63, The branch-handling logic in
submit_fixes.bat reuses an existing local branch instead of recreating it from
the freshly fetched base, which can leave stale history on %BRANCH%. Update the
branch setup flow around the git show-ref / git checkout block so that an
existing local %BRANCH% is replaced or reset to %REMOTE%/%BASE_BRANCH% before
continuing, ensuring the script always starts from the latest remote base rather
than an old local fix branch.
| :: acbu_escrow | ||
| if exist "%FIXES_DIR%\acbu_escrow\src\lib.rs" ( | ||
| if not exist "acbu_escrow\src" mkdir "acbu_escrow\src" | ||
| copy /Y "%FIXES_DIR%\acbu_escrow\src\lib.rs" "acbu_escrow\src\lib.rs" >nul | ||
| echo [INFO] Copied: acbu_escrow\src\lib.rs | ||
| ) else ( | ||
| echo [WARN] Not found, skipping: %FIXES_DIR%\acbu_escrow\src\lib.rs | ||
| ) | ||
|
|
||
| :: acbu_minting | ||
| if exist "%FIXES_DIR%\acbu_minting\src\lib.rs" ( | ||
| if not exist "acbu_minting\src" mkdir "acbu_minting\src" | ||
| copy /Y "%FIXES_DIR%\acbu_minting\src\lib.rs" "acbu_minting\src\lib.rs" >nul | ||
| echo [INFO] Copied: acbu_minting\src\lib.rs | ||
| ) else ( | ||
| echo [WARN] Not found, skipping: %FIXES_DIR%\acbu_minting\src\lib.rs | ||
| ) | ||
|
|
||
| :: shared errors | ||
| if exist "%FIXES_DIR%\shared\src\errors.rs" ( | ||
| if not exist "shared\src" mkdir "shared\src" | ||
| copy /Y "%FIXES_DIR%\shared\src\errors.rs" "shared\src\errors.rs" >nul | ||
| echo [INFO] Copied: shared\src\errors.rs | ||
| ) else ( | ||
| echo [WARN] Not found, skipping: %FIXES_DIR%\shared\src\errors.rs | ||
| ) | ||
|
|
||
| :: ── Stage ───────────────────────────────────────────────────────────────────── | ||
| echo [INFO] Staging changes... | ||
| git add acbu_escrow\src\lib.rs acbu_minting\src\lib.rs shared\src\errors.rs | ||
| if errorlevel 1 ( echo [ERROR] git add failed & pause & exit /b 1 ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail the copy/stage step per file instead of assuming every target exists.
Each copy block prints success without checking copy's exit code, and Lines 97-98 then stage all three paths even when an earlier block printed “skipping”. That either stages stale files or aborts on a missing path.
Suggested fix
+set COPIED_ANY=
+
:: acbu_escrow
if exist "%FIXES_DIR%\acbu_escrow\src\lib.rs" (
if not exist "acbu_escrow\src" mkdir "acbu_escrow\src"
copy /Y "%FIXES_DIR%\acbu_escrow\src\lib.rs" "acbu_escrow\src\lib.rs" >nul
+ if errorlevel 1 ( echo [ERROR] copy failed for acbu_escrow\src\lib.rs & pause & exit /b 1 )
+ git add "acbu_escrow\src\lib.rs"
+ if errorlevel 1 ( echo [ERROR] git add failed for acbu_escrow\src\lib.rs & pause & exit /b 1 )
+ set COPIED_ANY=1
echo [INFO] Copied: acbu_escrow\src\lib.rs
) else (
echo [WARN] Not found, skipping: %FIXES_DIR%\acbu_escrow\src\lib.rs
)
@@
-echo [INFO] Staging changes...
-git add acbu_escrow\src\lib.rs acbu_minting\src\lib.rs shared\src\errors.rs
-if errorlevel 1 ( echo [ERROR] git add failed & pause & exit /b 1 )
+if not defined COPIED_ANY (
+ echo [WARN] No fix files were copied
+ goto :push
+)🤖 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 `@submit_fixes.bat` around lines 68 - 98, The copy/stage flow in
submit_fixes.bat assumes every target file exists, so update each copy block to
verify the copy command succeeded before printing success and before staging
that file. Use the existing acbu_escrow, acbu_minting, and shared errors blocks
to track which files were actually copied, then gate the git add step so it only
stages files that were successfully copied instead of always adding all three
paths. If a source is missing or copy fails, fail that file’s step explicitly
rather than proceeding with stale or nonexistent targets.
| info "Creating branch '$BRANCH' from $REMOTE/$BASE_BRANCH..." | ||
| if git show-ref --quiet "refs/heads/$BRANCH"; then | ||
| warn "Branch '$BRANCH' already exists locally — checking it out" | ||
| git checkout "$BRANCH" | ||
| else | ||
| git checkout -b "$BRANCH" "$REMOTE/$BASE_BRANCH" | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't silently reuse an existing local fix branch.
If this branch already has local commits or is behind origin/dev, rerunning the script produces a PR from stale ancestry instead of a fresh branch from the fetched base. Abort here or explicitly reset the branch to $REMOTE/$BASE_BRANCH.
Suggested fix
info "Creating branch '$BRANCH' from $REMOTE/$BASE_BRANCH..."
if git show-ref --quiet "refs/heads/$BRANCH"; then
- warn "Branch '$BRANCH' already exists locally — checking it out"
- git checkout "$BRANCH"
-else
- git checkout -b "$BRANCH" "$REMOTE/$BASE_BRANCH"
+ die "Local branch '$BRANCH' already exists. Delete it or reset it to $REMOTE/$BASE_BRANCH before rerunning."
fi
+git checkout -b "$BRANCH" "$REMOTE/$BASE_BRANCH"📝 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.
| info "Creating branch '$BRANCH' from $REMOTE/$BASE_BRANCH..." | |
| if git show-ref --quiet "refs/heads/$BRANCH"; then | |
| warn "Branch '$BRANCH' already exists locally — checking it out" | |
| git checkout "$BRANCH" | |
| else | |
| git checkout -b "$BRANCH" "$REMOTE/$BASE_BRANCH" | |
| fi | |
| info "Creating branch '$BRANCH' from $REMOTE/$BASE_BRANCH..." | |
| if git show-ref --quiet "refs/heads/$BRANCH"; then | |
| die "Local branch '$BRANCH' already exists. Delete it or reset it to $REMOTE/$BASE_BRANCH before rerunning." | |
| fi | |
| git checkout -b "$BRANCH" "$REMOTE/$BASE_BRANCH" |
🤖 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 `@submit_fixes.sh` around lines 37 - 43, The branch handling in the script
currently reuses an existing local branch via git checkout, which can leave
stale commits behind. Update the logic around the branch creation flow so that
when "$BRANCH" already exists locally it either aborts with a clear message or
explicitly resets it to "$REMOTE/$BASE_BRANCH" before proceeding, rather than
silently checking it out. Use the existing info/warn checkout block and the
"$BRANCH", "$REMOTE", and "$BASE_BRANCH" symbols to locate the fix.
| copy_if_exists() { | ||
| local src="$1" dst="$2" | ||
| if [ -f "$src" ]; then | ||
| mkdir -p "$(dirname "$dst")" | ||
| cp "$src" "$dst" | ||
| info "Copied: $dst" | ||
| else | ||
| warn "Source not found, skipping: $src" | ||
| fi | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
git add contradicts the “skip missing fix” path.
copy_if_exists warns and continues when a source file is absent, but Lines 66-69 still stage all three paths unconditionally. If a skipped destination does not already exist, git add fails and the script exits anyway.
Suggested fix
+copied_files=()
+
copy_if_exists() {
local src="$1" dst="$2"
if [ -f "$src" ]; then
mkdir -p "$(dirname "$dst")"
cp "$src" "$dst"
+ copied_files+=("$dst")
info "Copied: $dst"
else
warn "Source not found, skipping: $src"
fi
}
@@
info "Staging changes..."
-git add \
- acbu_escrow/src/lib.rs \
- acbu_minting/src/lib.rs \
- shared/src/errors.rs
+if ((${`#copied_files`[@]})); then
+ git add -- "${copied_files[@]}"
+else
+ warn "No fix files were copied"
+fiAlso applies to: 64-73
🤖 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 `@submit_fixes.sh` around lines 48 - 57, The staging step contradicts the
missing-file skip behavior in copy_if_exists, since it still adds all expected
paths even when a source was skipped. Update the git add logic in
submit_fixes.sh so it only stages files that were actually copied or already
exist, and avoid failing when a destination is absent after copy_if_exists
warned and continued. Use the copy_if_exists helper and the later staging block
together to gate git add on file presence.
2 participants
close #355
close #357
close #361
close #362
Summary by CodeRabbit