Skip to content

AssignReviewers: skip people who are out, from the Rippling PTO calendar - #41

Merged
willson556 merged 2 commits into
mainfrom
pto-calendar
Oct 2, 2026
Merged

willson556 merged 2 commits into
mainfrom
pto-calendar

Conversation

@willson556

@willson556 willson556 commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

AssignReviewers now skips anyone the Rippling PTO calendar has out today or on the next business day, instead of anyone whose GitHub status is Busy. The team doesn't set GitHub status, so the old check never skipped anyone. This is part of Proposal 1 of the review throughput RFC, and it's the out-of-office source Proposal 2's SLA job will use (rollout plan).

Calendar entry Counts as out
<name> is Out of Office Yes
<name> on <leave type>, such as Vacation or Sick Leave Yes
<name> on Work From Home No

"Next business day" skips weekends and the company paid holidays in ReviewConfig/holidays.json, so a review assigned the Wednesday before Thanksgiving isn't given to someone out the following Monday.

New shared folder, ReviewConfig/:

  • pto.js reads the iCalendar feed: all-day and timed entries, folded lines, escaped text. It matches names to GitHub logins through a people map, ignoring case and extra spaces.
  • holidays.json holds 2026's 11 company paid holidays. Next year's list is added by hand; a year with no list is a warning on every run.

This repo is public, so the people map isn't here. It's the org variable REVIEW_PEOPLE: JSON mapping each login to a name as Rippling shows it, optional aliases (a legal name other systems use, say), and a slack member ID for Proposal 2's reminders. Fixtures and examples use made-up names.

New inputs, both optional:

pto-calendar-url: ${{ secrets.PTO_CALENDAR_URL }}
people: ${{ vars.REVIEW_PEOPLE }}

Without them nobody is skipped, so existing callers keep working until they add the two lines. This repo's own caller, added in the PR below this one, is the first to pass them. Until this merges, that caller still uses AssignReviewers@main, so its runs log an "Unexpected input" warning and skip nobody. If the feed can't be fetched, or the map is missing or isn't valid JSON, the action warns and assigns as before. It never fails the run. The feed URL carries a token, so neither it nor an error that might quote it is logged.

Testing:

  • node --test AssignReviewers/ ReviewConfig/: 34 cases. They cover out today, out next business day, Work From Home, time off further out, weekends and holidays, timed entries across the UTC date line, aliases, a failed fetch that mustn't log the URL, and a missing or broken people map. The test workflow now also runs on changes to ReviewConfig/.
  • Not yet run against the real feed. Once this merges, the five caller workflows (StaflLib, coit-tower-bms2000, goldengate-bms2000-string, stafl-bms2000-template, anza-bms2000) each get a PR adding the two inputs.

🤖 Generated with Claude Code

@willson556
willson556 changed the base branch from main to graphite-base/41 October 1, 2026 22:06
@willson556
willson556 changed the base branch from graphite-base/41 to assign-reviewers-workflow October 1, 2026 22:07
@willson556
willson556 marked this pull request as draft October 1, 2026 22:13
@willson556
willson556 marked this pull request as ready for review October 1, 2026 22:13
@staflsystemsci
staflsystemsci Bot requested review from a team, fcheein, gretl-silva-stafl, mament0, mns4595, mzhu48, nehalkpatel and thebhef and removed request for a team October 1, 2026 22:13
@willson556
willson556 changed the base branch from assign-reviewers-workflow to graphite-base/41 October 1, 2026 22:23
Replace the GitHub Busy status check, which the team doesn't use, with the
Rippling PTO calendar feed. A rotation reviewer or domain approver who is out
today or on the next business day isn't picked; Work From Home doesn't count
as out.

Adds ReviewConfig/, shared with the review SLA job to come:
- pto.js reads the iCalendar feed and matches entries to GitHub logins
  through the REVIEW_PEOPLE org variable (name, optional aliases, Slack ID),
  kept out of this public repo.
- holidays.json lists 2026's company paid holidays, used to find the next
  business day. Next year's list is added by hand.

New inputs: pto-calendar-url (the PTO_CALENDAR_URL org secret) and people
(the REVIEW_PEOPLE org variable). Without them nobody is skipped, so existing
callers keep working. A feed or map that can't be read is a warning, never a
failure, and the URL is never logged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@graphite-app
graphite-app Bot changed the base branch from graphite-base/41 to main October 1, 2026 22:24
This repo's own caller is the first to use the new inputs. Until this merges,
its runs use AssignReviewers@main, which doesn't know them yet, so they log an
"Unexpected input" warning and skip nobody.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread ReviewConfig/README.md
@staflsystemsci

staflsystemsci Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review SLA

Assignee Owes Due
fcheein First response Fri Oct 2, 15:24 PT Overdue

