feat: add qa-changes plugin for automated PR QA validation

[xingyaoww]

Summary

Add a new qa-changes plugin that goes beyond code review by actually running the code to verify PR changes work as described. While the existing pr-review plugin reads diffs and posts inline code comments, this plugin sets up the environment, runs the test suite, exercises changed behavior, and posts a structured QA report.

Plugin Structure

skills/qa-changes/SKILL.md              # Generic QA methodology skill
plugins/qa-changes/
├── README.md                           # Plugin documentation
├── action.yml                          # Composite GitHub Action
├── scripts/
│   ├── agent_script.py                 # Main QA agent script
│   └── prompt.py                       # Prompt template
├── skills/
│   └── qa-changes -> ../../../skills/qa-changes
└── workflows/
    └── qa-changes-by-openhands.yml     # Example workflow

Four-Phase QA Methodology

The skill defines a generic, language-agnostic methodology:

1. Understand — Read the diff, classify changes (new feature, bug fix, refactor, config/docs)
2. Setup — Bootstrap the repo: install deps, build, establish test baseline
3. Exercise — The core phase. Actually use the software: run CLI commands, make HTTP requests, open browsers. Go beyond "tests pass."
4. Report — Post structured PR comment with evidence and verdict (PASS / PASS WITH ISSUES / FAIL)

How It Differs from PR Review

Aspect	PR Review	QA Changes
Method	Reads the diff	Runs the code
Speed	2-3 minutes	5-15 minutes
Catches	Style, security, logic issues	Regressions, broken features, build failures
Output	Inline code comments	Structured QA report with evidence

Usage

- name: Run QA Changes
  uses: OpenHands/extensions/plugins/qa-changes@main
  with:
    llm-model: anthropic/claude-sonnet-4-5-20250929
    llm-api-key: ${{ secrets.LLM_API_KEY }}
    github-token: ${{ secrets.GITHUB_TOKEN }}

Triggers: qa-this label or openhands-agent reviewer request.

Design Decisions

• Generic skill: The SKILL.md is intentionally language/framework-agnostic. It teaches the agent how to think about QA, not specific commands. Project-specific details come from AGENTS.md or custom skills.
• Structured from pr-review: The agent_script.py and action.yml follow the same patterns as pr-review for consistency, but the prompt and skill are completely different.
• Security: Excludes FIRST_TIME_CONTRIBUTOR and NONE from automatic triggers since QA executes code.

Related

• Companion docs PR: OpenHands/docs (adds use-case page and SDK workflow guide)
• Inspired by Anthropic's harness design for long-running apps — the QA agent is essentially the "evaluator" in a generator-evaluator pattern.

This PR was created by an AI assistant (OpenHands).

[xingyaoww]

Rebased onto main to resolve merge conflicts (marketplace file was renamed from default.json → openhands-extensions.json). Also fixed the plugin.json author field to use the object format required by Claude Code compatibility tests, added the qa-changes entry to the marketplace, and ran sync_extensions.py to generate missing symlinks/commands.

All core tests pass (64/64). The test_qa_changes.py errors are expected — they require lmnr which is a CI-only dependency.

We've been using qa-changes for a while now and it's working well. Merging this to main so it can be included in the new openhands-verification-stack marketplace bundle alongside onboarding, pr-review, and iterate.

This comment was posted by an AI assistant (OpenHands) on behalf of the user.

[all-hands-bot]

🟡 Acceptable - Solid implementation with minor improvements needed

This PR adds valuable QA validation capabilities that go beyond static code review by actually executing PR changes. The implementation follows existing patterns from pr-review, includes comprehensive testing, and has well-documented security tradeoffs.

Three Questions:

1. ✅ Real problem? Yes - many PRs pass review but break when run
2. ✅ Simpler way? No - the complexity matches the problem
3. ✅ What breaks? Nothing - new plugin, well-isolated

Key Strengths:

• Reuses proven patterns from pr-review plugin
• Comprehensive test coverage for utility functions
• Security model is sound and well-documented
• Clear separation between skill (methodology) and plugin (automation)

Issues to address: See inline comments for documentation inconsistency, magic numbers, and readability improvements.

[enyst]

@OpenHands read the comments in this PR. First things first: disable uv cache or some other github cache if any. We should not share this workflow data with other workflows. Commit and push only that.

