refactor: Use composite GitHub Action for PR review workflow

[xingyaoww]

Summary

This PR adds a reusable composite GitHub Action for automated PR reviews using the OpenHands agent SDK.

Changes

• .github/actions/pr-review/action.yml (new):

  • Composite GitHub Action that encapsulates the entire PR review workflow
  • Configurable inputs for LLM model, base URL, review style, and SDK version
  • Handles SDK checkout, Python setup, dependency installation, and review execution
  • Supports use-pr-branch-for-sdk option for testing SDK changes on the SDK repo itself

• .github/workflows/pr-review-by-openhands.yml:

  • Simplified to use the new composite action
  • Reduced from ~90 lines to ~15 lines

• examples/03_github_workflows/02_pr_review/workflow.yml:

  • Updated example workflow to use the composite action
  • Simplified setup instructions

• examples/03_github_workflows/02_pr_review/README.md:

  • Updated documentation for the composite action
  • Added action inputs table

Benefits

• Reusability: Other repositories can easily adopt the PR review workflow by referencing the composite action
• Maintainability: Changes to the review workflow only need to be made in one place
• Simplicity: Users only need to configure a few inputs instead of copying the entire workflow
• Version Control: Users can pin to specific SDK versions using the sdk-version input

Usage

Repositories can use the composite action like this:

- name: Run PR Review
  uses: OpenHands/software-agent-sdk/.github/actions/pr-review@main
  with:
      llm-model: anthropic/claude-sonnet-4-5-20250929
      review-style: standard
      sdk-version: main
      llm-api-key: ${{ secrets.LLM_API_KEY }}
      github-token: ${{ secrets.GITHUB_TOKEN }}

Checklist

• If the PR is changing/adding functionality, are there tests to reflect this?
• If there is an example, have you run the example to make sure that it works?
• If there are instructions on how to run the code, have you followed the instructions and made sure that it works?
• If the feature is significant enough to require documentation, is there a PR open on the OpenHands/docs repository with the same branch name?
• Is the github CI passing?

---

Agent Server images for this PR

• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant	Architectures	Base Image	Docs / Tags
java	amd64, arm64	eclipse-temurin:17-jdk	Link
python	amd64, arm64	nikolaik/python-nodejs:python3.12-nodejs22	Link
golang	amd64, arm64	golang:1.21-bookworm	Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:daddf99-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-daddf99-python \
  ghcr.io/openhands/agent-server:daddf99-python

All tags pushed for this build

ghcr.io/openhands/agent-server:daddf99-golang-amd64
ghcr.io/openhands/agent-server:daddf99-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:daddf99-golang-arm64
ghcr.io/openhands/agent-server:daddf99-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:daddf99-java-amd64
ghcr.io/openhands/agent-server:daddf99-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:daddf99-java-arm64
ghcr.io/openhands/agent-server:daddf99-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:daddf99-python-amd64
ghcr.io/openhands/agent-server:daddf99-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-amd64
ghcr.io/openhands/agent-server:daddf99-python-arm64
ghcr.io/openhands/agent-server:daddf99-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-arm64
ghcr.io/openhands/agent-server:daddf99-golang
ghcr.io/openhands/agent-server:daddf99-java
ghcr.io/openhands/agent-server:daddf99-python

About Multi-Architecture Support

• Each variant tag (e.g., daddf99-python) is a multi-arch manifest supporting both amd64 and arm64
• Docker automatically pulls the correct architecture for your platform
• Individual architecture tags (e.g., daddf99-python-amd64) are also available if needed

[xingyaoww]

@OpenHands merge from main, resolve the conflict, then review the diff from main. Please remove ALL the additional code that adds the cloud mode. Let's focus on making the composite github action in this PR. Please update PR description & title when you are done

[openhands-ai]

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

[openhands-ai]

I have completed all the requested tasks:

Summary of Changes

✅ Checklist

• Merged from main branch and resolved conflicts
• Reviewed the diff from main to identify all cloud mode code
• Removed ALL cloud mode code from the PR
• Updated PR title and description to focus on composite GitHub action

Changes Made

1. Merged from main - Resolved the conflict in .github/workflows/pr-review-by-openhands.yml

2. Removed cloud mode code from:

   • agent_script.py - Restored to original version from main (removed run_cloud_mode(), _start_cloud_conversation(), post_github_comment(), and MODE handling)
   • action.yml - Removed mode, openhands-cloud-api-key inputs and all cloud mode logic
   • workflow.yml - Removed cloud mode references and simplified
   • README.md - Removed all cloud mode documentation
   • tests/github_workflows/test_pr_review_agent.py - Deleted (was cloud mode tests)

