Skip to content

Commit b706f85

Browse files
snomiaoclaude
andauthored
feat(backport): improve backport notifications with persistent warnings and author tagging (#179)
* feat(backport): improve backport notifications with persistent warnings and author tagging - Replace fixed maxReleasesToCheck with version-based filtering (4 minor versions behind) - Add core-backport-not-needed and cloud-backport-not-needed label support - Tag PR authors in Slack backport notifications - Fall back to release sheriff (from #frontend-releases channel description) for external contributors - Fix pre-existing TS errors in bot/slack-bot.ts, $pipeline.ts, EmailTasks.ts, filterDebugMessages.ts - Add comprehensive tests for new functionality Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(backport): add --dry-run flag and graceful error handling for compare API - Add --dry-run CLI flag to preview output without sending Slack messages - Skip Slack user lookups in dry-run mode (show @github-username instead) - Gracefully handle 404s from GitHub compare API (skip failed releases instead of crashing) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): add missing upsertSlackMarkdownMessage to mock in desktop-release spec The mock.module() for upsertSlackMessage only included upsertSlackMessage but not upsertSlackMarkdownMessage or mdFmt. When bun runs the test suite and gh-frontend-backport-checker tests run after desktop-release tests, the module mock is still active without the export, causing a SyntaxError. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(backport): guard profile.real_name against undefined before .replace() Optional chaining on .toLowerCase() returns undefined when real_name is missing, causing .replace() to throw. Normalize to empty string first. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(backport): paginate through all Slack users instead of only first 500 slack.users.list() returns at most one page of 500 users; large workspaces would silently fail to find authors beyond the first page. Now loops using response_metadata.next_cursor until all pages are fetched. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(backport): use <= for version-distance filter so releases exactly N behind are included The previous < comparison excluded releases exactly maxMinorVersionsBehind behind the latest. The PR intent is to include those releases, so <= is correct. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(backport): remove redundant filter in targetBranches branchName already came from labels.filter(reBackportTargets), so labels.some(...includes(branchName)) was always true. Remove the no-op. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(bot): remove duplicate ANSI escape regex in filterDebugMessages Lines 36 and 37 had identical patterns. Remove the duplicate. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: format code with prettier (no logic changes) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): align version-distance filter test with <= boundary change The test used < (strict) to mirror the old production code, but d0fd106 changed production to <=. Update the test to match: v1.36.0 (exactly 4 behind latest) is now expected to be included. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(slack): handle integer-only timestamps in slackTsToISO Channel creation timestamps from Slack API are plain Unix seconds (e.g. "1234567890") with no decimal/microseconds part. Splitting on "." leaves microseconds undefined, causing a crash. Default microseconds to "000" when the decimal part is absent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(backport): cache Slack member list and set bugfixCommits on failure Two fixes per Copilot review: 1. Cache the Slack workspace member list with a run-scoped promise so findSlackUserIdByGithubUsername doesn't paginate through all members on every author lookup (avoids repeated full-directory scans and Slack rate-limit exposure). 2. Include bugfixCommits: [] when saving a failed compareCommits task so downstream callers that flatMap over bugfixCommits get a consistent array shape regardless of task status. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(backport): add detailed doc comment and comprehensive edge-case tests Add thorough documentation comment at top of index.ts explaining the full workflow and all edge cases. Export middleTruncated and getBackportStatusEmoji for testing. Add 12 new test suites covering missing edge cases: middleTruncated, emoji mapping, backport commit filtering, bot comment regex, overall status derivation, compare link parsing, branch name generation, processSince filtering, targetBranches guard, idempotent Slack updates, author tag filtering, and report status categorization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style(backport): format code and use slackCached for member lookups Prettier formatting changes. Switch slack.users.list to slackCached.users.list for Slack member lookups. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b3dbac5 commit b706f85

20 files changed

Lines changed: 987 additions & 217 deletions

File tree

.husky/pre-commit

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/usr/bin/env bun
2-
bunx tsgo
3-
bunx oxlint --fix
4-
bunx oxfmt
2+
bun typecheck
3+
bun lint
4+
bunx lint-staged
55

app/tasks/gh-desktop-release-notification/index.spec.ts

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,14 @@ const createMockCollection = (collectionName?: string) => {
4242
}
4343
}
4444
// Check for deliveryId (webhook tests)
45-
if ((filter as { deliveryId?: string }).deliveryId && d.deliveryId === (filter as { deliveryId?: string }).deliveryId) return doc;
45+
if (
46+
(filter as { deliveryId?: string }).deliveryId &&
47+
d.deliveryId === (filter as { deliveryId?: string }).deliveryId
48+
)
49+
return doc;
4650
}
4751
// Fallback to findOneAndUpdate results for backward compatibility
48-
const existingOp = dbOperations.find(
49-
(op) => op.type === "findOneAndUpdate" && op.result,
50-
);
52+
const existingOp = dbOperations.find((op) => op.type === "findOneAndUpdate" && op.result);
5153
if (existingOp && filter.version) {
5254
const result = existingOp.result as { coreVersion?: string } | undefined;
5355
if (result?.coreVersion === filter.version) {
@@ -69,7 +71,7 @@ const createMockCollection = (collectionName?: string) => {
6971
},
7072
insertOne: async (doc: unknown) => {
7173
const id = `mock_id_${++docIdCounter}`;
72-
const docWithId = { ...doc as object, _id: id };
74+
const docWithId = { ...(doc as object), _id: id };
7375
docs.set(id, docWithId);
7476
return { insertedId: id };
7577
},
@@ -125,6 +127,14 @@ mock.module("./upsertSlackMessage", () => ({
125127
url: `https://slack.com/message/${Date.now()}`,
126128
};
127129
},
130+
upsertSlackMarkdownMessage: async (msg: SlackMessageType) => {
131+
mockSlackMessages.push(msg);
132+
return {
133+
...msg,
134+
url: `https://slack.com/message/${Date.now()}`,
135+
};
136+
},
137+
mdFmt: async (md: string) => md,
128138
}));
129139

