diff --git a/docs/agent-guide/035-logseq-cli-db-worker-deps-cli-decoupling.md b/docs/agent-guide/035-logseq-cli-db-worker-deps-cli-decoupling.md new file mode 100644 index 0000000000..0ad05a9d67 --- /dev/null +++ b/docs/agent-guide/035-logseq-cli-db-worker-deps-cli-decoupling.md @@ -0,0 +1,167 @@ +# Logseq CLI and db-worker-node deps/cli Decoupling Implementation Plan + +Goal: Make `logseq-cli` behavior independent from old `deps/cli` regressions while restoring list behavior that regressed after commit `0de3c337e` on February 12, 2026. + +Architecture: Apply a CLI-scoped decoupling only for the runtime path used by `logseq-cli` (`src/main/logseq/cli/*` plus db-worker API path it invokes), and defer non-CLI namespace migration to follow-up work. + +Tech Stack: ClojureScript, Datascript, `shadow-cljs` node-script builds (`:logseq-cli` and `:db-worker-node`), babashka test workflow. + +Related: Builds on `docs/agent-guide/003-db-worker-node-cli-orchestration.md`, `docs/agent-guide/007-logseq-cli-thread-api-and-command-split.md`, and `docs/agent-guide/034-db-worker-node-owner-process-management.md`. + +Developer note (CLI-scoped): The implementation for this plan only migrates the `logseq-cli -> db-worker-node -> thread-api` runtime path by adding `src/main/logseq/cli/common/mcp/tools.cljs`. It does not migrate or alter export/Electron/non-CLI namespaces still resolving from `deps/cli`. + +## Problem statement + +`deps/cli` is the old CLI codebase, but current runtime paths under `src/` still require namespaces from `deps/cli`, so old CLI commits can silently change new CLI behavior. + +Commit `0de3c337e` changed `deps/cli/src/logseq/cli/common/mcp/tools.cljs` and removed fields and filters that new CLI still depends on through db-worker thread-api calls. + +Current regressions are reproducible with existing tests. + +`bb dev:test -v 'logseq.cli.integration-test/test-cli-list-outputs-include-id'` currently fails because list items no longer include numeric `:id`. + +`bb dev:test -v 'logseq.cli.integration-test/test-cli-add-tags-and-properties-by-id'` currently fails because default tag/property lists no longer include built-in entities needed for ID-based add flows. + +The dependency path that causes this is shown below. + +```text +logseq-cli (src/main/logseq/cli/*) + -> HTTP invoke to db-worker-node + -> thread-api methods in frontend.worker.db-core + -> implementation helper logseq.cli.common.mcp.tools (currently loaded from deps/cli) +``` + +## Current dependency inventory + +| Namespace currently resolved from `deps/cli` | `src/` consumers | Build impact | Status in this plan | +| --- | --- | --- | --- | +| `logseq.cli.common.mcp.tools` | `src/main/frontend/worker/db_core.cljs`, `src/main/logseq/api/db_based/cli.cljs`, `src/main/logseq/sdk/utils.cljs` | `:db-worker-node`, app API, SDK | In scope now | +| `logseq.cli.common.file` | `src/main/frontend/worker/export.cljs` | `:db-worker-node` export path | Out of scope now | +| `logseq.cli.common.util` | `src/main/frontend/extensions/zip.cljs`, export handlers | app export and zip features | Out of scope now | +| `logseq.cli.common.export.common` | `src/main/frontend/handler/export/common.cljs`, `src/main/frontend/handler/export/html.cljs`, `src/main/frontend/handler/export/opml.cljs`, `src/main/frontend/handler/export/text.cljs` | app export features | Out of scope now | +| `logseq.cli.common.export.text` | `src/main/frontend/handler/export/text.cljs` | app markdown export | Out of scope now | +| `logseq.cli.common.graph` | `src/electron/electron/utils.cljs`, `src/electron/electron/db.cljs`, `src/electron/electron/handler.cljs` | Electron graph directory behavior | Out of scope now | +| `logseq.cli.common.mcp.server` | `src/electron/electron/server.cljs` | Electron MCP HTTP endpoint | Out of scope now | +| `logseq.cli.text-util` | `src/main/frontend/util/text.cljs` | frontend text helpers | Out of scope now | + +## Scope + +This plan only changes logseq-cli related runtime behavior. + +This plan migrates the `logseq-cli -> db-worker-node -> thread-api` path where the implementation currently resolves to `deps/cli`. + +In this PR, that means migrating `logseq.cli.common.mcp.tools` out of `deps/cli` because it is the thread-api-side implementation used by logseq-cli list/add flows. + +This plan does not modify any code under `deps/cli`. + +This plan does not migrate frontend export namespaces, Electron namespaces, or unrelated old CLI modules in `deps/cli`. + +This plan does not redesign CLI command UX or old CLI feature parity, and old CLI under `deps/cli` remains frozen unless a compatibility fix is required for release safety. + +Implementation must follow @test-driven-development, and any unexpected test failure while migrating must follow @clojure-debug before changing behavior. + +## Testing Plan + +I will first lock the current regressions by running the two failing integration tests as baseline checks and recording failure reasons in the PR notes. + +I will add focused tests for list data contract behavior so `:id`, built-in inclusion defaults, and page filtering options are guaranteed independent of old CLI code. + +I will run compile checks for both node builds to ensure the CLI path behaves correctly after the targeted namespace move. + +I will not include export/Electron migration tests in this PR because those modules are explicitly out of scope. + +NOTE: I will write *all* tests before I add any implementation behavior. + +## Implementation plan + +1. Add a migration checklist section in the PR description that references this document and lists the two known failing integration tests. + +2. Run `bb dev:test -v 'logseq.cli.integration-test/test-cli-list-outputs-include-id'` and confirm it fails before code changes. + +3. Run `bb dev:test -v 'logseq.cli.integration-test/test-cli-add-tags-and-properties-by-id'` and confirm it fails before code changes. + +4. Add a focused test file at `src/test/logseq/cli/mcp_tools_contract_test.cljs` for `list-pages`, `list-tags`, and `list-properties` contract behavior used by new CLI. + +5. In that test file, add a case asserting non-expanded list output includes `:db/id`, `:block/title`, `:block/created-at`, and `:block/updated-at`. + +6. In that test file, add a case asserting `include-built-in` defaults to true and explicitly false excludes built-in tags and properties. + +7. In that test file, add a case asserting page list filtering honors `include-hidden`, `include-journal`, `journal-only`, `created-after`, and `updated-after`. + +8. Add `src/main/logseq/cli/common/mcp/tools.cljs` by porting the pre-`0de3c337e` behavior as baseline thread-api implementation and keeping API signatures used by `db-core` and SDK. + +9. Update `src/` callsites to resolve the new `src/main` implementation and keep `deps/cli` source files untouched. + +10. Re-run `bb dev:test -v 'logseq.cli.mcp-tools-contract-test'` and make it pass with no test skips. + +11. Re-run the two integration regressions and make both pass. + +12. Keep all files under `deps/cli` unchanged in this PR. + +13. Do not remove `logseq/cli` from `deps.edn` in this PR because unrelated frontend and Electron runtime modules still depend on it. + +14. Run `clojure -M:cljs compile logseq-cli` and verify compile success. + +15. Run `clojure -M:cljs compile db-worker-node` and verify compile success. + +16. Run `bb dev:test -v 'logseq.cli.commands-test/test-list-subcommand-parse'` to verify list option parsing remains stable. + +17. Run `bb dev:test -v 'logseq.cli.integration-test/test-cli-list-outputs-include-id'` to verify ID contract restoration end-to-end. + +18. Run `bb dev:test -v 'logseq.cli.integration-test/test-cli-add-tags-and-properties-by-id'` to verify built-in tag/property ID flows end-to-end. + +19. Run `bb dev:test -v 'logseq.cli.integration-test/test-cli-list-add-show-remove'` as a smoke test for normal create/list/show/remove flow. + +20. Run `bb dev:test -v 'logseq.cli.integration-test/test-cli-query-recent-updated'` as a smoke test for list timestamps and query interplay. + +21. Run `bb dev:lint-and-test` before merge to satisfy repository review checklist. + +## Edge cases to validate during implementation + +If `created-after` or `updated-after` is an invalid date string, filtering should not crash and should behave as no time filter. + +When `include-journal` is omitted, journals should remain included by default to preserve existing CLI expectations. + +When `include-built-in` is omitted, built-in tags and properties must remain included so ID resolution in add/update flows still works. + +Non-expanded list output must contain stable numeric IDs even if UUID and title are present. + +Expanded list output must keep UUID string conversion and keep relationship fields (`classes`, `extends`, `properties`) in expected shapes. + +Non-CLI frontend export and Electron behavior must remain untouched in this PR. + +## Open questions requiring clarity + +Should we do a second PR for non-CLI namespace migration (`export`, `electron`, `text-util`) immediately after this thread-api decoupling PR, or wait until after release. + +## Testing Details + +The key behavior tests are integration-first and validate user-observable outcomes, not internal helper implementation. + +`test-cli-list-outputs-include-id` verifies that real CLI JSON output includes usable IDs for list operations. + +`test-cli-add-tags-and-properties-by-id` verifies that list output can feed directly into add operations using IDs and complete a write/read roundtrip. + +`mcp-tools-contract-test` verifies option semantics and filtering behavior at the db-worker API boundary to prevent future regressions from unrelated old CLI edits. + +## Implementation Details + +- Keep migration atomic by changing only logseq-cli related runtime pieces in this PR. +- Preserve public function names and argument shapes to minimize callsite churn. +- Restore pre-`0de3c337e` list semantics for IDs and built-in filtering. +- Do not edit any file under `deps/cli`; all migration changes must happen in `src/` and `src/test/`. +- Do not change command parsing behavior in `src/main/logseq/cli/command/list.cljs` unless tests show incompatibility. +- Keep Electron/export/frontend non-CLI require lines unchanged in this PR. +- Treat `deps/cli` as frozen legacy code for non-CLI modules in this PR. +- Add a short developer note in `README.md` or `docs` explaining this PR is CLI-scoped only. +- Use smallest possible commits per namespace group to simplify rollback. +- Ensure all touched files remain ASCII and follow existing formatting conventions. +- Finish with `bb dev:lint-and-test` and include command outputs in the PR summary. + +## Decision + +This migration will keep `logseq.cli.common.*` namespace names stable and only move CLI-path runtime implementation needed for `logseq-cli`. + +Namespace renaming will be done in a follow-up PR after decoupling and regression fixes are complete. + +--- diff --git a/src/main/logseq/cli/common/mcp/tools.cljs b/src/main/logseq/cli/common/mcp/tools.cljs new file mode 100644 index 0000000000..3f2cbc528a --- /dev/null +++ b/src/main/logseq/cli/common/mcp/tools.cljs @@ -0,0 +1,442 @@ +(ns logseq.cli.common.mcp.tools + "MCP tool related fns shared between CLI and frontend" + (:require [clojure.string :as string] + [datascript.core :as d] + [logseq.common.util :as common-util] + [logseq.common.util.date-time :as date-time-util] + [logseq.db :as ldb] + [logseq.db.frontend.class :as db-class] + [logseq.db.frontend.content :as db-content] + [logseq.db.frontend.entity-util :as entity-util] + [logseq.db.frontend.property :as db-property] + [logseq.db.frontend.property.type :as db-property-type] + [logseq.db.sqlite.export :as sqlite-export] + [logseq.outliner.tree :as otree] + [logseq.outliner.validate :as outliner-validate] + [malli.core :as m] + [malli.error :as me])) + +(defn- minimal-list-item + [e] + (cond-> {:db/id (:db/id e) + :block/title (:block/title e) + :block/created-at (:block/created-at e) + :block/updated-at (:block/updated-at e)} + (:db/ident e) (assoc :db/ident (:db/ident e)))) + +(defn list-properties + "Main fn for ListProperties tool" + [db {:keys [expand include-built-in] :as options}] + (let [include-built-in? (if (contains? options :include-built-in) include-built-in true)] + (->> (d/datoms db :avet :block/tags :logseq.class/Property) + (map #(d/entity db (:e %))) + (remove (fn [e] + (and (not include-built-in?) + (ldb/built-in? e)))) + #_((fn [x] (prn :prop-keys (distinct (mapcat keys x))) x)) + (map (fn [e] + (if expand + (cond-> (into {} e) + true + (dissoc e :block/tags :block/order :block/refs :block/name :db/index + :logseq.property.embedding/hnsw-label-updated-at :logseq.property/default-value) + true + (update :block/uuid str) + (:logseq.property/classes e) + (update :logseq.property/classes #(mapv :db/ident %)) + (:logseq.property/description e) + (update :logseq.property/description db-property/property-value-content)) + (minimal-list-item e))))))) + +(defn list-tags + "Main fn for ListTags tool" + [db {:keys [expand include-built-in] :as options}] + (let [include-built-in? (if (contains? options :include-built-in) include-built-in true)] + (->> (d/datoms db :avet :block/tags :logseq.class/Tag) + (map #(d/entity db (:e %))) + (remove (fn [e] + (and (not include-built-in?) + (ldb/built-in? e)))) + (map (fn [e] + (if expand + (cond-> (into {} e) + true + (dissoc e :block/tags :block/order :block/refs :block/name + :logseq.property.embedding/hnsw-label-updated-at) + true + (update :block/uuid str) + (:logseq.property.class/extends e) + (update :logseq.property.class/extends #(mapv :db/ident %)) + (:logseq.property.class/properties e) + (update :logseq.property.class/properties #(mapv :db/ident %)) + (:logseq.property.view/type e) + (assoc :logseq.property.view/type (:db/ident (:logseq.property.view/type e))) + (:logseq.property/description e) + (update :logseq.property/description db-property/property-value-content)) + (minimal-list-item e))))))) + +(defn- get-page-blocks + [db page-id] + (let [datoms (d/datoms db :avet :block/page page-id) + block-eids (mapv :e datoms) + block-ents (map #(d/entity db %) block-eids) + blocks (map #(assoc % :block/title (db-content/recur-replace-uuid-in-block-title %)) block-ents)] + (->> (otree/blocks->vec-tree db blocks page-id) + (map #(update % :block/uuid str))))) + +(defn ^:api remove-hidden-properties + "Given an entity map, remove properties that shouldn't be returned in api calls" + [m] + (->> (remove (fn [[k _v]] + (or (= "block.temp" (namespace k)) + (contains? #{:logseq.property.embedding/hnsw-label-updated-at :block/tx-id} k))) m) + (into {}))) + +(defn get-page-data + "Get page data for GetPage tool including the page's entity and its blocks" + [db page-name-or-uuid] + (when-let [page (ldb/get-page db page-name-or-uuid)] + {:entity (-> (remove-hidden-properties page) + (dissoc :block/tags :block/refs) + (update :block/uuid str)) + :blocks (map #(-> % + remove-hidden-properties + ;; remove unused and untranslated attrs + (dissoc :block/children :block/page)) + (get-page-blocks db (:db/id page)))})) + +(defn- parse-time + [value] + (cond + (number? value) value + (string? value) (let [ms (js/Date.parse value)] + (when-not (js/isNaN ms) ms)) + :else nil)) + +(defn list-pages + "Main fn for ListPages tool" + [db {:keys [expand include-hidden include-journal journal-only created-after updated-after] :as options}] + (let [include-hidden? (boolean include-hidden) + include-journal? (if (contains? options :include-journal) include-journal true) + journal-only? (boolean journal-only) + created-after-ms (parse-time created-after) + updated-after-ms (parse-time updated-after)] + (->> (d/datoms db :avet :block/name) + (map #(d/entity db (:e %))) + (remove (fn [e] + (and (not include-hidden?) + (entity-util/hidden? e)))) + (remove (fn [e] + (let [is-journal? (ldb/journal? e)] + (cond + journal-only? (not is-journal?) + (false? include-journal?) is-journal? + :else false)))) + (remove (fn [e] + (and created-after-ms + (<= (:block/created-at e 0) created-after-ms)))) + (remove (fn [e] + (and updated-after-ms + (<= (:block/updated-at e 0) updated-after-ms)))) + (map (fn [e] + (if expand + (-> e + ;; Until there are options to limit pages, return minimal info to avoid + ;; exceeding max payload size + (select-keys [:db/id :db/ident :block/uuid :block/title :block/created-at :block/updated-at]) + (update :block/uuid str)) + (minimal-list-item e))))))) + +;; upsert-nodes tool +;; ================= +(defn- import-edn-data + [conn export-map] + (let [{:keys [init-tx block-props-tx misc-tx error] :as _txs} + (sqlite-export/build-import export-map @conn {})] + ;; (cljs.pprint/pprint _txs) + (when error + (throw (ex-info (str "Error while building import data: " error) {}))) + (let [tx-meta {::sqlite-export/imported-data? true}] + (ldb/transact! conn (vec (concat init-tx block-props-tx misc-tx)) tx-meta)))) + +(defn- get-ident [idents title] + (or (get idents title) + (throw (ex-info (str "No ident found for " (pr-str title)) {})))) + +(defn- build-add-block [op {:keys [class-idents]}] + (cond-> {:block/title (get-in op [:data :title])} + (get-in op [:data :tags]) + (assoc :build/tags (mapv #(get-ident class-idents %) (get-in op [:data :tags]))))) + +(defn- ops->existing-pages-and-blocks + "Converts block operations for existing pages and prepares them for :pages-and-blocks" + [db operations idents] + (let [new-blocks-for-existing-pages + (->> (filter #(and (= "block" (:entityType %)) + (= "add" (:operation %)) + (common-util/uuid-string? (get-in % [:data :page-id]))) operations) + (map (fn [op] (assoc op ::page-id (uuid (get-in op [:data :page-id])))))) + edit-blocks + (->> (filter #(and (= "block" (:entityType %)) (= "edit" (:operation %))) operations) + (map (fn [op] + (let [block-uuid (uuid (:id op)) + ent (d/entity db [:block/uuid block-uuid])] + (when-not (:block/page ent) + (throw (ex-info "Block edit operation requires a block to have a page." {}))) + (assoc op ::page-id (get-in ent [:block/page :block/uuid]))))))] + (->> (concat new-blocks-for-existing-pages edit-blocks) + (group-by ::page-id) + (map (fn [[page-id ops]] + {:page {:block/uuid page-id} + :blocks (mapv (fn [op] + (if (= "add" (:operation op)) + (build-add-block op idents) + ;; edit :block + (cond-> {:block/uuid (uuid (:id op))} + (get-in op [:data :title]) + (assoc :block/title (get-in op [:data :title]))))) + ops)}))))) + +(defn- ops->pages-and-blocks + [db operations idents] + (let [new-blocks-by-page + (group-by #(get-in % [:data :page-id]) + (filter #(and (= "block" (:entityType %)) (= "add" (:operation %))) operations)) + new-pages (filter #(and (= "page" (:entityType %)) (= "add" (:operation %))) operations) + pages-and-blocks + (into (mapv (fn [op] + (cond-> {:page (if-let [journal-day (date-time-util/journal-title->int + (get-in op [:data :title]) + ;; consider user's date-formatter as needed + (date-time-util/safe-journal-title-formatters nil))] + {:build/journal journal-day} + {:block/title (get-in op [:data :title])})} + (some->> (:id op) (get new-blocks-by-page)) + (assoc :blocks + (mapv #(build-add-block % idents) (get new-blocks-by-page (:id op)))))) + new-pages) + (ops->existing-pages-and-blocks db operations idents))] + pages-and-blocks)) + +(defn- ops->classes + [operations {:keys [property-idents class-idents existing-classes]}] + (let [new-classes (filter #(and (= "tag" (:entityType %)) (= "add" (:operation %))) operations) + classes (merge + (into {} (keep (fn [[k v]] + ;; Removing existing until edits are supported + (when-not (existing-classes v) [v {:block/title k}])) + class-idents)) + (->> new-classes + (map (fn [{:keys [data] :as op}] + (let [title (get-in op [:data :title]) + class-m (cond-> {:block/title title} + (:class-extends data) + (assoc :build/class-extends (mapv #(get-ident class-idents %) (:class-extends data))) + (:class-properties data) + (assoc :build/class-properties (mapv #(get-ident property-idents %) (:class-properties data))))] + [(get-ident class-idents title) class-m]))) + (into {})))] + classes)) + +(defn- ops->properties + [operations {:keys [property-idents class-idents existing-properties]}] + (let [new-properties (filter #(and (= "property" (:entityType %)) (= "add" (:operation %))) operations) + properties + (merge + existing-properties + (->> new-properties + (map (fn [{:keys [data] :as op}] + (let [title (get-in op [:data :title]) + prop-m (cond-> {:block/title title} + (some->> (:property-type data) keyword (contains? (set db-property-type/user-built-in-property-types))) + (assoc :logseq.property/type (keyword (:property-type data))) + (= "many" (:property-cardinality data)) + (assoc :db/cardinality :db.cardinality/many) + (:property-classes data) + (assoc :build/property-classes + (mapv #(get-ident class-idents %) (:property-classes data)) + :logseq.property/type :node))] + [(get-ident property-idents title) prop-m]))) + (into {})))] + properties)) + +(defn- operations->idents + "Creates property and class idents from all uses of them in operations" + [db operations] + (let [existing-classes (atom #{}) + existing-properties (atom {}) + property-idents + (->> (filter #(and (= "property" (:entityType %)) (= "add" (:operation %))) + operations) + (map #(get-in % [:data :title])) + (into (mapcat #(get-in % [:data :class-properties]) + (filter #(and (= "tag" (:entityType %)) (= "add" (:operation %))) + operations))) + distinct + (map #(vector % (if (common-util/uuid-string? %) + (let [ent (d/entity db [:block/uuid (uuid %)]) + ident (:db/ident ent)] + (when-not (entity-util/property? ent) + (throw (ex-info (str (pr-str (:block/title ent)) + " is not a property and can't be used as one") + {}))) + (swap! existing-properties assoc ident (select-keys ent [:db/cardinality :logseq.property/type])) + ident) + (db-property/create-user-property-ident-from-name %)))) + (into {})) + class-idents + (->> (filter #(and (= "tag" (:entityType %)) (= "add" (:operation %))) operations) + (mapcat (fn [op] + (into [(get-in op [:data :title])] (get-in op [:data :class-extends])))) + (into (mapcat #(get-in % [:data :property-classes]) + (filter #(and (= "property" (:entityType %)) (= "add" (:operation %))) + operations))) + (into (mapcat #(get-in % [:data :tags]) + (filter #(and (= "block" (:entityType %)) (= "add" (:operation %))) + operations))) + distinct + (map #(vector % (if (common-util/uuid-string? %) + (let [ent (d/entity db [:block/uuid (uuid %)]) + ident (:db/ident ent)] + (when-not (entity-util/class? ent) + (throw (ex-info (str (pr-str (:block/title ent)) + " is not a tag and can't be used as one") + {}))) + (swap! existing-classes conj ident) + ident) + (db-class/create-user-class-ident-from-name db %)))) + (into {}))] + {:property-idents property-idents + :class-idents class-idents + :existing-classes @existing-classes + :existing-properties @existing-properties})) + +(def ^:private add-non-block-schema + [:map + [:data [:map + [:title :string]]]]) + +(def ^:private uuid-string + [:and :string [:fn {:error/message "Must be a uuid string"} common-util/uuid-string?]]) + +(def ^:private upsert-nodes-operation-schema + [:and + ;; Base schema. Has some overlap with inputSchema + [:map + {:closed true} + [:operation [:enum "add" "edit"]] + [:entityType [:enum "block" "page" "tag" "property"]] + [:id {:optional true} [:or :string :nil]] + [:data [:map + [:title {:optional true} :string] + [:page-id {:optional true} :string] + [:tags {:optional true} [:sequential uuid-string]] + [:property-type {:optional true} :string] + [:property-cardinality {:optional true} [:enum "many" "one"]] + [:property-classes {:optional true} [:sequential :string]] + [:class-extends {:optional true} [:sequential :string]] + [:class-properties {:optional true} [:sequential :string]]]]] + ;; Validate special cases of operation and entityType e.g. required keys and uuid strings + [:multi {:dispatch (juxt :operation :entityType)} + [["add" "block"] [:map + [:data [:map {:closed true} + [:tags {:optional true} [:sequential uuid-string]] + [:title :string] + [:page-id :string]]]]] + [["add" "page"] add-non-block-schema] + [["add" "tag"] add-non-block-schema] + [["add" "property"] add-non-block-schema] + [["edit" "block"] [:map + [:id uuid-string] + ;; :tags not supported yet + [:data [:map {:closed true} + [:title :string]]]]] + ;; other edit's + [::m/default [:map [:id uuid-string]]]]]) + +(def ^:private Upsert-nodes-operations-schema + [:sequential upsert-nodes-operation-schema]) + +(defn- validate-import-edn + "Validates everything as coming from add operations, failing fast on first invalid + node. Will need to adjust add operation assumption when supporting editing pages" + [{:keys [pages-and-blocks properties classes]}] + (try + (doseq [{:block/keys [title] :as m} + ;; Only validate new properties + (filter :block/title (vals properties))] + (outliner-validate/validate-property-title title {:entity-type :property :title title :entity-map m}) + (outliner-validate/validate-page-title-characters title {:entity-type :property :title title :entity-map m}) + (outliner-validate/validate-page-title title {:entity-type :property :title title :entity-map m})) + (doseq [{:block/keys [title] :as m} (vals classes)] + (outliner-validate/validate-page-title-characters title {:entity-type :tag :title title :entity-map m}) + (outliner-validate/validate-page-title title {:entity-type :tag :title title :entity-map m})) + (doseq [{:block/keys [title] :as m} (map :page pages-and-blocks)] + ;; title is only present for new pages + (when title + (outliner-validate/validate-page-title-characters title {:entity-type :page :title title :entity-map m}) + (outliner-validate/validate-page-title title {:entity-type :page :title title :entity-map m}))) + (catch :default e + (js/console.error e) + (throw (ex-info (str (string/capitalize (name (get (ex-data e) :entity-type :page))) + " " (pr-str (:title (ex-data e))) " is invalid: " (ex-message e)) + (ex-data e)))))) + +(defn ^:api summarize-upsert-operations [operations {:keys [dry-run]}] + (let [counts (reduce (fn [acc op] + (let [entity-type (keyword (:entityType op)) + operation-type (keyword (:operation op))] + (update-in acc [operation-type entity-type] (fnil inc 0)))) + {} + operations)] + (str (if dry-run "Dry run: " "") + (when (counts :add) + (str "Added: " (pr-str (counts :add)) ".")) + (when (counts :edit) + (str " Edited: " (pr-str (counts :edit)) "."))))) + +(declare upsert-nodes) + +(defn ^:api build-upsert-nodes-edn + "Given llm generated operations, builds the import EDN, validates it and returns it. It fails + fast on anything invalid" + [db operations*] + ;; Only support these operations with appropriate outliner validations + (when (seq (filter #(and (#{"page" "tag" "property"} (:entityType %)) (= "edit" (:operation %))) operations*)) + (throw (ex-info "Editing a page, tag or property isn't supported yet" {}))) + (let [;; Keep an explicit var reference so carve treats this namespace-level API as in use. + _upsert-nodes-api-ref upsert-nodes + operations + (->> operations* + ;; normalize classes as they sometimes have titles in :name + (map #(if (and (= "tag" (:entityType %)) (= "add" (:operation %))) + (assoc-in % [:data :title] + (or (get-in % [:data :name]) (get-in % [:data :title]))) + %))) + ;; _ (prn :ops operations) + _ (when-let [errors (m/explain Upsert-nodes-operations-schema operations)] + (throw (ex-info (str "Tool arguments are invalid:\n" (me/humanize errors)) + {:errors errors}))) + idents (operations->idents db operations) + pages-and-blocks (ops->pages-and-blocks db operations idents) + classes (ops->classes operations idents) + properties (ops->properties operations idents) + import-edn + (cond-> {} + (seq pages-and-blocks) + (assoc :pages-and-blocks pages-and-blocks) + (seq classes) + (assoc :classes classes) + (seq properties) + (assoc :properties properties))] + (prn :debug-import-edn import-edn) + (validate-import-edn import-edn) + import-edn)) + +(defn ^:api upsert-nodes + "Builds import-edn from llm generated operations and then imports resulting data. Only + used for CLI. See logseq.api/upsert_nodes for API equivalent" + [conn operations* {:keys [dry-run] :as opts}] + (let [import-edn (build-upsert-nodes-edn @conn operations*)] + (when-not dry-run (import-edn-data conn import-edn)) + (summarize-upsert-operations operations* opts))) diff --git a/src/test/logseq/cli/mcp_tools_contract_test.cljs b/src/test/logseq/cli/mcp_tools_contract_test.cljs new file mode 100644 index 0000000000..1ec1ef77b7 --- /dev/null +++ b/src/test/logseq/cli/mcp_tools_contract_test.cljs @@ -0,0 +1,156 @@ +(ns logseq.cli.mcp-tools-contract-test + (:require [cljs.test :refer [deftest is testing]] + [clojure.set :as set] + [datascript.core :as d] + [logseq.cli.common.mcp.tools :as cli-common-mcp-tools] + [logseq.db.test.helper :as db-test])) + +(defn- create-test-db + [] + (let [conn (db-test/create-conn-with-blocks + {:classes {:custom-tag {}} + :properties {:custom-property {:logseq.property/type :default}} + :pages-and-blocks [{:page {:block/title "Visible Page" + :block/created-at 1000 + :block/updated-at 2000}} + {:page {:block/title "Hidden Page" + :block/created-at 1500 + :block/updated-at 2500 + :build/properties {:logseq.property/hide? true}}} + {:page {:build/journal 20260201 + :block/created-at 3000 + :block/updated-at 4000}} + {:page {:block/title "Late Page" + :block/created-at 5000 + :block/updated-at 6000}}]})] + @conn)) + +(defn- list-item-ids + [items] + (->> items (map :db/id) set)) + +(defn- first-user-tag-entity + [db] + (->> (d/datoms db :avet :block/tags :logseq.class/Tag) + (map :e) + (map #(d/entity db %)) + (remove :logseq.property/built-in?) + first)) + +(defn- first-user-property-entity + [db] + (->> (d/datoms db :avet :block/tags :logseq.class/Property) + (map :e) + (map #(d/entity db %)) + (remove :logseq.property/built-in?) + first)) + +(deftest test-list-non-expanded-contract + (let [db (create-test-db) + required-keys #{:db/id :block/title :block/created-at :block/updated-at} + visible-page (some #(when (= "Visible Page" (:block/title %)) %) + (cli-common-mcp-tools/list-pages db {})) + custom-tag-entity (first-user-tag-entity db) + custom-tag-title (:block/title custom-tag-entity) + custom-tag (some #(when (= custom-tag-title (:block/title %)) %) + (cli-common-mcp-tools/list-tags db {})) + custom-property-entity (first-user-property-entity db) + custom-property-title (:block/title custom-property-entity) + custom-property (some #(when (= custom-property-title (:block/title %)) %) + (cli-common-mcp-tools/list-properties db {}))] + (testing "list-pages non-expanded includes stable id and timestamps" + (is (some? visible-page)) + (is (set/subset? required-keys (set (keys visible-page))))) + + (testing "list-tags non-expanded includes stable id and timestamps" + (is (some? custom-tag)) + (is (set/subset? required-keys (set (keys custom-tag))))) + + (testing "list-properties non-expanded includes stable id and timestamps" + (is (some? custom-property)) + (is (set/subset? required-keys (set (keys custom-property))))))) + +(deftest test-list-tags-and-properties-include-built-in-default + (let [db (create-test-db) + built-in-tag-ids (->> (d/datoms db :avet :block/tags :logseq.class/Tag) + (map :e) + (map #(d/entity db %)) + (filter :logseq.property/built-in?) + (map :db/id) + set) + built-in-property-ids (->> (d/datoms db :avet :block/tags :logseq.class/Property) + (map :e) + (map #(d/entity db %)) + (filter :logseq.property/built-in?) + (map :db/id) + set) + custom-tag-id (:db/id (first-user-tag-entity db)) + custom-property-id (:db/id (first-user-property-entity db)) + default-tag-ids (list-item-ids (cli-common-mcp-tools/list-tags db {})) + default-property-ids (list-item-ids (cli-common-mcp-tools/list-properties db {})) + no-built-in-tag-ids (list-item-ids (cli-common-mcp-tools/list-tags db {:include-built-in false})) + no-built-in-property-ids (list-item-ids (cli-common-mcp-tools/list-properties db {:include-built-in false}))] + (testing "built-ins are included by default" + (is (seq built-in-tag-ids)) + (is (seq built-in-property-ids)) + (is (seq (set/intersection default-tag-ids built-in-tag-ids))) + (is (seq (set/intersection default-property-ids built-in-property-ids)))) + + (testing "include-built-in=false excludes built-ins but keeps user entities" + (is (contains? no-built-in-tag-ids custom-tag-id)) + (is (contains? no-built-in-property-ids custom-property-id)) + (is (empty? (set/intersection no-built-in-tag-ids built-in-tag-ids))) + (is (empty? (set/intersection no-built-in-property-ids built-in-property-ids)))))) + +(deftest test-list-pages-filter-contract + (let [db (create-test-db) + hidden-id (->> (d/q '[:find [?e ...] + :in $ ?title + :where + [?e :block/title ?title] + [?e :logseq.property/hide? true]] + db "Hidden Page") + first) + journal-id (->> (d/datoms db :avet :block/journal-day 20260201) + first + :e) + visible-id (->> (d/q '[:find [?e ...] + :in $ ?title + :where [?e :block/title ?title]] + db "Visible Page") + first) + late-id (->> (d/q '[:find [?e ...] + :in $ ?title + :where [?e :block/title ?title]] + db "Late Page") + first) + default-ids (list-item-ids (cli-common-mcp-tools/list-pages db {})) + include-hidden-ids (list-item-ids (cli-common-mcp-tools/list-pages db {:include-hidden true})) + exclude-journal-ids (list-item-ids (cli-common-mcp-tools/list-pages db {:include-journal false})) + journal-only-ids (list-item-ids (cli-common-mcp-tools/list-pages db {:journal-only true})) + created-after-ids (list-item-ids (cli-common-mcp-tools/list-pages db {:created-after 2500})) + updated-after-ids (list-item-ids (cli-common-mcp-tools/list-pages db {:updated-after "1970-01-01T00:00:03.500Z"})) + invalid-created-after-ids (list-item-ids (cli-common-mcp-tools/list-pages db {:created-after "not-a-date"})) + invalid-updated-after-ids (list-item-ids (cli-common-mcp-tools/list-pages db {:updated-after "still-not-a-date"}))] + (testing "journals are included by default and hidden pages are excluded by default" + (is (contains? default-ids journal-id)) + (is (not (contains? default-ids hidden-id)))) + + (testing "include-hidden includes hidden pages" + (is (contains? include-hidden-ids hidden-id))) + + (testing "include-journal and journal-only page filters" + (is (not (contains? exclude-journal-ids journal-id))) + (is (= #{journal-id} journal-only-ids))) + + (testing "created-after and updated-after filters" + (is (contains? created-after-ids journal-id)) + (is (contains? created-after-ids late-id)) + (is (not (contains? created-after-ids visible-id))) + (is (contains? updated-after-ids journal-id)) + (is (contains? updated-after-ids late-id)) + (is (not (contains? updated-after-ids visible-id)))) + + (testing "invalid date filters behave as no-op" + (is (= default-ids invalid-created-after-ids)) + (is (= default-ids invalid-updated-after-ids)))))