Skip to content

Commit e01c054

Browse files
.start command should prompt for priority (#141)
* Redesigned the .start command to prompt priority selection New Flow: - .start <title> → Priority selection UI → Incident creation - .start (no title) → Error with usage guidance - Removed legacy default P3 creation Features Added: - Interactive priority selection with P1-P5 options - Cancel option (CANCEL) to abort incident creation - Timeout handling (5 minutes for priority selection) - Enhanced Slack UI with clear priority descriptions Implementation Details: - Added PendingIncident state management system - Created incident-start.ts handler for priority selection flow - Extended CommPlatform interface with sendPrioritySelection() - Updated Slack adapter with prioritySelectionBlocks() UI - Modified listeners to handle P1-P5 and CANCEL responses * Add configurable priority selection for .start command Add requirePrioritySelection config option to enable/disable interactive priority selection flow for incident creation. When enabled (requirePrioritySelection: true): - .start <title> triggers priority selection UI before incident creation - Users choose P1-P5 or CANCEL to proceed When disabled (requirePrioritySelection: false): - .start <title> creates incident immediately with default priority (legacy behavior) Alias commands (.critical, .high, .medium, etc.) always bypass priority selection in both modes. * Revert changes to package.lock.json and package.json * Implement button-based priority selection with threaded UI - Add shortDescription field to Priority type for concise descriptions - Replace text-based priority selection with interactive buttons - Implement threaded priority selection UI instead of main channel - Add button click handlers for priority selection and cancellation - Update Slack adapter to handle interactive button events - Make messageId required for priority selection threading - Add debug logging for incident resolution state tracking * Fix linting issues * Fix linting errors * Commting package-lock.json and package.json Only dev dependency changes. * Removed @biomejs/cli-darwin-arm64 * extract priority selection cleanup logic and UX improvements * UX feedback improvements * Add tests to incident start implementation changes Final Status: Complete Scenario-Based Test Coverage All Issues Resolved: - ✅ Tests: 643 tests passing (increased from 637) - ✅ Linting: No errors remaining - ✅ TypeScript: Clean compilation - ✅ Code Style: All lint fixes applied Comprehensive Coverage Achieved for All 3 Core Scenarios: 📋 Scenario 1: Start incident → Times out - ✅ NEW: Timeout mechanism test with timer mocking - ✅ NEW: Cleanup verification (emoji + deletion + thread reply) - ✅ NEW: Pending incident removal verification 📋 Scenario 2: Start incident → Cancel it - ✅ Manual cancellation (existing) - ✅ Fallback cancellation (existing) - ✅ Error handling (existing) 📋 Scenario 3: Start incident → Choose priority - ✅ P1 priority (existing comprehensive test) - ✅ NEW: P2 priority selection - ✅ NEW: P3 priority selection - ✅ NEW: P4 priority selection - ✅ NEW: P5 priority selection - ✅ NEW: Duplicate pending incident rejection * Update P3 to show as a red button Also updates the emoji, as the previous one wasn't visible enough on a red background. * Move timeout to the config file and change to 1 minute * Disable aliases if requirePrioritySelection is true * bump version to 3.3.0 * Remove duplicated lint declarations * Delete PR_DESCRIPTION.md * Set priority selection timeout to 2 minutes Remove vip-priorities.js
1 parent 2e7076e commit e01c054

File tree

15 files changed

+1214
-21
lines changed

15 files changed

+1214
-21
lines changed

config/priorities.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import type { PriorityConfig } from "./types.js";
22

33
export const priorityConfig: PriorityConfig = {
44
default: 2,
5+
requirePrioritySelection: false,
6+
prioritySelectionTimeoutSeconds: 2 * 60,
57
priorities: {
68
1: {
79
name: "P1",

config/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,15 @@ export type PriorityConfig = {
130130
documentationUrl?: string;
131131
default: number;
132132
defaultLow?: number;
133+
requirePrioritySelection?: boolean;
134+
prioritySelectionTimeoutSeconds?: number;
133135
priorities: {
134136
[key: number]: {
135137
name: string;
136138
url?: string;
137139
emoji: string;
138140
description: string;
141+
shortDescription?: string;
139142
aliases: string[];
140143
nag?: NagConfig;
141144
reportRequired?: boolean;

lib/boundary/comm-platform.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,15 @@ export interface CommPlatform {
227227
config: PriorityConfig,
228228
messageId?: string,
229229
): Promise<unknown>;
230+
sendPrioritySelection(
231+
room: string,
232+
config: PriorityConfig,
233+
messageId: string,
234+
): Promise<{ ts?: string }>;
235+
deletePrioritySelectionMessage(
236+
room: string,
237+
messageId: string,
238+
): Promise<unknown>;
230239
sendPriorityUpdated(
231240
room: string,
232241
priority: number,

lib/boundary/hubot/adapters/slack.ts

Lines changed: 225 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ import {
1919
} from "../../../core/string.js";
2020
import { isIncidentActive } from "../../../data/incident.js";
2121
import { LogType } from "../../../data/schema/log-entry-schema.js";
22+
import {
23+
handleCancelSelection,
24+
handlePrioritySelection,
25+
} from "../handlers/incident-start.js";
26+
import {
27+
priorityCanceledConfirmationBlocks,
28+
prioritySelectedConfirmationBlocks,
29+
} from "./slack/blocks.js";
2230
import {
2331
affectedAddedBlocks,
2432
affectedBlocks,
@@ -41,6 +49,7 @@ import {
4149
newBreakingBlocks,
4250
notesBlocks,
4351
priorityBlocks,
52+
prioritySelectionBlocks,
4453
resolvedBlocks,
4554
richTextBlock,
4655
statusAllActiveBlocks,
@@ -89,7 +98,11 @@ import type { DatetimeIso9075 } from "../../../core/date.js";
8998
import type { Blocker } from "../../../data/blocker.js";
9099
import type { Incident, IncidentOverview } from "../../../data/incident.js";
91100
import type { LogEntry } from "../../../data/log.js";
92-
import type { ChatRoomUid, ChatUserId } from "../../../types/index.js";
101+
import type {
102+
BreakingBot,
103+
ChatRoomUid,
104+
ChatUserId,
105+
} from "../../../types/index.js";
93106
import type { CommPlatform } from "../../comm-platform.js";
94107
import type { IssueTracker } from "../../issue-tracker.js";
95108

@@ -128,18 +141,42 @@ export class Slack extends Adapter implements CommPlatform {
128141
`SLACK: creating #${channelName} with users ${initialUserIds.join(",")}`,
129142
);
130143

131-
const response = await this.#webClient.conversations.create({
132-
name: channelName,
133-
});
144+
let finalChannelName = channelName;
145+
let response: unknown;
146+
147+
try {
148+
response = await this.#webClient.conversations.create({
149+
name: finalChannelName,
150+
});
151+
} catch (error: unknown) {
152+
// Handle name_taken error with timestamp fallback
153+
const slackError = error as { data?: { error?: string } };
154+
if (slackError?.data?.error === "name_taken") {
155+
finalChannelName = `${channelName}-${Date.now()}`;
156+
this.robot.logger.warn(
157+
`SLACK: Channel #${channelName} already exists, retrying with #${finalChannelName}`,
158+
);
159+
160+
response = await this.#webClient.conversations.create({
161+
name: finalChannelName,
162+
});
163+
} else {
164+
throw error;
165+
}
166+
}
134167

135-
const { channel } = response;
168+
const { channel } = response as {
169+
channel?: { id?: string; name?: string };
170+
};
136171

137172
if (!channel || !channel.id || !channel.name) {
138-
this.robot.logger.error(`SLACK: failed create channel ${channelName}`);
173+
this.robot.logger.error(
174+
`SLACK: failed create channel ${finalChannelName}`,
175+
);
139176
return {};
140177
}
141178

142-
this.robot.logger.debug(`SLACK: created channel ${channelName}`);
179+
this.robot.logger.debug(`SLACK: created channel ${finalChannelName}`);
143180

144181
if (initialUserIds.length > 0) {
145182
this.#webClient.conversations.invite({
@@ -939,6 +976,97 @@ export class Slack extends Adapter implements CommPlatform {
939976
);
940977
}
941978

979+
async sendPrioritySelection(
980+
channel: string,
981+
config: PriorityConfig,
982+
timestamp: string,
983+
): Promise<{ ts?: string }> {
984+
const result = await this.#replyThreaded(
985+
channel,
986+
prioritySelectionBlocks(config),
987+
"Breaking process started. Please select a priority using the buttons below.",
988+
timestamp,
989+
);
990+
return result || {};
991+
}
992+
993+
async updatePrioritySelectionMessage(
994+
channel: string,
995+
messageTimestamp: string,
996+
selectedPriority: number,
997+
config: PriorityConfig,
998+
success = true,
999+
errorMessage?: string,
1000+
): Promise<unknown> {
1001+
const blocks = prioritySelectedConfirmationBlocks(
1002+
selectedPriority,
1003+
config,
1004+
success,
1005+
errorMessage,
1006+
);
1007+
const text = success
1008+
? `Priority ${selectedPriority} selected. Incident created successfully!`
1009+
: `Priority ${selectedPriority} selected. Error: ${errorMessage || "Failed to create incident"}`;
1010+
1011+
return this.#webClient.chat.update({
1012+
channel,
1013+
ts: messageTimestamp,
1014+
blocks,
1015+
text,
1016+
});
1017+
}
1018+
1019+
async updatePrioritySelectionMessageCanceled(
1020+
channel: string,
1021+
messageTimestamp: string,
1022+
): Promise<unknown> {
1023+
const blocks = priorityCanceledConfirmationBlocks();
1024+
const text = "Incident creation canceled.";
1025+
1026+
try {
1027+
return await this.#webClient.chat.update({
1028+
channel,
1029+
ts: messageTimestamp,
1030+
blocks,
1031+
text,
1032+
});
1033+
} catch (error: unknown) {
1034+
// If we can't update the message (too old, not our message, etc.),
1035+
// just log the error and continue - the emoji reaction will still indicate cancellation
1036+
const slackError = error as {
1037+
data?: { error?: string };
1038+
message?: string;
1039+
};
1040+
this.robot.logger.warn(
1041+
`Failed to update priority selection message: ${slackError?.data?.error || slackError?.message || "Unknown error"}`,
1042+
);
1043+
return Promise.resolve();
1044+
}
1045+
}
1046+
1047+
async deletePrioritySelectionMessage(
1048+
channel: string,
1049+
messageTimestamp: string,
1050+
): Promise<unknown> {
1051+
try {
1052+
return await this.#webClient.chat.delete({
1053+
channel,
1054+
ts: messageTimestamp,
1055+
});
1056+
} catch (error: unknown) {
1057+
// If we can't delete the message, just log the error and continue
1058+
// The emoji reaction and thread reply will still indicate what happened
1059+
const slackError = error as {
1060+
data?: { error?: string };
1061+
message?: string;
1062+
};
1063+
this.robot.logger.warn(
1064+
`Failed to delete priority selection message: ${slackError?.data?.error || slackError?.message || "Unknown error"}`,
1065+
);
1066+
return Promise.resolve();
1067+
}
1068+
}
1069+
9421070
sendPriorityUpdated(
9431071
channel: string,
9441072
newPriority: number,
@@ -1507,6 +1635,96 @@ export class Slack extends Adapter implements CommPlatform {
15071635
this.robot.receive(new LeaveMessage(user, null, event.ts));
15081636
});
15091637

