Skip to content

Commit 6b82e6c

Browse files
committed
fix(mobile/search): revert drop-on-close — keep closed pads searchable
I had set tab.close to drop the pad's body from the content-search cache, on the theory that this stopped the "edited pad still matches old content" bug. But: - The original stale-content bug was the WebView's HTTP cache returning the old /export/txt response. That's already fixed by `cache: 'no-store'` on the refresh fetch. - Dropping on close threw away useful history. User just reported: open two pads (one with "Welcome" in the body), close both, search "welcome" — nothing comes up. Closed pads ARE a real switch target. Reverted the close-side drop. The cache now retains last-known content for any pad we've ever opened in the session. Re-opening a pad immediately refreshes its cache via the open path; searching while a pad is open also refreshes via searchPadContent. So the only "stale" entries are pads the user closed and hasn't re-opened — those keep their last-known body for search, which is the intended behaviour. Test rewritten to pin the new contract: open two pads, close both, search the welcome-only body — exactly the user's flow. Failed without this commit, passes with it.
1 parent d9e8dde commit 6b82e6c

3 files changed

Lines changed: 37 additions & 39 deletions

File tree

packages/mobile/src/platform/capacitor.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,15 @@ export function createCapacitorPlatform(): Platform {
100100
}),
101101
close: (input) =>
102102
wrap(async () => {
103-
// Drop the closed pad from the content-search cache so search
104-
// results only reflect currently-open pads. Without this, a
105-
// closed pad's last-known body keeps surfacing in hits even
106-
// though the user can't see it. We look up workspace+padName
107-
// BEFORE close removes the tab from the store.
108-
const tab = tabStore.listAll().find((t) => t.tabId === input.tabId);
109-
if (tab) padContentIndex.dropEntry(tab.workspaceId, tab.padName);
103+
// We keep the closed pad's body in the content-search cache.
104+
// Closed pads stay searchable using their last-known content
105+
// so the user can find a pad they viewed earlier in the
106+
// session via its body. On the next refresh (when re-opened
107+
// or on a tab.open elsewhere), the cache entry will be
108+
// overwritten with fresh content. The stale-content concern
109+
// from the original report turned out to be the WebView's
110+
// HTTP cache, addressed by `cache: 'no-store'` in
111+
// pad-content-index.
110112
tabStore.close(input.tabId);
111113
return { ok: true } as const;
112114
}),

packages/mobile/src/platform/pad-content-index.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -199,16 +199,6 @@ export function clear(workspaceId?: string): void {
199199
}
200200
}
201201

202-
/**
203-
* Drop the cached body for a single (workspace, pad). Called on
204-
* tab.close so search results only reflect the content of CURRENTLY-
205-
* OPEN pads — leaving the cache around after close lets stale text
206-
* from a previously-viewed pad keep showing up in search hits.
207-
*/
208-
export function dropEntry(workspaceId: string, padName: string): void {
209-
cache.delete(keyFor(workspaceId, padName));
210-
}
211-
212202
/** Test seam: lets tests poke entries in directly without going through
213203
* fetch. The capacitor.ts boot path only calls index() / search() /
214204
* clear() — this is wider but kept narrow by the `__` prefix. */

packages/mobile/tests/smoke.spec.ts

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -553,14 +553,18 @@ test('content search keeps working when the network goes down (uses cached body)
553553
await expect(results.filter({ hasText: 'welcome procedures' })).toHaveCount(1, { timeout: 5_000 });
554554
});
555555

