Repository navigation
Dispatch deferred commands directly instead of reposting the comment body - #189
Merged
Merged
Conversation
HadrienPatte
marked this pull request as ready for review
September 16, 2026 13:56
HadrienPatte
requested review from
sekhar-isovalent
and removed request for
a team
September 16, 2026 13:56
sekhar-isovalent
requested review from
nebril
and removed request for
sekhar-isovalent
September 16, 2026 13:58
Member
|
Sorry for long wait on that, I will reserve time next week to review and merge this one. |
HadrienPatte
force-pushed
the
pr/HadrienPatte/dispatch
branch
from
October 13, 2026 09:45
049a7d2 to
296faa5
Compare
nebril
approved these changes
Oct 20, 2026
nebril
left a comment
nebril
left a comment
Member
There was a problem hiding this comment.
Great work, LGTM overall!
We recently merged feedback.reactions option , which will mess with this code, I will think about how to integrate it best.
I think feedback.reactions shouldn't touch the "+1" comment reactions. Will add this to this PR.
When a trigger's dependencies completed, processDependantWorkflows re-posted the user's original command as a new PR comment. That comment existed only to re-enter the issue_comment handler, which parsed it and dispatched the workflows. Every input that path needs is already in scope at the point the comment was posted: the command body gives the trigger and its extra arguments, and the pull request gives the refs. Call processWorkflowsForTrigger directly instead. This removes the duplicate command comment users see on every deferred trigger, halves the round trip, and stops the dispatch from being re-authorized as the bot that posted the comment. Dependencies end up checked twice, once by the caller to decide whether to dispatch at all and once by processWorkflowsForTrigger immediately before dispatching. The second check is the authoritative one and also covers the window between the two. Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
GithubCommenter could only create reactions. Managing a reaction as durable state needs to find an existing one and to remove it once it has been acted on. Add findReaction, which filters an issue comment's reactions by emoji server-side and returns the ID of the one left by a given login, and removeReaction, which deletes it. findReaction distinguishes an absent reaction (0, nil) from a failed lookup so callers do not treat an API error as "no marker present". Identifying our own reaction cannot reuse the "organization name prefix plus [bot] suffix" convention the handlers apply to comment and PR authors: that convention deliberately admits any bot of the organization, Renovate included, whereas a pending-trigger marker is only meaningful if Ariane left it. The login is a property of the app and never changes while the process runs, so it is resolved once at startup and handed to the handlers that need it, in the same way the run delay is. Both helpers are unused until dependent triggers start being gated on a pending reaction marker. Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
HadrienPatte
force-pushed
the
pr/HadrienPatte/dispatch
branch
from
October 22, 2026 20:15
296faa5 to
9deb008
Compare
processDependantWorkflows dispatched any command it found in the last three hours of PR comments matching a trigger. The time window was the only thing bounding replay, and it knew nothing about whether the command was actually still awaited. In particular a command an unauthorized user posted, which the issue_comment handler declined and left on the PR, was dispatched all the same once a dependency turned green. Use the thumbs up the issue_comment handler already adds when it defers a command as the record that the command is pending. The workflow_run handler now requires that marker before dispatching, removes it once the dispatch succeeds, and reacts with a rocket in its place. A failed dispatch leaves the marker so the command is retried on the next dependency completion instead of being dropped. Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
A command deferred while its dependencies were running carries a marker until those dependencies complete. The dispatcher checks dependencies against the pull request's current head, so once new commits are pushed the command would be dispatched against code it was never requested for. Remove the marker from pending commands when a synchronize event arrives. The command has to be requested again to apply to the new head. Only comments that are a command on their own are considered, matching what the dispatcher will act on, so no reaction lookup is spent on comments that merely mention one. Nor is one spent on a command the listing already reports as carrying no thumbs up; an absent reaction summary proves nothing, so there the lookup still decides. Clearing runs before the allowed team check, because a push invalidates pending commands regardless of who pushed it, and a failure to clear does not prevent the default testsuite from running. Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
HadrienPatte
force-pushed
the
pr/HadrienPatte/dispatch
branch
from
October 22, 2026 20:40
9deb008 to
60d9461
Compare
Signed-off-by: Maciej Kwiek <mkwiek@cisco.com>
Signed-off-by: Maciej Kwiek <mkwiek@cisco.com>
nebril
approved these changes
Oct 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #181
Followup to #182
When a command's
depends_ontriggers were not yet satisfied, Ariane deferred it and later re-posted the full original comment body on the PR so that its ownissue_commenthandler would pick it up again. That is what makes Ariane look like it is parroting contributors back at them, and it is also an unbounded replay: the only thing gating the repost was a 3 hour comment window, with no record of whether the command was still awaited.This PR removes the repost entirely:
processDependantWorkflowsnow resolves the pending command from the PR comments and callsprocessWorkflowsForTriggeritself, so nothing is written back to the PR. Comment listing is also skipped unless the completed run is a dependency of at least one trigger.issue_commenthandler already adds when it defers a command becomes the record that the command is pending. Theworkflow_runhandler requires that marker before dispatching, then swaps it for a rocket. A failed dispatch leaves the marker in place so the command is retried on the next dependency completion. This also fixes commands from unauthorized users which previously declined but stayed on the PR, being dispatched once a dependency turned green.synchronizeevent clears pending markers, since dependencies are checked against the PR's current head and a deferred command was never requested for the new commits. The command has to be requested again.