# MPCkit review fixes: work plan

Written 2026-11-12; PR 8b added 2026-11-13; PRs 10-12 (readability) added 2026-11-13. Ten PRs (1-9 and 8b), worked one at a time, fix the findings of the MPCkit source code review of `e32802a` (v0.5.1, 2026-11-12). PRs 1-4 ship the security fixes as **0.5.2**; PRs 5-9, including 8b, ship the hardening work as **0.5.3**. Finding IDs (S1, M3, R1 and so on) refer to that review. Each PR below repeats what an agent needs from it, so the plan works without access to the review.

PRs 1-9 and 8b are done: 0.5.2 and 0.5.3 are released. PRs 10-12 tidy the code for human review (R2-R7). They change no behaviour and no version.

## Decisions

These were settled with the user on 2026-11-12. Agents follow them and do not reopen them.

| Topic | Decision |
| --- | --- |
| Decompression limit (S2) | 256 MB of decompressed project data (lowered from 512 MB on 2026-11-12, during PR 2: a blank project is 3 MB and 256 MB holds about half a million recorded events). |
| Bound paths (S4) | Approved: `LoadPath` and `AssignSamplePath` stop being bound; export methods stop taking a path. |
| Pad labels (M6) | Always the "A1" format (bank letter, pad number without a leading zero). |
| Releases | 0.5.2 after PRs 1-3, 0.5.3 after PRs 5-8 and 8b. PRs in between do not change the version. |
| CI (S6) | Out of scope. No CI files, no `task check`. |
| PR records (R8) | Keep the current layout. R8 is dropped. |
| Formatter (R2) | Approved: add `prettier` and `prettier-plugin-svelte` as dev dependencies and reformat the whole frontend in one PR (PR 11). Decided 2026-11-13. |
| Readability PRs (10-12) | No version bump and no release of their own; they ship in whichever release comes next. Decided 2026-11-13. |
| S5, M9, M10, M11 | Skipped for now (2026-11-13). They stay in the Backlog and need decisions before they can be planned. |
| Linked WAVs (PR 3) | A WAV the user picks that is a link is followed at assign: the pool file is named after the link, and its source is the file the link points to. Export refuses any source that is a link or sits in a linked folder. Decided 2026-11-12. |
| New sample names (PR 3) | Besides `/`, `\` and control characters, the characters the device's FAT card forbids (`: * ? " < > \|`) also become `_`, and a name Windows reserves for a device (`CON.wav`, `aux.wav`, `com1.wav`) gets `_` added (`CON_.wav`). Decided 2026-11-12 and 2026-11-13. |

Open: none. An agent that meets a choice this plan does not settle stops and asks bajdev in the PR record's Notes, instead of guessing.

## Guidelines for agents

These apply to every PR. Where they conflict with a PR's tasks, ask before going on.

### Branches and PRs

1. Work on one PR at a time, in the order below. Start each branch from the latest `origin/main` after the previous PR is merged.
2. Name the branch exactly as the plan does. Never push to `main`: it is protected on Gogs.
3. Push with `git push -u origin <branch>`, then give bajdev the compare link: `https://git.monoexp.com/bajdev/mpckit/compare/main...<branch>`. Agents cannot open PRs; bajdev opens and merges them.
4. Before adding commits to a pushed branch, check it is not merged yet (`git fetch --prune`, `git branch -r --merged origin/main`).
5. After bajdev says a PR is merged: `git fetch --prune`, delete the branch on Gogs by hand only if it is fully in `origin/main` (`git push origin --delete <branch>`), then `git branch -d <branch>`. MPCkit is not on the deploy pipeline, so nothing deletes it automatically.
6. End every commit message with the attribution line the session gives. Apart from that line, never put a model name in commits, code or PR records.

### PR records

Every PR adds `pull-requests/<branch>/<pr-summary>.md`, like the existing ones (see `pull-requests/mpckit-casing/`):

