fix(extension): repair Included Memories popup data handling and lifecycle#1261
fix(extension): repair Included Memories popup data handling and lifecycle#1261abhay-codes07 wants to merge 1 commit into
Conversation
…lifecycle The related-memories flow on ChatGPT/Claude/T3 round-tripped the background's string[] response through a dataset attribute, which coerces the array into one comma-joined string. The popup then re-split that string on /[,\n]/, so any memory whose content contains a comma was fragmented into multiple fake rows, and the index-based remove buttons spliced the wrong entries and rewrote a desynced prompt injection. Three more defects lived in the same duplicated block: - every fetch appended a fresh popup to <body> and registered a new document-level click listener that was never removed, so auto-search leaked one popup + one global listener per keystroke burst - the removal cleanup used length <= 1, so removing one of two memories silently threw away the last remaining memory too - removal re-joined memories with " ," and left the numbering baked into the strings, corrupting the injected prompt block The block was byte-identical across chatgpt.ts, claude.ts, and t3.ts, so the popup now lives in a shared utils/included-memories.ts helper: memories are stored as JSON on the dataset (commas survive), the prompt block is always rebuilt from that single source of truth, rows re-render from the array instead of juggling indices, removing entries keeps the last one until none remain (=== 0), and each popup disposes its predecessor (listener, node, auto-dismiss timer) before mounting. The background handler now returns raw memory strings and skips empty ones; numbering is applied at injection time so it stays correct after removals. Verified with tsc --noEmit and a full wxt build.
|
Hi @abhay-codes07 can you please fix the merge conflicts? |
|
Looking at the rebase, the conflicts are not mechanical. The Included Memories popup was reworked on One thing did survive the refactor though, and it is the original data-handling bug from #1257. In the new code the memories are stored as a comma-joined string and split back apart on
Because memory text itself can contain commas and newlines, a single memory gets shredded. For example "I live in Austin, Texas" renders as two separate rows ("I live in Austin" and "Texas"), and t3's per-item delete then splices the wrong index and rewrites the injected The focused fix on top of the new module is to make the round-trip delimiter-safe: serialize I would rather rebuild it that way than force this PR's old structure through. I can either repoint this PR to that focused change, or close it and open a fresh one against |
|
The popup fixes are useful, but this PR changes the shared GET_RELATED_MEMORIES response format without updating every consumer. When tested with the current main branch, this caused auto-recall regressions. This should preserve the existing response contract or update all ChatGPT, Claude, Gemini, and T3 consumers together with integration tests. Closing this version for now. |
|
Understood, thanks for the detail on the auto-recall regression. Opened #1339 as the contract-preserving version: it leaves the GET_RELATED_MEMORIES response and the injected |
Fixes #1257
What
The "Included Memories" popup pipeline on ChatGPT/Claude/T3 had four related defects, all rooted in the same ~180-line block duplicated across the three content scripts:
string[]; assigning that todataset.memoriesDatacoerces it into one comma-joined string, and the popup re-split it with.split(/[,\n]/). Any memory containing a comma showed up as several fake rows, and the index-based remove buttons spliced the wrong entries while rewriting a desynced prompt injection.length <= 1, so removing one of two memories also silently discarded the remaining one.<body>and registered a newdocument-level click listener that was never removed. With auto-search firing per debounced input change, a few minutes of prompt editing accumulates dozens of orphaned global click handlers and stacked popups." ,"and the numbering baked in by the background handler went stale, leaving the injected block as1. foo , 3. bar.Fix
Since the block was byte-identical in
chatgpt.ts,claude.ts, andt3.ts(apart from the prompt-element lookup), the popup now lives in a sharedutils/included-memories.ts:dataset.supermemoriesblock is always rebuilt from that single source of truth, with numbering applied at build time so it stays correct after removalsdata-memory-indexattributes=== 0), and only then clears the injection and restores the iconThe background handler now returns raw memory strings and skips empty results; each content script passes its own site-specific prompt-element lookup. Net −206 lines.
Testing
bun run compile(tsc --noEmit) — cleanbun run build(wxt, chrome-mv3) — cleanbiome checkon all touched files — cleanThe extension package has no test harness, so the helper was written as pure functions (
serializeMemories/parseMemories/buildPromptInjection) plus a self-contained popup factory to keep the behaviour easy to reason about and to unit-test later if a harness lands. Happy to add screenshots/screen recording of the popup flow if that helps review.cc @MaheshtheDev