From 66a5949c29ba188352c5d1c34d5d82197b986ce0 Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Mon, 17 Aug 2026 23:16:35 -0400 Subject: [PATCH] fix: desktop Export downloads -- native save dialog for the Wails webview Every Export button's Blob + synthetic-anchor-click download was silently inert on the installed desktop build: the Wails webview supplies no download handler for a synthetic anchor click, despite downloadJSON.ts's own header comment claiming webview parity. Server mode (a real browser tab) was never affected. downloadJSON now routes by runtime: desktop calls the new SettingsService.SaveTextFile binding (a native OS save dialog, backed by Wails3's own Dialog manager) and writes the chosen path directly; server mode keeps the existing Blob + anchor path unchanged. CompositionView's separate inline copy of the same broken mechanism is retrofitted onto the shared helper instead of carrying its own fix. Also fixes lefthook.yml's golangci-lint job to pass --build-tags=server, matching ci.yml's own args -- without it the local hook lints a wider (desktop-tag) file set than CI's job ever compiles, so it can fail on pre-existing findings CI's real gate never sees. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd --- .../services/settingssvc/settingsservice.ts | 20 ++++++++++ frontend/src/composition/CompositionView.tsx | 15 ++----- frontend/src/shared/downloadJSON.ts | 31 ++++++++------ .../settingssvc/settingsservice_savefile.go | 40 +++++++++++++++++++ .../settingsservice_savefile_test.go | 31 ++++++++++++++ lefthook.yml | 8 +++- 6 files changed, 121 insertions(+), 24 deletions(-) create mode 100644 internal/services/settingssvc/settingsservice_savefile.go create mode 100644 internal/services/settingssvc/settingsservice_savefile_test.go diff --git a/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts b/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts index 7a5d48df..82a5c8d6 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts @@ -356,6 +356,26 @@ export function RestoreSummonHotkey(): $CancellablePromise { return $Call.ByID(442997791); } +/** + * SaveTextFile prompts with the OS-native save dialog (suggestedName + * pre-filled) and writes content to whatever path the user picks -- + * the desktop-mode counterpart to a browser's own download prompt, + * which the Wails webview does not provide (an anchor-element download + * click is silently inert there). Returns the chosen path, or "" with + * a nil error when the user cancels. Server mode and any other run + * without a live windowed application (a headless `go test`, in + * particular) return a nil app from application.Get() -- the same + * no-op guard atlasservice_share.go's revealPath uses -- and this + * method reports that as an error rather than a silent no-op, since + * unlike an OS-reveal action a caller genuinely needs to know the + * save never happened. A real `-tags server` build additionally + * degrades through Wails3's own server-mode dialog stub, which + * already returns an equivalent "not available" error. + */ +export function SaveTextFile(suggestedName: string, content: string): $CancellablePromise { + return $Call.ByID(891957520, suggestedName, content); +} + /** * SetAttentionIdleThreshold persists seconds (a non-positive value * resets to the default, mirroring a cleared Settings field rather than diff --git a/frontend/src/composition/CompositionView.tsx b/frontend/src/composition/CompositionView.tsx index 42c2555f..57079ad9 100644 --- a/frontend/src/composition/CompositionView.tsx +++ b/frontend/src/composition/CompositionView.tsx @@ -12,6 +12,7 @@ import { InventoryList, type InventoryItem } from '../shared/InventoryList' import { ENTITY_ICON } from '../shared/entityIcons' import { formatUpdated, sortByUpdatedDesc } from '../shared/inventorySort' import { describeSeedReset } from '../shared/seedLifecycle' +import { downloadJSON } from '../shared/downloadJSON' import { RestoreExamplesButton } from '../shared/RestoreExamplesButton' import { useImportConfirm } from '../shared/useImportConfirm' import type { Workflow } from '../../bindings/github.com/alicoding/mill/internal/domain/composition/models' @@ -216,19 +217,11 @@ function CompositionView() { // Downloads id's current definition as a portable .json file -- // ExportWorkflow's own doc comment covers why the output is - // deterministic. A Blob + synthetic anchor click is the standard - // browser download mechanism, identical inside the Wails webview. + // deterministic; downloadJSON routes the actual save through the + // right mechanism for the current runtime. const exportWorkflow = (id: string, label: string) => { CompositionService.ExportWorkflow(id) - .then((json) => { - const blob = new Blob([json], { type: 'application/json' }) - const url = URL.createObjectURL(blob) - const a = document.createElement('a') - a.href = url - a.download = `${label.trim() || 'workflow'}.json` - a.click() - URL.revokeObjectURL(url) - }) + .then((json) => downloadJSON(`${label.trim() || 'workflow'}.json`, json)) .catch((err) => setImportError(String(err))) } diff --git a/frontend/src/shared/downloadJSON.ts b/frontend/src/shared/downloadJSON.ts index 2fba1f58..e1f152be 100644 --- a/frontend/src/shared/downloadJSON.ts +++ b/frontend/src/shared/downloadJSON.ts @@ -1,15 +1,22 @@ -// The standard browser Blob + synthetic-anchor-click download mechanism, -// used identically by every Configure entity's Export button -// (ConfigureRequests/Lists/MCPServers.tsx) -- extracted once the same -// ~8 lines were about to be written a third time (CLAUDE.md: three -// similar lines is fine, a fourth repetition is the actual signal to -// share it). Works the same inside Mill's own Wails webview as it does -// in a normal browser tab, no native file-save API needed. Composition's -// own Export button (CompositionView.tsx, built first) keeps its -// original inline version rather than being retrofitted to this -- -// already shipped and tested, and churning it for a marginal DRY gain -// isn't worth the re-verification cost. -export function downloadJSON(filename: string, json: string) { +import { SettingsService } from './bindings' + +// Every Export button's download mechanism (Atlas toolbar, Configure +// Requests/Lists/MCPServers, and CompositionView's own workflow +// export) routes through this one function. A synthetic anchor-click +// download needs the browser to supply a download handler -- the +// Wails webview doesn't provide one, so the anchor click is silently +// inert there even though nothing about it errors. Desktop mode +// therefore goes through SettingsService.SaveTextFile (the OS-native +// save dialog); server mode (a real browser tab) keeps the standard +// Blob + synthetic-anchor-click path, which works there exactly as it +// does in any other web app. +export async function downloadJSON(filename: string, json: string): Promise { + const buildInfo = await SettingsService.GetBuildInfo().catch(() => null) + const isNativeWebview = buildInfo != null && !buildInfo.Server + if (isNativeWebview) { + await SettingsService.SaveTextFile(filename, json) + return + } const blob = new Blob([json], { type: 'application/json' }) const url = URL.createObjectURL(blob) const a = document.createElement('a') diff --git a/internal/services/settingssvc/settingsservice_savefile.go b/internal/services/settingssvc/settingsservice_savefile.go new file mode 100644 index 00000000..f6b99a47 --- /dev/null +++ b/internal/services/settingssvc/settingsservice_savefile.go @@ -0,0 +1,40 @@ +package settingssvc + +import ( + "fmt" + "os" + + "github.com/wailsapp/wails/v3/pkg/application" +) + +// SaveTextFile prompts with the OS-native save dialog (suggestedName +// pre-filled) and writes content to whatever path the user picks -- +// the desktop-mode counterpart to a browser's own download prompt, +// which the Wails webview does not provide (an anchor-element download +// click is silently inert there). Returns the chosen path, or "" with +// a nil error when the user cancels. Server mode and any other run +// without a live windowed application (a headless `go test`, in +// particular) return a nil app from application.Get() -- the same +// no-op guard atlasservice_share.go's revealPath uses -- and this +// method reports that as an error rather than a silent no-op, since +// unlike an OS-reveal action a caller genuinely needs to know the +// save never happened. A real `-tags server` build additionally +// degrades through Wails3's own server-mode dialog stub, which +// already returns an equivalent "not available" error. +func (s *SettingsService) SaveTextFile(suggestedName, content string) (string, error) { + app := application.Get() + if app == nil { + return "", fmt.Errorf("native file save is not available in this mode") + } + path, err := app.Dialog.SaveFile().SetFilename(suggestedName).PromptForSingleSelection() + if err != nil { + return "", fmt.Errorf("native file save is not available in this mode: %w", err) + } + if path == "" { + return "", nil + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + return "", fmt.Errorf("write file: %w", err) + } + return path, nil +} diff --git a/internal/services/settingssvc/settingsservice_savefile_test.go b/internal/services/settingssvc/settingsservice_savefile_test.go new file mode 100644 index 00000000..8dcd3de5 --- /dev/null +++ b/internal/services/settingssvc/settingsservice_savefile_test.go @@ -0,0 +1,31 @@ +package settingssvc + +import ( + "log/slog" + "testing" + + "github.com/alicoding/mill/internal/services/compositionsvc" + "github.com/alicoding/mill/internal/services/servicetest" + "github.com/alicoding/mill/internal/services/triggersvc" +) + +// TestSaveTextFile_NoLiveApplicationErrors: a headless `go test` run +// never calls application.New, so application.Get() is nil -- the same +// condition revealPath (atlasservice_share.go) treats as a silent +// no-op. SaveTextFile must instead report a real error rather than +// panic on the nil app, since a caller cannot tell "cancelled" apart +// from "never ran" without one. +func TestSaveTextFile_NoLiveApplicationErrors(t *testing.T) { + store := servicetest.NewFakeStore() + comp := compositionsvc.NewCompositionService(store) + trig := triggersvc.NewTriggerService(comp, slog.Default(), store) + set := NewSettingsService(store, trig, false) + + path, err := set.SaveTextFile("export.json", `{"a":1}`) + if err == nil { + t.Fatal("SaveTextFile() with no live application: want an error, got nil") + } + if path != "" { + t.Errorf("SaveTextFile() path = %q, want empty on error", path) + } +} diff --git a/lefthook.yml b/lefthook.yml index 56a9d5ac..a79c2170 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -49,7 +49,13 @@ pre-commit: run: go vet . ./internal/... - name: golangci-lint glob: "*.go" - run: golangci-lint run . ./internal/... + # --build-tags=server matches ci.yml's own golangci-lint-action + # args exactly -- without it this job lints the DEFAULT (desktop) + # tag set, a strictly larger file set than CI's job ever compiles + # (`!server`-tagged files CI's own -tags=server run never sees), + # so a local pass here was checking something CI doesn't and a + # local fail could block a commit CI's real gate would accept. + run: golangci-lint run . ./internal/... --build-tags=server - name: go-test glob: "*.go" # Root package (`.`) included alongside ./internal/... -- the