556-
test('closed pads disappear from content-search hits', async ({ page }) => {
557-
// Regression: user noticed that searching "welcome" after editing a
558-
// pad to "moo" still returned the pad. The cached body wasn't being
559-
// cleared on close, so stale text from previously-viewed pads kept
560-
// surfacing. We now drop the cache entry on tab.close so search
561-
// only reflects currently-open pads.
562-
await page.route('**/p/stale-pad/export/txt', async (route) => {
563-
await route.fulfill({ status: 200, contentType: 'text/plain', body: 'Welcome to the stale pad' });
556+
test('closed pads stay searchable by their last-known content', async ({ page }) => {
557+
// User flow: open two pads (one body contains "Welcome"), close them
558+
// both, search "welcome" — expect the welcome-bodied pad to surface
559+
// using its cached body. Pads-in-history are a real switch target,
560+
// so the closed-pad path stays in the search index. (Stale-content
561+
// concerns are addressed by `cache: 'no-store'` on the refresh fetch
562+
// when the pad is currently open.)
563+
await page.route('**/p/closed-welcome/export/txt', async (route) => {
564+
await route.fulfill({ status: 200, contentType: 'text/plain', body: 'Welcome aboard the closed pad' });
565+
});
566+
await page.route('**/p/closed-other/export/txt', async (route) => {
567+
await route.fulfill({ status: 200, contentType: 'text/plain', body: 'Sprint planning notes here.' });
564568
});
565569

566570
await page.addInitScript(seedWorkspace);
@@ -569,31 +573,33 @@ test('closed pads disappear from content-search hits', async ({ page }) => {
569573
page.getByRole('button', { name: /open instance acme/i }),
570574
).toBeVisible({ timeout: 15_000 });
571575

572-
// Open, fetch content, then close.
573-
await openPad(page, WS_ID, 'stale-pad');
574-
await page.waitForResponse((r) => r.url().includes('/stale-pad/export/txt'));
575-
await page.evaluate((id) => {
576+
// Open both pads, wait for their bodies to be cached, then close both.
577+
await openPad(page, WS_ID, 'closed-welcome');
578+
await page.waitForResponse((r) => r.url().includes('/closed-welcome/export/txt'));
579+
await openPad(page, WS_ID, 'closed-other');
580+
await page.waitForResponse((r) => r.url().includes('/closed-other/export/txt'));
581+
582+
await page.evaluate(async (workspaceId) => {
576583
const platform = (window as unknown as { __test_platform: {
577584
tab: { close(input: { tabId: string }): Promise<unknown> };
578585
} }).__test_platform;
579-
return platform.tab.close({ tabId: id });
580-
}, `${WS_ID}::stale-pad`);
586+
await platform.tab.close({ tabId: `${workspaceId}::closed-welcome` });
587+
await platform.tab.close({ tabId: `${workspaceId}::closed-other` });
588+
}, WS_ID);
581589

582-
// Open QuickSwitcher and search the cached word.
590+
// Open QuickSwitcher and search "welcome". Only the welcome-bodied
591+
// pad should match (via the content-search path — neither pad name
592+
// contains "welcome").
583593
await page.evaluate(() => {
584594
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'k', ctrlKey: true, bubbles: true }));
585595
});
586596
await expect(page.getByRole('dialog', { name: /quick switcher/i })).toBeVisible({ timeout: 5_000 });
587597
await page.getByRole('textbox', { name: /quick switcher search input/i }).fill('welcome');
588598

589-
// The pad was closed → its cache entry was dropped → no content-
590-
// match snippet for "Welcome to the stale pad" should appear. (The
591-
// pad name doesn't contain "welcome" either, so we don't get the
592-
// name-match path.)
593-
await page.waitForTimeout(300); // > the 200ms content-search debounce
594599
const results = page.getByRole('option');
595-
await expect(results.filter({ hasText: 'Welcome to the stale pad' })).toHaveCount(0);
596-
await expect(results.filter({ hasText: 'stale-pad' })).toHaveCount(0);
600+
await expect(results.filter({ hasText: 'Welcome aboard' })).toHaveCount(1, { timeout: 5_000 });
601+
// The unrelated closed pad's body doesn't have "welcome" → must not show.
602+
await expect(results.filter({ hasText: 'Sprint planning' })).toHaveCount(0);
597603
});
598604

599605
// X-Frame-Options DENY / SAMEORIGIN detection from pure JS is unreliable

0 commit comments

Comments
 (0)