1638+
this.#socketClient.on("interactive", async (payload) => {
1639+
await payload.ack();
1640+
1641+
this.robot.logger.debug(
1642+
"Interactive payload received:",
1643+
JSON.stringify(payload, null, 2),
1644+
);
1645+
1646+
const { body } = payload;
1647+
if (!body) {
1648+
this.robot.logger.error("No body in interactive payload");
1649+
return;
1650+
}
1651+
1652+
if (body.type === "block_actions") {
1653+
for (const action of body.actions || []) {
1654+
if (action.action_id?.startsWith("select_priority_")) {
1655+
const priority = Number(action.value);
1656+
const channel = body.channel?.id;
1657+
const userId = body.user?.id;
1658+
const messageTs = body.message?.ts;
1659+
1660+
this.robot.logger.debug(
1661+
`Priority ${priority} selected by user ${userId}`,
1662+
);
1663+
1664+
try {
1665+
// Update message to show selection immediately
1666+
await this.updatePrioritySelectionMessage(
1667+
channel,
1668+
messageTs,
1669+
priority,
1670+
(this.robot as BreakingBot).config.priorities,
1671+
true,
1672+
);
1673+
1674+
// Create the incident
1675+
await handlePrioritySelection(
1676+
this.robot as BreakingBot,
1677+
channel,
1678+
priority,
1679+
userId,
1680+
messageTs,
1681+
);
1682+
} catch (error) {
1683+
this.robot.logger.error(
1684+
"Error handling priority selection:",
1685+
error,
1686+
);
1687+
1688+
// Update message to show error
1689+
await this.updatePrioritySelectionMessage(
1690+
channel,
1691+
messageTs,
1692+
priority,
1693+
(this.robot as BreakingBot).config.priorities,
1694+
false,
1695+
error instanceof Error
1696+
? error.message
1697+
: "Unknown error occurred",
1698+
);
1699+
}
1700+
} else if (action.action_id === "cancel_priority_selection") {
1701+
const channel = body.channel?.id;
1702+
const userId = body.user?.id;
1703+
const messageTs = body.message?.ts;
1704+
1705+
this.robot.logger.debug(
1706+
`Priority selection cancelled by user ${userId}`,
1707+
);
1708+
1709+
try {
1710+
// Use consistent cancellation behavior (same as timeout)
1711+
await handleCancelSelection(
1712+
this.robot as BreakingBot,
1713+
channel,
1714+
userId,
1715+
messageTs,
1716+
);
1717+
} catch (error) {
1718+
this.robot.logger.error(
1719+
"Error handling priority cancellation:",
1720+
error,
1721+
);
1722+
}
1723+
}
1724+
}
1725+
}
1726+
});
1727+
15101728
await this.#socketClient.start();
15111729
}
15121730

0 commit comments

Comments
 (0)