059-cli-doctor-server-revision-mismatch-warning.md

This commit is contained in:
rcmerci
2026-03-12 20:54:11 +08:00
parent 50261dbb95
commit 5e8462c08a
9 changed files with 438 additions and 34 deletions

View File

@@ -0,0 +1,237 @@
# 059 — Add doctor warning for server and CLI revision mismatch
## Summary
This plan adds a new `doctor` warning when a discovered db-worker server revision does not match the local `logseq-cli` revision.
The warning must be actionable: for each mismatched server, `doctor` should tell the user to run:
- `logseq server restart --graph <name>`
This closes the current observability gap where `server list` already exposes revision mismatch, but `doctor` does not.
---
## Product decisions (locked)
1. **Mismatch rule is exact string compare**.
- `cli-revision != server-revision` means mismatch.
- No normalization, no hash shortening, no suffix stripping.
2. **Severity is warning, not error**.
- Revision drift is diagnosable and recoverable.
- `doctor` should still return `:status :ok` at top-level transport with `:data {:status :warning ...}` behavior, consistent with existing warning checks.
3. **`doctor` remains fail-fast for hard preconditions**.
- If script check fails, stop immediately (current behavior).
- If data-dir check fails, stop immediately (current behavior).
- Revision mismatch check runs only when server checks are reached.
4. **Restart guidance is per affected graph/server**.
- The warning includes one restart command per mismatched graph.
- Command form is `logseq server restart --graph <name>`.
5. **Structured output must remain machine-friendly**.
- JSON/EDN outputs include structured mismatch check data.
- Human output includes readable warning text and restart guidance.
---
## Background
Current implementation already has the needed revision primitives:
- CLI revision source: `logseq.cli.version/revision`.
- Server revision source: `db-worker.lock` revision surfaced by `logseq.cli.server/list-servers`.
- `server list` already computes and warns on mismatch in human output.
Current `doctor` checks:
1. `db-worker-script`
2. `data-dir`
3. `running-servers` readiness
`doctor` does **not** currently compare server revision against CLI revision.
---
## Goals
- Add revision mismatch detection to `doctor` using existing revision sources.
- Provide actionable restart guidance for each mismatched graph.
- Keep mismatch semantics identical to existing `server list` behavior.
- Preserve existing `doctor` fail-fast behavior for script/data-dir errors.
## Non-goals
- No automatic server restart.
- No change to daemon ownership/permission rules.
- No new db-worker RPC endpoint for version info.
- No hard failure on mismatch.
---
## User-facing behavior
### Human output
When mismatches exist, `doctor` includes a warning check (example shape):
- `[warning] server-revision-mismatch - 2 servers use a different revision than this CLI`
- Followed by actionable per-graph commands, e.g.:
- `Run: logseq server restart --graph graph-a`
- `Run: logseq server restart --graph graph-b`
If graph names include spaces, command examples should quote names in guidance output.
### JSON and EDN output
`doctor --output json|edn` keeps structured check payload, including mismatch details.
Suggested check payload shape:
- `:id :server-revision-mismatch`
- `:status :warning | :ok`
- `:code :doctor-server-revision-mismatch` (when warning)
- `:cli-revision <string>`
- `:servers [{:repo <repo> :revision <server-revision> :graph <graph-name>}]`
- `:message <human summary>`
---
## Design
### 1) Reuse one mismatch computation path
Today, mismatch computation is a private helper in `logseq.cli.command.server`.
Plan:
- Move or extract mismatch computation into a shared helper (recommended in `logseq.cli.server`), so both `server list` and `doctor` use exactly the same comparison logic.
- Keep return shape stable enough to support both command paths.
This prevents semantic drift between `server list` and `doctor`.
### 2) Add a dedicated doctor check
Add a new check in `logseq.cli.command.doctor`:
- `check-server-revision-mismatch`
Input:
- local CLI revision (`logseq.cli.version/revision`)
- discovered servers from `list-servers`
Behavior:
- No mismatches -> check status `:ok`
- Any mismatch -> check status `:warning`, include mismatch metadata and restart guidance
### 3) Execution flow in doctor
Keep current order and fail-fast semantics for hard errors:
1. `check-db-worker-script` (error stops execution)
2. `check-data-dir` (error stops execution)
3. `check-running-servers`
4. `check-server-revision-mismatch` (new)
Final `doctor` status is:
- `:ok` when all checks are `:ok`
- `:warning` when any warning check exists (`running-servers` and/or `server-revision-mismatch`)
- `:error` only for hard failures as today
### 4) Restart guidance formatting strategy
Two viable options:
- **Option A (minimal formatter change):** put guidance directly into mismatch check `:message`.
- **Option B (cleaner long-term):** add a small `format-doctor` branch for `:server-revision-mismatch` to render command lines from structured fields.
Recommended: **Option B** if it stays small, because it preserves clean machine payload while improving human readability and escaping/quoting behavior.
### 5) Graph naming for restart commands
Restart command uses `--graph`, not repo id.
Plan:
- Derive graph name via existing repo/graph conversion helper before rendering command guidance.
- Ensure guidance is aligned with user-facing graph names shown in CLI output.
---
## Implementation plan (task list)
1. Extract revision mismatch helper from `command/server.cljs` into shared location.
2. Update `server list` command to call the shared helper (behavior unchanged).
3. Add `check-server-revision-mismatch` to `command/doctor.cljs`.
4. Extend `execute-doctor` to append the new check and compute combined warning status.
5. Add per-graph restart guidance to human doctor output.
6. Keep JSON/EDN payload structured and stable.
7. Update CLI docs for `doctor` warning behavior and remediation command.
---
## Testing plan
### Unit tests
- `src/test/logseq/cli/command/doctor_test.cljs`
- Adds warning check when one or more servers have mismatched revisions.
- Includes restart command guidance for each mismatched graph.
- Treats missing server revision as mismatch (exact compare behavior).
- Keeps existing fail-fast behavior for script/data-dir errors.
- `src/test/logseq/cli/command/server_test.cljs`
- Verifies `server list` still uses exact-string mismatch semantics after helper extraction.
- Confirms human mismatch metadata remains attached when expected.
- `src/test/logseq/cli/format_test.cljs`
- Human `doctor` output includes mismatch warning and restart guidance.
- JSON/EDN doctor outputs include structured mismatch fields.
### Regression checks
- Existing `running-servers` warning behavior is preserved.
- Existing `server list` mismatch warning behavior is preserved.
- `doctor` top-level status semantics remain unchanged (`ok|warning|error` through current response model).
---
## Risks and mitigations
- **Risk:** Duplicate warning noise (`running-servers` + mismatch) in one doctor run.
- **Mitigation:** Keep distinct check IDs/messages so users can tell readiness vs revision drift.
- **Risk:** Restart command may fail with owner restrictions.
- **Mitigation:** Keep guidance explicit but non-blocking; existing error hint `server-owned-by-other` remains the fallback behavior.
- **Risk:** Missing revision in legacy lock files increases warning count.
- **Mitigation:** Treat as mismatch by design; message should clearly indicate missing server revision value.
---
## Acceptance criteria
- `logseq doctor` emits a warning check when any discovered server revision differs from local CLI revision.
- Warning includes actionable restart guidance: `logseq server restart --graph <name>` for each mismatched graph.
- No mismatch warning when all discovered server revisions exactly equal CLI revision.
- `server list` mismatch behavior remains consistent with `doctor` (shared comparison semantics).
- `doctor --output json|edn` includes structured mismatch check data.
---
## File scope (expected)
- `src/main/logseq/cli/command/doctor.cljs`
- `src/main/logseq/cli/command/server.cljs`
- `src/main/logseq/cli/server.cljs`
- `src/main/logseq/cli/format.cljs` (if dedicated doctor warning rendering is added)
- `src/test/logseq/cli/command/doctor_test.cljs`
- `src/test/logseq/cli/command/server_test.cljs`
- `src/test/logseq/cli/format_test.cljs`
- `docs/cli/logseq-cli.md`
- `docs/agent-guide/059-cli-doctor-server-revision-mismatch-warning.md`

