From 0fa6ff07da5b0012546c987412b63faac44fedc5 Mon Sep 17 00:00:00 2001 From: Thomas Willson Date: Thu, 1 Oct 2026 15:03:27 -0700 Subject: [PATCH 1/2] AssignReviewers: skip people who are out, from the Rippling PTO calendar 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 --- .github/workflows/assign-reviewers-tests.yml | 6 +- AssignReviewers/README.md | 18 +- AssignReviewers/action.yml | 14 ++ AssignReviewers/assign.js | 39 ++-- AssignReviewers/assign.test.js | 91 +++++++-- ReviewConfig/README.md | 56 ++++++ ReviewConfig/holidays.json | 15 ++ ReviewConfig/pto.js | 185 +++++++++++++++++++ ReviewConfig/pto.test.js | 161 ++++++++++++++++ 9 files changed, 542 insertions(+), 43 deletions(-) create mode 100644 ReviewConfig/README.md create mode 100644 ReviewConfig/holidays.json create mode 100644 ReviewConfig/pto.js create mode 100644 ReviewConfig/pto.test.js diff --git a/.github/workflows/assign-reviewers-tests.yml b/.github/workflows/assign-reviewers-tests.yml index 1a857ff..51caafc 100644 --- a/.github/workflows/assign-reviewers-tests.yml +++ b/.github/workflows/assign-reviewers-tests.yml @@ -2,10 +2,10 @@ name: AssignReviewers tests on: pull_request: - paths: [AssignReviewers/**] + paths: [AssignReviewers/**, ReviewConfig/**] push: branches: [main] - paths: [AssignReviewers/**] + paths: [AssignReviewers/**, ReviewConfig/**] permissions: contents: read @@ -18,4 +18,4 @@ jobs: - uses: actions/setup-node@v4 with: node-version: 20 - - run: node --test AssignReviewers/ + - run: node --test AssignReviewers/ ReviewConfig/ diff --git a/AssignReviewers/README.md b/AssignReviewers/README.md index 89cd2c1..62109cf 100644 --- a/AssignReviewers/README.md +++ b/AssignReviewers/README.md @@ -15,8 +15,8 @@ What it does each time a PR is opened ready or marked ready: at once, it waits up to two minutes for that PR's own run. 3. **Picks the rest.** It adds the least-loaded domain approver if there's none, then the least-loaded rotation reviewer if there are fewer than two. Load is the number of open, ready PRs - already assigned to that person in the org; anyone whose GitHub status is Busy is skipped, and ties - rotate by PR number. + already assigned to that person in the org; anyone the Rippling PTO calendar has out today or on + the next business day is skipped (Work From Home doesn't count), and ties rotate by PR number. 4. **Requests reviewers.** It requests the review team (default `embeddedreviewers`) unless someone from it has already been requested or has reviewed, so restacks don't re-request approvers. An assignee from outside the team is requested individually. @@ -30,7 +30,11 @@ merge-queue PRs are skipped. Team review auto-assignment settings are left as th so the action uses an installation token from the existing `staflsystemsci` app (org variable `STAFL_CI_APP_ID`, org secret `STAFL_CI_PRIVATE_KEY`). It is installed on all repositories and has the permissions needed: *Pull requests* and *Issues* write, *Members* read. -2. **Each repository**: add `.github/workflows/assign-reviewers.yml`: +2. **PTO calendar**: the Rippling PTO calendar feed URL is the org secret `PTO_CALENDAR_URL`. People + are matched to it by name through the org variable `REVIEW_PEOPLE` (see + [ReviewConfig](../ReviewConfig/README.md)), kept out of this public repo. Without both, nobody is + skipped. +3. **Each repository**: add `.github/workflows/assign-reviewers.yml`: ```yaml name: Assign reviewers @@ -51,9 +55,11 @@ merge-queue PRs are skipped. Team review auto-assignment settings are left as th - uses: StaflSystems/CustomGithubActions/AssignReviewers@main with: token: ${{ steps.app.outputs.token }} + pto-calendar-url: ${{ secrets.PTO_CALENDAR_URL }} + people: ${{ vars.REVIEW_PEOPLE }} ``` -3. **Authors** no longer need to add reviewers by hand, and shouldn't assign themselves. +4. **Authors** no longer need to add reviewers by hand, and shouldn't assign themselves. ## Inputs @@ -64,9 +70,11 @@ merge-queue PRs are skipped. Team review auto-assignment settings are left as th | `domain-team` | `embeddedreviewersstaff` | Team the domain approver is picked from | | `review-team` | the rotation team | Team requested as reviewers | | `max-stack-depth` | `30` | How far down a stack to look for assignees to inherit | +| `pto-calendar-url` | none | Rippling PTO calendar feed; people out today or next business day aren't picked | +| `people` | none | `REVIEW_PEOPLE` org variable: GitHub login to Rippling name and Slack ID | ## Tests ```bash -node --test AssignReviewers/ +node --test AssignReviewers/ ReviewConfig/ ``` diff --git a/AssignReviewers/action.yml b/AssignReviewers/action.yml index 99139ef..45eda07 100644 --- a/AssignReviewers/action.yml +++ b/AssignReviewers/action.yml @@ -21,6 +21,16 @@ inputs: max-stack-depth: description: How many PRs down the stack to look for assignees to inherit. default: '30' + pto-calendar-url: + description: > + Rippling PTO calendar feed (iCalendar) URL, from the PTO_CALENDAR_URL org secret. Anyone it + has out today or next business day isn't picked. Without it, nobody is skipped. + default: '' + people: + description: > + People map from the REVIEW_PEOPLE org variable, as JSON: each GitHub login's name as Rippling + shows it and their Slack member ID. Kept out of this public repo. Needed with pto-calendar-url. + default: '' runs: using: composite @@ -32,6 +42,8 @@ runs: DOMAIN_TEAM: ${{ inputs.domain-team }} REVIEW_TEAM: ${{ inputs.review-team }} MAX_STACK_DEPTH: ${{ inputs.max-stack-depth }} + PTO_CALENDAR_URL: ${{ inputs.pto-calendar-url }} + REVIEW_PEOPLE: ${{ inputs.people }} with: github-token: ${{ inputs.token }} script: | @@ -45,5 +57,7 @@ runs: domainTeam: process.env.DOMAIN_TEAM, reviewTeam: process.env.REVIEW_TEAM, maxStackDepth: process.env.MAX_STACK_DEPTH, + ptoCalendarUrl: process.env.PTO_CALENDAR_URL, + people: process.env.REVIEW_PEOPLE, }, }); diff --git a/AssignReviewers/assign.js b/AssignReviewers/assign.js index dc1afed..c93b2f3 100644 --- a/AssignReviewers/assign.js +++ b/AssignReviewers/assign.js @@ -5,12 +5,14 @@ // - Mid-stack, it copies the assignees of the nearest PR below it that has any, so the same two // people own the whole stack. // - Otherwise it picks the least-loaded available member of each team: fewest open, ready PRs -// already assigned to them, skipping anyone whose GitHub status is Busy. +// already assigned to them, skipping anyone the PTO calendar has out today or next business day. // - The author is never an assignee (a self-assignment is removed). The review team is requested // unless someone from it already is, and an assignee outside the team is requested individually. // - A PR that already has a domain approver and a second assignee only gets the review requests // topped up, so reruns are harmless and hand-picked assignees are kept. +const { awaySoon } = require('../ReviewConfig/pto.js'); + function isBot(user) { return !user || user.type === 'Bot' || user.login.endsWith('[bot]'); } @@ -52,25 +54,21 @@ async function teamMembers(github, org, team) { return members.map((m) => m.login); } -// For each login: how many open, ready PRs in the org already have them as assignee, and whether -// their GitHub status says they're busy. One GraphQL query for all candidates. -async function loadAndAvailability(github, org, logins, core) { +// For each login: how many open, ready PRs in the org already have them as assignee. +async function reviewerLoad(github, org, logins, core) { if (logins.length === 0) return {}; try { - return await queryLoadAndAvailability(github, org, logins); + return await queryLoad(github, org, logins); } catch (error) { - core.warning(`Couldn't read reviewer load or status (${error.message}); picking without them.`); - return Object.fromEntries(logins.map((login) => [login, { load: 0, busy: false }])); + core.warning(`Couldn't read reviewer load (${error.message}); picking without it.`); + return Object.fromEntries(logins.map((login) => [login, 0])); } } // Load is counted from live PR data rather than GitHub search, whose index lags new assignments // by minutes: several PRs opened close together would otherwise all see stale counts. The query // covers the org's most recently pushed repositories, which is where open PRs live. -async function queryLoadAndAvailability(github, org, logins) { - const users = logins - .map((login, i) => `u${i}: user(login: ${JSON.stringify(login)}) { status { indicatesLimitedAvailability } }`) - .join('\n'); +async function queryLoad(github, org, logins) { const prFields = 'pageInfo { hasNextPage endCursor } nodes { isDraft assignees(first: 10) { nodes { login } } }'; const data = await github.graphql( `query($org: String!) { @@ -79,7 +77,6 @@ async function queryLoadAndAvailability(github, org, logins) { nodes { name pullRequests(states: OPEN, first: 100) { ${prFields} } } } } - ${users} }`, { org }, ); @@ -107,12 +104,7 @@ async function queryLoadAndAvailability(github, org, logins) { count(page); } } - return Object.fromEntries( - logins.map((login, i) => [ - login, - { load: load[login], busy: Boolean(data[`u${i}`]?.status?.indicatesLimitedAvailability) }, - ]), - ); + return load; } module.exports = async ({ github, context, core, inputs, sleep = (ms) => new Promise((r) => setTimeout(r, ms)) }) => { @@ -125,6 +117,7 @@ module.exports = async ({ github, context, core, inputs, sleep = (ms) => new Pro const maxDepth = Number(inputs.maxStackDepth || 30); const parentWaitMs = Number(inputs.parentWaitMs ?? 120000); const pollMs = Number(inputs.pollMs ?? 10000); + const pto = inputs.pto || {}; if (pr.draft) return core.info('Draft PR: nothing to do.'); if (isBot(pr.user)) return core.info(`Bot-authored PR (${author}): nothing to do.`); @@ -151,15 +144,17 @@ module.exports = async ({ github, context, core, inputs, sleep = (ms) => new Pro } // 2. Fill the domain approver, then the rotation reviewer, least-loaded first. + let away; const pick = async (team, members) => { const candidates = members.filter((login) => login !== author && !named.has(login)).sort(); if (candidates.length === 0) return null; - const stats = await loadAndAvailability(github, owner, candidates, core); - const available = candidates.filter((login) => !stats[login].busy); + away ??= await awaySoon({ url: inputs.ptoCalendarUrl, people: inputs.people, core, ...pto }); + const load = await reviewerLoad(github, owner, candidates, core); + const available = candidates.filter((login) => !away.has(login)); const pool = available.length > 0 ? available : candidates; // Least loaded wins; ties rotate by PR number so they don't always go to the same person. - const least = Math.min(...pool.map((login) => stats[login].load)); - const tied = pool.filter((login) => stats[login].load === least); + const least = Math.min(...pool.map((login) => load[login])); + const tied = pool.filter((login) => load[login] === least); const chosen = tied[pr.number % tied.length]; notes.push(`picked ${chosen} from ${team} (${least} open assigned PRs)`); return chosen; diff --git a/AssignReviewers/assign.test.js b/AssignReviewers/assign.test.js index 16d756b..a1499a9 100644 --- a/AssignReviewers/assign.test.js +++ b/AssignReviewers/assign.test.js @@ -6,16 +6,16 @@ const assign = require('./assign.js'); const STAFF = ['staffA', 'staffB', 'staffC']; const EVERYONE = [...STAFF, 'devD', 'devE', 'devF']; -// A fake GitHub: open PRs keyed by number, team membership, per-user load and Busy status. -function fakeGithub({ prs, load = {}, busy = [] }) { +// A fake GitHub: open PRs keyed by number, team membership and per-user load. +function fakeGithub({ prs, load = {} }) { const calls = { assign: [], unassign: [], review: [], graphql: 0 }; const byNumber = new Map(prs.map((p) => [p.number, { assignees: [], requested: [], teams: [], reviews: [], ...p }])); const teams = { embeddedreviewersstaff: STAFF, embeddedreviewers: EVERYONE }; const user = (login) => ({ login, type: 'User' }); const github = { paginate: async (fn, params) => (await fn(params)).data, - // Serves the load query from the fake PRs, as GitHub would, plus Busy status per user. - graphql: async (query) => { + // Serves the load query from the fake PRs, as GitHub would. + graphql: async () => { calls.graphql++; const out = { organization: { @@ -39,9 +39,6 @@ function fakeGithub({ prs, load = {}, busy = [] }) { }, }, }; - for (const [, key, login] of query.matchAll(/(u\d+): user\(login: "([^"]+)"\)/g)) { - out[key] = { status: { indicatesLimitedAvailability: busy.includes(login) } }; - } return out; }, rest: { @@ -91,7 +88,37 @@ function fakeGithub({ prs, load = {}, busy = [] }) { return { github, calls, pr: (n) => byNumber.get(n) }; } -function run(fake, number, overrides = {}, onSleep = () => {}) { +// A PTO calendar feed with all-day entries: [name, kind, first date, last date]. +function ptoFeed(entries) { + const day = (d) => d.replaceAll('-', ''); + const next = (d) => new Date(Date.parse(`${d}T12:00:00Z`) + 864e5).toISOString().slice(0, 10); + const events = entries.map( + ([name, kind, first, last = first]) => + `BEGIN:VEVENT\r\nDTSTART;VALUE=DATE:${day(first)}\r\nDTEND;VALUE=DATE:${day(next(last))}\r\nSUMMARY:${name} ${kind}\r\nEND:VEVENT`, + ); + return ['BEGIN:VCALENDAR', ...events, 'END:VCALENDAR'].join('\r\n'); +} + +// Thursday Oct 1, 2026, 10:00 Pacific. +const THURSDAY = new Date('2026-10-01T17:00:00Z'); +const PEOPLE = Object.fromEntries(EVERYONE.map((login) => [login, { name: `Person ${login}` }])); + +function withPto(entries, fetches = []) { + return { + ptoCalendarUrl: 'https://calendar.example/pto.ics', + pto: { + now: THURSDAY, + people: PEOPLE, + holidays: { 2026: [] }, + fetch: async (url) => { + fetches.push(url); + return { ok: true, status: 200, text: async () => ptoFeed(entries) }; + }, + }, + }; +} + +function run(fake, number, overrides = {}, onSleep = () => {}, extraInputs = {}) { const pr = fake.pr(number); const logs = []; const core = { info: (m) => logs.push(m), warning: (m) => logs.push(`WARN ${m}`) }; @@ -108,7 +135,7 @@ function run(fake, number, overrides = {}, onSleep = () => {}) { }, }, }; - const inputs = { rotationTeam: 'embeddedreviewers', domainTeam: 'embeddedreviewersstaff' }; + const inputs = { rotationTeam: 'embeddedreviewers', domainTeam: 'embeddedreviewersstaff', ...extraInputs }; return assign({ github: fake.github, context, core, inputs, sleep: async () => onSleep() }).then(() => logs); } @@ -185,18 +212,56 @@ test('fills the domain approver when the stack only had a rotation assignee', as assert.deepStrictEqual(fake.pr(41).assignees.sort(), ['devD', 'staffB']); }); -test('skips people whose GitHub status is Busy', async () => { +test('skips people the PTO calendar has out today or next business day', async () => { const fake = fakeGithub({ prs: [{ number: 50, author: 'devE', head: 'a', base: 'main' }], load: { staffA: 0, staffB: 3, staffC: 3, devD: 0, devF: 1 }, - busy: ['staffA', 'devD'], }); - await run(fake, 50); + const pto = withPto([ + ['Person staffA', 'is Out of Office', '2026-10-01'], + ['Person devD', 'on Vacation', '2026-10-02', '2026-10-09'], + ]); + await run(fake, 50, {}, undefined, pto); const assignees = fake.pr(50).assignees; assert.ok(!assignees.includes('staffA') && !assignees.includes('devD')); assert.ok(assignees.includes('devF')); }); +test('Work From Home and time off after the next business day do not count as out', async () => { + const fake = fakeGithub({ + prs: [{ number: 51, author: 'devE', head: 'a', base: 'main' }], + load: { staffA: 0, staffB: 3, staffC: 3, devD: 0, devF: 1 }, + }); + const pto = withPto([ + ['Person staffA', 'on Work From Home', '2026-10-01'], + ['Person devD', 'is Out of Office', '2026-10-05'], + ]); + await run(fake, 51, {}, undefined, pto); + assert.deepStrictEqual(fake.pr(51).assignees, ['staffA', 'devD']); +}); + +test('still assigns, with a warning, when the PTO calendar cannot be read', async () => { + const fake = fakeGithub({ prs: [{ number: 52, author: 'devE', head: 'a', base: 'main' }] }); + const pto = withPto([]); + pto.pto.fetch = async () => ({ ok: false, status: 403, text: async () => '' }); + const logs = await run(fake, 52, {}, undefined, pto); + assert.strictEqual(fake.pr(52).assignees.length, 2); + assert.ok(logs.some((l) => l === "WARN Couldn't read the PTO calendar (HTTP 403); not checking who is out.")); + assert.ok(logs.every((l) => !l.includes('calendar.example'))); +}); + +test('does not read the PTO calendar when the stack already has its assignees', async () => { + const fake = fakeGithub({ + prs: [ + { number: 53, author: 'devE', head: 'a', base: 'main', assignees: ['staffC', 'devD'] }, + { number: 54, author: 'devE', head: 'b', base: 'a' }, + ], + }); + const fetches = []; + await run(fake, 54, {}, undefined, withPto([], fetches)); + assert.deepStrictEqual(fetches, []); +}); + test('ties rotate by PR number rather than always going to the same person', async () => { const picks = new Set(); for (const number of [60, 61, 62]) { @@ -255,7 +320,7 @@ test('ignores drafts, bots and merge-queue PRs', async () => { assert.deepStrictEqual(fake.calls, { assign: [], unassign: [], review: [], graphql: 0 }); }); -test('still assigns when the load and status query fails', async () => { +test('still assigns when the load query fails', async () => { const fake = fakeGithub({ prs: [{ number: 95, author: 'devE', head: 'a', base: 'main' }] }); fake.github.graphql = async () => { throw new Error('Resource not accessible by integration'); diff --git a/ReviewConfig/README.md b/ReviewConfig/README.md new file mode 100644 index 0000000..11fa52b --- /dev/null +++ b/ReviewConfig/README.md @@ -0,0 +1,56 @@ +# Review config + +Shared by the review actions (`AssignReviewers`, and the review SLA job to come): who people are, +when they're out, and which days are company holidays. + +## People + +This repo is public, so the map from GitHub logins to people is the org variable `REVIEW_PEOPLE`, +not a file here. Each workflow passes it to the action as `people: ${{ vars.REVIEW_PEOPLE }}`. It's +JSON, one entry per engineer on the review teams: + +```json +{ + "octocat": { "name": "Mona Octocat", "aliases": ["Mona Lisa Octocat"], "slack": "U0123456789" } +} +``` + +`name` is the name exactly as Rippling shows it, and `slack` is their Slack member ID, for review +reminders. `aliases` is optional: any other names they may appear under, such as a legal name that +some systems use instead of the name they go by. A calendar entry matches the name or any alias. +Whoever adds or removes someone from the review teams updates it at the same time. + +## Files + +| File | What it holds | Who updates it | +| --- | --- | --- | +| `holidays.json` | Company paid holidays, by year | By hand, each year, from the holiday calendar People Operations publishes. Add next year's before Jan 1 | +| `pto.js` | Reads the Rippling PTO calendar feed | — | + +## Who is out + +The Rippling PTO calendar feed URL is the org secret `PTO_CALENDAR_URL`. It carries a token, so it's +passed in as an action input and never logged. + +Entries are named ` is Out of Office` or ` on `. Every entry counts as out +except Work From Home. Names are matched to each person's name and aliases in `REVIEW_PEOPLE`, +ignoring case and extra spaces. Entries for anyone not in it are ignored, since the feed covers the +whole company. Someone in `REVIEW_PEOPLE` with neither a name nor an alias gets a warning on every +run, because their time off can't be checked. + +All-day entries cover each day up to the day before their end date. Timed entries, such as a half +day, cover each Pacific date they touch. + +If the feed can't be fetched, or `REVIEW_PEOPLE` is missing or isn't valid JSON, the actions warn and +carry on as if nobody is out. + +## Business days + +Business days are Monday to Friday in Pacific time, minus the holidays in `holidays.json`. A year +with no list gets a warning on every run, and every weekday counts until it's added. + +## Tests + +```bash +node --test ReviewConfig/ +``` diff --git a/ReviewConfig/holidays.json b/ReviewConfig/holidays.json new file mode 100644 index 0000000..d542958 --- /dev/null +++ b/ReviewConfig/holidays.json @@ -0,0 +1,15 @@ +{ + "2026": [ + { "date": "2026-01-01", "name": "New Year's Day" }, + { "date": "2026-01-19", "name": "Martin Luther King Jr. Day" }, + { "date": "2026-02-16", "name": "Presidents' Day" }, + { "date": "2026-05-25", "name": "Memorial Day" }, + { "date": "2026-06-19", "name": "Juneteenth" }, + { "date": "2026-07-03", "name": "Independence Day (observed)" }, + { "date": "2026-09-07", "name": "Labor Day" }, + { "date": "2026-11-26", "name": "Thanksgiving Day" }, + { "date": "2026-11-27", "name": "Native American Heritage Day" }, + { "date": "2026-12-24", "name": "Christmas Eve" }, + { "date": "2026-12-25", "name": "Christmas Day" } + ] +} diff --git a/ReviewConfig/pto.js b/ReviewConfig/pto.js new file mode 100644 index 0000000..4393213 --- /dev/null +++ b/ReviewConfig/pto.js @@ -0,0 +1,185 @@ +// Who is out of office, from the Rippling PTO calendar feed (an iCalendar file). Shared by +// AssignReviewers and the review SLA job. +// +// Entries are named " is Out of Office" or " on ". Every entry counts as +// out except Work From Home. Names are matched to GitHub logins through the people map, the +// REVIEW_PEOPLE org variable (kept out of this public repo); entries for anyone not in it (the feed +// covers the whole company) are ignored. + +const fs = require('node:fs'); +const path = require('node:path'); + +const TIME_ZONE = 'America/Los_Angeles'; +const NOT_OUT = new Set(['work from home']); + +function readJson(file) { + return JSON.parse(fs.readFileSync(file, 'utf8')); +} + +const defaultHolidaysFile = path.join(__dirname, 'holidays.json'); + +// 'YYYY-MM-DD' of an instant, in Pacific time. +function pacificDate(instant) { + return new Intl.DateTimeFormat('en-CA', { timeZone: TIME_ZONE }).format(instant); +} + +function addDays(date, days) { + const d = new Date(`${date}T12:00:00Z`); + d.setUTCDate(d.getUTCDate() + days); + return d.toISOString().slice(0, 10); +} + +function isWeekend(date) { + const day = new Date(`${date}T12:00:00Z`).getUTCDay(); + return day === 0 || day === 6; +} + +// The company paid holidays for a year, or null when holidays.json has no list for it. +function holidaysFor(holidays, year) { + const list = holidays[String(year)]; + return list ? new Set(list.map((h) => h.date)) : null; +} + +// The first business day after `date`: not a weekend and not a listed holiday. +function nextBusinessDay(date, holidays) { + let next = addDays(date, 1); + while (isWeekend(next) || holidaysFor(holidays, next.slice(0, 4))?.has(next)) next = addDays(next, 1); + return next; +} + +// Unfolds continuation lines and returns each VEVENT as { NAME: { value, params } }. +function parseIcs(text) { + const events = []; + let event = null; + for (const line of text.replace(/\r?\n[ \t]/g, '').split(/\r?\n/)) { + if (line === 'BEGIN:VEVENT') event = {}; + else if (line === 'END:VEVENT') { + if (event) events.push(event); + event = null; + } else if (event) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const [name, ...params] = line.slice(0, colon).split(';'); + event[name.toUpperCase()] = { value: line.slice(colon + 1), params: params.map((p) => p.toUpperCase()) }; + } + } + return events; +} + +function unescapeText(value) { + return value.replace(/\\([\\;,nN])/g, (_, c) => (c === 'n' || c === 'N' ? ' ' : c)); +} + +// The Pacific date a DTSTART/DTEND falls on. UTC times are converted; times with a TZID, or none, +// are taken at their own date, which is Pacific for this feed. +function propertyDate(property) { + const v = property.value; + const date = `${v.slice(0, 4)}-${v.slice(4, 6)}-${v.slice(6, 8)}`; + if (!v.endsWith('Z')) return date; + return pacificDate(new Date(`${date}T${v.slice(9, 11)}:${v.slice(11, 13)}:${v.slice(13, 15)}Z`)); +} + +// Every Pacific date an event covers. All-day events end the day before DTEND; a timed entry +// (a half day, say) covers each date it touches. +function eventDates(event) { + if (!event.DTSTART) return []; + const allDay = event.DTSTART.params.includes('VALUE=DATE') || event.DTSTART.value.length === 8; + const start = propertyDate(event.DTSTART); + let last = start; + if (event.DTEND) { + const end = propertyDate(event.DTEND); + last = allDay ? addDays(end, -1) : end; + } + const dates = []; + for (let d = start; d <= last && dates.length < 366; d = addDays(d, 1)) dates.push(d); + return dates.length > 0 ? dates : [start]; +} + +// "Ada Lovelace is Out of Office" → { name: 'Ada Lovelace', kind: 'Out of Office' }. +function parseSummary(summary) { + const match = /^(.+?) (?:is|on) (.+)$/.exec(summary.trim()); + return match ? { name: match[1].trim(), kind: match[2].trim() } : null; +} + +const normalize = (name) => name.toLowerCase().replace(/\s+/g, ' ').trim(); + +// Every name a person may appear under: their name as Rippling shows it, plus any aliases (a legal +// name that other systems use, say). +const namesOf = (person) => [person?.name, ...(Array.isArray(person?.aliases) ? person.aliases : [])].filter( + (name) => typeof name === 'string' && name.trim(), +); + +// For each GitHub login in the people map, the set of Pacific dates the calendar has them out. +function outDates(icsText, people) { + const loginByName = new Map( + Object.entries(people).flatMap(([login, person]) => namesOf(person).map((name) => [normalize(name), login])), + ); + const out = {}; + for (const event of parseIcs(icsText)) { + const entry = event.SUMMARY && parseSummary(unescapeText(event.SUMMARY.value)); + if (!entry || NOT_OUT.has(entry.kind.toLowerCase())) continue; + const login = loginByName.get(normalize(entry.name)); + if (!login) continue; + out[login] ??= new Set(); + for (const date of eventDates(event)) out[login].add(date); + } + return out; +} + +// The logins who are out today or on the next business day (Pacific): a review assigned now is +// due within about one business day, so someone out tomorrow shouldn't get it today. +// +// `people` is the people map, as an object or as the JSON text of the REVIEW_PEOPLE org variable: +// { "": { "name": "", "aliases": [""], +// "slack": "" } }, with aliases optional. +// +// Returns an empty set, with a warning, when the feed or the map can't be read: picking someone who +// is out is better than not assigning at all. With no URL configured, it does nothing. +async function awaySoon({ + url, + core, + people, + now = new Date(), + fetch = globalThis.fetch, + holidays = readJson(defaultHolidaysFile), +}) { + if (!url) { + core.info('No PTO calendar configured; not checking who is out.'); + return new Set(); + } + try { + people = typeof people === 'string' ? JSON.parse(people || '{}') : people || {}; + } catch { + core.warning("The REVIEW_PEOPLE org variable isn't valid JSON; not checking who is out."); + return new Set(); + } + if (Object.keys(people).length === 0) { + core.warning('The REVIEW_PEOPLE org variable is empty or not passed in; not checking who is out.'); + return new Set(); + } + const unnamed = Object.entries(people).filter(([, person]) => namesOf(person).length === 0).map(([login]) => login); + if (unnamed.length > 0) { + core.warning(`No Rippling name in REVIEW_PEOPLE for ${unnamed.join(', ')}, so their PTO isn't checked.`); + } + const today = pacificDate(now); + if (!holidaysFor(holidays, today.slice(0, 4))) { + core.warning(`ReviewConfig/holidays.json has no holidays for ${today.slice(0, 4)}; every weekday counts as a business day.`); + } + const window = [today, nextBusinessDay(today, holidays)]; + let text; + try { + // The URL carries a token, so neither it nor an error that might quote it is logged. + const response = await fetch(url); + if (!response.ok) throw new Error(`HTTP ${response.status}`); + text = await response.text(); + } catch (error) { + core.warning(`Couldn't read the PTO calendar (${/^HTTP \d+$/.test(error.message) ? error.message : error.name}); not checking who is out.`); + return new Set(); + } + const dates = outDates(text, people); + const away = new Set(Object.keys(dates).filter((login) => window.some((d) => dates[login].has(d)))); + core.info(`Out ${window.join(' or ')}: ${[...away].join(', ') || 'nobody'}.`); + return away; +} + +module.exports = { awaySoon, outDates, nextBusinessDay, pacificDate, parseIcs }; diff --git a/ReviewConfig/pto.test.js b/ReviewConfig/pto.test.js new file mode 100644 index 0000000..4410e82 --- /dev/null +++ b/ReviewConfig/pto.test.js @@ -0,0 +1,161 @@ +// Run with: node --test ReviewConfig/ +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); +const { awaySoon, outDates, nextBusinessDay, pacificDate } = require('./pto.js'); + +const PEOPLE = { + alovelace: { name: 'Ada Lovelace' }, + ghopper: { name: 'Grace Hopper' }, + 'kjohnson': { name: 'Katherine Johnson' }, +}; +const HOLIDAYS = { 2026: [{ date: '2026-11-26' }, { date: '2026-11-27' }] }; + +const calendar = (...events) => + ['BEGIN:VCALENDAR', 'VERSION:2.0', ...events.flatMap((e) => ['BEGIN:VEVENT', ...e, 'END:VEVENT']), 'END:VCALENDAR'].join('\r\n'); + +const sorted = (dates) => Object.fromEntries(Object.entries(dates).map(([login, set]) => [login, [...set].sort()])); + +test('reads all-day entries, with DTEND the day after the last day', () => { + const ics = calendar( + ['DTSTART;VALUE=DATE:20261002', 'DTEND;VALUE=DATE:20261003', 'SUMMARY:Grace Hopper is Out of Office'], + ['DTSTART;VALUE=DATE:20261005', 'DTEND;VALUE=DATE:20261008', 'SUMMARY:Ada Lovelace on Vacation'], + ); + assert.deepStrictEqual(sorted(outDates(ics, PEOPLE)), { + ghopper: ['2026-10-02'], + alovelace: ['2026-10-05', '2026-10-06', '2026-10-07'], + }); +}); + +test('Work From Home is not out; other companies people and other events are ignored', () => { + const ics = calendar( + ['DTSTART;VALUE=DATE:20261001', 'SUMMARY:Ada Lovelace on Work From Home'], + ['DTSTART;VALUE=DATE:20261002', 'SUMMARY:Katherine Johnson on work from home'], + ['DTSTART;VALUE=DATE:20261001', "SUMMARY:Alex R's 5-year anniversary"], + ['DTSTART;VALUE=DATE:20261001', 'SUMMARY:Someone Else is Out of Office'], + ); + assert.deepStrictEqual(outDates(ics, PEOPLE), {}); +}); + +test('matches aliases as well as the name', () => { + const people = { kjohnson: { name: 'Katie Johnson', aliases: ['Katherine Johnson', 'K. Johnson'] } }; + const ics = calendar( + ['DTSTART;VALUE=DATE:20261002', 'SUMMARY:Katie Johnson is Out of Office'], + ['DTSTART;VALUE=DATE:20261005', 'SUMMARY:Katherine Johnson on Vacation'], + ['DTSTART;VALUE=DATE:20261006', 'SUMMARY:k. johnson on Sick Leave'], + ); + assert.deepStrictEqual(sorted(outDates(ics, people)), { kjohnson: ['2026-10-02', '2026-10-05', '2026-10-06'] }); +}); + +test('someone with only aliases still matches, and isn\'t warned about', async () => { + const people = { kjohnson: { name: '', aliases: ['Katherine Johnson'] } }; + const ics = calendar(['DTSTART;VALUE=DATE:20261001', 'SUMMARY:Katherine Johnson is Out of Office']); + const { core, logs } = logger(); + const now = new Date('2026-10-01T18:00:00Z'); + const away = await awaySoon({ url: 'u', core, now, fetch: feed(ics), people, holidays: HOLIDAYS }); + assert.deepStrictEqual([...away], ['kjohnson']); + assert.ok(logs.every((l) => !l.startsWith('WARN'))); +}); + +test('timed entries count on each Pacific date they touch', () => { + const ics = calendar( + // 09:00 to 13:00 Pacific on Oct 2, given in UTC. + ['DTSTART:20261002T160000Z', 'DTEND:20261002T200000Z', 'SUMMARY:Grace Hopper on Sick Leave'], + // 17:00 Pacific Oct 4 is already Oct 5 in UTC. + ['DTSTART:20261005T000000Z', 'DTEND:20261005T010000Z', 'SUMMARY:Ada Lovelace is Out of Office'], + ); + assert.deepStrictEqual(sorted(outDates(ics, PEOPLE)), { ghopper: ['2026-10-02'], alovelace: ['2026-10-04'] }); +}); + +test('unfolds long lines, unescapes text, and matches names regardless of case and spacing', () => { + const ics = calendar(['DTSTART;VALUE=DATE:20261002', 'SUMMARY:katherine johnson on Jury Duty\\, Superior', ' Court']); + assert.deepStrictEqual(sorted(outDates(ics, PEOPLE)), { 'kjohnson': ['2026-10-02'] }); +}); + +test('next business day skips weekends and listed holidays', () => { + assert.strictEqual(nextBusinessDay('2026-10-01', HOLIDAYS), '2026-10-02'); + assert.strictEqual(nextBusinessDay('2026-10-02', HOLIDAYS), '2026-10-05'); + assert.strictEqual(nextBusinessDay('2026-11-25', HOLIDAYS), '2026-11-30'); +}); + +test('dates are Pacific', () => { + assert.strictEqual(pacificDate(new Date('2026-10-02T06:00:00Z')), '2026-10-01'); +}); + +function logger() { + const logs = []; + return { logs, core: { info: (m) => logs.push(m), warning: (m) => logs.push(`WARN ${m}`) } }; +} + +const feed = (ics) => async () => ({ ok: true, status: 200, text: async () => ics }); + +test('away soon = out today or on the next business day', async () => { + const ics = calendar( + ['DTSTART;VALUE=DATE:20261127', 'SUMMARY:Ada Lovelace is Out of Office'], + ['DTSTART;VALUE=DATE:20261130', 'SUMMARY:Grace Hopper is Out of Office'], + ['DTSTART;VALUE=DATE:20261201', 'SUMMARY:Katherine Johnson is Out of Office'], + ); + const { core } = logger(); + // Wednesday before Thanksgiving: the next business day is the Monday after. + const now = new Date('2026-11-25T18:00:00Z'); + const away = await awaySoon({ url: 'u', core, now, fetch: feed(ics), people: PEOPLE, holidays: HOLIDAYS }); + assert.deepStrictEqual([...away], ['ghopper']); +}); + +test('warns about people with no Rippling name and years with no holidays', async () => { + const { core, logs } = logger(); + const people = { ...PEOPLE, newhire: { name: '' } }; + await awaySoon({ url: 'u', core, now: new Date('2027-01-04T18:00:00Z'), fetch: feed(calendar()), people, holidays: HOLIDAYS }); + assert.ok(logs.includes("WARN No Rippling name in REVIEW_PEOPLE for newhire, so their PTO isn't checked.")); + assert.ok(logs.includes('WARN ReviewConfig/holidays.json has no holidays for 2027; every weekday counts as a business day.')); +}); + +test('a failed fetch is a warning that never shows the URL', async () => { + const { core, logs } = logger(); + const url = 'https://app.rippling.com/secret-token/calendar.ics'; + const fetch = async () => { + throw new TypeError(`fetch failed for ${url}`); + }; + const away = await awaySoon({ url, core, fetch, people: PEOPLE, holidays: HOLIDAYS }); + assert.strictEqual(away.size, 0); + assert.ok(logs.includes("WARN Couldn't read the PTO calendar (TypeError); not checking who is out.")); + assert.ok(logs.every((l) => !l.includes('secret-token'))); +}); + +test('no URL configured: nothing is fetched and nobody is out', async () => { + const { core } = logger(); + const away = await awaySoon({ url: '', core, fetch: () => assert.fail('fetched'), people: PEOPLE, holidays: HOLIDAYS }); + assert.strictEqual(away.size, 0); +}); + +test('takes the people map as the JSON text of the REVIEW_PEOPLE variable', async () => { + const ics = calendar(['DTSTART;VALUE=DATE:20261001', 'SUMMARY:Ada Lovelace is Out of Office']); + const { core } = logger(); + const now = new Date('2026-10-01T18:00:00Z'); + const away = await awaySoon({ url: 'u', core, now, fetch: feed(ics), people: JSON.stringify(PEOPLE), holidays: HOLIDAYS }); + assert.deepStrictEqual([...away], ['alovelace']); +}); + +test('a missing or broken people map is a warning, and nobody is treated as out', async () => { + for (const [people, warning] of [ + ['', 'WARN The REVIEW_PEOPLE org variable is empty or not passed in; not checking who is out.'], + [undefined, 'WARN The REVIEW_PEOPLE org variable is empty or not passed in; not checking who is out.'], + ['{not json', "WARN The REVIEW_PEOPLE org variable isn't valid JSON; not checking who is out."], + ]) { + const { core, logs } = logger(); + const away = await awaySoon({ url: 'u', core, people, fetch: () => assert.fail('fetched'), holidays: HOLIDAYS }); + assert.strictEqual(away.size, 0); + assert.deepStrictEqual(logs, [warning]); + } +}); + +test('the checked-in holidays are weekdays in their own year', () => { + const holidays = JSON.parse(fs.readFileSync(path.join(__dirname, 'holidays.json'), 'utf8')); + for (const [year, list] of Object.entries(holidays)) { + for (const { date } of list) { + assert.ok(date.startsWith(`${year}-`), date); + assert.ok(![0, 6].includes(new Date(`${date}T12:00:00Z`).getUTCDay()), `${date} is a weekend`); + } + } +}); From f48b78021ef0fa9b529cf6f6bb2e3d3b33449c4f Mon Sep 17 00:00:00 2001 From: Thomas Willson Date: Thu, 1 Oct 2026 15:06:42 -0700 Subject: [PATCH 2/2] Pass the PTO calendar and people map to this repo's reviewer assignment 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 --- .github/workflows/assign-reviewers.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index c86f086..1ca2782 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -20,3 +20,5 @@ jobs: - uses: StaflSystems/CustomGithubActions/AssignReviewers@main with: token: ${{ steps.app.outputs.token }} + pto-calendar-url: ${{ secrets.PTO_CALENDAR_URL }} + people: ${{ vars.REVIEW_PEOPLE }}