Skip to content

React with emoji for better UX on explicit review ask #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
masih merged 2 commits into from
Aug 18, 2026
+95 −4
Merged

React with emoji for better UX on explicit review ask #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d556797
React with emoji for better UX on explicit review ask
masih Aug 17, 2026
3ce196d
Harden error handling
masih Aug 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/seidroid/ai-review/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use. Two workflows live in `.github/workflows/`:

| Workflow | Trigger (in the caller) | What it does |
|----------|-------------------------|--------------|
| `ai-review.yml` | `pull_request` and PR comment events | Three-pass review (OpenAI Codex ∥ Cursor → Claude synthesizes), posting **one** PR review + an `AI Review` check run. By default it reviews automatically once; callers can enable re-review on every push, and an active allowed-team member can request another review with an exact `@seidroid review` comment. Re-reviews resolve previous seidroid inline threads whose findings were addressed or superseded by a new inline comment. |
| `ai-review.yml` | `pull_request` and PR comment events | Three-pass review (OpenAI Codex ∥ Cursor → Claude synthesizes), posting **one** PR review + an `AI Review` check run. By default it reviews automatically once; callers can enable re-review on every push, and an active allowed-team member can request another review with an exact `@seidroid review` comment. Explicit requests receive a best-effort 👀 reaction while the review runs and 👍 when it completes successfully. Re-reviews resolve previous seidroid inline threads whose findings were addressed or superseded by a new inline comment. |
| `ai-assistant.yml` | `issue_comment`, `pull_request_review_comment`, `pull_request_review` | Conversational responder: mention `@seidroid` on a PR and the bot answers in-thread. |

## Base prompts (edit these)
Expand Down
97 changes: 94 additions & 3 deletions .github/workflows/ai-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ run-name: UCI / AI Review
# never see the app token or any write scope.
# * claude_review -> consumes BOTH, does its own review, MERGES all three, returns TYPED
# output, then a github-script step posts one PR review + a check run.
# * complete_review_reaction -> clears the in-progress reaction and, when the review
# succeeds, marks an explicit request complete.
#
# Skip switch: label a PR with the `skip-review-label` input value (default `ai: skip-review`)
# and the whole pipeline is skipped -- no scouts, no Claude review, no PR review, no
Expand Down Expand Up @@ -134,9 +136,10 @@ jobs:
}}
permissions:
contents: read
pull-requests: read
pull-requests: write

@seidroid seidroid Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] The most common trigger path is issue_comment (a @seidroid review comment on the PR conversation), whose node_id is an IssueComment. Reactions on issue comments are gated by the Issues permission, not Pull requests — this repo's own ai-assistant.yml declares issues: write # reply on the PR conversation timeline for exactly that (.github/workflows/ai-assistant.yml:94) and reacts via repos/.../issues/comments/{id}/reactions.

So when app credentials are absent and github.token is used as the fallback, addReaction here (and removeReaction in complete_review_reaction, which grants the same scope at line 1059) will 403 and only emit a warning — reactions silently never appear. Note this is a two-part fix: because a reusable workflow cannot elevate beyond the caller's grant, adding issues: write to these job blocks also requires adding it to .github/workflows/ai-review-self.yml and to the caller snippet in .github/seidroid/ai-review/README.md, otherwise the run fails outright for callers that don't grant it.

outputs:
should_run: ${{ steps.resolve.outputs.should_run }}
reaction_subject_id: ${{ steps.resolve.outputs.reaction_subject_id }}
pr_number: ${{ steps.resolve.outputs.pr_number }}
head_sha: ${{ steps.resolve.outputs.head_sha }}
base_sha: ${{ steps.resolve.outputs.base_sha }}
Expand Down Expand Up @@ -299,8 +302,34 @@ jobs:
} else {
core.notice("allowed-team is empty or invalid; denying request.");
}

core.setOutput("should_run", String(authorized));
if (!authorized) core.notice(`${actor} is not authorized to request a seidroid review.`);
if (!authorized) {
core.notice(`${actor} is not authorized to request a seidroid review.`);
return;
}

const subjectId = eventName === "pull_request_review"
? context.payload.review?.node_id
: context.payload.comment?.node_id;
if (!subjectId) {
core.warning("The review request has no reactable GitHub node ID.");
return;
}

try {
await github.graphql(
`mutation($subjectId: ID!) {
addReaction(input: {subjectId: $subjectId, content: EYES}) {
reaction { content }
}
}`,
{ subjectId },
);
core.setOutput("reaction_subject_id", subjectId);
} catch (error) {
core.warning(`Could not add the in-progress reaction: ${error.message}`);
}

- name: Fetch seidroid prompt files
if: steps.resolve.outputs.should_run == 'true'
Expand Down Expand Up @@ -1019,4 +1048,66 @@ jobs:
title: `Claude + Codex + Cursor Review: ${verdict}`,
summary: summary || "No summary provided.",
},
});
});

complete_review_reaction:
name: Complete review reaction
needs: [preflight, codex_review, cursor_review, claude_review]
if: ${{ always() && needs.preflight.outputs.reaction_subject_id != '' }}

@github-actions github-actions Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[blocker] The 👍 is posted unconditionally, so a failed review is reported as a successful one. The condition only checks always() and that a subject id exists — it ignores the results of the jobs it depends on. Consequences:

  • claude_review fails (or preflight fails after resolve set the output, so no review ever ran): 👀 is cleared and 👍 added anyway, while no AI Review check run was created.
  • The run is cancelled (a push during a comment-triggered review cancels it, since cancel-in-progress is true for pull_request events): always() still runs on cancellation → 👍 for a review that never finished.

Gate the success reaction on needs.claude_review.result == 'success' and, for other outcomes, either just clear 👀 (what ai-assistant.yml:236-242 does) or add a distinct reaction such as CONFUSED/THUMBS_DOWN so a broken pipeline is visible to the requester.

@seidroid seidroid Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] always() covers failed and skipped upstream jobs, but not run cancellation: cancel-in-progress is true for pull_request events (line 105), so a push while a comment-triggered review is in flight cancels this run, and a queued job in a cancelled run does not start regardless of always(). The 👀 then stays on the comment forever, which reads as "still working" indefinitely — and pushing a fix mid-review is a common flow.

Cheapest mitigation: have the preflight reaction step also remove any pre-existing EYES reaction from the same subject before adding a fresh one, so a subsequent run self-heals the stale state.

runs-on: ${{ inputs.runs-on }}
permissions:
pull-requests: write
steps:
- name: Detect app credentials
id: creds
env:
APP_ID: ${{ secrets.PLATFORM_CODE_AGENT_APP_ID }}
run: |
if [ -n "$APP_ID" ]; then echo "present=true" >> "$GITHUB_OUTPUT"; else echo "present=false" >> "$GITHUB_OUTPUT"; fi

- name: Generate GitHub App token
id: app-token
if: steps.creds.outputs.present == 'true'
continue-on-error: true

@seidroid seidroid Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] continue-on-error: true here silently changes the acting identity: if token generation fails, github-token falls back to github.token, and GraphQL removeReaction only removes the viewer's own reaction — so the app's 👀 cannot be cleared and the 👍 is posted by a different account. Preflight's equivalent step (line 173) has no continue-on-error, so this mismatch is only reachable via a transient failure; letting the step fail hard would at least make the leftover reaction traceable to a red job rather than a warning.

uses: actions/create-github-app-token@v3
with:
app-id: ${{ secrets.PLATFORM_CODE_AGENT_APP_ID }}
private-key: ${{ secrets.PLATFORM_CODE_AGENT_APP_PK }}
owner: ${{ github.repository_owner }}
repositories: ${{ github.event.repository.name }}

- name: Mark review request complete
uses: actions/github-script@v9
env:
SUBJECT_ID: ${{ needs.preflight.outputs.reaction_subject_id }}
REVIEW_SUCCEEDED: ${{ needs.claude_review.result == 'success' }}

@seidroid seidroid Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] When claude_review fails or is cancelled, the 👀 is cleared and nothing replaces it, so a failed review is indistinguishable from "the request was never picked up" — the requester has no reason to look at the Actions tab. Consider a distinct terminal reaction on the non-success branch (e.g. CONFUSED or THUMBS_DOWN) so the outcome is always visible on the comment.

with:
github-token: ${{ steps.app-token.outputs.token || github.token }}
script: |
const subjectId = process.env.SUBJECT_ID;
if (process.env.REVIEW_SUCCEEDED === "true") {
try {
await github.graphql(
`mutation($subjectId: ID!) {
addReaction(input: {subjectId: $subjectId, content: THUMBS_UP}) {
reaction { content }
}
}`,
{ subjectId },
);
} catch (error) {
core.warning(`Could not add the completion reaction: ${error.message}`);
}
}
try {
await github.graphql(
`mutation($subjectId: ID!) {
removeReaction(input: {subjectId: $subjectId, content: EYES}) {
subject { id }
}
}`,
{ subjectId },
);
} catch (error) {
core.warning(`Could not clear the in-progress reaction: ${error.message}`);
}
Loading