Skip to content

Commit f3bb1d0

Browse files
author
Sorra
committed
Resolve merge conflicts: use ProgressPresenter instance for processUrlWithProgress event loop (SB-0MN77JJ1M0KXNUP0)
Work item: SB-0MNS5UKUI00389MZ
2 parents 446b1b7 + 22dc55a commit f3bb1d0

File tree

3 files changed

+482
-319
lines changed

3 files changed

+482
-319
lines changed

src/index.ts

Lines changed: 81 additions & 152 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
formatQueueFailureMessage,
1717
formatQueuedUrlMessage,
1818
} from "./presenters/queue.js";
19+
import { ProgressPresenter } from "./presenters/progress.js";
1920
import {
2021
runAddCommand,
2122
runSummaryCommand,
@@ -470,50 +471,32 @@ async function processUrlWithProgress(
470471
authorId: message.author.id,
471472
});
472473

473-
let lastPhase: string | null = null;
474-
// Once we see a terminal phase ('completed' or 'failed') we should
475-
// suppress any subsequent progress updates emitted by the CLI to avoid
476-
// confusing the user with post-completion informational objects.
477-
let terminalPhaseSeen = false;
478474
let eventCount = 0;
479475
let finalResult: AddResult | undefined;
480-
// Keep a reference to the last posted Discord message so we can
481-
// append the created item link/ID to it when the CLI returns the ID.
482-
// In real runtime this will be a discord.js Message; in tests the
483-
// mocked send/reply helpers may return undefined which we handle.
484-
// Track last posted message using ProgressPresenter to encapsulate state
485-
let lastPostedMessage: any = null;
486-
476+
477+
// ProgressPresenter manages status message state and lifecycle. Create
478+
// a single instance per URL processing session so it can track phase
479+
// deduplication and terminal suppression across the event stream.
480+
const presenter = new ProgressPresenter(thread, message, logger);
481+
487482
// Process progress events using manual iteration to capture return value
488483
logger.info("Waiting for CLI progress events...", { messageId: message.id, url });
489-
// Instantiate a ProgressPresenter once per processing session so it
490-
// can encapsulate status message lifecycle, deduplication, and terminal
491-
// suppression across events.
492-
let presenter: any = null;
493-
try {
494-
const mod = await import("./presenters/progress.js");
495-
if (typeof mod.ProgressPresenter === "function") {
496-
presenter = new mod.ProgressPresenter(thread, message, logger);
497-
}
498-
} catch {
499-
// ignore - we'll fall back to inline posting
500-
}
501484