- First line: `# <PR title>`, then ``Branch: `<branch>` → `main` ``.
- **What changed**, in plain words, naming the finding IDs fixed.
- **Notes**: decisions made along the way, anything left for later, questions for bajdev.
- **Testing**: what ran on the server, then numbered steps for bajdev's Windows laptop (cmd or PowerShell commands, not `sh`).

### Checks before every push

The VPS has Node 24, the GTK 4 and WebKitGTK 6 build files and Xvfb, so all of these run there:

```
export PATH=$PATH:~/go/bin
export GOTOOLCHAIN=go1.27.4   # openSUSE's Go defaults to local and ignores go.mod's toolchain line
go test ./...
go vet ./...
staticcheck ./...
govulncheck ./...
(cd frontend && npm ci && npm run check && npm run build)
(cd frontend && npm run format:check)   # from PR 11 on
wails3 build
```

`npm ci` needs the lockfile from PR 8; until then use `npm install` and do not commit the lockfile it writes. `govulncheck` will keep reporting GO-2026-6859 until Go 1.27.4 is installed; mention it in Testing, do not try to work around it. To check that the app starts, run `timeout 15 xvfb-run -a bin/mpckit` and read its log.

### Code

- Match the code around the change: short functions, comments that say why, plain error messages. New identifiers follow the existing naming.
- Every bug fix comes with a test that fails before the fix. Build crafted project files inside the test (decode a file from `testdata/device`, change the tree, write it to `t.TempDir()`), rather than committing new binary fixtures.
- Do not change: `testdata/device/*` and `testdata/accepted/*`; the byte-identical round trip of `.xpj` files (the `xpj` tests prove it); the edit recipes in `docs/xpj-format-and-scope.md`; device limits in `project`.
- No new Go modules or npm packages without asking bajdev.
- After changing a bound method in `internal/app`, regenerate the bindings with `wails3 task common:generate:bindings` and commit `frontend/bindings`.
- Svelte 5: props are live, so a handler that clears the state behind a prop reads the prop first. Wails types Go slices as `T[] | null`; normalise with `?? []`.
- The app is "MPCkit" wherever people see it; file names, the module path and `mpckit-cli` stay lowercase. Keep the "not affiliated with Akai Professional" notes.

### Releases (PRs 4 and 9)

The README's "Releasing" paragraph lists the files that carry the version; change all of them. After bajdev merges, tag the merge commit with an annotated tag: `git tag -a v0.5.2 -m "MPCkit 0.5.2 (beta)" <merge commit>`, then `git push origin v0.5.2`.

## PR order

```
0.5.2: security fixes
  PR 1 harden-load  →  PR 2 limit-decompression  →  PR 3 safe-export  →  PR 4 release-0-5-2 (tag v0.5.2)
0.5.3: hardening
  PR 5 unbind-paths  →  PR 6 atomic-edits  →  PR 7 a1-labels  →  PR 8 build-cleanup  →  PR 8b bound-surface  →  PR 9 release-0-5-3 (tag v0.5.3)
Readability (no release)
  PR 10 go-readability  →  PR 11 frontend-format  →  PR 12 shared-modal
```

Each PR starts from `main` after the previous one is merged. The order matters in four places: PR 3 reuses PR 1's file name check, PR 6 builds on the nodes PR 1 stores on `Project`, PRs 5 and 7 both change `internal/app` and the frontend, and PR 8b finishes what PR 5 started and must be merged before PR 9. PR 12 starts after PR 11, so the new modal is written in the Prettier style.

## PRs and tasks