130140
// Now import the module to test (after all mocks are set up)
@@ -174,9 +184,7 @@ describe("GithubDesktopReleaseNotificationTask", () => {
174184
expect(saveOps.length).toBeGreaterThanOrEqual(1);
175185

176186
// Check if any save operation has slackMessageDrafting
177-
const hasDraftingMessage = saveOps.some(
178-
(op) => op.args[1]?.$set?.slackMessageDrafting,
179-
);
187+
const hasDraftingMessage = saveOps.some((op) => op.args[1]?.$set?.slackMessageDrafting);
180188
expect(hasDraftingMessage).toBe(true);
181189

182190
// Ensure slackMessage was NOT set for draft
@@ -261,9 +269,7 @@ describe("GithubDesktopReleaseNotificationTask", () => {
261269
expect(saveOps.length).toBeGreaterThanOrEqual(1);
262270

263271
// Check if any save operation has slackMessage
264-
const hasStableMessage = saveOps.some(
265-
(op) => op.args[1]?.$set?.slackMessage,
266-
);
272+
const hasStableMessage = saveOps.some((op) => op.args[1]?.$set?.slackMessage);
267273
expect(hasStableMessage).toBe(true);
268274
});
269275

@@ -343,9 +349,7 @@ describe("GithubDesktopReleaseNotificationTask", () => {
343349
expect(saveOps.length).toBeGreaterThanOrEqual(1);
344350

345351
// Check if any save operation has slackMessageDrafting
346-
const hasDraftingMessage = saveOps.some(
347-
(op) => op.args[1]?.$set?.slackMessageDrafting,
348-
);
352+
const hasDraftingMessage = saveOps.some((op) => op.args[1]?.$set?.slackMessageDrafting);
349353
expect(hasDraftingMessage).toBe(true);
350354
});
351355
});
@@ -372,9 +376,7 @@ describe("GithubDesktopReleaseNotificationTask", () => {
372376

373377
// Verify coreVersion was extracted
374378
const saveOps = dbOperations.filter((op) => op.type === "findOneAndUpdate");
375-
const hasCoreVersion = saveOps.some(
376-
(op) => op.args[1]?.$set?.coreVersion === "v0.2.0",
377-
);
379+
const hasCoreVersion = saveOps.some((op) => op.args[1]?.$set?.coreVersion === "v0.2.0");
378380
expect(hasCoreVersion).toBe(true);
379381
});
380382
});

0 commit comments

Comments
 (0)