Apart from that, investigate github docs and tell me if it’s possible to partition caches, e.g. somehow cache for workflows a, b and c, but NOT d, e, f.

[enyst]

@OpenHands read all comments here. Let’s focus on the blast radius, but only on this part:

this PR loads project skills from the checked-out repo,
Recommendations
Do not load repo-controlled agent guidance from the PR head
Load AGENTS.md / project skills from the base branch, or from a maintainer-controlled path outside the PR checkout.
This is one of the highest-leverage fixes in this design.

I agree, but I’m a little confused here: look up in agent-sdk repo (clone it from the same org) the system prompt construction; and I think you will find that it tells the agent that these files are untrusted. Verify what I just said. Are all these skills and AGENTS.md covered by the existing prompt?

Does the current behavior of the SDK address this concern already or do you have targeted recommendations? Only answer, directly as a comment on this PR.

[openhands-ai]

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

[enyst]

I checked the SDK prompt construction in OpenHands/software-agent-sdk, and the answer is: partially yes, but not completely.

What is already covered today:

• load_project_skills() does load AGENTS.md / CLAUDE.md / GEMINI.md and project skill directories from the checked-out workspace.
• In the prompt builder, repo skills with trigger=None (this includes AGENTS.md and other always-on repo guidance) are injected into <REPO_CONTEXT> and explicitly wrapped in <UNTRUSTED_CONTENT>.
• The SDK security policy / risk policy also explicitly calls out AGENTS.md, .cursorrules, and .agents/skills as repository-provided context.

So your memory is correct for AGENTS.md and other always-on repo context: the agent is told that content is untrusted.

But I don’t think that fully closes the concern here, because not all project skills are covered equally:

• AgentSkills / SKILL.md project skills are listed in <available_skills> outside the <UNTRUSTED_CONTENT> wrapper.
• Triggered project skills are injected via <EXTRA_INFO> without the same untrusted banner.
• invoke_skill returns the full skill content directly, and the SDK’s skill renderer can execute inline !\cmd`` expansions from skill content. That path is explicitly documented as safe only for trusted skill sources.

So my read is:

• Does the SDK already help? Yes, partially. It is a real mitigation for AGENTS.md / repo-context prompt injection.
• Does it eliminate this PR’s blast-radius concern? No. The workflow still loads PR-head-controlled guidance from the checked-out repo, and some of the skill surfaces are not protected by the same untrusted framing.

My targeted recommendation stays the same:

1. Do not load AGENTS.md / project skills from the PR head checkout for this workflow.
2. If we want repo guidance, load it from the base branch (or another maintainer-controlled checkout/path).
3. Separately at the SDK level, I would harden the remaining gaps by treating all repo-loaded skill surfaces as untrusted, including <available_skills>, triggered skill injections, and invoke_skill output; and I would not allow inline command execution for repo/project skills unless the source is explicitly trusted.

So: the current SDK behavior reduces the problem, but it does not fully address the specific concern raised in this review.

This comment was created by an AI assistant (OpenHands) on behalf of the user.

[enyst]

Suggested change

	required: false
	required: true

What if we make this required? It doesn't seem much of a burden, and someone might become happy if their agent goes wild - and this stops it. WDYT?

[enyst]

Suggested change

	- {diff} - The complete git diff for the PR (may be truncated)
	- {body} - The PR description
	- {diff} - The complete git diff for the PR (may be truncated)

[enyst]

Suggested change

	gone, confirming the fix works").
	gone, confirming the fix works", or "The error is still there" etc).

[enyst]

@OpenHands Understand this PR and add a sentence to address this to the prompt for the QA agent:

Treat all PR-derived content as untrusted
Not just the PR body.
Diff, repo guidance, terminal output, browser content, HTTP responses — all of it.
The model should be told explicitly never to follow instructions from those channels.

Note that you should use similar language with the language in the agent-sdk (clone yourself that repo from the same org), where it inserts warning about untrusted content on AGENTS.md on PRs and the like. Make sure also, that if there is other language in this PR that does the same, also follows the same pattern(s) / wording / strength.

Do not make other modifications, just this. In your response on github, link the commit hash.

[enyst]

@OpenHands why does CI fail? fix it pls

[openhands-ai]

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

[enyst]