| # | Branch | Fixes | Release |
| --- | --- | --- | --- |
| 1 | `harden-load` | S1 (load side), S3, M2 | 0.5.2 |
| 2 | `limit-decompression` | S2 | 0.5.2 |
| 3 | `safe-export` | S1 (export side and assign), M1, M5, R1 (part) | 0.5.2 |
| 4 | `release-0-5-2` | Version 0.5.2 | 0.5.2 |
| 5 | `unbind-paths` | S4 | 0.5.3 |
| 6 | `atomic-edits` | M3, M4 | 0.5.3 |
| 7 | `a1-labels` | M6 | 0.5.3 |
| 8 | `build-cleanup` | S6 (without CI), M7, M8, R1 (rest) | 0.5.3 |
| 8b | `bound-surface` | S4 (rest) | 0.5.3 |
| 9 | `release-0-5-3` | Version 0.5.3 | 0.5.3 |
| 10 | `go-readability` | R3 (Go), R4, R5, R6 | none |
| 11 | `frontend-format` | R2, R3 (Svelte) | none |
| 12 | `shared-modal` | R7 | none |

### PR 1. Reject malformed projects at load (`harden-load`)

**Why.** Pool file names come from the project file and are joined to folders as they are, so `../../../secret.txt` reads and writes outside the project (S1). Missing fields cause nil-pointer panics: a pool entry without `path` in `project.Load`, a sequence without `bpm` in `Sequences()`. In the app, a panic in the file-drop handler exits MPCkit (S3). The project's main nodes are looked up again on every call with the error dropped (M2).

**Tasks**

- [ ] In `project.wrap`, resolve `samples`, `tracks[0].samples`, `tracks[0].program.drum.instruments` and `sequences` once and keep them on `Project`; `pool()`, `trackPool()`, `pads()` and `sequences()` return the stored nodes.
- [ ] In `wrap` or `Load`, check every pool entry has a string `path`, and every pad layer's `sampleFile` is a string or missing. Each name must be a plain file name: not empty, not `.` or `..`, no `/`, `\` or control characters, and `filepath.Base(name) == name`. Refuse the file otherwise, naming the bad entry.
- [ ] Check each sequence has a numeric `key`, an object `value`, a number `bpm` and `lengthBars`, and a string `name`. Refuse the file with a clear error otherwise.
- [ ] Make `xpj.Node.Int`, `Float` and `Lookup` safe on a nil node (return an error or nil), and replace remaining direct `.Text`, `.Int()` and `.Float()` uses on lookups in `project` with `text()`, `num()` or a checked equivalent. This covers `data.version` in `Load`.
- [ ] In `main.go`, set `PanicHandler` in `application.Options`: log the panic and emit the `error` event with a short message, so a missed case cannot close the app.
- [ ] Add `FuzzLoad` in `internal/project`: seed it with the files in `testdata/device`, write each fuzzed input to `t.TempDir()`, load it and call `Pad`, `Sequences` and `PoolFiles`; none may panic.

**Done when**

- Tests show that loading a project with a pool `path` of `../x.wav`, `a/b.wav` or `..` fails, and so does a project missing `path` or `bpm`, each with an error naming the problem.
- `go test -fuzz=FuzzLoad -fuzztime=2m ./internal/project` finds nothing.
- All existing tests pass unchanged, including the round-trip tests.

### PR 2. Cap decompressed size (`limit-decompression`)

**Why.** `xpj.Read` reads the whole gzip stream into memory. A 1 MB file padded with spaces used 3.1 GB and 10 seconds (S2).

**Tasks**

- [ ] Add `const MaxDecompressed = 256 << 20` in `internal/xpj` with a comment giving the reason.
- [ ] In `Read`, read through `io.LimitReader(zr, MaxDecompressed+1)` and return `xpj: project is larger than 256 MB when decompressed` past the limit.

**Done when**

- A test writes a gzip stream that decompresses to just over the limit and gets the error. Build the stream in the test with `gzip.NewWriter`; do not commit it.
- A file at the limit still decodes.

### PR 3. Safe export (`safe-export`)

**Why.** Export copies each pool file from its source and writes it into the data folder by name (S1, export side). It follows symlinks, so a link in a shared kit's data folder exports whatever it points to. With overwrite set, `Folder` deletes the old export before moving the new one in; if a rename fails, both are lost, although the comment promises otherwise (M1, R1). The `export` package has the lowest coverage, 74.2%, mostly on these paths (M5). Found after PR 1: `AssignSample` names a new pool file after the WAV's own file name, so on Linux or macOS a file called `kick\one.wav` is accepted and exported, and loading that export then fails PR 1's check. On Windows and on the device's card the backslash would also read as a folder separator.

**Tasks**

- [ ] In `collect`, use `os.Lstat` and refuse a source that is a symlink or not a regular file; report it like a missing file.
- [ ] In `Folder` and `writeZip`, refuse a pool name that is not a plain file name (same rule as PR 1), as a second line of defence.
- [ ] In `AssignSample`, make the new pool file name pass `PlainFileName` before `uniqueFile`: replace `/`, `\` and control characters with `_`; if the result is empty, `.` or `..`, use `sample.wav`. The sample name (`stem`) follows from the cleaned file name. Do this in one helper in `project` with a comment giving the reason.
- [ ] Rewrite the overwrite step in `Folder`: rename the old `.xpj` and data folder to temporary names inside the temp folder, rename the new pair into place, then delete the old pair. If the second rename fails, put the old pair back.
- [ ] If `dest` does not exist, return `export folder does not exist: <dest>` instead of the bare `stat` error.
- [ ] Correct the `Folder` comment to say what the code now guarantees.
- [ ] Add tests for: a symlinked source, a bad pool name, overwrite when the new rename fails (make the destination name unusable, for example a directory where the `.xpj` should go), and a missing destination.

**Done when**

- Assigning a WAV named `kick\one.wav` (create it in the test; Linux allows the backslash) gives the pool name `kick_one.wav`, and the exported project loads again without error.
- Export of a project whose data folder holds a symlink fails and writes nothing.
- An overwrite that fails leaves the previous export complete.
- `go test -cover ./internal/export` shows at least 85%.

### PR 4. Release 0.5.2 (`release-0-5-2`)

**Tasks**

- [ ] Change the version to 0.5.2 in every file the README's "Releasing" paragraph lists, and update the stale example in the `Version` comment in `internal/app/app.go`.
- [ ] Add `toolchain go1.27.4` to `go.mod` so builds pick up the `net/http` fix (GO-2026-6859). If Go 1.27.4 is not yet released, stop and ask bajdev.
- [ ] The PR record lists what 0.5.2 fixes, for the beta testers.

**Done when** the laptop build shows "Version 0.5.2" in About and `govulncheck ./...` reports no vulnerabilities. After the merge, tag `v0.5.2` ("MPCkit 0.5.2 (beta)").

### PR 5. Stop exposing raw paths to the webview (`unbind-paths`)

**Why.** Any script in the webview can call `LoadPath`, `AssignSamplePath`, `ExportFolder` and `ExportZip` with any path; `ExportFolder` with overwrite set deletes a folder (S4). The frontend never calls the first two.

**Tasks**

- [ ] Mark `LoadPath` and `AssignSamplePath` with `//wails:ignore`. `main.go`'s file-drop handler still calls `AssignSamplePath` from Go.
- [ ] Have `ChooseExportFolder` and `ChooseZipPath` store the chosen path on the `Service`; `ExportFolder(overwrite)` and `ExportZip(overwrite)` use it and fail if none was chosen. Clear it after a successful export and on `New` and `Load`.
- [ ] Update `frontend/src/lib/store.svelte.ts` (`exportWith`) for the new signatures; regenerate the bindings.
- [ ] Update `internal/app` tests.

**Done when** the generated `service.ts` has no method taking a path, export with and without overwrite still works on the laptop, and `npm run check` is clean.

### PR 6. All-or-nothing edits (`atomic-edits`)

**Why.** `AssignSample` clears the pad and may add a pool entry before steps that can fail, so a failure leaves the pad empty. `SetTune` and `SetMuteGroup` set `Dirty` even when the write failed (M3). `app` writes `p.Dirty` and `export` reads `p.Sources` directly (M4).

**Tasks**

- [ ] In each edit method, do the lookups and checks that can fail first, then change the tree; set `Dirty` only after success.
- [ ] Add `Project.MarkExported()` and `Project.Source(file) (string, bool)`; make `Dirty` and `Sources` unexported and update `app`, `export`, the CLI and the tools.
- [ ] Tests: a failing `AssignSample` (for example a pad whose `layersv` is empty) leaves the pad and the pool as they were, and `Dirty` unchanged.

**Done when** those tests pass and no package outside `project` writes project fields.

### PR 7. One pad label format: "A1" (`a1-labels`)

**Why.** `app.Label` and `slots.ts` give "A01"; `project.SlotName` and the CLI use "A1". Decision: "A1" everywhere (M6).

**Tasks**

- [ ] Keep `project.SlotName` as the one Go implementation; remove `app.Label` and use `SlotName`.
- [ ] Change `slotLabel` in `frontend/src/lib/slots.ts` to "A1" and fix the comments that say "A01" (`app.go`, `SlotGrid.svelte`, `slots.ts`).
- [ ] Do not change sequence names such as "Sequence 01": that is the device's naming, not a slot label.
- [ ] Update tests that expect "A01".

**Done when** the pad grid, pad panel, sequence grid, confirm dialogs and the CLI summary all show "A1" style labels on the laptop, and the layout still fits the window with no scrolling.

### PR 8. Build and repository clean-up (`build-cleanup`)

**Why.** No lockfile, so builds can differ (S6). Server-mode and Docker tasks from the Wails template would expose the bound methods over HTTP (S6). The version lives in several files and `Service.Version()` is unused (M7). Template leftovers hide the real build steps (M8). The `wav` package comment says it does not decode audio, but it does (R1).

**Tasks**

- [ ] Commit `frontend/package-lock.json` from `npm install`. Remove `minimum-release-age` from `.npmrc` (npm warns on it) or ask bajdev whether to keep it for pnpm.
- [ ] Remove the `build:server`, `run:server`, `build:docker`, `run:docker` and `setup:docker` tasks and their entries in the root `Taskfile.yml`.
- [ ] Remove the unused blocks in `build/config.yml`: the `ios` comment block, the `fileAssociations` sample and `other: My Other Data`.
- [ ] Remove the bound `Service.Version()` (the frontend uses `About()`); regenerate the bindings.
- [ ] Fix the `wav` package comment.

**Done when** `npm ci` works from the lockfile, `wails3 build` still builds on the server and the laptop, and `wails3 task --list` shows no server or Docker tasks.

### PR 8b. Bind only what the window needs (`bound-surface`)

**Why.** PR 5 hid `LoadPath` and `AssignSamplePath` with `//wails:ignore`, but that directive only keeps them out of the generated `frontend/bindings`. At runtime Wails 3.1.0 registers every exported method of a bound type by reflection (`getMethods` in `pkg/application/bindings.go`), and reflection cannot see comments. So a script in the window can still call `Call.ByName("git.monoexp.com/bajdev/mpckit/internal/app.Service.LoadPath", "<any path>")`; on 2026-11-13 a scratch build did exactly that and Go loaded the file. The same holds for `AssignSamplePath` and the other Go-only methods (`Dirty`, `SetCloser`, `AudioMiddleware`). Also, when the user declines "Replace existing files?", Go keeps the chosen export target, so a later `ExportFolder(true)` would overwrite it without asking.

**Tasks**

- [ ] Keep `Service` as the type `main.go` binds, with only the methods the frontend calls. Move the Go-only methods (`LoadPath`, `AssignSamplePath`, `Dirty`, `SetCloser`, `AudioMiddleware`) to a second, unbound type in `internal/app` that shares the same state, for example `type Host struct{ s *Service }`, with `NewService` returning both. `Service.Load` and `AssignSampleDialog` call unexported helpers that `Host` also uses. Remove the `//wails:ignore` directives that are no longer needed.
- [ ] Update `main.go` to use `Host` for the close hook, the file-drop handler and the audio middleware.
- [ ] Add `TestBoundMethods` in `internal/app`: list the exported methods of `*Service` by reflection and compare them with an explicit list in the test. Its failure message says that a new bound method must not take a file path and must be added to the list on purpose.
- [ ] Add a bound `CancelExport()` that forgets the chosen targets, and call it from `exportWith` in `store.svelte.ts` when the user declines the replace prompt.
- [ ] Regenerate the bindings; update the `internal/app` tests.

**Done when**

- `TestBoundMethods` passes, and fails if a method such as `LoadPath(string)` is added back to `Service`.
- In a scratch build (temporary lines in `frontend/src/main.ts`, not committed), `Call.ByName(".../app.Service.LoadPath", path)` is rejected with "unknown bound method name". Say in the PR record how this was checked.
- On the laptop: Load, file drop onto a pad, the window-close prompt, export to folder and zip (with and without replace, and declining replace), and sample preview all work as before.

### PR 9. Release 0.5.3 (`release-0-5-3`)

Same steps as PR 4, with 0.5.3. The PR record lists what changed for testers, including the new "A1" labels. After the merge, tag `v0.5.3` ("MPCkit 0.5.3 (beta)").

### Readability PRs: what "no behaviour change" means

PRs 10-12 only rename, move, reformat and comment code. Each one proves that nothing changed:

- `go test ./...` passes with only renamed identifiers changed in the tests; no test is removed or loosened.
- **Exports are byte-identical.** Before the first commit, on `main`, run `go run ./cmd/mpckit-cli -load testdata/device/DemoFile.xpj -out <before>`; after the last, run it again into `<after>`, then `cmp <before>/DemoFile.xpj <after>/DemoFile.xpj` and `diff -r` the two data folders. Note the result in Testing.
- No bound method changes: `TestBoundMethods` passes untouched and `frontend/bindings` does not change.
- The PR record has a **Testing** list for the laptop that walks both tabs and every dialog.

### PR 10. Go readability (`go-readability`)

**Why.** Several names and functions make review harder than it needs to be. `state()` in `internal/app/app.go` uses `p`, `v`, `q`, `f`, `m` and `st` across one long function (R3). `wrap` builds a `Project` in `project` but prefixes an error in `xpj`; `edit` is an error-collecting type in `project` and a lock-and-return method on `Service` (R4). `main()` in `main.go` is 95 lines and mixes window sizing, the close guard, the file-drop handler and the panic handler; the CLI's `init()` in `cmd/mpckit-cli/main.go` registers nine flags as closures in 103 lines (R5). `uniqueFile` in `internal/project/pads.go` keeps file names and sample names in one map, told apart by a `"\x00"` suffix, and `!(bpm >= MinBPM && bpm <= MaxBPM)` in `sequences.go` is written that way to reject NaN with no comment saying so (R6).

**Tasks**

- [ ] Split `state()` into short helpers (for example `limits()`, `missingSources()`, `padState(slot)`, `sequenceStates()`) with descriptive local names.
- [ ] Rename `xpj.wrap` to `wrapErr`, `project.wrap` to `fromFile`, `Service.edit` to `apply`, and the `project.edit` type to `setter` (or names of the same clarity; say which in the record). Update the comments that mention them.
- [ ] In `main.go`, move the window sizing, the close guard and the file-drop handler into named functions, each with the comment that is there now. `main()` should read as a list of steps.
- [ ] In the CLI, give each flag its own named parse function (`parsePadOp`, `parseTuneOp`, ...) and register them from a short table in `init()`. Flag names, usage text and output stay exactly the same.
- [ ] Replace the `"\x00"` keys in `uniqueFile` with two maps, `takenFiles` and `takenNames`.
- [ ] Add a comment to the BPM check saying that it is written this way so NaN fails it.

**Done when** the "no behaviour change" checks pass, `go run ./cmd/mpckit-cli -h` prints the same text as before, and no function in `main.go`, `cmd/mpckit-cli/main.go` or `internal/app/app.go` is longer than 60 lines.

### PR 11. Format the frontend with Prettier (`frontend-format`)

**Why.** Eight markup lines are over 160 characters, the longest 250 (`App.svelte`, `SequencePanel.svelte`, `PadPanel.svelte`); a change to one attribute shows as the whole line changed (R2). There is no formatter, so style drifts. `save()` in `SequencePanel.svelte` uses `s`, `n`, `b` and `l` for slot, name, BPM and bars (R3).

**Tasks**

- [ ] Add `prettier` and `prettier-plugin-svelte` as dev dependencies (approved; take the current releases that `min-release-age` allows) and commit the updated lockfile.
- [ ] Add `frontend/.prettierrc` with `"printWidth": 120`, `"plugins": ["prettier-plugin-svelte"]`, and options that match the current style (double quotes, semicolons, trailing commas); add a `.prettierignore` for `dist`, `bindings` and `node_modules`.
- [ ] Add `"format": "prettier --write ."` and `"format:check": "prettier --check ."` to `frontend/package.json`.
- [ ] Commit the reformat on its own, with nothing else in that commit, so the PR has one large mechanical commit and small reviewable ones.
- [ ] In a separate commit, rename the one-letter variables in `save()` and `remove()` in `SequencePanel.svelte` (`slot`, `newName`, `newBpm`, `newBars`).
- [ ] Watch for whitespace changes in markup: Prettier can move text next to inline elements. Check each tab and dialog on the laptop for changed spacing and fix any with Prettier's own ignore comments only where needed.

**Done when** `npm run format:check` passes, no line under `frontend/src` is longer than 120 characters except where Prettier cannot break it, `npm run check` is clean, and the laptop walk-through shows no visual change.

### PR 12. One modal component (`shared-modal`)

**Why.** `AboutModal`, `ConfirmModal` and `PadPicker` each repeat the same backdrop, dialog box, Escape handling, focus-on-open and styles (R7). A fix to one is easily missed in the others.

**Tasks**

- [ ] Add `frontend/src/lib/Modal.svelte`: backdrop, dialog box and shared styles; props for the title id, an optional description id, `role` (`dialog` or `alertdialog`), `onclose` (called on Escape) and `closeOnBackdrop` (default false); the content as a snippet. The opener passes the element to focus, or `Modal` focuses the first focusable element.
- [ ] Rebuild the three dialogs on it, keeping each one's current behaviour exactly:

| Dialog | role | Escape | Click outside | Focus on open |
| --- | --- | --- | --- | --- |
| About | `dialog` | closes | closes | Close button |
| Confirm | `alertdialog`, with `aria-describedby` | Cancel | nothing | Cancel button |
| Move/Copy to pad | `dialog` | closes | nothing | Bank select |

- [ ] Remove the styles the three no longer need.

**Done when** `npm run check` and `npm run format:check` pass, and on the laptop each dialog matches the table: it opens with focus where the table says, Escape and clicking outside do what it says, Tab stays usable, and screen-reader names are unchanged (check with Narrator or the browser's accessibility tree in `wails3 dev`).

## Backlog

Not scheduled; skipped for now (2026-11-13). Each needs decisions before it can be planned; none blocks a release.

| Finding | What | Where |
| --- | --- | --- |
| S5 | Preview decode holds a long sample in memory three times; stream frames to the response instead | `internal/app/audio.go` |
| M9 | Dialog cancel is detected by comparing error text; pin it with a test | `dialogs.go` |
| M10 | Internal error text (`xpj: path ...`) reaches the user; show one plain sentence and log the detail | `internal/app`, `main.go` |
| M11 | `state()` stats every pool file and scans all events on each call, under the lock | `internal/app/app.go` |

R2-R7 are planned as PRs 10-12. Dropped: R8 (PR record layout) and CI (part of S6), by decision.