From 43624821c0d0a4dcb849ea233f84f54f75d6e29a Mon Sep 17 00:00:00 2001 From: MasterYoav Date: Fri, 18 Sep 2026 10:21:25 +0300 Subject: [PATCH] A successful upgrade kept a plain-SQL copy of the database. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing reads the dump once the new image is up — there is no rollback after a success — so it sat in Application Support until the next upgrade overwrote it. Co-Authored-By: Claude Opus 5 (1M context) --- .../XBotRuntime/RuntimeController.swift | 17 +++++++--- .../Tests/XBotRuntimeTests/RuntimeTests.swift | 33 +++++++++++++++++++ docs/05-mac-app.md | 2 +- docs/11-packaging-and-updates.md | 4 +++ 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/apps/mac/Sources/XBotRuntime/RuntimeController.swift b/apps/mac/Sources/XBotRuntime/RuntimeController.swift index 3b1f4d3..579f71d 100644 --- a/apps/mac/Sources/XBotRuntime/RuntimeController.swift +++ b/apps/mac/Sources/XBotRuntime/RuntimeController.swift @@ -415,6 +415,10 @@ public actor RuntimeController { image = newImage if await launchEngineWithoutAdoption(environment: environment) { + // Nothing reads the dump once the new image is up — there is no rollback after a + // success — so keeping it would leave a plain-SQL copy of the person's conversations + // and agents in Application Support until the next upgrade overwrote it. + removeDump() return .succeeded } @@ -432,6 +436,14 @@ public actor RuntimeController { return .failed } + /// The dump and the log beside it, gone. Called when a restore can no longer be wanted. + private func removeDump() { + try? FileManager.default.removeItem(at: dumpURL) + try? FileManager.default.removeItem( + at: dumpURL.deletingLastPathComponent().appendingPathComponent("restore.log") + ) + } + /// Whether a dump was taken before the last upgrade, and so whether a restore is possible. private var dumpedBeforeUpgrade = false @@ -588,10 +600,7 @@ public actor RuntimeController { } // The pre-upgrade dump is a plain-SQL copy of the database, kept outside the volume so a // rollback can use it — which is exactly why removing the volumes never removed it. - try? FileManager.default.removeItem(at: dumpURL) - try? FileManager.default.removeItem( - at: dumpURL.deletingLastPathComponent().appendingPathComponent("restore.log") - ) + removeDump() // Back to stopped, not notDetected: the runtime is still installed and still working — it // is only xBot's own data that is gone. state = .stopped diff --git a/apps/mac/Tests/XBotRuntimeTests/RuntimeTests.swift b/apps/mac/Tests/XBotRuntimeTests/RuntimeTests.swift index 6c88d86..643b6fe 100644 --- a/apps/mac/Tests/XBotRuntimeTests/RuntimeTests.swift +++ b/apps/mac/Tests/XBotRuntimeTests/RuntimeTests.swift @@ -515,6 +515,39 @@ struct RuntimeControllerTests { #expect(!(await driver.removedHandles.isEmpty)) } + /** + An upgrade that worked has no use left for the dump, so it should not keep it. + + The dump exists for one reason: restoring a database the new image migrated past what the old + one can read. Nothing reads it once the new image is up — there is no roll back to a previous + engine after a successful upgrade — so keeping it leaves a plain-SQL copy of the person's + conversations and agents in Application Support until the next upgrade happens to overwrite it. + */ + @Test func aSucceededUpgradeDoesNotLeaveTheDumpBehind() async { + let dumpURL = isolatedDumpURL() + let controller = RuntimeController( + driver: FakeDriver(), + image: ImageReference(repository: "xbot/engine", tag: "1"), + health: { _ in EngineHealth(engineVersion: "0.0.5", schemaVersion: "0000") }, + ports: isolatedPortStore(), + dumpURL: dumpURL + ) + await controller.start(environment: environment) + + let outcome = await controller.upgrade( + to: ImageReference( + repository: "ghcr.io/masteryoav/xbot-engine", + digest: "sha256:abc123def456" + ), + rollingBackTo: ImageReference(repository: "xbot/engine", tag: "1"), + environment: environment + ) + + #expect(outcome == .succeeded) + // The fake writes one when the dump runs, so its absence here is a removal, not a no-op. + #expect(!FileManager.default.fileExists(atPath: dumpURL.path)) + } + /** A pull that fails must leave the running engine exactly where it was. diff --git a/docs/05-mac-app.md b/docs/05-mac-app.md index becec93..157ad3a 100644 --- a/docs/05-mac-app.md +++ b/docs/05-mac-app.md @@ -158,7 +158,7 @@ See [06-onboarding.md](06-onboarding.md). | Main | `Window` | Rail, conversation, panel. Onboarding crossfades in via `AppShellView` on first run | | Onboarding | (in Main) | Fixed-size window mode during first run; not a separate scene | | Settings | (in Main) | **Not a `Settings` scene.** In the window, in place of the conversation, via `AppState.isShowingSettings`. A floating panel put them somewhere the person had to go and find, and it hid the rail — so which agent was selected stopped being visible while its model was changed | -| Plugins admin | `Window` | `WKWebView` at engine `/admin/plugins` | +| Engine admin | `Window` | One `WKWebView` for every admin surface. Opens at `/admin/plugins` from **Plugins…** or `/admin` from **Engine admin…**, whose upstream sidebar reaches the rest | | Agent screen (detached) | — | **Not built** | | Menu bar | — | **Not built** — planned for runtime status when the main window is closed | diff --git a/docs/11-packaging-and-updates.md b/docs/11-packaging-and-updates.md index a87b877..a11346e 100644 --- a/docs/11-packaging-and-updates.md +++ b/docs/11-packaging-and-updates.md @@ -258,6 +258,10 @@ Two judgement calls in it: attempts a restore if it was. - **`--single-transaction` on the way back in**, so a restore that fails part-way leaves the database as it was rather than half-replaced. +- **An upgrade that succeeded deletes the dump.** Nothing reads it once the new image is up, there + being no rollback after a success, so keeping it would leave a plain-SQL copy of somebody's + conversations and agents in Application Support until the next upgrade overwrote it. Uninstall + removes it too. ---