Skip to content

ci: request @clerk/team-mobile through the team review app - #1221

Merged
mikepitre merged 4 commits into
mainfrom
mike/team-review-app
Nov 3, 2026
Merged

mikepitre merged 4 commits into
mainfrom
mike/team-review-app

Conversation

@mikepitre

@mikepitre mikepitre commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

What & why

  • Team review request asked each person in .github/team-mobile because the Actions token can't see org teams. Dominic created a GitHub App for this and set TEAM_REVIEW_APP_CLIENT_ID and TEAM_REVIEW_APP_PRIVATE_KEY on the repo. With it, the workflow requests @clerk/team-mobile itself, and nobody has to keep a list in step with the team.
  • The workflow mints a token from the app with actions/create-github-app-token, scoped to members read and pull requests write. The script reads the members of @clerk/team-mobile with that token and makes the review request with it. Everything else, including the protected-path comment, still uses the Actions token, so existing comments keep the same author and can still be edited.
  • .github/team-mobile is deleted.

What to focus on

  • It requests the team instead of each member, and skips the request when the team is already requested, or when a member is already requested or has reviewed. Before, it asked the members who hadn't been asked yet. The skip keeps a push after a member's review from pinging the whole team again.

Testing

  • node --test .github/scripts/*.test.mjs: 270 tests pass. The fake GitHub in team-review-request.test.mjs checks that the app token reads the team and makes the request, and the cases cover the team request, the skip rules and protected paths.
  • actionlint passes on the workflow.
  • The workflow runs from main, so this PR can't run the new path. The first live run is the next PR that needs a Team Mobile review after this merges.

Risk

  • This changes paths in .github/protected-paths.
  • If the app isn't installed on this repo, or the secret is wrong, the token step fails, and PRs that need a Team Mobile review get no request until it's fixed. Merging isn't blocked: Code owner approval doesn't depend on this workflow.

How to revert

  • A plain revert restores the list and the per-person requests. The app and its secret can stay; nothing else uses them.

🤖 Generated with Claude Code

The Actions token can't see org teams, so Team review request asked each
person in .github/team-mobile instead. Dominic created a GitHub App with
org Members: Read and set TEAM_REVIEW_APP_CLIENT_ID and
TEAM_REVIEW_APP_PRIVATE_KEY on the repo. The workflow now mints a token
from that app, reads the members of @clerk/team-mobile, and requests the
team. The hand-kept list is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

This pull request changes files listed in .github/protected-paths, so it needs an approval from another Team Mobile member before it can merge. CodeRabbit's approval isn't enough for these files.

  • .github/scripts/code-owner-approval.mjs
  • .github/scripts/github.mjs
  • .github/scripts/team-review-request.mjs
  • .github/scripts/team-review-request.test.mjs
  • .github/workflows/team-review-request.yml

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Before this change, the workflow used a checked-in list of Team Mobile logins to request individual reviews. It now reads team membership and requests @clerk/team-mobile with a GitHub App token; it skips that request if the team is already requested or an eligible member is requested or has reviewed. For protected files, it keeps the notice and can re-request eligible members whose approval is for an older commit. Review the decision logic in team-review-request.mjs first, then check how the workflow supplies the Actions and team review tokens.

Walkthrough

The workflow now uses an app token to retrieve Team Mobile membership and submit team review requests. The script filters members through trusted and applies rules for team requests and stale approvals.

Changes

Team review request flow

Layer / File(s) Summary
Authenticated team lookup and workflow setup
.github/scripts/github.mjs, .github/scripts/code-owner-approval.mjs, .github/scripts/team-review-request.mjs, .github/workflows/team-review-request.yml, .github/team-mobile
The workflow mints and passes TEAM_REVIEW_TOKEN. The script uses the token to fetch Team Mobile members and filters them through trusted. The local member list is removed, and client supports API paths without a repository prefix.
Team request and stale-approval decisions
.github/scripts/team-review-request.mjs, .github/scripts/team-review-request.test.mjs
The script requests the team when no existing team request or qualifying member request or review prevents it. For qualifying protected-file requests, it requests eligible members individually when their approvals are stale. Tests cover team requests, stale approvals, member roles, and pull request states.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant TeamReviewWorkflow
  participant GitHubApp
  participant TeamReviewScript as team-review-request.mjs
  participant GitHubAPI
  TeamReviewWorkflow->>GitHubApp: Mint token with members-read and pull-requests-write permissions
  TeamReviewWorkflow->>TeamReviewScript: Pass TEAM_REVIEW_TOKEN
  TeamReviewScript->>GitHubAPI: Fetch team-mobile members
  TeamReviewScript->>GitHubAPI: Submit team review request
Loading

Possibly related PRs

Suggested reviewers: swolfand

Merge Risk: Moderate · previous review cea0e

The review-request automation can fail to ask an eligible Team Mobile member for review in some follow-up cases, which leaves protected-file PRs without a reviewer who can satisfy the approval check. This does not affect runtime behavior or block merging, but resolve it before relying on the workflow.

Pre-merge checks: Passed 6 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 6 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: requesting @clerk/team-mobile through the team review app.
Description check Passed The description covers the reason for the change, review focus, testing, risks, and revert plan. The Screenshots / video section is empty, but it is not critical for this workflow-only change.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Testing, Risk, And Revert Passed The PR description includes all three required sections with PR-specific content. Testing names the Node test command, 270 passing tests, fake GitHub coverage, and actionlint. Risk describes the app i…
Device Evidence Passed The reviewed diff changes only .github scripts, workflow files, and .github/team-mobile. It changes no file under source/api/, source/ui/, or e2e/, so device evidence is not required.
Full details: Testing, Risk, And Revert

Explanation

The PR description includes all three required sections with PR-specific content. Testing names the Node test command, 270 passing tests, fake GitHub coverage, and actionlint. Risk describes the app installation or secret failure and confirms merging is not blocked. How to revert states that a plain revert restores the previous list and per-person requests.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 4 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Automatic approval policy

Commit: cea0e6dc0226.

Setting Effective value Source
Automatic approvals Enabled Global configuration
Merge risk Moderate Global configuration
Security risk Not set Global configuration

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration
Commits

Reviewing files that changed from the base of the PR and between bfc44eb and cea0e6d.

Files selected for processing: 2
  • .github/scripts/team-review-request.mjs
  • .github/scripts/team-review-request.test.mjs

Limit details: You’ve used all 6 included reviews currently available. Your 40 included PR review attempts over the past 7 days set your current allowance at 6 reviews per hour.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pre-merge checks failed. Please resolve the failing checks before merging.

@clerk/team-mobile grants write, so a member without maintain or admin
could review and stop the team request while their approval would not
count. Filter the roster with the same role check Code owner approval uses.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai Bot previously approved these changes Nov 3, 2026
The Actions token can't see org teams, so pull.requested_teams may leave
out a pending @clerk/team-mobile request and a later run would ask the
team again. Read the pending reviewers with the app token.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread .github/scripts/team-review-request.mjs Resolved
@mikepitre
mikepitre disabled auto-merge November 3, 2026 12:56
@mikepitre
mikepitre merged commit dd3dd77 into main Nov 3, 2026
18 of 20 checks passed
@mikepitre
mikepitre deleted the mike/team-review-app branch November 3, 2026 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant