Skip to content

Commit c406099

Browse files
didineledidinele
committed
fix: atomic op concerns
1 parent dd955e9 commit c406099

4 files changed

Lines changed: 105 additions & 33 deletions

File tree

‎services/ama-bot/src/components/guestApprove.ts‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,16 @@ export default class GuestApproveComponent implements ComponentHandler<string> {
1414
public async handle(interaction: APIMessageComponentInteraction, questionIdStr: string) {
1515
const questionId = Number.parseInt(questionIdStr, 10);
1616

17+
// Ack within Discord's 3s window before doing any DB/REST work below; everything past this point
18+
// finishes via editReply/followUp instead of reply/updateMessage.
19+
await client.api.interactions.deferMessageUpdate(interaction.id, interaction.token);
20+
1721
const [question] = await getContext().db<AmaQuestions[]>`
1822
SELECT * FROM ama_questions WHERE id = ${questionId}
1923
`;
2024

2125
if (!question) {
22-
await client.api.interactions.reply(interaction.id, interaction.token, {
26+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
2327
content: 'Question not found. It may have been deleted.',
2428
flags: MessageFlags.Ephemeral,
2529
});
@@ -35,13 +39,29 @@ export default class GuestApproveComponent implements ComponentHandler<string> {
3539
}
3640

3741
if (session.ended) {
38-
await client.api.interactions.reply(interaction.id, interaction.token, {
42+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
3943
content: 'This AMA session has ended.',
4044
flags: MessageFlags.Ephemeral,
4145
});
4246
return;
4347
}
4448

49+
// Only claims from PENDING_GUEST_REVIEW so a concurrent answer/skip can't both win.
50+
const [claimed] = await getContext().db<AmaQuestions[]>`
51+
UPDATE ama_questions
52+
SET state = 'APPROVED', updated_at = now()
53+
WHERE id = ${question.id} AND state = 'PENDING_GUEST_REVIEW'
54+
RETURNING *
55+
`;
56+
57+
if (!claimed) {
58+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
59+
content: 'This question was already handled by someone else.',
60+
flags: MessageFlags.Ephemeral,
61+
});
62+
return;
63+
}
64+
4565
const user = await client.api.users.get(question.authorId);
4666
const member = interaction.guild_id
4767
? await client.api.guilds.getMember(interaction.guild_id, question.authorId).catch(() => undefined)
@@ -62,12 +82,10 @@ export default class GuestApproveComponent implements ComponentHandler<string> {
6282
});
6383

6484
await getContext().db`
65-
UPDATE ama_questions
66-
SET state = 'APPROVED', answers_message_id = ${msg.id}, updated_at = now()
67-
WHERE id = ${question.id}
85+
UPDATE ama_questions SET answers_message_id = ${msg.id} WHERE id = ${question.id}
6886
`;
6987

