# Safe export and clean sample names Branch: `safe-export` → `main` PR 3 of the review fixes plan (`docs/review-fixes-plan.md`). Part of 0.5.2; the version does not change in this PR. ## What changed **Export refuses links (S1, export side).** - A sample whose source is a link, or not a regular file, is reported as missing, the same way as a deleted file, and nothing is written. - So is a sample whose folder is itself a link. This was not in the plan; see Notes. - The check is `export.Usable`. The app uses the same check for the missing-file badges and missing count, so a pad with a linked source shows as missing and the export buttons are disabled, matching what export would do. **Export checks names again (S1).** `Folder` and the zip writer refuse a sample file name that is not a plain file name, using PR 1's rule (`project.PlainFileName`). This is a second line of defence: load and assign already ensure it. **New sample names are cleaned at assign (S1, assign).** - A new pool file name is passed through `poolFileName` in `project` before the 001, 002 suffixes are added. A comment there gives the reason. - The helper replaces `/`, `\`, control characters, and the characters the device's FAT card forbids (`: * ? " < > |`) with `_`. A name Windows reserves for a device (`CON`, `PRN`, `AUX`, `NUL`, `COM1`–`COM9`, `LPT1`–`LPT9`, in any case and with any extension) gets `_` after that part, so `CON.wav` becomes `CON_.wav`. A name that comes out empty, `.` or `..` becomes `sample.wav`. The sample name follows from the cleaned file name. - So `kick\one.wav`, which Linux and macOS allow, becomes `kick_one.wav`, and its export loads again. - Load still accepts existing names; only new assignments change. **Links you pick yourself still work.** When you assign a WAV that is a link, MPCkit follows it. The pool file is named after the link, and the source is the file it points to. Picking the target itself later reuses the same pool entry. **Overwrite cannot lose the old export (M1, R1).** With overwrite set, `Folder` used to delete the old `.xpj` and data folder before moving the new ones in, so a failed rename lost both. Now: 1. The new pair is written in full to a temporary folder inside the destination, as before. 2. The old pair is moved into that temporary folder. 3. The new pair is moved into place. 4. The old pair is deleted only after that. If any move fails, the moves already made are undone, newest first, and the old export is back as it was. If even putting it back fails, the temporary folder is kept, and the error says where the old export is. The `Folder` comment now says exactly this. **Missing destination.** Exporting into a folder that no longer exists now says `export folder does not exist: ` instead of a bare `stat` error. **Tests (M5).** Coverage of `internal/export` is up from 74.2% to 87.8%. New tests: - `TestSymlinkedSource`: a kit whose data folder holds a link to a file outside. Folder and Zip export both fail, naming the file, and write nothing. - `TestLinkedDataFolder`: a kit whose data folder is itself a link. Same result. - `TestBadPoolName`: a pool name `../evil.wav` is refused by Folder and Zip export, and nothing is written. - `TestOverwriteFailureKeepsOldExport`: makes each of the 4 renames of an overwrite fail in turn. Every time, the previous export is byte-for-byte as it was. - `TestOverwriteUndoFails`: the new `.xpj` cannot be moved in, and putting things back fails too. The error names the folder that holds the old export, and the export is complete there. - `TestOverwriteHalfPair`: an overwrite where only the old `.xpj` exists. - `TestMissingDestination`. - `TestExportErrors`: a bad project name, no recorded source, a destination that is a file, a zip into a missing folder, and the "already exists" message. - `TestBackslashNameExportsAndLoads`: assigns a WAV named `kick\one.wav`, gets `kick_one.wav`, exports, and loads the export. - In `project` and `app`: `TestPoolFileName`, `TestAssignFollowsLink` and `TestLinkedFileIsMissing`. Against the old code, every new export test fails. ## Notes - **Decisions (bajdev, 2026-11-12).** Two choices the plan left open were settled while starting this PR, and are now in the plan's Decisions table: - Links the user picks are followed at assign. - FAT-illegal characters are cleaned too. - **Linked data folder (not in the plan).** `Lstat` checks only the last part of a path. A kit whose whole `_[ProjectData]` folder is a link to, say, `~/.ssh`, with a pool entry named `id_rsa`, would still have exported that file. `Usable` now also requires the file's own folder to be a real folder. - Folders higher up may still be links; a project kept in a linked music folder works as before. - Samples you assign are unaffected, since their paths are resolved at assign. - **Making a rename fail.** The plan suggested a directory where the `.xpj` should go. With the new overwrite, that directory is simply moved aside and replaced, so it no longer fails. The tests instead swap the package's `rename` (normally `os.Rename`) for one that fails on chosen calls. This reaches every step on every system. - **The source shown for a linked WAV.** It is the file the link points to, not the link. - **Windows device names (bajdev, 2026-11-13).** These are renamed too. `ValidName` and `poolFileName` now share one list, `reservedOnWindows`. - **What the user sees.** Renaming is silent, like the 001 suffixes added for duplicate names. The pad shows the new name (`kick_one`), so the change is visible but not explained. Only names that could not be exported as they are change, and none of them can be created on Windows anyway. - **On Windows, the link tests skip** unless developer mode is on, because creating links needs it. `TestBackslashNameExportsAndLoads` also skips there, because Windows file names cannot hold `\`. ## Testing **On the server:** - `go test ./...` passes, including the round-trip tests, unchanged. - `go test -cover ./internal/export` shows 87.8%. - `go vet ./...` (also with `GOOS=windows`) and `staticcheck ./...` are clean. - `govulncheck ./...` reports only GO-2026-6859, which waits for Go 1.27.4 in PR 4. - `npm install`, `npm run check` (0 errors, 0 warnings) and `npm run build` pass. The lockfile `npm install` wrote was not committed. - `wails3 build` builds, and `timeout 15 xvfb-run -a bin/mpckit` starts and runs. The log holds only the usual headless GTK warnings. **On the laptop (PowerShell, in the repo):** 1. `git fetch; git checkout safe-export` 2. `go test -cover ./internal/export`: `ok`, coverage 87.8%. That is the same as on the server: the tests that skip on Windows add no coverage of their own. 3. `go test ./...`: all `ok`. 4. `wails3 dev`: load `testdata\device\DemoFile.xpj`, choose Export to folder and pick an empty folder. It writes `DemoFile.xpj` and `DemoFile_[ProjectData]` with 6 WAVs. 5. Change a pad's tune and export to the same folder again. Confirm the overwrite prompt. The export is replaced, and no `.mpckit-export-…` folder is left behind. 6. Load the exported `DemoFile.xpj`. It opens with the changed tune, and no pad shows as missing. 7. Export as zip. The zip holds the `.xpj` and the data folder with 6 WAVs. 8. Assign a WAV from your sample library to an empty pad. It gets the WAV's own name, as before.