feat(agricola): poll PR comments for fix commands - #132
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7895636764
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if comment.has_eyes: | ||
| continue |
There was a problem hiding this comment.
When another GitHub user reacts with eyes to a maintainer's /ag fix comment before the scheduled poll, pull_request_comments() turns the aggregate reactions.eyes count into has_eyes, and this branch skips the command without checking who reacted. An unauthorized user—or an informal eyes reaction from a reviewer—can therefore permanently suppress a valid command; verify that the Agricola App created the reaction or track processed comment IDs in trusted state instead of treating any eyes reaction as acknowledgement.
Useful? React with 👍 / 👎.
| agricola deliver-pr-actions "$RUNNER_TEMP/agricola-result.json" \ | ||
| --action-url "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" |
There was a problem hiding this comment.
When a scheduled poll overlaps a /ag fix webhook on the corresponding tracking issue, this step acknowledges the PR-local command while both workflows continue concurrently: scheduled runs use the agricola-control-plane group, whereas issue comments use agricola-fix-<issue>. Both revisions can pin the same PR head, and publish() rejects the later request after the branch advances because its distinct idempotency key is not present; since the PR command already has eyes, polling will not retry it. Use a shared per-PR/tracking-issue lock for both entry points or defer consumption until publication succeeds.
Useful? React with 👍 / 👎.
| for acknowledgement in acknowledgements: | ||
| client.react_to_issue_comment( | ||
| acknowledgement.repository, | ||
| acknowledgement.comment_id, | ||
| acknowledgement.content, | ||
| ) |
There was a problem hiding this comment.
If a later reaction or queued-reply API call fails after this loop has already reacted to one or more comments, deliver-pr-actions exits nonzero, so the run job fails and the generation jobs never start. The next scheduled poll skips the comments whose eyes reactions succeeded, permanently dropping their instructions; this can also drop every command when acknowledgement succeeds but posting the run-link reply fails. Persist processing state or make acknowledgement/reply delivery unable to abort generation, and only mark comments handled once their requests remain retryable.
Useful? React with 👍 / 👎.
| acknowledgements = revision_acknowledgements( | ||
| client, manifest, result.propagations | ||
| ) |
There was a problem hiding this comment.
If a maintainer posts /ag fix add tests on the downstream PR and, before the next poll, someone queues a revision from the tracking issue with a different instruction, this call acknowledges the unpolled PR comment even though its instruction was never added to result.propagations. The generator treats only the request's instruction as authorized and explicitly treats PR comments in .agricola/revision.md as untrusted, so add tests can be ignored while the eyes reaction prevents later polling. Only acknowledge PR commands whose instructions were incorporated into the queued request, or leave them for the poller.
Useful? React with 👍 / 👎.
1 participant
Motivation
Allow maintainers to request incremental Agricola revisions directly from recorded downstream pull requests without hosting an additional webhook service.
Summary
/ag fixcommentsKey design considerations