diff --git a/apps/web/src/components/ChatComposer.tsx b/apps/web/src/components/ChatComposer.tsx index d9d135697..9c9686878 100644 --- a/apps/web/src/components/ChatComposer.tsx +++ b/apps/web/src/components/ChatComposer.tsx @@ -47,6 +47,11 @@ import { type InlineMentionEntity, } from '../utils/inlineMentions'; import { isImeComposing } from '../utils/imeComposing'; +import { + reconcileInsertions, + stripPluginInsertedTokens, + type TrackedInsertion, +} from '../utils/pluginInsertionTracking'; import { ANNOTATION_EVENT, type AnnotationEventDetail } from "./PreviewDrawOverlay"; type TranslateFn = (key: keyof Dict, vars?: Record) => string; @@ -224,7 +229,23 @@ export const ChatComposer = forwardRef( ) { const t = useT(); const analytics = useAnalytics(); - const [draft, setDraft] = useState(() => initialDraft ?? loadComposerDraft(draftStorageKey) ?? ""); + const [draft, setDraft] = useState( + () => initialDraft ?? loadComposerDraft(draftStorageKey) ?? "", + ); + // Synchronous mirror of the latest committed draft value. + // `updateDraft` reads this as `prev` instead of relying on the + // closure `draft` (which only updates after re-render) or + // `setDraft((prev) => …)` (whose updater is double-invoked + // under React StrictMode and would mutate + // `pluginInsertedTokensRef` twice). The ref is updated + // synchronously by `updateDraft` before `setDraft`, so the + // next call sees a fresh `prev` even when React batches + // multiple updates within one tick. Initialized from the same + // source as the React state to keep the two in lockstep on + // first render. See `updateDraft` below and #2929 round 5. + const draftRef = useRef( + initialDraft ?? loadComposerDraft(draftStorageKey) ?? "", + ); // chat_panel page_view fires from ProjectView (which outlives // conversation switches) so the event measures real chat-panel @@ -271,6 +292,77 @@ export const ChatComposer = forwardRef( // or from the tools-menu "Details" affordance. const [detailsRecord, setDetailsRecord] = useState(null); const pluginsSectionRef = useRef(null); + // Instance-aware tracking for `@` mentions this surface + // inserted into the draft via the @-mention popover plugin-pick + // path (`insertPluginMention`). Each entry pins the precise + // start offset of `@`, so two `@Airbnb` mentions in the same + // draft (one composer-inserted, one user-authored) are + // distinguishable — the chip-clear strip removes only tracked + // instances (#2929 round 3). See utils/pluginInsertionTracking.ts + // for the diff/reconcile/strip primitives. + // + // Lifecycle invariants: + // - add: `insertPluginMention` pushes { token, start } using the + // `insertStart` returned by `replaceMentionWithText` + // - reconcile: `handleChange` runs LCP/LCS diff on each + // keystroke and shifts/drops entries whose offsets crossed + // the edit, plus revalidates surviving entries against the + // mention boundary so `@Airbnbify`-style corruption prunes + // - clear: `reset()` empties the array on send; `onCleared` + // strips by range and empties the array + // + // Tools-menu / details-modal applies route through + // `pluginsSectionRef.current.applyById` without writing to the + // draft, so the array stays empty for those surfaces and the + // post-clear strip is a no-op. Every draft mutation in this + // component goes through the `updateDraft` chokepoint, which + // runs `reconcileInsertions` against the prev → next diff. That + // includes typing, slash-command pick, file/MCP/connector + // insertion, skill chip remove, annotation append, imperative + // handle, post-send reset, and the on-cleared strip itself — + // so a tracked offset can never go stale relative to the draft + // and re-introduce the original #2881 orphan-mention symptom + // (#2929 round 4). + // + // Each entry carries the `pluginId` of the apply that produced + // it. When the active plugin changes (e.g. tools-menu `applyById` + // replaces plugin A with plugin B without writing to the draft), + // entries for the previous active plugin are dropped via + // `setActivePlugin`. Without that, clearing B's chip would still + // strip A's `@A` from the draft — silent user-text deletion in a + // supported replace-plugin flow (#2929 round 6). + const pluginInsertedTokensRef = useRef([]); + // The plugin id whose chip is currently mounted in PluginsSection's + // chip strip, or `null` after the strip clears or before any apply + // succeeds. Updated via `setActivePlugin`, which also drops any + // tracked entries whose `pluginId` does not match the new active + // — a no-op for `insertPluginMention` (the new entry it just + // pushed matches), critical for tools-menu / details-modal + // applies that arrive without an accompanying draft insertion. + const activePluginIdRef = useRef(null); + // Monotonic counter that hands out unique `insertionId` strings to + // entries pushed by `insertPluginMention`. The id survives + // `reconcileInsertions` (utils/pluginInsertionTracking.ts forwards + // the field) so the in-flight handler's failure path can locate + // its own tracked entry even after intervening reconciles or + // `onCleared` mutations of the array (#2929 round 10 codex + // review). Plain ref counter is enough — the id only needs to be + // unique within a single composer instance and is never persisted. + const insertionIdSeqRef = useRef(0); + + // Single chokepoint for setting the active plugin. Routes every + // `applyById` call so the tracker stays in lockstep with the + // chip strip's currently-mounted plugin. + function setActivePlugin(pluginId: string | null): void { + if (activePluginIdRef.current === pluginId) return; + if (pluginInsertedTokensRef.current.length > 0) { + pluginInsertedTokensRef.current = + pluginInsertedTokensRef.current.filter( + (entry) => entry.pluginId === pluginId, + ); + } + activePluginIdRef.current = pluginId; + } // Consolidated "tools" popover — a single dropdown anchored to the // leading sliders icon that hosts MCP / Import / Pet quick actions and // a shortcut to open the full Settings dialog. Replaces the previous @@ -299,7 +391,7 @@ export const ChatComposer = forwardRef( useEffect(() => { if (seededRef.current) return; if (initialDraft && initialDraft !== draft) { - setDraft(initialDraft); + updateDraft(initialDraft); seededRef.current = true; } else if (initialDraft === undefined) { seededRef.current = true; @@ -614,7 +706,7 @@ export const ChatComposer = forwardRef( // command's canonical insertion text. const replaced = before.replace(/\/[^\s/]*$/, cmd.insert); const next = replaced + after; - setDraft(next); + updateDraft(next); setSlash(null); requestAnimationFrame(() => { ta.focus(); @@ -658,7 +750,7 @@ export const ChatComposer = forwardRef( const trimmed = draft.trim(); if (!/^\/mcp\s*$/i.test(trimmed)) return false; onOpenMcpSettings(); - setDraft(''); + updateDraft(''); return true; } @@ -724,7 +816,7 @@ export const ChatComposer = forwardRef( return false; } } - setDraft(''); + updateDraft(''); return true; } @@ -732,7 +824,7 @@ export const ChatComposer = forwardRef( ref, () => ({ setDraft: (text: string) => { - setDraft(text); + updateDraft(text); seededRef.current = true; requestAnimationFrame(() => { const ta = textareaRef.current; @@ -743,7 +835,7 @@ export const ChatComposer = forwardRef( }); }, restoreDraft: ({ text, attachments = [], commentAttachments = [] }) => { - setDraft(text); + updateDraft(text); setStaged(attachments); setStagedVisualComments(commentAttachments); setStagedSkills([]); @@ -768,8 +860,39 @@ export const ChatComposer = forwardRef( [] ); + // Single chokepoint for every draft mutation. Reconciles the + // tracked plugin-mention offsets against the prev → next diff so + // any setDraft path — typing, slash command, file/MCP/connector + // insertion, skill chip removal, annotation append, imperative + // handle, post-send reset, on-cleared strip — keeps + // `pluginInsertedTokensRef` in lockstep with the draft. + // + // Implementation note (#2929 round 5): the reconcile and the + // ref mutation happen *outside* the `setDraft` updater, using + // the synchronous `draftRef` mirror as `prev`. Putting them + // inside `setDraft((prev) => …)` would not be safe under + // React StrictMode, which double-invokes setState updaters in + // development to detect impurity — the second invocation + // would re-shift or re-drop already-reconciled entries, + // bringing back the #2881 orphan-mention symptom for every + // user keystroke in the dev build. + function updateDraft(next: string | ((prev: string) => string)): void { + const prev = draftRef.current; + const value = typeof next === 'function' ? next(prev) : next; + if (prev === value) return; + if (pluginInsertedTokensRef.current.length > 0) { + pluginInsertedTokensRef.current = reconcileInsertions( + pluginInsertedTokensRef.current, + prev, + value, + ); + } + draftRef.current = value; + setDraft(value); + } + function reset() { - setDraft(""); + updateDraft(""); setStaged([]); setStagedVisualComments([]); setStagedSkills([]); @@ -778,6 +901,14 @@ export const ChatComposer = forwardRef( setUploadError(null); setMention(null); setSlash(null); + // Drop tracked plugin-mention insertions when the draft is wiped + // — otherwise a later chip clear would prune user-authored text + // that happened to share a label with a previously-applied + // plugin (#2929 round 2/3). Also clear the active-plugin id + // so the next applyById is treated as a fresh activation + // rather than a "same plugin re-apply" (#2929 round 6). + pluginInsertedTokensRef.current = []; + activePluginIdRef.current = null; } function currentCommentAttachments(extra: ChatCommentAttachment[] = []): ChatCommentAttachment[] { @@ -829,7 +960,7 @@ export const ChatComposer = forwardRef( // Also strip the matching `@` token from the draft so the chip // and the textarea stay in sync. We allow trailing whitespace to be // collapsed too. - setDraft((d) => + updateDraft((d) => d .replace(new RegExp(`(^|\\s)@${escapeRegExp(id)}(\\s|$)`, 'g'), '$1$2') .replace(/\s{2,}/g, ' '), @@ -1001,7 +1132,7 @@ export const ChatComposer = forwardRef( }), ]); } - if (detail.note) setDraft((d) => (d ? `${d}\n${detail.note}` : detail.note)); + if (detail.note) updateDraft((d) => (d ? `${d}\n${detail.note}` : detail.note)); setStreamingAnnotationSendPending(true); textareaRef.current?.focus(); ack({ ok: true }); @@ -1022,7 +1153,7 @@ export const ChatComposer = forwardRef( } if (detail.note) { - setDraft((d) => (d ? `${d}\n${detail.note}` : detail.note)); + updateDraft((d) => (d ? `${d}\n${detail.note}` : detail.note)); textareaRef.current?.focus(); } ack({ ok: true }); @@ -1118,7 +1249,10 @@ export const ChatComposer = forwardRef( function handleChange(e: React.ChangeEvent) { const value = e.target.value; const cursor = e.target.selectionStart; - setDraft(value); + // Goes through the `updateDraft` chokepoint so the + // plugin-mention offset reconcile runs on every keystroke, + // matching every other setDraft path for free. + updateDraft(value); // Keep the staged-skill chips in sync with the draft. If the user // hand-deletes an `@` token from the textarea, the chip must // disappear too — otherwise submit() would still forward that id in @@ -1165,7 +1299,7 @@ export const ChatComposer = forwardRef( const after = draft.slice(cursor); const replaced = before.replace(/@([^\s@]*)$/, `@${filePath} `); const next = replaced + after; - setDraft(next); + updateDraft(next); setMention(null); if (!staged.some((s) => s.path === filePath)) { setStaged((s) => [ @@ -1185,28 +1319,175 @@ export const ChatComposer = forwardRef( } async function insertPluginMention(record: InstalledPluginRecord) { - const inserted = replaceMentionWithText(`${inlineMentionToken(record.title)} `); - if (!inserted) return; - await pluginsSectionRef.current?.applyById(record.id, record); + // Snapshot tracker AND draft state before any mutation so we + // can roll back if `applyById` fails (#2929 round 7). Without + // this, an `/apply` 5xx leaves the draft holding a freshly + // inserted `@` whose chip never mounted — a user + // clearing the previously-active plugin's chip would then + // strip the user-visible `@` they just picked, even + // though that text is the only signal they have that + // anything happened. + const prevDraftValue = draftRef.current; + const prevEntries = pluginInsertedTokensRef.current; + const prevActiveId = activePluginIdRef.current; + + const result = replaceMentionWithText(`${inlineMentionToken(record.title)} `); + if (!result) return; + // Capture the post-insert draft *snapshot* — the value the + // composer is in immediately after our optimistic write. + // Used as a sentinel during the rollback below: if the + // textarea is still in this state when `applyById` fails + // (no user keystrokes during the await), we can fully + // restore `prevDraftValue`. If the user typed during the + // await, the draft has moved past the snapshot and we MUST + // NOT clobber those edits with the stale `prevDraftValue` + // (#2929 round 8 — the textarea stays interactive while + // `/apply` is in flight, so this is a real prompt-data-loss + // path). + const postInsertDraft = draftRef.current; + // Track the precise start offset of the inserted `@` so the + // post-clear strip can excise exactly this instance, leaving + // any user-authored `@` elsewhere in the draft + // untouched (#2929 round 3). Entry carries `pluginId` so a + // later replace-plugin flow can drop it cleanly (#2929 round 6), + // and an `insertionId` so this handler's failure path can + // locate the entry it pushed even after `reconcileInsertions` + // shifted offsets or `onCleared` mutated the array + // (#2929 round 10). + // + // Push the new entry but DO NOT yet drop entries from the + // previously-active plugin — that filter is committed only + // after `applyById` resolves successfully (#2929 round 9 + // codex review). During the await, the chip strip still + // shows the previously-mounted plugin and the textarea is + // interactive: a user click on that chip's × must strip its + // tracked entries (not the optimistic `@` we just + // pushed). `onCleared` filters by + // `pluginsSectionRef.current?.getActiveRecord()?.id` so a + // pending-window clear scopes to the actually-mounted + // plugin's tracked tokens. + const ourInsertionId = `i${++insertionIdSeqRef.current}`; + pluginInsertedTokensRef.current = [ + ...pluginInsertedTokensRef.current, + { + token: record.title, + start: result.insertStart, + pluginId: record.id, + insertionId: ourInsertionId, + }, + ]; + + const applyResult = await pluginsSectionRef.current?.applyById( + record.id, + record, + ); + if (!applyResult) { + // Two failure modes to disambiguate (#2929 round 10): + // + // (a) "no intervening clear" — the user neither cleared + // the previously-mounted chip nor anything else + // mutated the tracker beyond our push + reconciles + // from user keystrokes. `prevEntries` and + // `prevActiveId` are still the truth. We restore the + // tracker wholesale and restore the draft only if + // the user did not type during the await + // (round 7/8 path). + // + // (b) "intervening clear" — `onCleared` ran during the + // await for the previously-mounted chip, stripped + // its tokens from the draft, and nulled + // `activePluginIdRef`. Restoring `prevEntries` + // wholesale here would resurrect already-stripped + // entries with stale offsets, AND leave our + // optimistic `@` orphaned in the draft (the + // original #2881 symptom recurring inside the + // failure window). Instead we surgically remove ONLY + // our own optimistic entry by `insertionId`, strip + // its `@` from the draft, and leave + // everything `onCleared` did intact. + // + // Detection: `onCleared` always nulls + // `activePluginIdRef.current`; our deferred + // `setActivePlugin` never ran (we are in the failure + // branch). So `activePluginIdRef.current === null` while + // `prevActiveId !== null` is the smoking gun for an + // intervening clear. (If `prevActiveId` was already null, + // there was no chip to clear — no race possible.) + const intervenedClear = + activePluginIdRef.current === null && prevActiveId !== null; + if (intervenedClear) { + const cur = pluginInsertedTokensRef.current; + const idx = cur.findIndex( + (e) => e.insertionId === ourInsertionId, + ); + if (idx >= 0) { + const ourEntry = cur[idx]!; + // Splice our entry out first so `updateDraft`'s + // internal `reconcileInsertions` operates on a tracker + // that already excludes it (the strip range overlaps + // the entry, which would drop it anyway, but splicing + // first keeps the invariant explicit and avoids + // depending on the reconcile drop edge case). + pluginInsertedTokensRef.current = [ + ...cur.slice(0, idx), + ...cur.slice(idx + 1), + ]; + updateDraft((d) => stripPluginInsertedTokens(d, [ourEntry])); + } + // Don't touch `activePluginIdRef` — `onCleared` set it + // to null and that is the truth (no chip is mounted). + return; + } + // (a) round 7/8 path: no intervening clear. + pluginInsertedTokensRef.current = prevEntries; + activePluginIdRef.current = prevActiveId; + // Restore the draft only if no user keystrokes arrived + // during the await — overwriting newer edits with the + // stale pre-pick snapshot would be a worse bug than the + // leftover `@` styled mention this branch leaves + // behind. The orphan stays as a styled mention but no + // future chip clear will touch it (tracker is empty for + // it now), and the user can edit it manually + // (#2929 round 8). + if (draftRef.current === postInsertDraft) { + setDraft(prevDraftValue); + draftRef.current = prevDraftValue; + } + return; + } + // Apply succeeded. Now commit the active-plugin switch — + // this drops any entries from the previously-active plugin + // (a no-op for the entry we just pushed since it matches + // `record.id`) and updates `activePluginIdRef`. Deferring + // until after the await means an `onCleared` triggered + // during the in-flight window saw the still-mounted plugin + // as the active one and stripped only that plugin's tokens + // (#2929 round 9). + setActivePlugin(record.id); } - function replaceMentionWithText(text: string): boolean { - if (!mention) return false; + function replaceMentionWithText( + text: string, + ): { insertStart: number } | null { + if (!mention) return null; const ta = textareaRef.current; const cursor = mention.cursor; const before = draft.slice(0, cursor); const after = draft.slice(cursor); const replaced = before.replace(/(^|\s)@([^\s@]*)$/, `$1${text}`); const next = replaced + after; - setDraft(next); + updateDraft(next); setMention(null); + // The inserted text was appended onto `replaced`, so its first + // char (the `@`) sits at `replaced.length - text.length`. + const insertStart = replaced.length - text.length; requestAnimationFrame(() => { if (!ta) return; ta.focus(); const pos = replaced.length; ta.setSelectionRange(pos, pos); }); - return true; + return { insertStart }; } function insertMcpMention(server: McpServerConfig) { @@ -1234,7 +1515,7 @@ export const ChatComposer = forwardRef( function removeStaged(p: string) { setStaged((s) => s.filter((a) => a.path !== p)); setStagedVisualComments((current) => current.filter((attachment) => attachment.screenshotPath !== p)); - setDraft((current) => stripInlineMentionToken(current, p)); + updateDraft((current) => stripInlineMentionToken(current, p)); } function removeCommentAttachment(id: string) { @@ -1473,12 +1754,73 @@ export const ChatComposer = forwardRef( showRail={false} onApplied={(brief) => { // Use functional setState so stale closures from the @-mention - // flow (which awaits applyById after setDraft) still see the - // latest draft value before deciding whether to seed. + // flow (which awaits applyById after updateDraft) still see + // the latest draft value before deciding whether to seed. if (typeof brief === 'string' && brief.length > 0) { - setDraft((cur) => (cur.trim().length === 0 ? brief : cur)); + updateDraft((cur) => (cur.trim().length === 0 ? brief : cur)); } }} + onCleared={() => { + // Removing the chip strip must drop the `@…` tokens + // this surface authored, otherwise the textarea is + // left holding orphaned mentions whose chips just + // unmounted (#2881). We strip *only* the tracked + // insertions (by precise start offset) so + // user-authored text that happens to share a label + // with a chip is preserved (#2929 round 3). + // + // The chip strip can clear while an `applyById` for + // a *different* plugin is mid-await — the @-popover + // optimistically writes `@` and pushes a + // tracked entry synchronously, then awaits the + // apply (#2929 round 9 codex review). During that + // window the ref carries entries for both the + // still-mounted plugin (the chip the user is + // removing) and the in-flight target. Trusting the + // ref wholesale here would strip the optimistic + // `@` and leave the unmounting plugin's + // `@` orphaned — a recurrence of #2881 in a + // pending-apply window. + // + // PluginsSection only flips `activeRecord` after + // `applyPlugin` resolves successfully (see + // `PluginsSection.tsx`), so `getActiveRecord()` at + // the moment `onCleared` fires reports the plugin + // whose chip is currently being unmounted — exactly + // the one whose tracked entries we should strip. + // Filter to that id; entries for any in-flight + // replace target are left in place (the in-flight + // handler's success path will commit + // `setActivePlugin(target)` and drop them; its + // failure path will roll the tracker back). + const unmountingId = + pluginsSectionRef.current?.getActiveRecord()?.id ?? null; + const entries = pluginInsertedTokensRef.current; + if (entries.length > 0) { + const toStrip = unmountingId + ? entries.filter((e) => e.pluginId === unmountingId) + : entries; + if (toStrip.length > 0) { + // `updateDraft` runs `reconcileInsertions` + // against the prev → next diff inside the + // chokepoint, so any in-flight target's entries + // get their offsets shifted to track the + // post-strip draft. We must re-read the ref + // *after* `updateDraft` returns instead of + // filtering the pre-strip `entries` snapshot, + // otherwise we would clobber the reconciled + // offsets and a later clear of the in-flight + // chip would no-op via `isInsertionStillValid`. + updateDraft((d) => stripPluginInsertedTokens(d, toStrip)); + } + pluginInsertedTokensRef.current = unmountingId + ? pluginInsertedTokensRef.current.filter( + (e) => e.pluginId !== unmountingId, + ) + : []; + } + activePluginIdRef.current = null; + }} onChipDetails={(item: ContextItem) => { if (item.kind !== 'plugin') return; const record = installedPlugins.find((p) => p.id === item.id); @@ -1700,11 +2042,32 @@ export const ChatComposer = forwardRef( plugins={pluginsForComposer} activePluginId={pinnedPluginId} onApply={async (record) => { + // Tools-menu apply: no draft write, so the + // tracked-insertion array gets no new + // entry. The active-plugin switch (which + // drops previously-tracked entries from a + // prior @-popover pick of a different + // plugin, #2929 round 6) is deferred until + // `applyById` resolves successfully so + // that an `onCleared` triggered during the + // in-flight window still sees the + // still-mounted plugin's entries and + // strips them correctly via the + // `getActiveRecord()` filter in + // `onCleared` (#2929 round 9). + // + // No synchronous mutation in this branch + // means no rollback snapshot is needed: + // the failure path is just an early return + // (#2929 round 7's snapshot was needed + // because `setActivePlugin` was eager). const result = await pluginsSectionRef.current?.applyById( record.id, record, ); - if (result) setToolsOpen(false); + if (!result) return; + setActivePlugin(record.id); + setToolsOpen(false); }} onShowDetails={(record) => { setDetailsRecord(record); @@ -1726,7 +2089,7 @@ export const ChatComposer = forwardRef( const before = currentDraft.slice(0, cursor); const after = currentDraft.slice(cursor); const next = before + insert + after; - setDraft(next); + updateDraft(next); setToolsOpen(false); requestAnimationFrame(() => { const el = textareaRef.current; @@ -1750,7 +2113,7 @@ export const ChatComposer = forwardRef( const before = draft.slice(0, cursor); const after = draft.slice(cursor); const next = before + insert + after; - setDraft(next); + updateDraft(next); setToolsOpen(false); requestAnimationFrame(() => { const el = textareaRef.current; @@ -1905,7 +2268,24 @@ export const ChatComposer = forwardRef( record={detailsRecord} onClose={() => setDetailsRecord(null)} onUse={async (record) => { - await pluginsSectionRef.current?.applyById(record.id, record); + // Details-modal apply: same shape as tools-menu apply + // (no draft write). The active-plugin switch is + // deferred until `applyById` resolves successfully so + // that an `onCleared` triggered during the in-flight + // window still sees the still-mounted plugin's + // entries and strips them correctly (#2929 round 9). + // + // Modal closes regardless of apply outcome so the + // user is not stuck on the details view if `/apply` + // 5xx'd. Failure is a no-op: no synchronous mutation + // happened, so nothing to roll back (#2929 round 7's + // snapshot was needed because `setActivePlugin` was + // eager — round 9 made it lazy). + const result = await pluginsSectionRef.current?.applyById( + record.id, + record, + ); + if (result) setActivePlugin(record.id); setDetailsRecord(null); }} /> diff --git a/apps/web/src/utils/inlineMentions.ts b/apps/web/src/utils/inlineMentions.ts index 587fa4638..590f72992 100644 --- a/apps/web/src/utils/inlineMentions.ts +++ b/apps/web/src/utils/inlineMentions.ts @@ -142,11 +142,35 @@ function pickEarlierMention( return known.token.length >= unknown.token.length ? known : unknown; } -function isMentionBoundary(text: string, start: number): boolean { +/** + * Left boundary rule for inline mentions: `@` is a candidate + * mention only when the character before `@` is the start of the + * string or whitespace / opening bracket / quote. Exported so the + * draft-side plugin-insertion tracker stays in lockstep with this + * parser — see `apps/web/src/utils/pluginInsertionTracking.ts`. + */ +export function isMentionBoundary(text: string, start: number): boolean { if (start === 0) return true; return /[\s([{"']/.test(text[start - 1] ?? ''); } +/** + * Right boundary rule for inline mentions: the parser's unknown + * mention regex is `/@[^\s@]+/`, so a `@` candidate is the + * full mention only when the character after the token is the end + * of the string, whitespace, or another `@` (which would start a + * new mention). Anything else extends the parser's tokenization + * past the candidate — e.g. `@Airbnb/foo` is parsed as a single + * mention even when `@Airbnb` is a known plugin. Exported for the + * same reason as `isMentionBoundary`: the draft-side tracker must + * not declare an entry "still valid" when the parser would no + * longer see the tracked token as a standalone mention. + */ +export function isMentionRightBoundary(text: string, end: number): boolean { + if (end >= text.length) return true; + return /[\s@]/.test(text[end] ?? ''); +} + function coalesceTextParts(parts: InlineMentionPart[]): InlineMentionPart[] { const result: InlineMentionPart[] = []; for (const part of parts) { diff --git a/apps/web/src/utils/pluginInsertionTracking.ts b/apps/web/src/utils/pluginInsertionTracking.ts new file mode 100644 index 000000000..b657f0619 --- /dev/null +++ b/apps/web/src/utils/pluginInsertionTracking.ts @@ -0,0 +1,224 @@ +// Instance-aware tracking for `@` mentions that ChatComposer +// inserts into the draft via the @-mention popover plugin-pick path +// (#2881, #2929). Each tracked insertion records the precise start +// offset of `@`, so two `@Airbnb` instances in the same draft (one +// composer-inserted, one user-typed) are individually distinguishable +// — the chip-clear strip only removes the tracked one (#2929 round 3). +// +// Boundary rules are imported from `./inlineMentions` so the +// "tracker thinks this is still a valid mention" predicate stays in +// lockstep with the actual mention parser. Without that, drafts +// like `@Airbnb/foo` (where the parser tokenizes the full +// `@Airbnb/foo` as one mention) would still satisfy a permissive +// tracker boundary, and the post-clear strip would tear out only +// `@Airbnb`, leaving `/foo` as orphaned user-authored text +// (#2929 round 5). +import { + isMentionBoundary, + isMentionRightBoundary, +} from './inlineMentions'; + +export type TrackedInsertion = { + /** Bare token without the leading `@`, matching `inlineMentionToken` payload. */ + token: string; + /** Position of `@` in the draft. The full mention occupies [start, start + token.length + 1). */ + start: number; + /** + * `id` of the `InstalledPluginRecord` whose `applyById` produced this + * insertion. Used by ChatComposer to scope the post-clear strip to the + * currently active plugin: when the user replaces plugin A with plugin + * B (e.g. via the tools menu's `applyById` without writing to the + * draft), the entries for A must be dropped from the tracker, otherwise + * clearing B's chip would silently strip A's `@A` from the draft — + * user-text deletion in a supported replace-plugin flow (#2929 round 6). + */ + pluginId: string; + /** + * Optional unique handle assigned by the producer (ChatComposer's + * `insertPluginMention`) when the entry is pushed. Survives + * `reconcileInsertions` so the producer's failure-path rollback can + * locate "the entry I just pushed" even after intervening reconciles + * shifted offsets or after `onCleared` mutated the array. Without + * this, a `(token, pluginId)`-only match is ambiguous if the user + * replays the same plugin pick during the await window + * (#2929 round 10 codex review). + */ + insertionId?: string; +}; + +export type EditRange = { + /** First index where prev and next differ. */ + start: number; + /** Index in prev one past the last differing char. */ + oldEnd: number; + /** Index in next one past the last differing char. */ + newEnd: number; +}; + +/** + * Longest-common-suffix + longest-common-prefix diff of two strings. + * Returns the minimal `[editStart, oldEnd, newEnd]` range that contains + * every byte that differs between `prev` and `next`. + * + * Suffix is computed first; the prefix is then capped so the two + * matches do not overlap. This ordering matters when the inserted + * text shares a leading character with `prev` — e.g. prepending + * `@github ` before `@Airbnb ` to get `@github @Airbnb `. A + * prefix-first walk would greedily claim the leading `@` (LCP=1) + * and then split the diff window at index 1, which crosses through + * a tracked `@Airbnb` entry that is structurally untouched. Suffix + * first claims the entire `@Airbnb ` from the right, leaving LCP=0 + * and a clean prepend window of `[0, 0, 8]` — the entry shifts + * cleanly by `delta`. + * + * Single-point edits (typing/deleting/pasting at one location) are + * 100% accurate. Multi-point simultaneous edits (rare) collapse into + * one wider range, which conservatively invalidates any tracked + * insertion overlapping that range. + */ +export function computeEditRange(prev: string, next: string): EditRange { + if (prev === next) return { start: 0, oldEnd: 0, newEnd: 0 }; + const minLen = Math.min(prev.length, next.length); + // Longest common suffix first — capped at minLen so it does not + // walk past the start of either string. + let suffix = 0; + while ( + suffix < minLen && + prev.charCodeAt(prev.length - 1 - suffix) === + next.charCodeAt(next.length - 1 - suffix) + ) { + suffix++; + } + // Longest common prefix, capped so it does not overlap the suffix. + const maxStart = minLen - suffix; + let start = 0; + while (start < maxStart && prev.charCodeAt(start) === next.charCodeAt(start)) { + start++; + } + return { + start, + oldEnd: prev.length - suffix, + newEnd: next.length - suffix, + }; +} + +/** + * True iff the draft still contains `@` at the given start + * offset AND the surrounding characters make the parser see it as + * exactly that mention (not a longer one). Boundaries delegate to + * the same `isMentionBoundary` / `isMentionRightBoundary` helpers + * the parser uses, so the tracker cannot diverge from the parser + * and inadvertently strip a prefix of a longer parser-recognized + * mention (#2929 round 5). + * + * Concretely: a tracked `@Airbnb` at offset 0 in `@Airbnb/foo` is + * INVALID under this rule because the parser treats the full + * `@Airbnb/foo` as one mention (its `@[^\s@]+` greedy regex extends + * through `/foo`). Stripping just `@Airbnb` would leave `/foo` + * dangling — that is user-authored text mutation, not an orphan + * removal. Invalidating the entry on clear is the conservative + * choice: the post-clear strip becomes a no-op, the orphan + * styled mention remains visible, and the user can edit it + * manually if they want. + */ +export function isInsertionStillValid( + draft: string, + start: number, + token: string, +): boolean { + if (start < 0 || token.length === 0) return false; + const tokenLen = token.length + 1; // include leading `@` + if (start + tokenLen > draft.length) return false; + if (draft.slice(start, start + tokenLen) !== `@${token}`) return false; + if (!isMentionBoundary(draft, start)) return false; + if (!isMentionRightBoundary(draft, start + tokenLen)) return false; + return true; +} + +/** + * Re-map tracked insertion offsets across a draft edit. Entries + * entirely before the edit keep their offset; entries entirely + * after shift by `delta`; entries that overlap the edit are + * dropped. Survivors are revalidated against the new draft so any + * boundary corruption (e.g. user typed letters touching the right + * edge of `@Airbnb` to form `@Airbnbify`) prunes the entry. + */ +export function reconcileInsertions( + entries: ReadonlyArray, + prev: string, + next: string, +): TrackedInsertion[] { + if (entries.length === 0) return []; + if (prev === next) return entries.slice(); + const { start: editStart, oldEnd, newEnd } = computeEditRange(prev, next); + const delta = newEnd - oldEnd; + const result: TrackedInsertion[] = []; + for (const e of entries) { + const tokenLen = e.token.length + 1; + const entryEnd = e.start + tokenLen; + let nextStart: number; + if (entryEnd <= editStart) { + nextStart = e.start; // entry entirely before edit + } else if (e.start >= oldEnd) { + nextStart = e.start + delta; // entry entirely after edit → shift + } else { + continue; // edit overlaps entry → drop + } + if (isInsertionStillValid(next, nextStart, e.token)) { + const reconciled: TrackedInsertion = { + token: e.token, + start: nextStart, + pluginId: e.pluginId, + }; + if (e.insertionId !== undefined) reconciled.insertionId = e.insertionId; + result.push(reconciled); + } + } + return result; +} + +/** + * Remove tracked `@` insertions from the draft by slicing each + * entry's range. Sorts descending by start so earlier offsets stay + * valid as later ones are excised. Invalidated entries (boundary + * corruption since the last reconcile) are skipped — the safe failure + * mode is under-delete, never over-delete. + * + * Whitespace handling (#2929 round 8): when both the character before + * `@` and the character after the token are whitespace, the slice + * removes one of them in addition to the token to avoid leaving + * doubled whitespace at the seam (e.g. `text @Airbnb more` → `text + * more`, not `text more`). Whitespace ELSEWHERE in the draft is + * never touched — a previous version of this function ran a global + * `[ \t]{2,}` collapse over the entire result, which silently + * rewrote any user-authored multi-space spans (e.g. `keep gap` → + * `keep gap`). Round 8 reviewer flagged that as prompt corruption + * in the changed flow. + */ +export function stripPluginInsertedTokens( + draft: string, + entries: ReadonlyArray, +): string { + if (!draft || entries.length === 0) return draft; + const valid = entries + .filter((e) => isInsertionStillValid(draft, e.start, e.token)) + .sort((a, b) => b.start - a.start); + let next = draft; + for (const e of valid) { + const tokenLen = e.token.length + 1; + const leftIdx = e.start - 1; + const rightIdx = e.start + tokenLen; + const leftIsWs = + leftIdx >= 0 && /[ \t]/.test(next[leftIdx] ?? ''); + const rightIsWs = + rightIdx < next.length && /[ \t]/.test(next[rightIdx] ?? ''); + // Seam-local collapse only: if both sides are whitespace, + // extend the slice by one character so the seam ends up with + // a single space instead of two. Anything outside this range + // — including user-authored multi-space spans elsewhere — is + // left untouched. + const sliceEnd = leftIsWs && rightIsWs ? rightIdx + 1 : rightIdx; + next = next.slice(0, e.start) + next.slice(sliceEnd); + } + return next; +} diff --git a/apps/web/tests/components/ChatComposer.plugin-clear-prunes-draft.test.tsx b/apps/web/tests/components/ChatComposer.plugin-clear-prunes-draft.test.tsx new file mode 100644 index 000000000..20c7dff14 --- /dev/null +++ b/apps/web/tests/components/ChatComposer.plugin-clear-prunes-draft.test.tsx @@ -0,0 +1,1434 @@ +// @vitest-environment jsdom +// +// Regression coverage for nexu-io/open-design#2881 and the #2929 review +// follow-ups. +// +// When the user picks a plugin from the @-mention popover, ChatComposer +// inserts `@${plugin.title}` into the draft and PluginsSection mounts a +// ContextChipStrip + a typed inputs form. Clicking *any* chip's `×` +// invokes PluginsSection's internal `clear()`, which fires `onCleared` +// and unmounts the chip strip and the inputs form. +// +// Three invariants: +// 1. After clear, the inserted `@…` token is gone from the textarea +// (the original #2881 symptom — orphan styled mention). +// 2. Tools-menu / details-modal applies route through `applyById` +// without writing to the draft, so user-authored `@…` text that +// happens to share a label with a chip is left alone after clear +// (#2929 review — preserves user data). +// 3. The strip catches mentions sitting next to punctuation, not just +// whitespace. `(@Airbnb)` and `@Airbnb,` must be cleaned because +// the inline-mention parser still treats them as styled mentions +// (#2929 review — boundary alignment). + +import { StrictMode } from 'react'; +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ChatComposer } from '../../src/components/ChatComposer'; + +const PLUGIN = { + id: 'airbnb', + title: 'Airbnb', + version: '1.0.0', + trust: 'restricted' as const, + sourceKind: 'bundled' as const, + source: 'bundled/airbnb', + capabilitiesGranted: [], + manifest: { + name: 'airbnb', + title: 'Airbnb', + description: 'Airbnb-flavoured layout system', + od: { kind: 'skill' }, + }, + fsPath: '/plugins/airbnb', + installedAt: 0, + updatedAt: 0, +}; + +const APPLY_RESULT = { + ok: true, + query: 'Make a {{topic}} brief.', + contextItems: [ + { kind: 'design-system', id: 'airbnb', label: 'Airbnb' }, + { kind: 'asset', id: 'design-md', name: 'DESIGN.md' }, + ], + inputs: [{ name: 'topic', type: 'string', required: true, label: 'Topic' }], + assets: [], + mcpServers: [], + trust: 'restricted', + capabilitiesGranted: ['prompt:inject'], + capabilitiesRequired: ['prompt:inject'], + appliedPlugin: { + snapshotId: 'snap-airbnb', + pluginId: PLUGIN.id, + pluginVersion: '1.0.0', + manifestSourceDigest: 'a'.repeat(64), + inputs: {}, + resolvedContext: { items: [] }, + capabilitiesGranted: ['prompt:inject'], + capabilitiesRequired: ['prompt:inject'], + assetsStaged: [], + taskKind: 'new-generation', + appliedAt: 0, + connectorsRequired: [], + connectorsResolved: [], + mcpServers: [], + status: 'fresh', + }, + projectMetadata: {}, +}; + +let fetchMock: ReturnType; + +function renderComposer() { + return render( + 'project-1'} + onSend={vi.fn()} + onStop={vi.fn()} + onOpenMcpSettings={vi.fn()} + />, + ); +} + +beforeEach(() => { + fetchMock = vi.fn(async (url: string) => { + if (url === '/api/mcp/servers') { + return new Response(JSON.stringify({ servers: [], templates: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/plugins') { + return new Response(JSON.stringify({ plugins: [PLUGIN] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url.includes('/api/plugins/') && url.endsWith('/apply')) { + return new Response(JSON.stringify(APPLY_RESULT), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/skills') { + return new Response(JSON.stringify({ skills: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + vi.stubGlobal('fetch', fetchMock); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + cleanup(); +}); + +describe('ChatComposer plugin clear prunes draft (#2881)', () => { + it('drops the inserted `@${plugin.title}` token after the user removes the plugin chip', async () => { + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // @-popover plugin pick: composer inserts `@Airbnb ` into the draft + // and mounts the chip strip. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + expect(input.value).not.toContain('@Airbnb'); + }); + + it('does not erase user-authored text when the plugin was applied without writing to the draft (#2929)', async () => { + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // The user types `@Airbnb` themselves (e.g. discussing the brand) + // without going through the @-popover plugin-pick path. + fireEvent.change(input, { + target: { value: 'compare @Airbnb with our spec', selectionStart: 28 }, + }); + + // Drive the apply through the tools-menu Plugins tab — this is the + // production path that calls `pluginsSectionRef.current.applyById` + // *without* writing to the draft, mirroring the details-modal + // "Apply" path. Nothing should land in pluginInsertedTokensRef + // because the composer never inserted a token for this apply. + const trigger = document.querySelector( + '.composer-tools-trigger', + ) as HTMLButtonElement | null; + expect(trigger).toBeTruthy(); + fireEvent.click(trigger!); + await waitFor(() => expect(screen.getByRole('menu')).toBeTruthy()); + + // Pick the plugin from inside the now-open tools popover. The + // plugin row's main button title lives inside a child; we + // match against that to avoid the row's description trailing text. + const popoverPluginButton = Array.from( + document.querySelectorAll('.composer-tools-row-main'), + ).find( + (btn) => btn.querySelector('strong')?.textContent?.trim() === 'Airbnb', + ); + expect(popoverPluginButton).toBeTruthy(); + fireEvent.click(popoverPluginButton!); + + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + + // The draft is untouched — the composer never inserted anything, + // so pluginInsertedTokensRef is empty. + expect(input.value).toBe('compare @Airbnb with our spec'); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // The user's hand-typed `@Airbnb` survives the clear because the + // explicit-tracking set was empty for this apply path (#2929). + expect(input.value).toBe('compare @Airbnb with our spec'); + }); + + it('leaves a parser-extended mention alone on chip clear (#2929 round 5)', async () => { + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // @-popover apply seeds `@Airbnb ` into the draft. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + + // Simulate the user authoring a sentence around the inserted token + // incrementally — first prepending `see (`, then appending `), + // then ship`. Each edit is a small append that the offset diff + // can shift cleanly without overlapping the tracked entry. A + // single-shot whole-draft replacement (e.g. paste-over-select) + // would legitimately drop the entry, which is the safe failure + // mode for instance-aware tracking; that path is exercised by + // the round-2 specs below. + fireEvent.change(input, { + target: { value: 'see (@Airbnb ', selectionStart: 5 }, + }); + fireEvent.change(input, { + target: { value: 'see (@Airbnb), then ship', selectionStart: 24 }, + }); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // Round 5: parser tokenizes `@Airbnb)` greedily (the closing + // paren is in `[^\s@]`), so the parser sees `@Airbnb)` as a + // single mention rather than a `@Airbnb` mention followed by + // `)`. The tracker is now aligned with the parser via + // `isMentionRightBoundary`, so it invalidates this entry and + // leaves the draft alone — stripping `@Airbnb` would have + // mutated user-authored text by tearing apart what the parser + // treats as a single token. The orphan styled mention staying + // visible is the conservative trade-off documented in + // utils/pluginInsertionTracking.ts. + expect(input.value).toBe('see (@Airbnb), then ship'); + }); + + it('drops tracked tokens when the user manually deletes the inserted text, then preserves a later user-authored re-type on clear (#2929 round 2)', async () => { + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // @-popover plugin pick seeds `@Airbnb ` and registers the token in + // pluginInsertedTokensRef. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + // The user manually wipes the inserted token from the draft. The + // chip stays mounted (PluginsSection owns its own state) but the + // ref must drop "Airbnb" because the inserted text is gone. + fireEvent.change(input, { target: { value: '', selectionStart: 0 } }); + + // The user now hand-types a *new* sentence containing `@Airbnb` — + // this is user-authored, not composer-inserted. + fireEvent.change(input, { + target: { + value: 'compare @Airbnb with our spec', + selectionStart: 29, + }, + }); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // The user-authored `@Airbnb` survives because the lifecycle prune + // dropped the tracked token when the original insertion was wiped. + expect(input.value).toBe('compare @Airbnb with our spec'); + }); + + it('drops tracked tokens after send resets the draft, preserving a later user-authored re-type on clear (#2929 round 2)', async () => { + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // @-popover plugin pick seeds `@Airbnb ` and registers the token. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + + // Round out the prompt and submit. ChatComposer.reset() runs, which + // wipes the draft *and* clears pluginInsertedTokensRef so the next + // turn does not inherit a stale tracking entry. + fireEvent.change(input, { + target: { value: '@Airbnb plan a trip', selectionStart: 19 }, + }); + fireEvent.keyDown(input, { key: 'Enter', code: 'Enter' }); + await waitFor(() => expect(input.value).toBe('')); + + // The chip survives the send (PluginsSection owns its state). The + // user starts a fresh prompt that mentions `@Airbnb` themselves. + fireEvent.change(input, { + target: { value: '@Airbnb again, brief me', selectionStart: 23 }, + }); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // Without the reset() ref-clear, this user-authored `@Airbnb` + // would have been deleted (#2929 round 2 regression). + expect(input.value).toBe('@Airbnb again, brief me'); + }); + + it('keeps a user-authored duplicate `@Airbnb` mention intact when the chip clears (#2929 round 3)', async () => { + // The reviewer-flagged sequence: composer inserts `@Airbnb` via + // the popover; the user keeps that insertion AND types a + // separate `@Airbnb` of their own elsewhere in the same draft. + // Range-based tracking pins the composer instance at start=0 + // so the chip clear strips only that range — the user's + // duplicate, untracked, survives. + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + // User extends the draft by appending text that includes a + // hand-typed `@Airbnb`. Each fireEvent is a single-point append + // so the diff sees a clean `[oldLen, oldLen, newLen]` range that + // sits *after* the entry — the entry stays at start=0. + fireEvent.change(input, { + target: { value: '@Airbnb compare ', selectionStart: 16 }, + }); + fireEvent.change(input, { + target: { + value: '@Airbnb compare @Airbnb with our spec', + selectionStart: 37, + }, + }); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // The composer's `@Airbnb` at offset 0 is excised by the + // range strip; the user's hand-typed `@Airbnb` at offset 16 + // is untracked and stays. Leading space is collapsed by + // `[ \t]{2,}` only when adjacent to another space — since + // the strip leaves `' compare @Airbnb with our spec'`, the + // single leading space remains. + expect(input.value).toContain('@Airbnb with our spec'); + expect(input.value).not.toContain('@Airbnb compare'); + expect(input.value).toMatch(/^\s?compare @Airbnb with our spec$/); + }); + + it('reconciles tracked offsets across edits before and after the entry (#2929 round 3)', async () => { + // Append-after-entry then prepend-before-entry: the diff sees + // each edit as a small range that does not overlap the tracked + // insertion, so the offset shifts correctly and chip clear + // still removes only the composer instance. + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + + // Append `here is more text` after the entry — entry stays at 0. + fireEvent.change(input, { + target: { value: '@Airbnb here is more text', selectionStart: 25 }, + }); + // Prepend `prefix ` before the entry — entry shifts to start=7. + fireEvent.change(input, { + target: { + value: 'prefix @Airbnb here is more text', + selectionStart: 7, + }, + }); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // The tracked `@Airbnb` is gone but `prefix` and the appended + // text on either side of it are intact. + expect(input.value).not.toContain('@Airbnb'); + expect(input.value).toContain('prefix'); + expect(input.value).toContain('here is more text'); + }); + + it('reconciles tracked offsets across a tools-menu MCP insert that lands before the entry (#2929 round 4)', async () => { + // The reviewer-flagged sequence: composer inserts `@Airbnb` via + // the popover; the user then opens the tools menu, picks an MCP + // server, and the tools panel prepends `@ ` at cursor 0. + // That setDraft path bypassed handleChange before round 4, so + // the tracked offset stayed at 0 even though the actual `@Airbnb` + // had moved. The post-clear strip then no-op'd via the + // `isInsertionStillValid` guard, leaving an orphaned mention — + // exactly the original #2881 symptom in a supported flow. + const mcpFetchMock = vi.fn(async (url: string) => { + if (url === '/api/mcp/servers') { + // Return one enabled MCP server so the tools menu's MCP tab + // has something to insert. + return new Response( + JSON.stringify({ + servers: [ + { + id: 'github', + label: 'github', + transport: 'stdio', + enabled: true, + }, + ], + templates: [], + }), + { + status: 200, + headers: { 'content-type': 'application/json' }, + }, + ); + } + if (url === '/api/plugins') { + return new Response(JSON.stringify({ plugins: [PLUGIN] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url.includes('/api/plugins/') && url.endsWith('/apply')) { + return new Response(JSON.stringify(APPLY_RESULT), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/skills') { + return new Response(JSON.stringify({ skills: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + vi.stubGlobal('fetch', mcpFetchMock); + + render( + 'project-1'} + onSend={vi.fn()} + onStop={vi.fn()} + onOpenMcpSettings={vi.fn()} + />, + ); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // 1. @-popover pick lands `@Airbnb ` at offset 0. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + // 2. Open the composer tools menu, switch to the MCP tab. + const trigger = document.querySelector( + '.composer-tools-trigger', + ) as HTMLButtonElement | null; + expect(trigger).toBeTruthy(); + fireEvent.click(trigger!); + await waitFor(() => expect(screen.getByRole('menu')).toBeTruthy()); + + const mcpTabButton = Array.from( + document.querySelectorAll('.composer-tools-tab'), + ).find((btn) => btn.textContent?.toLowerCase().includes('mcp')); + expect(mcpTabButton).toBeTruthy(); + fireEvent.click(mcpTabButton!); + + const githubRow = await waitFor(() => { + const row = Array.from( + document.querySelectorAll('.composer-tools-row'), + ).find((btn) => btn.querySelector('strong')?.textContent === 'github'); + expect(row).toBeTruthy(); + return row!; + }); + // 3. Position the textarea cursor at offset 0 immediately before + // triggering onInsert. React re-renders during the menu open + // + tab switch can drop the cursor on jsdom under CI (locally + // they preserve selection but Actions does not), so we set + // it as late as possible to make the read deterministic. + input.focus(); + input.setSelectionRange(0, 0); + + // 4. Click the github server. The MCP panel's onInsert prepends + // `@github ` at cursor 0, going through `updateDraft` which + // must reconcile the tracked Airbnb offset from 0 → 8. + fireEvent.click(githubRow); + + await waitFor(() => expect(input.value).toBe('@github @Airbnb ')); + + // 4. Click the chip × — strip should excise the (now reconciled) + // Airbnb at offset 8, leaving the user's MCP `@github` alone. + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // Without the round-4 updateDraft chokepoint, this would still + // contain `@Airbnb` (orphan mention) because the tracked offset + // would point at `@github`'s start, fail validity, and the strip + // would no-op. + expect(input.value).not.toContain('@Airbnb'); + expect(input.value).toContain('@github'); + }); + + it('tracks two consecutive composer-inserted `@Airbnb` instances independently (#2929 round 3)', async () => { + // User picks the same plugin from the @-popover twice in a row. + // Each insertion gets its own entry in pluginInsertedTokensRef, + // and the chip clear strips both ranges. + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // First pick. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + // User triggers the popover again at the end of the draft. The + // chip already shows "Airbnb" too, so we have to scope the click + // to the mention-popover container; otherwise getByText resolves + // to the chip label and the click does nothing. + fireEvent.change(input, { + target: { value: '@Airbnb @air', selectionStart: 12 }, + }); + await waitFor(() => { + const popover = screen.getByTestId('mention-popover'); + const airbnbButton = popover.querySelector( + '.mention-item--plugin', + ); + expect(airbnbButton?.textContent).toContain('Airbnb'); + }); + const popoverAirbnbButton = screen + .getByTestId('mention-popover') + .querySelector('.mention-item--plugin')!; + fireEvent.click(popoverAirbnbButton); + // Second pick lands `@Airbnb ` after the first; the input now + // holds two composer-inserted instances. + await waitFor(() => + expect(input.value).toBe('@Airbnb @Airbnb '), + ); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // Both tracked ranges removed; the residual whitespace gets + // collapsed to a single space (or empty) by the strip. + expect(input.value).not.toContain('@Airbnb'); + expect(input.value.trim()).toBe(''); + }); + + it('does not tear `@Airbnb/foo` apart on chip clear (#2929 round 5)', async () => { + // The parser's `@[^\s@]+` greedy regex tokenizes `@Airbnb/foo` + // as a single mention. If the tracker considered the `@Airbnb` + // prefix valid for stripping, the post-clear strip would + // remove `@Airbnb` and leave `/foo` dangling — that is + // user-authored text mutation, not orphan removal. The + // round-5 right-boundary rule (aligned with the parser via + // `isMentionRightBoundary`) invalidates this entry on clear, + // leaving the draft alone. + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + + // User extends the inserted token by deleting the trailing + // space and typing `/foo`. After the edit, the parser sees + // `@Airbnb/foo` as one mention. + fireEvent.change(input, { + target: { value: '@Airbnb', selectionStart: 7 }, + }); + fireEvent.change(input, { + target: { value: '@Airbnb/foo', selectionStart: 11 }, + }); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // Draft stays intact — the strip was a no-op because the entry + // was invalidated by the round-5 right-boundary rule. + expect(input.value).toBe('@Airbnb/foo'); + }); + + it('preserves the previous-plugin `@` when a new plugin is applied via tools-menu and then cleared (#2929 round 6)', async () => { + // Reviewer-flagged replace-plugin sequence: + // 1. @-pick plugin A → draft=`@A `, tracked entry for A + // 2. tools-menu apply plugin B → chip strip switches A → B, + // `applyById(B)` does NOT touch the draft, so `@A` is now + // visually orphaned but still in the textarea. + // 3. clear B's chip → onCleared must NOT strip the user's + // (now-orphaned) `@A`, since it was never B's. Pre-round-6 + // tracking was global-per-composer and `onCleared` removed + // every entry regardless of which plugin owned it, so + // step 3 deleted `@A`. The fix scopes entries by `pluginId` + // and drops non-active entries on `setActivePlugin`. + const PLUGIN_B = { + ...PLUGIN, + id: 'second-plugin', + title: 'SecondPlugin', + manifest: { ...PLUGIN.manifest, name: 'second-plugin', title: 'SecondPlugin' }, + }; + const replaceFetchMock = vi.fn(async (url: string) => { + if (url === '/api/plugins') { + return new Response(JSON.stringify({ plugins: [PLUGIN, PLUGIN_B] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/mcp/servers') { + return new Response(JSON.stringify({ servers: [], templates: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url.includes('/api/plugins/') && url.endsWith('/apply')) { + return new Response(JSON.stringify(APPLY_RESULT), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/skills') { + return new Response(JSON.stringify({ skills: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + vi.stubGlobal('fetch', replaceFetchMock); + + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // Step 1: @-popover pick plugin A (Airbnb). + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + // Step 2: tools-menu plugins-tab apply plugin B (SecondPlugin). + // This goes through `pluginsSectionRef.current.applyById` without + // writing to the draft, so the textarea retains `@Airbnb `. + const trigger = document.querySelector( + '.composer-tools-trigger', + ) as HTMLButtonElement | null; + expect(trigger).toBeTruthy(); + fireEvent.click(trigger!); + await waitFor(() => expect(screen.getByRole('menu')).toBeTruthy()); + + const popoverPluginButton = Array.from( + document.querySelectorAll('.composer-tools-row-main'), + ).find( + (btn) => btn.querySelector('strong')?.textContent?.trim() === 'SecondPlugin', + ); + expect(popoverPluginButton).toBeTruthy(); + fireEvent.click(popoverPluginButton!); + + await waitFor(() => + expect(screen.getByLabelText(/Remove Plugin SecondPlugin/i)).toBeTruthy(), + ); + // Draft is unchanged — applyById did not write — but the chip + // strip now reflects SecondPlugin instead of Airbnb. + expect(input.value).toBe('@Airbnb '); + + // Step 3: clear SecondPlugin's chip. `@Airbnb` belongs to a + // different (no-longer-active) plugin and must survive. + fireEvent.click(screen.getByLabelText(/Remove Plugin SecondPlugin/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + expect(input.value).toBe('@Airbnb '); + }); + + // Round 7 — transactional applyById. PluginsSectionHandle.applyById + // returns null on `/apply` failure; before round 7 the tracker was + // committed eagerly (active id changed, entries pushed/dropped) + // before that result was known. A failure left the chip strip + // showing the previously-active plugin while the tracker reflected + // a half-applied state, so subsequent clears either deleted + // user-visible just-inserted text or no-op'd over an orphan that + // should have been cleaned. The fix snapshots tracker (and draft, + // for the picker path) before mutation and rolls back when + // `applyById` returns null. + function makeFailingApplyFetch() { + return vi.fn(async (url: string) => { + if (url === '/api/mcp/servers') { + return new Response(JSON.stringify({ servers: [], templates: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/plugins') { + return new Response(JSON.stringify({ plugins: [PLUGIN] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url.includes('/api/plugins/') && url.endsWith('/apply')) { + // Simulate `/apply` 5xx — `applyPlugin()` returns null, + // `PluginsSectionHandle.applyById` propagates that null. + return new Response('apply failed', { status: 500 }); + } + if (url === '/api/skills') { + return new Response(JSON.stringify({ skills: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + } + + it('rolls back the @-picker insertion when applyById fails (#2929 round 7)', async () => { + // The picker path commits the draft (replaceMentionWithText) then + // pushes a tracked entry then awaits applyById. If apply fails, + // the draft must be restored — the chip never mounted, so a + // later "clear the previously-active chip" must not strip the + // ghost `@` that the user can see in the textarea but + // can't tie to any chip. + vi.stubGlobal('fetch', makeFailingApplyFetch()); + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + + // Apply 500'd, so the chip strip never mounts. Wait for the + // network rejection path to settle, then assert the textarea + // has been rolled back to the user's pre-pick `@air` query. + await waitFor(() => { + expect(screen.queryByTestId('context-chip-strip')).toBeNull(); + expect(input.value).toBe('@air'); + }); + }); + + it('preserves user keystrokes that arrive during a pending applyById that fails (#2929 round 8)', async () => { + // Reviewer-flagged sequence: textarea stays interactive during + // the `/apply` await, so the user can type more characters + // between the popover pick and the apply response. If apply + // 500s, the round-7 unconditional `setDraft(prevDraftValue)` + // rollback would clobber those newer keystrokes — real prompt- + // data-loss in the changed flow. The round-8 fix gates the + // draft-restore on the textarea still being in the post- + // optimistic-write state; if it has moved past, only the + // tracker is restored and the user's edits stay. + // + // The test uses a deferred Response so the `/apply` fetch is + // pending after the popover click; we then fire a `change` + // event simulating the user typing during the await, and only + // then resolve the deferred Response with a 500. + let resolveApply: ((value: Response) => void) | null = null; + const deferredApply = new Promise((resolve) => { + resolveApply = resolve; + }); + const fetchMock = vi.fn(async (url: string) => { + if (url === '/api/mcp/servers') { + return new Response(JSON.stringify({ servers: [], templates: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/plugins') { + return new Response(JSON.stringify({ plugins: [PLUGIN] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url.includes('/api/plugins/') && url.endsWith('/apply')) { + return deferredApply; + } + if (url === '/api/skills') { + return new Response(JSON.stringify({ skills: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + vi.stubGlobal('fetch', fetchMock); + + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + + // The optimistic write happened; `/apply` is in flight. Draft + // is currently `@Airbnb `. Simulate user typing more text + // before the apply response arrives. + expect(input.value).toBe('@Airbnb '); + fireEvent.change(input, { + target: { value: '@Airbnb extra typing', selectionStart: 20 }, + }); + + // Now resolve the in-flight apply with a 500. + expect(resolveApply).not.toBeNull(); + resolveApply!(new Response('apply failed', { status: 500 })); + + // Wait for the rollback path to run. The tracker should be + // restored (so a future chip clear is a no-op for this entry), + // but the draft must NOT be rewritten back to `@air` — that + // would clobber the user's `extra typing`. + await waitFor(() => { + expect(screen.queryByTestId('context-chip-strip')).toBeNull(); + }); + expect(input.value).toBe('@Airbnb extra typing'); + }); + + it('rolls back the active-plugin id when tools-menu applyById fails (#2929 round 7)', async () => { + // The tools-menu / details-modal paths do not write to the + // draft, but they DO change the active-plugin id (filtering + // out entries from any previously-active plugin). If apply + // fails, the tracker must restore the original active id so + // a later clear of the (still-mounted) original chip strips + // its `@` correctly. + // + // Sequence: @-popover pick A succeeds (apply mock returns + // success the first time), tracker = [Airbnb at 0]; tools-menu + // re-pick A but apply now 500s (we install a failing fetch + // mock right before the second click); tracker should NOT have + // dropped Airbnb's entry. Clear A's chip — `@Airbnb` strips. + let applyCallCount = 0; + const fetchMock = vi.fn(async (url: string) => { + if (url === '/api/mcp/servers') { + return new Response(JSON.stringify({ servers: [], templates: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/plugins') { + return new Response(JSON.stringify({ plugins: [PLUGIN] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url.includes('/api/plugins/') && url.endsWith('/apply')) { + applyCallCount++; + if (applyCallCount === 1) { + return new Response(JSON.stringify(APPLY_RESULT), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + // Second apply (tools-menu re-pick) fails. + return new Response('apply failed', { status: 500 }); + } + if (url === '/api/skills') { + return new Response(JSON.stringify({ skills: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + vi.stubGlobal('fetch', fetchMock); + + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // Step 1: @-popover pick (apply succeeds). + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + // Step 2: tools-menu re-pick (apply will fail). The tracker + // pre-round-7 dropped Airbnb's entry on `setActivePlugin` and + // never restored it; round 7's snapshot+rollback restores it. + const trigger = document.querySelector( + '.composer-tools-trigger', + ) as HTMLButtonElement | null; + expect(trigger).toBeTruthy(); + fireEvent.click(trigger!); + await waitFor(() => expect(screen.getByRole('menu')).toBeTruthy()); + + const popoverPluginButton = Array.from( + document.querySelectorAll('.composer-tools-row-main'), + ).find( + (btn) => btn.querySelector('strong')?.textContent?.trim() === 'Airbnb', + ); + expect(popoverPluginButton).toBeTruthy(); + fireEvent.click(popoverPluginButton!); + + // Wait for the failed apply to settle. The chip strip should + // still be the original (no remount), and clicking × on it must + // strip `@Airbnb` from the draft — which only works if + // Airbnb's entry survived the rollback. + await waitFor(() => expect(applyCallCount).toBe(2)); + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + expect(input.value).not.toContain('@Airbnb'); + }); + + it('strips the unmounting plugin\'s `@` (not the in-flight target\'s) when the user clears the original chip during a pending @-popover replace (#2929 round 9)', async () => { + // Round 9 (codex) — pending-apply race in `insertPluginMention`. + // + // Pre-fix, `insertPluginMention` ran: + // pluginInsertedTokensRef.current.push(B); // sync + // setActivePlugin(B.id); // sync — drops A's entries + // await pluginsSectionRef.current.applyById(B); + // + // So during the await, the chip strip still showed A but the + // tracker reflected B. If the user clicked A's `×` in this + // window, `onCleared` saw only B's entry, stripped the freshly + // optimistic `@B` from the draft (deleting user-visible text), + // and left A's `@A` orphaned (the original #2881 symptom). + // + // Fix: defer `setActivePlugin(B.id)` until the apply resolves + // successfully, AND have `onCleared` filter the tracker by + // `pluginsSectionRef.current?.getActiveRecord()?.id` so the + // strip is scoped to the plugin whose chip is actually + // unmounting. PluginsSection's `activeRecord` only flips on + // successful apply, so during the pending window + // `getActiveRecord()` reports A — exactly the chip being + // removed. + let resolveApply: ((value: Response) => void) | null = null; + let applyCallCount = 0; + const PLUGIN_B = { + ...PLUGIN, + id: 'second-plugin', + title: 'SecondPlugin', + manifest: { + ...PLUGIN.manifest, + name: 'second-plugin', + title: 'SecondPlugin', + }, + }; + const pendingFetchMock = vi.fn(async (url: string) => { + if (url === '/api/mcp/servers') { + return new Response(JSON.stringify({ servers: [], templates: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/plugins') { + return new Response( + JSON.stringify({ plugins: [PLUGIN, PLUGIN_B] }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ); + } + if (url.includes('/api/plugins/') && url.endsWith('/apply')) { + applyCallCount++; + if (applyCallCount === 1) { + // First apply (Airbnb) succeeds immediately so the chip + // mounts and the user has something to click. + return new Response(JSON.stringify(APPLY_RESULT), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + // Second apply (SecondPlugin) hangs until the test + // resolves it, simulating a slow `/apply` endpoint. + return new Promise((resolve) => { + resolveApply = resolve; + }); + } + if (url === '/api/skills') { + return new Response(JSON.stringify({ skills: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + vi.stubGlobal('fetch', pendingFetchMock); + + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // Step 1: @-popover pick Airbnb — apply succeeds, chip mounts, + // tracker = [Airbnb at offset 0]. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByLabelText(/Remove Plugin Airbnb/i)).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + // Step 2: type `@sec` after the existing `@Airbnb ` so the + // mention popover anchors at the new position, then pick + // SecondPlugin. The optimistic write lands `@SecondPlugin ` + // immediately; applyById is now waiting on `resolveApply`. + fireEvent.change(input, { + target: { value: '@Airbnb @sec', selectionStart: 12 }, + }); + await waitFor(() => { + const popover = screen.getByTestId('mention-popover'); + expect(popover.textContent).toContain('SecondPlugin'); + }); + const popoverSecondButton = screen + .getByTestId('mention-popover') + .querySelector('.mention-item--plugin')!; + fireEvent.click(popoverSecondButton); + + await waitFor(() => expect(input.value).toBe('@Airbnb @SecondPlugin ')); + expect(applyCallCount).toBe(2); + expect(resolveApply).not.toBeNull(); + // Chip strip still shows Airbnb because PluginsSection only + // flips `activeRecord` on successful apply. + expect(screen.getByLabelText(/Remove Plugin Airbnb/i)).toBeTruthy(); + + // Step 3: in this pending window, user clicks Airbnb's `×`. + // Pre-fix: tracker had only SecondPlugin's entry (Airbnb's + // dropped by eager setActivePlugin), so onCleared stripped + // `@SecondPlugin` and left `@Airbnb` orphaned. Post-fix: + // setActivePlugin is deferred, tracker still has both + // entries, and onCleared filters by getActiveRecord() = + // Airbnb so only `@Airbnb` is stripped. + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => { + expect(input.value).not.toContain('@Airbnb'); + expect(input.value).toContain('@SecondPlugin'); + }); + + // Step 4: resolve the in-flight apply. PluginsSection now + // mounts SecondPlugin's chip; setActivePlugin(SecondPlugin) + // commits, dropping any stale Airbnb entries (none — already + // stripped by step 3). The remaining `@SecondPlugin` token + // is properly tied to the new chip. + resolveApply!( + new Response(JSON.stringify(APPLY_RESULT), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + await waitFor(() => + expect(screen.getByLabelText(/Remove Plugin SecondPlugin/i)).toBeTruthy(), + ); + + // Step 5: clearing SecondPlugin's chip strips `@SecondPlugin`, + // verifying that the deferred setActivePlugin fired on + // success and the entry survived the pending-window clear. + fireEvent.click(screen.getByLabelText(/Remove Plugin SecondPlugin/i)); + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + expect(input.value).not.toContain('@SecondPlugin'); + }); + + it('strips the @-popover plugin\'s `@` when the user clears the original chip during a pending tools-menu replace (#2929 round 9)', async () => { + // Round 9 (codex) — pending-apply race in the tools-menu + // `onApply` path. Tools-menu apply doesn't write to the draft; + // pre-fix it called `setActivePlugin(target)` synchronously, + // which dropped the previously-active plugin's entries from + // the tracker before `applyById` resolved. If the user clicked + // the original chip's `×` in that pending window, `onCleared` + // saw an empty tracker and no-op'd — leaving `@` + // orphaned in the draft (#2881 symptom recurring). + // + // Fix: defer setActivePlugin until applyById resolves + // successfully, plus the onCleared filter that scopes the + // strip to `getActiveRecord()?.id`. + let resolveApply: ((value: Response) => void) | null = null; + let applyCallCount = 0; + const PLUGIN_B = { + ...PLUGIN, + id: 'second-plugin', + title: 'SecondPlugin', + manifest: { + ...PLUGIN.manifest, + name: 'second-plugin', + title: 'SecondPlugin', + }, + }; + const toolsMenuPendingFetchMock = vi.fn(async (url: string) => { + if (url === '/api/mcp/servers') { + return new Response(JSON.stringify({ servers: [], templates: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/plugins') { + return new Response( + JSON.stringify({ plugins: [PLUGIN, PLUGIN_B] }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ); + } + if (url.includes('/api/plugins/') && url.endsWith('/apply')) { + applyCallCount++; + if (applyCallCount === 1) { + return new Response(JSON.stringify(APPLY_RESULT), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + return new Promise((resolve) => { + resolveApply = resolve; + }); + } + if (url === '/api/skills') { + return new Response(JSON.stringify({ skills: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + vi.stubGlobal('fetch', toolsMenuPendingFetchMock); + + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // Step 1: @-popover pick Airbnb — apply succeeds, chip mounts, + // tracker = [Airbnb at offset 0], draft = `@Airbnb `. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByLabelText(/Remove Plugin Airbnb/i)).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + // Step 2: open tools-menu Plugins tab and pick SecondPlugin. + // Tools-menu apply doesn't write to the draft, but pre-fix it + // dropped Airbnb's tracker entry synchronously. applyById is + // now waiting on `resolveApply` — chip strip still shows + // Airbnb. + const trigger = document.querySelector( + '.composer-tools-trigger', + ) as HTMLButtonElement | null; + expect(trigger).toBeTruthy(); + fireEvent.click(trigger!); + await waitFor(() => expect(screen.getByRole('menu')).toBeTruthy()); + + const popoverSecondPluginButton = Array.from( + document.querySelectorAll('.composer-tools-row-main'), + ).find( + (btn) => btn.querySelector('strong')?.textContent?.trim() === 'SecondPlugin', + ); + expect(popoverSecondPluginButton).toBeTruthy(); + fireEvent.click(popoverSecondPluginButton!); + + await waitFor(() => expect(applyCallCount).toBe(2)); + expect(resolveApply).not.toBeNull(); + // Chip strip still mounted for Airbnb (apply hasn't resolved). + expect(screen.getByLabelText(/Remove Plugin Airbnb/i)).toBeTruthy(); + // Draft unchanged because tools-menu doesn't write. + expect(input.value).toBe('@Airbnb '); + + // Step 3: user clicks Airbnb's `×` while tools-menu's + // applyById(SecondPlugin) is still in flight. Pre-fix: + // setActivePlugin(SecondPlugin) at the start of `onApply` + // already dropped Airbnb's entry, so onCleared had nothing to + // strip and `@Airbnb` stayed orphaned. Post-fix: + // setActivePlugin is deferred, tracker still has Airbnb's + // entry, and onCleared filters by getActiveRecord() = Airbnb + // so `@Airbnb` is stripped. + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => expect(input.value).not.toContain('@Airbnb')); + + // Step 4: resolve the in-flight tools-menu apply. Chip strip + // remounts for SecondPlugin and setActivePlugin(SecondPlugin) + // commits. + resolveApply!( + new Response(JSON.stringify(APPLY_RESULT), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + await waitFor(() => + expect(screen.getByLabelText(/Remove Plugin SecondPlugin/i)).toBeTruthy(), + ); + // Draft remains stripped — applyById's success doesn't + // re-introduce `@Airbnb`. + expect(input.value).not.toContain('@Airbnb'); + }); + + it('strips the failed target insertion (and leaves intervening-clear state intact) when applyById fails after the user clears the original chip mid-flight (#2929 round 10)', async () => { + // Round 10 (codex) — failure-path race in `insertPluginMention`. + // + // Pre-fix, when `applyById` returned null, the rollback ran: + // pluginInsertedTokensRef.current = prevEntries; + // activePluginIdRef.current = prevActiveId; + // + // restoring the tracker as it was before the optimistic push. + // That assumed nothing else had touched the tracker during the + // await — but the round-9 `onCleared` filter explicitly mutates + // the tracker to strip the unmounting plugin's entries. If the + // user clicked the still-mounted original chip's × during the + // pending replace AND then `applyById` resolved with a 500, + // wholesale-restoring `prevEntries` would (a) resurrect the + // already-stripped entries with stale offsets and (b) leave the + // optimistic `@` orphaned in the draft with no chip + // mounted — the original #2881 symptom recurring inside the + // failure window. + // + // Fix: detect "intervening clear" via + // `activePluginIdRef.current === null && prevActiveId !== null` + // (onCleared nulls the active id, the deferred setActivePlugin + // never ran in the failure branch), and on detection remove + // ONLY our optimistic entry (located by `insertionId` so a + // duplicate-pick of the same plugin during the await is still + // disambiguated) and strip ONLY its `@` from the draft, + // leaving the rest of `onCleared`'s work intact. + let resolveApply: ((value: Response) => void) | null = null; + let applyCallCount = 0; + const PLUGIN_B = { + ...PLUGIN, + id: 'second-plugin', + title: 'SecondPlugin', + manifest: { + ...PLUGIN.manifest, + name: 'second-plugin', + title: 'SecondPlugin', + }, + }; + const r10FetchMock = vi.fn(async (url: string) => { + if (url === '/api/mcp/servers') { + return new Response(JSON.stringify({ servers: [], templates: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (url === '/api/plugins') { + return new Response( + JSON.stringify({ plugins: [PLUGIN, PLUGIN_B] }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ); + } + if (url.includes('/api/plugins/') && url.endsWith('/apply')) { + applyCallCount++; + if (applyCallCount === 1) { + // First apply (Airbnb) succeeds so the chip mounts. + return new Response(JSON.stringify(APPLY_RESULT), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + // Second apply (SecondPlugin) hangs until the test resolves + // it — we will resolve with a 500 below. + return new Promise((resolve) => { + resolveApply = resolve; + }); + } + if (url === '/api/skills') { + return new Response(JSON.stringify({ skills: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + vi.stubGlobal('fetch', r10FetchMock); + + renderComposer(); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // Step 1: @-popover pick Airbnb — apply succeeds, chip mounts. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByLabelText(/Remove Plugin Airbnb/i)).toBeTruthy(), + ); + expect(input.value).toBe('@Airbnb '); + + // Step 2: pick SecondPlugin from the @-popover — applyById is + // deferred, draft now holds `@Airbnb @SecondPlugin `. + fireEvent.change(input, { + target: { value: '@Airbnb @sec', selectionStart: 12 }, + }); + await waitFor(() => { + const popover = screen.getByTestId('mention-popover'); + expect(popover.textContent).toContain('SecondPlugin'); + }); + const popoverSecondButton = screen + .getByTestId('mention-popover') + .querySelector('.mention-item--plugin')!; + fireEvent.click(popoverSecondButton); + + await waitFor(() => expect(input.value).toBe('@Airbnb @SecondPlugin ')); + expect(applyCallCount).toBe(2); + expect(resolveApply).not.toBeNull(); + expect(screen.getByLabelText(/Remove Plugin Airbnb/i)).toBeTruthy(); + + // Step 3: clear Airbnb's chip while applyById(SecondPlugin) is + // in flight. Round 9's getActiveRecord-scoped onCleared strips + // `@Airbnb` and leaves `@SecondPlugin` intact; activePluginIdRef + // is nulled. + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + await waitFor(() => { + expect(input.value).not.toContain('@Airbnb'); + expect(input.value).toContain('@SecondPlugin'); + }); + + // Step 4: resolve the in-flight apply with a 500. Pre-fix: + // wholesale `prevEntries` restore resurrects Airbnb's stale + // entry, AND `@SecondPlugin` stays orphaned in the draft + // (no chip mounts because apply failed). Post-fix: + // intervenedClear branch removes ONLY SecondPlugin's entry and + // strips ONLY `@SecondPlugin` from the draft. + resolveApply!(new Response('apply failed', { status: 500 })); + + await waitFor(() => expect(input.value).not.toContain('@SecondPlugin')); + // Both `@` insertions are gone, no chip is mounted, and + // the tracker holds no stale entries (verified indirectly: a + // hand-typed `@Airbnb` later would survive a never-mounted-chip + // clear, but we cannot hand-type one here without remounting + // the chip — the empty value below is the strongest assertion + // we can make on the public surface). + expect(screen.queryByTestId('context-chip-strip')).toBeNull(); + expect(input.value.trim()).toBe(''); + }); + + it('keeps tracked offsets correct under React StrictMode double-invoke (#2929 round 5)', async () => { + // React's StrictMode (enabled in apps/web/next.config.ts via + // `reactStrictMode: true`) double-invokes setState updaters in + // development to surface impurity. An earlier version of + // `updateDraft` mutated `pluginInsertedTokensRef.current` + // inside the `setDraft` updater, which meant the ref was + // shifted twice for every tracked-mention-moving edit (e.g. + // the round-4 tools-menu MCP prepend) and the second shift + // pushed the offset out of bounds — `stripPluginInsertedTokens` + // then no-op'd via `isInsertionStillValid`, reproducing the + // original #2881 orphan-mention symptom on every keystroke in + // dev. The fix moves reconcile out of the updater and uses a + // synchronous `draftRef` mirror as `prev`, so double-invoke + // is harmless. Rendering inside `` here would + // catch a regression at the integration layer. + render( + + 'project-1'} + onSend={vi.fn()} + onStop={vi.fn()} + onOpenMcpSettings={vi.fn()} + /> + , + ); + const input = screen.getByTestId('chat-composer-input') as HTMLTextAreaElement; + + // @-popover pick lands `@Airbnb ` at offset 0. + fireEvent.change(input, { target: { value: '@air', selectionStart: 4 } }); + await waitFor(() => expect(screen.getByText('Airbnb')).toBeTruthy()); + fireEvent.click(screen.getByText('Airbnb')); + await waitFor(() => + expect(screen.getByTestId('context-chip-strip')).toBeTruthy(), + ); + + // User keeps the insertion and types a separate user-authored + // `@Airbnb` later — the round-3 reviewer scenario. With the + // round-5 fix, the tracked entry stays at offset 0 even after + // multiple StrictMode-doubled updateDraft passes; without the + // fix, it would shift to 8, then 16, drop, and the strip would + // remove nothing (or in the worst case both `@Airbnb`s). + fireEvent.change(input, { + target: { value: '@Airbnb compare ', selectionStart: 16 }, + }); + fireEvent.change(input, { + target: { + value: '@Airbnb compare @Airbnb with our spec', + selectionStart: 37, + }, + }); + + fireEvent.click(screen.getByLabelText(/Remove Plugin Airbnb/i)); + + await waitFor(() => + expect(screen.queryByTestId('context-chip-strip')).toBeNull(), + ); + // Composer's `@Airbnb` at offset 0 stripped; user's at offset + // 16 preserved. Same assertion as the round-3 spec, but now + // under StrictMode. + expect(input.value).toContain('@Airbnb with our spec'); + expect(input.value).not.toContain('@Airbnb compare'); + }); +}); diff --git a/apps/web/tests/utils/pluginInsertionTracking.test.ts b/apps/web/tests/utils/pluginInsertionTracking.test.ts new file mode 100644 index 000000000..72ba06273 --- /dev/null +++ b/apps/web/tests/utils/pluginInsertionTracking.test.ts @@ -0,0 +1,310 @@ +import { describe, expect, it } from 'vitest'; + +import { + computeEditRange, + isInsertionStillValid, + reconcileInsertions, + stripPluginInsertedTokens, + type TrackedInsertion, +} from '../../src/utils/pluginInsertionTracking'; + +// Pure-function coverage for the diff/reconcile/strip primitives that +// back ChatComposer's instance-aware plugin mention tracking +// (#2929 round 3). The integration spec +// (`ChatComposer.plugin-clear-prunes-draft.test.tsx`) exercises the +// end-to-end React path; this file pins the edge cases the integration +// flow is unlikely to hit, so a regression in the math surfaces here +// before it can corrupt user drafts. + +describe('computeEditRange', () => { + it('returns an empty range when the strings are equal', () => { + expect(computeEditRange('abc', 'abc')).toEqual({ start: 0, oldEnd: 0, newEnd: 0 }); + }); + + it('detects a pure prefix append', () => { + // `prev` is the suffix of `next`; the diff sits at the very start. + expect(computeEditRange('world', 'hello world')).toEqual({ + start: 0, + oldEnd: 0, + newEnd: 6, + }); + }); + + it('detects a pure suffix append', () => { + expect(computeEditRange('hello', 'hello world')).toEqual({ + start: 5, + oldEnd: 5, + newEnd: 11, + }); + }); + + it('detects a middle replacement', () => { + expect(computeEditRange('abc XYZ def', 'abc 12345 def')).toEqual({ + start: 4, + oldEnd: 7, + newEnd: 9, + }); + }); + + it('detects a full deletion to empty', () => { + expect(computeEditRange('hello', '')).toEqual({ start: 0, oldEnd: 5, newEnd: 0 }); + }); + + it('does not let prefix and suffix overlap when one string is a substring of the other', () => { + // Both strings share the leading `aa`. If suffix matching greedily + // walked past `start`, the range could go negative. + const r = computeEditRange('aa', 'aaa'); + expect(r.start).toBeLessThanOrEqual(r.oldEnd); + expect(r.start).toBeLessThanOrEqual(r.newEnd); + }); + + it('treats prepended text that shares a leading char with prev as a clean prepend (#2929 round 4)', () => { + // Inserting `@github ` before `@Airbnb ` gives `@github @Airbnb `. + // A naive LCP-first algorithm matches the leading `@`, then walks + // LCS backwards through `Airbnb `, and reports the edit as + // `editStart=1, oldEnd=1, newEnd=9`. That window cuts through a + // tracked entry at offset 0 even though `@Airbnb` was not + // structurally touched. LCS-first is required so the entire + // `@Airbnb ` suffix is claimed by the right side and the diff + // collapses to a clean prepend of `[0, 0, 8]`. + const r = computeEditRange('@Airbnb ', '@github @Airbnb '); + expect(r).toEqual({ start: 0, oldEnd: 0, newEnd: 8 }); + }); +}); + +describe('isInsertionStillValid', () => { + it('accepts a token at the start of the draft', () => { + expect(isInsertionStillValid('@Airbnb plan', 0, 'Airbnb')).toBe(true); + }); + + it('accepts a token after a whitespace boundary', () => { + expect(isInsertionStillValid('see @Airbnb', 4, 'Airbnb')).toBe(true); + }); + + it('rejects when the surrounding letter forms a longer mention', () => { + // `@Airbnbx` would render as a single mention so the tracked range + // is no longer the intended target. + expect(isInsertionStillValid('@Airbnbx', 0, 'Airbnb')).toBe(false); + }); + + it('rejects when the left boundary is a non-mention character', () => { + // `x@Airbnb` is not a valid mention per inlineMentions boundary + // rules — the `x` immediately to the left is a word char. + expect(isInsertionStillValid('x@Airbnb', 1, 'Airbnb')).toBe(false); + }); + + it('rejects when the offset no longer points at the token', () => { + expect(isInsertionStillValid('compare @Airbnb', 0, 'Airbnb')).toBe(false); + }); + + it('rejects negative or out-of-range offsets', () => { + expect(isInsertionStillValid('@Airbnb', -1, 'Airbnb')).toBe(false); + expect(isInsertionStillValid('@Airbnb', 100, 'Airbnb')).toBe(false); + }); + + // Parser-alignment cases (#2929 round 5): the inline-mention parser + // tokenizes `@` greedily through `[^\s@]`, then prefers the + // longer match at the same start offset. A tracked entry must + // therefore be invalidated whenever the right-boundary character + // would extend the parser's mention past the tracked range — that + // is, anything other than EOS, whitespace, or another `@`. Without + // these rejections the post-clear strip would carve `@Airbnb` out + // of `@Airbnb/foo`, leaving `/foo` dangling as user-authored text + // mutation. + it('rejects when followed by `/` (parser would tokenize a longer mention)', () => { + expect(isInsertionStillValid('@Airbnb/foo', 0, 'Airbnb')).toBe(false); + }); + + it('rejects when followed by `.`', () => { + expect(isInsertionStillValid('@Airbnb.test', 0, 'Airbnb')).toBe(false); + }); + + it('rejects when followed by `,`', () => { + expect(isInsertionStillValid('@Airbnb,', 0, 'Airbnb')).toBe(false); + }); + + it('rejects when followed by `)` (parser would extend through the paren)', () => { + expect(isInsertionStillValid('see (@Airbnb), then ship', 5, 'Airbnb')).toBe(false); + }); + + it('accepts when followed by another `@` (next mention starts there)', () => { + expect(isInsertionStillValid('@Airbnb@other', 0, 'Airbnb')).toBe(true); + }); + + it('accepts at end-of-string', () => { + expect(isInsertionStillValid('@Airbnb', 0, 'Airbnb')).toBe(true); + }); + + it('accepts when followed by whitespace', () => { + expect(isInsertionStillValid('@Airbnb plan', 0, 'Airbnb')).toBe(true); + }); +}); + +describe('reconcileInsertions', () => { + const entry: TrackedInsertion = { token: 'Airbnb', start: 0, pluginId: 'airbnb' }; + + it('returns a fresh copy when nothing changed', () => { + const out = reconcileInsertions([entry], '@Airbnb ', '@Airbnb '); + expect(out).toEqual([entry]); + expect(out).not.toBe([entry]); // new array + }); + + it('keeps an entry whose tail sits before the edit', () => { + // `@Airbnb` at [0,7], edit happens at index 8 (typing after the trailing space) + const next = reconcileInsertions( + [entry], + '@Airbnb ', + '@Airbnb compare', + ); + expect(next).toEqual([entry]); + }); + + it('shifts an entry whose head sits after the edit', () => { + // Insert `prefix ` (7 chars) at the beginning. Entry start moves 0 → 7. + const next = reconcileInsertions( + [entry], + '@Airbnb ', + 'prefix @Airbnb ', + ); + expect(next).toEqual([{ token: 'Airbnb', start: 7, pluginId: 'airbnb' }]); + }); + + it('drops an entry the edit overlaps', () => { + // User selects through the entry and replaces it with other text. + const next = reconcileInsertions( + [entry], + '@Airbnb plan', + '@Air-other plan', + ); + expect(next).toEqual([]); + }); + + it('drops an entry whose right boundary is corrupted (letters touching)', () => { + // Typing `ify` immediately after `@Airbnb` makes it `@Airbnbify` + // which is no longer a valid mention. + const next = reconcileInsertions( + [entry], + '@Airbnb', + '@Airbnbify', + ); + expect(next).toEqual([]); + }); + + it('handles multiple entries with mixed shift / keep / drop outcomes', () => { + // prev: `@A xxx @B yyy` + // entry1 at 0 entry2 at 11 + // edit: replace `xxx` (cols 4-6) with `12345` (delta = +2) + const e1: TrackedInsertion = { token: 'A', start: 0, pluginId: 'a' }; + const e2: TrackedInsertion = { token: 'B', start: 11, pluginId: 'b' }; + const prev = '@A xxx @B yyy'; + const next = '@A 12345 @B yyy'; + const out = reconcileInsertions([e1, e2], prev, next); + expect(out).toEqual([ + { token: 'A', start: 0, pluginId: 'a' }, + { token: 'B', start: 13, pluginId: 'b' }, // 11 + 2 + ]); + }); + + it('returns an empty list when the entries are empty', () => { + expect(reconcileInsertions([], 'a', 'b')).toEqual([]); + }); + + // Purity guard (#2929 round 5): reconcile must not mutate its + // inputs and must produce the same output regardless of how many + // times it is called with the same arguments. React StrictMode + // double-invokes setState updaters in development; the previous + // implementation called reconcile *inside* the updater and + // accumulated shifts (entry at 0 → 8 → 16) on the second + // invocation, dropping the entry as out-of-range. The fix moves + // reconcile out of the updater, but pinning purity here too so a + // future regression there is caught at the algorithm layer. + it('is pure: invoking twice with the same args returns equivalent output and does not mutate input', () => { + const entries: TrackedInsertion[] = [{ token: 'Airbnb', start: 0, pluginId: 'airbnb' }]; + const frozen = Object.freeze([...entries]) as ReadonlyArray; + const first = reconcileInsertions(frozen, '@Airbnb ', '@github @Airbnb '); + const second = reconcileInsertions(frozen, '@Airbnb ', '@github @Airbnb '); + expect(first).toEqual([{ token: 'Airbnb', start: 8, pluginId: 'airbnb' }]); + expect(second).toEqual([{ token: 'Airbnb', start: 8, pluginId: 'airbnb' }]); + // Frozen input was not mutated (any attempt would have thrown + // in strict mode). + expect(frozen).toEqual([{ token: 'Airbnb', start: 0, pluginId: 'airbnb' }]); + }); +}); + +describe('stripPluginInsertedTokens', () => { + it('returns the draft unchanged when there are no entries', () => { + expect(stripPluginInsertedTokens('@Airbnb ', [])).toBe('@Airbnb '); + }); + + it('removes a single tracked token at the start of the draft', () => { + expect( + stripPluginInsertedTokens('@Airbnb ', [{ token: 'Airbnb', start: 0, pluginId: 'airbnb' }]), + ).toBe(' '); // trailing space from inserted text remains; integration trims as needed + }); + + it('removes a tracked token while preserving an untracked duplicate (#2929 round 3)', () => { + // The whole point: composer-inserted `@Airbnb` at offset 0 gets + // removed; the user-authored `@Airbnb` at offset 16 is untracked + // and therefore preserved. + const draft = '@Airbnb compare @Airbnb with our spec'; + const out = stripPluginInsertedTokens(draft, [{ token: 'Airbnb', start: 0, pluginId: 'airbnb' }]); + expect(out).toBe(' compare @Airbnb with our spec'); + }); + + it('slices multiple tracked tokens in one pass without offset drift', () => { + // Two tracked entries, descending sort means the right one is + // sliced first so the left one's offset stays valid. + const draft = '@A and @B'; + const out = stripPluginInsertedTokens(draft, [ + { token: 'A', start: 0, pluginId: 'a' }, + { token: 'B', start: 7, pluginId: 'b' }, + ]); + expect(out).toBe(' and '); + }); + + it('drops invalidated entries instead of corrupting unrelated text', () => { + // The tracked offset no longer points at `@Airbnb` (user retyped). + // strip should be a no-op rather than deleting whatever sits at the + // stale offset. + const draft = 'hello world'; + const out = stripPluginInsertedTokens(draft, [ + { token: 'Airbnb', start: 0, pluginId: 'airbnb' }, + ]); + expect(out).toBe('hello world'); + }); + + it('collapses double whitespace left behind by the strip', () => { + const draft = 'see @Airbnb here'; + const out = stripPluginInsertedTokens(draft, [ + { token: 'Airbnb', start: 4, pluginId: 'airbnb' }, + ]); + // After slicing `@Airbnb`: `see here` (two spaces) → collapse to `see here` + expect(out).toBe('see here'); + }); + + it('does not normalize user-authored multi-space spans elsewhere in the draft (#2929 round 8)', () => { + // Reviewer-flagged: the previous global `[ \t]{2,}` collapse + // would rewrite any user-authored double-space span to a + // single space, even ones unrelated to the strip seam. The + // seam-local collapse here only touches the whitespace + // adjacent to the removed range. + const draft = 'keep gap @Airbnb here'; + const out = stripPluginInsertedTokens(draft, [ + { token: 'Airbnb', start: 10, pluginId: 'airbnb' }, + ]); + // `keep gap` (two spaces) is preserved; the `@Airbnb` seam + // collapses to a single space. + expect(out).toBe('keep gap here'); + }); + + it('preserves multi-space spans on both sides of an unrelated mention (#2929 round 8)', () => { + // Two user-authored double-space spans flank an `@Airbnb` + // that is not tracked. Strip should be a no-op (no entries + // for it) — verifies that nothing in the function reaches + // out and normalizes whitespace when it has no entries to + // operate on. + const draft = 'one two @Untracked three four'; + const out = stripPluginInsertedTokens(draft, []); + expect(out).toBe('one two @Untracked three four'); + }); +});