Fix SDK submodule update to handle commit SHAs

[juanmichelini]

This PR fixes the SDK submodule update step in the build-swebenchmultimodal-images workflow to properly handle commit SHAs.

Problem:
The workflow was trying to git fetch origin <commit-sha>, which doesn't work for commit SHAs - it only works for branch names and refs.

Solution:

• Fetch all from origin first to ensure the commit is available locally
• Then checkout the specific commit/ref directly
• This allows passing both branch names and commit SHAs to the sdk-commit input

Testing:
This fix is needed for the current evaluation run (20979458219) which is failing because the benchmarks build can't fetch the SDK commit.

[openhands-ai]

Looks like there are a few issues preventing this PR from being merged!

• GitHub Actions are failing:
  • Run Eval (SDK)

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #311 at branch `fix-sdk-submodule-fetch`

Feel free to include any additional details that might help me get this PR into a better state.

_{^{You can manage your notification settings}}

[juanmichelini]

@OpenHands to make this PR cleaner

keep the changes in

.github/workflows/build-swebenchmultimodal-images.yml benchmarks/swebenchmultimodal/build_images.py

but revert the changes in vendor/software-agent-sdk

[openhands-ai]

I'm on it! juanmichelini can track my progress at all-hands.dev

[openhands-ai]

Based on my review of the current state of the repository, there are no changes to push. The repository is on the master branch with no commits yet, and only contains an untracked .vscode/ directory.

It appears this is a fresh workspace without any prior work to summarize. If you had a specific task you'd like me to help with, please let me know and I'll be happy to assist!

View full conversation

[neubig]

@OpenHands /codereview-roasted

[openhands-ai]

I'm on it! neubig can track my progress at all-hands.dev

[openhands-ai]

Code Review Summary (Linus Torvalds Style 🔥)

I provided a critical code review of PR #311 "Fix SDK submodule update to handle commit SHAs" as requested with the /codereview-roasted command.

Verdict: 🔴 Needs improvement

Key Issues Identified:

1. Workflow Change (.github/workflows/build-swebenchmultimodal-images.yml):

   • The fix uses git fetch origin (fetches ALL refs - wasteful)
   • Followed by a redundant git fetch origin ${{ inputs.sdk-commit }} 2>/dev/null || true
   • The 2>/dev/null || true pattern silently swallows errors - this is error hiding, not error handling
   • Recommended fix: Just use 2 lines instead of 4:

     git fetch origin
     git checkout ${{ inputs.sdk-commit }}

2. Python Change (benchmarks/swebenchmultimodal/build_images.py):

   • ✅ This part is clean and acceptable - minimal, focused parameter addition

Key Insight:

The original problem was real (you can't git fetch origin <sha>), but the solution added unnecessary complexity. When fixing broken code, understand why it was broken and write the minimal fix rather than piling on more code until it works.

---

No code changes were made - this was a review-only request.

View full conversation

[neubig]

Could we check if the above simplification that openhands suggested works?