70-
await client.api.interactions.updateMessage(interaction.id, interaction.token, {
88+
await client.api.interactions.editReply(interaction.application_id, interaction.token, {
7189
components: [
7290
{
7391
type: ComponentType.ActionRow,
@@ -84,8 +102,11 @@ export default class GuestApproveComponent implements ComponentHandler<string> {
84102
],
85103
});
86104
} catch (error) {
105+
// The row is already claimed (state flipped) at this point; if the answers-channel post itself
106+
// failed, the question is stuck claimed with no downstream message and needs manual follow-up —
107+
// logged loudly here rather than attempting a rollback/saga for what should be a rare failure mode.
87108
getContext().logger.error({ error, questionId }, 'Failed to approve question');
88-
await client.api.interactions.reply(interaction.id, interaction.token, {
109+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
89110
content: 'Failed to approve question. Please try again.',
90111
flags: MessageFlags.Ephemeral,
91112
});

‎services/ama-bot/src/components/guestSkip.ts‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,16 @@ export default class GuestSkipComponent implements ComponentHandler<string> {
1313
public async handle(interaction: APIMessageComponentInteraction, questionIdStr: string) {
1414
const questionId = Number.parseInt(questionIdStr, 10);
1515

16+
// Ack within Discord's 3s window before doing any DB/REST work below; everything past this point
17+
// finishes via editReply/followUp instead of reply/updateMessage.
18+
await client.api.interactions.deferMessageUpdate(interaction.id, interaction.token);
19+
1620
const [question] = await getContext().db<AmaQuestions[]>`
1721
SELECT * FROM ama_questions WHERE id = ${questionId}
1822
`;
1923

2024
if (!question) {
21-
await client.api.interactions.reply(interaction.id, interaction.token, {
25+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
2226
content: 'Question not found. It may have been deleted.',
2327
flags: MessageFlags.Ephemeral,
2428
});
@@ -34,19 +38,31 @@ export default class GuestSkipComponent implements ComponentHandler<string> {
3438
}
3539

3640
if (session.ended) {
37-
await client.api.interactions.reply(interaction.id, interaction.token, {
41+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
3842
content: 'This AMA session has ended.',
3943
flags: MessageFlags.Ephemeral,
4044
});
4145
return;
4246
}
4347

4448
try {
45-
await getContext().db`
46-
UPDATE ama_questions SET state = 'DENIED', updated_at = now() WHERE id = ${question.id}
49+
// Only skips from PENDING_GUEST_REVIEW so a concurrent answer/skip can't both win.
50+
const [skipped] = await getContext().db<AmaQuestions[]>`
51+
UPDATE ama_questions
52+
SET state = 'DENIED', updated_at = now()
53+
WHERE id = ${question.id} AND state = 'PENDING_GUEST_REVIEW'
54+
RETURNING *
4755
`;
4856

49-
await client.api.interactions.updateMessage(interaction.id, interaction.token, {
57+
if (!skipped) {
58+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
59+
content: 'This question was already handled by someone else.',
60+
flags: MessageFlags.Ephemeral,
61+
});
62+
return;
63+
}
64+
65+
await client.api.interactions.editReply(interaction.application_id, interaction.token, {
5066
components: [
5167
{
5268
type: ComponentType.ActionRow,
@@ -66,7 +82,7 @@ export default class GuestSkipComponent implements ComponentHandler<string> {
6682
getContext().logger.info({ questionId, amaId: question.amaId }, 'Question skipped by guest');
6783
} catch (error) {
6884
getContext().logger.error({ err: error, questionId }, 'Failed to skip question');
69-
await client.api.interactions.reply(interaction.id, interaction.token, {
85+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
7086
content: 'Failed to skip question. Please try again.',
7187
flags: MessageFlags.Ephemeral,
7288
});

‎services/ama-bot/src/components/modApprove.ts‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@ export default class ModApproveComponent implements ComponentHandler<string> {
1414
public async handle(interaction: APIMessageComponentInteraction, questionIdStr: string) {
1515
const questionId = Number.parseInt(questionIdStr, 10);
1616

17-
// Fetch the question and AMA session
17+
// Ack within Discord's 3s window before doing any DB/REST work below; everything past this point
18+
// finishes via editReply/followUp instead of reply/updateMessage.
19+
await client.api.interactions.deferMessageUpdate(interaction.id, interaction.token);
20+
1821
const [question] = await getContext().db<AmaQuestions[]>`
1922
SELECT * FROM ama_questions WHERE id = ${questionId}
2023
`;
2124

2225
if (!question) {
23-
await client.api.interactions.reply(interaction.id, interaction.token, {
26+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
2427
content: 'Question not found. It may have been deleted.',
2528
flags: MessageFlags.Ephemeral,
2629
});
@@ -36,13 +39,33 @@ export default class ModApproveComponent implements ComponentHandler<string> {
3639
}
3740

3841
if (session.ended) {
39-
await client.api.interactions.reply(interaction.id, interaction.token, {
42+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
4043
content: 'This AMA session has ended.',
4144
flags: MessageFlags.Ephemeral,
4245
});
4346
return;
4447
}
4548

49+
// Determine the next queue up front so the claim below can move the row straight to its target
50+
// state; this also doubles as a lock — only one concurrent click can win the row.
51+
const nextQueue = getNextQueue(CurrentlyInQueue.mod, session);
52+
const targetState = nextQueue?.kind === CurrentlyInQueue.guest ? 'PENDING_GUEST_REVIEW' : 'APPROVED';
53+
54+
const [claimed] = await getContext().db<AmaQuestions[]>`
55+
UPDATE ama_questions
56+
SET state = ${targetState}, updated_at = now()
57+
WHERE id = ${question.id} AND state = 'PENDING_MOD_REVIEW'
58+
RETURNING *
59+
`;
60+
61+
if (!claimed) {
62+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
63+
content: 'This question was already handled by another moderator.',
64+
flags: MessageFlags.Ephemeral,
65+
});
66+
return;
67+
}
68+
4669
// Get user details from the interaction
4770
const user = await client.api.users.get(question.authorId);
4871
const member = interaction.guild_id
@@ -53,9 +76,6 @@ export default class ModApproveComponent implements ComponentHandler<string> {
5376
// question text itself comes straight from the DB (the source message's text has a footer baked in).
5477
const attachments = interaction.message.attachments ?? [];
5578

56-
// Determine the next queue
57-
const nextQueue = getNextQueue(CurrentlyInQueue.mod, session);
58-
5979
try {
6080
if (nextQueue?.kind === CurrentlyInQueue.guest) {
6181
// Post to guest queue
@@ -69,9 +89,7 @@ export default class ModApproveComponent implements ComponentHandler<string> {
6989
});
7090

7191
await getContext().db`
72-
UPDATE ama_questions
73-
SET state = 'PENDING_GUEST_REVIEW', guest_queue_message_id = ${msg.id}, updated_at = now()
74-
WHERE id = ${question.id}
92+
UPDATE ama_questions SET guest_queue_message_id = ${msg.id} WHERE id = ${question.id}
7593
`;
7694
} else {
7795
// Post directly to answers channel
@@ -85,14 +103,12 @@ export default class ModApproveComponent implements ComponentHandler<string> {
85103
});
86104

87105
await getContext().db`
88-
UPDATE ama_questions
89-
SET state = 'APPROVED', answers_message_id = ${msg.id}, updated_at = now()
90-
WHERE id = ${question.id}
106+
UPDATE ama_questions SET answers_message_id = ${msg.id} WHERE id = ${question.id}
91107
`;
92108
}
93109

94110
// Update the message to show it was approved
95-
await client.api.interactions.updateMessage(interaction.id, interaction.token, {
111+
await client.api.interactions.editReply(interaction.application_id, interaction.token, {
96112
components: [
97113
{
98114
type: ComponentType.ActionRow,
@@ -109,8 +125,11 @@ export default class ModApproveComponent implements ComponentHandler<string> {
109125
],
110126
});
111127
} catch (error) {
128+
// The row is already claimed (state flipped) at this point; if the queue post itself failed, the
129+
// question is stuck claimed with no downstream message and needs manual follow-up — logged loudly
130+
// here rather than attempting a rollback/saga for what should be a rare failure mode.
112131
getContext().logger.error({ error, questionId }, 'Failed to approve question');
113-
await client.api.interactions.reply(interaction.id, interaction.token, {
132+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
114133
content: 'Failed to approve question. Please try again.',
115134
flags: MessageFlags.Ephemeral,
116135
});

‎services/ama-bot/src/components/modDeny.ts‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,17 @@ export default class ModDenyComponent implements ComponentHandler<string> {
1313
public async handle(interaction: APIMessageComponentInteraction, questionIdStr: string) {
1414
const questionId = Number.parseInt(questionIdStr, 10);
1515

16+
// Ack within Discord's 3s window before doing any DB/REST work below; everything past this point
17+
// finishes via editReply/followUp instead of reply/updateMessage.
18+
await client.api.interactions.deferMessageUpdate(interaction.id, interaction.token);
19+
1620
// Fetch the question to verify it exists
1721
const [question] = await getContext().db<AmaQuestions[]>`
1822
SELECT * FROM ama_questions WHERE id = ${questionId}
1923
`;
2024

2125
if (!question) {
22-
await client.api.interactions.reply(interaction.id, interaction.token, {
26+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
2327
content: 'Question not found. It may have been deleted.',
2428
flags: MessageFlags.Ephemeral,
2529
});
@@ -35,20 +39,32 @@ export default class ModDenyComponent implements ComponentHandler<string> {
3539
}
3640

3741
if (session.ended) {
38-
await client.api.interactions.reply(interaction.id, interaction.token, {
42+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
3943
content: 'This AMA session has ended.',
4044
flags: MessageFlags.Ephemeral,
4145
});
4246
return;
4347
}
4448

4549
try {
46-
await getContext().db`
47-
UPDATE ama_questions SET state = 'DENIED', updated_at = now() WHERE id = ${question.id}
50+
// Only denies from PENDING_MOD_REVIEW so a concurrent approve/deny can't both win.
51+
const [denied] = await getContext().db<AmaQuestions[]>`
52+
UPDATE ama_questions
53+
SET state = 'DENIED', updated_at = now()
54+
WHERE id = ${question.id} AND state = 'PENDING_MOD_REVIEW'
55+
RETURNING *
4856
`;
4957

58+
if (!denied) {
59+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
60+
content: 'This question was already handled by another moderator.',
61+
flags: MessageFlags.Ephemeral,
62+
});
63+
return;
64+
}
65+
5066
// Update the message to show it was denied
51-
await client.api.interactions.updateMessage(interaction.id, interaction.token, {
67+
await client.api.interactions.editReply(interaction.application_id, interaction.token, {
5268
components: [
5369
{
5470
type: ComponentType.ActionRow,
@@ -68,7 +84,7 @@ export default class ModDenyComponent implements ComponentHandler<string> {
6884
getContext().logger.info({ questionId, amaId: question.amaId }, 'Question denied by moderator');
6985
} catch (error) {
7086
getContext().logger.error({ err: error, questionId }, 'Failed to deny question');
71-
await client.api.interactions.reply(interaction.id, interaction.token, {
87+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
7288
content: 'Failed to deny question. Please try again.',
7389
flags: MessageFlags.Ephemeral,
7490
});

0 commit comments

Comments
 (0)