fix(mattermost): deduplicate truncated patch edits; delete orphan only after fallback succeeds

Two fixes from latest Codex review:

1. Truncation dedup: compare lastSentText against the truncated text (not the full
   rawText) in both schedulePatch and flushPendingPatch. Previously, once a reply
   grew past textLimit the guard compared the growing rawText against the stored
   rawText, so the post would be patched every 200 ms with the same truncated body
   — running into avoidable Mattermost rate limiting on long responses.

2. Orphan cleanup order: in the final-edit fallback path, deliver the replacement
   message first and only delete the orphaned stream post afterward. If the fallback
   send also fails, the user keeps the partial preview instead of losing all visible
   output.
This commit is contained in:
teconomix 2026-03-18 12:37:22 +00:00
parent 7ed3db579f
commit 276e5d735b

View File

@ -1440,10 +1440,13 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
await new Promise<void>((r) => setTimeout(r, 20));
}
const rawText = pendingPatchText;
if (!rawText || rawText === lastSentText) return;
if (!rawText) return;
// Truncate to textLimit so intermediate patches never exceed the server limit.
// Final delivery applies full chunking; streaming posts only need the first chunk.
const text = rawText.length > textLimit ? rawText.slice(0, textLimit) : rawText;
// Guard on the truncated text so long replies (past textLimit) do not keep
// re-patching with the same truncated content every 200 ms and hit rate limits.
if (text === lastSentText) return;
if (!streamMessageId) {
try {
const result = await sendMessageMattermost(to, text, {
@ -1451,7 +1454,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
replyToId: effectiveReplyToId,
});
streamMessageId = result.messageId;
lastSentText = rawText;
lastSentText = text;
runtime.log?.(`stream-patch started ${streamMessageId}`);
} catch (err) {
logVerboseMessage(`mattermost stream-patch flush send failed: ${String(err)}`);
@ -1462,7 +1465,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
postId: streamMessageId,
message: text,
});
lastSentText = rawText;
lastSentText = text;
runtime.log?.(`stream-patch flushed ${streamMessageId}`);
} catch (err) {
logVerboseMessage(`mattermost stream-patch flush failed: ${String(err)}`);
@ -1476,9 +1479,12 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
if (patchInterval) return;
patchInterval = setInterval(() => {
const rawText = pendingPatchText;
if (!rawText || rawText === lastSentText || patchSending) return;
if (!rawText || patchSending) return;
// Truncate to textLimit so intermediate patches never exceed the server limit.
const text = rawText.length > textLimit ? rawText.slice(0, textLimit) : rawText;
// Guard on the truncated text so long replies (past textLimit) do not keep
// re-patching with the same truncated content every 200 ms and hit rate limits.
if (text === lastSentText) return;
patchSending = true;
void (async () => {
try {
@ -1489,7 +1495,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
replyToId: effectiveReplyToId,
});
streamMessageId = result.messageId;
lastSentText = rawText;
lastSentText = text;
runtime.log?.(`stream-patch started ${streamMessageId}`);
} catch (err) {
logVerboseMessage(`mattermost stream-patch send failed: ${String(err)}`);
@ -1500,7 +1506,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
postId: streamMessageId,
message: text,
});
lastSentText = rawText;
lastSentText = text;
runtime.log?.(`stream-patch edited ${streamMessageId}`);
} catch (err) {
logVerboseMessage(`mattermost stream-patch edit failed: ${String(err)}`);
@ -1553,11 +1559,10 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
);
const orphanId = streamMessageId;
streamMessageId = null;
try {
await deleteMattermostPost(blockStreamingClient!, orphanId);
} catch {
// Ignore delete failure — delivering the complete message takes priority
}
// Deliver the fallback message first. Only delete the orphaned
// stream post after we know the replacement was successfully sent —
// if delivery also fails the user keeps the partial preview rather
// than losing all visible output.
await deliverMattermostReplyPayload({
core,
cfg,
@ -1573,6 +1578,12 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
tableMode,
sendMessage: sendMessageMattermost,
});
// Fallback succeeded — now clean up the orphaned partial.
try {
await deleteMattermostPost(blockStreamingClient!, orphanId);
} catch {
// Ignore — the complete message was already delivered.
}
return;
}
// Successful final patch: reset all streaming state.