Greptile Summary

This PR fixes the WebChat final-event history reconciliation race by teaching shouldReloadHistoryForFinalEvent to return false when the chat final event already carries a renderable assistant payload, and by adding a !terminalEventIsForDifferentActiveRun guard to the deferred session.message reload resolution so that a terminal event from an unrelated run cannot prematurely clear the pending key. Tests cover the key scenarios: renderable final → no reload, silent/no-payload final → reload, and deferred reload surviving unrelated terminal events.

Confidence Score: 4/5

Safe to merge; changes are narrow, well-tested, and address a documented race condition without touching unrelated code paths.

No P0/P1 findings. One P2 note about the SILENT_REPLY_PATTERN regex being case-sensitive (potential miss for lowercase/mixed-case sentinel variants). Logic is consistent across all reviewed edge cases.

chat-event-reload.ts — verify SILENT_REPLY_PATTERN case sensitivity is intentional.

This is a comment left during a code review.
Path: ui/src/ui/chat-event-reload.ts
Line: 5

Comment:
**Case-sensitive sentinel may silently miss variants**

`SILENT_REPLY_PATTERN` only matches the exact uppercase string `NO_REPLY`. If the backend ever emits `no_reply`, `No_Reply`, or `NO_REPLY ` with a trailing newline inside the content block, `hasRenderableAssistantFinalMessage` would return `true`, suppressing the history reload and leaving the chat in a stale state. If the sentinel is contractually uppercase-only this is fine, but it is worth an explicit comment or a case-insensitive flag (`/^\s*NO_REPLY\s*$/i`) to make the intent clear.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(ui): stabilize WebChat final reload ..." | Re-trigger Greptile}

Thanks for picking this up and the kind credit! Appreciate you driving this forward systematically.

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

const SILENT_REPLY_PATTERN = /^\s*NO_REPLY\s*$/;
...
return typeof text === "string" && text.trim() !== "" && !SILENT_REPLY_PATTERN.test(text);

const SUPPRESSED_CONTROL_TOKENS = ["NO_REPLY", "ANNOUNCE_SKIP", "REPLY_SKIP"] as const;
const SILENT_REPLY_PATTERN = new RegExp(`^\\s*(?:${SUPPRESSED_CONTROL_TOKENS.join("|")})\\s*$`, "i");

Analyzed PR: #72325 at commit 97aba96

_{Last updated on: 2026-04-27T19:51:38Z}

ProjectClownfish follow-up pushed: c362014

Fix:

• rebased onto current main
• resolved changelog conflict
• addressed Aisle's UI message-integrity concern by keeping plain ANNOUNCE_SKIP/REPLY_SKIP assistant text visible; only legacy NO_REPLY remains hidden client-side

Blacksmith validation:

• pnpm test:serial ui/src/ui/app-gateway.node.test.ts ui/src/ui/chat-event-reload.test.ts ui/src/ui/controllers/chat.test.ts covered the gateway/reload shard; controller is routed separately
• node scripts/run-vitest.mjs run --config test/vitest/vitest.unit-ui.config.ts ui/src/ui/controllers/chat.test.ts
• pnpm check:changed

Rebased again onto current main after unrelated CI noise: 97aba96

No conflict resolution was needed on the final rebase; branch diff is still the same UI/WebChat surface. Prior Blacksmith validation remains scoped to this diff:

• pnpm test:serial ui/src/ui/app-gateway.node.test.ts ui/src/ui/chat-event-reload.test.ts ui/src/ui/controllers/chat.test.ts for gateway/reload shard
• node scripts/run-vitest.mjs run --config test/vitest/vitest.unit-ui.config.ts ui/src/ui/controllers/chat.test.ts
• pnpm check:changed