diff --git a/AGENTS.md b/AGENTS.md index 8f28629..0a4aaf9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -136,9 +136,14 @@ tests/ # bun test unit tests (webhook, formatter, discord, tel ## Message Format Spec -- Every embed title must start with the repo, then optional `#number`, then `: subject`: +- Every message title must start with the repo, then optional `#number`, then `: subject`: `{repo}{#number}: {subject}` (e.g. `acme/widget#7: Add feature`). Repo comes from `payload.repository.full_name`; fall back to `t("common.repository")` when missing. +- Only the repo head is hyperlinked (never the whole title). Drivers split the title via + `splitMessageTitle`/`repoUrlFromMessage` (`server/lib/formatters/helpers.ts`): the Discord + embed title is `{repo}{#number}` linked to the repository URL and `: {subject}` renders as + the first description line; Telegram keeps the one-line title with an inline repo link and + a plain subject. Messages without a `: ` separator keep the legacy whole-title link. - Do NOT use `"Comment on org/repo"` / `"Review on org/repo"` prefixes. Comments, reviews and inline comments use the same `{repo}{#number}: {title}` title as their parent object. - All event-specific emoji live in `server/lib/formatters/` (via the `emojiPrefix` helper), never in diff --git a/server/lib/drivers/discord/render.ts b/server/lib/drivers/discord/render.ts index 9646cb0..46c7a48 100644 --- a/server/lib/drivers/discord/render.ts +++ b/server/lib/drivers/discord/render.ts @@ -1,4 +1,5 @@ import type { FormattedMessage, NeutralActionStyle, NeutralMessage } from "../../types"; +import { repoUrlFromMessage, splitMessageTitle } from "../../formatters/helpers"; function toStyle(style: NeutralActionStyle): number { switch (style) { @@ -15,14 +16,26 @@ export function renderNeutralMessage(message: NeutralMessage): FormattedMessage const content = message.mentionRoleIds?.length ? message.mentionRoleIds.map((id) => `<@&${id}>`).join(" ") : undefined; + + // Discord embed titles can only be linked as a whole, so only the repo head + // goes into the title (linked to the repository); the `: subject` text is + // rendered as the first line of the description, unlinked. + const { head, subject } = splitMessageTitle(message.title); + const repoUrl = repoUrlFromMessage(message.url); + const description = subject + ? message.description + ? `${subject}\n${message.description}` + : subject + : message.description; + return { content, embeds: [ { - title: message.title, - url: message.url, + title: head, + url: subject ? repoUrl : message.url, color: message.color, - description: message.description, + description, author: message.author ? { name: message.author.name, diff --git a/server/lib/drivers/telegram/render.ts b/server/lib/drivers/telegram/render.ts index 4d92ce3..f9e55a2 100644 --- a/server/lib/drivers/telegram/render.ts +++ b/server/lib/drivers/telegram/render.ts @@ -1,4 +1,5 @@ import type { NeutralMessage } from "../../types"; +import { repoUrlFromMessage, splitMessageTitle } from "../../formatters/helpers"; function esc(s: string): string { return s @@ -32,10 +33,16 @@ function formatTimestamp(ts?: string): string { export function renderNeutralMessage(message: NeutralMessage): string { const parts: string[] = []; - const title = message.url - ? `${mdToHtml(message.title)}` - : mdToHtml(message.title); - parts.push(`${title}`); + // HTML allows partial links, so keep the `{repo}{#number}: {subject}` line + // intact with only the repo head linked (the subject stays plain text). + const { head, subject } = splitMessageTitle(message.title); + const repoUrl = repoUrlFromMessage(message.url); + const title = subject + ? `${repoUrl ? `${mdToHtml(head)}` : mdToHtml(head)}: ${mdToHtml(subject)}` + : message.url + ? `${mdToHtml(message.title)}` + : `${mdToHtml(message.title)}`; + parts.push(title); if (message.author) { const name = mdToHtml(message.author.name); diff --git a/server/lib/formatters/helpers.ts b/server/lib/formatters/helpers.ts index 3831001..a3d19f2 100644 --- a/server/lib/formatters/helpers.ts +++ b/server/lib/formatters/helpers.ts @@ -12,6 +12,42 @@ export function makeT(tr?: Translations): T { return (key, params) => translate(key, params, undefined, tr); } +export interface TitleParts { + /** `{repo}` or `{repo}#{number}` — what embed titles link (the repo). */ + head: string; + /** Text after `": "`, undefined when the title has no `: ` separator. */ + subject?: string; +} + +/** + * Split a `{repo}{#number}: {subject}` title into the repo head and the + * subject. Discord embed titles can only link as a whole, so drivers render + * the head as the linked title and the subject as plain text (description / + * unlinked remainder) to avoid hyperlinking the whole title. + */ +export function splitMessageTitle(title: string): TitleParts { + const idx = title.indexOf(": "); + if (idx <= 0) return { head: title }; + return { head: title.slice(0, idx), subject: title.slice(idx + 2) }; +} + +/** + * Repository URL derived from an event URL (origin + owner + repo). Returns + * undefined when there is no URL to derive from — callers then render the + * title without a link. + */ +export function repoUrlFromMessage(url: string | undefined): string | undefined { + if (!url) return undefined; + try { + const parsed = new URL(url); + const segments = parsed.pathname.split("/").filter(Boolean); + if (segments.length >= 2) return `${parsed.origin}/${segments[0]}/${segments[1]}`; + } catch { + // not a parseable URL — no link + } + return undefined; +} + export function buildMessage( partial: Omit, "title"> & { title: string }, t: T, diff --git a/tests/discord.test.ts b/tests/discord.test.ts index cdcc710..dc16edc 100644 --- a/tests/discord.test.ts +++ b/tests/discord.test.ts @@ -309,7 +309,8 @@ describe("dispatchEvent fallback routing", () => { const logBody = JSON.parse(sent[1]!.body) as { embeds?: Array<{ title?: string; color?: number; fields?: Array<{ value: string }> }>; }; - expect(logBody.embeds?.[0]?.title).toBe("owner/repo: push"); + expect(logBody.embeds?.[0]?.title).toBe("owner/repo"); + expect(logBody.embeds?.[0]?.description).toContain("push"); expect(logBody.embeds?.[0]?.color).toBe(0x3fb950); expect(logBody.embeds?.[0]?.fields?.[0]?.value).toContain("✅ Push Route → 111"); expect(logBody.embeds?.[0]?.fields?.[1]?.value).toBe("deliv-1"); @@ -460,8 +461,6 @@ describe("dispatchEvent fallback routing", () => { const parsed = JSON.parse(b) as { embeds?: Array<{ title?: string }> }; return parsed.embeds?.[0]?.title ?? ""; }); - expect(titles.sort()).toEqual( - ["owner/repo: 推送了 1 个提交", "owner/repo: Pushed 1 commit"].sort(), - ); + expect(titles.sort()).toEqual(["owner/repo", "owner/repo"].sort()); }); }); diff --git a/tests/telegram.test.ts b/tests/telegram.test.ts index 5922d08..ecb7320 100644 --- a/tests/telegram.test.ts +++ b/tests/telegram.test.ts @@ -25,7 +25,7 @@ describe("telegram renderNeutralMessage", () => { }; const out = renderNeutralMessage(message); expect(out).toContain( - 'acme/widget: Add feature', + 'acme/widget: Add feature', ); expect(out).toContain("Status: success"); expect(out).toContain("acme/widget"); diff --git a/tests/webhook-tenant.test.ts b/tests/webhook-tenant.test.ts index 5a279c8..3e77e9f 100644 --- a/tests/webhook-tenant.test.ts +++ b/tests/webhook-tenant.test.ts @@ -236,9 +236,9 @@ describe("processWebhook", () => { const parsed = JSON.parse(sent[0]!.body) as { embeds?: Array<{ title?: string; color?: number; description?: string }>; }; - expect(parsed.embeds?.[0]?.title).toBe("acme/widget: Deploy failed"); + expect(parsed.embeds?.[0]?.title).toBe("acme/widget"); expect(parsed.embeds?.[0]?.color).toBe(0xf85149); - expect(parsed.embeds?.[0]?.description).toBe("prod down"); + expect(parsed.embeds?.[0]?.description).toBe("Deploy failed\nprod down"); }); it("rejects custom webhooks without a signature header", async () => {