mirror of
https://github.com/nexu-io/open-design.git
synced 2026-05-31 19:04:39 +07:00
fix(web): prune draft tokens when the plugin chip strip clears (#2881) (#3356)
Some checks failed
visual-baseline / Capture visual baselines (push) Waiting to run
ci / Detect CI change scopes (push) Successful in 0s
nix-check / build (push) Failing after 1s
ci / Validate Nix flake (push) Has been skipped
ci / Preflight (push) Failing after 1s
ci / Workspace unit tests (push) Failing after 1s
ci / Daemon workspace tests (push) Failing after 1s
ci / Web workspace tests (push) Failing after 1s
ci / Browser tests (push) Failing after 1s
ci / Build workspaces (push) Failing after 1s
ci / Validate workspace (push) Failing after 0s
ci / Runtime trace (push) Has been skipped
Some checks failed
visual-baseline / Capture visual baselines (push) Waiting to run
ci / Detect CI change scopes (push) Successful in 0s
nix-check / build (push) Failing after 1s
ci / Validate Nix flake (push) Has been skipped
ci / Preflight (push) Failing after 1s
ci / Workspace unit tests (push) Failing after 1s
ci / Daemon workspace tests (push) Failing after 1s
ci / Web workspace tests (push) Failing after 1s
ci / Browser tests (push) Failing after 1s
ci / Build workspaces (push) Failing after 1s
ci / Validate workspace (push) Failing after 0s
ci / Runtime trace (push) Has been skipped
ChatComposer tracks the `@…` tokens this surface authored via the
@-mention popover plugin-pick path. When PluginsSection's chip strip
clears, we wire its `onCleared` and prune *only* those tracked
insertions from the draft so the textarea no longer holds orphaned
styled mentions whose chips just unmounted.
Architecture summary (rounds 1–9 collapsed; round 10 detailed below):
- `Array<{token, start, pluginId, insertionId?}>` tracking with
start offsets reconciled across each keystroke via an LCS+LCP
edit-range diff in
`apps/web/src/utils/pluginInsertionTracking.ts` (round 3-4).
`insertionId` is forwarded by `reconcileInsertions` so the
producer can locate its own entry across reconciles
(round 10).
- All draft mutations route through a single `updateDraft`
chokepoint that runs `reconcileInsertions` outside the
`setDraft` updater so React StrictMode's double-invoke is
harmless (round 4-5).
- Boundaries delegate to the shared
`inlineMentions.isMentionBoundary` /
`inlineMentions.isMentionRightBoundary` helpers so the
tracker can never diverge from the parser (round 5).
- `setActivePlugin` is a chokepoint for every applyById path,
filtering tracked entries to those matching the new active
plugin so a replace-plugin flow can never let stale entries
survive (round 6).
- Picker rollback double-snapshots draft + tracker so apply-
failure restores the tracker but only rewrites the draft
when no user keystrokes arrived during the await
(round 7-8).
- `stripPluginInsertedTokens` collapses whitespace seam-local
so user-authored multi-space spans elsewhere are preserved
(round 8).
- `setActivePlugin` is deferred past `await applyById` on
every path, and `onCleared` filters by
`pluginsSectionRef.current?.getActiveRecord()?.id` so a
pending-window clear scopes to the actually-mounted
plugin's tokens (round 9).
race in the picker rollback:
Round 9 made `onCleared` mutate the tracker and the draft when
it ran during a pending replace, and added the `getActiveRecord`
filter so the strip targets the still-mounted plugin's entries
only. The picker's failure-path rollback, however, still
restored `prevEntries` / `prevActiveId` wholesale — assuming
nothing else had touched the tracker during the await. If the
user clicked the still-mounted original chip's × during the
pending replace AND the deferred `applyById` then resolved
with a 500, the wholesale restore (a) resurrected entries that
`onCleared` had legitimately stripped (now stale offsets) and
(b) left the optimistic `@<target>` orphaned in the draft with
no chip ever having mounted — the original #2881 symptom
recurring inside the failure window.
Fix splits the failure rollback into two paths:
1. **Detect "intervening clear" via `activePluginIdRef.current
=== null && prevActiveId !== null`.** `onCleared` always
nulls the active id as its last action; our deferred
`setActivePlugin` never ran in the failure branch. So the
null-while-prev-not-null state is the smoking gun for an
intervening clear during the await.
2. **On detection, surgically remove only our optimistic
entry and only its `@<target>`.** Locate the entry by
`insertionId` (added to `TrackedInsertion` as an optional
field, forwarded by `reconcileInsertions` so the id
survives offset shifts) — this disambiguates the case
where the user picked the same plugin from the @-popover
more than once during the await window. Splice that entry
out and run `updateDraft((d) => stripPluginInsertedTokens(
d, [ourEntry]))` so the draft loses `@<target>` and any
remaining tracked entries (the in-flight target would have
no others, but a co-pending second pick could) get their
offsets reconciled. `activePluginIdRef` stays at `null` —
`onCleared`'s truth, since no chip is mounted.
The "no intervening clear" branch is the round 7/8 path:
restore `prevEntries`/`prevActiveId` wholesale and rewrite
the draft only if `draftRef.current === postInsertDraft`
(no user keystrokes during the await).
Regression coverage (additions):
- `apps/web/tests/components/ChatComposer.plugin-clear-prunes-draft.test.tsx`
— 18 integration specs total (17 prior + 1 new round-10):
* `@-popover pick A → @-popover pick B (apply pending) →
clear A's chip → resolve B with 500 → assert no orphan
@<target>, no orphan @A, no chip mounted, no stale
tracker entries`. Uses a deferred `Promise<Response>` so
the apply stays in flight while the chip-clear is fired,
then resolves with a 500 to drive the failure path. Pre-
fix this would resurrect Airbnb's stale entry AND leave
`@SecondPlugin` orphaned in the draft.
PluginsSection.tsx is unchanged. The host-local tracking +
draft-update chokepoint + parser-aligned boundaries + deferred
active-plugin scoping + transactional applyById + intervening-
clear-aware rollback + filtered `onCleared` keep the cross-
component contract identical to main — only ChatComposer touches
behavior, plus the utils module and two `inlineMentions` exports.
Validation:
- pnpm exec vitest run tests/utils/pluginInsertionTracking.test.ts → 36/36 passed
- pnpm exec vitest run tests/components/ChatComposer.plugin-clear-prunes-draft.test.tsx → 18/18 passed
- pnpm exec vitest run -c vitest.config.ts (full apps/web suite, 228 files) → 2202/2202 passed
- pnpm --filter @open-design/web typecheck → green
- pnpm guard → green
This commit is contained in:
estelledc
GitHub
parent
f4c5d22f22
commit
1a6face04c
5 changed files with 2401 additions and 29 deletions
|
|
@ -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, string | number>) => string;
|
||||
|
|
@ -224,7 +229,23 @@ export const ChatComposer = forwardRef<ChatComposerHandle, Props>(
|
|||
) {
|
||||
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<string>(
|
||||
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<ChatComposerHandle, Props>(
|
|||
// or from the tools-menu "Details" affordance.
|
||||
const [detailsRecord, setDetailsRecord] = useState<InstalledPluginRecord | null>(null);
|
||||
const pluginsSectionRef = useRef<PluginsSectionHandle | null>(null);
|
||||
// Instance-aware tracking for `@<token>` 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<TrackedInsertion[]>([]);
|
||||
// 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<string | null>(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<ChatComposerHandle, Props>(
|
|||
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<ChatComposerHandle, Props>(
|
|||
// 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<ChatComposerHandle, Props>(
|
|||
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<ChatComposerHandle, Props>(
|
|||
return false;
|
||||
}
|
||||
}
|
||||
setDraft('');
|
||||
updateDraft('');
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
@ -732,7 +824,7 @@ export const ChatComposer = forwardRef<ChatComposerHandle, Props>(
|
|||
ref,
|
||||
() => ({
|
||||
setDraft: (text: string) => {
|
||||
setDraft(text);
|
||||
updateDraft(text);
|
||||
seededRef.current = true;
|
||||
requestAnimationFrame(() => {
|
||||
const ta = textareaRef.current;
|
||||
|
|
@ -743,7 +835,7 @@ export const ChatComposer = forwardRef<ChatComposerHandle, Props>(
|
|||
});
|
||||
},
|
||||
restoreDraft: ({ text, attachments = [], commentAttachments = [] }) => {
|
||||
setDraft(text);
|
||||
updateDraft(text);
|
||||
setStaged(attachments);
|
||||
setStagedVisualComments(commentAttachments);
|
||||
setStagedSkills([]);
|
||||
|
|
@ -768,8 +860,39 @@ export const ChatComposer = forwardRef<ChatComposerHandle, Props>(
|
|||
[]
|
||||
);
|
||||
|
||||
// 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<ChatComposerHandle, Props>(
|
|||
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<ChatComposerHandle, Props>(
|
|||
// Also strip the matching `@<id>` 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<ChatComposerHandle, Props>(
|
|||
}),
|
||||
]);
|
||||
}
|
||||
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<ChatComposerHandle, Props>(
|
|||
}
|
||||
|
||||
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<ChatComposerHandle, Props>(
|
|||
function handleChange(e: React.ChangeEvent<HTMLTextAreaElement>) {
|
||||
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 `@<id>` 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<ChatComposerHandle, Props>(
|
|||
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<ChatComposerHandle, Props>(
|
|||
}
|
||||
|
||||
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 `@<token>` whose chip never mounted — a user
|
||||
// clearing the previously-active plugin's chip would then
|
||||
// strip the user-visible `@<token>` 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 `@<sameLabel>` 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 `@<target>` 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 `@<target>` 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 `@<target>` 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 `@<token>` 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<ChatComposerHandle, Props>(
|
|||
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<ChatComposerHandle, Props>(
|
|||
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 `@<target>` 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
|
||||
// `@<target>` and leave the unmounting plugin's
|
||||
// `@<token>` 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<ChatComposerHandle, Props>(
|
|||
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<ChatComposerHandle, Props>(
|
|||
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<ChatComposerHandle, Props>(
|
|||
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<ChatComposerHandle, Props>(
|
|||
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);
|
||||
}}
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -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: `@<token>` 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 `@<token>` 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) {
|
||||
|
|
|
|||
224
apps/web/src/utils/pluginInsertionTracking.ts
Normal file
224
apps/web/src/utils/pluginInsertionTracking.ts
Normal file
|
|
@ -0,0 +1,224 @@
|
|||
// Instance-aware tracking for `@<token>` 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 `@<token>` 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<TrackedInsertion>,
|
||||
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 `@<token>` 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<TrackedInsertion>,
|
||||
): 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;
|
||||
}
|
||||
File diff suppressed because it is too large
Load diff
310
apps/web/tests/utils/pluginInsertionTracking.test.ts
Normal file
310
apps/web/tests/utils/pluginInsertionTracking.test.ts
Normal file
|
|
@ -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 `@<token>` 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<TrackedInsertion>;
|
||||
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');
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue