Skip to content

refactor(backup): race fix, env sanitization, restore flow#1246

Merged
datlechin merged 15 commits into
mainfrom
refactor/backup-restore-polish
May 13, 2026
Merged

refactor(backup): race fix, env sanitization, restore flow#1246
datlechin merged 15 commits into
mainfrom
refactor/backup-restore-polish

Conversation

@datlechin
Copy link
Copy Markdown
Member

Summary

Follow-up polish for the backup/restore feature merged in #1211. Addresses every blocking and smaller issue called out in the review. One concern per commit so the history is bisect-friendly.

Blocking fixes

  • Continuation race in ProcessPostgresDumpRunner — termination handler could run between the process.isRunning check and the continuation assignment in result, leaving the awaiter hung forever. Both branches now serialize through stateLock, and the termination outcome is cached so a late await result returns the buffered value instead of stalling.
  • Localization cleanup — removed orphan Localizable.xcstrings keys left over from the dump↔backup rename (Choose Backup File, Save Backup, Choose where to save the backup of "%@", Select a backup file created with..., pg_dump exited with code %d, pg_restore exited with code %d, Backups are only supported..., Restore is only supported..., Connect to the database before starting a backup/restore, A backup/restore is already running, ASCII Backup Dump... / Restore Dump...). The strings catalog is back in sync with the code.
  • HIG ellipsis — File menu items use Unicode U+2026 (Backup Dump…, Restore Dump…) to match the primary button label.
  • CHANGELOG — moved the backup/restore entries from [0.40.0] (already shipped without this feature) to [Unreleased], and rewrote them to match the menu names.

Polish

  • Minimal subprocess envpg_dump and pg_restore now receive only PATH, HOME, USER, LOGNAME, SHELL, TMPDIR, LANG, LC_ALL plus PGPASSWORD/PGSSLMODE. No more ambient libpq env leaking through.
  • stderr cap dedup — dropped the mutable instance stderrCap field; reads command.stderrByteCap directly inside the readability handler.
  • Restore source pick before sheetMainContentCommandActions.restoreDatabase() opens NSOpenPanel on the main window first; only after the user picks a file does it set coordinator.activeSheet = .restoreDatabase(fileURL:). ActiveSheet.restoreDatabase carries the URL, and RestoreDatabaseFlow no longer needs a .needsSource placeholder phase.
  • Failure-detail scroll viewBackupResultSheet's failure case now uses a height-capped ScrollView with monospaced text instead of lineLimit(8), so multi-screen pg_restore stderr is readable without breaking the sheet layout.
  • Parameterized size queryPostgresDumpService.estimatedDatabaseSize uses executeParameterized(query: "SELECT pg_database_size($1)", parameters: [database]) instead of string-escaping.
  • NamingDatabaseSwitcherSheet.Mode.switch (keyword-escaped) renamed to .switching.
  • Tests don't sleepPostgresDumpService exposes stateUpdates() returning an AsyncStream<PostgresDumpState> (eager subscription via makeStream() so transitions aren't dropped). State-machine tests await the next matching state from the stream instead of polling.
  • nonisolated static helpersbuildCommand, minimalEnvironment, pgSSLMode, inheritedEnvironmentKeys are pure and now correctly marked nonisolated, so command-builder tests compile.
  • FakeDumpRunner re-entrant — buffers the outcome if finish() is called before result is awaited, mirroring the production fix.
  • DataGridRowViewSetValueTests@MainActor-annotated and imports sorted so the test target compiles (pre-existing regression from fix(datagrid): edit date cells inline and add date presets to Set Value #1233, blocking the whole test suite).

Test plan

  • swiftlint lint --strict clean on every changed file.
  • xcodebuild build succeeds (the two Running SwiftLint for CodeEditSourceEditor/CodeEditTextView failures are the known headless plugin-sandbox issue, unrelated).
  • PostgresDumpServiceCommandTests (8 cases including new environmentIsMinimal()) all pass.
  • PostgresDumpServiceStateMachineTests (5 cases) all pass.
  • DataGridRowViewSetValueTests (7 cases) all pass.
  • Smoke-test from Xcode: Backup Dump… on a Postgres connection, Restore Dump… on a fresh DB, cancel mid-flow, force a failure (bad credentials) and confirm the scroll view in the result sheet.

@datlechin datlechin changed the title refactor(backup): polish PR #1211 — race fix, env sanitization, restore flow refactor(backup): race fix, env sanitization, restore flow May 13, 2026
@datlechin datlechin merged commit e4269a4 into main May 13, 2026
2 checks passed
@datlechin datlechin deleted the refactor/backup-restore-polish branch May 13, 2026 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant