clone/logseq
1
0
Fork 0
mirror of https://github.com/logseq/logseq.git synced 2026-05-16 17:02:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

038-electron-db-worker-switch-graph.md

Browse Source
This commit is contained in:
rcmerci
2026-02-25 16:10:51 +08:00
parent ce103fea48
commit 4ac10a144c
7 changed files with 313 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

187
docs/agent-guide/038-electron-db-worker-switch-graph.md Normal file
View File

@@ -0,0 +1,187 @@
# Electron Db Worker Switch Graph Implementation Plan
Goal: Fix Electron graph switching with db-worker-node so switching to a new graph never leaves the renderer bound to a stopped runtime.
Architecture: Keep db-worker runtime lifecycle in the `db-worker-runtime` path and window close path, and treat `setCurrentGraph` as window graph metadata synchronization only.
Architecture: Remove or guard the duplicate release path in `setCurrentGraph` that can stop the newly started runtime after `persist-db/<fetch-init-data` already switched the window to the new repo.
Architecture: Add regression tests for the switch order and run manual Electron smoke checks that validate runtime continuity and no stale runtime ownership after repeated switches.
Tech Stack: ClojureScript, Electron main process IPC, Promesa, db-worker-node, babashka tests.
Related: Builds on `docs/agent-guide/033-desktop-db-worker-node-backend.md`.
Related: Relates to `docs/agent-guide/034-db-worker-node-owner-process-management.md`.
## Problem statement
Current graph switching in the renderer calls `persist-db/<fetch-init-data` before `state/set-current-repo!`.
In Electron mode, `persist-db/<fetch-init-data` calls `ipc/ipc "db-worker-runtime"` and binds the window to the new repo runtime via `/Users/rcmerci/gh-repos/logseq/src/electron/electron/db_worker.cljs`.
After the runtime is already switched, `state/set-current-repo!` calls `ipc/ipc "setCurrentGraph"` from `/Users/rcmerci/gh-repos/logseq/src/main/frontend/state.cljs`.
`handle :setCurrentGraph` in `/Users/rcmerci/gh-repos/logseq/src/electron/electron/handler.cljs` currently calls `db-worker/release-window!` whenever graph path changes.
That second release runs after the window has already been rebound to the new repo, so it can stop the new runtime and make switch graph fail with downstream invoke errors.
The issue is reproducible when switching between two db graphs in one window with db-worker-node backend enabled.
The expected behavior is that switching from graph A to graph B stops graph A when needed and keeps graph B alive for immediate reads and writes.
## Current and target event sequence
```text
Current sequence.
Renderer restore graph B.
-> persist-db/<fetch-init-data B.
-> ipc "db-worker-runtime" B.
-> main db-worker manager switches A -> B.
-> state/set-current-repo! B.
-> ipc "setCurrentGraph" B.
-> handler calls release-window! again.
-> B runtime may be stopped unexpectedly.
Target sequence.
Renderer restore graph B.
-> persist-db/<fetch-init-data B.
-> ipc "db-worker-runtime" B.
-> main db-worker manager switches A -> B.
-> state/set-current-repo! B.
-> ipc "setCurrentGraph" B.
-> handler only updates window graph path.
-> B runtime stays available.
```
## Testing Plan
I will follow `@test-driven-development` and add failing tests before each behavior change.
I will add a failing regression test in `/Users/rcmerci/gh-repos/logseq/src/test/electron/db_worker_manager_test.cljs` that encodes the expected post-switch invariant that the new runtime remains active until explicit release on window close.
I will add a failing test in `/Users/rcmerci/gh-repos/logseq/src/test/frontend/persist_db_test.cljs` for the graph switch flow to assert runtime rebinding happens once per target repo and is not reinitialized by graph metadata sync.
I will add a focused unit test file `/Users/rcmerci/gh-repos/logseq/src/test/electron/graph_switch_flow_test.cljs` for extracted pure graph-switch decision logic so the release/no-release condition is testable without Electron GUI dependencies.
I will run focused tests after each phase and finish with `bb dev:lint-and-test`.
I will perform manual Electron smoke checks that open graph A, switch to graph B, execute thread-api reads and writes, and then switch back to graph A.
I will review changes against `@prompts/review.md` before merge.
NOTE: I will write *all* tests before I add any implementation behavior.
## Implementation plan
### Phase 1: Add failing tests that reproduce the switch-order regression.
1. Add a failing test in `/Users/rcmerci/gh-repos/logseq/src/test/electron/db_worker_manager_test.cljs` that simulates A -> B switch and asserts no stop is triggered for B before explicit release.
2. Add a failing test in `/Users/rcmerci/gh-repos/logseq/src/test/frontend/persist_db_test.cljs` for the sequence `<fetch-init-data B` followed by `set-current-repo! B`.
3. Add `/Users/rcmerci/gh-repos/logseq/src/test/electron/graph_switch_flow_test.cljs` with a failing case that `setCurrentGraph` graph-path synchronization does not request runtime release on normal switch.
4. Run `bb dev:test -v 'electron.db-worker-manager-test'` and confirm failures match the new assertions.
5. Run `bb dev:test -v 'frontend.persist-db-test'` and confirm failures match the new assertions.
### Phase 2: Extract graph switch release decision into a pure helper.
6. Add `/Users/rcmerci/gh-repos/logseq/src/electron/electron/graph_switch_flow.cljs` with a pure function that decides when runtime release is allowed during `setCurrentGraph`.
7. Keep the helper contract explicit with inputs for previous graph identity and next graph identity so race-prone implicit state is avoided.
8. Make `/Users/rcmerci/gh-repos/logseq/src/test/electron/graph_switch_flow_test.cljs` pass with minimal logic.
9. Run `bb dev:test -v 'electron.graph-switch-flow-test'` and confirm green.
### Phase 3: Update Electron `setCurrentGraph` handler to avoid stopping the new runtime.
10. Update `handle :setCurrentGraph` in `/Users/rcmerci/gh-repos/logseq/src/electron/electron/handler.cljs` to use the new pure decision helper.
11. Remove direct unconditional `db-worker/release-window!` on graph-path change from `/Users/rcmerci/gh-repos/logseq/src/electron/electron/handler.cljs`.
12. Keep window graph-path state updates in `/Users/rcmerci/gh-repos/logseq/src/electron/electron/handler.cljs` unchanged so existing window routing semantics remain stable.
13. Preserve runtime cleanup responsibility in `/Users/rcmerci/gh-repos/logseq/src/electron/electron/db_worker.cljs` and `/Users/rcmerci/gh-repos/logseq/src/electron/electron/window.cljs`.
14. Run `bb dev:test -v 'electron.graph-switch-flow-test'` and confirm green.
### Phase 4: Harden manager behavior for repeated switch calls.
15. Add a guard in `/Users/rcmerci/gh-repos/logseq/src/electron/electron/db_worker.cljs` that keeps switch idempotency for repeated `ensure-started!` calls to the same repo and window.
16. Ensure stale release operations cannot remove a window mapping for a newer repo assignment in `/Users/rcmerci/gh-repos/logseq/src/electron/electron/db_worker.cljs`.
17. Update `/Users/rcmerci/gh-repos/logseq/src/test/electron/db_worker_manager_test.cljs` assertions to cover stale-release no-op behavior.
18. Run `bb dev:test -v 'electron.db-worker-manager-test'` and confirm green.
### Phase 5: Verify renderer remote runtime rebinding contract.
19. Update `/Users/rcmerci/gh-repos/logseq/src/main/frontend/persist_db.cljs` only if needed to keep `<ensure-remote!` single-responsibility for runtime rebinding.
20. Ensure `/Users/rcmerci/gh-repos/logseq/src/main/frontend/db/restore.cljs` keeps order `<fetch-init-data` before `set-current-repo!` unchanged unless tests prove a safer order is required.
21. Extend `/Users/rcmerci/gh-repos/logseq/src/test/frontend/persist_db_test.cljs` to assert no duplicate remote stop/start on metadata-only sync.
22. Run `bb dev:test -v 'frontend.persist-db-test'` and confirm green.
### Phase 6: Add targeted diagnostics and manual smoke script.
23. Add temporary structured logs with `lambdaisland.glogi` in `/Users/rcmerci/gh-repos/logseq/src/electron/electron/db_worker.cljs` around repo switch and release decisions.
24. Add temporary structured logs in `/Users/rcmerci/gh-repos/logseq/src/electron/electron/handler.cljs` around `setCurrentGraph` branch selection.
25. Execute manual smoke flow in Electron dev mode by switching A -> B -> A and invoking read and write actions after each switch.
26. Validate no immediate invoke failures after switch and confirm runtime stays available for the active graph.
27. Remove temporary logs that are not needed for long-term maintainability.
### Phase 7: Final verification and review gate.
28. Run `bb dev:test -v 'electron.graph-switch-flow-test'`.
29. Run `bb dev:test -v 'electron.db-worker-manager-test'`.
30. Run `bb dev:test -v 'frontend.persist-db-test'`.
31. Run `bb dev:lint-and-test`.
32. Confirm each command reports `0 failures, 0 errors`.
33. Run final review checklist pass against `@prompts/review.md`.
## Edge cases
| Scenario | Expected behavior |
|---|---|
| Switch A -> B in one window where both graphs are local db graphs. | Runtime for B remains active and calls succeed immediately after switch. |
| Switch A -> B -> A quickly before all async handlers settle. | Late `setCurrentGraph` sync does not stop the currently active runtime. |
| Two windows share graph A and one window switches to graph B. | Graph A runtime remains alive for the other window and graph B runtime starts for the switching window. |
| Re-select current graph B from UI without actual graph change. | No runtime restart and no runtime release occurs. |
| Window closes right after switch to B. | Runtime release happens exactly once via close flow and does not leave stale window mapping. |
| Runtime ownership is external (`:owned? false`). | Graph switch does not attempt to stop external runtime unexpectedly. |
| Restore fails after runtime bind but before UI route redirect. | Failure handling does not silently stop the newly bound runtime unless explicit cleanup path runs. |
## Verification commands and expected outputs
```bash
bb dev:test -v 'electron.graph-switch-flow-test'
bb dev:test -v 'electron.db-worker-manager-test'
bb dev:test -v 'frontend.persist-db-test'
bb dev:lint-and-test
```
Each command should finish with `0 failures, 0 errors`.
Manual Electron switch checks should show successful read and write operations immediately after each switch.
No `db-worker invoke failed` errors should appear during normal A -> B -> A switching.
## Testing Details
The new tests verify behavior around event ordering and runtime lifecycle boundaries instead of implementation details.
The manager tests validate that stale or duplicate release operations cannot terminate the active repo runtime.
The frontend tests validate that runtime rebinding is driven by repo changes and not by metadata synchronization calls.
Manual smoke checks validate real Electron runtime behavior that unit tests cannot fully represent.
## Implementation Details
- Keep `setCurrentGraph` focused on graph-path synchronization and remove lifecycle side effects from that path.
- Keep runtime start and stop orchestration centralized in `electron.db-worker` manager APIs.
- Use a small pure helper for release decision logic so regression tests do not depend on Electron runtime modules.
- Preserve existing `db-worker-runtime` IPC contract from renderer to main process.
- Keep old graph cleanup tied to explicit switch lifecycle and window close lifecycle only.
- Validate switch behavior with both single-window and multi-window tests.
- Use `@test-driven-development` for red-green implementation order.
- Follow `@prompts/review.md` checks before merging.
## Question
Decision: On failed graph restore, keep the newly bound runtime alive for fast retry in the same window.
Decision: Do not add a short-lived debug flag for switch sequencing logs in development builds.
Decision: Add an E2E scenario in `/Users/rcmerci/gh-repos/logseq/clj-e2e` for switch graph with db-worker-node and treat it as a release gate.
---

2
src/electron/electron/db_worker.cljs
View File

@@ -42,7 +42,7 @@
[state nil]
(let [remaining (disj (:windows entry) window-id)
state' (cond-> state
true
(= repo (get-in state [:window->repo window-id]))
(update :window->repo dissoc window-id)
(seq remaining)

13
src/electron/electron/graph_switch_flow.cljs Normal file
View File

@@ -0,0 +1,13 @@
(ns electron.graph-switch-flow)
(defn release-runtime-on-set-current-graph?
"Decides whether `setCurrentGraph` should release db-worker runtime.
Returns `false` by design.
In Electron, `setCurrentGraph` is metadata synchronization for window graph
path only. Runtime lifecycle is handled by the `db-worker-runtime` IPC path
(switch/start) and window close path (release). Releasing here reintroduces
the stale double-release bug that can stop the newly bound runtime."
[_switch]
false)

8
src/electron/electron/handler.cljs
View File

@@ -27,6 +27,7 @@
[electron.state :as state]
[electron.utils :as utils]
[electron.window :as win]
[electron.graph-switch-flow :as graph-switch-flow]
[logseq.cli.common.graph :as cli-common-graph]
[logseq.common.graph :as common-graph]
[logseq.db.sqlite.util :as sqlite-util]
@@ -326,8 +327,11 @@
(defmethod handle :setCurrentGraph [^js window [_ graph-name]]
(let [next-graph-path (when graph-name (utils/get-graph-dir graph-name))
current-graph-path (state/get-window-graph-path window)]
(p/let [_ (when (not= current-graph-path next-graph-path)
current-graph-path (state/get-window-graph-path window)
release-runtime? (graph-switch-flow/release-runtime-on-set-current-graph?
{:previous-graph-path current-graph-path
:next-graph-path next-graph-path})]
(p/let [_ (when release-runtime?
(db-worker/release-window! (.-id window)))]
(if next-graph-path
(set-current-graph! window next-graph-path)

44
src/test/electron/db_worker_manager_test.cljs
View File

@@ -77,6 +77,50 @@
(is false (str "unexpected error: " e))))
(p/finally (fn [] (done)))))))
(deftest ensure-stopped-stale-repo-does-not-clear-new-window-mapping
(async done
(let [stop-calls (atom [])
manager (db-worker/create-manager
{:start-daemon! (fn [repo] (p/resolved (runtime repo)))
:stop-daemon! (fn [rt]
(swap! stop-calls conj (:repo rt))
(p/resolved true))})]
(-> (p/let [_ (db-worker/ensure-started! manager "graph-a" :window-1)
_ (db-worker/ensure-started! manager "graph-a" :window-2)
_ (db-worker/ensure-started! manager "graph-b" :window-1)
;; simulate late/stale release for previous repo after window-1 already moved to graph-b
_ (db-worker/ensure-stopped! manager "graph-a" :window-1)
manager-state @(:state manager)]
(is (= "graph-b" (get-in manager-state [:window->repo :window-1])))
(is (= #{:window-2} (get-in manager-state [:repos "graph-a" :windows])))
(is (= #{:window-1} (get-in manager-state [:repos "graph-b" :windows])))
(is (empty? @stop-calls)))
(p/catch (fn [e]
(is false (str "unexpected error: " e))))
(p/finally (fn [] (done)))))))
(deftest ensure-stopped-stale-intermediate-repo-after-switch-back-keeps-current-repo
(async done
(let [stop-calls (atom [])
manager (db-worker/create-manager
{:start-daemon! (fn [repo] (p/resolved (runtime repo)))
:stop-daemon! (fn [rt]
(swap! stop-calls conj (:repo rt))
(p/resolved true))})]
(-> (p/let [_ (db-worker/ensure-started! manager "graph-a" :window-1)
_ (db-worker/ensure-started! manager "graph-b" :window-1)
_ (db-worker/ensure-started! manager "graph-a" :window-1)
;; stale cleanup for graph-b arrives after the window is already back on graph-a
_ (db-worker/ensure-stopped! manager "graph-b" :window-1)
manager-state @(:state manager)]
(is (= "graph-a" (get-in manager-state [:window->repo :window-1])))
(is (= #{:window-1} (get-in manager-state [:repos "graph-a" :windows])))
(is (nil? (get-in manager-state [:repos "graph-b"])))
(is (= ["graph-a" "graph-b"] @stop-calls)))
(p/catch (fn [e]
(is false (str "unexpected error: " e))))
(p/finally (fn [] (done)))))))
(deftest ensure-window-stopped-releases-active-runtime-by-window
(async done
(let [stop-calls (atom [])

15
src/test/electron/graph_switch_flow_test.cljs Normal file
View File

@@ -0,0 +1,15 @@
(ns electron.graph-switch-flow-test
(:require [cljs.test :refer [deftest is]]
[electron.graph-switch-flow :as graph-switch-flow]))
(deftest set-current-graph-switch-does-not-release-runtime
(is (false?
(graph-switch-flow/release-runtime-on-set-current-graph?
{:previous-graph-path "graph-a"
:next-graph-path "graph-b"}))))
(deftest set-current-graph-reselect-does-not-release-runtime
(is (false?
(graph-switch-flow/release-runtime-on-set-current-graph?
{:previous-graph-path "graph-a"
:next-graph-path "graph-a"}))))

47
src/test/frontend/persist_db_test.cljs
View File

@@ -5,6 +5,7 @@
[frontend.persist-db :as persist-db]
[frontend.persist-db.protocol :as protocol]
[frontend.persist-db.remote :as remote]
[frontend.storage :as storage]
[frontend.state :as state]
[frontend.util :as util]
[promesa.core :as p]))
@@ -112,6 +113,52 @@
(set! remote/stop! original-stop!)
(done)))))))
(deftest electron-fetch-init-data-then-set-current-repo-does-not-rebind-runtime
(async done
(let [ipc-calls (atom [])
start-calls (atom [])
stop-calls (atom [])
wrapped-worker (fn [& _] nil)
original-state @state/state
original-electron? util/electron?
original-ipc ipc/ipc
original-start! remote/start!
original-stop! remote/stop!
original-storage-set storage/set
original-storage-remove storage/remove]
(reset-runtime-state!)
(set! util/electron? (constantly true))
(set! ipc/ipc (fn [channel repo]
(swap! ipc-calls conj [channel repo])
(p/resolved nil)))
(set! remote/start! (fn [{:keys [repo]}]
(swap! start-calls conj repo)
(->FakeRemote repo wrapped-worker)))
(set! remote/stop! (fn [client]
(swap! stop-calls conj (:repo client))
(p/resolved true)))
(set! storage/set (fn [& _] nil))
(set! storage/remove (fn [& _] nil))
(-> (p/let [repo "logseq_db_graph_a"
_ (persist-db/<fetch-init-data repo {})
_ (state/set-current-repo! repo)]
(is (= [["db-worker-runtime" "logseq_db_graph_a"]
["setCurrentGraph" "logseq_db_graph_a"]]
@ipc-calls))
(is (= ["logseq_db_graph_a"] @start-calls))
(is (empty? @stop-calls)))
(p/catch (fn [e]
(is false (str "unexpected error: " e))))
(p/finally (fn []
(reset! state/state original-state)
(set! util/electron? original-electron?)
(set! ipc/ipc original-ipc)
(set! remote/start! original-start!)
(set! remote/stop! original-stop!)
(set! storage/set original-storage-set)
(set! storage/remove original-storage-remove)
(done)))))))
(deftest electron-list-db-without-current-repo-does-not-bootstrap-runtime
(async done
(let [ipc-calls (atom [])