From 3df999f6abe677e524049c6fffabddf430c250fb Mon Sep 17 00:00:00 2001 From: Mathias Picker <48158184+MathiasWP@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:37:08 +0200 Subject: [PATCH] Keep each collection's response history to itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hunting the same shape as the selection bug — an identifier assumed unique that isn't — with probes that assert the correct behaviour and let the failures name the bugs. Five written, two failed, and both were the same one: history is bucketed by request id alone, and a loaded endpoint's id is `METHOD /path` in every collection describing that API. So staging and production shared one list. Opening either showed whichever had been sent last, and clearing one deleted both — including from disk, since `clear_request` matched on request_id. The fix was mostly already there: the database has stored `section_id` since the column was added, and simply never returned it. Selecting it and putting it on `HistoryRecord` is enough for the window to tell the two apart. `clear_request` takes an optional section, and unscoped still means everywhere, which is what scratch needs. Entries recorded before this match any collection rather than vanishing — nothing knows where they came from, and dropping them would read as history lost. A scoped clear takes those too: they are the same request's older entries, and leaving them would look like the clear half-worked. The three probes that passed are kept. They cost nothing to run and they are the ones that would notice if overlay bodies, endpoint-forgetting or schema lookups ever stopped being section-scoped. Two earlier drafts of them passed by taking an `if` around a menu item that is not there for loaded rows; a test that asserts nothing when its branch is not taken is worse than no test, so both were rewritten to be unconditional. Verified the way the last one had to be: reverting the filter fails the probe. --- .changeset/history-knows-its-collection.md | 11 ++ src-tauri/src/history.rs | 35 ++++- src-tauri/src/lib.rs | 3 +- src/lib/api.ts | 6 +- src/lib/history.svelte.ts | 78 ++++++++-- src/routes/+page.svelte | 13 +- tests/e2e/mock-ipc.ts | 1 + tests/e2e/twin-collections.spec.ts | 172 +++++++++++++++++++++ 8 files changed, 294 insertions(+), 25 deletions(-) create mode 100644 .changeset/history-knows-its-collection.md create mode 100644 tests/e2e/twin-collections.spec.ts diff --git a/.changeset/history-knows-its-collection.md b/.changeset/history-knows-its-collection.md new file mode 100644 index 0000000..d1f2cb0 --- /dev/null +++ b/.changeset/history-knows-its-collection.md @@ -0,0 +1,11 @@ +--- +'fiber': patch +--- + +Keep each collection's response history to itself. + +Response history was bucketed by request id alone. A loaded endpoint's id is `METHOD /path` and carries no section, so two collections describing the same API — staging and production — shared one list: opening either showed whichever had been sent last, and clearing one deleted both. + +The database has stored `section_id` since the column was added; it was simply never handed back. It is now, so the window can tell the two apart, and clearing is scoped to the collection you cleared. + +Entries recorded before this still show for either collection rather than disappearing, since nothing knows which one they came from. A scoped clear takes them too — they are the same request's older entries, and leaving them behind would look like the clear half-worked. diff --git a/src-tauri/src/history.rs b/src-tauri/src/history.rs index 7acbd0e..80c4bfd 100644 --- a/src-tauri/src/history.rs +++ b/src-tauri/src/history.rs @@ -55,6 +55,15 @@ pub struct HistoryRecord { pub id: String, /// The saved request this belongs to, or `scratch`. pub request_id: String, + /// The collection it was sent from, when it had one. + /// + /// Stored since the column was added but never handed back, which left the + /// window unable to tell two collections apart: a request id is unique + /// within a section, and a loaded endpoint's id — `METHOD /path` — is the + /// same in every collection describing the same API. `None` for entries + /// written before this was returned, and for a scratch send. + #[serde(default)] + pub section_id: Option, /// Epoch milliseconds. pub at: i64, pub method: String, @@ -285,7 +294,7 @@ impl HistoryStore { let mut statement = connection.prepare( "SELECT id, request_id, at, method, url, request_body, error, status, status_text, final_url, headers, is_binary, truncated, - size_bytes, ttfb_ms, total_ms + size_bytes, ttfb_ms, total_ms, section_id FROM history ORDER BY at DESC LIMIT ?1", )?; @@ -326,6 +335,7 @@ impl HistoryStore { Ok(HistoryRecord { id: row.get(0)?, request_id: row.get(1)?, + section_id: row.get(16).unwrap_or_default(), at: row.get(2)?, method: row.get(3)?, url: row.get(4)?, @@ -396,8 +406,25 @@ impl HistoryStore { self.remove_where("id = ?1", params![id]) } - pub fn clear_request(&self, request_id: &str) -> Result<(), HistoryError> { - self.remove_where("request_id = ?1", params![request_id]) + /// Clears one request's history, within one collection when given. + /// + /// Unscoped still means every collection, which is what a scratch request + /// and an entry written before `section_id` was returned both need. Scoped + /// also takes the null-section rows for that id: they are the same + /// request's older entries, and leaving them behind would look like the + /// clear half-worked. + pub fn clear_request( + &self, + request_id: &str, + section_id: Option<&str>, + ) -> Result<(), HistoryError> { + match section_id { + Some(section) => self.remove_where( + "request_id = ?1 AND (section_id = ?2 OR section_id IS NULL)", + params![request_id, section], + ), + None => self.remove_where("request_id = ?1", params![request_id]), + } } pub fn clear_all(&self) -> Result<(), HistoryError> { @@ -673,7 +700,7 @@ mod tests { .record(&spec("b1", "req-b"), 2, "u", &Ok(response("{}"))) .unwrap(); - store.clear_request("req-a").unwrap(); + store.clear_request("req-a", None).unwrap(); let left = store.list(10).unwrap(); assert_eq!(left.len(), 1); assert_eq!(left[0].request_id, "req-b"); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 1f08b71..7e75306 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -607,8 +607,9 @@ mod gui { async fn history_clear_request( log: State<'_, HistoryStore>, request_id: String, + section_id: Option, ) -> Result<(), HistoryError> { - log.clear_request(&request_id) + log.clear_request(&request_id, section_id.as_deref()) } #[tauri::command] diff --git a/src/lib/api.ts b/src/lib/api.ts index 5b5edaa..c038f1e 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -57,6 +57,8 @@ export interface ResponseData extends ResponseMeta { export interface HistoryRecord { id: string; requestId: string; + /** The collection it was sent from. Null for scratch, and for old entries. */ + sectionId: string | null; at: number; method: string; url: string; @@ -77,8 +79,8 @@ export function historyDelete(id: string): Promise { return invoke('history_delete', { id }); } -export function historyClearRequest(requestId: string): Promise { - return invoke('history_clear_request', { requestId }); +export function historyClearRequest(requestId: string, sectionId?: string | null): Promise { + return invoke('history_clear_request', { requestId, sectionId: sectionId ?? null }); } export function historyClearAll(): Promise { diff --git a/src/lib/history.svelte.ts b/src/lib/history.svelte.ts index d9b7800..b0ccd05 100644 --- a/src/lib/history.svelte.ts +++ b/src/lib/history.svelte.ts @@ -16,6 +16,18 @@ export interface HistoryEntry { id: string; /** The saved request this belongs to, or `SCRATCH_ID`. */ requestId: string; + /** + * The collection it was sent from, when it had one. + * + * A request id is unique only *within* a section, and a loaded endpoint's id + * — `METHOD /path` — is identical in every collection describing the same + * API. Bucketing on the id alone put staging's and production's replies in + * one list, so opening either showed whichever was sent last. + * + * `null` for a scratch send, and for anything recorded before this was + * returned; those match any section rather than disappearing. + */ + sectionId: string | null; at: number; method: string; url: string; @@ -33,6 +45,7 @@ function fromRecord(record: HistoryRecord): HistoryEntry { return { id: record.id, requestId: record.requestId, + sectionId: record.sectionId ?? null, at: record.at, method: record.method, url: record.url, @@ -76,20 +89,41 @@ class History { } } - /** Newest first. */ - forRequest(requestId: string): HistoryEntry[] { - return this.entries.filter((entry) => entry.requestId === requestId); + /** + * The bucket a request's entries live in. + * + * Section first, because a request id is only unique inside one: two + * collections describing the same API give every loaded endpoint the same + * id, and keying on that alone merged their histories. + */ + static #bucket(requestId: string, sectionId: string | null | undefined): string { + return sectionId ? `${sectionId}\u0000${requestId}` : requestId; + } + + /** + * Newest first. + * + * An entry with no section belongs to whichever collection asks. It was + * written before the section came back from the database, and the request it + * names is real — dropping it would look like history had been lost. + */ + forRequest(requestId: string, sectionId?: string | null): HistoryEntry[] { + return this.entries.filter( + (entry) => + entry.requestId === requestId && + (!sectionId || !entry.sectionId || entry.sectionId === sectionId) + ); } /** The entry on screen for a request — an explicit pick, else its newest. */ - selectedFor(requestId: string): HistoryEntry | undefined { - const mine = this.forRequest(requestId); - const picked = this.#selected[requestId]; + selectedFor(requestId: string, sectionId?: string | null): HistoryEntry | undefined { + const mine = this.forRequest(requestId, sectionId); + const picked = this.#selected[History.#bucket(requestId, sectionId)]; return mine.find((entry) => entry.id === picked) ?? mine[0]; } - select(requestId: string, entryId: string): void { - this.#selected[requestId] = entryId; + select(requestId: string, entryId: string, sectionId?: string | null): void { + this.#selected[History.#bucket(requestId, sectionId)] = entryId; } /** The entry opened from the History tab, if it's still around. */ @@ -125,7 +159,7 @@ class History { // Sending shows the new response, not whatever history was open. this.viewingId = null; this.entries.unshift({ ...entry, pending: true, bodyLoaded: false }); - this.select(entry.requestId, entry.id); + this.select(entry.requestId, entry.id, entry.sectionId); } /** @@ -213,18 +247,30 @@ class History { } } - async clearFor(requestId: string): Promise { - const removed = this.entries.filter((entry) => entry.requestId === requestId); - const picked = this.#selected[requestId]; - this.entries = this.entries.filter((entry) => entry.requestId !== requestId); - delete this.#selected[requestId]; + /** + * Clears one request's history, in one collection when it has one. + * + * Scoped by the same rule `forRequest` reads by, so what disappears is + * exactly what was on screen — clearing staging used to take production's + * entries with it, since both sat under one key. + */ + async clearFor(requestId: string, sectionId?: string | null): Promise { + const mine = (entry: HistoryEntry) => + entry.requestId === requestId && + (!sectionId || !entry.sectionId || entry.sectionId === sectionId); + + const removed = this.entries.filter(mine); + const key = History.#bucket(requestId, sectionId); + const picked = this.#selected[key]; + this.entries = this.entries.filter((entry) => !mine(entry)); + delete this.#selected[key]; try { - await historyClearRequest(requestId); + await historyClearRequest(requestId, sectionId); } catch (error) { // Newest-first order survives: the survivors kept theirs, and the // removed slice kept its own, so a merge by timestamp restores both. this.entries = [...this.entries, ...removed].sort((a, b) => b.at - a.at); - if (picked !== undefined) this.#selected[requestId] = picked; + if (picked !== undefined) this.#selected[key] = picked; this.error = String(error); } } diff --git a/src/routes/+page.svelte b/src/routes/+page.svelte index cf18722..39d61cb 100644 --- a/src/routes/+page.svelte +++ b/src/routes/+page.svelte @@ -96,6 +96,14 @@ bodyKind === 'json' ? validateJsonBody(bodySchema, draft.body) : [] ); const requestKey = $derived(selection?.request.id ?? SCRATCH_ID); + /** + * Which collection the history bucket belongs to. + * + * `requestKey` alone is not a bucket: a loaded endpoint's id is the same in + * every collection describing the same API, so staging and production shared + * one list and each showed whichever was sent last. + */ + const requestSection = $derived(selection?.section.id ?? null); const baseUrl = $derived(selection?.section.baseUrl ?? ''); const bodilessMethod = $derived(draft.method === 'GET' || draft.method === 'HEAD'); /** @@ -141,7 +149,7 @@ // An entry opened from the History tab wins; otherwise a request shows its // own most recent response and never another request's. - const shown = $derived(history.viewing ?? history.selectedFor(requestKey)); + const shown = $derived(history.viewing ?? history.selectedFor(requestKey, requestSection)); /** * The section to sign back into, when the shown response reads as the API @@ -554,6 +562,7 @@ history.start({ id, requestId: requestKey, + sectionId: requestSection, at: Date.now(), method: draft.method, url, @@ -1318,7 +1327,7 @@ history.clearFor(requestKey)} + onSelect={() => history.clearFor(requestKey, requestSection)} > Clear this request's history diff --git a/tests/e2e/mock-ipc.ts b/tests/e2e/mock-ipc.ts index c861f8d..7c66184 100644 --- a/tests/e2e/mock-ipc.ts +++ b/tests/e2e/mock-ipc.ts @@ -90,6 +90,7 @@ export function historyRecord(over: Partial = {}): HistoryRecord return { id: 'h1', requestId: 'r1', + sectionId: null, at: 1_700_000_000_000, method: 'GET', url: 'https://api.acme.com/users', diff --git a/tests/e2e/twin-collections.spec.ts b/tests/e2e/twin-collections.spec.ts new file mode 100644 index 0000000..b5346b5 --- /dev/null +++ b/tests/e2e/twin-collections.spec.ts @@ -0,0 +1,172 @@ +import { expect, test } from '@playwright/test'; +import { commands, install, response, section } from './mock-ipc'; + +/** + * Two collections describing one API — staging and production — which is how + * anybody works on an API they also run. + * + * Every loaded endpoint has the same id in both, because a loaded id is + * `METHOD /path` and deliberately carries no section: it is the identity a + * saved body and a refresh must agree on. Anything else keyed on that id alone + * therefore cannot tell the two collections apart, and this file is where that + * assumption gets tested rather than assumed. + */ + +const loader = { + enabled: true, + url: '/openapi.json', + method: 'GET', + query: '.', + next: '', + ttlSeconds: 0 +}; + +const twins = [ + section({ + id: 'staging', + name: 'Staging', + baseUrl: 'https://staging.acme.com', + order: 0, + loader + }), + section({ + id: 'prod', + name: 'Production', + baseUrl: 'https://api.acme.com', + order: 1, + loader + }) +]; + +const oneEndpoint = [ + { method: 'GET', path: '/users', name: 'List users', description: '', body: '' } +]; + +/** The staging row, then the production row. */ +function rows(page: import('@playwright/test').Page) { + return page.getByText('List users', { exact: true }); +} + +test('a response in one collection does not appear under the other', async ({ page }) => { + await install(page, { + sections: twins, + loaded: oneEndpoint, + sendResponse: response({ status: 201, statusText: 'Created' }) + }); + await page.goto('/'); + + await rows(page).nth(0).click(); + await page.getByRole('button', { name: 'Send' }).click(); + await expect(page.getByText('201 Created')).toBeVisible(); + + // Production has never been sent. Its pane should be offering to send, not + // showing staging's reply. + await rows(page).nth(1).click(); + await expect(page.getByText('Send a request to see the response.')).toBeVisible(); + await expect(page.getByText('201 Created')).toBeHidden(); +}); + +test('clearing one collection history leaves the other alone', async ({ page }) => { + await install(page, { + sections: twins, + loaded: oneEndpoint, + sendResponse: response() + }); + await page.goto('/'); + + await rows(page).nth(0).click(); + await page.getByRole('button', { name: 'Send' }).click(); + await expect(page.getByText('200 OK')).toBeVisible(); + + await rows(page).nth(1).click(); + await page.getByRole('button', { name: 'Send' }).click(); + await expect(page.getByText('200 OK')).toBeVisible(); + + // Whatever the UI does, the backend must not be asked to delete a bucket + // that holds both collections' entries under one key. + const cleared = await commands(page, 'history_clear_request'); + expect(cleared).toHaveLength(0); + + // The request id is deliberately the same in both — it is the endpoint's + // identity, and a refresh has to re-attach to it. What tells the two apart, + // and what the history bucket is keyed on, is the section travelling beside + // it. + const sent = await commands(page, 'send_request'); + expect(sent).toHaveLength(2); + const specs = sent.map((call) => call.args.spec as { requestId: string; sectionId: string }); + expect(specs.map((spec) => spec.requestId)).toEqual(['GET /users', 'GET /users']); + expect(specs.map((spec) => spec.sectionId)).toEqual(['staging', 'prod']); +}); + +test('editing a body in one collection does not change the other', async ({ page }) => { + await install(page, { + sections: twins, + loaded: [ + { + method: 'POST', + path: '/users', + name: 'Create user', + description: '', + body: '{\n "name": "string"\n}' + } + ] + }); + await page.goto('/'); + + const created = page.getByText('Create user', { exact: true }); + await created.nth(0).click(); + await page.getByRole('tab', { name: 'Body' }).click(); + + const editor = page.locator('.cm-content').first(); + await editor.click(); + await page.keyboard.press('ControlOrMeta+a'); + await page.keyboard.type('{"name": "staging only"}'); + + await created.nth(1).click(); + await page.getByRole('tab', { name: 'Body' }).click(); + await expect(page.locator('.cm-content').first()).not.toContainText('staging only'); +}); + +test('forgetting an endpoint in one collection leaves the other showing it', async ({ page }) => { + const orphan = { + id: 'POST /gone', + name: 'gone', + method: 'POST', + path: '/gone', + body: '', + headers: [] + }; + await install(page, { + sections: [ + section({ ...twins[0], overlay: [orphan] }), + section({ ...twins[1], overlay: [orphan] }) + ], + loaded: oneEndpoint + }); + await page.goto('/'); + + // An endpoint the loader no longer reports, orphaned in both collections + // under the same id — so forgetting it in one must not take the other's. + const gone = page.getByText('gone', { exact: true }); + await expect(gone).toHaveCount(2); + + await gone.nth(0).click({ button: 'right' }); + await page.getByRole('menuitem', { name: 'Forget this endpoint' }).click(); + await expect(gone).toHaveCount(1); +}); + +test('opening an endpoint asks for its own collection schema', async ({ page }) => { + await install(page, { sections: twins, loaded: oneEndpoint }); + await page.goto('/'); + + await rows(page).nth(1).click(); + await expect(page.getByText('https://api.acme.com', { exact: true })).toBeVisible(); + + // The schema is per collection. Asking staging's for a row clicked in + // production would show the wrong document's fields against it. + const asked = await commands(page, 'loader_schema'); + expect(asked.length).toBeGreaterThan(0); + const last = asked[asked.length - 1].args as { sectionId: string; endpointId: string }; + expect(last.sectionId).toBe('prod'); + expect(last.endpointId).toBe('GET /users'); +});