fix: address commented reviews that mention the bot, scope runs to actionable feedback - #104
Merged
Merged
Conversation
…tionable feedback Relay/poll review runs only acted on CHANGES_REQUESTED reviews, so a COMMENTED review wrapping a @bot mention (e.g. devintern#102) was silently no-oped. Changes: - address-review: fall back to the latest COMMENTED review when no changes_requested review exists, gated on a bot mention in the review body or its comments (matches the webhook trigger semantics); pass the real review state into the prompt instead of hardcoding changes_requested - address-review: scope each run to the chosen review's threads plus explicit bot mentions — stray or skipped informal comments are no longer swept into later runs, and already-addressed (hooray) comments are never repeated; log out-of-scope counts and skip reasons - relay: propagate the addressPr outcome and log addressed/not-clean instead of silently discarding it; warn with the reason when PR envelopes arrive without GitHub credentials
The address-review CLI built its client token-first, so with a human GITHUB_TOKEN set the App auth was never initialized and getBotUsername() returned null — the commented-review mention gate then failed closed and skipped every run (devintern#102). Prefer App auth like the worker's own poller client so `slug[bot]` resolves for @mention matching.
Relay-managed PRs are associated with the DevIntern AI App identity (devintern-ai), whose private key stays on DevIntern infrastructure — so a local worker can never resolve it via App auth and @devintern-ai mentions were correctly skipped by the commented-review gate. - mention-sweep: add GITHUB_BOT_ALIASES (comma-separated logins) and mentionsAnyBot/botMentionCandidates helpers; the sweep now matches the resolved login plus aliases and stays dormant only when neither exists - relay: a relay-connected worker injects devintern-ai as an alias into the worker env, so the address-review subprocess and every mention gate respond to @devintern-ai without the key - address-review: the commented-review mention gate and per-comment scoping match all candidate identities; skip messages list them - docs: document GITHUB_BOT_ALIASES under GitHub authentication
- configuration.md: the GitHub App setup now lists Reactions: Read & write — the 🎉 reaction is the addressed-marker, and an installation without it fails marking with 403 and re-processes feedback later - address-review: reaction failures matching GitHub's App-permission error now log the fix (grant Reactions, or mark manually) inline
When a review run is triggered by a mention of the relay App identity (devintern-ai) and the local credentials could not mark the comments, the worker asks the relay to react as devintern-ai (POST /v1/reactions, see private control-plane PR) so the addressed-marker exists in relay-only setups too. - address-review: return a structured result (aliasMentioned + unmarkedComments); markCommentsAddressed reports skipped replies and failed reactions; the gate records whether the mention matched the relay identity; index.ts emits the result via DEVINTERN_RESULT_FD - runAddressReviewViaCli: optional onResult captures the subprocess result through the result fd (same pattern as resolve-conflicts) - fleet: after a run with a CLI result, request relay reactions (loadRelayState + POST); failures logged, non-fatal - docs: correct the App permission list — Reactions is NOT required; review-comment reactions need Pull requests write and conversation-comment reactions need Issues write (verified empirically against an App installation); a 403 on reactions means a stale installation token or missing PR/Issues write
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
1 participant
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.
Problem
Review feedback on #102 was received by the worker but never processed. Logs showed
📌 [relay] review feedback on getdevintern/devintern#102followed immediately byprocessed 1 envelope— a fast, silent no-op.Root cause:
devintern address-reviewonly acted onCHANGES_REQUESTEDreviews. The actual feedback was aCOMMENTEDreview (empty body) wrapping an inline comment that mentioned@devintern-ai. The relay dispatched correctly, the run found nochanges_requestedreview, printed "No pending changes_requested reviews found", and exited 0.Changes
address-review.tsCOMMENTEDreview when noCHANGES_REQUESTEDreview exists — gated on a bot@-mention in the review body or its comments, matching the documented webhook trigger semantics (changes_requestedstays unconditional). Fails closed when no bot identity is resolvable.pull_request_review_id, resolved through reply chains) or explicitly mention the bot. Previously all unaddressed comments on the PR were batched, so informal comments skipped for lack of a mention could be swept into a laterchanges_requestedrun, and a new informal review could bypass the mention gate entirely whenever an olderCHANGES_REQUESTEDreview existed.N unaddressed but out of scope for this run).💬 Commented) instead of hardcodingchanges_requested.Relay / workspace worker
RelayHandlers.addressPrnow returnsPromise<boolean>; the relay logs✅ … feedback addressedor⚠️ … did not complete cleanlyinstead of silently discarding the outcome.Resulting semantics
changes_requestedreviewcommentedreview / stray inline comment@-mentionedVerification
bun run typecheck,bun run lint,bun run formatcleanbun run --filter @getdevintern/code test: 1108 pass, 0 failFixes #102