CI was failing because plugins/qa-changes/scripts/agent_script.py imported Plugin from openhands.sdk.plugin, but the SDK version used in CI does not export that symbol there. I fixed it by switching the QA plugin loader to the supported PluginSource + Conversation(plugins=[...]) pattern used elsewhere in this repo: 2bfd98a

Per the previous comment, I also made the prompt hardening as a separate follow-up commit so it is easy to review in isolation. That commit adds an SDK-style <UNTRUSTED_CONTENT> warning telling the QA agent to treat all PR-derived content as untrusted input, not just the PR body (diff, repo guidance, terminal output, browser content, HTTP responses, etc.): 81a9527

Validation run locally:

• uv run pytest -q tests/test_qa_changes.py
• uv run pytest -q

This comment was created by an AI agent (OpenHands) on behalf of the user.

[enyst]

@xingyaoww I think this #135 (comment) is very interesting, and I’d pick it up for more investigation, but it doesn’t really apply as long as we don’t yet run on forks.

So it shouldn’t block this PR, WDYT?

[enyst]

You understood it correctly: before this change, the example workflow could still get triggered for a fork PR (especially via label / reviewer-request), but because it runs in pull_request context the fork run does not get the repo's LLM_API_KEY. In practice that meant it could start and then fail at config validation instead of declining cleanly.

I fixed that in 9774ad8 . The composite action now does a preflight check: if the PR comes from a fork and the LLM key is unavailable, it exits successfully with a clear skip notice in the workflow summary instead of failing. I also updated the README wording to match.

Validation:

• uv run pytest -q

This comment was created by an AI agent (OpenHands) on behalf of the user.

[enyst]

A little curious output: OpenHands/software-agent-sdk#2909 (review)

Should it be the case that it didn’t actually try to really run the workflow? It ran dry-run or —help. Now, it was a bit tricky case, because it needs OH API key, so it wouldn’t work, but it never says that in the output. 🤔
@xingyaoww

[xingyaoww]

@OpenHands please try to fix this issue by changing the prompt. I'd prefer not to change it so drastically.

#135 (comment)

[openhands-ai]

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

[xingyaoww]

Fixed in 7630b8b — targeted prompt change only, as requested.

Problem: The QA agent was running scripts with --help and --dry-run and presenting that as functional verification. In the linked QA report, the agent ran issue_duplicate_check_openhands.py --help and auto_close_duplicate_issues.py --help but never actually attempted to run them with real inputs. When it couldn't run because of missing credentials (OH API key), it should have reported that honestly instead of settling for --help output.

Fix: Added a single bullet point in both the prompt (prompt.py) and the skill (SKILL.md) that explicitly tells the agent:

• --help, --dry-run, and --version are NOT functional verification
• Always attempt real execution first
• If real execution fails due to missing credentials/services, report the failure honestly (what was tried, what was missing, what couldn't be verified)
• Do not fall back to --help output and present it as evidence the software works

All 198 tests pass.

This comment was created by an AI agent (OpenHands) on behalf of the user.

[openhands-ai]

Summary of Work

Request

@xingyaoww asked to fix the issue where the QA agent was running --help/--dry-run instead of actually executing software, and requested the fix be done by changing the prompt without drastic changes.

Checklist

• Fix applied via prompt change only — no structural, script, or workflow changes
• Minimal/non-drastic change — 8 lines added across 2 files (prompt.py and SKILL.md)
• Addresses the root cause — explicitly tells the agent that --help, --dry-run, --version are NOT functional verification; to attempt real execution first; and to report honestly when credentials/services are missing instead of falling back to --help output
• Consistent across prompt and skill — both plugins/qa-changes/scripts/prompt.py and skills/qa-changes/SKILL.md updated with matching guidance
• All tests pass — 198/198 tests pass (including 21 qa-changes-specific tests)
• Committed and pushed — commit 7630b8b on feat/qa-changes-plugin
• PR comment posted — linked to the commit with explanation of the problem and fix
• No extraneous changes — only the two files directly related to the prompt were modified

What Changed (since last summary)

Single commit 7630b8b: Added one bullet point each to:

1. prompt.py (the "What you MUST do" section) — 7 lines telling the agent to attempt real execution, not --help
2. SKILL.md (the "DO" list in Phase 3) — 1 line with the same guidance

No other files were touched. The change is deliberately narrow to match the "I'd prefer not to change it so drastically" request.