From f300bdadd6451ca92a06df09de1e7bac6ca97866 Mon Sep 17 00:00:00 2001 From: Vigneswaran Rajkumar <118706051+IAmVigneswaran@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:44:01 +0800 Subject: [PATCH 1/3] Fix Pagemaker alert presentation and Duplicate profile error reporting. Pagemaker's JavaScript panels now use the Icon Composer icon via a shared makeAlert, drop the generic Alert and Confirm titles, return the pressed button from confirm, and stop force-unwrapping the key window. Database profile duplication no longer swallows its error inside a Task, so the Databases alert reports why a duplicate was refused. Agent docs match. --- .cursorrules | 4 +- AGENT.md | 9 +-- ARCHITECTURE.md | 6 +- GUARDRAILS.md | 6 +- .../Models/Database/DatabaseManager.swift | 24 ++++---- .../Models/Errors/DatabaseErrors.swift | 11 ++++ .../Pagemaker/PagemakerUIDelegate.swift | 55 +++++++++++++------ .../Database/DatabaseSettingsView.swift | 6 +- .../Views/Extensions/DialogIcon.swift | 5 ++ 9 files changed, 85 insertions(+), 41 deletions(-) diff --git a/.cursorrules b/.cursorrules index 66f74523..2722aedf 100644 --- a/.cursorrules +++ b/.cursorrules @@ -54,7 +54,7 @@ Keep these four files consistent when architecture or agent guidance changes. - Do not assume behavior of `airlift` / `csv2notion_neo` beyond CLI usage in `DatabaseUploader` (and Dropbox setup in `DropboxSetupModel`). - Do not break Share Destination scripting (`Resources/OSAScriptingDefinition.sdef` + Obj‑C) or Workflow Extension disk/DNC contracts. - The app warns if not run from `/Applications`; do not remove that check (extension opens that path). -- SwiftUI `.alert` / `.confirmationDialog` icons: chain `.appDialogIcon()` after every dialog. `MarkerDataAppIcon` is `@MainActor` (`DialogIcon.swift`). Main-app Dock / About / alerts use Icon Composer `Marker-Data.icon` (`ASSETCATALOG_COMPILER_APPICON_NAME` = Marker-Data). Workflow Extension **header** loads the icon from the containing `Marker Data.app` (walk `appex` → `PlugIns` → `Contents` → `.app`); the appex’s `applicationIconImage` is its own `AppIcon.appiconset` (or Final Cut Pro). Do not flatten the layer PNG into `AppIconSingle`. Do not replace the Workflow Extension bundle/plugin `AppIcon.appiconset`. Uninstaller uses `applicationIconImage`. +- SwiftUI `.alert` / `.confirmationDialog` icons: chain `.appDialogIcon()` after every dialog. `MarkerDataAppIcon` is `@MainActor` (`DialogIcon.swift`). AppKit `NSAlert` needs `MarkerDataAppIcon.alertImage` — Pagemaker's WebView JavaScript panels are the only `NSAlert`s and must be built by `PagemakerUIDelegate.makeAlert(message:)` (no generic "Alert" / "Confirm" title; `.informational`, message as `messageText`). Main-app Dock / About / alerts use Icon Composer `Marker-Data.icon` (`ASSETCATALOG_COMPILER_APPICON_NAME` = Marker-Data). Workflow Extension **header** loads the icon from the containing `Marker Data.app` (walk `appex` → `PlugIns` → `Contents` → `.app`); the appex’s `applicationIconImage` is its own `AppIcon.appiconset` (or Final Cut Pro). Do not flatten the layer PNG into `AppIconSingle`. Do not replace the Workflow Extension bundle/plugin `AppIcon.appiconset`. Uninstaller uses `applicationIconImage`. - Database models use property name `plaform` — keep consistent unless intentionally migrating. - If adding new on-disk paths the app uses, update `MarkerDataUninstaller` cleanup paths too. - Do not upload Queue jobs using only absolute `ExtractInfo.jsonURL` when a same-named JSON exists in the dropped/scanned folder — use `manifestURL`. @@ -73,5 +73,5 @@ Keep these four files consistent when architecture or agent guidance changes. - File menu still replaces `.newItem` with app actions; system Close stays last (no second Close; do not empty `.newItem`). - Drop overlays still work on main-app Extract / Roles / Queue **and** Workflow Extension Extract + Roles; queue still finds/uploads `extract_info.json` folders via `manifestURL`. - Skipped / no-media swatch finishes as “Extract done” (not “Analysing swatch done”). -- New alerts **and** confirmation dialogs use `.appDialogIcon()`; Workflow Extension header still shows the main-app Icon Composer icon (not the appex `AppIcon.appiconset`); settings changes include version + migration + wiring as required above. +- New alerts **and** confirmation dialogs use `.appDialogIcon()` (`NSAlert` → `MarkerDataAppIcon.alertImage`); Workflow Extension header still shows the main-app Icon Composer icon (not the appex `AppIcon.appiconset`); settings changes include version + migration + wiring as required above. - Update `AGENT.md` / `ARCHITECTURE.md` / `GUARDRAILS.md` / `.cursorrules` when behavior changes. diff --git a/AGENT.md b/AGENT.md index 76d762d1..fb29041e 100644 --- a/AGENT.md +++ b/AGENT.md @@ -85,7 +85,7 @@ For deeper module/data-flow detail, see **`ARCHITECTURE.md`**. For hard always/n | Settings migrations | `Source/Marker Data/Marker Data/Models/Settings/SettingsVersioningManager.swift` | | Canonical disk paths | `Source/Marker Data/Marker Data/Utilities/Extensions/URLExtension.swift` | | Notification names | `Source/Marker Data/Marker Data/Utilities/Extensions/NotificationNameExtension.swift` | -| App / dialog icon | `Views/Extensions/DialogIcon.swift` — `@MainActor` `MarkerDataAppIcon` + `.appDialogIcon()`; Icon Composer `Marker-Data.icon` | +| App / dialog icon | `Views/Extensions/DialogIcon.swift` — `@MainActor` `MarkerDataAppIcon` + `.appDialogIcon()` (SwiftUI) / `alertImage` (AppKit `NSAlert`); Icon Composer `Marker-Data.icon` | | About | `Views/Detail Views/AboutView.swift` — `MarkerDataAppIcon.image()` at 200×200 | | Workflow Extension header icon | Same `MarkerDataAppIcon.image()` at 100×100; in the appex, resolve from containing `Marker Data.app` | | Apple Event open | `FCP Share Destination/OpenEventHandler.swift` — `kAEOpen` → `.openFile`; re-register on `.FCPShareStart` (main queue) | @@ -251,8 +251,9 @@ Also: getter for `colorSwatchSettings` **forces `enableSwatch = false` when extr - **Resolution (`displayIcon`):** 1. If running as `.appex`: load from the containing `Marker Data.app` (`appex` → `PlugIns` → `Contents` → `.app`): `Bundle.image(forResource: "Marker-Data")`, else `NSWorkspace.icon(forFile:)`. Do **not** use the appex’s `applicationIconImage` (that is `AppIcon.appiconset`, or Final Cut Pro). 2. Else: `NSImage(named: "Marker-Data")`, then `NSApplication.shared.applicationIconImage`. -- **Surfaces:** About `MarkerDataAppIcon.image()` **200×200**; Workflow Extension header **100×100**; alerts **and** confirmation dialogs `.appDialogIcon()`. +- **Surfaces:** About `MarkerDataAppIcon.image()` **200×200**; Workflow Extension header **100×100**; alerts **and** confirmation dialogs `.appDialogIcon()`; AppKit `NSAlert` `MarkerDataAppIcon.alertImage`. - **Main-app `.alert` / `.confirmationDialog` (chain `.appDialogIcon()`):** `Marker_DataApp` (library folders); `ContentView` (install location); `ExtractView` (extract + upload, ×2); `ConfigurationSettingsView` (unsaved-switch, unsaved-add, and delete confirmations + alert); `DatabaseSettingsView` (remove + duplicate alerts, ×2, plus delete-profile confirmation); `CreateDBProfileSheet`; `DropboxSetupView`; `InstallShareDestinationView`. +- **AppKit `NSAlert`:** only Pagemaker’s JavaScript panels. Build them through `PagemakerUIDelegate.makeAlert(message:)` — it applies `MarkerDataAppIcon.alertImage`, forces `.informational`, and puts the message in `messageText` (no generic “Alert” / “Confirm” title). Do not add bare `NSAlert()`s elsewhere; use SwiftUI `.alert` + `.appDialogIcon()`. - Uninstaller is a **separate target**: `UninstallerView` uses `NSApplication.shared.applicationIconImage`, not `MarkerDataAppIcon`. - Do **not** change the Workflow Extension’s bundle/plugin icon (`ASSETCATALOG_COMPILER_APPICON_NAME` = AppIcon, existing `AppIcon.appiconset`). - Workflow Extension Compile Sources must include `DialogIcon.swift` (header UI). Keep Extract/Roles `TabView` `.padding(.bottom, 40)` so `overlayHelpButton` does not sit on the drop zone. @@ -309,7 +310,7 @@ Exact overlay strings and always/never rules: **`GUARDRAILS.md`**. - **Queue relocated folders:** never upload only `ExtractInfo.jsonURL` — use `manifestURL` (see Signs in `GUARDRAILS.md`). - **Progress `reset()` / wrong swatch label:** never `reset()` before swatch; never retitle to “Analysing swatch” until render will run. Skipped swatch must finish as **“Extract done”**, not **“Analysing swatch done”**. `await applyTaskAppearance` from `ColorPaletteRenderer` (MainActor). - **FCP timeline → Dock:** often pasteboard-only; Extract panel drop is the supported UI path for timelines. -- **Alert / confirmation icons:** chain `.appDialogIcon()` after every `.alert` and `.confirmationDialog`. `MarkerDataAppIcon` is `@MainActor`; source is compiled `Marker-Data.icon`. Do not flatten the layer PNG into `AppIconSingle`. In the Workflow Extension header, load from the containing `Marker Data.app` — not the appex `AppIcon.appiconset`. +- **Alert / confirmation icons:** chain `.appDialogIcon()` after every `.alert` and `.confirmationDialog`; AppKit `NSAlert` needs `MarkerDataAppIcon.alertImage` instead (it does not inherit the Icon Composer icon on its own). `MarkerDataAppIcon` is `@MainActor`; source is compiled `Marker-Data.icon`. Do not flatten the layer PNG into `AppIconSingle`. In the Workflow Extension header, load from the containing `Marker Data.app` — not the appex `AppIcon.appiconset`. - **Dual Sparkle controllers:** both `Marker_DataApp` and `ApplicationDelegate` construct `SPUStandardUpdaterController`; update-available UI relies on `ApplicationDelegate.bestValidUpdate(...)` posting `.updateAvailable`. Don’t “simplify” without understanding that path. - **`OpenEventHandler`:** must re-register on `.FCPShareStart`; registration should stay on the main queue (see comments in that file). - **Shared `pbxproj` rows:** do not remove a second `DialogIcon.swift in Sources` (or other shared files) — the file is compiled into two targets. @@ -327,7 +328,7 @@ Exact overlay strings and always/never rules: **`GUARDRAILS.md`**. - Drop overlays still work on main-app Extract / Roles / Queue **and** Workflow Extension Extract + Roles. - Queue scan/upload still works for folders containing `extract_info.json`, including **moved/copied** folders (`manifestURL`). - Skipped / no-media swatch finishes as **“Extract done”** (not **“Analysing swatch done”**). -- Any new `.alert` or `.confirmationDialog` uses `.appDialogIcon()` (`MarkerDataAppIcon` from compiled `Marker-Data.icon`). +- Any new `.alert` or `.confirmationDialog` uses `.appDialogIcon()` (`MarkerDataAppIcon` from compiled `Marker-Data.icon`); any `NSAlert` goes through `PagemakerUIDelegate.makeAlert(message:)` / `MarkerDataAppIcon.alertImage`. - Workflow Extension header still shows the main-app Icon Composer icon (containing `Marker Data.app`); bundle/plugin `AppIcon.appiconset` unchanged. - If settings changed: version bumped + migration case + UI wired + export bridge if needed. - If behavior/architecture changed: update `AGENT.md`, `ARCHITECTURE.md`, `GUARDRAILS.md`, and `.cursorrules`. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1b6b48c1..54c67300 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -479,6 +479,7 @@ Settings model: `ColorSwatchSettingsModel` (nested under SettingsStore, Codable) - Setting a DB profile forces the MarkersExtractor export format for that platform so a JSON manifest exists - Validation: unique profile names; must not collide with extract-only format display names - Property spelling **`plaform`** is intentional in current code — preserve when editing +- `duplicateProfile(profileName:)` appends `" copy"` and runs `addProfile(saveToDisk: true)` synchronously on the main actor, so `DatabaseValidationError.nameAlreadyExists` reaches `DatabaseSettingsView`’s “Failed to duplicate profile” alert. Duplicating the same profile twice is refused by design (matching `ConfigurationsViewModel.duplicateConfiguration`, which also uses a flat `" copy"`); neither panel auto-increments. | Model | Notable fields | |-------|----------------| @@ -502,10 +503,12 @@ Notable modules under `Views/`: | Components | `DropTargetOverlay` (main app + WE), `FCPXMLDropModifier`, `HelpButton` / `OverlayHelpButton`, shared controls | | Extensions | **`MarkerDataAppIcon` / `.appDialogIcon()`** (`DialogIcon.swift`, `@MainActor`) — About, Workflow Extension header, alerts and confirmation dialogs; see **App icon and dialogs**. Also `ApplyPickerSizing`, `OptionalKeyboardShortcut`. | | Other | `FailedExtractionsView` (truncate + `.help()` tooltips; min ~640×240) | -| Pagemaker | `PagemakerView` WebView + `PagemakerPDFExportHandler` (JS → Swift PDF via `NSSavePanel`) | +| Pagemaker | `PagemakerView` WebView + `PagemakerPDFExportHandler` (JS → Swift PDF via `NSSavePanel`) + `PagemakerUIDelegate` (JavaScript `alert` / `confirm` / folder picker) | Install-location warning: on appear, if not under `/Applications` and `@AppStorage("ignoreInstallLocation")` is false, show alert (with `.appDialogIcon()`). +Pagemaker JavaScript panels are the app’s only `NSAlert`s. `PagemakerUIDelegate.makeAlert(message:)` is the single constructor: it sets `MarkerDataAppIcon.alertImage`, forces `.informational` (`NSAlert` defaults to `.warning`), and uses the JavaScript string as `messageText` — splitting at the first blank line so a trailing paragraph becomes `informativeText`. There is no generic “Alert” / “Confirm” / “Prompt” title. `present(_:)` sheets on `NSApp.keyWindow` and falls back to `runModal()`; `confirm` returns `.alertFirstButtonReturn`, so Cancel reaches JavaScript as `false`. + Drop overlay copy and invariants: **`GUARDRAILS.md`**. ```mermaid @@ -686,6 +689,7 @@ flowchart TB | About | `MarkerDataAppIcon.image()` — size **200×200** only in `AboutView` | | Workflow Extension header | `MarkerDataAppIcon.image()` — **100×100** in `WorkflowExtensionView` | | `.alert` / `.confirmationDialog` | `.appDialogIcon()` after every dialog. Call sites: `Marker_DataApp`, `ContentView`, `ExtractView` (×2), `ConfigurationSettingsView` (3 confirmations + alert), `DatabaseSettingsView` (×2 alerts + delete confirmation), `CreateDBProfileSheet`, `DropboxSetupView`, `InstallShareDestinationView`. Uninstaller uses `applicationIconImage`, not this helper. | +| AppKit `NSAlert` | `MarkerDataAppIcon.alertImage` — the AppKit counterpart of `.appDialogIcon()`. Only call site is `PagemakerUIDelegate.makeAlert(message:)`, which serves the three `WKUIDelegate` JavaScript panels (alert / confirm / prompt). | `Marker-Data.icon` lives in `Source/Marker Data/Marker Data/` beside the catalog. Workflow Extension `ASSETCATALOG_COMPILER_APPICON_NAME` remains **AppIcon** (existing PNG `AppIcon.appiconset`) until that plugin icon is updated separately. diff --git a/GUARDRAILS.md b/GUARDRAILS.md index 16977868..03c79f8d 100644 --- a/GUARDRAILS.md +++ b/GUARDRAILS.md @@ -19,7 +19,7 @@ Read with **`AGENT.md`** (how to change things), **`ARCHITECTURE.md`** (how it w |------|------| | **Settings** | Bump `SettingsStore.version`, add a dict migration `case`, wire UI, and pass export-related fields through `markersExtractorSettings(fcpxmlFileUrl:)`. The Workflow Extension compiles `SettingsStore` and decodes prefs **without** running migrations — main app must launch after a schema bump. | | **Config names** | Throw `ConfigurationSaveError.nameAlreadyExists` on add / rename / duplicate collisions — never overwrite `{name}.json` silently. | -| **Alerts** | Chain `.appDialogIcon()` after every `.alert` / `.confirmationDialog` (`DialogIcon.swift` → `@MainActor` `MarkerDataAppIcon` from compiled `Marker-Data.icon`). Uninstaller is a separate target and uses `applicationIconImage`. | +| **Alerts** | Chain `.appDialogIcon()` after every `.alert` / `.confirmationDialog` (`DialogIcon.swift` → `@MainActor` `MarkerDataAppIcon` from compiled `Marker-Data.icon`). AppKit `NSAlert` uses the counterpart `MarkerDataAppIcon.alertImage`; Pagemaker’s JavaScript panels are the only ones, and must be built by `PagemakerUIDelegate.makeAlert(message:)`. Uninstaller is a separate target and uses `applicationIconImage`. | | **Extract intake** | Route Finder files, text clippings, and Final Cut Pro pasteboard drops through `FCPXMLIntake` → `ExtractionModel.receiveFiles` / `receiveItemProviders`. | | **Drop overlays** | Use shared `DropTargetOverlay` on main-app Extract / Roles / Queue **and** Workflow Extension Extract + Roles. Shared files have **two** Compile Sources memberships in `project.pbxproj` (one file, two targets) — never delete one row. Extension membership includes overlay + `ColorExtension` + `DialogIcon.swift` + `HelpButton` / `OverlayHelpButton` plus Roles/settings files listed in **ARCHITECTURE.md**. | | **Shared WE chrome color** | Use `Color.markerAccent` (system indigo) for overlay borders and other shared chrome hosted in FCP — not `Color.accentColor`. | @@ -90,6 +90,8 @@ Shared component: `Views/Components/DropTargetOverlay.swift` (main app + Workflo 15. **Workflow Extension does not migrate settings.** `RolesManager` JSON-decodes `SettingsStore` with no `SettingsVersioningManager`. Open the main app after a schema bump. 16. **FCPXML `UTType` force-unwrap** — `UTType("com.apple.finalcutpro.xml")!` traps on Extract first layout when Final Cut Pro is absent (App Preview / review Macs). Use `importedAs` fallback and Info.plist imported types. Do not copy the old Marker Data force-unwrap. 17. **File menu Close last:** custom File items replace `.newItem`. Emptying `.newItem` makes system Close the first File item; do not duplicate Close. +18. **`Task { try await MainActor.run { … } }` inside a `throws` method swallows the error.** The task result is discarded, so the method always returns normally and the caller’s `catch` is dead. `DatabaseManager` is already `@MainActor` — call directly and let the error propagate. `duplicateProfile` was fixed this way; `setActiveProfile` and `loadProfilesFromDisk` still use the wrapper but throw nothing, so they are harmless. Do not reintroduce the pattern for anything that validates. +19. **`NSAlert` does not pick up the Icon Composer icon**, and its `alertStyle` defaults to `.warning`. Pagemaker’s WebView JavaScript panels are the app’s only `NSAlert`s — route them through `PagemakerUIDelegate.makeAlert(message:)` (sets `MarkerDataAppIcon.alertImage`, `.informational`, message as `messageText`). Never re-add generic “Alert” / “Confirm” / “Prompt” titles; SwiftUI dialogs elsewhere have no such title. Keep `confirm` returning `.alertFirstButtonReturn` so Cancel reaches JavaScript as `false`. --- @@ -103,7 +105,7 @@ Shared component: `Views/Components/DropTargetOverlay.swift` (main app + Workflo - [ ] Drop overlays appear on main-app Extract / Roles / Queue **and** Workflow Extension Extract + Roles with the copy above. - [ ] Queue finds `extract_info.json` folders and uploads via `manifestURL` (relocated folders work). - [ ] No-media / skipped swatch finishes as **“Extract done”** (not **“Analysing swatch done”**); `ColorPaletteRenderer.render` → `Bool` drives finish path. -- [ ] New alerts **and** confirmation dialogs use `.appDialogIcon()` (`MarkerDataAppIcon` from compiled `Marker-Data.icon`). +- [ ] New alerts **and** confirmation dialogs use `.appDialogIcon()` (`MarkerDataAppIcon` from compiled `Marker-Data.icon`); any `NSAlert` uses `MarkerDataAppIcon.alertImage` via `PagemakerUIDelegate.makeAlert(message:)`. - [ ] App icon still Icon Composer `Marker-Data.icon` for the **main app** (not inside `Assets.xcassets`; do not flatten layer PNG into `AppIconSingle`). Workflow Extension **header** uses the containing `Marker Data.app` icon; bundle/plugin icon stays `AppIcon.appiconset`. - [ ] Settings changes include version + migration + UI + export bridge when required. - [ ] New on-disk paths are reflected in the Uninstaller. diff --git a/Source/Marker Data/Marker Data/Models/Database/DatabaseManager.swift b/Source/Marker Data/Marker Data/Models/Database/DatabaseManager.swift index 2276f4d1..fc5ef7fe 100644 --- a/Source/Marker Data/Marker Data/Models/Database/DatabaseManager.swift +++ b/Source/Marker Data/Marker Data/Models/Database/DatabaseManager.swift @@ -92,21 +92,17 @@ class DatabaseManager: ObservableObject { } func duplicateProfile(profileName: String) throws { - Task { - try await MainActor.run { - guard let profile = self.profiles.first(where: { $0.name == profileName }) else { - throw DatabaseProfileDuplicationError.noProfileFound - } - - guard let duplicate = profile.copy() else { - throw DatabaseProfileDuplicationError.failedToDeepCopy - } - - duplicate.name += " copy" - - try self.addProfile(duplicate, saveToDisk: true) - } + guard let profile = self.profiles.first(where: { $0.name == profileName }) else { + throw DatabaseProfileDuplicationError.noProfileFound } + + guard let duplicate = profile.copy() else { + throw DatabaseProfileDuplicationError.failedToDeepCopy + } + + duplicate.name += " copy" + + try self.addProfile(duplicate, saveToDisk: true) } /// Saves a profile to disk diff --git a/Source/Marker Data/Marker Data/Models/Errors/DatabaseErrors.swift b/Source/Marker Data/Marker Data/Models/Errors/DatabaseErrors.swift index 829bc234..3453b634 100644 --- a/Source/Marker Data/Marker Data/Models/Errors/DatabaseErrors.swift +++ b/Source/Marker Data/Marker Data/Models/Errors/DatabaseErrors.swift @@ -109,3 +109,14 @@ enum DatabaseProfileDuplicationError: Error { case noProfileFound case failedToDeepCopy } + +extension DatabaseProfileDuplicationError: LocalizedError { + public var errorDescription: String? { + switch self { + case .noProfileFound: + "Could not find the selected profile" + case .failedToDeepCopy: + "Failed to copy the profile" + } + } +} diff --git a/Source/Marker Data/Marker Data/Pagemaker/PagemakerUIDelegate.swift b/Source/Marker Data/Marker Data/Pagemaker/PagemakerUIDelegate.swift index 1b69e417..1c7814f0 100644 --- a/Source/Marker Data/Marker Data/Pagemaker/PagemakerUIDelegate.swift +++ b/Source/Marker Data/Marker Data/Pagemaker/PagemakerUIDelegate.swift @@ -29,7 +29,11 @@ class PagemakerUIDelegate: NSObject, WKUIDelegate { panel.message = "Select a folder extracted from Marker Data" panel.prompt = "Select Folder" - let response = await panel.beginSheetModal(for: NSApp.keyWindow!) + let response: NSApplication.ModalResponse = if let window = NSApp.keyWindow { + await panel.beginSheetModal(for: window) + } else { + await panel.begin() + } return response == .OK ? panel.urls : nil } @@ -40,13 +44,10 @@ class PagemakerUIDelegate: NSObject, WKUIDelegate { runJavaScriptAlertPanelWithMessage message: String, initiatedByFrame frame: WKFrameInfo, ) async { - let alert = NSAlert() - alert.messageText = "Alert" - alert.informativeText = message - alert.alertStyle = .informational + let alert = makeAlert(message: message) alert.addButton(withTitle: "OK") - await alert.beginSheetModal(for: NSApp.keyWindow!) + await present(alert) } // Handle JavaScript confirm dialogs @@ -55,15 +56,11 @@ class PagemakerUIDelegate: NSObject, WKUIDelegate { runJavaScriptConfirmPanelWithMessage message: String, initiatedByFrame frame: WKFrameInfo ) async -> Bool { - let alert = NSAlert() - alert.messageText = "Confirm" - alert.informativeText = message - alert.alertStyle = .warning + let alert = makeAlert(message: message) alert.addButton(withTitle: "OK") alert.addButton(withTitle: "Cancel") - await alert.beginSheetModal(for: NSApp.keyWindow!) - return true + return await present(alert) == .alertFirstButtonReturn } // Handle JavaScript prompt dialogs @@ -73,10 +70,7 @@ class PagemakerUIDelegate: NSObject, WKUIDelegate { defaultText: String?, initiatedByFrame frame: WKFrameInfo ) async -> String? { - let alert = NSAlert() - alert.messageText = "Prompt" - alert.informativeText = prompt - alert.alertStyle = .informational + let alert = makeAlert(message: prompt) alert.addButton(withTitle: "OK") alert.addButton(withTitle: "Cancel") @@ -86,7 +80,34 @@ class PagemakerUIDelegate: NSObject, WKUIDelegate { alert.accessoryView = input - let response = await alert.beginSheetModal(for: NSApp.keyWindow!) + let response = await present(alert) return response == .alertFirstButtonReturn ? input.stringValue : nil } + + /// Matches the app's SwiftUI dialogs: Icon Composer app icon, the JavaScript + /// message as the title, and any paragraph after a blank line as detail. + private func makeAlert(message: String) -> NSAlert { + let paragraphs = message + .trimmingCharacters(in: .whitespacesAndNewlines) + .components(separatedBy: "\n\n") + + let alert = NSAlert() + alert.icon = MarkerDataAppIcon.alertImage + // NSAlert defaults to .warning; the app's SwiftUI dialogs are informational. + alert.alertStyle = .informational + alert.messageText = paragraphs.first.flatMap { $0.isEmpty ? nil : $0 } ?? Bundle.main.appName + alert.informativeText = paragraphs.dropFirst().joined(separator: "\n\n") + + return alert + } + + /// Falls back to app-modal when no window is key (background app, open menu). + @discardableResult + private func present(_ alert: NSAlert) async -> NSApplication.ModalResponse { + guard let window = NSApp.keyWindow else { + return alert.runModal() + } + + return await alert.beginSheetModal(for: window) + } } diff --git a/Source/Marker Data/Marker Data/Views/Detail Views/Database/DatabaseSettingsView.swift b/Source/Marker Data/Marker Data/Views/Detail Views/Database/DatabaseSettingsView.swift index 376e4f4d..8ef05218 100644 --- a/Source/Marker Data/Marker Data/Views/Detail Views/Database/DatabaseSettingsView.swift +++ b/Source/Marker Data/Marker Data/Views/Detail Views/Database/DatabaseSettingsView.swift @@ -20,6 +20,7 @@ struct DatabaseSettingsView: View { @State var showProfileRemoveAlert = false @State var showDuplicationAlert = false + @State var duplicationErrorMessage = "" @State var showDeleteConfirm = false @@ -46,7 +47,9 @@ struct DatabaseSettingsView: View { } .alert("Failed to remove profile", isPresented: $showProfileRemoveAlert) { } .appDialogIcon() - .alert("Failed to duplicate profile", isPresented: $showDuplicationAlert) { } + .alert("Failed to duplicate profile", isPresented: $showDuplicationAlert) { } message: { + Text(duplicationErrorMessage) + } .appDialogIcon() .confirmationDialog("Delete profile \(selection?.quoted ?? "")? This action cannot be undone.", isPresented: $showDeleteConfirm) { Button("Delete", role: .destructive) { @@ -117,6 +120,7 @@ struct DatabaseSettingsView: View { do { try databaseManager.duplicateProfile(profileName: profileName) } catch { + duplicationErrorMessage = error.localizedDescription showDuplicationAlert = true } } diff --git a/Source/Marker Data/Marker Data/Views/Extensions/DialogIcon.swift b/Source/Marker Data/Marker Data/Views/Extensions/DialogIcon.swift index 9e6e4176..575e3bd7 100644 --- a/Source/Marker Data/Marker Data/Views/Extensions/DialogIcon.swift +++ b/Source/Marker Data/Marker Data/Views/Extensions/DialogIcon.swift @@ -55,6 +55,11 @@ enum MarkerDataAppIcon { Image(nsImage: displayIcon) } + /// AppKit counterpart of `.appDialogIcon()` for `NSAlert` (Pagemaker WebView panels). + static var alertImage: NSImage { + displayIcon + } + /// Unsized icon content. Callers apply layout (About 200×200, Workflow Extension 100×100). static func image() -> some View { Image(nsImage: displayIcon) From a0ff1b06eae36cc38c3add59e45140410b67592a Mon Sep 17 00:00:00 2001 From: Vigneswaran Rajkumar <118706051+IAmVigneswaran@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:52:24 +0800 Subject: [PATCH 2/3] Record an extract failure once in Failed Tasks. extractAndUpdateProgress appended to failedTasks and then tripped the exportResultisNil guard, so one failed file produced two entries with the same identity and the second read "Failed to get export result". The extractor error is now rethrown after the KVO observation is invalidated, leaving the task group as the single recording point. Agent docs match. --- ARCHITECTURE.md | 2 ++ GUARDRAILS.md | 3 ++- .../Models/Extract/Extraction Model/ExtractionModel.swift | 8 +++++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 54c67300..0e0756fa 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -354,6 +354,8 @@ Pipeline (`performExtraction`): - If DB profile selected: `DatabaseUploader.uploadToDatabase(jsonManifestPath, profile)` 4. Aggregate `ExportExitStatus` + `ExtractionFailure[]`; notifications; optional Finder/Pagemaker open +Failure recording: `extractAndUpdateProgress` **rethrows** the extractor error rather than appending, so each file contributes at most one `.failedToExtract` entry — appended by the task group `catch`, with the real message. `ExtractError.exportResultisNil` is a safety net that no longer fires. The upload block still runs after a failed extract, so a file can also contribute a `.failedToUpload` entry (`missingJsonFile`); `ExtractionFailure.id` is the URL, so those two rows share an identity in `FailedExtractionsView`. + Cancellation: `cancelAll()` cancels extraction `Task` and terminates upload `Process`es. External file gate: if open/Workflow Extension arrives without a valid export folder, set `externalFileRecieved` / `externalFileURL` and wait for `processExternalFile` after the user picks a destination. diff --git a/GUARDRAILS.md b/GUARDRAILS.md index 03c79f8d..991f5225 100644 --- a/GUARDRAILS.md +++ b/GUARDRAILS.md @@ -91,7 +91,8 @@ Shared component: `Views/Components/DropTargetOverlay.swift` (main app + Workflo 16. **FCPXML `UTType` force-unwrap** — `UTType("com.apple.finalcutpro.xml")!` traps on Extract first layout when Final Cut Pro is absent (App Preview / review Macs). Use `importedAs` fallback and Info.plist imported types. Do not copy the old Marker Data force-unwrap. 17. **File menu Close last:** custom File items replace `.newItem`. Emptying `.newItem` makes system Close the first File item; do not duplicate Close. 18. **`Task { try await MainActor.run { … } }` inside a `throws` method swallows the error.** The task result is discarded, so the method always returns normally and the caller’s `catch` is dead. `DatabaseManager` is already `@MainActor` — call directly and let the error propagate. `duplicateProfile` was fixed this way; `setActiveProfile` and `loadProfilesFromDisk` still use the wrapper but throw nothing, so they are harmless. Do not reintroduce the pattern for anything that validates. -19. **`NSAlert` does not pick up the Icon Composer icon**, and its `alertStyle` defaults to `.warning`. Pagemaker’s WebView JavaScript panels are the app’s only `NSAlert`s — route them through `PagemakerUIDelegate.makeAlert(message:)` (sets `MarkerDataAppIcon.alertImage`, `.informational`, message as `messageText`). Never re-add generic “Alert” / “Confirm” / “Prompt” titles; SwiftUI dialogs elsewhere have no such title. Keep `confirm` returning `.alertFirstButtonReturn` so Cancel reaches JavaScript as `false`. +19. **Record an extract failure once.** `extractAndUpdateProgress` must rethrow the `MarkersExtractor` error, not append to `failedTasks` — appending there and then tripping the `exportResultisNil` guard produced two entries for one file, the second reading “Failed to get export result”, and duplicate `ExtractionFailure.id` (the URL) in `FailedExtractionsView`. The task group `catch` is the single recording point. +20. **`NSAlert` does not pick up the Icon Composer icon**, and its `alertStyle` defaults to `.warning`. Pagemaker’s WebView JavaScript panels are the app’s only `NSAlert`s — route them through `PagemakerUIDelegate.makeAlert(message:)` (sets `MarkerDataAppIcon.alertImage`, `.informational`, message as `messageText`). Never re-add generic “Alert” / “Confirm” / “Prompt” titles; SwiftUI dialogs elsewhere have no such title. Keep `confirm` returning `.alertFirstButtonReturn` so Cancel reaches JavaScript as `false`. --- diff --git a/Source/Marker Data/Marker Data/Models/Extract/Extraction Model/ExtractionModel.swift b/Source/Marker Data/Marker Data/Models/Extract/Extraction Model/ExtractionModel.swift index ea1324a1..9ae7b350 100644 --- a/Source/Marker Data/Marker Data/Models/Extract/Extraction Model/ExtractionModel.swift +++ b/Source/Marker Data/Marker Data/Models/Extract/Extraction Model/ExtractionModel.swift @@ -134,17 +134,23 @@ final class ExtractionModel: ObservableObject, Sendable { } var exportResult: ExportResult? = nil + var extractionError: Error? = nil do { // Do extraction exportResult = try await extractor.extract() } catch { Self.logger.error("Failed to extract: \(error.localizedDescription)") - failedTasks.append(ExtractionFailure(url: url, exitStatus: .failedToExtract, errorMessage: error.localizedDescription)) + extractionError = error } observation.invalidate() + // Rethrow so the task group records this failure once, with the real message + if let extractionError { + throw extractionError + } + guard let exportResult = exportResult else { throw ExtractError.exportResultisNil } From 44b7086c4faed6476870d4c84c5312e64f1c03f7 Mon Sep 17 00:00:00 2001 From: Vigneswaran Rajkumar <118706051+IAmVigneswaran@users.noreply.github.com> Date: Thu, 10 Sep 2026 10:25:39 +0800 Subject: [PATCH 3/3] Audit agent docs: dialog icon propagation, Pagemaker confirm, Color persistence. --- .cursorrules | 4 +++- AGENT.md | 5 ++++- ARCHITECTURE.md | 15 +++++++++++---- CHANGELOG.md | 12 ++++++++++++ GUARDRAILS.md | 5 ++++- 5 files changed, 34 insertions(+), 7 deletions(-) diff --git a/.cursorrules b/.cursorrules index 2722aedf..9adee0e8 100644 --- a/.cursorrules +++ b/.cursorrules @@ -54,7 +54,7 @@ Keep these four files consistent when architecture or agent guidance changes. - Do not assume behavior of `airlift` / `csv2notion_neo` beyond CLI usage in `DatabaseUploader` (and Dropbox setup in `DropboxSetupModel`). - Do not break Share Destination scripting (`Resources/OSAScriptingDefinition.sdef` + Obj‑C) or Workflow Extension disk/DNC contracts. - The app warns if not run from `/Applications`; do not remove that check (extension opens that path). -- SwiftUI `.alert` / `.confirmationDialog` icons: chain `.appDialogIcon()` after every dialog. `MarkerDataAppIcon` is `@MainActor` (`DialogIcon.swift`). AppKit `NSAlert` needs `MarkerDataAppIcon.alertImage` — Pagemaker's WebView JavaScript panels are the only `NSAlert`s and must be built by `PagemakerUIDelegate.makeAlert(message:)` (no generic "Alert" / "Confirm" title; `.informational`, message as `messageText`). Main-app Dock / About / alerts use Icon Composer `Marker-Data.icon` (`ASSETCATALOG_COMPILER_APPICON_NAME` = Marker-Data). Workflow Extension **header** loads the icon from the containing `Marker Data.app` (walk `appex` → `PlugIns` → `Contents` → `.app`); the appex’s `applicationIconImage` is its own `AppIcon.appiconset` (or Final Cut Pro). Do not flatten the layer PNG into `AppIconSingle`. Do not replace the Workflow Extension bundle/plugin `AppIcon.appiconset`. Uninstaller uses `applicationIconImage`. +- SwiftUI `.alert` / `.confirmationDialog` icons: chain `.appDialogIcon()` after every dialog in new code — but it wraps `.dialogIcon(_:)` and propagates via the environment, so an existing single trailing call already covers a whole chain (`ConfigurationSettingsView`: four dialogs, one call — not a bug). `MarkerDataAppIcon` is `@MainActor` (`DialogIcon.swift`). AppKit `NSAlert` needs `MarkerDataAppIcon.alertImage` — Pagemaker's WebView JavaScript panels are the only `NSAlert`s and must be built by `PagemakerUIDelegate.makeAlert(message:)` (no generic "Alert" / "Confirm" title; `.informational`, message as `messageText`). Main-app Dock / About / alerts use Icon Composer `Marker-Data.icon` (`ASSETCATALOG_COMPILER_APPICON_NAME` = Marker-Data). Workflow Extension **header** loads the icon from the containing `Marker Data.app` (walk `appex` → `PlugIns` → `Contents` → `.app`); the appex’s `applicationIconImage` is its own `AppIcon.appiconset` (or Final Cut Pro). Do not flatten the layer PNG into `AppIconSingle`. Do not replace the Workflow Extension bundle/plugin `AppIcon.appiconset`. Uninstaller uses `applicationIconImage`. - Database models use property name `plaform` — keep consistent unless intentionally migrating. - If adding new on-disk paths the app uses, update `MarkerDataUninstaller` cleanup paths too. - Do not upload Queue jobs using only absolute `ExtractInfo.jsonURL` when a same-named JSON exists in the dropped/scanned folder — use `manifestURL`. @@ -62,6 +62,8 @@ Keep these four files consistent when architecture or agent guidance changes. - `applyTaskAppearance` is `@MainActor` — always `await` it from `ColorPaletteRenderer`. - FCP timeline drops onto the Dock icon are often pasteboard-only; Extract panel drop is the supported timeline path. - Do not use `Color.accentColor` / `.accent` for shared chrome hosted in Final Cut Pro — use `Color.markerAccent`. +- `ColorExtension.swift` overrides `Color ==` with a 0.1-per-channel tolerance that shadows SwiftUI's exact operator app-wide and decides `SettingsStore` equality (so small colour edits do not mark a configuration dirty). It is not transitive — never put `Color` or `SettingsStore` in a `Set`, key a dictionary on them, or dedupe an array. Do not delete the tolerance without also fixing `Color.hex`, which truncates; see **GUARDRAILS.md → Signs**. +- Pagemaker's JavaScript `confirm` panel is unreachable in-app (gated behind `isMacOSSafari()` in bundled `Pagemaker.html`; WKWebView's user agent has no `Safari` token). Keep the delegate correct; do not try to verify it by clicking. - Do not delete a second `DialogIcon.swift in Sources` (or other shared files) from `project.pbxproj` — that is two-target membership, not a duplicate file. - Do not force-unwrap `UTType("com.apple.finalcutpro.xml")` / `.xmld` — resolve with `UTType(identifier) ?? UTType(importedAs:identifier, conformingTo:)` and keep `UTImportedTypeDeclarations` in `Marker-Data-Info.plist`. First Extract layout evaluates these types; FCP may be absent (App Preview). Do not split the Share Destination **Asset Description File** document type. - File menu custom items replace `.newItem` so system Close stays last — do not empty `.newItem` and do not add a second Close. diff --git a/AGENT.md b/AGENT.md index fb29041e..73181f74 100644 --- a/AGENT.md +++ b/AGENT.md @@ -72,7 +72,7 @@ For deeper module/data-flow detail, see **`ARCHITECTURE.md`**. For hard always/n | Info.plist | `Source/Marker Data/Marker-Data-Info.plist` — Sparkle, Share Destination document types (**Asset Description File** name is a contract), imported FCPXML UTIs | | Drop overlay (Extract / Roles / Queue + WE) | `Views/Components/DropTargetOverlay.swift` (also Workflow Extension Compile Sources) | | Workflow Extension UI | `Source/Marker Data/Workflow Extension/WorkflowExtensionView.swift` — Extract + Roles tabs; **two** Compile Sources rows (main app + appex — do not delete the main-app membership). `WorkflowExtensionViewController.swift` is appex-only. | -| Color helpers (`markerAccent`, `heroGradient`) | `Utilities/Extensions/ColorExtension.swift` | +| Color helpers + `Color` persistence | `Utilities/Extensions/ColorExtension.swift` — `markerAccent`, `heroGradient`, `init(hex:)` / `hex`, `Codable` conformance (how `fontColor` / `strokeColor` reach `preferences.json`), and a tolerant `static func ==` that shadows SwiftUI’s exact operator (see **GUARDRAILS.md → Signs**) | | Queue UI | `Views/Detail Views/QueueView.swift` | | Queue scan/upload | `Models/Queue/QueueModel.swift` | | Queue row + local-first manifest | `Models/Queue/QueueInstance.swift` (`manifestURL`) | @@ -253,6 +253,7 @@ Also: getter for `colorSwatchSettings` **forces `enableSwatch = false` when extr 2. Else: `NSImage(named: "Marker-Data")`, then `NSApplication.shared.applicationIconImage`. - **Surfaces:** About `MarkerDataAppIcon.image()` **200×200**; Workflow Extension header **100×100**; alerts **and** confirmation dialogs `.appDialogIcon()`; AppKit `NSAlert` `MarkerDataAppIcon.alertImage`. - **Main-app `.alert` / `.confirmationDialog` (chain `.appDialogIcon()`):** `Marker_DataApp` (library folders); `ContentView` (install location); `ExtractView` (extract + upload, ×2); `ConfigurationSettingsView` (unsaved-switch, unsaved-add, and delete confirmations + alert); `DatabaseSettingsView` (remove + duplicate alerts, ×2, plus delete-profile confirmation); `CreateDBProfileSheet`; `DropboxSetupView`; `InstallShareDestinationView`. +- `.appDialogIcon()` wraps SwiftUI `.dialogIcon(_:)` and **propagates through the environment**: one trailing call covers every dialog earlier in the same modifier chain. `ConfigurationSettingsView` deliberately has four dialogs and one call; `DatabaseSettingsView` has three calls (redundant, harmless). Add one per dialog in new code, but do not treat an existing single trailing call as a missing icon. - **AppKit `NSAlert`:** only Pagemaker’s JavaScript panels. Build them through `PagemakerUIDelegate.makeAlert(message:)` — it applies `MarkerDataAppIcon.alertImage`, forces `.informational`, and puts the message in `messageText` (no generic “Alert” / “Confirm” title). Do not add bare `NSAlert()`s elsewhere; use SwiftUI `.alert` + `.appDialogIcon()`. - Uninstaller is a **separate target**: `UninstallerView` uses `NSApplication.shared.applicationIconImage`, not `MarkerDataAppIcon`. - Do **not** change the Workflow Extension’s bundle/plugin icon (`ASSETCATALOG_COMPILER_APPICON_NAME` = AppIcon, existing `AppIcon.appiconset`). @@ -318,6 +319,8 @@ Exact overlay strings and always/never rules: **`GUARDRAILS.md`**. - **WE cache folder:** the extension does not `mkdir` Movies cache; a first-time drop can fail if the main app has never created `~/Movies/Marker Data Cache/`. - **FCPXML `UTType` force-unwrap:** `UTType("com.apple.finalcutpro.xml")!` traps on first Extract layout when Final Cut Pro is not installed (App Preview / machines without FCP). Resolve with `UTType(identifier) ?? UTType(importedAs:identifier, conformingTo:)` and keep `UTImportedTypeDeclarations` in `Marker-Data-Info.plist`. Do not split the Share Destination **Asset Description File** document type. - **File menu Close last:** custom File items replace `.newItem` (`FileCommands`). Do not empty `.newItem` (that puts system Close first) and do not add a second Close. +- **Pagemaker `confirm` is unreachable in-app:** the only `confirm(` in bundled `Resources/Pagemaker.html` sits behind `isMacOSSafari()`, and WKWebView’s default user agent has no `Safari` token. Keep the delegate correct, but you cannot exercise it from the UI — and `update_pagemaker.yml` can change that HTML upstream. +- **Tolerant `Color ==`:** `ColorExtension.swift` overrides `Color ==` with a 0.1-per-channel tolerance that shadows SwiftUI’s and drives `SettingsStore` equality, so small colour edits do not mark a configuration dirty. It is not transitive — never put `Color` or `SettingsStore` in a `Set`, key a dictionary on them, or dedupe an array. Full reasoning in **GUARDRAILS.md → Signs**. ## Definition of done for most changes - App builds **unsigned** Debug **and** Release for **arm64** (`CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO`; Workflow Extension still embeds). diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 0e0756fa..f24edb7c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -261,6 +261,7 @@ Migrations operate on `[String: Any]` — not Codable — so renames and nested - **Unique names:** `configurationNameExists` checks in-memory list **and** on-disk file; collisions → `ConfigurationSaveError.nameAlreadyExists`. Illegal names: empty or `"Default"`. - Rename = duplicate-as-new + remove-old (`ConfigurationsViewModel.rename`); same-name rename is a no-op; sheets dismiss only on success. - `unsavedChanges`: Default compares to `defaults()`; named configs compare to on-disk file. +- **Colour caveat:** `SettingsStore` gets synthesized `Equatable`, and its two `Color` properties (`fontColor`, `strokeColor`) are compared by the app’s **own** `Color ==` in `ColorExtension.swift`, which is tolerant (`isEqual(to:tolerance: 0.1)`) and shadows SwiftUI’s exact operator. Colour edits smaller than ~25/255 per channel therefore leave `unsavedChanges` false, disabling ⌘S / ⌘Z, even though the `$store` sink has already written `preferences.json`. See **GUARDRAILS.md → Signs** for the reason (truncating `Color.hex`) and the correct fix. - Optional ⌘1…⌘9 shortcuts via `@UserDefaultsArray("configurationShortcuts")`. #### Bridge to extraction (`markersExtractorSettings`) @@ -668,7 +669,8 @@ flowchart TB MD["MarkerDataAppIcon.displayIcon"] ABOUT["AboutView 200×200"] WEH["WE header 100×100"] - ALERTS[".appDialogIcon"] + ALERTS["dialogImage
.appDialogIcon (SwiftUI)"] + NSA["alertImage
PagemakerUIDelegate.makeAlert (AppKit)"] DOC --> MD CAT -.->|not artwork source| MD @@ -676,6 +678,9 @@ flowchart TB MD --> ABOUT MD --> WEH MD --> ALERTS + MD --> NSA + ALERTS -->|environment: one call covers a chain| DLG[".alert / .confirmationDialog"] + NSA --> JS["JS alert / confirm / prompt"] ``` **Resolution:** @@ -690,8 +695,10 @@ flowchart TB | Dock | `ASSETCATALOG_COMPILER_APPICON_NAME` = Marker-Data (`Marker-Data.icon`, not inside `Assets.xcassets`) | | About | `MarkerDataAppIcon.image()` — size **200×200** only in `AboutView` | | Workflow Extension header | `MarkerDataAppIcon.image()` — **100×100** in `WorkflowExtensionView` | -| `.alert` / `.confirmationDialog` | `.appDialogIcon()` after every dialog. Call sites: `Marker_DataApp`, `ContentView`, `ExtractView` (×2), `ConfigurationSettingsView` (3 confirmations + alert), `DatabaseSettingsView` (×2 alerts + delete confirmation), `CreateDBProfileSheet`, `DropboxSetupView`, `InstallShareDestinationView`. Uninstaller uses `applicationIconImage`, not this helper. | -| AppKit `NSAlert` | `MarkerDataAppIcon.alertImage` — the AppKit counterpart of `.appDialogIcon()`. Only call site is `PagemakerUIDelegate.makeAlert(message:)`, which serves the three `WKUIDelegate` JavaScript panels (alert / confirm / prompt). | +| `.alert` / `.confirmationDialog` | `.appDialogIcon()` — wraps SwiftUI `.dialogIcon(_:)`, which propagates through the **environment**, so one call at the end of a modifier chain covers every dialog attached beneath it (see note below). Views with dialogs: `Marker_DataApp`, `ContentView`, `ExtractView` (2 alerts, 2 calls), `ConfigurationSettingsView` (3 confirmations + 1 alert, **1** trailing call), `DatabaseSettingsView` (2 alerts + delete confirmation, 3 calls), `CreateDBProfileSheet`, `DropboxSetupView`, `InstallShareDestinationView`. Uninstaller uses `applicationIconImage`, not this helper. | +| AppKit `NSAlert` | `MarkerDataAppIcon.alertImage` — the AppKit counterpart of `.appDialogIcon()`. Only call site is `PagemakerUIDelegate.makeAlert(message:)`, which serves the three `WKUIDelegate` JavaScript panels (alert / confirm / prompt). The fourth `webView(_:runOpenPanelWith:…)` delegate method is an `NSOpenPanel` folder picker, not an `NSAlert`. | + +**Why the call counts differ.** `.appDialogIcon()` is not per-dialog bookkeeping — it sets an environment value, so a single trailing call covers every `.alert` / `.confirmationDialog` earlier in the same modifier chain. `ConfigurationSettingsView` therefore needs only **one** call for its four dialogs, and `DatabaseSettingsView`’s three calls are redundant but harmless. A view with dialogs and **zero** `.appDialogIcon()` anywhere in its chain is the only broken case. Guidance elsewhere says “chain it after every dialog” because that is the safe habit for new code, not because one-per-dialog is required. `Marker-Data.icon` lives in `Source/Marker Data/Marker Data/` beside the catalog. Workflow Extension `ASSETCATALOG_COMPILER_APPICON_NAME` remains **AppIcon** (existing PNG `AppIcon.appiconset`) until that plugin icon is updated separately. @@ -801,7 +808,7 @@ Source/Marker Data/Marker Data/ Install View/, Objective-C Code/, OpenEventHandler.swift Pagemaker/ # PagemakerView, PDF export handler, UIDelegate, WebViewStateManager Utilities/ - Extensions/ # URL, Color (markerAccent, heroGradient), UTType (`UTType.fcpxml` / `.fcpxmld`, never `!`), NotificationName, … + Extensions/ # URL, Color (markerAccent, heroGradient, hex, Codable, tolerant `==` — see Signs), UTType (`UTType.fcpxml` / `.fcpxmld`, never `!`), NotificationName, … Shell/, Notifications/, Other/ # FCPXMLIntake, TextClippingReader, LibraryFolders, FileWatcher, … Resources/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 94edb7a3..ef0c06d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +### 2.1.2 (26) + +**🎉 Released:** +- 10th September 2026 + +**🐞 Bug Fix:** +- Pagemaker alerts now show the app icon instead of a generic “Alert” title. +- Duplicating a database profile now reports why it failed. +- Failed extractions now appear once in Failed Tasks, with the actual error. + +--- + ### 2.1.1 (25) **🎉 Released:** diff --git a/GUARDRAILS.md b/GUARDRAILS.md index 991f5225..af6db90e 100644 --- a/GUARDRAILS.md +++ b/GUARDRAILS.md @@ -19,7 +19,7 @@ Read with **`AGENT.md`** (how to change things), **`ARCHITECTURE.md`** (how it w |------|------| | **Settings** | Bump `SettingsStore.version`, add a dict migration `case`, wire UI, and pass export-related fields through `markersExtractorSettings(fcpxmlFileUrl:)`. The Workflow Extension compiles `SettingsStore` and decodes prefs **without** running migrations — main app must launch after a schema bump. | | **Config names** | Throw `ConfigurationSaveError.nameAlreadyExists` on add / rename / duplicate collisions — never overwrite `{name}.json` silently. | -| **Alerts** | Chain `.appDialogIcon()` after every `.alert` / `.confirmationDialog` (`DialogIcon.swift` → `@MainActor` `MarkerDataAppIcon` from compiled `Marker-Data.icon`). AppKit `NSAlert` uses the counterpart `MarkerDataAppIcon.alertImage`; Pagemaker’s JavaScript panels are the only ones, and must be built by `PagemakerUIDelegate.makeAlert(message:)`. Uninstaller is a separate target and uses `applicationIconImage`. | +| **Alerts** | Chain `.appDialogIcon()` after every `.alert` / `.confirmationDialog` (`DialogIcon.swift` → `@MainActor` `MarkerDataAppIcon` from compiled `Marker-Data.icon`). It is environment-propagating, so an existing single trailing call already covers a whole chain — see **Signs**. AppKit `NSAlert` uses the counterpart `MarkerDataAppIcon.alertImage`; Pagemaker’s JavaScript panels are the only ones, and must be built by `PagemakerUIDelegate.makeAlert(message:)`. Uninstaller is a separate target and uses `applicationIconImage`. | | **Extract intake** | Route Finder files, text clippings, and Final Cut Pro pasteboard drops through `FCPXMLIntake` → `ExtractionModel.receiveFiles` / `receiveItemProviders`. | | **Drop overlays** | Use shared `DropTargetOverlay` on main-app Extract / Roles / Queue **and** Workflow Extension Extract + Roles. Shared files have **two** Compile Sources memberships in `project.pbxproj` (one file, two targets) — never delete one row. Extension membership includes overlay + `ColorExtension` + `DialogIcon.swift` + `HelpButton` / `OverlayHelpButton` plus Roles/settings files listed in **ARCHITECTURE.md**. | | **Shared WE chrome color** | Use `Color.markerAccent` (system indigo) for overlay borders and other shared chrome hosted in FCP — not `Color.accentColor`. | @@ -93,6 +93,9 @@ Shared component: `Views/Components/DropTargetOverlay.swift` (main app + Workflo 18. **`Task { try await MainActor.run { … } }` inside a `throws` method swallows the error.** The task result is discarded, so the method always returns normally and the caller’s `catch` is dead. `DatabaseManager` is already `@MainActor` — call directly and let the error propagate. `duplicateProfile` was fixed this way; `setActiveProfile` and `loadProfilesFromDisk` still use the wrapper but throw nothing, so they are harmless. Do not reintroduce the pattern for anything that validates. 19. **Record an extract failure once.** `extractAndUpdateProgress` must rethrow the `MarkersExtractor` error, not append to `failedTasks` — appending there and then tripping the `exportResultisNil` guard produced two entries for one file, the second reading “Failed to get export result”, and duplicate `ExtractionFailure.id` (the URL) in `FailedExtractionsView`. The task group `catch` is the single recording point. 20. **`NSAlert` does not pick up the Icon Composer icon**, and its `alertStyle` defaults to `.warning`. Pagemaker’s WebView JavaScript panels are the app’s only `NSAlert`s — route them through `PagemakerUIDelegate.makeAlert(message:)` (sets `MarkerDataAppIcon.alertImage`, `.informational`, message as `messageText`). Never re-add generic “Alert” / “Confirm” / “Prompt” titles; SwiftUI dialogs elsewhere have no such title. Keep `confirm` returning `.alertFirstButtonReturn` so Cancel reaches JavaScript as `false`. +21. **`Color ==` is tolerant, app-wide, and not transitive.** `ColorExtension.swift` defines `static func == (Color, Color)` delegating to `isEqual(to:tolerance: 0.1)`; Swift prefers the same-module declaration, so this shadows SwiftUI's exact `==` for **every** `Color` comparison compiled into the main app — including synthesized `Equatable` on `SettingsStore` (verified). Consequences: a colour tweak under ~25/255 in all three channels leaves `SettingsContainer.checkForUnsavedChanges()` false, so the Configurations “Changed” badge and ⌘S / ⌘Z stay disabled even though `preferences.json` was already updated; and `0.0 == 0.1`, `0.1 == 0.2`, `0.0 != 0.2` breaks `Equatable` transitivity, so never put `Color` (or `SettingsStore`) in a `Set`, use it as a dictionary key, or dedupe an array of them. The tolerance exists because `Color.hex` **truncates** (`Int(components.red * 255.0)`): 179 of 256 sampled colours drift one step through encode/decode, which under exact equality would mark the store permanently dirty. Fix properly by rounding in `hex` and comparing `lhs.hex == rhs.hex` — measured zero round-trip failures — not by lowering the tolerance alone. Note `hex` also feeds `imageLabelFontColor` / `imageLabelFontStrokeColor`, so the drift reaches rendered labels. +22. **Pagemaker’s `confirm` panel is unreachable in-app — do not “verify” it by clicking.** The only `confirm(` in bundled `Resources/Pagemaker.html` is gated behind `isMacOSSafari()`, and WKWebView’s default user agent carries no `Safari` token, so the branch never runs inside Marker Data. `webView(_:runJavaScriptConfirmPanelWithMessage:…)` must still return `.alertFirstButtonReturn` correctly (Sign 20), but that contract cannot be exercised from the UI — reason about it, don’t test-and-conclude. Two things would make it live: setting `applicationNameForUserAgent` to include `Safari`, or an upstream Pagemaker change, since `update_pagemaker.yml` refreshes that HTML outside this repo’s review. +23. **`.appDialogIcon()` propagates through the environment.** It wraps SwiftUI `.dialogIcon(_:)`, so one trailing call covers every `.alert` / `.confirmationDialog` earlier in the same modifier chain. `ConfigurationSettingsView` has four dialogs and **one** call — that is correct, not a missing icon; `DatabaseSettingsView`’s three calls are redundant but harmless. Only a dialog-bearing chain with **no** call anywhere is broken. Keep adding one per dialog in new code (safe habit), but do not “fix” an existing single trailing call. ---