Merge branch 'bound-surface' of bajdev/mpckit into main

frontend/bindings/git.monoexp.com/bajdev/mpckit/internal/app/service.ts

@@ -2,8 +2,11 @@
 // This file is automatically generated. DO NOT EDIT
 
 /**
- * Service is registered with Wails; its exported methods become the
- * frontend bindings.
+ * Service is the type main.go binds. Wails registers every exported method
+ * of a bound type by reflection, whatever the comments say, so the window's
+ * scripts can call each one by name. Service therefore has only the methods
+ * the frontend needs, none of which takes a file path; TestBoundMethods
+ * keeps that list on purpose. Go-only methods are on Host.
  * @module
  */
 
@@ -38,6 +41,15 @@ export function AssignSampleDialog(slot: number): $CancellablePromise<$models.St
     return $Call.ByID(254710842, slot);
 }
 
+/**
+ * CancelExport forgets the chosen export targets. The frontend calls it
+ * when the user declines to replace an existing export, so that a later
+ * ExportFolder(true) or ExportZip(true) cannot overwrite it without asking.
+ */
+export function CancelExport(): $CancellablePromise<void> {
+    return $Call.ByID(1632066973);
+}
+
 /**
  * ChooseExportFolder asks for the folder to export into and remembers it
  * for ExportFolder. It returns the folder, or "" if cancelled.

frontend/src/lib/store.svelte.ts

@@ -161,7 +161,11 @@ async function exportWith(
         items: res.exists,
         action: "Replace",
       });
-      if (!ok) return;
+      if (!ok) {
+        // Go forgets the target, so nothing can overwrite it without asking.
+        await Service.CancelExport();
+        return;
+      }
       res = await write(true);
     }
     app.error = "";

internal/app/app.go

@@ -120,8 +120,11 @@ type Dialogs interface {
 	SaveZip(defaultName string) (string, error)
 }
 
-// Service is registered with Wails; its exported methods become the
-// frontend bindings.
+// Service is the type main.go binds. Wails registers every exported method
+// of a bound type by reflection, whatever the comments say, so the window's
+// scripts can call each one by name. Service therefore has only the methods
+// the frontend needs, none of which takes a file path; TestBoundMethods
+// keeps that list on purpose. Go-only methods are on Host.
 type Service struct {
 	mu      sync.Mutex
 	p       *project.Project
@@ -134,18 +137,28 @@ type Service struct {
 	exportDir, zipPath string
 }
 
+// Host gives main.go what the window must not reach: loading and
+// assigning by path, the dirty flag, the window closer and the audio
+// server. It shares the Service's state but is not bound.
+type Host struct{ s *Service }
+
 // SetCloser sets what CloseWindow calls.
-//
-//wails:ignore
-func (s *Service) SetCloser(f func()) { s.closer = f }
+func (h *Host) SetCloser(f func()) { h.s.closer = f }
 
 // Dirty reports whether the project has changes not yet exported.
-//
-//wails:ignore
-func (s *Service) Dirty() bool {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	return s.p.Dirty()
+func (h *Host) Dirty() bool {
+	h.s.mu.Lock()
+	defer h.s.mu.Unlock()
+	return h.s.p.Dirty()
+}
+
+// LoadPath opens the project at path in place of the current one.
+func (h *Host) LoadPath(path string) (LoadResult, error) { return h.s.loadPath(path) }
+
+// AssignSamplePath puts the WAV at path on the pad in slot. The file drop
+// handler uses it.
+func (h *Host) AssignSamplePath(slot int, path string) (State, error) {
+	return h.s.assignSample(slot, path)
 }
 
 // CloseWindow closes the app without asking about unsaved changes. The
@@ -156,11 +169,12 @@ func (s *Service) CloseWindow() {
 	}
 }
 
-// NewService returns a service holding a new, empty project.
-func NewService(d Dialogs) *Service {
+// NewService returns a service holding a new, empty project, and the Host
+// that shares it. Bind only the Service.
+func NewService(d Dialogs) (*Service, *Host) {
 	s := &Service{dialogs: d}
 	s.reset()
-	return s
+	return s, &Host{s: s}
 }
 
 func (s *Service) reset() {
@@ -290,14 +304,10 @@ func (s *Service) AssignSampleDialog(slot int) (State, error) {
 	if err != nil || path == "" {
 		return s.State(), err
 	}
-	return s.AssignSamplePath(slot, path)
+	return s.assignSample(slot, path)
 }
 
-// AssignSamplePath puts the WAV at path on the pad in slot. main.go's file
-// drop handler uses it; it is not bound, so the window cannot pass a path.
-//
-//wails:ignore
-func (s *Service) AssignSamplePath(slot int, path string) (State, error) {
+func (s *Service) assignSample(slot int, path string) (State, error) {
 	return s.edit(func(p *project.Project) error { return p.AssignSample(slot, path) })
 }
 
@@ -377,6 +387,15 @@ func (s *Service) ExportZip(overwrite bool) (ExportResult, error) {
 	return s.exported(path, err)
 }
 
+// CancelExport forgets the chosen export targets. The frontend calls it
+// when the user declines to replace an existing export, so that a later
+// ExportFolder(true) or ExportZip(true) cannot overwrite it without asking.
+func (s *Service) CancelExport() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.exportDir, s.zipPath = "", ""
+}
+
 func (s *Service) exported(path string, err error) (ExportResult, error) {
 	var exists *export.ExistsError
 	if errors.As(err, &exists) {
@@ -413,15 +432,11 @@ func (s *Service) Load() (LoadResult, error) {
 	if err != nil || path == "" {
 		return LoadResult{State: s.State()}, err
 	}
-	return s.LoadPath(path)
+	return s.loadPath(path)
 }
 
-// LoadPath opens the project at path in place of the current one. Load
-// uses it after the dialog; it is not bound, so the window cannot pass a
-// path.
-//
-//wails:ignore
-func (s *Service) LoadPath(path string) (LoadResult, error) {
+// loadPath opens the project at path in place of the current one.
+func (s *Service) loadPath(path string) (LoadResult, error) {
 	p, warnings, err := project.Load(path)
 	if err != nil {
 		return LoadResult{State: s.State()}, fmt.Errorf("%s: %w", filepath.Base(path), err)

internal/app/app_test.go

@@ -11,6 +11,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -35,9 +36,13 @@ func (f *fakeDialogs) SaveZip(name string) (string, error) {
 
 func newService() (*Service, *fakeDialogs) {
 	d := &fakeDialogs{}
-	return NewService(d), d
+	s, _ := NewService(d)
+	return s, d
 }
 
+// hostOf returns the Host that shares s, as NewService does.
+func hostOf(s *Service) *Host { return &Host{s: s} }
+
 func must(t *testing.T, err error) {
 	t.Helper()
 	if err != nil {
@@ -110,7 +115,7 @@ func TestPadEdits(t *testing.T) {
 		t.Errorf("after assign: %+v", p)
 	}
 
-	st, err = s.AssignSamplePath(16, kick)
+	st, err = hostOf(s).AssignSamplePath(16, kick)
 	must(t, err)
 	if st.Pads[16].File != "Linn 9000 kick.wav" || st.SampleCount != 1 {
 		t.Error("drop of the same file did not share the pool entry")
@@ -141,7 +146,7 @@ func TestPadEdits(t *testing.T) {
 	}
 
 	// Errors still return the current state.
-	st, err = s.AssignSamplePath(1, filepath.Join(testdata, "nope.wav"))
+	st, err = hostOf(s).AssignSamplePath(1, filepath.Join(testdata, "nope.wav"))
 	if err == nil || st.Pads[16].File == "" {
 		t.Errorf("bad path: %v", err)
 	}
@@ -178,7 +183,7 @@ func TestExport(t *testing.T) {
 	s, d := newService()
 	_, err := s.SetName("Kit")
 	must(t, err)
-	_, err = s.AssignSamplePath(0, filepath.Join(kickDir, "Linn 9000 kick.wav"))
+	_, err = hostOf(s).AssignSamplePath(0, filepath.Join(kickDir, "Linn 9000 kick.wav"))
 	must(t, err)
 
 	d.folder = t.TempDir()
@@ -278,7 +283,7 @@ func TestExportTarget(t *testing.T) {
 		t.Error("New kept the export targets")
 	}
 	choose()
-	_, err = s.LoadPath(filepath.Join(testdata, "device", "DemoBlank.xpj"))
+	_, err = hostOf(s).LoadPath(filepath.Join(testdata, "device", "DemoBlank.xpj"))
 	must(t, err)
 	if s.exportDir != "" || s.zipPath != "" {
 		t.Error("Load kept the export targets")
@@ -299,9 +304,9 @@ func TestExportMatchesCLIRecipe(t *testing.T) {
 	s, _ := newService()
 	_, err := s.SetName("MpckitT1")
 	must(t, err)
-	_, err = s.AssignSamplePath(0, filepath.Join(kickDir, "Linn 9000 kick.wav"))
+	_, err = hostOf(s).AssignSamplePath(0, filepath.Join(kickDir, "Linn 9000 kick.wav"))
 	must(t, err)
-	_, err = s.AssignSamplePath(16, filepath.Join(kickDir, "Linn 9000 kick001.wav"))
+	_, err = hostOf(s).AssignSamplePath(16, filepath.Join(kickDir, "Linn 9000 kick001.wav"))
 	must(t, err)
 	_, err = s.SetTune(16, 5, 0)
 	must(t, err)
@@ -324,11 +329,11 @@ func TestExportMatchesCLIRecipe(t *testing.T) {
 func TestAudio(t *testing.T) {
 	s, _ := newService()
 	kick := filepath.Join(kickDir, "Linn 9000 kick.wav")
-	st, err := s.AssignSamplePath(3, kick)
+	st, err := hostOf(s).AssignSamplePath(3, kick)
 	must(t, err)
 
 	next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("asset")) })
-	h := s.AudioMiddleware(next)
+	h := hostOf(s).AudioMiddleware(next)
 	get := func(url string) *httptest.ResponseRecorder {
 		rec := httptest.NewRecorder()
 		h.ServeHTTP(rec, httptest.NewRequest("GET", url, nil))
@@ -373,11 +378,11 @@ func TestMoveCopySwap(t *testing.T) {
 	s, _ := newService()
 	kick := filepath.Join(kickDir, "Linn 9000 kick.wav")
 	kick2 := filepath.Join(kickDir, "Linn 9000 kick001.wav")
-	_, err := s.AssignSamplePath(0, kick)
+	_, err := hostOf(s).AssignSamplePath(0, kick)
 	must(t, err)
 	_, err = s.SetTune(0, 3, 0)
 	must(t, err)
-	_, err = s.AssignSamplePath(1, kick2)
+	_, err = hostOf(s).AssignSamplePath(1, kick2)
 	must(t, err)
 
 	// Copy across banks keeps the tuning and shares the sample.
@@ -434,7 +439,7 @@ func TestLoad(t *testing.T) {
 	}
 
 	// A bad file keeps the current project.
-	_, err = s.LoadPath(filepath.Join(testdata, "device", "DemoFile_[ProjectData]", "Bass-Pure C.wav"))
+	_, err = hostOf(s).LoadPath(filepath.Join(testdata, "device", "DemoFile_[ProjectData]", "Bass-Pure C.wav"))
 	if err == nil || s.State().Name != "DemoFile" {
 		t.Errorf("bad load: %v", err)
 	}
@@ -482,8 +487,12 @@ func TestGoldenT1ThroughService(t *testing.T) {
 	s, _ := newService()
 	calls := []func() (State, error){
 		func() (State, error) { return s.SetName("MpckitT1") },
-		func() (State, error) { return s.AssignSamplePath(0, filepath.Join(kickDir, "Linn 9000 kick.wav")) },
-		func() (State, error) { return s.AssignSamplePath(16, filepath.Join(kickDir, "Linn 9000 kick001.wav")) },
+		func() (State, error) {
+			return hostOf(s).AssignSamplePath(0, filepath.Join(kickDir, "Linn 9000 kick.wav"))
+		},
+		func() (State, error) {
+			return hostOf(s).AssignSamplePath(16, filepath.Join(kickDir, "Linn 9000 kick001.wav"))
+		},
 		func() (State, error) { return s.SetTune(16, 5, 0) },
 		func() (State, error) { return s.SetSequence(0, "Verse", 96, 8) },
 		func() (State, error) { return s.AddSequence(1) },
@@ -524,7 +533,7 @@ func TestMissingFiles(t *testing.T) {
 	must(t, os.Remove(filepath.Join(dir, "DemoFile"+project.DataSuffix, "80s-Snr-Dreamer SA.wav")))
 
 	s, _ := newService()
-	res, err := s.LoadPath(path)
+	res, err := hostOf(s).LoadPath(path)
 	must(t, err)
 	st := res.State
 	if st.MissingCount != 1 {
@@ -548,7 +557,7 @@ func TestMissingFiles(t *testing.T) {
 	}
 
 	// Replacing the pad's sample fixes it.
-	st, err = s.AssignSamplePath(1, filepath.Join(kickDir, "Linn 9000 kick.wav"))
+	st, err = hostOf(s).AssignSamplePath(1, filepath.Join(kickDir, "Linn 9000 kick.wav"))
 	must(t, err)
 	if st.MissingCount != 0 || st.Pads[1].Missing {
 		t.Error("still missing after replace")
@@ -562,12 +571,12 @@ func TestCloseWindow(t *testing.T) {
 	s, _ := newService()
 	s.CloseWindow() // no closer set: does nothing
 	closed := false
-	s.SetCloser(func() { closed = true })
-	if s.Dirty() {
+	hostOf(s).SetCloser(func() { closed = true })
+	if hostOf(s).Dirty() {
 		t.Error("new project dirty")
 	}
 	_, _ = s.SetName("Other")
-	if !s.Dirty() {
+	if !hostOf(s).Dirty() {
 		t.Error("not dirty after rename")
 	}
 	s.CloseWindow()
@@ -587,9 +596,53 @@ func TestLinkedFileIsMissing(t *testing.T) {
 		t.Skipf("cannot create a symlink: %v", err)
 	}
 	s, _ := newService()
-	res, err := s.LoadPath(path)
+	res, err := hostOf(s).LoadPath(path)
 	must(t, err)
 	if st := res.State; st.MissingCount != 1 || !st.Pads[1].Missing {
 		t.Errorf("missing count %d, A2 missing %v; want 1, true", st.MissingCount, st.Pads[1].Missing)
 	}
 }
+
+// TestBoundMethods pins the methods the window can call. Wails binds every
+// exported method of Service by reflection, so each one is reachable from
+// any script in the window by name.
+func TestBoundMethods(t *testing.T) {
+	want := []string{
+		"About", "AddSequence", "AssignSampleDialog", "CancelExport",
+		"ChooseExportFolder", "ChooseZipPath", "ClearPad", "CloseWindow",
+		"CopyPad", "ExportFolder", "ExportZip", "Load", "MovePad", "New",
+		"RemoveSequence", "SetMuteGroup", "SetName", "SetSequence", "SetTune",
+		"State", "SwapPads",
+	}
+	typ := reflect.TypeOf(&Service{})
+	var got []string
+	for i := range typ.NumMethod() {
+		got = append(got, typ.Method(i).Name)
+	}
+	if strings.Join(got, ",") != strings.Join(want, ",") {
+		t.Errorf("Service's exported methods are\n  %v\nwant\n  %v\n"+
+			"Every exported method of Service can be called by any script in the window. "+
+			"A new one must not take a file path (Go-only methods belong on Host), "+
+			"and must be added to this list on purpose.", got, want)
+	}
+}
+
+func TestCancelExport(t *testing.T) {
+	// Declining "Replace existing files?" forgets the target, so a later
+	// overwrite cannot reach it without the dialog.
+	s, d := newService()
+	d.folder, d.zip = t.TempDir(), filepath.Join(t.TempDir(), "kit.zip")
+	_, err := s.ChooseExportFolder()
+	must(t, err)
+	_, err = s.ChooseZipPath()
+	must(t, err)
+	s.CancelExport()
+	for _, export := range []func(bool) (ExportResult, error){s.ExportFolder, s.ExportZip} {
+		if _, err := export(true); err == nil || err.Error() != "choose where to export first" {
+			t.Errorf("overwrite after cancel: %v", err)
+		}
+	}
+	if ents, _ := os.ReadDir(d.folder); len(ents) != 0 {
+		t.Errorf("export after cancel wrote %d entries", len(ents))
+	}
+}

internal/app/audio.go

@@ -18,16 +18,14 @@ import (
 const AudioPrefix = "/audio/"
 
 // AudioMiddleware serves AudioPrefix and passes every other request on.
-// It is for main.go's asset server, not the frontend.
-//
-//wails:ignore
-func (s *Service) AudioMiddleware(next http.Handler) http.Handler {
+// It is for main.go's asset server.
+func (h *Host) AudioMiddleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if !strings.HasPrefix(r.URL.Path, AudioPrefix) {
 			next.ServeHTTP(w, r)
 			return
 		}
-		s.serveAudio(w, r)
+		h.s.serveAudio(w, r)
 	})
 }
 

main.go

@@ -42,7 +42,7 @@ func init() {
 
 func main() {
 	d := &dialogs{}
-	svc := app.NewService(d)
+	svc, host := app.NewService(d)
 	var a *application.App
 	a = application.New(application.Options{
 		Name:        "MPCkit",
@@ -59,7 +59,7 @@ func main() {
 		},
 		Assets: application.AssetOptions{
 			Handler:    application.AssetFileServerFS(assets),
-			Middleware: svc.AudioMiddleware,
+			Middleware: host.AudioMiddleware,
 		},
 		Mac: application.MacOptions{
 			ApplicationShouldTerminateAfterLastWindowClosed: true,
@@ -95,12 +95,12 @@ func main() {
 	// user chooses to discard them, the frontend calls CloseWindow, which
 	// closes for real.
 	var closeConfirmed atomic.Bool
-	svc.SetCloser(func() {
+	host.SetCloser(func() {
 		closeConfirmed.Store(true)
 		w.Close()
 	})
 	w.RegisterHook(events.Common.WindowClosing, func(e *application.WindowEvent) {
-		if closeConfirmed.Load() || !svc.Dirty() {
+		if closeConfirmed.Load() || !host.Dirty() {
 			return
 		}
 		e.Cancel()
@@ -121,7 +121,7 @@ func main() {
 		}
 		for _, f := range ctx.DroppedFiles() {
 			if strings.EqualFold(filepath.Ext(f), ".wav") {
-				st, err := svc.AssignSamplePath(slot, f)
+				st, err := host.AssignSamplePath(slot, f)
 				a.Event.Emit(eventState, st)
 				if err != nil {
 					a.Event.Emit(eventError, err.Error())

pull-requests/bound-surface/bind-only-what-the-window-needs.md

@@ -0,0 +1,73 @@
+# Bind only what the window needs
+
+Branch: `bound-surface` → `main`
+
+PR 8b of the review fixes plan (`docs/review-fixes-plan.md`), part of the 0.5.3 series; the version does not change in this PR.
+
+## What changed
+
+**The window can no longer reach the Go-only methods (S4, rest).**
+- **The problem:** PR 5 hid `LoadPath` and `AssignSamplePath` with `//wails:ignore`. That only keeps them out of the generated bindings. Wails 3.1.0 binds every exported method of a bound type by reflection, so any script in the window could still call them by name with any path.
+- **Reproduced first:** on `main`, a scratch build confirmed it; see Testing.
+- **The fix:**
+  - **`Service`**, the type `main.go` binds, now has only the 21 methods the frontend calls. None takes a file path.
+  - **The Go-only methods move to a new, unbound `Host`:** `LoadPath`, `AssignSamplePath`, `Dirty`, `SetCloser` and `AudioMiddleware`. `Host` shares the `Service`'s state, and `NewService` returns both.
+  - **Shared helpers:** `Service.Load` and `AssignSampleDialog` call unexported helpers (`loadPath`, `assignSample`) that `Host` uses too.
+  - **No more `//wails:ignore`:** nothing on `Service` needs it now.
+- **`main.go`** uses `Host` for the close hook, the file-drop handler and the audio middleware, and binds only `Service`.
+
+**Declining "Replace existing files?" forgets the target.**
+- Before, Go kept the chosen folder or zip path, so a later `ExportFolder(true)` would have overwritten it without asking.
+- The new bound `CancelExport()` forgets both targets. `exportWith` in `store.svelte.ts` calls it when the user declines.
+
+**Tests.**
+- **`TestBoundMethods`** lists `*Service`'s exported methods by reflection and compares them with an explicit list. If they differ, the message explains:
+  - every exported method can be called by any script in the window;
+  - a new one must not take a file path, since Go-only methods belong on `Host`;
+  - it must be added to the list on purpose.
+  
+  With `LoadPath(string)` temporarily added back to `Service`, it fails as intended.
+- **`TestCancelExport`:** after `CancelExport`, `ExportFolder(true)` and `ExportZip(true)` fail with "choose where to export first" and write nothing.
+- **Existing tests** reach the Go-only methods through `hostOf(s)`.
+
+## Notes
+
+- **The scratch probe.** These lines were appended to `frontend/src/main.ts` and not committed; the file is back to its committed state:
+  - It calls `Call.ByName("git.monoexp.com/bajdev/mpckit/internal/app.Service.LoadPath", <path>)`, and in the fixed build also `AssignSamplePath`.
+  - The webview's console can't be seen in a headless run, so the probe reports through timing, using the bound `CloseWindow`:
+    - if a call succeeds, it closes the app at once;
+    - if every call is rejected with "unknown bound method name", it closes after 6 seconds;
+    - anything else leaves the app running until the 15–20 second timeout.
+  - Startup takes about 4 seconds, so success is about 4 s and rejection about 10 s.
+- **The probe results:**
+
+  | Build | Probe | Result |
+  | --- | --- | --- |
+  | `main` before this PR | `LoadPath` | Closed after **4 s**: the call succeeded and Go loaded the file. |
+  | `main` before this PR | control: a method that does not exist | Closed after **10 s**: rejected as unknown. |
+  | This branch | `LoadPath` and `AssignSamplePath` | Closed after **10 s**, twice: both rejected with "unknown bound method name". |
+- **`CloseWindow` stays bound**, because the frontend needs it after "Close without exporting?". It takes no arguments and only closes the window.
+
+## Testing
+
+**On the server, every command with `GOTOOLCHAIN=go1.27.4`:**
+- `go test ./...` passes, including `TestBoundMethods` and `TestCancelExport`.
+- `go vet ./...` (also with `GOOS=windows`) and `staticcheck ./...` are clean.
+- `govulncheck ./...`: "No vulnerabilities found."
+- In `frontend`: `npm ci`, `npm run check` (0 errors, 0 warnings) and `npm run build`.
+- `wails3 build` builds, and the app starts under Xvfb.
+- The scratch probe, as above.
+
+**On the laptop (PowerShell, in the repo):**
+1. `git fetch; git checkout bound-surface`
+2. `go test ./internal/app`: `ok`.
+3. `wails3 dev`, then:
+   - **Load:** load `testdata\device\DemoFile.xpj`. It opens as before.
+   - **Preview:** click A1, then B2. Each plays.
+   - **File drop:** drop a WAV file onto an empty pad. It is assigned.
+   - **Close prompt:** change something, then close the window. "Close without exporting?" appears. Cancel keeps the app open; choosing to close closes it.
+4. Run `wails3 dev` again, assign a WAV, and export:
+   - **Export to folder** into an empty folder: it exports.
+   - **Export to folder** into the same folder again, and choose **Replace**: it exports.
+   - Once more, and choose **Cancel** at "Replace existing files?": nothing is written, and no error shows.
+   - **Export zip:** the same three steps (new file, Replace, Cancel).