502485
while (true) {
503486
const iteration = await addGenerator.next();
504-
487+
505488
if (iteration.done) {
506489
// Generator completed - capture the return value
507490
finalResult = iteration.value;
508-
logger.info("CLI generator completed", {
509-
messageId: message.id,
510-
url,
491+
logger.info("CLI generator completed", {
492+
messageId: message.id,
493+
url,
511494
eventCount,
512-
hasResult: !!finalResult
495+
hasResult: !!finalResult,
513496
});
514497
break;
515498
}
516-
499+
517500
// Process yielded progress event
518501
const event = iteration.value;
519502
eventCount++;
@@ -524,91 +507,56 @@ async function processUrlWithProgress(
524507
eventCount,
525508
});
526509

527-
// If we've already observed a terminal phase, ignore any further
528-
// progress events to avoid confusing follow-up messages (some CLI
529-
// implementations emit informational objects after completion).
530-
if (terminalPhaseSeen) {
531-
logger.debug("Ignoring CLI progress event after terminal phase", {
510+
try {
511+
await presenter.handleProgressEvent(event);
512+
} catch (err) {
513+
// Defensive fallback: if the presenter fails for any reason, log and
514+
// attempt a minimal inline post so users still see progress.
515+
logger.warn("ProgressPresenter.handleProgressEvent failed; falling back to inline posting", {
532516
messageId: message.id,
533-
url,
534-
eventCount,
517+
error: err instanceof Error ? err.message : String(err),
535518
});
536-
continue;
537-
}
538-
539-
// Only send update if phase changed (avoid spam)
540-
if (event.phase !== lastPhase) {
541-
lastPhase = event.phase;
542-
543-
const progressMsg = formatProgressMessage(event);
544519

545-
// Always ensure URLs shown to users are wrapped in backticks to avoid embeds.
546-
// If event contains a url or title, prefer showing the title wrapped in ticks.
547-
const safeProgressMsg = ((): string => {
548-
try {
549-
if (event.title) return progressMsg.replace(event.title, `\`${event.title}\``);
550-
if (event.url) return progressMsg.replace(event.url, `\`${event.url}\``);
551-
return progressMsg;
552-
} catch {
553-
return progressMsg;
554-
}
555-
})();
556-
557-
// Prefer the instantiated presenter; fall back to inline posting
558-
if (presenter) {
559-
try {
560-
await presenter.handleProgressEvent(event);
561-
lastPostedMessage = presenter.getLastPostedMessage() ?? lastPostedMessage;
562-
} catch (err) {
563-
logger.warn("ProgressPresenter.handleProgressEvent failed; falling back to inline posting", { messageId: message.id, error: err instanceof Error ? err.message : String(err) });
564-
// fall through to inline posting below
565-
try {
566-
if (thread) {
567-
const posted = await thread.send(safeProgressMsg);
568-
lastPostedMessage = posted ?? lastPostedMessage;
569-
} else {
570-
const posted = await message.reply(safeProgressMsg);
571-
lastPostedMessage = posted ?? lastPostedMessage;
572-
}
573-
} catch (err2) {
574-
logger.warn("Failed to post progress update via fallback path", { messageId: message.id, error: err2 instanceof Error ? err2.message : String(err2) });
575-
}
576-
}
577-
} else {
578-
try {
579-
if (thread) {
580-
const posted = await thread.send(safeProgressMsg);
581-
lastPostedMessage = posted ?? lastPostedMessage;
582-
} else {
583-
const posted = await message.reply(safeProgressMsg);
584-
lastPostedMessage = posted ?? lastPostedMessage;
585-
}
586-
} catch (err) {
587-
logger.warn("Failed to send progress update to channel/thread", { messageId: message.id, phase: event.phase, error: err instanceof Error ? err.message : String(err) });
588-
}
520+
// Build a safe progress message (wrap title/url in backticks)
521+
const progressMsg = formatProgressMessage(event);
522+
const safeProgressMsg = (() => {
523+
try {
524+
if (event.title) return progressMsg.replace(event.title, `\`${event.title}\``);
525+
if (event.url) return progressMsg.replace(event.url, `\`${event.url}\``);
526+
return progressMsg;
527+
} catch {
528+
return progressMsg;
589529
}
590-
}
530+
})();
591531

592-
// If this event indicates a terminal state, mark it so we ignore
593-
// any subsequent non-actionable events.
594532
try {
595-
if (event.phase === "completed" || event.phase === "failed") {
596-
terminalPhaseSeen = true;
533+
if (thread) {
534+
await thread.send(safeProgressMsg);
535+
} else {
536+
await message.reply(safeProgressMsg);
597537
}
598-
} catch {
599-
// ignore
538+
} catch (err2) {
539+
logger.warn("Failed to post progress update via fallback path", {
540+
messageId: message.id,
541+
error: err2 instanceof Error ? err2.message : String(err2),
542+
});
600543
}
601544
}
545+
}
546+
547+
// Retrieve presenter state for final result handling
548+
const lastPhase = presenter.getLastPhase();
549+
let lastPostedMessage: any = presenter.getLastPostedMessage();
602550
if (finalResult) {
603551
logger.info("CLI processing complete", {
604552
messageId: message.id,
605553
url,
606-
success: finalResult?.success,
607-
title: finalResult?.title,
608-
error: finalResult?.error,
554+
success: finalResult.success,
555+
title: finalResult.title,
556+
error: finalResult.error,
609557
});
610558

611-
if (finalResult?.success) {
559+
if (finalResult.success) {
612560
await removeReaction(message, PROCESSING_REACTION);
613561
await addReaction(message, SUCCESS_REACTION);
614562

@@ -626,73 +574,54 @@ async function processUrlWithProgress(
626574
if (!alreadyCompleted) {
627575
// Post a final success message if the completed phase wasn't already
628576
// observed via progress events.
629-
if (thread) {
630-
try {
631-
const posted = await thread.send(successMsg);
632-
lastPostedMessage = posted ?? lastPostedMessage;
633-
} catch (error) {
634-
logger.warn("Failed to send final success message to thread; falling back to channel reply", {
635-
threadId: thread.id,
636-
error: error instanceof Error ? error.message : String(error),
637-
});
638-
summaryTargetThread = null;
577+
if (thread) {
639578
try {
640-
const posted = await message.reply(successMsg);
579+
const posted = await thread.send(successMsg);
641580
lastPostedMessage = posted ?? lastPostedMessage;
642-
} catch (err) {
643-
logger.warn("Failed to send fallback success reply to channel", {
644-
messageId: message.id,
645-
error: err instanceof Error ? err.message : String(err),
581+
} catch (error) {
582+
logger.warn("Failed to send final success message to thread; falling back to channel reply", {
583+
threadId: thread.id,
584+
error: error instanceof Error ? error.message : String(error),
646585
});
586+
summaryTargetThread = null;
587+
try {
588+
const posted = await message.reply(successMsg);
589+
lastPostedMessage = posted ?? lastPostedMessage;
590+
} catch (err) {
591+
logger.warn("Failed to send fallback success reply to channel", {
592+
messageId: message.id,
593+
error: err instanceof Error ? err.message : String(err),
594+
});
595+
}
647596
}
597+
} else {
598+
const posted = await message.reply(successMsg);
599+
lastPostedMessage = posted ?? lastPostedMessage;
648600
}
649-
} else {
650-
const posted = await message.reply(successMsg);
651-
lastPostedMessage = posted ?? lastPostedMessage;
652-
}
653601
} else {
654602
// completed phase was already posted; try to append item link to
655603
// the last posted message, or fall back to posting the link.
656604
if (itemId !== undefined) {
657605
try {
658-
if (presenter && typeof presenter.appendItemLink === "function") {
659-
try {
660-
await presenter.appendItemLink(itemLink, itemId, successMsg);
661-
} catch (err) {
662-
logger.warn("ProgressPresenter.appendItemLink failed; falling back to inline posting", { messageId: message.id, error: err instanceof Error ? err.message : String(err) });
663-
const appended = `\n\nOpenBrain item: <${itemLink}>\nItem ID: ${itemId}`;
664-
if (lastPostedMessage && typeof lastPostedMessage.edit === "function") {
665-
const prevContent = typeof lastPostedMessage.content === "string" ? lastPostedMessage.content : successMsg;
666-
try {
667-
await lastPostedMessage.edit(prevContent + appended);
668-
} catch (err2) {
669-
logger.warn("Failed to edit completed message with item id", { messageId: message.id, error: err2 instanceof Error ? err2.message : String(err2) });
670-
if (thread) await thread.send(`✅ OpenBrain item: <${itemLink}>`);
671-
else await message.reply(`✅ OpenBrain item: <${itemLink}>`);
672-
}
673-
} else {
674-
if (thread) await thread.send(`✅ OpenBrain item: <${itemLink}>`);
675-
else await message.reply(`✅ OpenBrain item: <${itemLink}>`);
676-
}
677-
}
678-
} else {
679-
const appended = `\n\nOpenBrain item: <${itemLink}>\nItem ID: ${itemId}`;
606+
// Prefer using the presenter's appendItemLink() helper which will
607+
// attempt to edit the last posted message or post a follow-up when
608+
// editing is not possible.
609+
await presenter.appendItemLink(itemLink, itemId, successMsg);
610+
} catch (err) {
611+
logger.warn("ProgressPresenter.appendItemLink failed; falling back to inline posting", { messageId: message.id, error: err instanceof Error ? err.message : String(err) });
612+
const appended = `\n\nOpenBrain item: <${itemLink}>\nItem ID: ${itemId}`;
613+
try {
680614
if (lastPostedMessage && typeof lastPostedMessage.edit === "function") {
681615
const prevContent = typeof lastPostedMessage.content === "string" ? lastPostedMessage.content : successMsg;
682-
try {
683-
await lastPostedMessage.edit(prevContent + appended);
684-
} catch (err2) {
685-
logger.warn("Failed to edit completed message with item id", { messageId: message.id, error: err2 instanceof Error ? err2.message : String(err2) });
686-
if (thread) await thread.send(`✅ OpenBrain item: <${itemLink}>`);
687-
else await message.reply(`✅ OpenBrain item: <${itemLink}>`);
688-
}
616+
await lastPostedMessage.edit(prevContent + appended);
617+
} else if (thread) {
618+
await thread.send(`✅ OpenBrain item: <${itemLink}>`);
689619
} else {
690-
if (thread) await thread.send(`✅ OpenBrain item: <${itemLink}>`);
691-
else await message.reply(`✅ OpenBrain item: <${itemLink}>`);
620+
await message.reply(`✅ OpenBrain item: <${itemLink}>`);
692621
}
622+
} catch (err2) {
623+
logger.warn("Failed to post item link follow-up", { messageId: message.id, error: err2 instanceof Error ? err2.message : String(err2) });
693624
}
694-
} catch (err) {
695-
logger.warn("Failed to post item link follow-up", { messageId: message.id, error: err instanceof Error ? err.message : String(err) });
696625
}
697626
} else {
698627
logger.debug("Skipping duplicate final success message because completed event was already posted", { messageId: message.id, url });

0 commit comments

Comments
 (0)