3. Updated PR title and description:

   • New title: "feat: Add composite GitHub Action for PR review workflow"
   • Updated description to focus on the composite action benefits

Final PR State

The PR now focuses solely on adding a reusable composite GitHub Action for PR reviews:

• 4 files changed: +213 insertions, -181 deletions
• New composite action at .github/actions/pr-review/action.yml
• Simplified workflow using the composite action
• Updated documentation

PR Link: #1927

View full conversation

[all-hands-bot]

Review Summary

Excellent work on creating a reusable composite action for PR reviews! The overall design is solid and will greatly improve maintainability. However, I found a critical path issue with log artifact uploads and some consistency concerns that should be addressed before merging.

Key Issues:

• 🔴 Critical: Artifact upload paths are incorrect
• 🟠 Important: Inconsistent default review style
• 🟡 Suggestions: UX and documentation improvements

[all-hands-bot]

🔴 Critical: The artifact paths are incorrect. The script runs with cd pr-repo (line 114), so logs are written to pr-repo/*.log and pr-repo/output/. However, in composite actions, each step starts from the original working directory, so this upload step will look in the wrong location.

Fix:

Suggested change

	output/
	pr-repo/*.log
	pr-repo/output/

[all-hands-bot]

🟡 Suggestion: Consider documenting the security implications of the sdk-version input. When set to a PR branch/commit (as done in the SDK's own workflow), it executes potentially untrusted code. Users should understand this is safe here because the composite action itself is versioned separately from the SDK code being tested.

[all-hands-bot]

🟠 Important: This checkout step is necessary but not intuitive for users. The requirement to checkout the SDK repo just to reference the composite action creates friction.

Better UX options:

1. Reference the action from GitHub directly: uses: OpenHands/software-agent-sdk/.github/actions/pr-review@v1.0.0 (no local checkout needed)
2. Document WHY the checkout is needed in a comment above this step

The current approach only makes sense if users want to modify the action locally, which seems unlikely for most use cases.

[all-hands-bot]

🟢 Nit: This is clever for testing SDK changes on the SDK repo itself, but consider adding a comment explaining why this differs from the example workflow. It helps future maintainers understand the self-referential testing pattern.

Suggested change

	sdk-version: ${{ github.event.pull_request.head.sha }}
	# Use the PR's head commit SHA to test SDK changes on the SDK repo itself
	sdk-version: ${{ github.event.pull_request.head.sha }}

[all-hands-bot]

🟢 Nit: Empty string for LLM_BASE_URL is handled correctly by the Python script (agent_script.py:219-220 only sets base_url if truthy), but consider using null or omitting the env var entirely for clarity. Empty strings in environment variables can be ambiguous.

[all-hands-bot]

🟢 Nit: Using --system with uv pip is correct for CI environments, but consider adding a comment explaining why this is needed (to install into the system Python rather than creating a venv).

Suggested change

	run: |
	# Install to system Python (no venv needed in CI)
	uv pip install --system ./software-agent-sdk/openhands-sdk ./software-agent-sdk/openhands-tools

[enyst]

@OpenHands Do a /codereview-roasted on this PR. Post the review on gthub as a comment, using remote gh. Note that it will be rendered as markdown.

[openhands-ai]

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

[enyst]

🟡 Acceptable — You simplified the workflow, but you replaced a bunch of implicit guarantees with wishful thinking. Composite actions don’t magically remove coupling; they just hide it.

[CRITICAL ISSUES]

• [.github/actions/pr-review/action.yml, L38–115] Breaking change / compatibility: You hard-coded the composite action to always fetch the SDK from OpenHands/software-agent-sdk and then execute agent_script.py from that checkout, ignoring the repo that hosts the action. That means anyone vendoring this action or forking it can’t run their own script or SDK without editing the action. This is a usability regression compared to the old workflow that naturally used the checked-out repo. At minimum, allow an input to specify the SDK repository or support ./ when used locally.
• [.github/workflows/pr-review-by-openhands.yml, L41–49] Broken self-hosting story: The workflow sets sdk-version to github.event.pull_request.head.sha. That only works when the PR is in this repo. For external consumers, this is nonsense and will 404. Either document this as “SDK repo only” or stop using the head SHA here.
• [.github/actions/pr-review/action.yml, L45–52 + L99–115] Trust boundary confusion: You checkout untrusted PR code (from a fork) and then run a script from another repo that will use gh and network access with write tokens. That’s acceptable only because you avoid persisting credentials, but you still execute in the PR repo’s working directory. If the script relies on local files, you’re letting untrusted code influence the run. At least make it explicit in the action docs and consider running the script from the SDK repo while pointing it at the PR repo path.

[IMPROVEMENT OPPORTUNITIES]

• [.github/actions/pr-review/action.yml, L64–68] Unnecessary apt installs: You apt-get gh every run. That’s slow and flaky. Use actions/github-script or prebuilt cli/cli setup action. If the only use is API calls, don’t install CLI at all.
• [.github/actions/pr-review/action.yml, L70–73] Over-coupled dependency install: uv pip install --system from local paths forces a full install of the SDK on every run. The old workflow did this too, but the composite action is your chance to allow caching or a use-uv-cache toggle. Right now, you just moved the pain to a different file.
• [examples/03_github_workflows/02_pr_review/README.md, L122–127] Docs lie: The action input table says the default review-style is standard, but the action default is roasted. If you’re going to be brutal, at least be consistent.

[STYLE NOTES]

• [.github/actions/pr-review/action.yml, L20–21] YAML readability: The multi-line description for review-style is wrapped mid-sentence, which renders badly in GitHub UI. Use a block scalar or keep it on one line.

VERDICT:
❌ Needs rework. You simplified the workflow, but you smuggled in a bunch of hard-coded assumptions that make the action brittle outside this repo. Fix the SDK repo coupling and the wrong defaults, then we can talk.

KEY INSIGHT:
A reusable action isn’t reusable if it’s welded to one repository and one execution path.

[enyst]

(OpenHands-GPT-5.2-codex)

Here’s my re‑think of the non‑struck parts of the comment, and what I now consider necessary vs nice‑to‑have fixes.

---

Re‑review of the remaining points

1) Hard‑coded SDK repo (action.yml checkout + execution)

Original claim: This makes the action non‑reusable for forks/vendoring.

Re‑think:
This is partly fair but overstated. The action is explicitly described as a reusable action hosted in this repo. It’s valid for a composite action to pull the SDK from its own source of truth. That said, if someone forks the repo and wants to run their fork’s code, they must edit the action or change the repo input. That’s not a functional bug, but it does reduce portability.

Verdict now: Not “critical.” It’s a design limitation.
Importance: Medium.
Necessary fix? Optional. Add an input for sdk-repo if you want broader reusability.

---

2) sdk-version set to head SHA in workflow

Original claim: Will 404 for external consumers.

Re‑think:
This is correct in the context of the main workflow in this repo. Using github.event.pull_request.head.sha only makes sense if the PR is in this repo. But that workflow lives in this repo, so that may be intentional (to test SDK changes in‑repo). For external consumers, they’d use the example workflow.

Verdict now: Fair, but not a bug. It’s a repo‑specific choice.
Importance: Low–Medium.
Necessary fix? Only if the workflow is marketed as copy‑paste for other repos. Otherwise, just document this as SDK‑repo‑only.

---

3) Trust boundary / running from PR repo

Original claim: Untrusted code can influence the run.

Re‑think:
The action checks out the PR code and runs the SDK script from the PR repo directory (cd pr-repo). That could let a PR alter runtime behavior (e.g., by shadowing paths or expecting relative files). However, since the script lives in the SDK repo and uses environment variables + network calls, the risk is mostly behavioral, not an obvious security break. Credentials aren’t persisted to the PR repo, which helps.

Verdict now: Partly fair but a bit dramatic.
Importance: Medium if you care about strict isolation.
Necessary fix? Optional. You could run the script from SDK repo and pass the PR repo path explicitly, or at least document the execution context.

---

4) Installing gh via apt every run

Original claim: Slow/flaky.

Re‑think:
This is correct but mostly a performance concern, not correctness. If the review script relies on gh, you need it installed. There are better setup actions, but it’s not wrong.

Verdict now: Minor improvement.
Importance: Low.
Necessary fix? Not necessary.

---

Summary: What’s actually “necessary” to fix?

Strictly necessary:

• None of the remaining points are true “must fix.” They’re design‑choice tradeoffs.

If you want better portability / clarity (recommended):

1. Document repo‑specific assumptions (especially the sdk-version head SHA usage).
2. Consider allowing sdk-repo as an input if you want forks/derivatives to reuse without editing the action.
3. Clarify execution context (script run from PR repo) or run from SDK repo and pass path.

Priority ranking:

1. Document sdk-version behavior (low effort, avoids confusion).
2. Optional sdk-repo input (medium effort, higher flexibility).
3. Isolation of run directory (nice‑to‑have).

---

Bottom line

The original “critical” framing was too harsh. The remaining issues are design limitations and clarity gaps, not correctness bugs. If this action is primarily for the OpenHands repo itself, it’s already reasonable. If the goal is broad public reuse, the documentation and repo‑hardcoding should be revisited.

[xingyaoww]

@OpenHands review #1927 (comment) and #1927 (comment) and check the latest change. Let's add it to the documentation that we encourage people to customize their own codereview skill by adding .openhands/skills/code-review.md instead of forking the agent_script.py similar to this repo.

We should update docs in https://docs.openhands.dev/sdk/guides/github-workflows/pr-review of OpenHands/docs repo as well

[openhands-ai]

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

[openhands-ai]

Summary

I've completed the documentation updates to encourage users to customize their code review by adding .openhands/skills/code-review.md instead of forking the agent_script.py.

Changes Made

1. software-agent-sdk (PR #1927 updated)

File: examples/03_github_workflows/02_pr_review/README.md

Added a new "Customizing the Code Review" section that includes:

• Explanation of how custom skills work (project skills override public skills)
• Complete example of a custom code-review.md skill with frontmatter
• Benefits of using custom skills (no forking, version controlled, easy updates, team alignment)
• Reference to the software-agent-sdk's own code-review skill as an example
• Fixed the Action Inputs table to show correct default (roasted instead of standard)

Commit: da2095ea - pushed to openhands/pr-review-cloud-mode

2. OpenHands/docs (New PR #306 created)

File: sdk/guides/github-workflows/pr-review.mdx

• Added "Customizable" to the Features list
• Added complete "Customizing the Code Review" section with the same content
• Updated the reference workflow to use the new composite action (simplified from ~90 lines to ~15 lines)
• Added Action Inputs table documenting all available inputs
• Added link to composite action in Related Files section

PR: OpenHands/docs#306 (draft)

Checklist

• ✅ Reviewed the PR comments (3865166456 and 3865152151)
• ✅ Added documentation for customizing code review via .openhands/skills/code-review.md
• ✅ Referenced the software-agent-sdk's own code-review skill as an example
• ✅ Updated docs in OpenHands/docs repo at the pr-review page
• ✅ Changes are concise and focused on documentation only

View full conversation

[all-hands-bot]

🤖 Code Review

This refactoring successfully consolidates the PR review workflow into a reusable composite action, improving maintainability and making it easier for external repositories to adopt the workflow. However, there are several important issues that should be addressed before merging.

---

🔴 Critical Issues

Security Consideration in .github/workflows/pr-review-by-openhands.yml:49

Using github.event.pull_request.head.sha to test the SDK with its own PR changes creates a potential security risk:

The Risk: A malicious PR could modify agent_script.py to exfiltrate the LLM_API_KEY or ALLHANDS_BOT_GITHUB_PAT secrets, and this workflow would run that malicious code with access to those secrets.

Why this happens: pull_request_target runs in the base branch context (giving access to secrets), but this action explicitly checks out the PR code (which includes potentially malicious changes).

Recommended mitigation: Pin to a stable SDK version (main or a specific tag) for the PR review workflow. Only use PR SHA for specific testing workflows that don't expose sensitive secrets.

---

🟠 Important Issues

1. Hard-coded Path in .github/actions/pr-review/action.yml:119

The path ../software-agent-sdk/examples/03_github_workflows/02_pr_review/agent_script.py is hard-coded and assumes a specific directory structure. If the SDK checkout fails silently, this will fail with a cryptic error.

Suggestion: Add validation before running the script:

if [ ! -f ../software-agent-sdk/examples/03_github_workflows/02_pr_review/agent_script.py ]; then
  echo "Error: agent_script.py not found. SDK checkout may have failed."
  exit 1
fi
uv run python ../software-agent-sdk/examples/03_github_workflows/02_pr_review/agent_script.py

2. Ambiguous Log Upload Path in .github/actions/pr-review/action.yml:121-128

After cd pr-repo on line 118, the logs will be collected from the pr-repo directory, but it's not clear if that's where the agent writes its logs. If the agent script writes logs to a different directory, they won't be uploaded.

Suggestion: Use explicit paths:

path: |
    pr-repo/*.log
    pr-repo/output/

3. Missing SDK Checkout Validation in .github/actions/pr-review/action.yml:77

The installation will fail silently if the SDK checkout (lines 42-47) didn't work properly.

Suggestion: Add validation:

if [ ! -d ./software-agent-sdk/openhands-sdk ]; then
  echo "Error: SDK directory not found"
  exit 1
fi
uv pip install --system ./software-agent-sdk/openhands-sdk ./software-agent-sdk/openhands-tools

4. Potential Breaking Change in .github/workflows/pr-review-by-openhands.yml:44

The original workflow used litellm_proxy/claude-sonnet-4-5-20250929 with base URL https://llm-proxy.app.all-hands.dev, but this now uses direct Anthropic API (anthropic/claude-sonnet-4-5-20250929).

Impact: This will fail if LLM_API_KEY is not an Anthropic API key, or if there are different usage limits/costs.

Question: Is this intentional? If so, please update the secret or document why the proxy was removed.

5. Observability Removed

The original workflow had LMNR_PROJECT_API_KEY for observability, which has been removed. Consider documenting this change if intentional.

---

🟡 Suggestions

1. Redundant Checkout in examples/03_github_workflows/02_pr_review/workflow.yml:35-41

The example workflow uses sparse-checkout to get just the action directory, but then the action itself checks out the entire SDK repository again. This is redundant.

Alternative: Reference the action directly from GitHub:

- name: Run PR Review
  uses: OpenHands/software-agent-sdk/.github/actions/pr-review@main
  with:
    # ... inputs

2. Confusing Empty String in README Example

examples/03_github_workflows/02_pr_review/README.md:48 shows llm-base-url: '' which might be confusing. Either omit it or show a real example with a comment.

3. Inefficient apt-get Update in .github/actions/pr-review/action.yml:70

Running apt-get update on every workflow run is slow. Consider checking if gh exists first:

if ! command -v gh &> /dev/null; then
  sudo apt-get update
  sudo apt-get install -y gh
fi

---

🟢 Minor Nits

1. Missing Fork PR Explanation

examples/03_github_workflows/02_pr_review/workflow.yml:19 uses pull_request trigger which won't work for fork PRs. Consider adding a comment explaining this limitation.

2. Aggressive Default Review Style

.github/actions/pr-review/action.yml:23 defaults to roasted (Linus Torvalds-style). Consider making standard the default for broader adoption, letting teams opt-in to roasted.

---

✅ What's Good

• Clean refactoring that significantly reduces duplication
• Good documentation in the README
• Proper use of composite actions
• Security-conscious use of persist-credentials: false
• Good input parameter design with sensible defaults
• Comprehensive README updates

---

Summary

Overall Assessment: This is a valuable refactoring that improves the codebase. The critical security issue should be addressed, and the important issues around error handling and validation will make the action more robust. Once these are resolved, this will be ready to merge.

Priority Order:

1. Address the security concern (use stable SDK version)
2. Add validation for SDK checkout and paths
3. Clarify/fix the breaking change in LLM configuration
4. Consider the suggestions for better user experience

[xingyaoww]

#1927 (comment)

We've already discussed the first risk a while ago and that's some risk we were willing to take (already partly mitigated by sdk's secret config)

[enyst]

Out of curiosity, why not REQUEST CHANGES if it has concerns?

[xingyaoww]

Because then itll be slightlytricky for all-hands-bot to re-review and unblock again. in general, i dont want to see reviewer bot is blocking our PR with some trivialsuggestion 😓

[enyst]

TBH in my experience so far, my concern is more the other way around: if it has blocking concerns, then it tends to disregard or minimize the other comments 🤔

That's why the idea I brought in slack about a workflow forcing it to address all comments, no matter how trivial, to make it think about them before dismissing them -that feels like a thing my yolo repos need 😅

I feel maybe it's good to keep it simpler... I'm not sure it'd be tricky the other way around, but maybe it would need to re-run in the same session, not from github actions with fresh context every time maybe (my little agents were run from CLI runs on my machine, and re-reviewed from the same session)

[xingyaoww]

good point, maybe we can leave it as customizable for each repo. which probably outof the conext of this PR; but yeah agree on the blocker workflow

[enyst]

Thank you for this! I think maybe we can reuse it in other repos