diff --git a/package-lock.json b/package-lock.json index 0bfb907..2146031 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "codercops-discord-bot", - "version": "0.3.0", + "version": "0.4.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "codercops-discord-bot", - "version": "0.3.0", + "version": "0.4.0", "license": "MIT", "dependencies": { "@resvg/resvg-wasm": "^2.6.2" diff --git a/package.json b/package.json index 9cc3fc3..7d63136 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codercops-discord-bot", - "version": "0.3.0", + "version": "0.4.0", "private": true, "description": "The CODERCOPS community bot: posts releases, starter issues and merged PRs from our open source repos into Discord.", "license": "MIT", diff --git a/src/commands.ts b/src/commands.ts index 5efbb7d..721c0e5 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -3,9 +3,9 @@ import { listCheckins, localDate, rankStreaks, recordCheckin } from "./checkin"; import type { Config, Env } from "./config"; import type { MessagePayload } from "./discord"; -import { countMergedPrs, GithubError, listStarterIssues } from "./github"; +import { countMergedPrs, getLoginById, GithubError, listStarterIssues } from "./github"; import { getLeaderboard } from "./leaderboard"; -import { getLink, linkedMembers, removeLink } from "./links"; +import { getLink, linkedMembers, removeLink, saveLink } from "./links"; import { checkinReply, helpReply, leaderboardMessage, linkPrompt, meReply, starterIssuesReply, streaksMessage } from "./messages"; import { authorizeUrl, linkingEnabled, signState } from "./oauth"; import { grantEarnedRoles, isMaintainer, removeTierRoles } from "./roles"; @@ -23,6 +23,7 @@ export interface CommandUser { } const IN_SERVER_ONLY: MessagePayload = { content: "Run this inside the server, not in a DM." }; +const RELINK_GITHUB: MessagePayload = { content: "I couldn't find your linked GitHub account. Run `/link` again to reconnect it." }; export async function runCommand( data: CommandData, @@ -83,7 +84,9 @@ export async function runCommand( return leaderboardMessage(board, program, { final: programStatus(program, now) === "ended", url: leaderboardUrl(cfg, program), + finalNote: cfg.finalResultsNote, members: await linkedMembers(env, board.rows.slice(0, 10).map((r) => r.login)), + linking: linkingEnabled(env), }); } case "link": { @@ -105,9 +108,31 @@ export async function runCommand( } case "me": { if (!user) return IN_SERVER_ONLY; - const link = await getLink(env, user.id); + let link = await getLink(env, user.id); if (!link) return { content: "You haven't linked a GitHub account yet. Run `/link` first." }; - const merged = await countMergedPrs(env, cfg, link.githubLogin); + let merged: number; + try { + merged = await countMergedPrs(env, cfg, link.githubLogin); + } catch (err) { + if (!(err instanceof GithubError) || err.status !== 422) throw err; + let login: string | null; + try { + login = await getLoginById(env, link.githubId); + } catch (lookupError) { + if (lookupError instanceof GithubError && (lookupError.status === 403 || lookupError.status === 429)) throw lookupError; + return RELINK_GITHUB; + } + if (!login) return RELINK_GITHUB; + link = { ...link, githubLogin: login }; + await saveLink(env, link); + // Retry once; another invalid login must not restart recovery. + try { + merged = await countMergedPrs(env, cfg, login); + } catch (retryError) { + if (retryError instanceof GithubError && retryError.status === 422) return RELINK_GITHUB; + throw retryError; + } + } // Catches up on a role if a merge was missed. const newRoles = await grantEarnedRoles(env, cfg, link, merged).catch((err) => { console.error("granting roles failed", err); diff --git a/src/github.ts b/src/github.ts index 98b32b7..f3a8916 100644 --- a/src/github.ts +++ b/src/github.ts @@ -43,6 +43,15 @@ async function gh(env: Env, path: string): Promise { return (await res.json()) as T; } +/** Resolves a linked account's current login using the id that survives renames. */ +export async function getLoginById(env: Env, githubId: string): Promise { + if (!/^[1-9]\d*$/.test(githubId)) return null; + const user = await gh<{ id?: number; login?: unknown } | null>(env, `/user/${githubId}`); + if (!user || String(user.id) !== githubId || typeof user.login !== "string") return null; + // Apply the same login constraints as the verified Discord connection in oauth.ts. + return /^[a-z\d](?:[a-z\d-]{0,38})$/i.test(user.login) ? user.login : null; +} + export interface StarterIssue { repo: string; number: number; @@ -110,17 +119,27 @@ interface SearchResult { items: { html_url: string; number: number; title: string; repository_url: string; user: { login: string } | null; pull_request?: { merged_at: string | null } }[]; } -/** Every merged PR that counts for a program: labeled, in its repos, merged inside its window. */ -export async function searchMergedPrs(env: Env, cfg: Config, program: Program): Promise { - const { label, start, end } = program; - const iso = (d: Date) => d.toISOString().replace(/\.\d{3}Z$/, "Z"); - const q = [ +/** The GitHub search query for a program's merged PRs. */ +export function mergedPrQuery( + cfg: Config, + program: Program, + { author, dayOnly = false }: { author?: string; dayOnly?: boolean } = {} +): string { + const date = (d: Date) => + dayOnly ? d.toISOString().slice(0, 10) : d.toISOString().replace(/\.\d{3}Z$/, "Z"); + return [ "is:pr", "is:merged", - `label:"${label}"`, - `merged:${iso(start)}..${iso(end)}`, - ...program.repos.map((r) => `repo:${cfg.org}/${r}`), + ...(author ? [`author:${author}`] : []), + `label:"${program.label}"`, + `merged:${date(program.start)}..${date(program.end)}`, + ...program.repos.map((repo) => `repo:${cfg.org}/${repo}`), ].join(" "); +} + +/** Every merged PR that counts for a program: labeled, in its repos, merged inside its window. */ +export async function searchMergedPrs(env: Env, cfg: Config, program: Program): Promise { + const q = mergedPrQuery(cfg, program); const prs: MergedPr[] = []; // The search API returns at most 1000 results (10 pages of 100). diff --git a/src/index.ts b/src/index.ts index 690d5ec..f3d0560 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,7 +3,7 @@ import { runCommand, type CommandData, type CommandUser } from "./commands"; import { DAILY_CRON, getConfig, WELCOME_CRON, type Config, type Env } from "./config"; import { editInteractionReply, sendMessage } from "./discord"; import { getLeaderboard } from "./leaderboard"; -import { GithubError } from "./github"; +import { GithubError, mergedPrQuery } from "./github"; import { linkedMembers } from "./links"; import { completeLink } from "./linking"; import { leaderboardMessage, LINK_BUTTON_ID, streaksMessage } from "./messages"; @@ -106,7 +106,7 @@ async function handleLeaderboardPage(request: Request, env: Env, id: string | un const cfg = getConfig(env); const now = new Date(); const program = id === undefined ? currentProgram(cfg, now) : findProgram(cfg, id); - if (!program) return new Response("not found", { status: 404 }); + if (!program) return new Response("not found", { status: 404, headers: asJson ? { "Access-Control-Allow-Origin": "*" } : undefined }); // Browsers and the edge may reuse the response for five minutes. const cache = "public, max-age=300"; try { @@ -123,8 +123,22 @@ async function handleLeaderboardPage(request: Request, env: Env, id: string | un .filter((p) => p.id !== program.id) .reverse() .map((p) => ({ title: p.title, path: `/leaderboard/${p.id}` })); + + // When linking is enabled, look up which qualifying users have linked their GitHub account. + const linking = linkingEnabled(env); + let members: Map | undefined; + if (linking) { + const qualifying = board.rows.filter((r) => r.qualifies).map((r) => r.login); + try { + members = await linkedMembers(env, qualifying); + } catch (err) { + console.error("failed to look up linked members", err); + } + } + const html = leaderboardPage(board, program, { issues: env.ISSUES_URL, discord: env.DISCORD_INVITE, self, image, others }, now, { - linking: linkingEnabled(env), + linking, + members, }); return new Response(html, { headers: { @@ -141,11 +155,21 @@ async function handleLeaderboardPage(request: Request, env: Env, id: string | un console.error("leaderboard page failed", err); return new Response(limited ? "GitHub is rate limiting us. Try again in a minute." : "The leaderboard is unavailable right now.", { status: 503, - headers: { "Content-Type": "text/plain; charset=utf-8", "Retry-After": "60" }, + headers: { "Content-Type": "text/plain; charset=utf-8", "Retry-After": "60", ...(asJson ? { "Access-Control-Allow-Origin": "*" } : {}) }, }); } } +// The cache key for a drawn image: its path and the version worked out here, +// whatever `?v=` the request carries. Keyed on the address asked for, every new +// `?v=` value would mean a new picture to draw. The key is always a GET, because +// the cache only takes those, so a HEAD finds and stores the image under the +// same key as a GET (otherwise the cache would throw, and the image would be +// left empty for the next GET). +function imageKey(path: string, version: string, request: Request): Request { + return new Request(new URL(`${path}?v=${encodeURIComponent(version)}`, request.url).href); +} + // A program's leaderboard as a share image: /leaderboard/{id}.png. The page's // og:image points here. async function handleLeaderboardImage(request: Request, env: Env, ctx: ExecutionContext, id: string): Promise { @@ -156,13 +180,6 @@ async function handleLeaderboardImage(request: Request, env: Env, ctx: Execution // The image is the same for everyone and costs some CPU to draw, so keep it at the edge. const cache = typeof caches !== "undefined" ? await caches.open("cards").catch(() => undefined) : undefined; - // The cache only takes GET requests, and it keys on the address. So a HEAD - // asks for the image under the key a GET uses, and stores it there too: - // otherwise the cache would throw, and the image would be left empty for the - // next GET. - const cacheKey = request.method === "GET" ? request : new Request(request.url); - const cached = await cache?.match(cacheKey); - if (cached) return cached; let board; try { @@ -175,6 +192,10 @@ async function handleLeaderboardImage(request: Request, env: Env, ctx: Execution }); } + const cacheKey = imageKey(`/leaderboard/${program.id}.png`, boardVersion(board.rows, program, now), request); + const cached = await cache?.match(cacheKey); + if (cached) return cached; + // Loaded on demand: this module pulls in the rasterizer and the fonts. const { fetchAvatar, renderPng } = await import("./card-image"); const avatars = await Promise.all(board.rows.slice(0, BOARD_ROWS).map((row) => fetchAvatar(row.login))); @@ -234,16 +255,6 @@ async function handleContributor( const program = findProgram(cfg, programId); if (!program) return new Response("not found", { status: 404 }); - // The card is the same for everyone and costs some CPU to draw, so keep it at the edge. - const cache = asPng && typeof caches !== "undefined" ? await caches.open("cards").catch(() => undefined) : undefined; - // The cache only takes GET requests, and it keys on the address. So a HEAD - // asks for the card under the key a GET uses, and stores it there too: - // otherwise the cache would throw, and the card would be left empty for the - // next GET. - const cacheKey = request.method === "GET" ? request : new Request(request.url); - const cached = await cache?.match(cacheKey); - if (cached) return cached; - let board; try { board = await getLeaderboard(env, cfg, program); @@ -257,26 +268,40 @@ async function handleContributor( const index = board.rows.findIndex((r) => r.login.toLowerCase() === login.toLowerCase()); const row = board.rows[index]; const boardPath = `/leaderboard/${program.id}`; + const status = programStatus(program, now); if (!row) { if (asPng) return new Response("not found", { status: 404 }); const body = noticePage( "No card yet", [ `${login} has no counted pull requests in ${program.title}.`, - `A card appears with the first merged pull request that counts. The leaderboard is at ${new URL(boardPath, request.url).href}`, + status === "ended" + ? `${program.title} is over, so no more pull requests can count.` + : "A card appears with the first merged pull request that counts.", ], false, - "No card" + "No card", + [ + { label: "See the leaderboard", href: boardPath }, + { label: "Find an issue", href: env.ISSUES_URL }, + ] ); return new Response(body, { status: 404, headers: { ...PAGE_HEADERS, "Cache-Control": "no-store", "Content-Security-Policy": CONTRIBUTOR_CSP } }); } const rank = index + 1; const of = board.rows.length; - const status = programStatus(program, now); const path = `${boardPath}/u/${encodeURIComponent(row.login)}`; + // The version makes share previews pick up a new image when the numbers change. + const version = `${row.count}-${rank}-${of}-${status}`; if (asPng) { + // The card is the same for everyone and costs some CPU to draw, so keep it at the edge. + const cache = typeof caches !== "undefined" ? await caches.open("cards").catch(() => undefined) : undefined; + const cacheKey = imageKey(`${path}.png`, version, request); + const cached = await cache?.match(cacheKey); + if (cached) return cached; + // Loaded on demand: this module pulls in the rasterizer and the fonts. const { fetchAvatar, renderPng } = await import("./card-image"); const svg = cardSvg({ @@ -296,9 +321,7 @@ async function handleContributor( return response; } - // The version makes share previews pick up a new image when the numbers change. - const version = `${row.count}-${rank}-${of}-${status}`; - const prsQuery = `org:${cfg.org} is:pr is:merged author:${row.login} label:"${program.label}" merged:${program.start.toISOString().slice(0, 10)}..${program.end.toISOString().slice(0, 10)}`; + const prsQuery = mergedPrQuery(cfg, program, { author: row.login, dayOnly: true }); const html = contributorPage( { program, row, rank, of, generatedAt: board.generatedAt }, { diff --git a/src/messages.ts b/src/messages.ts index 9b0e637..be7d457 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -156,14 +156,21 @@ export function leaderboardMessage( const prs = `${row.count} merged ${row.count === 1 ? "PR" : "PRs"}`; // Same as the chips on the page: a countdown while it runs, nothing once it's over. const countdown = opts.final ? "" : ` · ${minPrs - row.count} more to qualify`; - const note = row.winner ? (opts.final ? " · winner" : " · in the top " + topN) : row.qualifies ? " · qualifies" : countdown; + const note = row.winner + ? opts.final ? " · winner" : " · in the top " + topN + : row.qualifies ? (opts.final ? " · qualified" : " · qualifies") : countdown; return `${i + 1}. ${who(row.login)} · ${prs}${note}`; }); const winners = board.rows.filter((r) => r.winner); let description: string; if (lines.length === 0) { - description = "No merged PRs counted yet. Pick a starter issue and be the first."; + if (opts.final) { + description = "No merged pull requests were counted."; + if (opts.finalNote) description += `\n\n${opts.finalNote}`; + } else { + description = "No merged PRs counted yet. Pick a starter issue and be the first."; + } } else { description = lines.join("\n"); if (opts.final) { diff --git a/src/page.ts b/src/page.ts index c0b8b41..9856df4 100644 --- a/src/page.ts +++ b/src/page.ts @@ -111,6 +111,7 @@ h1{font:700 clamp(28px,6vw,40px)/1.15 ui-monospace,SFMono-Regular,Menlo,monospac .status{display:inline-block;font:600 13px/1 ui-monospace,SFMono-Regular,Menlo,monospace;padding:7px 11px;border-radius:999px;border:1px solid var(--line);color:var(--muted);margin:0 0 28px} .status.running{color:var(--cyan);border-color:var(--cyan)} .status.ended{color:var(--gold);border-color:var(--gold)} +.unlinked{margin:0 0 24px;color:var(--muted);font-size:14px;line-height:1.5} .lede{margin-bottom:14px} .others{margin:0 0 36px} .others h2{font:600 13px/1 ui-monospace,SFMono-Regular,Menlo,monospace;letter-spacing:.18em;text-transform:uppercase;color:var(--muted);margin:0 0 10px} @@ -132,7 +133,7 @@ main.wide{max-width:920px} h2.small{font:600 13px/1 ui-monospace,SFMono-Regular,Menlo,monospace;letter-spacing:.18em;text-transform:uppercase;color:var(--muted);margin:0 0 12px} @media (max-width:520px){.tiles{grid-template-columns:1fr}.cta a{flex:1 1 100%;text-align:center}} footer{color:var(--muted);font-size:13px} -@media (max-width:520px){.chip{display:none}} +@media (max-width:520px){.board summary{gap:8px}.rank{width:auto}.board img{width:28px;height:28px}.chip,.count-label{display:none}} `; export interface PageLinks { @@ -147,13 +148,15 @@ export interface PageLinks { others: { title: string; path: string }[]; } +const joinNames = (names: string[]) => names.join(", ").replace(/, ([^,]*)$/, " and $1"); + export function leaderboardPage( board: Leaderboard, program: Program, links: PageLinks, now: Date, /** `linking`: members can link their GitHub account with /link, which is how we find a winner on Discord. */ - opts: { linking?: boolean } = {} + opts: { linking?: boolean; members?: Map } = {} ): string { const { title, label, start, end, minPrs, topN, reward, note } = program; const heading = `${title} leaderboard`; @@ -169,19 +172,39 @@ export function leaderboardPage( ? `Share image: ${heading}. ${top}.` : `Share image: ${heading}. No merged pull requests counted yet.`; + let unlinkedNotice = ""; + if (opts.linking && opts.members) { + const candidates = state.status === "ended" + ? board.rows.filter((r) => r.winner) + : board.rows.filter((r) => r.qualifies); + const unlinked = candidates.filter((r) => !opts.members?.has(r.login.toLowerCase())); + if (unlinked.length > 0) { + const names = joinNames( + unlinked.map((r) => `${escapeHtml(r.login)}`) + ); + if (state.status === "ended") { + const verb = unlinked.length === 1 ? "won but hasn't" : "won but haven't"; + unlinkedNotice = `

${names} ${verb} linked their GitHub yet. Join the Discord and run /link there, so we can reach you.

`; + } else { + const qualifyVerb = unlinked.length === 1 ? "qualifies but hasn't" : "qualify but haven't"; + unlinkedNotice = `

${names} ${qualifyVerb} linked their GitHub yet. Join the Discord and run /link there, so we can reach you if you win.

`; + } + } + } + const rows = board.rows .map((row, i) => { const login = escapeHtml(row.login); const chip = standing(row, program, state.status); const chipHtml = chip ? `${escapeHtml(chip.text)}` : ""; - const card = `Card`; + const card = `Card`; const prList = row.prs .map( (pr) => `
  • ${escapeHtml(pr.repo)}#${pr.number}${escapeHtml(pr.title)}
    • ` ) .join("\n"); - return `
  • ${i + 1}${login}${row.count} merged ${row.count === 1 ? "PR" : "PRs"}${chipHtml}${card}\n
      \n${prList}\n
    • `; + return `
  • ${i + 1}${login}${row.count} merged ${row.count === 1 ? "PR" : "PRs"}${chipHtml}${card}\n
      \n${prList}\n
    • `; }) .join("\n"); @@ -192,6 +215,7 @@ export function leaderboardPage( ${escapeHtml(heading)} + @@ -211,6 +235,7 @@ export function leaderboardPage(

    ${escapeHtml(heading)}

    Merged pull requests in ${escapeHtml(repos)}, ${when}.

    ${escapeHtml(state.text)}

    +${unlinkedNotice} ${board.rows.length ? `
      \n${rows}\n
    ` : `

    ${state.status === "ended" ? "No merged pull requests were counted." : "No merged pull requests counted yet. Pick an issue and be the first."}

    `}

    How it works

    @@ -232,7 +257,13 @@ ${links.others.length ? `
    \n

    Other programs

    \n
      } /** A small page with a heading and a few paragraphs, for the result of /link. Everything is escaped. */ -export function noticePage(title: string, lines: string[], ok: boolean, label = ok ? "Done" : "Not linked"): string { +export function noticePage( + title: string, + lines: string[], + ok: boolean, + label = ok ? "Done" : "Not linked", + links: { label: string; href: string }[] = [] +): string { return ` @@ -250,7 +281,7 @@ export function noticePage(title: string, lines: string[], ok: boolean, label =
      ${lines.map((line) => `

      ${escapeHtml(line)}

      `).join("\n")}
      -

      ${escapeHtml(label)}

      +${links.length ? `
      \n${links.map((link) => `${escapeHtml(link.label)}`).join("\n")}\n
      \n` : ""}

      ${escapeHtml(label)}

      `; diff --git a/src/webhook.ts b/src/webhook.ts index bfa1d4a..e32f604 100644 --- a/src/webhook.ts +++ b/src/webhook.ts @@ -1,5 +1,5 @@ // Handles GitHub webhook events: releases, starter issues and merged PRs. -import type { Config, Env } from "./config"; +import type { Config, Env, Program } from "./config"; import { DiscordError, editMessage, sendMessage } from "./discord"; import { countMergedPrs, getIssue, type GithubIssue, type StarterIssue } from "./github"; import { cacheLeaderboard, invalidateLeaderboard, recentMerges, rememberMerges, searchCountedPrs } from "./leaderboard"; @@ -30,6 +30,8 @@ interface IssuesEvent { interface PullRequestEvent { action: string; repository: Repository; + /** The label that was added, on labeled events. */ + label?: { name: string }; pull_request: { number: number; title: string; @@ -60,7 +62,7 @@ export async function handleGithubEvent(event: string, payload: unknown, env: En } async function onRelease(e: ReleaseEvent, env: Env, cfg: Config): Promise { - if (e.action !== "published" || e.release.draft || e.release.prerelease || !ours(cfg, e.repository)) { + if (e.action !== "released" || e.release.draft || e.release.prerelease || !ours(cfg, e.repository)) { return "ignored release"; } await sendMessage(env, env.CHANNEL_RELEASES, releaseMessage(e.repository.name, e.release)); @@ -199,12 +201,78 @@ export async function resyncStarterIssues(env: Env, cfg: Config): Promise } } +/** The shoutout a merged PR got, so a late label can edit it instead of posting again. */ +interface ShoutoutRecord { + messageId: string; + /** True when the shoutout already carries a count line. */ + counted: boolean; + discordId?: string; + linking?: boolean; + newRoles?: string[]; +} + +const shoutoutKey = (repo: string, number: number) => `merged-pr:${repo}#${number}`; +/** A late label usually lands soon after the merge, so a few days is plenty. */ +const SHOUTOUT_TTL_SECONDS = 3 * 24 * 60 * 60; + +/** + * Counts a PR for a program and caches the board, the way a merge does. + * Returns undefined when the count couldn't be worked out; the cached board + * is cleared then, so a stale one is never served. + */ +async function countForProgram( + env: Env, + cfg: Config, + program: Program, + e: PullRequestEvent, + mergedAt: Date +): Promise[1] | undefined> { + const pr = e.pull_request; + try { + // GitHub's search index can lag a few seconds behind the merge, so add + // this event's PR, and the ones merged just before it that search doesn't + // have yet either, before counting and caching the leaderboard. + const login = pr.user.login.toLowerCase(); + const searched = await searchCountedPrs(env, cfg, program); + const indexed = new Set(searched.map((p) => p.url)); + const pending = [ + ...(await recentMerges(env, program)), + { + login: pr.user.login, + mergedAt: mergedAt.toISOString(), + url: pr.html_url, + title: pr.title, + repo: e.repository.name, + number: pr.number, + }, + ].filter((p, i, all) => !indexed.has(p.url) && all.findIndex((q) => q.url === p.url) === i); + await rememberMerges(env, program, pending); + const prs = [...searched, ...pending]; + await cacheLeaderboard(env, cfg, program, prs); + const others = prs.filter( + (p) => p.login.toLowerCase() === login && p.url !== pr.html_url && Date.parse(p.mergedAt) <= mergedAt.getTime() + ); + const count = others.length + 1; + return { + count, + label: program.title, + // Said once, on the merge that takes them to the minimum. + qualified: count === program.minPrs ? { topN: program.topN, reward: program.reward, note: program.note } : undefined, + }; + } catch { + // The shoutout matters more than the count; don't serve an old board. + await invalidateLeaderboard(env, program); + return undefined; + } +} + async function onPullRequest(e: PullRequestEvent, env: Env, cfg: Config): Promise { const pr = e.pull_request; - if (e.action !== "closed" || !pr.merged || !ours(cfg, e.repository)) return "ignored pull request"; + if (!ours(cfg, e.repository)) return "ignored pull request"; if (pr.user.type === "Bot" || cfg.excludedUsers.has(pr.user.login.toLowerCase())) return "ignored pull request author"; + if (e.action === "labeled") return onLateLabel(e, env, cfg); + if (e.action !== "closed" || !pr.merged) return "ignored pull request"; - let countInWindow: Parameters[1]; const mergedAt = pr.merged_at ? new Date(pr.merged_at) : new Date(); // The program this PR counts for, if any. const program = cfg.programs.find( @@ -214,43 +282,7 @@ async function onPullRequest(e: PullRequestEvent, env: Env, cfg: Config): Promis mergedAt >= p.start && mergedAt <= p.end ); - if (program) { - try { - // GitHub's search index can lag a few seconds behind the merge, so add - // this event's PR, and the ones merged just before it that search doesn't - // have yet either, before counting and caching the leaderboard. - const login = pr.user.login.toLowerCase(); - const searched = await searchCountedPrs(env, cfg, program); - const indexed = new Set(searched.map((p) => p.url)); - const pending = [ - ...(await recentMerges(env, program)), - { - login: pr.user.login, - mergedAt: mergedAt.toISOString(), - url: pr.html_url, - title: pr.title, - repo: e.repository.name, - number: pr.number, - }, - ].filter((p, i, all) => !indexed.has(p.url) && all.findIndex((q) => q.url === p.url) === i); - await rememberMerges(env, program, pending); - const prs = [...searched, ...pending]; - await cacheLeaderboard(env, cfg, program, prs); - const others = prs.filter( - (p) => p.login.toLowerCase() === login && p.url !== pr.html_url && Date.parse(p.mergedAt) <= mergedAt.getTime() - ); - const count = others.length + 1; - countInWindow = { - count, - label: program.title, - // Said once, on the merge that takes them to the minimum. - qualified: count === program.minPrs ? { topN: program.topN, reward: program.reward, note: program.note } : undefined, - }; - } catch { - // The shoutout matters more than the count; don't serve an old board. - await invalidateLeaderboard(env, program); - } - } + const countInWindow = program ? await countForProgram(env, cfg, program, e, mergedAt) : undefined; // If the author linked their GitHub account, show who they are here and // hand out any role this merge earned. The shoutout goes out either way. @@ -265,7 +297,7 @@ async function onPullRequest(e: PullRequestEvent, env: Env, cfg: Config): Promis } } - await sendMessage( + const messageId = await sendMessage( env, env.CHANNEL_MERGED_PRS, mergedPrMessage( @@ -282,5 +314,72 @@ async function onPullRequest(e: PullRequestEvent, env: Env, cfg: Config): Promis countInWindow ) ); + // Kept so a program label added after the merge can edit this shoutout + // instead of posting a second one. It expires on its own. + await env.STATE.put( + shoutoutKey(e.repository.name, pr.number), + JSON.stringify({ + messageId, + counted: countInWindow !== undefined, + discordId: link?.discordId, + linking: linkingEnabled(env), + newRoles, + } satisfies ShoutoutRecord), + { expirationTtl: SHOUTOUT_TTL_SECONDS } + ); return "posted merged pull request"; } + +/** + * A program label added after the merge, e.g. by the accepted-label workflow + * when a maintainer missed it. The PR already has its shoutout, so this edits + * that message to the text it would have had at the merge, and refreshes the + * board. A PR whose shoutout already has the count, or a label that isn't a + * program's, changes nothing. + */ +async function onLateLabel(e: PullRequestEvent, env: Env, cfg: Config): Promise { + const pr = e.pull_request; + if (!pr.merged || !pr.merged_at) return "ignored pull request"; + const added = e.label?.name; + if (!added) return "ignored pull request"; + const mergedAt = new Date(pr.merged_at); + const program = cfg.programs.find( + (p) => p.repos.includes(e.repository.name) && p.label === added && mergedAt >= p.start && mergedAt <= p.end + ); + if (!program) return "ignored pull request"; + + const record = await env.STATE.get(shoutoutKey(e.repository.name, pr.number), "json"); + if (record?.counted) return "ignored pull request"; + + const countInWindow = await countForProgram(env, cfg, program, e, mergedAt); + if (!countInWindow) return "late label, shoutout unchanged"; + // No stored shoutout (it predates this, or its record expired): the board + // is refreshed, but there's no message to edit, and guessing at one would + // risk editing the wrong message. + if (!record) return "late label, board refreshed"; + + await editMessage( + env, + env.CHANNEL_MERGED_PRS, + record.messageId, + mergedPrMessage( + { + repo: e.repository.name, + number: pr.number, + title: pr.title, + url: pr.html_url, + login: pr.user.login, + discordId: record.discordId, + linking: record.linking, + newRoles: record.newRoles, + }, + countInWindow + ) + ); + await env.STATE.put( + shoutoutKey(e.repository.name, pr.number), + JSON.stringify({ ...record, counted: true } satisfies ShoutoutRecord), + { expirationTtl: SHOUTOUT_TTL_SECONDS } + ); + return "updated merged pull request shoutout"; +} diff --git a/test/card.test.ts b/test/card.test.ts index 37f56f8..07b8a83 100644 --- a/test/card.test.ts +++ b/test/card.test.ts @@ -2,8 +2,8 @@ import { beforeEach, afterEach, describe, expect, it, vi } from "vitest"; import { boardSvg, boardVersion, cardSvg, escapeXml, fitMono, progressBar } from "../src/card"; import { getConfig } from "../src/config"; import worker from "../src/index"; -import { contributorPage } from "../src/page"; -import { FakeCache, makeCtx, makeEnv, mockFetch } from "./helpers"; +import { contributorPage, noticePage } from "../src/page"; +import { FakeCache, makeCtx, makeEnv, mockFetch, OCTOBER } from "./helpers"; // The real module needs the WebAssembly rasterizer and the font files, which // only the Workers bundler can load. The route is tested with a stand-in. @@ -266,6 +266,24 @@ describe("contributorPage", () => { }); }); +describe("noticePage links", () => { + it("shows supplied links with the cta styling and escapes labels and destinations", () => { + const html = noticePage("No card yet", ["Nothing here."], false, "No card", [ + { label: ``, href: `https://example.com/?a="b"&c=` }, + ]); + expect(html).toContain('
      '); + expect(html).toContain('href="https://example.com/?a="b"&c=<d>"'); + expect(html).toContain("<script>"label"</script>"); + expect(html).not.toContain("', 6, true, true)] }, + program, + links, + during, + { linking: true, members: new Map() } + ); + expect(evil).not.toContain(""); + expect(evil).toContain("<script>alert(1)</script>"); + }); + + it("shows the unlinked line at phone width", () => { + const htmlWithNotice = leaderboardPage(board, program, links, during, { linking: true, members: new Map() }); + expect(htmlWithNotice).toContain('class="unlinked"'); + expect(htmlWithNotice).toContain(".chip,.count-label{display:none}"); + expect(htmlWithNotice).not.toContain(".unlinked{display:none}"); + }); + it("links to issues, Discord and the platform, with escaped URLs", () => { expect(html).toContain('href="https://github.com/search?q=x&type=issues"'); expect(html).toContain('href="https://discord.gg/example"'); @@ -141,6 +227,12 @@ describe("leaderboardPage", () => { expect(html).toContain(''); }); + it("declares the leaderboard page as an Open Graph website", () => { + expect(html.match(//g)).toEqual([ + '', + ]); + }); + it("has the same share preview tags as a contributor page, pointing at its own image", () => { expect(html).toContain(''); expect(html).toContain(''); @@ -328,7 +420,13 @@ describe("GET /leaderboard", () => { const json = await worker.fetch(new Request("https://bot.example/leaderboard/2026-10.json"), env, ctx); expect(((await json.json()) as { id: string }).id).toBe("2026-10"); - expect((await worker.fetch(new Request("https://bot.example/leaderboard/2025-10"), env, ctx)).status).toBe(404); + const unknownHtml = await worker.fetch(new Request("https://bot.example/leaderboard/2025-10"), env, ctx); + expect(unknownHtml.status).toBe(404); + expect(unknownHtml.headers.get("Access-Control-Allow-Origin")).toBeNull(); + expect(await unknownHtml.text()).toBe("not found"); + const unknownJson = await worker.fetch(new Request("https://bot.example/leaderboard/2025-10.json"), env, ctx); + expect(unknownJson.status).toBe(404); + expect(unknownJson.headers.get("Access-Control-Allow-Origin")).toBe("*"); expect((await worker.fetch(new Request("https://bot.example/leaderboard", { method: "POST" }), env, ctx)).status).toBe(404); const none = await worker.fetch(new Request("https://bot.example/leaderboard"), makeEnv({ PROGRAMS: [] }), ctx); expect(none.status).toBe(404); @@ -362,5 +460,94 @@ describe("GET /leaderboard", () => { const res = await worker.fetch(new Request("https://bot.example/leaderboard"), env, ctx); expect(res.status).toBe(503); expect(res.headers.get("Retry-After")).toBe("60"); + expect(res.headers.get("Access-Control-Allow-Origin")).toBeNull(); + expect(await res.text()).toBe("GitHub is rate limiting us. Try again in a minute."); + const json = await worker.fetch(new Request("https://bot.example/leaderboard.json"), env, ctx); + expect(json.status).toBe(503); + expect(json.headers.get("Content-Type")).toBe("text/plain; charset=utf-8"); + expect(json.headers.get("Access-Control-Allow-Origin")).toBe("*"); + expect(await json.text()).toBe("GitHub is rate limiting us. Try again in a minute."); + }); + + it("names qualifying unlinked members when linking is enabled", async () => { + env = makeEnv({ LINKING: "on", DISCORD_CLIENT_SECRET: "secret" }); + mockFetch(() => ({ + total_count: 11, + items: [ + ...Array.from({ length: 6 }, (_, i) => ({ + html_url: `https://github.com/codercops/toolbelt/pull/${i + 1}`, + number: i + 1, + title: "PR", + repository_url: "https://api.github.com/repos/codercops/toolbelt", + user: { login: "ada" }, + pull_request: { merged_at: "2026-10-02T00:00:00Z" }, + })), + ...Array.from({ length: 5 }, (_, i) => ({ + html_url: `https://github.com/codercops/toolbelt/pull/${i + 7}`, + number: i + 7, + title: "PR", + repository_url: "https://api.github.com/repos/codercops/toolbelt", + user: { login: "bob" }, + pull_request: { merged_at: "2026-10-03T00:00:00Z" }, + })), + ], + })); + + const res = await worker.fetch(new Request("https://bot.example/leaderboard"), env, ctx); + expect(res.status).toBe(200); + const text = await res.text(); + expect(text).toContain('class="unlinked"'); + expect(text).toContain(">ada<"); + expect(text).toContain(">bob<"); + expect(text).toContain("qualify but haven't linked their GitHub yet"); + }); + + it("only looks up qualifying rows in KV and keeps the page working if the lookup fails", async () => { + env = makeEnv({ LINKING: "on", DISCORD_CLIENT_SECRET: "secret" }); + mockFetch(() => ({ + total_count: 6, + items: [ + ...Array.from({ length: 5 }, (_, i) => ({ + html_url: `https://github.com/codercops/toolbelt/pull/${i + 1}`, + number: i + 1, + title: "PR", + repository_url: "https://api.github.com/repos/codercops/toolbelt", + user: { login: "ada" }, + pull_request: { merged_at: "2026-10-02T00:00:00Z" }, + })), + { + html_url: "https://github.com/codercops/toolbelt/pull/6", + number: 6, + title: "PR", + repository_url: "https://api.github.com/repos/codercops/toolbelt", + user: { login: "nonqualifying" }, + pull_request: { merged_at: "2026-10-02T00:00:00Z" }, + }, + ], + })); + + const lookups: string[] = []; + const origGet = env.STATE.get.bind(env.STATE); + env.STATE.get = (async (key: string, type?: "json") => { + lookups.push(key); + return origGet(key, type as any); + }) as any; + + const res = await worker.fetch(new Request("https://bot.example/leaderboard"), env, ctx); + expect(res.status).toBe(200); + // Only ada qualified (5 PRs >= minPrs 5), nonqualifying had 1 PR + const linkLookups = lookups.filter((k) => k.startsWith("link:github:")); + expect(linkLookups).toEqual(["link:github:ada"]); + expect(linkLookups).not.toContain("link:github:nonqualifying"); + + // If KV lookup throws for link keys, page still renders 200 without the line + env.STATE.get = (async (key: string, type?: "json") => { + if (key.startsWith("link:github:")) throw new Error("KV read failed"); + return origGet(key, type as any); + }) as any; + const failRes = await worker.fetch(new Request("https://bot.example/leaderboard"), env, ctx); + expect(failRes.status).toBe(200); + const failHtml = await failRes.text(); + expect(failHtml).not.toContain('class="unlinked"'); }); -}); \ No newline at end of file +}); diff --git a/test/webhook.test.ts b/test/webhook.test.ts index 9248e0f..1db29d5 100644 --- a/test/webhook.test.ts +++ b/test/webhook.test.ts @@ -301,9 +301,9 @@ describe("starter issue feed", () => { describe("releases", () => { const release = { name: "v0.6.2", tag_name: "v0.6.2", body: "Fixes", html_url: "https://github.com/codercops/toolbelt/releases/tag/v0.6.2", draft: false, prerelease: false }; - it("posts published releases to the releases channel", async () => { + it("posts releases to the releases channel", async () => { const fetched = mockFetch(); - expect(await handleGithubEvent("release", { action: "published", repository, release }, env, cfg)).toBe("posted release"); + expect(await handleGithubEvent("release", { action: "released", repository, release }, env, cfg)).toBe("posted release"); expect(fetched.discord()[0]?.url).toBe("https://discord.com/api/v10/channels/chan-releases/messages"); expect(fetched.discord()[0]?.body.embeds[0].title).toBe("toolbelt v0.6.2"); }); @@ -311,10 +311,19 @@ describe("releases", () => { it("ignores drafts, prereleases and other actions", async () => { const fetched = mockFetch(); await handleGithubEvent("release", { action: "created", repository, release }, env, cfg); - await handleGithubEvent("release", { action: "published", repository, release: { ...release, draft: true } }, env, cfg); - await handleGithubEvent("release", { action: "published", repository, release: { ...release, prerelease: true } }, env, cfg); + await handleGithubEvent("release", { action: "published", repository, release }, env, cfg); + await handleGithubEvent("release", { action: "released", repository, release: { ...release, draft: true } }, env, cfg); + await handleGithubEvent("release", { action: "released", repository, release: { ...release, prerelease: true } }, env, cfg); expect(fetched.discord()).toHaveLength(0); }); + + it("posts a prerelease once it's promoted to a full release", async () => { + const fetched = mockFetch(); + await handleGithubEvent("release", { action: "published", repository, release: { ...release, prerelease: true } }, env, cfg); + await handleGithubEvent("release", { action: "edited", repository, release }, env, cfg); + expect(await handleGithubEvent("release", { action: "released", repository, release }, env, cfg)).toBe("posted release"); + expect(fetched.discord()).toHaveLength(1); + }); }); describe("merged pull requests", () => { @@ -609,6 +618,123 @@ describe("merged pull requests", () => { }); }); +describe("late program label on a merged pull request", () => { + const mergedPr = (over: Record = {}) => ({ + number: 139, + title: "fix(url-encoder): show an error", + html_url: "https://github.com/codercops/toolbelt/pull/139", + repository_url: "https://api.github.com/repos/codercops/toolbelt", + merged: true, + merged_at: "2026-10-11T16:20:00Z", + user: { login: "ada", type: "User" }, + labels: [{ name: "hacktoberfest-accepted" }], + ...over, + }); + const prEvent = (action: string, overPr: Record = {}, label?: string) => ({ + action, + repository, + ...(label ? { label: { name: label } } : {}), + pull_request: mergedPr(overPr), + }); + const earlier = (n: number) => ({ + total_count: n, + items: Array.from({ length: n }, (_, i) => ({ + html_url: `https://github.com/codercops/toolbelt/pull/${i + 1}`, + number: i + 1, + title: "Fix a thing", + repository_url: "https://api.github.com/repos/codercops/toolbelt", + user: { login: "ada" }, + pull_request: { merged_at: `2026-10-0${i + 1}T00:00:00Z` }, + })), + }); + // A merge that arrived without the program label, so its shoutout has no count yet. + // No search runs for it, so the mock's search answer doesn't matter here. + const postUncountedShoutout = async () => { + await handleGithubEvent("pull_request", prEvent("closed", { labels: [] }), env, cfg); + }; + const board = () => JSON.parse(env.STATE.store.get("leaderboard:v3:2026-10") ?? "{}"); + + it("edits the shoutout to the count and qualify lines, and refreshes the board", async () => { + const fetched = mockFetch(() => earlier(4)); + await postUncountedShoutout(); + expect(fetched.discord()).toHaveLength(1); + expect(fetched.discord()[0]?.body.embeds[0].description).not.toContain("That's their"); + fetched.clear(); + + expect(await handleGithubEvent("pull_request", prEvent("labeled", {}, "hacktoberfest-accepted"), env, cfg)).toBe( + "updated merged pull request shoutout" + ); + const edits = fetched.discord(); + expect(edits).toHaveLength(1); + expect(edits[0]?.method).toBe("PATCH"); + expect(edits[0]?.url).toBe("https://discord.com/api/v10/channels/chan-merged/messages/msg-1"); + const description = edits[0]?.body.embeds[0].description as string; + expect(description).toContain("That's their 5th merged PR on the Hacktoberfest 2026 leaderboard."); + expect(description).toContain("With 5 they now qualify"); + expect(board().rows).toEqual([expect.objectContaining({ login: "ada", count: 5 })]); + }); + + it("edits in the count without the qualify line when the minimum isn't reached", async () => { + const fetched = mockFetch(() => earlier(1)); + await postUncountedShoutout(); + fetched.clear(); + await handleGithubEvent("pull_request", prEvent("labeled", {}, "hacktoberfest-accepted"), env, cfg); + const description = fetched.discord()[0]?.body.embeds[0].description as string; + expect(description).toContain("That's their 2nd merged PR"); + expect(description).not.toContain("they now qualify"); + expect(board().rows).toEqual([expect.objectContaining({ login: "ada", count: 2 })]); + }); + + it("doesn't count twice when search already includes this PR", async () => { + const fetched = mockFetch(() => ({ + total_count: 5, + items: [ + ...earlier(4).items, + { html_url: "https://github.com/codercops/toolbelt/pull/139", number: 139, title: "fix(url-encoder): show an error", repository_url: "https://api.github.com/repos/codercops/toolbelt", user: { login: "ada" }, pull_request: { merged_at: "2026-10-11T16:20:00Z" } }, + ], + })); + await postUncountedShoutout(); + fetched.clear(); + await handleGithubEvent("pull_request", prEvent("labeled", {}, "hacktoberfest-accepted"), env, cfg); + expect(fetched.discord()[0]?.body.embeds[0].description).toContain("That's their 5th merged PR"); + expect(board().rows).toEqual([expect.objectContaining({ login: "ada", count: 5 })]); + }); + + it("changes nothing for an open PR, another label, or a shoutout that already has the count", async () => { + const fetched = mockFetch(() => earlier(0)); + expect(await handleGithubEvent("pull_request", prEvent("labeled", { merged: false, merged_at: null }, "hacktoberfest-accepted"), env, cfg)).toBe("ignored pull request"); + expect(await handleGithubEvent("pull_request", prEvent("labeled", { labels: [{ name: "bug" }] }, "bug"), env, cfg)).toBe("ignored pull request"); + expect(await handleGithubEvent("pull_request", prEvent("labeled", { merged_at: "2026-11-02T00:00:00Z" }, "hacktoberfest-accepted"), env, cfg)).toBe("ignored pull request"); + expect(fetched.discord()).toHaveLength(0); + + // This merge had the label all along, so its shoutout already has the count. + await handleGithubEvent("pull_request", prEvent("closed"), env, cfg); + fetched.clear(); + expect(await handleGithubEvent("pull_request", prEvent("labeled", {}, "hacktoberfest-accepted"), env, cfg)).toBe("ignored pull request"); + expect(fetched.discord()).toHaveLength(0); + }); + + it("refreshes the board but doesn't edit when the shoutout record is gone", async () => { + const fetched = mockFetch(() => earlier(4)); + await postUncountedShoutout(); + await env.STATE.delete("merged-pr:toolbelt#139"); + fetched.clear(); + expect(await handleGithubEvent("pull_request", prEvent("labeled", {}, "hacktoberfest-accepted"), env, cfg)).toBe("late label, board refreshed"); + expect(fetched.discord()).toHaveLength(0); + expect(board().rows).toEqual([expect.objectContaining({ login: "ada", count: 5 })]); + }); + + it("leaves the shoutout alone and clears the board when search fails", async () => { + mockFetch(() => earlier(4)); + await postUncountedShoutout(); + await env.STATE.put("leaderboard:v3:2026-10", JSON.stringify({ rows: [], generatedAt: "earlier" })); + const fetched = mockFetch(() => new Response("unavailable", { status: 503 })); + expect(await handleGithubEvent("pull_request", prEvent("labeled", {}, "hacktoberfest-accepted"), env, cfg)).toBe("late label, shoutout unchanged"); + expect(fetched.discord()).toHaveLength(0); + expect(env.STATE.store.has("leaderboard:v3:2026-10")).toBe(false); + }); +}); + const rateLimit = () => new Response(JSON.stringify({ retry_after: 30 }), { status: 429 }); const onGithub = (over: Record = {}) => () => ({ ...issueEvent("edited").issue, ...over });