Due times are in business hours, 10:00 to 17:00 Pacific: 4 h for a PR under 250 added lines, 7 h otherwise, and 4 h for a re-review. Mid-stack, an assignee owes a response on the lowest PR they haven't approved. Dashboard

@staflsystemsci staflsystemsci Bot assigned thebhef and unassigned nehalkpatel Oct 2, 2026
@staflsystemsci

This comment has been minimized.

@willson556 willson556 assigned nehalkpatel and unassigned thebhef Oct 2, 2026

willson556 commented Oct 2, 2026 •

Copy link
Copy Markdown
Member Author

Merge activity

  • Oct 2, 10:56 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Oct 2, 10:56 PM UTC: @willson556 merged this pull request with Graphite.

@willson556
willson556 merged commit b47e807 into main Oct 2, 2026
2 checks passed
willson556 added a commit that referenced this pull request Oct 2, 2026
…rd (#44)

Step 2 of Proposal 2's rollout ([rollout plan](https://claude.ai/code/artifact/12932aa2-661e-44c3-a59e-d17a639f6b95)): the ReviewSLA action. Merging it changes nothing by itself.

The scheduled workflow that runs it lives in a private repo, not this one, because this repo's run logs are public and the job's would show who is out of office and activity in the private repos. The README has the workflow. It's skipped until that repo's `REVIEW_SLA_MODE` variable is set, and setting it back to `off` is the rollback.

Every 30 minutes during business hours (10:00 to 17:00 Pacific, business days), it reads the open PRs in the five pilot repos and this one. From their timelines it rebuilds each assignee's clock (`ReviewSLA/owed.js`):
- **First response:** 4 business hours under 250 added lines, 7 otherwise. It starts at the latest of: the PR marked ready, the person assigned, and, mid-stack, their approval of the PR below or that PR merging.
- **Stacks:** an assignee owes only on the lowest ready PR they're assigned to and haven't approved. Requesting changes doesn't move them up.
- **Re-review:** 4 business hours from a re-request, but only if the author has pushed since that reviewer's last review. Graphite's re-requests on `gt submit` start nothing.
- **Drafts** stop every clock on the PR, and marking it ready again starts them over.

What each mode does:

| Mode | Effect |
| --- | --- |
| `shadow` | A Review SLA comment on each PR where someone owes a review (who owes what, due when), the Confluence dashboard and the 10:00 Slack digest |
| `remind` | Also a Slack DM when a review is due, and reassignment of anyone the PTO calendar has out before their review is due |
| `enforce` | Reassignment at twice the target is step 3; for now it warns and runs as `remind` |

Details:
- **Reminders are sent once.** The comment's hidden marker records each reminder. The comment is written before the DM goes out, so a failure means a missed reminder, not a repeated one. Runs don't overlap.
- **Out-of-office reassignment** replaces only the person who's out, on every PR in the stack they're assigned to. A domain approver is replaced from the domain team, a rotation reviewer from the rotation team. A comment on the lowest of those PRs says who took over. The least-loaded pick moves from `AssignReviewers` to `ReviewConfig/pick.js`, so both pick the same way.
- **The digest** comes from the 10:00 Pacific cron, one for daylight time and one for standard time, so a late scheduled run still posts it once.
- **The dashboard** stores a hash of its content in the page version's message. It saves only when that changes, as a minor edit, because Confluence rewrites the page body on save.
- **API errors are warnings,** and that PR, repo, channel or page is skipped for the run.
- **Re-review** compares the PR's head commit with the commit each review was left on, not commit dates. Commit dates need the app's Contents permission in private repos, and a rebase rewrites them.

Setup: the org secrets and variables in the README are all set, including `CONFLUENCE_URL` and `CONFLUENCE_USER`. Still to do: the private repo with the workflow and its `REVIEW_SLA_MODE` variable.

Testing:
- `node --test AssignReviewers/ ReviewConfig/ ReviewSLA/`: 81 cases, 26 of them new.
  - The clock rules: the 16:30 Friday small PR, the three-PR stack, approving the bottom, requesting changes, a draft below, the PR below merging, re-review with and without a push, draft and ready again, late assignment, and bots and merge queue.
  - Whole runs against a fake org: `off`, outside business hours, shadow vs remind, one DM per clock across runs, reassignment across a two-PR stack, someone out only after their due time, the digest cron in daylight and standard time, the dashboard saving only on change, an unreadable repo, a failed comment write sending no DM, a missing Slack ID, and `enforce`.
- Dry run of the GitHub query and clocks, read-only, against all six repos today: 197 open PRs read. 2 clocks are running, both on this repo's own stack. StaflLib and coit-tower have 6 ready PRs with assignees, all already reviewed.
- A `shadow` run in Actions from a throwaway branch (since deleted) read the open PRs, wrote the Review SLA comments on #41 and #43, and saved the Confluence dashboard. Before the commit-ID change, the private repos failed with "Resource not accessible by integration". Slack hasn't been called for real yet.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.

4 participants