mpckit/pull-requests/safe-export/safe-export-and-clean-sample-names.md

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: <dest> 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.