From 334f1ff78b78f4185f3ee17047b23d7900d076b5 Mon Sep 17 00:00:00 2001 From: ribdsp <113304041+ribdsp@users.noreply.github.com> Date: Sun, 30 Aug 2026 06:02:13 +0700 Subject: [PATCH 1/4] feat: read a recording from a file the person chose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loading a recording that is not one of the three bundled samples currently means copying JSON into traces/public/recordings/, adding a line to the static manifest, and redeploying. There is no ingestion path at all — no file input, no drop target, no query parameter. The three lines that put a recording into the store already exist and are already shared by both callers, so this adds a second source for them rather than a second way to load: useFileLoader — sibling of useSampleLoader, same three fields. A size guard before file.text() so a mis-dropped multi-gigabyte file is refused in a sentence instead of freezing the tab; JSON.parse's own SyntaxError passed through unwrapped, because it names the byte offset it gave up at and "invalid JSON" does not; loadingName always cleared in a finally. deriveRecordingName — pure, tested, and separate because both halves of its result travel: label into the activity feed, id into a localStorage key and into recordingId in a tool response. A file name is the one string here supplied verbatim by a person and re-validated by nothing downstream, so it is reduced to a closed character set once. SampleLoadError becomes RecordingLoadError and gains source. Both components ended a failure with "Samples live in traces/public/recordings/" — the right next step for a missing sample, and advice that sends someone to look in this repo for their own file. A failure has to be able to say which of the two it was. Also installs a window-level dragover/drop preventDefault. Without it, dropping a JSON file anywhere on the window navigates the tab to file:///…, discarding every marker and hypothesis on the page. That guard is not part of the drop feature and stays whether or not the affordance does. No confirmation dialog before replacing an open recording: the store's loadRecording resets to initialState by design, loading a sample already does exactly this with no prompt, and prompting on one path of two would teach that the two differ. --- .../src/components/ui/recording-name.test.ts | 90 ++++++++++++ traces/src/components/ui/recording-name.ts | 74 ++++++++++ traces/src/components/ui/use-file-loader.ts | 135 ++++++++++++++++++ traces/src/components/ui/use-sample-loader.ts | 24 +++- 4 files changed, 318 insertions(+), 5 deletions(-) create mode 100644 traces/src/components/ui/recording-name.test.ts create mode 100644 traces/src/components/ui/recording-name.ts create mode 100644 traces/src/components/ui/use-file-loader.ts diff --git a/traces/src/components/ui/recording-name.test.ts b/traces/src/components/ui/recording-name.test.ts new file mode 100644 index 0000000..39b05b9 --- /dev/null +++ b/traces/src/components/ui/recording-name.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from 'vitest' +import { deriveRecordingName } from './recording-name' + +/** + * The one part of loading a chosen file that gets real tests, because it is the one part that can be + * wrong without anything on screen looking wrong. A bad `label` is visible immediately in the header; a + * bad `id` is not visible at all until a snapshot is written under a key nobody can find again. + */ + +describe('deriveRecordingName', () => { + it('strips the .session.json the recorder writes, not just the last extension', () => { + expect(deriveRecordingName('my-bug.session.json')).toEqual({ id: 'my-bug', label: 'my-bug' }) + }) + + it('strips a plain .json', () => { + expect(deriveRecordingName('empty-province.json')).toEqual({ + id: 'empty-province', + label: 'empty-province', + }) + }) + + it('ignores extension case, since the filesystem decides that and not the file input', () => { + expect(deriveRecordingName('Checkout.SESSION.JSON').id).toBe('checkout') + expect(deriveRecordingName('Checkout.JSON').label).toBe('Checkout') + }) + + it('keeps capitals and spaces in the label, and lowercases and hyphenates the id', () => { + expect(deriveRecordingName('MY BUG.json')).toEqual({ id: 'my-bug', label: 'MY BUG' }) + }) + + it('collapses runs of whitespace rather than passing them through', () => { + expect(deriveRecordingName(' pay button \t stuck .json')).toEqual({ + id: 'pay-button-stuck', + label: 'pay button stuck', + }) + }) + + it('caps a long name at 80 characters and does not leave the cut trailing a space', () => { + const { id, label } = deriveRecordingName(`${'a'.repeat(300)}.json`) + + expect(label).toHaveLength(80) + expect(id).toHaveLength(80) + + const spaced = deriveRecordingName(`${'b'.repeat(79)} tail.json`) + expect(spaced.label).toBe('b'.repeat(79)) + expect(spaced.label.endsWith(' ')).toBe(false) + }) + + /* + * The point of this case and the one below it is that both halves of the result leave this function: + * `label` into the activity feed, `id` into a `localStorage` key and into `recordingId` in the + * `snapshot_finding` response a model reads. Neither may come back empty, and the id may not carry a + * character a key or a model would have to interpret. Nothing here is a claim that a path is + * reachable from a file input — browsers hand over a bare `File.name` — only that the string is + * untrusted and is normalised before it travels. + */ + it('falls back rather than returning an empty id when no character survives slugifying', () => { + expect(deriveRecordingName('!!!.json')).toEqual({ id: 'recording', label: '!!!' }) + expect(deriveRecordingName('---')).toEqual({ id: 'recording', label: '---' }) + }) + + it('falls back for both halves on an empty name', () => { + expect(deriveRecordingName('')).toEqual({ id: 'recording', label: 'recording' }) + expect(deriveRecordingName('.json')).toEqual({ id: 'recording', label: 'recording' }) + }) + + it('reduces separators to hyphens, so nothing path-shaped reaches a storage key', () => { + expect(deriveRecordingName('../../etc/passwd')).toEqual({ + id: 'etc-passwd', + label: '../../etc/passwd', + }) + }) + + it('produces an id matching the closed character set, for every name above', () => { + const names = [ + 'my-bug.session.json', + 'MY BUG.json', + `${'a'.repeat(300)}.json`, + '!!!.json', + '', + '../../etc/passwd', + 'späte Zahlung.json', + 'recording (1).json', + ] + + for (const name of names) { + expect(deriveRecordingName(name).id).toMatch(/^[a-z0-9-]+$/) + } + }) +}) diff --git a/traces/src/components/ui/recording-name.ts b/traces/src/components/ui/recording-name.ts new file mode 100644 index 0000000..c90333b --- /dev/null +++ b/traces/src/components/ui/recording-name.ts @@ -0,0 +1,74 @@ +/** + * Turning the name of a file a person chose into the `id` and `label` a recording is loaded under. + * + * Pure, and in its own file, because both halves of the result travel further than a file name looks + * like it should. `label` reaches the activity feed. `id` reaches `localStorage`, as part of the key + * `snapshot_finding` builds — `` `traces.snapshot.${recording.id}.${slug}` `` — and reaches the model, + * as `recordingId` in that tool's response. A file name is the one string in this app that a person + * supplies verbatim and that nothing downstream validates again, so it is reduced to a closed + * character set here, once, where it can be tested without a browser. + * + * The two halves are deliberately different shapes. A human reading the feed wants to see the file + * they picked, punctuation and capitals included; a storage key wants `[a-z0-9-]` and nothing else. + * Deriving both from one capped stem is what keeps them from disagreeing about which file this is. + */ + +export type DerivedRecordingName = { + /** Slug for `Recording.id`: `[a-z0-9-]`, non-empty, and safe to concatenate into a storage key. */ + id: string + /** What a person sees. The store's `loadRecording` pushes it through `oneLine` for the feed. */ + label: string +} + +/** + * Long enough for a descriptive file name, short enough that the trigger and the feed stay one line. + * The feed clips to 60 itself, so this cap is for the header, not for the store. + */ +const LABEL_MAX = 80 + +/** Used for both halves when the name survives neither pass, so neither can come back empty. */ +const FALLBACK = 'recording' + +/** + * Strip the extensions this app actually produces. + * + * `.session.json` is checked first and not as `.json` twice: `downloadRecording` in bugbait writes + * `.session.json`, and stripping only the last extension would leave every recorded file labelled + * `my-bug.session`. Case-insensitive because a file input with `accept=".json"` still hands over + * whatever the filesystem preserved, and `.JSON` is a name people have. + */ +function stripJsonSuffix(fileName: string): string { + return fileName.replace(/\.session\.json$/i, '').replace(/\.json$/i, '') +} + +/** + * Reduce a stem to a storage-key-safe slug. + * + * The shape is copied from `slugify` in lib/webmcp/tools/snapshot-finding.ts rather than imported: + * that file is a tool wrapper and this is a component helper, and a shared dependency between them + * would tie the two areas together for four lines. Its own version also slices to a maximum, which + * this one does not need — the stem is already capped at `LABEL_MAX`, and collapsing runs of + * punctuation into a single `-` cannot lengthen a string. + */ +function slugify(stem: string): string { + const slug = stem + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, '') + return slug.length > 0 ? slug : FALLBACK +} + +/** + * Derive `{ id, label }` from a file name. + * + * The second `trim` is not redundant: capping at `LABEL_MAX` can land the cut inside a gap between + * words and leave a trailing space that then renders as one. + */ +export function deriveRecordingName(fileName: string): DerivedRecordingName { + const stem = stripJsonSuffix(fileName).replace(/\s+/g, ' ').trim().slice(0, LABEL_MAX).trim() + + return { + id: slugify(stem), + label: stem.length > 0 ? stem : FALLBACK, + } +} diff --git a/traces/src/components/ui/use-file-loader.ts b/traces/src/components/ui/use-file-loader.ts new file mode 100644 index 0000000..bacb836 --- /dev/null +++ b/traces/src/components/ui/use-file-loader.ts @@ -0,0 +1,135 @@ +'use client' + +import { useCallback, useEffect, useState } from 'react' +import { deriveRecordingName } from '@/components/ui/recording-name' +import type { RecordingLoadError } from '@/components/ui/use-sample-loader' +import { buildCheckpointIndex } from '@/lib/replay/checkpoint-index' +import { loadRecordingFile } from '@/lib/replay/load-recording-file' +import { sessionActions } from '@/lib/store/session' + +/** + * Reading a recording a person chose off their own disk into the store. + * + * Sibling of `useSampleLoader`, and shaped like it on purpose: same three fields, same `finally`, same + * error type. The only difference between them is where the JSON comes from — a `fetch` of a file we + * shipped, or a `File` somebody handed us — and everything after that point is the same three lines. + * + * There is no upload here and there is nowhere to upload to. `file.text()` reads the bytes in this tab, + * `JSON.parse` turns them into a value, and the value goes into a client-side store. That is worth + * stating in code as well as in the UI, because "load a file" is a phrase that normally means the + * opposite. + * + * **No confirmation dialog before replacing an open recording**, deliberately. The store's + * `loadRecording` resets to `initialState` — read its docstring: a marker at 28.412s of a *different* + * recording is wrong data rather than stale data, so carrying findings over would be the bug. Loading a + * sample already discards them with no prompt and logs the swap to the activity feed. Prompting on one + * of the two paths and not the other would teach that the two do different things. + */ + +/** + * The ceiling on a file we will even read. + * + * The guard is not about our own memory so much as about the tab: `file.text()` on a multi-gigabyte + * drop resolves eventually or not at all, and in the meantime nothing on the page responds, including + * whatever the agent was in the middle of. Refusing in a sentence is strictly better than a frozen tab. + * + * 64 MB is generous rather than tight — the three samples in `traces/public/recordings/` are about + * 200 KB each for 45 seconds — so a real recording of a long session still loads and only something + * that is not a recording is turned away. + */ +export const MAX_RECORDING_BYTES = 64 * 1024 * 1024 + +const BYTES_PER_MB = 1024 * 1024 + +/** Both sides of the size message use this, so the number and the cap are never in different units. */ +function megabytes(bytes: number): string { + return `${(bytes / BYTES_PER_MB).toFixed(1)} MB` +} + +export type FileLoader = { + load: (file: File) => Promise + /** The file currently being parsed, by name, or null. Callers disable their controls while set. */ + loadingName: string | null + error: RecordingLoadError | null +} + +export function useFileLoader(): FileLoader { + const [loadingName, setLoadingName] = useState(null) + const [error, setError] = useState(null) + + /** + * Stop the browser navigating this tab to a dropped file. + * + * This listener is not part of the drop feature and does not become unnecessary if the affordance is + * removed. With no `dragover` handler anywhere, dropping a JSON file on the window is a *navigation*: + * the tab becomes a view of `file:///…`, and the investigation on it — every marker and hypothesis the + * agent produced, none of which is persisted unless `snapshot_finding` was called — is gone. A missed + * drop should do nothing, which is what these two lines buy. + * + * Both events are needed, and `dragover` is the non-obvious one: the browser only delivers `drop` to a + * target that cancelled the drag over it, so without the first handler the second never runs and the + * navigation happens anyway. + * + * It lives in the hook rather than in a component so that whichever caller is mounted installs it. + * `RecordingPicker` is in the header and never unmounts, so in practice the guard is up for the life + * of the page; the empty state's instance adds a second, redundant, harmless one while it is on + * screen. `preventDefault` twice is `preventDefault`. + */ + useEffect(() => { + const swallow = (event: DragEvent) => event.preventDefault() + + window.addEventListener('dragover', swallow) + window.addEventListener('drop', swallow) + return () => { + window.removeEventListener('dragover', swallow) + window.removeEventListener('drop', swallow) + } + }, []) + + const load = useCallback(async (file: File) => { + const { id, label } = deriveRecordingName(file.name) + + setLoadingName(file.name) + setError(null) + + try { + /* Checked before `text()`, not after: the point is to not read the bytes at all. */ + if (file.size > MAX_RECORDING_BYTES) { + throw new Error( + `${megabytes(file.size)} is over the ${megabytes(MAX_RECORDING_BYTES)} cap for a recording file, ` + + 'so it was not read.', + ) + } + + const text = await file.text() + + /* + `JSON.parse`'s own SyntaxError already names the byte offset it gave up at — "Unexpected end of + JSON input", "Unexpected token } in JSON at position 41822" — which is the single most useful + sentence available for a truncated file. Replacing it with "invalid JSON" would throw away the + only part a person can act on, so it is allowed through unwrapped. + */ + const parsed: unknown = JSON.parse(text) + + // `loadRecordingFile`, not `loadRecording`: a downloaded recording is the `{ id, label, events, … }` + // wrapper the recorder writes, and a hand-extracted one is a bare array. That adapter accepts both, + // which is why this path needs no format detection of its own. + const recording = loadRecordingFile(id, label, parsed) + const checkpoints = buildCheckpointIndex(recording.events, recording.startedAt) + + sessionActions().loadRecording(recording, checkpoints) + } catch (cause) { + // `file.name` rather than the derived id: this names the file that failed, and a person who has to + // go and look at it on disk needs the name they will actually see in a folder. + setError({ + id: file.name, + message: cause instanceof Error ? cause.message : String(cause), + source: 'file', + }) + } finally { + setLoadingName(null) + } + }, []) + + return { load, loadingName, error } +} diff --git a/traces/src/components/ui/use-sample-loader.ts b/traces/src/components/ui/use-sample-loader.ts index c405a58..93cf701 100644 --- a/traces/src/components/ui/use-sample-loader.ts +++ b/traces/src/components/ui/use-sample-loader.ts @@ -19,19 +19,29 @@ import { sessionActions } from '@/lib/store/session' * feedback has to be local or it would report on a component that no longer exists. */ -/** Which sample failed, and why. `id` so the message can name the file rather than "the recording". */ -export type SampleLoadError = { id: string; message: string } +/** + * Which recording failed, and why. `id` so the message can name the file rather than "the recording". + * + * Shared with `useFileLoader`, which reports the same two facts about a file a person chose, and lives + * here rather than in a third module because this is where the shape and its consumers already were. + * + * `source` exists for one reason: the remedial hint. Both components used to end a failure with *"Samples + * live in `traces/public/recordings/`"*, which is the correct next step for a missing sample and actively + * wrong advice for a file the person picked off their own disk — it sends them to look in our repo for + * their file. A failure has to be able to say which of the two it was, and colour cannot say it. + */ +export type RecordingLoadError = { id: string; message: string; source: 'sample' | 'file' } export type SampleLoader = { load: (sample: SampleRecording) => Promise /** The sample currently in flight, or null. Callers disable every button while this is set. */ loadingId: string | null - error: SampleLoadError | null + error: RecordingLoadError | null } export function useSampleLoader(): SampleLoader { const [loadingId, setLoadingId] = useState(null) - const [error, setError] = useState(null) + const [error, setError] = useState(null) const load = useCallback(async (sample: SampleRecording) => { const url = sampleRecordingUrl(sample.id) @@ -56,7 +66,11 @@ export function useSampleLoader(): SampleLoader { sessionActions().loadRecording(recording, checkpoints) } catch (cause) { - setError({ id: sample.id, message: cause instanceof Error ? cause.message : String(cause) }) + setError({ + id: sample.id, + message: cause instanceof Error ? cause.message : String(cause), + source: 'sample', + }) } finally { setLoadingId(null) } From 7f7df91899c49ab6cca37949632625a512b5c69a Mon Sep 17 00:00:00 2001 From: ribdsp <113304041+ribdsp@users.noreply.github.com> Date: Sun, 30 Aug 2026 06:26:14 +0700 Subject: [PATCH 2/4] feat: offer the file loader from the picker and the empty stage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two entry points onto `useFileLoader`, plus the drop hazard that exists whether or not a drop target does. The picker grows a fourth listbox row. It has to live inside the `
    `: the panel's key handler closes on Tab, so a control after the list would be unreachable by keyboard. That also forced one source of index truth — `activeIndex`, `aria-activedescendant`, the scroll effect and the End bound each indexed `SAMPLE_RECORDINGS` separately, and index 3 was `undefined`, which would have pointed `aria-activedescendant` at no element. They now derive from one `ROWS` array. The empty stage grows a matching row and is itself the drop target, since someone arriving with a file they just recorded has no reason to guess the header holds a menu. Dropping a file on the window navigates the tab to it by default, discarding every marker and hypothesis in the session. `useFileLoader` cancels that at the window level, so a missed drop does nothing. Each component tracks which of the two loaders it last used. Without it a failed file left its alert on screen next to a sample that had since loaded fine, because each hook only clears its own error. The remedial hint branches on the same fact: pointing someone at this repository to find their own file is the wrong next step. --- .../components/player/stage-empty-state.tsx | 138 ++++++++++- traces/src/components/ui/recording-picker.tsx | 217 +++++++++++++++--- 2 files changed, 316 insertions(+), 39 deletions(-) diff --git a/traces/src/components/player/stage-empty-state.tsx b/traces/src/components/player/stage-empty-state.tsx index 6b3fe72..e40bd5b 100644 --- a/traces/src/components/player/stage-empty-state.tsx +++ b/traces/src/components/player/stage-empty-state.tsx @@ -1,7 +1,9 @@ 'use client' -import { CirclePlay, Layers, Plug, TriangleAlert } from 'lucide-react' +import { CirclePlay, FolderOpen, Layers, Plug, TriangleAlert } from 'lucide-react' +import { useRef, useState } from 'react' import { SAMPLE_RECORDINGS } from '@/components/ui/sample-recordings' +import { useFileLoader } from '@/components/ui/use-file-loader' import { useSampleLoader } from '@/components/ui/use-sample-loader' /** @@ -17,6 +19,10 @@ import { useSampleLoader } from '@/components/ui/use-sample-loader' * 3. how to get WebMCP at all, since on a browser without it nothing below is callable; * 4. a recording, in one click. Prose that ends without an action gets read once and then closed. * + * The recording can now be the reader's own, which is why this panel is also the drop target: someone + * arriving from the README with a file they just recorded has no reason to guess that the header holds a + * menu, and the largest empty rectangle on screen is the one they will aim at. + * * It also holds the long description that used to truncate in the header, and that is the right home for * it: it is onboarding, so it is wanted exactly when there is no recording and in the way once there is. * The header keeps a short standing subtitle — see `page.tsx`. @@ -29,10 +35,51 @@ import { useSampleLoader } from '@/components/ui/use-sample-loader' * reports. */ export function StageEmptyState() { - const { load, loadingId, error } = useSampleLoader() + const { load: loadSample, loadingId, error: sampleError } = useSampleLoader() + const { load: loadFile, loadingName, error: fileError } = useFileLoader() + + /** See the picker for why the last attempt has to be tracked: two hooks, one alert. */ + const [lastAttempt, setLastAttempt] = useState<'sample' | 'file' | null>(null) + /** A file is over the panel. Changes the control's border *and* its wording — see below. */ + const [over, setOver] = useState(false) + + const fileInputRef = useRef(null) + + const busy = loadingId !== null || loadingName !== null + const error = lastAttempt === 'file' ? fileError : sampleError + + const takeFile = (file: File | undefined) => { + if (!file) return + setLastAttempt('file') + void loadFile(file) + } return ( -
    +
    { + event.preventDefault() + setOver(true) + }} + onDragLeave={(event) => { + /* `dragleave` also fires crossing into a child. Only a `relatedTarget` outside this element is + the cursor actually leaving the panel; without the check the affordance strobes on every row + boundary the cursor passes. */ + const leaving = event.relatedTarget + if (!(leaving instanceof Node) || !event.currentTarget.contains(leaving)) setOver(false) + }} + onDrop={(event) => { + event.preventDefault() + setOver(false) + takeFile(event.dataTransfer.files[0]) + }} + className="max-h-full w-full max-w-xl overflow-y-auto px-4 py-1 text-body leading-relaxed" + >

    Session replay an AI agent can interrogate

    @@ -58,7 +105,9 @@ export function StageEmptyState() { Load a recording -

    Three samples. One click each.

    +

    + Three samples, one click each — or a recording of your own. +

      {SAMPLE_RECORDINGS.map((sample) => ( @@ -69,8 +118,11 @@ export function StageEmptyState() { */} ))} + + {/* + Same row shape as the three above, prose title instead of a monospace stem so it does not + read as a fourth sample. The border change while a file is over the panel is not the whole + signal — the second line changes with it, because a dashed outline is a colour-and-shape cue + and one of those is not a state. + */} +
    • + +
    + {/* Reset before loading so the same file can be chosen twice running; see the picker's copy of + this, which explains what the second silent pick would otherwise look like. */} + { + const file = event.target.files?.[0] + event.target.value = '' + takeFile(file) + }} + /> + {error ? (

    - {error.id} did not load: {error.message}. Samples live in{' '} - traces/public/recordings/. + {/* Conditional period: `loadRecording` throws whole sentences, a `SyntaxError` and an HTTP + status do not, and both are quoted verbatim. See the picker's copy. */} + {error.id} did not load: {error.message} + {error.message.endsWith('.') ? '' : '.'}{' '} + {/* A missing sample and a rejected file need opposite next steps, and pointing someone at + this repository to find their own file is the wrong one. */} + {error.source === 'sample' ? ( + <> + Samples live in traces/public/recordings/. + + ) : ( + <> + Traces reads rrweb JSON: an event array, or the{' '} + {'{ events: … }'} wrapper a downloaded recording + has. + + )}

    ) : null} @@ -141,4 +261,4 @@ export function StageEmptyState() {
    ) -} \ No newline at end of file +} diff --git a/traces/src/components/ui/recording-picker.tsx b/traces/src/components/ui/recording-picker.tsx index 64f548a..a93c4dc 100644 --- a/traces/src/components/ui/recording-picker.tsx +++ b/traces/src/components/ui/recording-picker.tsx @@ -1,14 +1,16 @@ 'use client' -import { Check, ChevronDown, ChevronUp, TriangleAlert } from 'lucide-react' +import { Check, ChevronDown, ChevronUp, FolderOpen, TriangleAlert } from 'lucide-react' import { useEffect, useRef, useState } from 'react' import { SAMPLE_RECORDINGS, type SampleRecording } from '@/components/ui/sample-recordings' import { formatSeconds } from '@/components/ui/format-time' +import { useFileLoader } from '@/components/ui/use-file-loader' import { useSampleLoader } from '@/components/ui/use-sample-loader' import { useSessionStore } from '@/lib/store/session' /** - * Loads a sample recording. The only control on the page that has to work before anything else does. + * Loads a recording — one of the three samples, or a file off the reader's own disk. The only control on + * the page that has to work before anything else does. * * All three states are real here, and none of them is theoretical: * @@ -19,10 +21,13 @@ import { useSessionStore } from '@/lib/store/session' * the next person debugs the player instead of the missing file. * loaded — the trigger *is* the answer to "which one is this", so the header stops needing a legend. * - * The fetch itself lives in `useSampleLoader`, shared with the empty state's one-click load. + * Neither load lives here: `useSampleLoader` fetches a sample, `useFileLoader` reads a chosen file, and + * both are shared with the empty state, which offers the same two things to a reader who has not found + * the header yet. * * The labels are the file stems rather than prose, deliberately: they are the same ids the agent sees in - * `read_session_meta`, so a human reading over the agent's shoulder does not have to translate. + * `read_session_meta`, so a human reading over the agent's shoulder does not have to translate. A loaded + * file gets the same treatment — its id is derived from its name, and the trigger shows that id. * * Why a dropdown rather than the three bare buttons this used to be: three toggles of equal weight said * nothing about which was open, cost the width of all three ids in a header that has to survive 720px, @@ -46,20 +51,73 @@ function optionId(id: string): string { return `recording-option-${id}` } +/** + * The file row's key, in the same namespace as the sample stems because `optionId` hands both to one + * `document.getElementById`. That it collides with none of them is checkable by reading + * `sample-recordings.ts`, which is three entries long. + */ +const FILE_ROW_KEY = 'load-a-file' + +type Row = + | { readonly kind: 'sample'; readonly key: string; readonly sample: SampleRecording } + | { readonly kind: 'file'; readonly key: string } + +/** + * Every row in the listbox, in order, and the only thing that knows how many there are. + * + * One array rather than "the samples, and also the file one": `activeIndex`, `aria-activedescendant`, + * the scroll-into-view effect and the arrow-key bounds are four things that must agree about what row 3 + * is, and they agreed only by coincidence when each indexed `SAMPLE_RECORDINGS` separately. An index + * that runs one past that array does not throw under `noUncheckedIndexedAccess`; it yields `undefined`, + * and the visible symptom is `aria-activedescendant` pointing at an element that does not exist, which + * nothing on screen shows and only a screen reader reports. + * + * **The file row is an option inside the list, not a control after it, and that is load-bearing.** This + * panel's key handler closes on `Tab` — see `onListKeyDown` — so a button placed after the `
      ` could + * never be reached by keyboard: the keystroke meant to move onto it unmounts it first. Anything the + * panel offers has to be a row. + * + * Module scope because it is genuinely constant: `SAMPLE_RECORDINGS` is a module constant too, so + * rebuilding this per render would only give the effects below a new dependency identity every time. + */ +const ROWS: readonly Row[] = [ + ...SAMPLE_RECORDINGS.map((sample): Row => ({ kind: 'sample', key: sample.id, sample })), + { kind: 'file', key: FILE_ROW_KEY }, +] + +const LAST_ROW = ROWS.length - 1 + export function RecordingPicker() { const recording = useSessionStore((s) => s.recording) - const { load, loadingId, error } = useSampleLoader() + const { load: loadSample, loadingId, error: sampleError } = useSampleLoader() + const { load: loadFile, loadingName, error: fileError } = useFileLoader() const [open, setOpen] = useState(false) /** Which row the arrows are on. Separate from the selection: moving is not choosing. */ const [activeIndex, setActiveIndex] = useState(0) + /** + * Which of the two loaders was asked last, so the alert shows that one's failure and not the other's. + * + * Each hook clears its own error when it starts, which is all a hook can do and not enough for a + * component holding two of them: a file that failed, followed by a sample that loaded fine, would + * otherwise leave the file's alert on screen beside a recording that is open and working. An alert + * that outlives its failure is worse than no alert, because the next thing the reader distrusts is the + * recording. + */ + const [lastAttempt, setLastAttempt] = useState<'sample' | 'file' | null>(null) const triggerRef = useRef(null) const listRef = useRef(null) const wrapRef = useRef(null) + const fileInputRef = useRef(null) const openId = recording?.id ?? null - const openIndex = SAMPLE_RECORDINGS.findIndex((sample) => sample.id === openId) + /** + * -1 once a file is loaded, since its derived id is in no sample's row, and the tick correctly goes + * nowhere. A file named after a sample is the one case that ticks a sample row, which is not a lie: + * the recording's id *is* that string, and it is the string the trigger and `read_session_meta` show. + */ + const openIndex = ROWS.findIndex((row) => row.kind === 'sample' && row.sample.id === openId) /** Opening lands on the current recording, or the top of the list when nothing is loaded yet. */ const show = () => { @@ -72,9 +130,19 @@ export function RecordingPicker() { if (returnFocus) triggerRef.current?.focus() } - const choose = (sample: SampleRecording) => { + const choose = (row: Row) => { hide(true) - void load(sample) + + if (row.kind === 'file') { + /* Opening the picker is the whole action; the load starts in the input's `change`, whenever the + person gets round to it. Closing first is deliberate — the native dialog is modal, and a panel + left open behind it is still there on cancel. */ + fileInputRef.current?.click() + return + } + + setLastAttempt('sample') + void loadSample(row.sample) } /* Focus the panel itself and drive it with `aria-activedescendant`, rather than moving DOM focus @@ -99,21 +167,19 @@ export function RecordingPicker() { /** Keep the arrow-selected row visible in a panel that scrolls at narrow heights. */ useEffect(() => { if (!open) return - const row = document.getElementById(optionId(SAMPLE_RECORDINGS[activeIndex]?.id ?? '')) - row?.scrollIntoView({ block: 'nearest' }) + const key = ROWS[activeIndex]?.key + if (key) document.getElementById(optionId(key))?.scrollIntoView({ block: 'nearest' }) }, [open, activeIndex]) const onListKeyDown = (event: React.KeyboardEvent) => { - const last = SAMPLE_RECORDINGS.length - 1 - switch (event.key) { case 'ArrowDown': event.preventDefault() - setActiveIndex((index) => (index >= last ? 0 : index + 1)) + setActiveIndex((index) => (index >= LAST_ROW ? 0 : index + 1)) return case 'ArrowUp': event.preventDefault() - setActiveIndex((index) => (index <= 0 ? last : index - 1)) + setActiveIndex((index) => (index <= 0 ? LAST_ROW : index - 1)) return case 'Home': event.preventDefault() @@ -121,15 +187,15 @@ export function RecordingPicker() { return case 'End': event.preventDefault() - setActiveIndex(last) + setActiveIndex(LAST_ROW) return case 'Enter': case ' ': { event.preventDefault() // `noUncheckedIndexedAccess`: `activeIndex` is only ever set from this list's own bounds, but // the compiler cannot know that and a silent no-op is the right answer if it is ever wrong. - const sample = SAMPLE_RECORDINGS[activeIndex] - if (sample) choose(sample) + const row = ROWS[activeIndex] + if (row) choose(row) return } case 'Escape': @@ -137,7 +203,8 @@ export function RecordingPicker() { hide(true) return case 'Tab': - // Let focus leave normally, but do not leave an orphaned panel open behind it. + // Let focus leave normally, but do not leave an orphaned panel open behind it. This is the line + // the file row has to be inside the list to survive — see the note on `ROWS`. setOpen(false) return default: @@ -145,7 +212,10 @@ export function RecordingPicker() { } } - const loading = loadingId !== null + const activeKey = ROWS[activeIndex]?.key + const loading = loadingId !== null || loadingName !== null + /* `lastAttempt` is null only before either loader has run, when `sampleError` is null as well. */ + const error = lastAttempt === 'file' ? fileError : sampleError return (
      @@ -168,8 +238,10 @@ export function RecordingPicker() { className="flex min-w-0 max-w-[14rem] items-center gap-1.5 rounded-sm border border-line-strong bg-raised px-1.5 py-0.5 shadow-raised hover:border-faint" > rec + {/* A file in flight shows by name rather than by derived id: the name is what the person just + picked out of a folder, and until it parses there is nothing else honest to call it. */} - {loadingId ?? openId ?? 'none loaded'} + {loadingId ?? loadingName ?? openId ?? 'none loaded'} {loading ? ( /* @@ -191,13 +263,42 @@ export function RecordingPicker() { )} + {/* + Outside the panel's conditional on purpose. `choose` closes the panel and clicks this in the same + breath, and the `change` event arrives long after — as late as the person takes to find the file. + Rendered inside the `
        ` it would be unmounted before either happened, and the chosen file + would go nowhere with nothing on screen to say so. + + `hidden` rather than a styled control: a native file input cannot be restyled to match anything, + and the row above is already the label, the target and the keyboard route. Programmatic `.click()` + on a `display: none` input opens the dialog in every browser this app runs in. + */} + { + const file = event.target.files?.[0] + /* Cleared before the load, so choosing the same file twice fires `change` twice. Left set, a + second identical pick is silently nothing — the exact failure this component is written to + never have. */ + event.target.value = '' + if (!file) return + setLastAttempt('file') + void loadFile(file) + }} + /> + {open ? (
          - {SAMPLE_RECORDINGS.map((sample, index) => { - const isOpen = sample.id === openId + {ROWS.map((row, index) => { const isActive = index === activeIndex + if (row.kind === 'file') { + return ( +
        • choose(row)} + onMouseEnter={() => setActiveIndex(index)} + /* A rule above it, and prose instead of a monospace stem, so four rows do not read as + four samples. Padding written out rather than `py-1.5` plus an override, so the + extra room above the rule does not depend on which utility Tailwind emits last. */ + className={`mt-1 cursor-pointer rounded-sm border-t border-line px-2 pb-1.5 pt-2 ${ + isActive ? 'bg-panel' : '' + }`} + > +

          + + Load a file… +

          +

          + An rrweb JSON file from your own app. Nothing is uploaded. +

          +
          • + ) + } + + const { sample } = row + const isOpen = sample.id === openId + return (
        • choose(sample)} + onClick={() => choose(row)} onMouseEnter={() => setActiveIndex(index)} className={`cursor-pointer rounded-sm px-2 py-1.5 ${ isActive ? 'bg-panel' : '' @@ -280,10 +418,29 @@ export function RecordingPicker() { > - {error.id} did not load: {error.message}.{' '} + {/* The period is conditional because the two sources punctuate differently and both are + quoted verbatim: `loadRecording` throws whole sentences ending in one, while a + `SyntaxError` and an HTTP status do not. Appending unconditionally gave "no events..", + and not appending ran the reason straight into the hint. */} + {error.id} did not load: {error.message} + {error.message.endsWith('.') ? '' : '.'}{' '} + {/* The hint has to branch. Sending someone to `traces/public/recordings/` is the right next + step for a sample that is not there, and nonsense for a file they picked off their own + disk — it tells them to look for their file inside this repository. */} - Samples live in traces/public/recordings/ — record one - against bugbait if it is not there yet. + {error.source === 'sample' ? ( + <> + Samples live in traces/public/recordings/ — record + one against bugbait if it is not there yet. + + ) : ( + <> + Traces reads rrweb JSON: the event array{' '} + record collects, or the{' '} + {'{ events: … }'} wrapper a downloaded recording + has. + + )}

          From 6f63132687e06a1a3eecfbd32845f7bbe91acb69 Mon Sep 17 00:00:00 2001 From: ribdsp <113304041+ribdsp@users.noreply.github.com> Date: Sun, 30 Aug 2026 06:26:27 +0700 Subject: [PATCH 3/4] docs: say how to load a file, and how to record one worth loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Drop the downloaded JSON into Traces" described nothing that existed. It now names where the control is. Adds what `npm i rrweb` plus `record()` leaves out, verified against bugbait/src/lib/record.ts rather than assumed: the checkout interval, the fetch patch, the console-event normalisation, the user-agent stamp, and `maskAllInputs`, which is not a preference — without it the recording holds the password and the card number in plain text. Also the gap: a fetch patch sees fetch only, so XMLHttpRequest traffic is invisible to read_network and shows as an empty timeline rather than an error. Every fixture here uses fetch, so none of them expose it. Cost is quoted from the three sample files rather than estimated, and the sandbox claim about replaying someone else's recording is the one already recorded in replay-engine.ts from a spike. --- README.md | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index dc3229a..bb56690 100644 --- a/README.md +++ b/README.md @@ -307,8 +307,54 @@ reproduce the bug, and download the recording: | `localhost:3001/checkout?bug=race` | the dropdown renders before its data arrives and never re-renders | | `localhost:3001/checkout?bug=overlay` | the pay button is present and enabled but covered, so clicks never land | -Drop the downloaded JSON into Traces. **Keep recordings of real users out of version control** — see -[T5](docs/threat-model.md#t5--a-real-user-session-ending-up-in-a-public-repository). +The download lands as `.session.json`. To open it, use **Load a file…** — the last row of the +recording menu in the header, and a matching row under the three samples on the empty stage. You can +also drop the file anywhere on the empty stage. Nothing is uploaded, because there is nowhere to upload +to: the file is read in the tab and parsed into memory. The name becomes the recording's label, and a +slug of it becomes its id. + +**Keep recordings of real users out of version control** — see +[T5](docs/threat-model.md#t5--a-real-user-session-ending-up-in-a-public-repository). Loading a file +never writes to `traces/public/recordings/`, so a recording you open stays out of the repository unless +you put it there yourself. + +### Recording your own app + +`npm i rrweb` and calling `record()` produces a file Traces will load and then serve badly. Five +options do the actual work, and `bugbait/src/lib/record.ts` is the reference implementation: + +| Add this | What silently breaks without it | +|---|---| +| `checkoutEveryNms: 5000` | rrweb emits **one** full snapshot, at the start. The checkpoint index has a single entry, so every seek replays from the beginning — and `bisect`, which seeks repeatedly by design, pays that cost on each step. Nothing errors; the player just crawls | +| `maskAllInputs: true` | every keystroke is recorded verbatim. See below — this one is not a preference | +| a `window.fetch` patch emitting `addCustomEvent('network-request', …)` | `read_network` returns nothing. rrweb records DOM mutations, not requests; the network panel of a recording is whatever you put there yourself | +| normalising console events to `{ type: 3, source: 11 }` | the console plugin emits `{ type: 6, data: { plugin: 'rrweb/console@1' } }`, which nothing downstream reads. `consoleErrors` stays `0` on a session full of errors — the most misleading of the five, because zero looks like an answer | +| stamping `userAgent` onto the first Meta event | the session summary cannot say what browser it was | + +**`maskAllInputs: true` is mandatory, not a preference.** rrweb records input values by default, so a +recorder without it is a credential logger: the password, the card number and the one-time code are all +in the JSON, in plain text, for anyone the file is later shared with. Traces truncates a `value` to 20 +characters when a tool reads one, which limits what an agent sees and does nothing about what the file +contains. Mask at the recorder or accept that the recording is a secret. + +**A gap worth knowing before you trust `read_network`:** a `fetch` patch sees `fetch` only. +`XMLHttpRequest` traffic — anything on axios's default adapter, jQuery, or an older SDK — is invisible +to it, and a recording of such an app will show an empty network timeline rather than an error. Every +fixture here uses `fetch`, so none of them expose this. + +**The cost, measured** from the three files in `traces/public/recordings/`: 184–213 KiB for ~45 seconds +of a small checkout page, or roughly 4–5 KiB per second, at 209–223 events. A recording is JSON and +compresses well in transit; in memory it is the whole array. A ten-minute session of a heavier app is +plausibly tens of megabytes, which is what the 64 MB ceiling on a loaded file is sized for. + +### Opening a recording you didn't make + +Worth stating, since the feature above invites it. Scripts inside a recording **cannot execute**: the +replay iframe is sandboxed with `allow-same-origin` and not `allow-scripts`, so a `