When #218 and #229 were integrated in modified form and closed, their unresolved review threads went with them. Two of the findings apply to the code that actually shipped. Sweeping them here rather than leaving them attached to closed PRs.
1. A second desktop notice is not tracked as plugin-generated (billing risk)
sendIgnoredMessage sets the tracked id only when the placement helper returns one:
if (messageID) request.body.messageID = messageID
…
if (request.body.messageID) { options.onPreparedMessageId?.(request.body.messageID) }notificationMessageIdBeforeAssistant returns undefined when its candidate would sort at or below the latest user message. After notice 1 is inserted it is the latest user message, so a second notice against the same active assistant gets no id, the host assigns an untracked one, and isDesktopNoticeMessage returns false for it. It is then treated as a genuine user message — which revokes the delivery lease and is the same path that produced a duplicate billed provider turn in #162.
Reproduced against the helper's real logic:
notice 1 id : msg_0000000effffzzzzzzzzzzzzzz
notice 2 id : undefined <- host assigns an untracked id
The switch→restoration pair is exactly a two-notice sequence, so this is reachable on the path the recovery fix serves. Original finding: cubic on #229 (index.ts:657), confidence 8.
2. desktopNoticeMessageIds is unbounded in session count
Every sibling in that family is bounded two ways — per-entry and across sessions:
pendingDesktopNotices size > 128 cap ✓
desktopNoticeSafeSessions size > 128 cap ✓
desktopNoticeMessageIds per-set cap of 4, no map-level cap ✗
The only eviction is session.deleted. Sessions that are never deleted — the normal case — leave an entry per session for the process lifetime. Small per entry (a Set of ≤4 short strings), so this is a slow leak rather than a bug, but it is an inconsistency with its siblings in a process designed to run for weeks. Original finding: cubic on #218 (index.ts:5320), confidence 8.
Happy to send a PR for either; #2 is a four-line change matching the sibling pattern, #1 needs a decision on whether to mint a unique ordered id per notice or correlate the host-assigned one.
When #218 and #229 were integrated in modified form and closed, their unresolved review threads went with them. Two of the findings apply to the code that actually shipped. Sweeping them here rather than leaving them attached to closed PRs.
1. A second desktop notice is not tracked as plugin-generated (billing risk)
sendIgnoredMessagesets the tracked id only when the placement helper returns one:notificationMessageIdBeforeAssistantreturnsundefinedwhen its candidate would sort at or below the latest user message. After notice 1 is inserted it is the latest user message, so a second notice against the same active assistant gets no id, the host assigns an untracked one, andisDesktopNoticeMessagereturns false for it. It is then treated as a genuine user message — which revokes the delivery lease and is the same path that produced a duplicate billed provider turn in #162.Reproduced against the helper's real logic:
The switch→restoration pair is exactly a two-notice sequence, so this is reachable on the path the recovery fix serves. Original finding: cubic on #229 (
index.ts:657), confidence 8.2.
desktopNoticeMessageIdsis unbounded in session countEvery sibling in that family is bounded two ways — per-entry and across sessions:
The only eviction is
session.deleted. Sessions that are never deleted — the normal case — leave an entry per session for the process lifetime. Small per entry (a Set of ≤4 short strings), so this is a slow leak rather than a bug, but it is an inconsistency with its siblings in a process designed to run for weeks. Original finding: cubic on #218 (index.ts:5320), confidence 8.Happy to send a PR for either; #2 is a four-line change matching the sibling pattern, #1 needs a decision on whether to mint a unique ordered id per notice or correlate the host-assigned one.