Add issue duplicate checker workflow#987
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Mock-LLM Docker E2E Test Results12/12 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
|
@all-hands-bot This PR is ready for review: it adds duplicate issue checking using the reusable This comment was created by an AI agent (OpenHands) on behalf of the user. |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable — The workflows are well-structured with good concurrency controls and a clean three-mode design, but there are a few correctness and security issues worth addressing before merge.
[CRITICAL ISSUES]
- [
.github/workflows/issue-duplicate-checker.yml, Line 60] Correctness:inputs.issue_number != nulldoes not work as intended. In GitHub Actions, an unfilledworkflow_dispatchinput of typenumberevaluates to''(empty string), not JSONnull. As written,'' != nullevaluates totrue, meaning theissue-duplicate-checkjob can be triggered inissue-checkmode without a valid issue number — the action will receive an empty string and likely fail at runtime. Change toinputs.issue_number != ''(or simplyinputs.issue_number).
[IMPROVEMENT OPPORTUNITIES]
-
[
.github/workflows/issue-duplicate-checker.yml, Lines 68 & 88;.github/workflows/remove-duplicate-candidate-label.yml, Line 27] Supply Chain / Pinning: All three steps useOpenHands/extensions/plugins/issue-duplicate-checker@main. Pinning to a mutable branch ref means any breaking change (or a compromised push) tomainimmediately affects every repo running this workflow with no review gate. Pin to a specific commit SHA (e.g.@abc1234) or a tagged release and update it via a controlled PR. This is standard practice for third-party/composite Actions that touch secrets (OPENHANDS_API_KEY,OPENHANDS_BOT_GITHUB_PAT_PUBLIC). -
[
.github/workflows/issue-duplicate-checker.yml, Lines 39–40] Redundant step: Thesmoke-clonejob runsactions/checkout@v6and then immediately does a manualgit cloneto/tmp/repo-clone. The checkout step populates$GITHUB_WORKSPACEbut nothing else in the job uses it — the manual clone is the actual smoke test. Removing theactions/checkoutstep eliminates one unnecessary step and any GITHUB_TOKEN credential exposure.
[TESTING GAPS]
- [PR description] Weak Evidence: The stated verification ("parsed new workflow YAML with PyYAML") confirms the file is syntactically valid YAML, not that any workflow path executes correctly end-to-end. A smoke-clone dispatch run or a test
issue-checkrun against a real or mock issue would provide meaningful evidence. Consider runningworkflow_dispatchwithmode: smoke-cloneafter merge to confirm the reusable action resolves properly onceOpenHands/extensions#283lands.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
This PR introduces two new event-triggered workflows that use a mutable@mainaction reference and hold repo secrets (OPENHANDS_API_KEY, bot PAT). Theinputs.issue_number != nullbug meansmode: issue-checkdispatches without an issue number will silently misbehave. Risk is elevated above Low due to the pinning gap and the correctness issue on the dispatch path; no existing workflow behavior is broken.
VERDICT: ❌ Needs rework — The inputs.issue_number != null check is a real bug that should be fixed, and the mutable @main pin is a meaningful security/stability concern. Both are straightforward one-line fixes.
KEY INSIGHT: The three-mode workflow_dispatch design is clean, but the null-check for the optional issue_number input uses a comparison that never triggers as intended in GitHub Actions expression syntax.
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing. See the customization docs for the required frontmatter format.- Re-request a review — the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
Was this review helpful? React with 👍 or 👎 to give feedback.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
❌ Mock-LLM Docker E2E Test Results7/12 passed · 2 failed · 3 skipped Commit:
🔍 Failure details (2)❌ mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 3: verify automation and run on the automations page❌ mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 1: create an LLM profile pointing at the mock LLM serverPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
|
@all-hands-bot Addressed the requested changes in 6f6a94f: fixed the workflow_dispatch issue-number guard, removed the redundant smoke-clone checkout, and pinned all duplicate-checker action invocations to full extensions SHA This comment was created by an AI agent (OpenHands) on behalf of the user. |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
Review
All three issues flagged in the previous review have been correctly resolved in the current HEAD:
- ✅ Null-check corrected —
inputs.issue_number != ''correctly guards theissue-checkdispatch path; the empty-string comparison is right for how GitHub Actions evaluates unfillednumberinputs. - ✅ Redundant
actions/checkoutremoved —smoke-clonenow runs only the explicitgit clone, eliminating unnecessaryGITHUB_TOKENcredential exposure. - ✅ SHA-pinned action refs — all three action uses are now pinned to
fb020c71eb8e7cd2678f0251ab24327d3738da7dinstead of the mutable@mainref.
The overall design is clean: three well-separated jobs with precise triggering conditions, correct concurrency groups (all cancel-in-progress: false, which is right for side-effectful operations), and minimal RBAC (contents: read, issues: write). The remove-duplicate-candidate-label.yml guard conditions are thorough — correctly excluding bot comments and both OpenHands HTML comment markers (<!-- openhands-duplicate-check and <!-- openhands-duplicate-veto).
[IMPROVEMENT OPPORTUNITIES] (non-blocking)
-
Missing
timeout-minutesinsmoke-clone—git clone --depth 1should complete in seconds. Without an explicit timeout the job inherits GitHub’s 6-hour default, so a transient network hang won’t surface for hours.timeout-minutes: 5is appropriate. (See inline note.) -
Missing
timeout-minutesinremove-duplicate-candidate-label.yml— label removal is a single API call. Similarly,timeout-minutes: 10would make runaway/hung runs visible quickly. (See inline note.) -
Smoke-clone summary missing HEAD SHA —
echo "Repository HEAD: $(git -C /tmp/repo-clone rev-parse --short HEAD)"outputs to stdout but not to$GITHUB_STEP_SUMMARY. Moving that line into the heredoc block would make the run annotation include the actual commit validated.
[TESTING GAPS]
- The stated verification (“parsed new workflow YAML with PyYAML”) confirms syntactic validity only. Running
workflow_dispatchwithmode: smoke-cloneafter the dependency (OpenHands/extensions#283) lands is the meaningful smoke test. Consider adding a checklist item to block merge until that dispatch run completes successfully.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW — All previously flagged correctness and supply-chain concerns have been addressed. The pinned SHA and corrected empty-string guard eliminate the prior medium-risk issues. The remaining suggestions are defensive/cosmetic; no blocking concerns remain. - The primary operational dependency —
OpenHands/extensions#283landing so the pinned SHA is reachable frommain— is correctly documented in the PR description.
VERDICT: ✅ Worth merging — Core logic is sound. The three-mode dispatch design is clean, concurrency handling is correct, and all prior review findings have been resolved. Minor timeout and summary improvements are good-to-have but not blocking.
KEY INSIGHT: The workflow correctly separates the AI-powered duplicate-detection path (issue-duplicate-check, timeout 35 min, OPENHANDS_API_KEY) from the fast housekeeping paths (auto-close and remove-label, GitHub token only) — exactly the right separation of concerns for cost and permission scoping.
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing. See the customization docs for the required frontmatter format.- Re-request a review — the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
Was this review helpful? React with 👍 or 👎 to give feedback.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
| jobs: | ||
| smoke-clone: | ||
| if: github.event_name == 'workflow_dispatch' && inputs.mode == 'smoke-clone' | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
No timeout-minutes is set for smoke-clone. A git clone --depth 1 should complete in seconds; adding timeout-minutes: 5 here would surface any unexpected hang quickly instead of waiting for GitHub’s 6-hour default.
runs-on: ubuntu-latest
timeout-minutes: 5| github.event.comment.user.type != 'Bot' && | ||
| !startsWith(github.event.comment.body || '', '<!-- openhands-duplicate-check') && | ||
| !startsWith(github.event.comment.body || '', '<!-- openhands-duplicate-veto') | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
No timeout-minutes is set for this job. Label removal is a single GitHub API call; adding timeout-minutes: 10 would make any runaway/hung execution visible quickly.
runs-on: ubuntu-latest
timeout-minutes: 10Co-authored-by: openhands <openhands@all-hands.dev>
✅ Mock-LLM Docker E2E Test Results12/12 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
📸 Snapshot Test Report✅ All snapshots match the main branch baselines.
✅ Unchanged snapshots (73)
Generated by the Snapshot Tests workflow. This comment was created by an AI agent (OpenHands) on behalf of the repo maintainers. |
|
@all-hands-bot Addressed the remaining cleanup in 93f9335: added explicit timeouts, included the smoke-clone HEAD in the step summary, and removed the unused This comment was created by an AI agent (OpenHands) on behalf of the user. |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
Review
This is a re-review of the PR at the latest HEAD (93f9335). All issues raised in previous reviews have been addressed, and the final polish commit fully closes the remaining non-blocking gaps.
What changed in the latest commit (93f9335 — "chore: polish duplicate checker workflows")
| Item | Status |
|---|---|
timeout-minutes: 5 added to smoke-clone |
✅ Done |
REPO_HEAD captured into $GITHUB_ENV and printed in $GITHUB_STEP_SUMMARY |
✅ Done |
timeout-minutes: 10 added to remove-duplicate-candidate-label.yml |
✅ Done |
Spurious repository: input removed from remove-label mode |
✅ Done |
Current state of each issue previously raised
| Issue | Resolution |
|---|---|
inputs.issue_number != null bug |
✅ Fixed — now != '' |
Redundant actions/checkout in smoke-clone |
✅ Removed |
Mutable @main action pins |
✅ All three uses pinned to fb020c71 |
Missing timeout-minutes in smoke-clone |
✅ Set to 5 |
Missing timeout-minutes in remove-duplicate-candidate-label.yml |
✅ Set to 10 |
| HEAD SHA not surfaced in step summary | ✅ Now written to $GITHUB_STEP_SUMMARY via REPO_HEAD env var |
Design observations (informational, non-blocking)
Workflow-level permissions apply to all jobs. issue-duplicate-checker.yml sets contents: read, issues: write at the workflow level. The smoke-clone job (which only does git clone --depth 1 via HTTPS) doesn't need contents: read from the token and certainly not issues: write. Per-job permissions: overrides would be the strictest posture, but this is cosmetic for a workflow_dispatch-only job.
close_after_days default of 3 is intentional and reasonable. Worth noting for future operators: teams sometimes prefer 7 days for newly-opened issues, but this can be overridden at dispatch time.
Dependency gating. The PR correctly documents that OpenHands/extensions#283 must land before the pinned SHA fb020c71eb8e7cd2678f0251ab24327d3738da7d is reachable on main. Merging this PR before that dependency lands will cause all three uses: steps to fail at runtime. Consider adding a merge blocker (label or branch protection rule) until the dependency is confirmed merged.
Risk Assessment
🟢 LOW — All critical and improvement items have been resolved. The three-mode dispatch design is clean, concurrency handling is correct (cancel-in-progress: false on all side-effectful jobs), permissions are scoped appropriately (contents: read, issues: write), and all action refs are SHA-pinned. The only remaining operational prerequisite is the upstream OpenHands/extensions#283 dependency.
VERDICT: ✅ Approved — Ready to merge once the OpenHands/extensions#283 dependency lands and the smoke-clone dispatch has been validated post-merge.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
Summary
OpenHands/extensions/plugins/issue-duplicate-checker@mainactionDepends on OpenHands/extensions#283 landing first so the shared action exists on
main.Verification
This PR was created by an AI agent (OpenHands) on behalf of the user.
@enyst can click here to continue refining the PR
🐳 Docker images for this PR
• GHCR package: https://github.com/OpenHands/agent-canvas/pkgs/container/agent-canvas
ghcr.io/openhands/agent-canvasghcr.io/openhands/agent-server:1.24.0-pythonopenhands-automation==1.0.0a593f9335a617945dcc2c73771ee273d3b76b1be35Pull (multi-arch manifest)
# Multi-arch manifest — Docker automatically pulls the correct architecture docker pull ghcr.io/openhands/agent-canvas:sha-93f9335Run
All tags pushed for this build
About Multi-Architecture Support
sha-93f9335) is a multi-arch manifest supporting both amd64 and arm64sha-93f9335-amd64) are also available if needed