https://docs.all-hands.dev/ logo
Join Slack
Powered by
<@U0BME18CF7S> please review <https://github.com/O...
# automations-debug
c
@fresh-machine-37269 please review https://github.com/OpenHands/software-agent-sdk/pull/5032 and if you think it’s okay feel free to approve it, or request changes if you see fit. it’s needed for another pr.
👀 1
f
🤖 On it! View progress here: oss-agent-canvas.ngrok.dev/conversations/…
✅ Done! Reviewed and approved 👍 The compatibility-check change is tightly scoped and fail-closed: it only accepts the 
lte: 100
→ 
maximum: 100
repair on exactly four search endpoints (GET-only, matching 
operationId
against the baseline, with an integer 
limit
param that has legacy 
lte: 100
and no prior 
maximum
). Everything else still fails the check, and the tests cover all the negative cases and mixed-breakage scenarios well. One small non-blocking note I left on the PR: the 
text
matcher pins the exact oasdiff 
100.00
wording, so it's tied to oasdiff 1.19.1 phrasing — worth a glance whenever oasdiff gets bumped, though the check degrades safely (fail-closed).
c
@fresh-machine-37269 out of curiosity, this comment is made by what workflow and what are its on: events? it seems maybe it should have “sync” or whats its name, when a pr is updated https://github.com/OpenHands/software-agent-sdk/pull/4991#issuecomment-5665539616
👀 1
f
✅ Done! That comment isn't from a GitHub Actions workflow at all — it's from an OpenHands automation (the thing that posts as 
all-hands-bot
), specifically "GitHub PR Reviewer (multi-repo): extensions, OpenHands, software-agent-sdk, automation, enterprise". Its trigger config (in 
automation.yaml
in the 
oss-agent-canvas
repo) is:
Copy code
trigger:
  source: github-events
  type: event
  on: '*'
  destination: dispatch_run
  filter: |
    contains(['OpenHands/extensions', 'OpenHands/OpenHands',
              'OpenHands/software-agent-sdk', 'OpenHands/automation'],
             repository.full_name)
    && contains(keys(@), 'pull_request')
    && !contains(keys(@), 'comment')
    && !contains(keys(@), 'review')
    && contains(['opened', 'reopened', 'synchronize',
                 'ready_for_review', 'review_requested'], action)
