Skip to content

Commit 7ba7bc8

Browse files
scosmanclaude
andcommitted
Add /nathan pr_report command for on-demand PR reports via DM
Extract generateReport from postDailyReport so both the scheduled daily report and the new on-demand command share the same sweep, input gathering, and report building. The on-demand command acks immediately, then sends the full report as a DM to the requesting user (threaded for multi-part reports). It does not record the report, so the next scheduled daily report is unaffected. Also allows underscores in subcommand names (COMMAND_NAME regex). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 4399422 commit 7ba7bc8

7 files changed

Lines changed: 224 additions & 10 deletions

File tree

‎docs/setup.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ Before starting: steps 1–7 done for staging, `npm run verify:github -- staging
195195

196196
- [ ] `/healthz` returns `{"env":"staging",…}`.
197197
- [ ] Slack's event URL is *Verified*, and GitHub's `ping` redelivery got a `202`.
198-
- [ ] The Home tab shows Nathan's sections; `/nathan-staging help` lists `prs`.
198+
- [ ] The Home tab shows Nathan's sections; `/nathan-staging help` lists `prs` and `pr_report`.
199199

200200
**Request PR form**
201201

‎src/features/pr_management/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { GITHUB_EVENTS } from "../../github";
33
import { prConfigSchema } from "./config";
44
import { createPeople } from "./people";
55
import { registerPersonalQueue } from "./personal_queue";
6+
import { registerPRReport } from "./pr_report";
67
import { type PRContext, refreshPullRequest } from "./refresh";
78
import { postDailyReport, reportSchedule } from "./report";
89
import { registerRequestPR } from "./request";
@@ -50,5 +51,6 @@ export const prManagement = defineFeature({
5051

5152
registerRequestPR(registrar, ctx);
5253
registerPersonalQueue(registrar, ctx);
54+
registerPRReport(registrar, ctx);
5355
},
5456
});
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import type { Registrar } from "../../core/feature";
2+
import type { PRConfig } from "./config";
3+
import type { PRContext } from "./refresh";
4+
import { generateReport } from "./report";
5+
6+
// On-demand PR report (task: nathan-pr-report-command): the same report the daily schedule would
7+
// post, delivered as a DM to whoever ran the command. Does not record the report, so the next
8+
// scheduled one is unaffected.
9+
10+
export const PR_REPORT_COMMAND = "pr_report";
11+
export const ACK_TEXT = "Building your PR report — I'll send it to you as a DM.";
12+
13+
export function registerPRReport(registrar: Registrar<PRConfig>, ctx: PRContext): void {
14+
const { services } = ctx;
15+
16+
registrar.slack.command(PR_REPORT_COMMAND, {
17+
description: "Get the current PR report as a DM",
18+
ack: async () => ACK_TEXT,
19+
lazy: async (request) => {
20+
const now = services.clock.now();
21+
const {
22+
messages: [first, ...rest],
23+
} = await generateReport(ctx, now);
24+
const posted = await services.slack.sendDirectMessage(request.userId, {
25+
text: first.text,
26+
blocks: first.blocks,
27+
});
28+
for (const part of rest) {
29+
await services.slack.postMessage({
30+
channel: posted.channel,
31+
thread_ts: posted.ts,
32+
text: part.text,
33+
blocks: part.blocks,
34+
});
35+
}
36+
},
37+
});
38+
}

‎src/features/pr_management/report.ts‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,16 @@ export function buildReport(input: ReportInput): [ReportMessage, ...ReportMessag
107107
return [{ text, blocks: [...blocks.slice(0, firstRoom), context(CONTINUED_TEXT)] }, ...rest];
108108
}
109109

110+
export interface GeneratedReport {
111+
messages: [ReportMessage, ...ReportMessage[]];
112+
openCount: number;
113+
}
114+
110115
/**
111-
* The `daily_report` task. Runs the sweep first so the report reflects GitHub now; if the sweep
112-
* fails, it is reported and the report goes out from the stored records.
116+
* Sweeps, gathers inputs and builds the report. Does not post or record the report, so callers
117+
* control where it goes and whether it counts as a scheduled report.
113118
*/
114-
export async function postDailyReport(ctx: PRContext, slot: DateTime): Promise<void> {
119+
export async function generateReport(ctx: PRContext, slot: DateTime): Promise<GeneratedReport> {
115120
const { config, services, store } = ctx;
116121
try {
117122
await sweep(ctx);
@@ -130,7 +135,7 @@ export async function postDailyReport(ctx: PRContext, slot: DateTime): Promise<v
130135
services.github.reader.recentPullRequests(config.repos, historySince),
131136
]);
132137

133-
const [first, ...rest] = buildReport({
138+
const messages = buildReport({
134139
slot,
135140
now,
136141
since,
@@ -144,8 +149,23 @@ export async function postDailyReport(ctx: PRContext, slot: DateTime): Promise<v
144149
triager: config.triager,
145150
oldDraftDays: config.drafts.reportAfterDays,
146151
});
152+
153+
return { messages, openCount: openPRs(records).length };
154+
}
155+
156+
/**
157+
* The `daily_report` task. Runs the sweep first so the report reflects GitHub now; if the sweep
158+
* fails, it is reported and the report goes out from the stored records.
159+
*/
160+
export async function postDailyReport(ctx: PRContext, slot: DateTime): Promise<void> {
161+
const {
162+
messages: [first, ...rest],
163+
openCount,
164+
} = await generateReport(ctx, slot);
165+
const { config, services, store } = ctx;
166+
const now = services.clock.now();
147167
const posted = await services.slack.postMessage({ channel: config.channel, ...first });
148-
await store.saveReport({ slot, openCount: openPRs(records).length }, now);
168+
await store.saveReport({ slot, openCount }, now);
149169
for (const part of rest) {
150170
await services.slack.postMessage({ channel: posted.channel, thread_ts: posted.ts, ...part });
151171
}

‎src/slack/registry.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export interface Registered<H> {
117117
}
118118

119119
export const HELP_COMMAND = "help";
120-
const COMMAND_NAME = /^[a-z][a-z0-9-]*$/;
120+
const COMMAND_NAME = /^[a-z][a-z0-9_-]*$/;
121121

122122
/** All features' Slack registrations. IDs are global, so a clash between features fails at startup. */
123123
export class SlackHandlers {
@@ -134,7 +134,8 @@ export class SlackHandlers {
134134
add(this.viewSubmissions, "view submission", callbackId, { featureId, handler }),
135135
action: (actionId, handler) => add(this.actions, "action", actionId, { featureId, handler }),
136136
command: (name, handler) => {
137-
if (!COMMAND_NAME.test(name)) throw new Error(`Subcommand "${name}" must be lowercase letters, digits and -`);
137+
if (!COMMAND_NAME.test(name))
138+
throw new Error(`Subcommand "${name}" must be lowercase letters, digits, _ and -`);
138139
if (name === HELP_COMMAND) throw new Error(`Subcommand "${HELP_COMMAND}" is built in`);
139140
add(this.commands, "subcommand", name, { featureId, handler });
140141
},
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
import { createExecutionContext, waitOnExecutionContext } from "cloudflare:test";
2+
import { DateTime } from "luxon";
3+
import { describe, expect, it, vi } from "vitest";
4+
import { ACK_TEXT } from "../../../src/features/pr_management/pr_report";
5+
import { GitHubApiError } from "../../../src/github";
6+
import type { MessageBlock } from "../../../src/slack";
7+
import { aPR, aPRHistory } from "../../builders/github";
8+
import { aRecord } from "../../builders/pr_record";
9+
import { type PRTestApp, prApp, REPO } from "../../helpers/pr";
10+
import { commandBody, signedSlackRequest } from "../../helpers/slack";
11+
12+
const at = (iso: string) => DateTime.fromISO(iso, { zone: "utc" });
13+
14+
async function send(h: PRTestApp, body: string) {
15+
const ctx = createExecutionContext();
16+
const response = await h.app.fetch(await signedSlackRequest(body), ctx);
17+
const text = await response.text();
18+
await waitOnExecutionContext(ctx);
19+
return { status: response.status, text };
20+
}
21+
22+
const dms = (h: PRTestApp) => h.slack.dms;
23+
const PR_CHANNEL = "CPRS";
24+
const channelReports = (h: PRTestApp) =>
25+
h.slack.posts.filter((p) => p.channel === PR_CHANNEL && p.text.startsWith("PR report"));
26+
27+
/** Every mrkdwn line from blocks. */
28+
function sectionLines(blocks: readonly MessageBlock[]): string[] {
29+
return blocks.flatMap((block) => (block.type === "section" && block.text ? block.text.text.split("\n") : []));
30+
}
31+
32+
/** Ticks the scheduler at `iso` so a scheduled report is recorded. */
33+
async function tickAt(h: PRTestApp, iso: string) {
34+
h.clock.set(iso);
35+
await h.app.scheduled(Date.parse(iso));
36+
}
37+
38+
describe("/nathan pr_report", () => {
39+
it("acks with a brief message and sends the report as a DM", async () => {
40+
const h = prApp();
41+
h.github.upsert(aPR({ pendingReviewers: ["bob"] }));
42+
h.clock.set("2026-10-09T15:00:00Z");
43+
await h.refresh(101);
44+
45+
const result = await send(h, commandBody("pr_report"));
46+
47+
expect(result.status).toBe(200);
48+
expect(result.text).toBe(ACK_TEXT);
49+
expect(dms(h)).toHaveLength(1);
50+
expect(dms(h)[0]?.userId).toBe("UALICE");
51+
expect(dms(h)[0]?.message.text).toContain("PR report");
52+
expect(dms(h)[0]?.message.text).toContain("1 open PR");
53+
});
54+
55+
it("does not post to the PR channel", async () => {
56+
const h = prApp();
57+
h.clock.set("2026-10-09T15:00:00Z");
58+
await send(h, commandBody("pr_report"));
59+
60+
expect(channelReports(h)).toHaveLength(0);
61+
});
62+
63+
it("does not record the report, so the next scheduled one is unaffected", async () => {
64+
const h = prApp();
65+
h.github.upsert(aPR({ number: 1, createdAt: at("2026-10-08T10:00:00Z") }));
66+
67+
// A scheduled report at 09:30 records open count = 1.
68+
await tickAt(h, "2026-10-08T13:30:00Z");
69+
expect(channelReports(h)).toHaveLength(1);
70+
71+
// On-demand report at 14:00: a second PR exists now.
72+
h.github.upsert(aPR({ number: 2, createdAt: at("2026-10-08T14:00:00Z") }));
73+
h.clock.set("2026-10-08T18:00:00Z");
74+
await send(h, commandBody("pr_report"));
75+
expect(dms(h)).toHaveLength(1);
76+
77+
// Next scheduled report should still compare against the 09:30 report (open count = 1),
78+
// not the on-demand one. The blocks contain "+1 since the last report".
79+
await tickAt(h, "2026-10-09T13:30:00Z");
80+
const lastReport = channelReports(h).at(-1);
81+
const lines = sectionLines((lastReport?.blocks ?? []) as MessageBlock[]);
82+
expect(lines.some((line) => line.includes("+1 since the last report"))).toBe(true);
83+
});
84+
85+
it("includes weekly sections on Monday", async () => {
86+
const h = prApp();
87+
h.github.history.push(
88+
aPRHistory({
89+
author: "bob",
90+
createdAt: at("2026-10-05T10:00:00Z"),
91+
mergedAt: at("2026-10-07T10:00:00Z"),
92+
closedAt: at("2026-10-07T10:00:00Z"),
93+
}),
94+
);
95+
// Monday in New York (EDT, UTC-4).
96+
h.clock.set("2026-10-12T16:00:00Z");
97+
await send(h, commandBody("pr_report"));
98+
99+
const lines = sectionLines((dms(h)[0]?.message.blocks ?? []) as MessageBlock[]);
100+
expect(lines.some((line) => line.startsWith("*Trends*"))).toBe(true);
101+
expect(lines.some((line) => line.startsWith("*People*"))).toBe(true);
102+
});
103+
104+
it("does not include weekly sections on a non-Monday", async () => {
105+
const h = prApp();
106+
// Wednesday in New York.
107+
h.clock.set("2026-10-07T16:00:00Z");
108+
await send(h, commandBody("pr_report"));
109+
110+
const lines = sectionLines((dms(h)[0]?.message.blocks ?? []) as MessageBlock[]);
111+
expect(lines.some((line) => line.startsWith("*Trends*"))).toBe(false);
112+
});
113+
114+
it("threads multi-part reports in the DM", async () => {
115+
const h = prApp();
116+
vi.spyOn(h.github.reader, "openPullRequests").mockRejectedValue(new GitHubApiError("down", 502));
117+
for (let number = 1; number <= 900; number++) {
118+
await h.store.insert(
119+
aRecord({
120+
repo: REPO,
121+
number,
122+
title: `A very long PR title that keeps going and going to fill the line ${"x".repeat(40)}`,
123+
category: "oss",
124+
author: "outsider",
125+
state: "awaiting_review",
126+
owners: ["bob"],
127+
stateSince: at("2026-10-09T13:00:00Z"),
128+
}),
129+
);
130+
}
131+
h.clock.set("2026-10-09T15:00:00Z");
132+
133+
await send(h, commandBody("pr_report"));
134+
135+
// First part is a DM.
136+
expect(dms(h)).toHaveLength(1);
137+
expect(dms(h)[0]?.userId).toBe("UALICE");
138+
139+
// Continuation parts are posted to the DM channel, threaded under the first message.
140+
const dmChannel = `DUALICE`;
141+
const threadParts = h.slack.posts.filter((p) => p.channel === dmChannel && p.thread_ts);
142+
expect(threadParts.length).toBeGreaterThan(0);
143+
// Nothing went to the PR channel.
144+
expect(channelReports(h)).toHaveLength(0);
145+
});
146+
147+
it("shows in /nathan help", async () => {
148+
const h = prApp();
149+
const result = await send(h, commandBody("help"));
150+
expect(result.text).toContain("pr_report");
151+
expect(result.text).toContain("PR report");
152+
});
153+
});

‎test/slack/registry.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ describe("SlackHandlers", () => {
2525
expect(() => register(handlers.forFeature("beta"))).toThrow(`Slack ${kind} "x" is already registered by alpha`);
2626
});
2727

28-
it.each(["Prs", "my_cmd", "-x", ""])("rejects the subcommand name %j", (name) => {
28+
it.each(["Prs", "-x", ""])("rejects the subcommand name %j", (name) => {
2929
expect(() => new SlackHandlers().forFeature("alpha").command(name, { description: "d" })).toThrow(
30-
"must be lowercase letters, digits and -",
30+
"must be lowercase letters, digits, _ and -",
3131
);
3232
});
3333

0 commit comments

Comments
 (0)