fix: link only the repo head in message titles, keep the subject plain

This commit is contained in:
RhenCloud 2026-08-13 19:18:55 +08:00
parent 84d8a5384d
commit dfb65ef70d
No known key found for this signature in database
GPG key ID: A574A617378C4E0B
7 changed files with 75 additions and 15 deletions

View file

@ -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

View file

@ -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,

View file

@ -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
? `<a href="${esc(message.url)}">${mdToHtml(message.title)}</a>`
: mdToHtml(message.title);
parts.push(`<b>${title}</b>`);
// 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
? `<b>${repoUrl ? `<a href="${esc(repoUrl)}">${mdToHtml(head)}</a>` : mdToHtml(head)}: ${mdToHtml(subject)}</b>`
: message.url
? `<b><a href="${esc(message.url)}">${mdToHtml(message.title)}</a></b>`
: `<b>${mdToHtml(message.title)}</b>`;
parts.push(title);
if (message.author) {
const name = mdToHtml(message.author.name);

View file

@ -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<Partial<NeutralMessage>, "title"> & { title: string },
t: T,

View file

@ -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());
});
});

View file

@ -25,7 +25,7 @@ describe("telegram renderNeutralMessage", () => {
};
const out = renderNeutralMessage(message);
expect(out).toContain(
'<b><a href="https://github.com/acme/widget">acme/widget: Add feature</a></b>',
'<b><a href="https://github.com/acme/widget">acme/widget</a>: Add feature</b>',
);
expect(out).toContain("<b>Status</b>: success");
expect(out).toContain("<i>acme/widget</i>");

View file

@ -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 () => {