So the "sync when a PR is updated" event you're thinking of — 
synchronize
— is already in the list. A push to the PR branch fires 
synchronize
, which re-triggers the run and re-sweeps the repo; the comment itself says "this is re-checked automatically once you push a new commit". The actions it listens for are 
opened
, 
reopened
, 
synchronize
, 
ready_for_review
, and 
review_requested
(with comments and reviews explicitly filtered out so it doesn't react to its own posts).
c
@fresh-machine-37269 ah, thanks. hmm but it didn’t work on 5032, there was such comment until I deleted it manually, but CI was green
👀 1
f
✅ Done! Good catch — but it actually did work correctly. There was a genuinely red check, it just wasn't a test. Looking at the check-runs for the PR head commit, the failing one was:
Copy code
'Validate PR description'   status=completed   conclusion='failure'
while all the real tests (
agent-server-tests
, 
sdk-tests
, 
cross-tests
, 
build-binary-and-test
, etc.) were 
success
. So at a glance the PR looked green, but that one check was red. The automation's CI gate (
_ci_status
in 
oss-agent-canvas
→ 
main.py
) flags a commit as failed if any check-run has 
conclusion in ("failure", "timed_out", "action_required")
. 
Validate PR description
returned 
failure
, so it correctly posted the "CI is currently failing" notice. Why that check failed: your PR body's HUMAN section still had the unreplaced placeholder comment —
Copy code
<!--
Human author: please replace this comment with a short note (at least 20 visible
characters) before marking ready for review.
AI agents: you must not edit this section.
-->
That's exactly what the 
pr-description-check
/ 
check_pr_description.py
workflow validates (it came from 
.github/workflows/pr-description-check.yml
). The timeline lines up: • 09-13 20:41 — head commit pushed, 
Validate PR description
→ 
failure
• 09-14 14:25 — bot posts the CI-failing comment (referencing head 
8a9e0bb
) • 09-14 14:26 — 
Validate PR description
→ 
success
(right after the description was corrected) So the two things worth flagging: 1. It wasn't a false positive — the description check was red while the tests were green. 2. The automation never deletes the CI-failing comment once CI turns green — it only posts, or edits in place on a new failing commit. So that comment lingering until you removed it is expected behavior, not a bug in the comment staying.
c
@fresh-machine-37269 but then it’s missing the event that means “someone edited the pr description”
👀 1
f
✅ Done! Right — that's exactly the gap. The trigger filter only listens for these 
pull_request
actions:
Copy code
opened, reopened, synchronize, ready_for_review, review_requested
But editing the PR title/body fires a 
pull_request
webhook with 
action: "edited"
, which isn't in that list. So a description-only edit (which is precisely what flipped 
Validate PR description
from 
failure
→ 
success
) generated no event the automation cares about, and it never got the chance to re-evaluate. Two separate things compound here: 1. Missing 
edited
action → description edits don't re-trigger the run. Adding 
edited
to the filter would fix the re-trigger (though you might then also want to gate which sub-fields changed, since 
edited
fires on both title and body changes). 2. No deletion/cleanup path → even if it had re-checked and seen CI as green, the bot has no code that removes or clears its own "CI is currently failing" comment. It only posts or edits-in-place on a new failing commit. So the comment would have lingered regardless until a human deleted it. So it's not really "the bot misjudged CI" — it's that the re-evaluation trigger surface doesn't cover 
edited
, and there's no stale-comment remediation either.
c
@fresh-machine-37269 edited fires on what body changes exactly? e.g. does it fire on a comment change?
👀 1
f
✅ Done! 
edited
on a 
pull_request
webhook fires for exactly three things: 1. Title change → payload has 
changes.title
2. Body change → payload has 
changes.body
(this is the PR description, not comments) 3. Base branch change → payload has 
changes.base
That's it. It does not fire for labels, assignees, reviewers, milestones, draft-status toggles (those are `ready_for_review`/`converted_to_draft`), or attachments. A comment is completely separate — it isn't part of the PR object at all. Comments arrive on their own webhook events: • 
issue_comment
(top-level PR/issue comments) — where editing a comment fires 
edited
• 
pull_request_review_comment
(inline diff comments) • 
pull_request_review
(reviews) So a comment edit fires 
edited
on 
issue_comment
, not on 
pull_request
. And since this automation's filter already has 
!contains(keys(@), 'comment')
, it would explicitly reject those anyway. In short: for your PR, the description (body) edit was the relevant 
pull_request
edited
event the trigger is currently missing — which is why the re-check never happened.
c
@fresh-machine-37269 is this automation available in openhands/extensions repo too? or wait, did you say it’s the pr-review automation, not a different one?
👀 1
f
✅ Done! Both, actually — it's the same one. This is the PR-review automation, and it's a single multi-repo automation named:
"GitHub PR Reviewer (multi-repo): extensions, OpenHands, software-agent-sdk, automation, enterprise"
One automation handles all those repos via a single webhook filter, not five separate ones. The filter is:
Copy code
contains(['OpenHands/extensions', 'OpenHands/OpenHands',
          'OpenHands/software-agent-sdk', 'OpenHands/automation'],
         repository.full_name) && ...
(The "enterprise" in the title is the fifth repo but isn't in this particular filter's list — private, and not wired into the public webhook the same way.) So yes, 
OpenHands/extensions
is covered by the exact same automation, and the same rules apply there (CI gate, description check, trusted-contributor review, Slack ping for untrusted). And the same two gaps I flagged — no 
edited
action in the filter, no stale-comment cleanup — apply across all of those repos, including extensions.
c
@fresh-machine-37269 so if the filter passes (that is, if CI is green), is there another event that the actual run of the pr-review script depends on, or does it just start executing?
👀 1
f
✅ Done! No second event — once the filter passes, the run starts executing immediately. The key distinction is that the filter is not the CI gate. The filter is just the entry condition: it checks things like 
repository.full_name
matches and the action is one of the allowed values. It does not look at CI at all. CI status is decided inside the script, per PR, at review time (
_ci_status(...)
), not at the webhook/trigger layer. So the flow is: 1. Webhook arrives → filter decides whether to launch a run (no CI involved). 2. If it launches, 
main()
→ 
_drive_repo()
sweeps the repo and, for each PR, reads CI via the GitHub API. 3. It then re-sweeps on its own within a single run via that in-process wait loop (
_drive_repo
, 
PR_INLINE_WAIT_BUDGET = 1200s
, checking every 
PR_INLINE_POLL_INTERVAL
). This is the stand-in for a cron: if CI is still `running`/`unknown`, it flags 
_CI_PENDING
and loops again, up to the budget. So "CI is green" doesn't gate the run itself — it gates what the run does with each PR (post CI-failing → skip; green → review or Slack-ping). The script doesn't wait for another event or a 
check_suite.completed
signal; it polls GitHub's check-runs directly and only surrenders when the run's time budget runs out, deferring the rest to the next 
pull_request
delivery.
c
@bland-bear-604 remind me of this thread tomorrow morning
4 Views
Open in Slack
PreviousNext
Powered by