From ebfe93e55bc0a3196618d314be763dac52b64ff7 Mon Sep 17 00:00:00 2001 From: vitorhugo-dotnet Date: Fri, 25 Sep 2026 10:43:28 -0300 Subject: [PATCH 01/31] docs: specify profile editing and Windows installer --- ...ipt-editor-and-windows-installer-design.md | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-25-profile-script-editor-and-windows-installer-design.md diff --git a/docs/superpowers/specs/2026-09-25-profile-script-editor-and-windows-installer-design.md b/docs/superpowers/specs/2026-09-25-profile-script-editor-and-windows-installer-design.md new file mode 100644 index 0000000..ba360ab --- /dev/null +++ b/docs/superpowers/specs/2026-09-25-profile-script-editor-and-windows-installer-design.md @@ -0,0 +1,108 @@ +# Profile Management, SQL Script Editing, and Windows Distribution Design + +**Date:** 2026-09-25 +**Status:** Design approved in conversation; awaiting written specification review. + +## Goal + +Extend the existing Wails desktop app so users can edit stored SQL scripts, delete and clone profiles, and choose between a portable executable and a guided Windows installer. The GitHub Release produced by CI will contain both Windows distribution options. + +## Current context + +- The React UI already lists, enables, removes, and reorders scripts, and edits profile connection and execution settings. +- SQL files live under each profile directory. React calls Go application services through Wails bindings; it does not access profile files directly. +- The storage repository already supports deleting a profile directory, but the service/backend/UI do not expose profile deletion. +- The service supports updating a profile while retaining its ID and script references. It does not yet clone profiles or edit stored SQL content. +- CI currently builds and publishes a single Wails executable named `go-script-sql-runner.exe`. +- The update checker currently searches GitHub Release assets for that fixed executable name. + +## Approved design + +### Script editing with Monaco + +Add an **Edit** action for every script beside the existing ordering controls. It opens a larger in-app dialog containing Monaco Editor configured for SQL syntax highlighting. The editor loads content through the Wails API and saves it through the Go application service to the existing profile-owned SQL file. The frontend will not write files directly. + +Saving changes only the script content; its profile, ID, name, order, enabled state, transaction mode, and file reference remain unchanged. Closing a modified editor without saving asks the user to confirm discarding the changes. Editing is unavailable while a profile run is active. + +### Profile deletion + +Add a **Delete profile** action for the selected profile. Before deletion, show a confirmation that identifies the profile by name and explains that its stored scripts are removed with it. The service deletes the profile through the repository. On success, refresh the profile list and select another profile if one remains; on failure, preserve the selection and show the error. + +### Profile cloning + +Add a **Clone profile** action. Create a separate profile with a new generated ID and a derived copy name. Copy connection settings including the password, execution settings, all script metadata, and the SQL file contents into the new profile's directory. Editing or deleting the clone must not change the source profile. The cloned profile can be renamed later using the existing profile editing dialog. + +Cloning and deletion run through the Go application service and storage repository. The frontend does not copy or remove filesystem paths. + +### Portable executable and installer + +Keep a standalone, non-installing executable and name its release asset `go-script-sql-runner-portable.exe`. It continues to store profiles and logs in the current per-user AppData location. + +Add a WiX-authored MSI named `go-script-sql-runner-setup.msi` with a guided installation flow and a scope choice: + +- **Current user:** install under `%LOCALAPPDATA%\Programs\Go Script SQL Runner` without administrator rights. +- **All users:** install under `%ProgramFiles%\Go Script SQL Runner`; Windows Installer requests elevation for this machine-wide installation. + +Regardless of installation scope, application data remains per-user in AppData. The installer supplies normal application shortcuts and an uninstall registration, and uses stable package identity/versioning so later releases can upgrade an earlier MSI installation. + +CI builds and verifies both artifacts and publishes both assets on the existing automated GitHub Release. The update checker switches its fixed asset lookup to `go-script-sql-runner-portable.exe`; opening an update continues to download the portable build. + +### Alternatives considered + +- **Editing through the system's default SQL editor:** avoids embedding an editor, but requires launching an external application and complicates the save/reload path. Monaco was selected for an in-app editing flow. +- **Separate per-user and per-machine MSI files:** duplicates installer maintenance and makes release choice less clear. A single dual-purpose MSI with a scope choice was selected. +- **Single executable only:** keeps the current release simple but does not provide the requested guided installation or machine-wide option. + +## Components and interfaces + +The implementation will follow existing boundaries: + +- `internal/storage`: safe read/write access to a script file, profile deletion, and cloning/copying profile-owned files. +- `internal/app`: context-aware use cases for script content read/write, profile deletion, and profile cloning, with validation and clear errors. +- `internal/ui/wails`: bind the new service operations to Wails; do not expose raw paths for frontend file access. +- `frontend/src/api`: extend the `RunnerApi` and Wails adapter with the new calls. +- `frontend/src/components`: Monaco SQL editor dialog and profile deletion confirmation. +- `frontend/src/App.tsx` and `frontend/src/state/useRunnerController.ts`: add the script and profile actions and refresh/selection behavior. +- `.github/workflows/ci.yml` and a WiX source/build script under `build/windows` or `scripts`: build, verify, upload, and publish the portable EXE and MSI. +- `internal/updatecheck`: search for the portable release asset's new filename. +- `README.md`: explain the portable executable and both MSI installation scopes. + +Exact file names and helper boundaries may follow nearby repository conventions while preserving the behavior above. + +## Error handling and data safety + +- Validate profile and script IDs before resolving paths; resolve SQL paths only from validated profile metadata and enforce that resolved paths remain inside the profile directory. +- Use atomic replacement for SQL edits so an interrupted write does not leave a truncated script. +- Cloning should fail without modifying the source; avoid leaving an incomplete destination profile if copying or persistence fails. +- A failed deletion must be visible to the user. Confirmation precedes recursive profile removal. +- Treat installer and portable artifacts as equivalent app versions. Data remains in per-user AppData and is not moved or deleted by install/uninstall. +- Continue embedding WebView2 according to the current Wails build configuration. + +## Verification and acceptance criteria + +### Backend + +- Unit tests cover script content reads/writes, unknown profile/script IDs, validation, preserving all script metadata during content updates, and ensuring writes stay within profile storage. +- Service/repository tests cover clone independence (new ID, retained settings/password, identical SQL bytes, independent subsequent edits/deletes), deletion of all files for a profile, and no effect on other profiles. +- Wails-facing tests or the existing bridge tests verify calls and error propagation. + +### Frontend + +- Component tests cover opening the editor with the selected SQL, saving content, cancelling unchanged content, confirming a dirty close, and disabled state while running. +- UI tests cover profile deletion confirmation/cancel/error/success and cloning plus profile selection/refresh. +- Frontend typecheck/build confirms Monaco and worker assets bundle correctly for Wails' local WebView. + +### Windows distribution + +- CI verifies both expected output files exist before upload and both named assets are published. +- Validate the MSI's wizard has a current-user and all-users choice; verify the former installs to LocalAppData without elevation and the latter targets Program Files and requests elevation. +- Verify MSI upgrade/uninstall behavior and that application profiles remain in per-user AppData. +- Update-checker tests select the new portable filename and continue to work when an expected asset is absent. +- Run the Go and frontend test suites and typecheck through CI. + +## Out of scope + +- SQL semantic validation, database-aware completion, formatting, or schema introspection in Monaco. +- Editing script names, order, or execution properties in the SQL editor dialog. +- Moving profile data into the all-users installation directory or sharing profiles between Windows accounts. +- Automatic in-place application of updates; the update notice continues to open the portable asset/release page behavior already supported by the app. From 00540f5708e29e8958be3b5af59a10b375cdfd31 Mon Sep 17 00:00:00 2001 From: vitorhugo-dotnet Date: Fri, 25 Sep 2026 11:36:02 -0300 Subject: [PATCH 02/31] chore: ignore local git worktrees --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 2aa3aa9..8df9778 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ frontend/dist/* !frontend/dist/index.html # Local runtime/profile artifacts +.worktrees/ GoScriptSQLRunner/ appdata/ .local/ From 81d04e0cb24bbc32bf7affe42ff3c578d0c5045d Mon Sep 17 00:00:00 2001 From: vitorhugo-dotnet Date: Fri, 25 Sep 2026 11:48:31 -0300 Subject: [PATCH 03/31] feat: add profile clone and delete operations --- internal/app/profile_lifecycle.go | 54 ++++++++++ internal/app/profile_lifecycle_test.go | 138 +++++++++++++++++++++++++ internal/storage/repository.go | 99 +++++++++++++++++- internal/storage/repository_test.go | 135 +++++++++++++++++++++++- 4 files changed, 424 insertions(+), 2 deletions(-) create mode 100644 internal/app/profile_lifecycle.go create mode 100644 internal/app/profile_lifecycle_test.go diff --git a/internal/app/profile_lifecycle.go b/internal/app/profile_lifecycle.go new file mode 100644 index 0000000..c6e8652 --- /dev/null +++ b/internal/app/profile_lifecycle.go @@ -0,0 +1,54 @@ +package app + +import ( + "context" + "fmt" + + "github.com/vitorhugo-dotnet/go-script-sql-runner/internal/id" + "github.com/vitorhugo-dotnet/go-script-sql-runner/internal/profile" +) + +func (s *Service) CloneProfile(ctx context.Context, profileID string) (profile.Profile, error) { + if err := ctx.Err(); err != nil { + return profile.Profile{}, fmt.Errorf("clone profile %q: %w", profileID, err) + } + source, err := s.repository.Get(profileID) + if err != nil { + return profile.Profile{}, fmt.Errorf("load profile %q for clone: %w", profileID, err) + } + clone := source + clone.ID = "" + clone.Name = source.Name + " (copy)" + generated, err := id.New() + if err != nil { + return profile.Profile{}, fmt.Errorf("generate clone id for profile %q: %w", profileID, err) + } + clone.ID = generated + if err := s.repository.Clone(profileID, clone); err != nil { + return profile.Profile{}, fmt.Errorf("clone profile %q: %w", profileID, err) + } + return clone, nil +} + +func (s *Service) DeleteProfile(ctx context.Context, profileID string) error { + if err := ctx.Err(); err != nil { + return fmt.Errorf("delete profile %q: %w", profileID, err) + } + s.connectionMu.Lock() + defer s.connectionMu.Unlock() + if err := ctx.Err(); err != nil { + return fmt.Errorf("delete profile %q: %w", profileID, err) + } + if err := s.repository.Delete(profileID); err != nil { + return fmt.Errorf("delete profile %q: %w", profileID, err) + } + if s.activeProfileID == profileID { + client := s.activeClient + s.activeClient = nil + s.activeProfileID = "" + if err := client.Close(); err != nil { + return fmt.Errorf("close connection for deleted profile %q: %w", profileID, err) + } + } + return nil +} diff --git a/internal/app/profile_lifecycle_test.go b/internal/app/profile_lifecycle_test.go new file mode 100644 index 0000000..892a791 --- /dev/null +++ b/internal/app/profile_lifecycle_test.go @@ -0,0 +1,138 @@ +package app + +import ( + "context" + "database/sql" + "errors" + "os" + "path/filepath" + "reflect" + "testing" + + "github.com/vitorhugo-dotnet/go-script-sql-runner/internal/database" + "github.com/vitorhugo-dotnet/go-script-sql-runner/internal/profile" + "github.com/vitorhugo-dotnet/go-script-sql-runner/internal/storage" +) + +func lifecycleProfile(name string) profile.Profile { + return profile.Profile{ + Name: name, + Connection: profile.Connection{Host: "db.example", Port: 3306, Username: "root", Password: "secret"}, + Execution: profile.Execution{OnError: profile.OnErrorStop, TransactionMode: profile.TransactionRunnerManaged}, + } +} + +func TestCloneProfileCopiesSettingsScriptsAndGeneratesUniqueIDs(t *testing.T) { + repo := storage.NewRepository(t.TempDir()) + service := NewService(repo) + first, err := service.CreateProfile(context.Background(), lifecycleProfile("Shared name")) + if err != nil { + t.Fatal(err) + } + second, err := service.CreateProfile(context.Background(), lifecycleProfile("Shared name")) + if err != nil { + t.Fatal(err) + } + sourcePath := filepath.Join(t.TempDir(), "setup.sql") + if err := os.WriteFile(sourcePath, []byte("SELECT 42;\n"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := service.AddScript(context.Background(), first.ID, sourcePath); err != nil { + t.Fatal(err) + } + first, err = repo.Get(first.ID) + if err != nil { + t.Fatal(err) + } + clone, err := service.CloneProfile(context.Background(), first.ID) + if err != nil { + t.Fatalf("CloneProfile() error: %v", err) + } + if clone.ID == "" || clone.ID == first.ID || clone.ID == second.ID || clone.Name != "Shared name (copy)" { + t.Fatalf("clone identity = %#v", clone) + } + if clone.Version != first.Version || clone.Connection != first.Connection || clone.Execution != first.Execution || !reflect.DeepEqual(clone.Scripts, first.Scripts) { + t.Fatalf("clone lost settings or scripts: %#v; source %#v", clone, first) + } + stored, err := repo.Get(clone.ID) + if err != nil || !reflect.DeepEqual(stored, clone) { + t.Fatalf("stored clone = %#v, %v", stored, err) + } + data, err := os.ReadFile(filepath.Join(repo.ScriptRoot(clone.ID), filepath.Base(first.Scripts[0].File))) + if err != nil || string(data) != "SELECT 42;\n" { + t.Fatalf("clone script = %q, %v", data, err) + } + otherClone, err := service.CloneProfile(context.Background(), second.ID) + if err != nil || otherClone.ID == clone.ID || otherClone.ID == second.ID || otherClone.Name != clone.Name { + t.Fatalf("second clone = %#v, %v", otherClone, err) + } +} + +func TestDeleteProfileClosesOnlyMatchingActiveConnection(t *testing.T) { + repo := storage.NewRepository(t.TempDir()) + service := NewService(repo) + active, err := service.CreateProfile(context.Background(), lifecycleProfile("Active")) + if err != nil { + t.Fatal(err) + } + other, err := service.CreateProfile(context.Background(), lifecycleProfile("Other")) + if err != nil { + t.Fatal(err) + } + recorded := &recordingConn{} + service.connectServer = func(ctx context.Context, _ profile.Connection) (*database.Client, database.ConnectionResult, error) { + db := sql.OpenDB(recordingConnector{conn: recorded}) + if err := db.PingContext(ctx); err != nil { + return nil, database.ConnectionResult{}, err + } + return &database.Client{DB: db}, database.ConnectionResult{}, nil + } + if _, err := service.Connect(context.Background(), active.ID); err != nil { + t.Fatal(err) + } + if err := service.DeleteProfile(context.Background(), other.ID); err != nil { + t.Fatalf("DeleteProfile(other) error: %v", err) + } + if recorded.closed != 0 || service.activeClient == nil || service.activeProfileID != active.ID { + t.Fatal("deleting another profile closed the active connection") + } + if _, err := repo.Get(other.ID); !errors.Is(err, storage.ErrNotFound) { + t.Fatalf("deleted profile Get() error = %v", err) + } + if err := service.DeleteProfile(context.Background(), "../outside"); err == nil { + t.Fatal("DeleteProfile(invalid ID) succeeded") + } + if recorded.closed != 0 || service.activeClient == nil { + t.Fatal("failed deletion changed active connection") + } + if err := service.DeleteProfile(context.Background(), active.ID); err != nil { + t.Fatalf("DeleteProfile(active) error: %v", err) + } + if recorded.closed != 1 || service.activeClient != nil || service.activeProfileID != "" { + t.Fatalf("active connection after deletion: closed=%d client=%#v id=%q", recorded.closed, service.activeClient, service.activeProfileID) + } + if _, err := repo.Get(active.ID); !errors.Is(err, storage.ErrNotFound) { + t.Fatalf("active profile Get() error = %v", err) + } +} + +func TestProfileLifecycleCancelledContextDoesNotMutate(t *testing.T) { + repo := storage.NewRepository(t.TempDir()) + service := NewService(repo) + source, err := service.CreateProfile(context.Background(), lifecycleProfile("Source")) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := service.CloneProfile(ctx, source.ID); !errors.Is(err, context.Canceled) { + t.Fatalf("CloneProfile(cancelled) error = %v", err) + } + if err := service.DeleteProfile(ctx, source.ID); !errors.Is(err, context.Canceled) { + t.Fatalf("DeleteProfile(cancelled) error = %v", err) + } + profiles, err := repo.List() + if err != nil || len(profiles) != 1 || profiles[0].ID != source.ID { + t.Fatalf("profiles after cancelled operations = %#v, %v", profiles, err) + } +} diff --git a/internal/storage/repository.go b/internal/storage/repository.go index dcb7880..4bf2874 100644 --- a/internal/storage/repository.go +++ b/internal/storage/repository.go @@ -7,7 +7,9 @@ import ( "io" "os" "path/filepath" + "reflect" "sort" + "strings" "github.com/vitorhugo-dotnet/go-script-sql-runner/internal/profile" ) @@ -79,7 +81,7 @@ func (r *Repository) List() ([]profile.Profile, error) { } profiles := make([]profile.Profile, 0, len(entries)) for _, entry := range entries { - if !entry.IsDir() { + if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".clone-") { continue } p, err := r.Get(entry.Name()) @@ -107,6 +109,101 @@ func (r *Repository) Delete(id string) error { return nil } +// Clone copies a profile's managed SQL files before publishing the destination. +func (r *Repository) Clone(sourceID string, destination profile.Profile) error { + if !validLookupID(sourceID) || !validLookupID(destination.ID) { + return fmt.Errorf("invalid profile id") + } + if err := profile.Validate(destination); err != nil { + return fmt.Errorf("validate clone destination: %w", err) + } + if sourceID == destination.ID { + return fmt.Errorf("clone destination %q is the source profile", destination.ID) + } + source, err := r.Get(sourceID) + if err != nil { + return fmt.Errorf("load clone source %q: %w", sourceID, err) + } + if !reflect.DeepEqual(destination.Scripts, source.Scripts) { + return fmt.Errorf("clone destination scripts differ from source %q", sourceID) + } + if err := r.ensure(); err != nil { + return err + } + profilesRoot := filepath.Join(r.root, "profiles") + destinationDir := r.ProfileDir(destination.ID) + if _, err := os.Lstat(destinationDir); err == nil { + return fmt.Errorf("clone destination %q already exists", destination.ID) + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("check clone destination %q: %w", destination.ID, err) + } + + sourceDir := r.ProfileDir(sourceID) + sourceInfo, err := os.Lstat(sourceDir) + if err != nil { + return fmt.Errorf("stat clone source %q: %w", sourceID, err) + } + if !sourceInfo.IsDir() { + return fmt.Errorf("clone source %q is not a directory", sourceID) + } + resolvedSource, err := filepath.EvalSymlinks(sourceDir) + if err != nil { + return fmt.Errorf("resolve clone source %q: %w", sourceID, err) + } + stagingDir, err := os.MkdirTemp(profilesRoot, ".clone-") + if err != nil { + return fmt.Errorf("create clone staging directory: %w", err) + } + defer os.RemoveAll(stagingDir) + + for _, script := range source.Scripts { + if !strings.EqualFold(filepath.Ext(script.File), ".sql") { + return fmt.Errorf("clone script %q is not a SQL file", script.ID) + } + sourcePath := filepath.Join(sourceDir, filepath.FromSlash(script.File)) + rel, err := filepath.Rel(sourceDir, sourcePath) + if err != nil || rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return fmt.Errorf("clone script %q escapes source profile", script.ID) + } + resolvedPath, err := filepath.EvalSymlinks(sourcePath) + if err != nil { + return fmt.Errorf("resolve clone script %q: %w", script.ID, err) + } + resolvedRel, err := filepath.Rel(resolvedSource, resolvedPath) + if err != nil || resolvedRel == "." || resolvedRel == ".." || strings.HasPrefix(resolvedRel, ".."+string(filepath.Separator)) { + return fmt.Errorf("clone script %q escapes source profile", script.ID) + } + info, err := os.Lstat(sourcePath) + if err != nil { + return fmt.Errorf("stat clone script %q: %w", script.ID, err) + } + if !info.Mode().IsRegular() { + return fmt.Errorf("clone script %q is not a regular file", script.ID) + } + data, err := os.ReadFile(sourcePath) + if err != nil { + return fmt.Errorf("read clone script %q: %w", script.ID, err) + } + if err := atomicWrite(filepath.Join(stagingDir, rel), data, 0o600); err != nil { + return fmt.Errorf("write clone script %q: %w", script.ID, err) + } + } + if err := os.MkdirAll(filepath.Join(stagingDir, "scripts"), 0o700); err != nil { + return fmt.Errorf("create clone script directory: %w", err) + } + var encoded bytes.Buffer + if err := profile.Encode(&encoded, destination); err != nil { + return fmt.Errorf("encode clone destination: %w", err) + } + if err := atomicWrite(filepath.Join(stagingDir, "profile.yaml"), encoded.Bytes(), 0o600); err != nil { + return fmt.Errorf("write clone profile: %w", err) + } + if err := os.Rename(stagingDir, destinationDir); err != nil { + return fmt.Errorf("publish clone destination %q: %w", destination.ID, err) + } + return nil +} + func (r *Repository) AddScript(profileID string, script profile.Script, sourcePath string) error { p, err := r.Get(profileID) if err != nil { diff --git a/internal/storage/repository_test.go b/internal/storage/repository_test.go index 3282617..0a321c9 100644 --- a/internal/storage/repository_test.go +++ b/internal/storage/repository_test.go @@ -3,6 +3,7 @@ package storage import ( "os" "path/filepath" + "reflect" "testing" "github.com/vitorhugo-dotnet/go-script-sql-runner/internal/profile" @@ -27,6 +28,12 @@ func TestRepositoryCRUDAndLayout(t *testing.T) { if err := repo.Save(p); err != nil { t.Fatalf("Save() error: %v", err) } + other := p + other.ID = "profile-2" + other.Name = "Other Profile" + if err := repo.Save(other); err != nil { + t.Fatalf("Save(other) error: %v", err) + } for _, path := range []string{ filepath.Join(root, "profiles", p.ID, "profile.yaml"), filepath.Join(root, "profiles", p.ID, "scripts"), @@ -42,7 +49,7 @@ func TestRepositoryCRUDAndLayout(t *testing.T) { t.Fatalf("Get() = %#v, %v", got, err) } list, err := repo.List() - if err != nil || len(list) != 1 || list[0].ID != p.ID { + if err != nil || len(list) != 2 || list[0].ID != other.ID || list[1].ID != p.ID { t.Fatalf("List() = %#v, %v", list, err) } if err := repo.Delete(p.ID); err != nil { @@ -51,6 +58,132 @@ func TestRepositoryCRUDAndLayout(t *testing.T) { if _, err := repo.Get(p.ID); err != ErrNotFound { t.Fatalf("Get() after delete error = %v, want ErrNotFound", err) } + if _, err := os.Stat(repo.ProfileDir(p.ID)); !os.IsNotExist(err) { + t.Fatalf("deleted profile directory remains: %v", err) + } + if got, err := repo.Get(other.ID); err != nil || got.ID != other.ID { + t.Fatalf("other profile after delete = %#v, %v", got, err) + } +} + +func TestDeleteRejectsInvalidIDs(t *testing.T) { + root := t.TempDir() + repo := NewRepository(root) + outside := filepath.Join(root, "outside") + if err := os.Mkdir(outside, 0o700); err != nil { + t.Fatal(err) + } + p := testProfile() + if err := repo.Save(p); err != nil { + t.Fatal(err) + } + for _, id := range []string{"../outside", ".", ""} { + t.Run(id, func(t *testing.T) { + if err := repo.Delete(id); err == nil { + t.Fatalf("Delete(%q) succeeded", id) + } + for _, dir := range []string{outside, repo.ProfileDir(p.ID)} { + if _, err := os.Stat(dir); err != nil { + t.Fatalf("Delete(%q) changed %q: %v", id, dir, err) + } + } + }) + } +} + +func TestRepositoryCloneCopiesManagedSQLAndIsolatesProfiles(t *testing.T) { + repo := NewRepository(t.TempDir()) + source := testProfile() + source.Scripts = []profile.Script{ + {ID: "one", Name: "First", File: "scripts/one.sql", Enabled: true, Order: 10, TransactionMode: profile.TransactionRunnerManaged}, + {ID: "two", Name: "Second", File: "scripts/two.sql", Enabled: false, Order: 20, TransactionMode: profile.TransactionScriptManaged}, + } + if err := repo.Save(source); err != nil { + t.Fatal(err) + } + contents := map[string]string{"one.sql": "SELECT 1;\n", "two.sql": "SELECT 2;\n"} + for name, data := range contents { + if err := os.WriteFile(filepath.Join(repo.ScriptRoot(source.ID), name), []byte(data), 0o600); err != nil { + t.Fatal(err) + } + } + clone := source + clone.ID = "copy-1" + clone.Name += " (copy)" + if err := repo.Clone(source.ID, clone); err != nil { + t.Fatalf("Clone() error: %v", err) + } + got, err := repo.Get(clone.ID) + if err != nil || !reflect.DeepEqual(got, clone) { + t.Fatalf("cloned metadata = %#v, %v; want %#v", got, err, clone) + } + for name, data := range contents { + copied, err := os.ReadFile(filepath.Join(repo.ScriptRoot(clone.ID), name)) + if err != nil || string(copied) != data { + t.Fatalf("cloned %s = %q, %v", name, copied, err) + } + } + if err := os.WriteFile(filepath.Join(repo.ScriptRoot(clone.ID), "one.sql"), []byte("changed"), 0o600); err != nil { + t.Fatal(err) + } + if err := repo.RemoveScript(clone.ID, "two"); err != nil { + t.Fatal(err) + } + for name, data := range contents { + original, err := os.ReadFile(filepath.Join(repo.ScriptRoot(source.ID), name)) + if err != nil || string(original) != data { + t.Fatalf("source %s after clone mutation = %q, %v", name, original, err) + } + } + if err := repo.Clone(source.ID, clone); err == nil { + t.Fatal("Clone() replaced an existing destination") + } + if got, err := repo.Get(clone.ID); err != nil || len(got.Scripts) != 1 { + t.Fatalf("existing destination changed: %#v, %v", got, err) + } +} + +func TestRepositoryCloneCleansUpAfterSourceErrors(t *testing.T) { + for _, tc := range []struct { + name string + setup func(string) error + }{ + {name: "missing file", setup: func(string) error { return nil }}, + {name: "directory as SQL file", setup: func(path string) error { return os.Mkdir(path, 0o700) }}, + } { + t.Run(tc.name, func(t *testing.T) { + repo := NewRepository(t.TempDir()) + source := testProfile() + source.Scripts = []profile.Script{ + {ID: "one", Name: "First", File: "scripts/one.sql", Enabled: true, Order: 10}, + {ID: "two", Name: "Second", File: "scripts/two.sql", Enabled: true, Order: 20}, + } + if err := repo.Save(source); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo.ScriptRoot(source.ID), "one.sql"), []byte("SELECT 1;"), 0o600); err != nil { + t.Fatal(err) + } + if err := tc.setup(filepath.Join(repo.ScriptRoot(source.ID), "two.sql")); err != nil { + t.Fatal(err) + } + clone := source + clone.ID = "copy-1" + if err := repo.Clone(source.ID, clone); err == nil { + t.Fatal("Clone() succeeded with an invalid source script") + } + if _, err := os.Stat(repo.ProfileDir(clone.ID)); !os.IsNotExist(err) { + t.Fatalf("partial destination remains: %v", err) + } + entries, err := os.ReadDir(filepath.Join(repo.Root(), "profiles")) + if err != nil || len(entries) != 1 || entries[0].Name() != source.ID { + t.Fatalf("staging directory remains after clone failure: %#v, %v", entries, err) + } + if got, err := repo.Get(source.ID); err != nil || !reflect.DeepEqual(got, source) { + t.Fatalf("source changed: %#v, %v", got, err) + } + }) + } } func TestAddRemoveAndReorderScripts(t *testing.T) { From f09aedadcfaa2b86f6ce50bd5b67ac66f32795ed Mon Sep 17 00:00:00 2001 From: vitorhugo-dotnet Date: Fri, 25 Sep 2026 11:52:00 -0300 Subject: [PATCH 04/31] fix: prevent reconnecting deleted profiles --- internal/app/profile_lifecycle_test.go | 38 ++++++++++++++++++++++++++ internal/app/service.go | 4 +++ 2 files changed, 42 insertions(+) diff --git a/internal/app/profile_lifecycle_test.go b/internal/app/profile_lifecycle_test.go index 892a791..f2cd357 100644 --- a/internal/app/profile_lifecycle_test.go +++ b/internal/app/profile_lifecycle_test.go @@ -116,6 +116,44 @@ func TestDeleteProfileClosesOnlyMatchingActiveConnection(t *testing.T) { } } +func TestConnectDoesNotPublishAfterProfileDeletion(t *testing.T) { + repo := storage.NewRepository(t.TempDir()) + service := NewService(repo) + p, err := service.CreateProfile(context.Background(), lifecycleProfile("Concurrent")) + if err != nil { + t.Fatal(err) + } + started := make(chan struct{}) + release := make(chan struct{}) + recorded := &recordingConn{} + service.connectServer = func(ctx context.Context, _ profile.Connection) (*database.Client, database.ConnectionResult, error) { + close(started) + <-release + db := sql.OpenDB(recordingConnector{conn: recorded}) + if err := db.PingContext(ctx); err != nil { + return nil, database.ConnectionResult{}, err + } + return &database.Client{DB: db}, database.ConnectionResult{}, nil + } + connected := make(chan error, 1) + go func() { + _, err := service.Connect(context.Background(), p.ID) + connected <- err + }() + <-started + if err := service.DeleteProfile(context.Background(), p.ID); err != nil { + close(release) + t.Fatalf("DeleteProfile() error: %v", err) + } + close(release) + if err := <-connected; !errors.Is(err, storage.ErrNotFound) { + t.Fatalf("Connect() after deletion error = %v, want ErrNotFound", err) + } + if service.activeClient != nil || service.activeProfileID != "" || recorded.closed != 1 { + t.Fatalf("deleted profile reconnected: client=%#v id=%q closes=%d", service.activeClient, service.activeProfileID, recorded.closed) + } +} + func TestProfileLifecycleCancelledContextDoesNotMutate(t *testing.T) { repo := storage.NewRepository(t.TempDir()) service := NewService(repo) diff --git a/internal/app/service.go b/internal/app/service.go index deb23e5..a1eb49f 100644 --- a/internal/app/service.go +++ b/internal/app/service.go @@ -64,6 +64,10 @@ func (s *Service) Connect(ctx context.Context, profileID string) (database.Conne s.connectionMu.Unlock() return database.ConnectionResult{}, errors.Join(fmt.Errorf("service is closed"), client.Close()) } + if _, err := s.repository.Get(profileID); err != nil { + s.connectionMu.Unlock() + return database.ConnectionResult{}, errors.Join(fmt.Errorf("get profile %q before publishing connection: %w", profileID, err), client.Close()) + } previous := s.activeClient s.activeClient = client s.activeProfileID = profileID From f1f8f0ecc3dae001370e0a74cb459f6910f78596 Mon Sep 17 00:00:00 2001 From: vitorhugo-dotnet Date: Fri, 25 Sep 2026 11:58:22 -0300 Subject: [PATCH 05/31] fix: reject profile path aliases and preserve delete success --- internal/app/profile_lifecycle.go | 4 +- internal/app/profile_lifecycle_test.go | 61 +++++++++++++++++++++++--- internal/storage/repository.go | 15 ++++++- internal/storage/repository_test.go | 35 ++++++++++++++- 4 files changed, 105 insertions(+), 10 deletions(-) diff --git a/internal/app/profile_lifecycle.go b/internal/app/profile_lifecycle.go index c6e8652..079ddca 100644 --- a/internal/app/profile_lifecycle.go +++ b/internal/app/profile_lifecycle.go @@ -46,9 +46,7 @@ func (s *Service) DeleteProfile(ctx context.Context, profileID string) error { client := s.activeClient s.activeClient = nil s.activeProfileID = "" - if err := client.Close(); err != nil { - return fmt.Errorf("close connection for deleted profile %q: %w", profileID, err) - } + _ = client.Close() } return nil } diff --git a/internal/app/profile_lifecycle_test.go b/internal/app/profile_lifecycle_test.go index f2cd357..ee6d616 100644 --- a/internal/app/profile_lifecycle_test.go +++ b/internal/app/profile_lifecycle_test.go @@ -3,6 +3,7 @@ package app import ( "context" "database/sql" + "database/sql/driver" "errors" "os" "path/filepath" @@ -22,6 +23,21 @@ func lifecycleProfile(name string) profile.Profile { } } +type closeFailConn struct { + recordingConn + err error +} + +func (c *closeFailConn) Close() error { + _ = c.recordingConn.Close() + return c.err +} + +type closeFailConnector struct{ conn *closeFailConn } + +func (c closeFailConnector) Connect(context.Context) (driver.Conn, error) { return c.conn, nil } +func (c closeFailConnector) Driver() driver.Driver { return recordingDriver{} } + func TestCloneProfileCopiesSettingsScriptsAndGeneratesUniqueIDs(t *testing.T) { repo := storage.NewRepository(t.TempDir()) service := NewService(repo) @@ -99,11 +115,16 @@ func TestDeleteProfileClosesOnlyMatchingActiveConnection(t *testing.T) { if _, err := repo.Get(other.ID); !errors.Is(err, storage.ErrNotFound) { t.Fatalf("deleted profile Get() error = %v", err) } - if err := service.DeleteProfile(context.Background(), "../outside"); err == nil { - t.Fatal("DeleteProfile(invalid ID) succeeded") - } - if recorded.closed != 0 || service.activeClient == nil { - t.Fatal("failed deletion changed active connection") + for _, invalidID := range []string{"../outside", active.ID + ".", active.ID + " "} { + if err := service.DeleteProfile(context.Background(), invalidID); err == nil { + t.Fatalf("DeleteProfile(%q) succeeded", invalidID) + } + if recorded.closed != 0 || service.activeClient == nil { + t.Fatalf("failed deletion of %q changed active connection", invalidID) + } + if got, err := repo.Get(active.ID); err != nil || got.ID != active.ID { + t.Fatalf("active profile after invalid deletion = %#v, %v", got, err) + } } if err := service.DeleteProfile(context.Background(), active.ID); err != nil { t.Fatalf("DeleteProfile(active) error: %v", err) @@ -116,6 +137,36 @@ func TestDeleteProfileClosesOnlyMatchingActiveConnection(t *testing.T) { } } +func TestDeleteProfileSucceedsWhenConnectionCloseFails(t *testing.T) { + repo := storage.NewRepository(t.TempDir()) + service := NewService(repo) + p, err := service.CreateProfile(context.Background(), lifecycleProfile("Close failure")) + if err != nil { + t.Fatal(err) + } + closeErr := errors.New("close failed") + conn := &closeFailConn{err: closeErr} + service.connectServer = func(ctx context.Context, _ profile.Connection) (*database.Client, database.ConnectionResult, error) { + db := sql.OpenDB(closeFailConnector{conn: conn}) + if err := db.PingContext(ctx); err != nil { + return nil, database.ConnectionResult{}, err + } + return &database.Client{DB: db}, database.ConnectionResult{}, nil + } + if _, err := service.Connect(context.Background(), p.ID); err != nil { + t.Fatal(err) + } + if err := service.DeleteProfile(context.Background(), p.ID); err != nil { + t.Fatalf("DeleteProfile() returned a close error after deleting profile: %v", err) + } + if _, err := repo.Get(p.ID); !errors.Is(err, storage.ErrNotFound) { + t.Fatalf("deleted profile Get() error = %v", err) + } + if conn.closed != 1 || service.activeClient != nil || service.activeProfileID != "" { + t.Fatalf("connection state after deletion: closes=%d client=%#v id=%q", conn.closed, service.activeClient, service.activeProfileID) + } +} + func TestConnectDoesNotPublishAfterProfileDeletion(t *testing.T) { repo := storage.NewRepository(t.TempDir()) service := NewService(repo) diff --git a/internal/storage/repository.go b/internal/storage/repository.go index 4bf2874..32e2d73 100644 --- a/internal/storage/repository.go +++ b/internal/storage/repository.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "reflect" + "regexp" "sort" "strings" @@ -16,6 +17,8 @@ import ( var ErrNotFound = errors.New("profile not found") +var canonicalProfileID = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$`) + type Repository struct { root string } @@ -35,6 +38,9 @@ func (r *Repository) ensure() error { } func (r *Repository) Save(p profile.Profile) error { + if !validLookupID(p.ID) { + return fmt.Errorf("invalid profile id") + } if err := profile.Validate(p); err != nil { return err } @@ -341,7 +347,14 @@ func atomicWrite(path string, data []byte, mode os.FileMode) error { } func validLookupID(id string) bool { - if id == "" || id == "." || id == ".." { + if !canonicalProfileID.MatchString(id) || strings.HasSuffix(id, ".") { + return false + } + stem := strings.ToUpper(strings.SplitN(id, ".", 2)[0]) + if stem == "CON" || stem == "PRN" || stem == "AUX" || stem == "NUL" { + return false + } + if len(stem) == 4 && (stem[:3] == "COM" || stem[:3] == "LPT") && stem[3] >= '1' && stem[3] <= '9' { return false } return filepath.Base(id) == id && filepath.Clean(id) == id diff --git a/internal/storage/repository_test.go b/internal/storage/repository_test.go index 0a321c9..c17219a 100644 --- a/internal/storage/repository_test.go +++ b/internal/storage/repository_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "reflect" + "strings" "testing" "github.com/vitorhugo-dotnet/go-script-sql-runner/internal/profile" @@ -77,7 +78,7 @@ func TestDeleteRejectsInvalidIDs(t *testing.T) { if err := repo.Save(p); err != nil { t.Fatal(err) } - for _, id := range []string{"../outside", ".", ""} { + for _, id := range []string{"../outside", ".", "", p.ID + ".", p.ID + " ", " " + p.ID, "CON", "nul.txt", "LPT1"} { t.Run(id, func(t *testing.T) { if err := repo.Delete(id); err == nil { t.Fatalf("Delete(%q) succeeded", id) @@ -91,6 +92,38 @@ func TestDeleteRejectsInvalidIDs(t *testing.T) { } } +func TestRepositoryCloneRejectsWindowsAliasIDs(t *testing.T) { + repo := NewRepository(t.TempDir()) + source := testProfile() + if err := repo.Save(source); err != nil { + t.Fatal(err) + } + for _, alias := range []string{source.ID + ".", source.ID + " ", " " + source.ID} { + t.Run(alias, func(t *testing.T) { + invalid := source + invalid.ID = alias + if err := repo.Save(invalid); err == nil || !strings.Contains(err.Error(), "invalid profile id") { + t.Fatalf("Save(alias %q) error = %v, want invalid profile id", alias, err) + } + clone := source + clone.ID = "copy-1" + if err := repo.Clone(alias, clone); err == nil || !strings.Contains(err.Error(), "invalid profile id") { + t.Fatalf("Clone(alias source %q) error = %v, want invalid profile id", alias, err) + } + clone.ID = alias + if err := repo.Clone(source.ID, clone); err == nil || !strings.Contains(err.Error(), "invalid profile id") { + t.Fatalf("Clone(alias destination %q) error = %v, want invalid profile id", alias, err) + } + if got, err := repo.Get(source.ID); err != nil || got.ID != source.ID { + t.Fatalf("genuine profile changed after clone aliases: %#v, %v", got, err) + } + if _, err := os.Stat(repo.ProfileDir("copy-1")); !os.IsNotExist(err) { + t.Fatalf("clone was published from alias: %v", err) + } + }) + } +} + func TestRepositoryCloneCopiesManagedSQLAndIsolatesProfiles(t *testing.T) { repo := NewRepository(t.TempDir()) source := testProfile() From ddd26c59ff88bf1f9915bbe31d9652afd6c64c21 Mon Sep 17 00:00:00 2001 From: vitorhugo-dotnet Date: Fri, 25 Sep 2026 12:02:29 -0300 Subject: [PATCH 06/31] fix: require exact profile identity before deletion --- internal/app/profile_lifecycle_test.go | 7 +++-- internal/storage/repository.go | 16 +++++++++++ internal/storage/repository_test.go | 39 ++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/internal/app/profile_lifecycle_test.go b/internal/app/profile_lifecycle_test.go index ee6d616..bb5459c 100644 --- a/internal/app/profile_lifecycle_test.go +++ b/internal/app/profile_lifecycle_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "reflect" + "strings" "testing" "github.com/vitorhugo-dotnet/go-script-sql-runner/internal/database" @@ -87,7 +88,9 @@ func TestCloneProfileCopiesSettingsScriptsAndGeneratesUniqueIDs(t *testing.T) { func TestDeleteProfileClosesOnlyMatchingActiveConnection(t *testing.T) { repo := storage.NewRepository(t.TempDir()) service := NewService(repo) - active, err := service.CreateProfile(context.Background(), lifecycleProfile("Active")) + activeInput := lifecycleProfile("Active") + activeInput.ID = "case-sensitive" + active, err := service.CreateProfile(context.Background(), activeInput) if err != nil { t.Fatal(err) } @@ -115,7 +118,7 @@ func TestDeleteProfileClosesOnlyMatchingActiveConnection(t *testing.T) { if _, err := repo.Get(other.ID); !errors.Is(err, storage.ErrNotFound) { t.Fatalf("deleted profile Get() error = %v", err) } - for _, invalidID := range []string{"../outside", active.ID + ".", active.ID + " "} { + for _, invalidID := range []string{"../outside", active.ID + ".", active.ID + " ", strings.ToUpper(active.ID)} { if err := service.DeleteProfile(context.Background(), invalidID); err == nil { t.Fatalf("DeleteProfile(%q) succeeded", invalidID) } diff --git a/internal/storage/repository.go b/internal/storage/repository.go index 32e2d73..6ae1031 100644 --- a/internal/storage/repository.go +++ b/internal/storage/repository.go @@ -44,6 +44,16 @@ func (r *Repository) Save(p profile.Profile) error { if err := profile.Validate(p); err != nil { return err } + if info, err := os.Lstat(r.ProfileDir(p.ID)); err == nil { + if !info.IsDir() { + return fmt.Errorf("profile %q path is not a directory", p.ID) + } + if _, err := r.Get(p.ID); err != nil && !errors.Is(err, ErrNotFound) { + return fmt.Errorf("load existing profile %q before save: %w", p.ID, err) + } + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("check existing profile %q: %w", p.ID, err) + } if err := r.ensure(); err != nil { return err } @@ -74,6 +84,9 @@ func (r *Repository) Get(id string) (profile.Profile, error) { if err != nil { return profile.Profile{}, err } + if p.ID != id { + return profile.Profile{}, fmt.Errorf("profile id mismatch: requested %q, stored %q", id, p.ID) + } return p, nil } @@ -109,6 +122,9 @@ func (r *Repository) Delete(id string) error { if !validLookupID(id) { return fmt.Errorf("invalid profile id") } + if _, err := r.Get(id); err != nil { + return fmt.Errorf("load profile %q before delete: %w", id, err) + } if err := os.RemoveAll(r.ProfileDir(id)); err != nil { return fmt.Errorf("delete profile: %w", err) } diff --git a/internal/storage/repository_test.go b/internal/storage/repository_test.go index c17219a..8978a56 100644 --- a/internal/storage/repository_test.go +++ b/internal/storage/repository_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "reflect" + "runtime" "strings" "testing" @@ -124,6 +125,44 @@ func TestRepositoryCloneRejectsWindowsAliasIDs(t *testing.T) { } } +func TestRepositoryRejectsCaseAliasIdentity(t *testing.T) { + repo := NewRepository(t.TempDir()) + source := testProfile() + if err := repo.Save(source); err != nil { + t.Fatal(err) + } + alias := strings.ToUpper(source.ID) + if _, err := repo.Get(alias); err == nil { + t.Fatalf("Get(%q) accepted an alias for %q", alias, source.ID) + } + clone := source + clone.ID = "copy-1" + if err := repo.Clone(alias, clone); err == nil { + t.Fatalf("Clone(%q) accepted an alias source", alias) + } + if runtime.GOOS == "windows" { + aliasedSave := source + aliasedSave.ID = alias + if err := repo.Save(aliasedSave); err == nil { + t.Fatalf("Save(%q) replaced an existing profile via alias", alias) + } + aliasedDestination := source + aliasedDestination.ID = alias + if err := repo.Clone(source.ID, aliasedDestination); err == nil { + t.Fatalf("Clone(destination %q) replaced an existing profile via alias", alias) + } + } + if err := repo.Delete(alias); err == nil { + t.Fatalf("Delete(%q) accepted an alias for %q", alias, source.ID) + } + if got, err := repo.Get(source.ID); err != nil || !reflect.DeepEqual(got, source) { + t.Fatalf("genuine profile changed after case aliases: %#v, %v", got, err) + } + if _, err := os.Stat(repo.ProfileDir("copy-1")); !os.IsNotExist(err) { + t.Fatalf("clone was published from case alias: %v", err) + } +} + func TestRepositoryCloneCopiesManagedSQLAndIsolatesProfiles(t *testing.T) { repo := NewRepository(t.TempDir()) source := testProfile() From 214b39bb779365f78e68219d41aa024e7a9fc754 Mon Sep 17 00:00:00 2001 From: vitorhugo-dotnet Date: Fri, 25 Sep 2026 12:07:17 -0300 Subject: [PATCH 07/31] feat: expose profile lifecycle actions to desktop --- frontend/src/api/runner.ts | 4 + frontend/src/api/types.ts | 2 + .../src/state/useRunnerController.test.tsx | 90 +++++++++++++++++++ frontend/src/state/useRunnerController.ts | 34 +++++++ internal/ui/bridge.go | 10 +++ internal/ui/bridge_test.go | 42 +++++++++ internal/ui/wails/desktop_app.go | 16 ++++ 7 files changed, 198 insertions(+) diff --git a/frontend/src/api/runner.ts b/frontend/src/api/runner.ts index 283b505..47f8008 100644 --- a/frontend/src/api/runner.ts +++ b/frontend/src/api/runner.ts @@ -15,6 +15,8 @@ interface DesktopBinding { GetProfile(profileID: string): Promise CreateProfile(profile: Profile): Promise UpdateProfile(profile: Profile): Promise + DeleteProfile(profileID: string): Promise + CloneProfile(profileID: string): Promise AddScriptFromDialog(profileID: string): Promise