View File

@@ -247,12 +247,13 @@ Output formats:
```
- JSON example: `{"status":"ok","data":{"result":[123]}}`
- EDN example: `{:status :ok, :data {:result [123]}}`
- `doctor` output includes overall status (`ok`, `warning`, `error`) and per-check rows for `db-worker-script`, `data-dir`, and `running-servers`. For scripting, `--output json|edn` keeps the structured check payload.
- Common doctor failures:
- `doctor` output includes overall status (`ok`, `warning`, `error`) and per-check rows for `db-worker-script`, `data-dir`, `running-servers`, and `server-revision-mismatch`. For scripting, `--output json|edn` keeps the structured check payload.
- Common doctor failures and warnings:
- `doctor-script-missing`: `db-worker-node.js` runtime target is missing (default target: `dist/db-worker-node.js`; use `doctor --dev-script` to check `static/db-worker-node.js`).
- `doctor-script-unreadable`: script path exists but is not a readable file.
- `data-dir-permission`: configured data dir is not readable or writable.
- `doctor-server-not-ready`: one or more lock-discovered servers are still in `:starting` state (warning).
- `doctor-server-revision-mismatch`: one or more discovered servers use a different revision than the local CLI revision (warning). Follow the printed remediation command for each affected graph: `logseq server restart --graph <name>`.
- If bundled runtime startup fails with missing-module or missing-file errors, rebuild with `yarn db-worker-node:release:bundle` and confirm `dist/db-worker-node.js` exists and every path listed in `dist/db-worker-node-assets.json` is present next to it.
- `query` human output returns a plain string (the query result rendered via `pr-str`), which is convenient for pipelines like `logseq query ... | xargs logseq show --id`.
- Built-in named queries currently include `block-search`, `task-search`, `recent-updated`, `list-status`, and `list-priority`. Use `query list` to see the full set for your config.

View File

@@ -5,6 +5,7 @@
[logseq.cli.command.core :as core]
[logseq.cli.data-dir :as data-dir]
[logseq.cli.server :as cli-server]
[logseq.cli.version :as version]
[promesa.core :as p]))
(def entries
@@ -101,6 +102,7 @@
(if (seq starting)
{:ok? true
:warning? true
:servers servers
:check {:id :running-servers
:status :warning
:code :doctor-server-not-ready
@@ -112,6 +114,7 @@
(string/join ", " (map :repo starting)))}}
{:ok? true
:warning? false
:servers servers
:check {:id :running-servers
:status :ok
:servers servers
@@ -126,6 +129,37 @@
:message (or (.-message e)
"running server check failed")}}))))
(defn- check-server-revision-mismatch
[cli-revision servers]
(let [revision-mismatch (cli-server/compute-revision-mismatches cli-revision servers)
mismatch-servers (mapv (fn [{:keys [repo revision]}]
{:repo repo
:graph (core/repo->graph repo)
:revision revision})
(or (:servers revision-mismatch) []))
mismatch-count (count mismatch-servers)]
(if (pos? mismatch-count)
{:ok? true
:warning? true
:check {:id :server-revision-mismatch
:status :warning
:code :doctor-server-revision-mismatch
:cli-revision cli-revision
:servers mismatch-servers
:message (str mismatch-count
" server"
(when (> mismatch-count 1) "s")
" "
(if (= 1 mismatch-count) "uses" "use")
" a different revision than this CLI")}}
{:ok? true
:warning? false
:check {:id :server-revision-mismatch
:status :ok
:cli-revision cli-revision
:servers []
:message "All discovered servers match this CLI revision"}})))
(defn execute-doctor
[action config]
(p/let [script-check (check-db-worker-script action)]
@@ -138,11 +172,20 @@
(let [check (:check data-dir-check)
checks (conj checks check)]
(doctor-error checks (:code check) (:message check)))
(p/let [server-check (check-running-servers config)
checks (conj checks (:check data-dir-check) (:check server-check))]
(p/let [server-check (check-running-servers config)]
(if-not (:ok? server-check)
(let [check (:check server-check)]
(let [checks (conj checks (:check data-dir-check) (:check server-check))
check (:check server-check)]
(doctor-error checks (:code check) (:message check)))
{:status :ok
:data {:status (if (:warning? server-check) :warning :ok)
:checks checks}})))))))
(let [revision-check (check-server-revision-mismatch (version/revision)
(:servers server-check))
checks (conj checks
(:check data-dir-check)
(:check server-check)
(:check revision-check))]
{:status :ok
:data {:status (if (or (:warning? server-check)
(:warning? revision-check))
:warning
:ok)
:checks checks}}))))))))

View File

@@ -70,22 +70,10 @@
{:status :error
:error (:error result)}))
(defn- compute-revision-mismatches
[cli-revision servers]
(let [mismatch-servers (->> (or servers [])
(filter (fn [{:keys [revision]}]
(not= cli-revision revision)))
(mapv (fn [{:keys [repo revision]}]
{:repo repo
:revision revision})))]
(when (seq mismatch-servers)
{:cli-revision cli-revision
:servers mismatch-servers})))
(defn execute-list
[_action config]
(-> (p/let [servers (cli-server/list-servers config)
revision-mismatch (compute-revision-mismatches (version/revision) servers)]
revision-mismatch (cli-server/compute-revision-mismatches (version/revision) servers)]
(cond-> {:status :ok
:data {:servers servers}}
revision-mismatch

View File

@@ -527,16 +527,32 @@
"Updated")]
(str verb " graph " (pr-str graph))))
(defn- quote-cli-arg
[value]
(let [value (normalize-cell value)]
(if (re-find #"\s" value)
(str "\"" (string/replace value #"\"" "\\\"") "\"")
value)))
(defn- format-doctor-check
[{:keys [id status message servers]}]
(let [check-line (str "[" (name (or status :unknown))
"] "
(name (or id :unknown))
(when (seq message)
(str " - " message)))]
(if (= :server-revision-mismatch id)
(let [guidance-lines (mapv (fn [{:keys [graph repo]}]
(str " Run: logseq server restart --graph "
(quote-cli-arg (or graph repo))))
(or servers []))]
(into [check-line] guidance-lines))
[check-line])))
(defn- format-doctor
[status checks]
(let [header (str "Doctor: " (name (or status :unknown)))
check-lines (mapv (fn [{:keys [id status message]}]
(str "[" (name (or status :unknown))
"] "
(name (or id :unknown))
(when (seq message)
(str " - " message))))
(or checks []))]
check-lines (mapcat format-doctor-check (or checks []))]
(string/join "\n" (into [header] check-lines))))
(defn- ->human

View File

@@ -298,6 +298,18 @@
:revision (:revision lock)
:status (if ready :ready :starting)})))))
(defn compute-revision-mismatches
[cli-revision servers]
(let [mismatch-servers (->> (or servers [])
(filter (fn [{:keys [revision]}]
(not= cli-revision revision)))
(mapv (fn [{:keys [repo revision]}]
{:repo repo
:revision revision})))]
(when (seq mismatch-servers)
{:cli-revision cli-revision
:servers mismatch-servers})))
(defn list-graphs
[config]
(let [data-dir (resolve-data-dir config)

View File

@@ -4,6 +4,7 @@
[logseq.cli.commands :as commands]
[logseq.cli.data-dir :as data-dir]
[logseq.cli.server :as cli-server]
[logseq.cli.version :as cli-version]
[promesa.core :as p]
[logseq.cli.command.doctor :as doctor-command]))
@@ -60,15 +61,16 @@
(deftest test-execute-doctor-all-checks-pass
(async done
(-> (p/with-redefs [data-dir/ensure-data-dir! (fn [_] "/tmp/logseq-doctor")
cli-version/revision (fn [] "cli-rev")
cli-server/list-servers (fn [_] (p/resolved []))]
(p/let [result (commands/execute {:type :doctor
:script-path "src/main/logseq/cli/commands.cljs"}
{:data-dir "/tmp/logseq-doctor"})]
(is (= :ok (:status result)))
(is (= :ok (get-in result [:data :status])))
(is (= [:db-worker-script :data-dir :running-servers]
(is (= [:db-worker-script :data-dir :running-servers :server-revision-mismatch]
(mapv :id (get-in result [:data :checks]))))
(is (= [:ok :ok :ok]
(is (= [:ok :ok :ok :ok]
(mapv :status (get-in result [:data :checks]))))))
(p/catch (fn [e]
(is false (str "unexpected error: " e))))
@@ -77,11 +79,13 @@
(deftest test-execute-doctor-starting-server-warning
(async done
(-> (p/with-redefs [data-dir/ensure-data-dir! (fn [_] "/tmp/logseq-doctor")
cli-version/revision (fn [] "cli-rev")
cli-server/list-servers (fn [_]
(p/resolved [{:repo "logseq_db_demo"
:status :starting
:host "127.0.0.1"
:port 9010}]))]
:port 9010
:revision "cli-rev"}]))]
(p/let [result (commands/execute {:type :doctor
:script-path "src/main/logseq/cli/commands.cljs"}
{:data-dir "/tmp/logseq-doctor"})]
@@ -92,7 +96,62 @@
(is (= :warning
(get-in result [:data :checks 2 :status])))
(is (= :doctor-server-not-ready
(get-in result [:data :checks 2 :code])))))
(get-in result [:data :checks 2 :code])))
(is (= :server-revision-mismatch
(get-in result [:data :checks 3 :id])))
(is (= :ok
(get-in result [:data :checks 3 :status])))))
(p/catch (fn [e]
(is false (str "unexpected error: " e))))
(p/finally done))))
(deftest test-execute-doctor-server-revision-mismatch-warning
(async done
(-> (p/with-redefs [data-dir/ensure-data-dir! (fn [_] "/tmp/logseq-doctor")
cli-version/revision (fn [] "cli-rev")
cli-server/list-servers (fn [_]
(p/resolved [{:repo "logseq_db_graph-a"
:status :ready
:revision "cli-rev"}
{:repo "logseq_db_graph-b"
:status :ready
:revision "worker-rev"}]))]
(p/let [result (commands/execute {:type :doctor
:script-path "src/main/logseq/cli/commands.cljs"}
{:data-dir "/tmp/logseq-doctor"})]
(is (= :ok (:status result)))
(is (= :warning (get-in result [:data :status])))
(is (= :server-revision-mismatch
(get-in result [:data :checks 3 :id])))
(is (= :warning
(get-in result [:data :checks 3 :status])))
(is (= :doctor-server-revision-mismatch
(get-in result [:data :checks 3 :code])))
(is (= "cli-rev"
(get-in result [:data :checks 3 :cli-revision])))
(is (= ["logseq_db_graph-b"]
(mapv :repo (get-in result [:data :checks 3 :servers]))))
(is (= ["graph-b"]
(mapv :graph (get-in result [:data :checks 3 :servers]))))))
(p/catch (fn [e]
(is false (str "unexpected error: " e))))
(p/finally done))))
(deftest test-execute-doctor-missing-server-revision-is-mismatch
(async done
(-> (p/with-redefs [data-dir/ensure-data-dir! (fn [_] "/tmp/logseq-doctor")
cli-version/revision (fn [] "cli-rev")
cli-server/list-servers (fn [_]
(p/resolved [{:repo "logseq_db_graph-a"
:status :ready}]))]
(p/let [result (commands/execute {:type :doctor
:script-path "src/main/logseq/cli/commands.cljs"}
{:data-dir "/tmp/logseq-doctor"})]
(is (= :ok (:status result)))
(is (= :warning (get-in result [:data :status])))
(is (= :warning
(get-in result [:data :checks 3 :status])))
(is (nil? (get-in result [:data :checks 3 :servers 0 :revision])))))
(p/catch (fn [e]
(is false (str "unexpected error: " e))))
(p/finally done))))

View File

@@ -6,8 +6,7 @@
[promesa.core :as p]))
(deftest compute-revision-mismatches-uses-exact-string-compare
(let [compute-revision-mismatches #'server-command/compute-revision-mismatches
mismatch (compute-revision-mismatches
(let [mismatch (cli-server/compute-revision-mismatches
"cli-rev"
[{:repo "graph-a" :revision "cli-rev"}
{:repo "graph-b" :revision "cli-rev-dirty"}])]

View File

@@ -811,6 +811,22 @@
"[ok] db-worker-script - Found readable file: /tmp/db-worker-node.js\n"
"[ok] data-dir - Read/write access confirmed: /tmp/logseq/graphs\n"
"[warning] running-servers - 1 server is still starting")
result))))
(testing "doctor includes restart guidance for revision mismatch"
(let [result (format/format-result {:status :ok
:command :doctor
:data {:status :warning
:checks [{:id :server-revision-mismatch
:status :warning
:message "2 servers use a different revision than this CLI"
:servers [{:graph "graph-a"}
{:graph "team graph"}]}]}}
{:output-format nil})]
(is (= (str "Doctor: warning\n"
"[warning] server-revision-mismatch - 2 servers use a different revision than this CLI\n"
" Run: logseq server restart --graph graph-a\n"
" Run: logseq server restart --graph \"team graph\"")
result)))))
(deftest test-doctor-json-edn-output
@@ -844,3 +860,36 @@
(is (= :data-dir-permission (get-in parsed-edn [:error :code])))
(is (= :data-dir (get-in parsed-edn [:data :checks 1 :id])))
(is (= :error (get-in parsed-edn [:data :checks 1 :status]))))))
(deftest test-doctor-json-edn-output-includes-revision-mismatch-check
(let [payload {:status :warning
:checks [{:id :server-revision-mismatch
:status :warning
:code :doctor-server-revision-mismatch
:cli-revision "cli-rev"
:servers [{:repo "logseq_db_graph-b"
:graph "logseq_db_graph-b"
:revision "worker-rev"}]
:message "1 server uses a different revision than this CLI"}]}
json-result (format/format-result {:status :ok
:command :doctor
:data payload}
{:output-format :json})
edn-result (format/format-result {:status :ok
:command :doctor
:data payload}
{:output-format :edn})
parsed-json (js->clj (js/JSON.parse json-result) :keywordize-keys true)
parsed-edn (reader/read-string edn-result)]
(is (= "ok" (:status parsed-json)))
(is (= "warning" (get-in parsed-json [:data :status])))
(is (= "server-revision-mismatch" (get-in parsed-json [:data :checks 0 :id])))
(is (= "doctor-server-revision-mismatch" (get-in parsed-json [:data :checks 0 :code])))
(is (= "graph-b" (get-in parsed-json [:data :checks 0 :servers 0 :repo])))
(is (= "graph-b" (get-in parsed-json [:data :checks 0 :servers 0 :graph])))
(is (= :ok (:status parsed-edn)))
(is (= :warning (get-in parsed-edn [:data :status])))
(is (= :server-revision-mismatch (get-in parsed-edn [:data :checks 0 :id])))
(is (= :doctor-server-revision-mismatch (get-in parsed-edn [:data :checks 0 :code])))
(is (= "graph-b" (get-in parsed-edn [:data :checks 0 :servers 0 :repo])))
(is (= "graph-b" (get-in parsed-edn [:data :checks 0 :servers 0 :graph])))))