diff --git a/AGENTS.md b/AGENTS.md index eb0645b1815..9865f54af82 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -127,15 +127,32 @@ When implementation work for a user request is done (code, docs, config — not `fork/changes`. 5. Never assume an earlier PR in the session is still open. -## Discord-originated pull requests +## Discord-originated commits (REQUIRED) -When opening a PR from a Discord thread request, append this footer at the end of the PR description (use the current requester and that thread’s real jump link): +When the Discord turn includes an **Identity map** block with ready-to-paste `Co-authored-by` trailers, attribution is **mandatory**, not optional: + +1. Keep the environment default **author/committer** (usually the GitHub App bot). +2. **Every** `git commit` you create for that work MUST end with those exact trailers after a blank line. Do not invent emails for unmapped people. +3. Before `git push` / opening a PR, verify with `git log -1 --format=%B` that the trailers are present on each new commit. +4. A Discord-originated commit **without** the mapped trailers is incomplete — fix it (amend if not pushed, or a follow-up commit is not enough for GitHub multi-author on already-pushed SHAs; amend/rebase when safe). + +GitHub multi-author avatars (`bot & human`) come from commit trailers, not from PR body prose alone. + +## Discord-originated pull requests (REQUIRED) + +When opening or updating a PR from a Discord thread: + +1. **Discord footer (required in the PR description).** Append this exact footer form at the end of the PR body (use the **thread starter** when known, otherwise the current requester, and that thread’s real jump link): ```md opened by [](discord_user_id) in chat thread **Discord** · [Thread Title](https://discord.com/channels///) ``` -If Discord turn context lists **Linked work items** / Jira issues for the thread, include those Jira issue links in the PR description (and prefer the primary key in the title/branch when one is clear). +Prefer the thread starter’s Discord id/display name from turn context. Do not skip this because the bot _might_ patch the body later — still write it when you create the PR so the first revision is correct. The bot may also hard-append the footer when a PR URL is linked; that is a safety net, not a reason to omit it. + +2. If Discord turn context lists **Linked work items** / Jira issues for the thread, include those Jira issue links in the PR description (and prefer the primary key in the title/branch when one is clear). + +3. Prefer opening the PR only after commits already include the Identity map `Co-authored-by` trailers (see above). ## Task Completion Requirements diff --git a/apps/discord-bot/src/features/ThreadInfoPin.ts b/apps/discord-bot/src/features/ThreadInfoPin.ts index d657872730f..ea70cee639d 100644 --- a/apps/discord-bot/src/features/ThreadInfoPin.ts +++ b/apps/discord-bot/src/features/ThreadInfoPin.ts @@ -10,6 +10,14 @@ import { extractJiraIssueKeysFromDiscordMessage, mergeJiraIssueKeys, } from "../presentation/jiraLinks.ts"; +import { + buildDiscordThreadJumpUrl, + ensureDiscordPrAttributionFooters, + formatDiscordPrAttributionFooter, + starterDisplayName, + starterUserId, + type DiscordThreadStarterLike, +} from "../presentation/discordPrAttribution.ts"; import { extractPullRequestUrlsFromDiscordMessage, mergePullRequestUrls, @@ -36,10 +44,22 @@ interface DiscordMessageSummary { readonly description?: string | null; readonly footer?: { readonly text?: string | null } | null; }> | null; - readonly author?: { readonly id?: string; readonly bot?: boolean } | null; + readonly author?: { + readonly id?: string; + readonly bot?: boolean; + readonly username?: string; + readonly global_name?: string | null; + } | null; readonly timestamp?: string | null; } +interface DiscordChannelSummary { + readonly id: string; + readonly name?: string | null; + readonly parent_id?: string | null; + readonly owner_id?: string | null; +} + export interface ThreadInfoPinMessageRef { readonly channelId: string; readonly messageId: string; @@ -196,6 +216,185 @@ const resolveChannelGithubRepoSlug = (input: { return normalizeGithubRepoSlug(githubUrl); }); +/** + * Load Discord thread starter (public-thread parent message or oldest thread message) + * and the current thread title for hardcoded PR attribution footers. + */ +const loadDiscordThreadAttributionContext = (input: { + readonly discordThreadId: string; + readonly parentChannelId: string | null; + readonly guildId: string; + readonly baseUrl: string; + readonly botToken: string; +}) => + Effect.gen(function* () { + const channel = yield* Effect.tryPromise({ + try: () => + discordApiJson({ + baseUrl: input.baseUrl, + botToken: input.botToken, + path: `/channels/${input.discordThreadId}`, + }), + catch: (cause) => cause, + }).pipe(Effect.orElseSucceed((): DiscordChannelSummary | null => null)); + + const parentChannelId = + input.parentChannelId ?? + (channel?.parent_id !== null && channel?.parent_id !== undefined && channel.parent_id !== "" + ? channel.parent_id + : null); + + let starter: DiscordThreadStarterLike | null = null; + if (parentChannelId !== null) { + // Public threads created from a message use the starter message id as the thread id. + starter = yield* Effect.tryPromise({ + try: () => + discordApiJson({ + baseUrl: input.baseUrl, + botToken: input.botToken, + path: `/channels/${parentChannelId}/messages/${input.discordThreadId}`, + }), + catch: (cause) => cause, + }).pipe( + Effect.map( + (message): DiscordThreadStarterLike => ({ + id: message.id, + author: { + id: message.author?.id, + username: message.author?.username, + displayName: message.author?.global_name ?? message.author?.username, + }, + }), + ), + Effect.orElseSucceed((): DiscordThreadStarterLike | null => null), + ); + } + + if (starter === null) { + const listed = yield* Effect.tryPromise({ + try: () => + discordApiJson>({ + baseUrl: input.baseUrl, + botToken: input.botToken, + path: `/channels/${input.discordThreadId}/messages?limit=5&after=0`, + }), + catch: (cause) => cause, + }).pipe(Effect.orElseSucceed((): ReadonlyArray => [])); + + const oldest = listed.at(-1) ?? listed[0]; + if (oldest !== undefined) { + starter = { + id: oldest.id, + author: { + id: oldest.author?.id, + username: oldest.author?.username, + displayName: oldest.author?.global_name ?? oldest.author?.username, + }, + }; + } + } + + const threadTitle = + channel?.name !== null && channel?.name !== undefined && channel.name.trim() !== "" + ? channel.name.trim() + : "Discord thread"; + + const userId = starterUserId(starter); + if (userId === null) return null; + + return { + footer: formatDiscordPrAttributionFooter({ + starterDisplayName: starterDisplayName(starter), + starterUserId: userId, + threadTitle, + threadJumpUrl: buildDiscordThreadJumpUrl({ + guildId: input.guildId, + discordThreadId: input.discordThreadId, + messageId: starter?.id ?? null, + }), + }), + threadTitle, + starterUserId: userId, + } as const; + }); + +/** + * When GitHub PR URLs are observed on a Discord-linked thread, hard-append the + * Discord attribution footer using the **thread starter** + thread title. + * Idempotent (skips bodies that already have the footer). Best-effort only. + */ +const ensureAttributionFootersForIncomingPrs = (input: { + readonly discordThreadId: string; + readonly link: ThreadLink | null; + readonly incomingPrUrls: ReadonlyArray; +}) => + Effect.gen(function* () { + const prUrls = mergePullRequestUrls([], input.incomingPrUrls); + if (prUrls.length === 0) return; + + const links = yield* ThreadLinkStore; + const link = input.link ?? (yield* links.getByDiscordThreadId(input.discordThreadId)); + if (link === null) { + yield* Effect.logWarning("Skipping Discord PR attribution footer: no thread link", { + discordThreadId: input.discordThreadId, + prCount: prUrls.length, + }); + return; + } + + const discordConfig = yield* DiscordConfig.DiscordConfig; + const botToken = Redacted.value(discordConfig.token); + const baseUrl = discordConfig.rest.baseUrl; + + const attribution = yield* loadDiscordThreadAttributionContext({ + discordThreadId: input.discordThreadId, + parentChannelId: link.channelId, + guildId: link.guildId, + baseUrl, + botToken, + }); + if (attribution === null) { + yield* Effect.logWarning("Skipping Discord PR attribution footer: no thread starter", { + discordThreadId: input.discordThreadId, + prCount: prUrls.length, + }); + return; + } + + const results = yield* Effect.tryPromise({ + try: () => + ensureDiscordPrAttributionFooters({ + prUrls, + footer: attribution.footer, + }), + catch: (cause) => cause, + }).pipe( + Effect.catch((error) => + Effect.logWarning("Discord PR attribution footer ensure failed", { + discordThreadId: input.discordThreadId, + error: String(error), + }).pipe(Effect.as([] as const)), + ), + ); + + for (const result of results) { + if (result.status === "updated") { + yield* Effect.logInfo("Appended Discord PR attribution footer", { + discordThreadId: input.discordThreadId, + prUrl: result.url, + threadTitle: attribution.threadTitle, + starterUserId: attribution.starterUserId, + }); + } else if (result.status === "error") { + yield* Effect.logWarning("Failed to append Discord PR attribution footer", { + discordThreadId: input.discordThreadId, + prUrl: result.url, + detail: result.detail ?? null, + }); + } + } + }); + /** * Create or update the pinned thread-info message and ensure it stays pinned. */ @@ -359,6 +558,22 @@ export const upsertThreadInfoPin = (input: { } } + // Hardcode Discord PR attribution (thread starter + title) — no agent prompt. + // Only runs for *incoming* PR URLs this call (pin refresh with empty incoming is a no-op). + // ensureDiscordPrAttributionFooters is idempotent if the footer is already present. + yield* ensureAttributionFootersForIncomingPrs({ + discordThreadId: input.discordThreadId, + link: existing, + incomingPrUrls: input.incomingPrUrls ?? [], + }).pipe( + Effect.catch((error) => + Effect.logWarning("Discord PR attribution side-effect failed", { + discordThreadId: input.discordThreadId, + error: String(error), + }), + ), + ); + const nextModelLine = input.modelSelection === null || input.modelSelection === undefined ? null diff --git a/apps/discord-bot/src/identityMap.test.ts b/apps/discord-bot/src/identityMap.test.ts index b5779e77bd4..d921c4605b8 100644 --- a/apps/discord-bot/src/identityMap.test.ts +++ b/apps/discord-bot/src/identityMap.test.ts @@ -8,6 +8,7 @@ import { formatCoAuthoredByTrailer, formatIdentityAttributionBlock, loadIdentityMapFromFileSync, + makeRefreshingIdentityMapStore, parseIdentityMapDocument, parseSimpleIdentityYaml, resolveGitHubCoAuthorEmail, @@ -248,3 +249,75 @@ describe("loadIdentityMapFromFileSync", () => { expect(people[0]?.github?.id).toBe("2"); }); }); + +describe("makeRefreshingIdentityMapStore", () => { + it("reloads after the TTL expires and keeps the prior map on load failure", async () => { + const dir = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-identity-ttl-")); + const path = NodePath.join(dir, "identity-map.json"); + await NodeFSP.writeFile( + path, + JSON.stringify({ + people: [ + { + name: "A", + discord: { id: "1" }, + github: { login: "a", id: "11" }, + }, + ], + }), + "utf8", + ); + + let now = 1_000_000; + let loads = 0; + const store = makeRefreshingIdentityMapStore({ + filePath: path, + ttlMs: 60_000, + now: () => now, + load: (p) => { + loads += 1; + return loadIdentityMapFromFileSync(p); + }, + }); + + expect(store.list()).toHaveLength(1); + expect(store.resolveByDiscordId("1")?.name).toBe("A"); + expect(loads).toBe(1); + + // Within TTL — no reload + now += 30_000; + expect(store.list()).toHaveLength(1); + expect(loads).toBe(1); + + // After TTL — pick up new file contents + await NodeFSP.writeFile( + path, + JSON.stringify({ + people: [ + { + name: "A", + discord: { id: "1" }, + github: { login: "a", id: "11" }, + }, + { + name: "Davide Di Pumpo", + discord: { id: "150802733316702208" }, + github: { login: "MakhBeth", id: "2373426" }, + }, + ], + }), + "utf8", + ); + now += 60_000; + expect(store.list()).toHaveLength(2); + expect(store.resolveByDiscordId("150802733316702208")?.github?.login).toBe("MakhBeth"); + expect(loads).toBe(2); + + // Corrupt file after TTL — keep last good snapshot + await NodeFSP.writeFile(path, "{ not valid json", "utf8"); + now += 60_000; + expect(store.list()).toHaveLength(2); + expect(store.resolveByDiscordId("150802733316702208")?.name).toBe("Davide Di Pumpo"); + expect(loads).toBe(3); + }); +}); diff --git a/apps/discord-bot/src/identityMap.ts b/apps/discord-bot/src/identityMap.ts index 1f72686f559..362cf4c5f86 100644 --- a/apps/discord-bot/src/identityMap.ts +++ b/apps/discord-bot/src/identityMap.ts @@ -462,8 +462,9 @@ export function formatIdentityAttributionBlock(input: { ); const lines: string[] = [ - "### Identity map (git / GitHub / Jira attribution)", - "Operator-maintained map from Discord users to GitHub (and optional Jira). Use this for commit/PR attribution — do not invent emails or logins.", + "### Identity map (git / GitHub / Jira attribution) — REQUIRED", + "Operator-maintained map from Discord users to GitHub (and optional Jira).", + "**Do not invent emails or logins.** Attribution is mandatory for Discord-originated commits/PRs when trailers are listed below.", ]; for (const p of participants) { @@ -502,21 +503,24 @@ export function formatIdentityAttributionBlock(input: { } lines.push(""); - lines.push("When creating commits for this Discord work:"); + lines.push("**REQUIRED when creating commits for this Discord work:**"); lines.push("1. Keep the environment default author/committer (usually the GitHub App bot)."); lines.push( - "2. Append these `Co-authored-by` trailers for **mapped** participants (thread starter and/or current requester). Skip unmapped people — do not invent emails.", + "2. **Every** new commit message MUST end with the `Co-authored-by` trailers below for **mapped** participants (thread starter and/or current requester). Skip unmapped people — do not invent emails.", ); lines.push( - "3. Put trailers at the end of the commit message after a blank line. Prefer the exact lines below.", + "3. Put trailers at the end of the commit message after a blank line. Use the exact lines below.", ); lines.push( - "4. When opening a PR, you may also list co-authors in the body; GitHub primarily uses commit trailers for multi-author avatars.", + "4. Verify with `git log -1 --format=%B` before push/PR. Commits missing these trailers are incomplete.", + ); + lines.push( + "5. When opening a PR: include the Discord description footer from AGENTS.md (opened by … in chat thread **Discord**). The bot may hard-append the footer later — still write it on create. GitHub multi-author avatars come from **commit** trailers, not PR body prose alone.", ); if (trailers.length > 0) { lines.push(""); - lines.push("Ready-to-paste trailers for this turn:"); + lines.push("**Mandatory** ready-to-paste trailers for this turn (append to every new commit):"); lines.push("```"); for (const t of trailers) lines.push(t); lines.push("```"); @@ -563,9 +567,14 @@ export class IdentityMapStore extends Context.Service, -): IdentityMapStoreService => { +/** How long a loaded identity map stays hot before re-reading the file (no bot restart). */ +export const IDENTITY_MAP_CACHE_TTL_MS = 60_000; + +function indexesFromPeople(people: ReadonlyArray): { + readonly people: ReadonlyArray; + readonly byId: ReadonlyMap; + readonly byUsername: ReadonlyMap; +} { const byId = new Map(); const byUsername = new Map(); for (const person of people) { @@ -576,22 +585,95 @@ export const makeIdentityMapStore = ( byUsername.set(person.discord.username.toLowerCase(), person); } } + return { people, byId, byUsername }; +} + +export const makeIdentityMapStore = ( + people: ReadonlyArray, +): IdentityMapStoreService => { + const index = indexesFromPeople(people); return IdentityMapStore.of({ - list: () => people, - resolveByDiscordId: (discordId) => byId.get(discordId.trim()) ?? null, - resolveByDiscordUsername: (username) => byUsername.get(username.trim().toLowerCase()) ?? null, + list: () => index.people, + resolveByDiscordId: (discordId) => index.byId.get(discordId.trim()) ?? null, + resolveByDiscordUsername: (username) => + index.byUsername.get(username.trim().toLowerCase()) ?? null, resolveParticipant: (input) => resolveParticipantIdentity({ role: input.role, discordId: input.discordId, discordUsername: input.discordUsername, discordDisplayName: input.discordDisplayName, - people, + people: index.people, }), }); }; +/** + * File-backed identity map with a short TTL cache so operators can edit + * identity-map.yaml without restarting the Discord bot. + * + * - Eager-loads once at construction (throws on first failure). + * - Re-reads the file at most every `ttlMs` (default 60s) on access. + * - On later load failures, keeps the last good snapshot and waits another TTL. + */ +export function makeRefreshingIdentityMapStore(input: { + readonly filePath: string; + readonly ttlMs?: number; + readonly now?: () => number; + readonly load?: (path: string) => ReadonlyArray; + /** Optional hook when a reload succeeds (for tests / diagnostics). */ + readonly onReload?: (people: ReadonlyArray) => void; +}): IdentityMapStoreService { + const ttlMs = input.ttlMs ?? IDENTITY_MAP_CACHE_TTL_MS; + const now = input.now ?? (() => Date.now()); + const load = input.load ?? loadIdentityMapFromFileSync; + const path = input.filePath.trim(); + + let index = indexesFromPeople(load(path)); + let loadedAt = now(); + input.onReload?.(index.people); + + const refreshIfStale = () => { + const t = now(); + if (t - loadedAt < ttlMs) return; + try { + const nextPeople = load(path); + index = indexesFromPeople(nextPeople); + loadedAt = t; + input.onReload?.(index.people); + } catch { + // Keep serving the last good map; delay the next retry by a full TTL. + loadedAt = t; + } + }; + + return IdentityMapStore.of({ + list: () => { + refreshIfStale(); + return index.people; + }, + resolveByDiscordId: (discordId) => { + refreshIfStale(); + return index.byId.get(discordId.trim()) ?? null; + }, + resolveByDiscordUsername: (username) => { + refreshIfStale(); + return index.byUsername.get(username.trim().toLowerCase()) ?? null; + }, + resolveParticipant: (participant) => { + refreshIfStale(); + return resolveParticipantIdentity({ + role: participant.role, + discordId: participant.discordId, + discordUsername: participant.discordUsername, + discordDisplayName: participant.discordDisplayName, + people: index.people, + }); + }, + }); +} + export const layerFromOptionalPath = (filePath: string | undefined) => Layer.effect( IdentityMapStore, @@ -602,19 +684,21 @@ export const layerFromOptionalPath = (filePath: string | undefined) => ); return makeIdentityMapStore([]); } - const people = yield* Effect.try({ - try: () => loadIdentityMapFromFileSync(filePath), + const resolvedPath = filePath.trim(); + const store = yield* Effect.try({ + try: () => makeRefreshingIdentityMapStore({ filePath: resolvedPath }), catch: (cause) => { if (isIdentityMapLoadError(cause)) return cause; return new IdentityMapLoadError({ - path: filePath, + path: resolvedPath, message: cause instanceof Error ? cause.message : String(cause), }); }, }); + const count = store.list().length; yield* Effect.logInfo( - `Loaded ${people.length} identity map entr${people.length === 1 ? "y" : "ies"} from ${filePath}`, + `Loaded ${count} identity map entr${count === 1 ? "y" : "ies"} from ${resolvedPath} (reload TTL ${IDENTITY_MAP_CACHE_TTL_MS / 1000}s)`, ); - return makeIdentityMapStore(people); + return store; }), ); diff --git a/apps/discord-bot/src/presentation/discordPrAttribution.test.ts b/apps/discord-bot/src/presentation/discordPrAttribution.test.ts new file mode 100644 index 00000000000..58ceef6f0e0 --- /dev/null +++ b/apps/discord-bot/src/presentation/discordPrAttribution.test.ts @@ -0,0 +1,175 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { + appendDiscordPrAttributionFooter, + buildDiscordThreadJumpUrl, + DISCORD_PR_ATTRIBUTION_MARKER, + ensureDiscordPrAttributionFooters, + formatDiscordPrAttributionFooter, + newlyObservedPullRequestUrls, + prBodyHasDiscordAttribution, + starterDisplayName, + starterUserId, +} from "./discordPrAttribution.ts"; + +describe("formatDiscordPrAttributionFooter", () => { + it("formats starter + thread title + jump link", () => { + expect( + formatDiscordPrAttributionFooter({ + starterDisplayName: "joshuadima", + starterUserId: "593167616273809448", + threadTitle: "Open Random PR Test", + threadJumpUrl: + "https://discord.com/channels/1083767712431480922/1531376362399465595/1531376362399465595", + }), + ).toBe( + "opened by [joshuadima](593167616273809448) in chat thread **Discord** · [Open Random PR Test](https://discord.com/channels/1083767712431480922/1531376362399465595/1531376362399465595)", + ); + }); + + it("escapes markdown brackets in names/titles", () => { + const footer = formatDiscordPrAttributionFooter({ + starterDisplayName: "a[b]c", + starterUserId: "1", + threadTitle: "Title [x]", + threadJumpUrl: "https://discord.com/channels/1/2/3", + }); + expect(footer).toContain("[a\\[b\\]c](1)"); + expect(footer).toContain("[Title \\[x\\]](https://discord.com/channels/1/2/3)"); + }); +}); + +describe("buildDiscordThreadJumpUrl", () => { + it("prefers message id when provided", () => { + expect( + buildDiscordThreadJumpUrl({ + guildId: "g", + discordThreadId: "t", + messageId: "m", + }), + ).toBe("https://discord.com/channels/g/t/m"); + }); + + it("falls back to thread id as message id", () => { + expect( + buildDiscordThreadJumpUrl({ + guildId: "g", + discordThreadId: "t", + messageId: null, + }), + ).toBe("https://discord.com/channels/g/t/t"); + }); +}); + +describe("starter helpers", () => { + it("prefers displayName over username", () => { + expect( + starterDisplayName({ + id: "m1", + author: { id: "u1", username: "user", displayName: "Display" }, + }), + ).toBe("Display"); + expect(starterUserId({ id: "m1", author: { id: "u1" } })).toBe("u1"); + }); + + it("handles missing starter", () => { + expect(starterDisplayName(null)).toBe("unknown"); + expect(starterUserId(null)).toBeNull(); + }); +}); + +describe("prBodyHasDiscordAttribution / appendDiscordPrAttributionFooter", () => { + const footer = formatDiscordPrAttributionFooter({ + starterDisplayName: "joshuadima", + starterUserId: "593167616273809448", + threadTitle: "Thread", + threadJumpUrl: "https://discord.com/channels/1/2/3", + }); + + it("detects existing footer marker", () => { + expect(prBodyHasDiscordAttribution(`hello\n${DISCORD_PR_ATTRIBUTION_MARKER}\n`)).toBe(true); + expect(prBodyHasDiscordAttribution("## Summary\n- stuff")).toBe(false); + }); + + it("appends footer with separator when missing", () => { + expect(appendDiscordPrAttributionFooter("## Summary\n- a", footer)).toBe( + `## Summary\n- a\n\n---\n\n${footer}\n`, + ); + }); + + it("returns null when already present (idempotent)", () => { + const body = `## Summary\n\n---\n\n${footer}\n`; + expect(appendDiscordPrAttributionFooter(body, footer)).toBeNull(); + }); + + it("works on empty body", () => { + expect(appendDiscordPrAttributionFooter("", footer)).toBe(`${footer}\n`); + }); +}); + +describe("newlyObservedPullRequestUrls", () => { + it("returns only new canonical PR urls", () => { + expect( + newlyObservedPullRequestUrls( + ["https://github.com/owner/repo/pull/1"], + [ + "https://github.com/owner/repo/pull/1/files", + "https://github.com/owner/repo/pull/2", + "https://example.com/not-a-pr", + ], + ), + ).toEqual(["https://github.com/owner/repo/pull/2"]); + }); +}); + +describe("ensureDiscordPrAttributionFooters", () => { + it("patches missing footers and skips ones already present", async () => { + const bodies = new Map([ + ["repos/o/r/pulls/1", "## Summary\n- a\n"], + [ + "repos/o/r/pulls/2", + `## Summary\n\n---\n\nopened by [x](1) ${DISCORD_PR_ATTRIBUTION_MARKER} [t](https://discord.com/channels/1/2/3)\n`, + ], + ]); + const patched: string[] = []; + + const results = await ensureDiscordPrAttributionFooters({ + prUrls: [ + "https://github.com/o/r/pull/1", + "https://github.com/o/r/pull/2", + "https://github.com/o/r/pull/3", + ], + footer: formatDiscordPrAttributionFooter({ + starterDisplayName: "joshuadima", + starterUserId: "593167616273809448", + threadTitle: "Open Random PR Test", + threadJumpUrl: "https://discord.com/channels/1/2/3", + }), + execFile: async (_file, args) => { + const path = String(args[1] ?? ""); + if (args.includes("--jq")) { + if (!bodies.has(path)) { + throw new Error(`not found: ${path}`); + } + return { stdout: bodies.get(path) ?? "", stderr: "" }; + } + if (args.includes("PATCH")) { + patched.push(path); + return { stdout: "", stderr: "" }; + } + throw new Error(`unexpected gh args: ${args.join(" ")}`); + }, + }); + + expect(results).toEqual([ + { url: "https://github.com/o/r/pull/1", status: "updated" }, + { url: "https://github.com/o/r/pull/2", status: "already_present" }, + { + url: "https://github.com/o/r/pull/3", + status: "error", + detail: "not found: repos/o/r/pulls/3", + }, + ]); + expect(patched).toEqual(["repos/o/r/pulls/1"]); + }); +}); diff --git a/apps/discord-bot/src/presentation/discordPrAttribution.ts b/apps/discord-bot/src/presentation/discordPrAttribution.ts new file mode 100644 index 00000000000..2a13d6a6e5e --- /dev/null +++ b/apps/discord-bot/src/presentation/discordPrAttribution.ts @@ -0,0 +1,238 @@ +/** + * Hardcoded Discord PR attribution footer. + * + * When a PR URL is first linked to a Discord thread, the bot appends a footer + * using the **thread starter** (not the current requester) and the Discord + * thread title. No agent prompt is required. + */ +// @effect-diagnostics nodeBuiltinImport:off +import * as NodeChildProcess from "node:child_process"; +import * as NodeFs from "node:fs/promises"; +import * as NodeOs from "node:os"; +import * as NodePath from "node:path"; +import * as NodeUtil from "node:util"; + +import { gitCommandEnv } from "./githubLinks.ts"; +import { normalizePullRequestUrl } from "./prLinks.ts"; + +const execFile = NodeUtil.promisify(NodeChildProcess.execFile); + +type ExecFileResult = { + readonly stdout: string; + readonly stderr: string; +}; + +type ExecFileLike = ( + file: string, + args: ReadonlyArray, + options?: NodeChildProcess.ExecFileOptions, +) => Promise; + +/** Marker used to detect an existing Discord attribution footer (idempotent). */ +export const DISCORD_PR_ATTRIBUTION_MARKER = "in chat thread **Discord** ·"; + +export type DiscordPrAttributionInput = { + /** Thread starter display name (fallback: username). */ + readonly starterDisplayName: string; + /** Thread starter Discord snowflake user id. */ + readonly starterUserId: string; + /** Discord thread title (channel name). */ + readonly threadTitle: string; + /** Jump URL into the Discord thread (prefer starter message). */ + readonly threadJumpUrl: string; +}; + +export type DiscordThreadStarterLike = { + readonly id: string; + readonly author?: + | { + readonly id?: string | undefined; + readonly username?: string | undefined; + readonly displayName?: string | undefined; + } + | undefined + | null; +}; + +/** + * Build the single-line attribution footer from thread starter + title. + */ +export function formatDiscordPrAttributionFooter(input: DiscordPrAttributionInput): string { + const displayName = input.starterDisplayName.trim() || "unknown"; + const userId = input.starterUserId.trim(); + const title = sanitizeMarkdownLinkLabel(input.threadTitle.trim() || "Discord thread"); + const jumpUrl = input.threadJumpUrl.trim(); + + // Keep display name linked to the raw Discord user id (matches AGENTS.md format). + return `opened by [${escapeMarkdownLinkLabel(displayName)}](${userId}) in chat thread **Discord** · [${escapeMarkdownLinkLabel(title)}](${jumpUrl})`; +} + +export function buildDiscordThreadJumpUrl(input: { + readonly guildId: string; + readonly discordThreadId: string; + /** Prefer the starter message id; falls back to the thread id. */ + readonly messageId?: string | null | undefined; +}): string { + const messageId = + input.messageId !== null && input.messageId !== undefined && input.messageId.trim() !== "" + ? input.messageId.trim() + : input.discordThreadId; + return `https://discord.com/channels/${input.guildId}/${input.discordThreadId}/${messageId}`; +} + +export function starterDisplayName(starter: DiscordThreadStarterLike | null | undefined): string { + if (starter === null || starter === undefined) return "unknown"; + const display = starter.author?.displayName?.trim(); + if (display !== undefined && display.length > 0) return display; + const username = starter.author?.username?.trim(); + if (username !== undefined && username.length > 0) return username; + return "unknown"; +} + +export function starterUserId(starter: DiscordThreadStarterLike | null | undefined): string | null { + const id = starter?.author?.id?.trim(); + return id !== undefined && id.length > 0 ? id : null; +} + +/** True when the PR body already has a Discord attribution footer. */ +export function prBodyHasDiscordAttribution(body: string | null | undefined): boolean { + if (body === null || body === undefined || body.length === 0) return false; + return ( + body.includes(DISCORD_PR_ATTRIBUTION_MARKER) || + /opened by \[.+?\]\(.+?\) in chat thread/u.test(body) + ); +} + +/** + * Append the footer when missing. Returns null when body already has attribution + * (caller should skip the GitHub update). + */ +export function appendDiscordPrAttributionFooter( + body: string | null | undefined, + footer: string, +): string | null { + const trimmedFooter = footer.trim(); + if (trimmedFooter.length === 0) return null; + if (prBodyHasDiscordAttribution(body)) return null; + + const base = (body ?? "").replace(/\s+$/u, ""); + if (base.length === 0) return `${trimmedFooter}\n`; + return `${base}\n\n---\n\n${trimmedFooter}\n`; +} + +/** + * Newly observed PR URLs that are not already in the durable first-seen list. + */ +export function newlyObservedPullRequestUrls( + existing: ReadonlyArray | null | undefined, + incoming: ReadonlyArray | null | undefined, +): ReadonlyArray { + const seen = new Set(); + for (const raw of existing ?? []) { + const normalized = normalizePullRequestUrl(raw); + if (normalized !== null) seen.add(normalized.url); + } + + const result: string[] = []; + for (const raw of incoming ?? []) { + const normalized = normalizePullRequestUrl(raw); + if (normalized === null || seen.has(normalized.url)) continue; + seen.add(normalized.url); + result.push(normalized.url); + } + return result; +} + +function escapeMarkdownLinkLabel(value: string): string { + return value.replace(/\[/gu, "\\[").replace(/\]/gu, "\\]"); +} + +function sanitizeMarkdownLinkLabel(value: string): string { + // Collapse newlines / excessive whitespace so the footer stays one line. + return value.replace(/\s+/gu, " ").trim(); +} + +export type EnsureDiscordPrAttributionResult = { + readonly url: string; + readonly status: "updated" | "already_present" | "skipped" | "error"; + readonly detail?: string | undefined; +}; + +/** + * For each PR URL, append the Discord attribution footer when missing. + * Best-effort: failures are returned per-URL and never throw. + */ +export async function ensureDiscordPrAttributionFooters(input: { + readonly prUrls: ReadonlyArray; + readonly footer: string; + readonly execFile?: ExecFileLike; +}): Promise> { + const execImpl = input.execFile ?? execFile; + const results: EnsureDiscordPrAttributionResult[] = []; + + for (const raw of input.prUrls) { + const normalized = normalizePullRequestUrl(raw); + if (normalized === null) { + results.push({ url: raw, status: "skipped", detail: "not a github pull request url" }); + continue; + } + + try { + const currentBody = await readPullRequestBody(normalized, execImpl); + const nextBody = appendDiscordPrAttributionFooter(currentBody, input.footer); + if (nextBody === null) { + results.push({ url: normalized.url, status: "already_present" }); + continue; + } + + await writePullRequestBody(normalized, nextBody, execImpl); + results.push({ url: normalized.url, status: "updated" }); + } catch (error) { + results.push({ + url: normalized.url, + status: "error", + detail: error instanceof Error ? error.message : String(error), + }); + } + } + + return results; +} + +async function readPullRequestBody( + pr: { readonly owner: string; readonly repo: string; readonly number: number }, + execImpl: ExecFileLike, +): Promise { + const { stdout } = await execImpl( + "gh", + ["api", `repos/${pr.owner}/${pr.repo}/pulls/${pr.number}`, "--jq", '.body // ""'], + { env: gitCommandEnv(), maxBuffer: 4 * 1024 * 1024 }, + ); + return stdout; +} + +async function writePullRequestBody( + pr: { readonly owner: string; readonly repo: string; readonly number: number }, + body: string, + execImpl: ExecFileLike, +): Promise { + const tempDir = await NodeFs.mkdtemp(NodePath.join(NodeOs.tmpdir(), "t3-discord-pr-body-")); + const bodyFile = NodePath.join(tempDir, "body.json"); + try { + await NodeFs.writeFile(bodyFile, JSON.stringify({ body }), "utf8"); + await execImpl( + "gh", + [ + "api", + `repos/${pr.owner}/${pr.repo}/pulls/${pr.number}`, + "-X", + "PATCH", + "--input", + bodyFile, + ], + { env: gitCommandEnv(), maxBuffer: 4 * 1024 * 1024 }, + ); + } finally { + await NodeFs.rm(tempDir, { recursive: true, force: true }).catch(() => undefined); + } +} diff --git a/apps/discord-bot/src/presentation/prLinks.test.ts b/apps/discord-bot/src/presentation/prLinks.test.ts index 915674d6da7..af7b06ae902 100644 --- a/apps/discord-bot/src/presentation/prLinks.test.ts +++ b/apps/discord-bot/src/presentation/prLinks.test.ts @@ -6,6 +6,7 @@ import { formatPullRequestLabel, formatPullRequestLinksForDiscord, mergePullRequestUrls, + sortPullRequestUrlsForDisplay, normalizeGithubRepoSlug, normalizePullRequestUrl, } from "./prLinks.ts"; @@ -143,4 +144,35 @@ describe("formatPullRequestLinksForDiscord", () => { it("returns null for empty lists", () => { expect(formatPullRequestLinksForDiscord([])).toBeNull(); }); + + it("sorts current-project PRs above foreign repos while keeping first-seen order within groups", () => { + expect( + formatPullRequestLinksForDiscord( + [ + "https://github.com/example-org/configurator/pull/10", + "https://github.com/example-org/scanner/pull/2", + "https://github.com/other/repo/pull/99", + "https://github.com/example-org/scanner/pull/1", + "https://github.com/example-org/configurator/pull/11", + ], + { channelRepoSlug: "example-org/scanner" }, + ), + ).toBe( + [ + "**PRs**", + "• [PR #2](https://github.com/example-org/scanner/pull/2)", + "• [PR #1](https://github.com/example-org/scanner/pull/1)", + "• [example-org/configurator PR #10](https://github.com/example-org/configurator/pull/10)", + "• [other/repo PR #99](https://github.com/other/repo/pull/99)", + "• [example-org/configurator PR #11](https://github.com/example-org/configurator/pull/11)", + ].join("\n"), + ); + }); +}); + +describe("sortPullRequestUrlsForDisplay", () => { + it("leaves order unchanged when channel repo is unknown", () => { + const urls = ["https://github.com/a/b/pull/1", "https://github.com/c/d/pull/2"]; + expect(sortPullRequestUrlsForDisplay(urls, null)).toEqual(urls); + }); }); diff --git a/apps/discord-bot/src/presentation/prLinks.ts b/apps/discord-bot/src/presentation/prLinks.ts index 47dda1521dd..4d26f66c228 100644 --- a/apps/discord-bot/src/presentation/prLinks.ts +++ b/apps/discord-bot/src/presentation/prLinks.ts @@ -149,6 +149,33 @@ export function mergePullRequestUrls( return result; } +/** + * Order PR URLs for the pinned thread header: current project/repo first, + * then everything else. Within each group, preserve first-seen order. + */ +export function sortPullRequestUrlsForDisplay( + urls: ReadonlyArray, + channelRepoSlug?: string | null, +): ReadonlyArray { + const ordered = mergePullRequestUrls([], urls); + if (ordered.length === 0) return ordered; + + const channel = normalizeGithubRepoSlug(channelRepoSlug); + if (channel === null) return ordered; + + const current: string[] = []; + const other: string[] = []; + for (const url of ordered) { + const normalized = normalizePullRequestUrl(url); + if (normalized !== null && normalized.repoSlug === channel) { + current.push(normalized.url); + } else { + other.push(normalized?.url ?? url); + } + } + return [...current, ...other]; +} + export function formatPullRequestLinksForDiscord( urls: ReadonlyArray, options?: { @@ -156,7 +183,7 @@ export function formatPullRequestLinksForDiscord( readonly channelRepoSlug?: string | null; }, ): string | null { - const ordered = mergePullRequestUrls([], urls); + const ordered = sortPullRequestUrlsForDisplay(urls, options?.channelRepoSlug); if (ordered.length === 0) return null; const lines = ordered.map((url) => {