diff --git a/docs/agent-guide/045-logseq-cli-property-type-and-upsert-option-unification.md b/docs/agent-guide/045-logseq-cli-property-type-and-upsert-option-unification.md new file mode 100644 index 0000000000..c15b2def60 --- /dev/null +++ b/docs/agent-guide/045-logseq-cli-property-type-and-upsert-option-unification.md @@ -0,0 +1,180 @@ +# Logseq CLI Property Type and Upsert Option Unification Implementation Plan + +Goal: Add a property type column to `list property`, add `--id` update-mode semantics to `upsert block/page/tag/property`, and remove duplicated `--tags` or `--properties` options from `upsert block/page` in favor of `--update-tags` or `--update-properties`. + +Architecture: Keep the existing `logseq-cli -> transport/invoke -> db-worker-node :thread-api/*` contract unchanged and implement behavior changes in CLI parsing, action building, execution, and formatting. +Architecture: Extend property list payload shaping so non-expanded property items include `:logseq.property/type`, then render a dedicated property table in human output with a `TYPE` column. +Architecture: Treat `--id` as an explicit update signal for all upsert entity commands, and keep create paths only when `--id` is absent. + +Tech Stack: ClojureScript, babashka.cli, Promesa, Datascript, logseq-cli command modules, db-worker-node thread APIs. + +Related: Builds on `docs/agent-guide/044-logseq-cli-upsert-block-page.md` and relates to `docs/agent-guide/043-logseq-cli-tag-property-management.md`. + +## Problem statement + +Current `list property` human output in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/format.cljs` uses the same formatter as `list tag`, so no property-type column is rendered. + +Current non-expanded property list items from `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/common/mcp/tools.cljs` are built by `minimal-list-item`, which does not include `:logseq.property/type`. + +Current `upsert block` already supports `--id` and treats it as update mode via `update-mode?` in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs`, but `upsert page`, `upsert tag`, and `upsert property` do not accept `--id`. + +Current `upsert block/page` specs include both `--tags` or `--properties` and `--update-tags` or `--update-properties` in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs`, which duplicates semantics and increases parser and action complexity. + +Current parser validation in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/commands.cljs` requires `--page` for `upsert page` and requires `--name` for `upsert property`, so there is no update-by-id mode for those commands. + +## Testing Plan + +I will use `@test-driven-development` for all implementation batches. + +I will add parser and action RED tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/commands_test.cljs` for new `--id` contracts on `upsert page`, `upsert tag`, and `upsert property`. + +I will add formatter RED tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/format_test.cljs` for the `list property` `TYPE` column and its value normalization. + +I will add contract RED tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/mcp_tools_contract_test.cljs` to ensure non-expanded property list items carry `:logseq.property/type`. + +I will add integration RED tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/integration_test.cljs` for update-by-id flows and for rejecting removed `--tags` or `--properties` flags on `upsert block/page`. + +I will use `@clojure-debug` only when failures indicate fixture or async harness issues rather than missing behavior. + +NOTE: I will write *all* tests before I add any implementation behavior. + +## Current implementation baseline + +| Requirement | Current behavior | Gap | +| --- | --- | --- | +| `list property` shows type column. | `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/format.cljs` renders tag and property with the same columns (`ID`, `TITLE`, optional `IDENT`, timestamps). | No `TYPE` column in human output, and non-expanded payload currently omits type. | +| `upsert block/page/tag/property` supports `--id` and `--id` forces update mode. | `upsert block` supports this already, but `upsert page/tag/property` specs do not expose `--id` and still depend on page/name creation-first contracts. | Missing update-by-id mode for three upsert commands. | +| `upsert block/page` removes `--tags` or `--properties` and uses `--update-tags` or `--update-properties` only. | Both old and new options are accepted and merged in action building and execution. | Duplicate option surface and duplicate parsing paths remain. | + +## Target contract + +`upsert block` keeps current `--id` update-mode behavior and remove legacy create-only `--tags` or `--properties` options. + +`upsert page` accepts `--id` as update mode, and accepts `--page` only for create mode. + +`upsert tag` accepts `--id` as update mode, and keeps `--name` for create mode. + +`upsert property` accepts `--id` as update mode, and keeps `--name` for create mode. + +`upsert tag --id ` with no additional mutation options is a successful no-op after id lookup and tag-class validation. + +`upsert page --id --page ` is invalid and must fail as conflicting selectors. + +When `--id` is provided for any upsert command, create-specific resolution paths must be skipped and the command must fail if the target id does not exist or has the wrong entity class. + +`upsert block/page` should reject `--tags` and `--properties` as unknown options after spec cleanup, with guidance to use `--update-tags` and `--update-properties`. + +Update-by-id failures should use new id-mode specific error codes so scripts can distinguish id lookup and id class mismatch from create-mode validation failures. + +## Architecture sketch + +```text +list property + -> /src/main/logseq/cli/command/list.cljs execute-list-property + -> /src/main/frontend/worker/db_core.cljs :thread-api/api-list-properties + -> /src/main/logseq/cli/common/mcp/tools.cljs list-properties + -> /src/main/logseq/cli/format.cljs format-list-property (new dedicated formatter) +``` + +```text +upsert page/tag/property --id + -> /src/main/logseq/cli/commands.cljs parse + finalize-command + -> /src/main/logseq/cli/command/upsert.cljs update-mode detection and action build + -> transport/invoke :thread-api/pull for id/entity validation + -> transport/invoke :thread-api/apply-outliner-ops for update ops only +``` + +## Detailed implementation plan + +1. Add RED parser tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/commands_test.cljs` asserting `upsert page --id 10 --update-properties ...` parses as `:upsert-page` and no longer requires `--page`. +2. Add RED parser tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/commands_test.cljs` asserting `upsert tag --id 10` parses and `upsert property --id 10 --type node` parses. +3. Add RED parser tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/commands_test.cljs` asserting `upsert block --tags ...` and `upsert page --properties ...` fail with `:invalid-options` due to removed flags. +4. Add RED parser tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/commands_test.cljs` asserting `upsert page --id --page ` fails with a selector conflict error. +5. Add RED build-action tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/commands_test.cljs` for `upsert page/tag/property` actions that include `:mode :update` when `--id` is present. +6. Add RED execute tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/commands_test.cljs` verifying `upsert tag --id ` with no update fields returns `:ok` and no mutation ops. +7. Add RED execute tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/commands_test.cljs` verifying update-by-id rejects missing ids and wrong entity classes with new id-mode specific error codes. +8. Add RED formatter tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/format_test.cljs` asserting `list property` human output includes `TYPE` header and per-row values. +9. Add RED contract tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/mcp_tools_contract_test.cljs` asserting non-expanded `list-properties` items include `:logseq.property/type`. +10. Add RED integration tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/integration_test.cljs` for `upsert page --id`, `upsert tag --id`, and `upsert property --id` update mode behavior, including tag no-op behavior. +11. Add RED integration tests in `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/integration_test.cljs` verifying `upsert page --id --page` fails with selector conflict and `upsert block/page` reject removed `--tags` and `--properties` options. +12. Run focused RED commands and verify failures are behavior assertions, not fixture failures. +13. Update specs in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs` to remove `:tags` and `:properties` from block/page specs and add `:id` to page/tag/property specs. +14. Update option validation in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/commands.cljs` so `upsert page` and `upsert property` required-field checks are mode-aware instead of unconditional, and add explicit selector-conflict validation for `upsert page --id --page`. +15. Refactor `build-page-action` in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs` to support create mode by `--page` and update mode by `--id`. +16. Refactor `build-tag-action` and `build-property-action` in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs` to support update mode by `--id` with mode-specific required options. +17. Add shared helper(s) in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs` to pull entities by id and validate class/type constraints before updates. +18. Update `execute-upsert-page`, `execute-upsert-tag`, and `execute-upsert-property` in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs` so update mode uses id lookup, skips creation paths, and applies only update semantics, with `upsert tag --id` no-op when no mutation fields are provided. +19. Introduce explicit id-mode error codes in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs` and `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/commands.cljs` for id-not-found and id-type-mismatch failures. +20. Remove all `:tags` and `:properties` action wiring from page/block upsert flows in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs`, and keep only `:update-tags` and `:update-properties`. +21. Update `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/common/mcp/tools.cljs` to include `:logseq.property/type` in non-expanded property list items. +22. Update `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/format.cljs` to split property formatting from tag formatting and render a dedicated `TYPE` column for `:list-property`. +23. Update docs in `/Users/rcmerci/gh-repos/logseq/docs/cli/logseq-cli.md` to remove `--tags` or `--properties` from `upsert block/page` docs, document update-by-id behavior across all upsert commands, and document selector conflict behavior. +24. Run focused GREEN tests for commands, format, and integration, then run `bb dev:lint-and-test`. +25. Refactor only after GREEN to reduce duplication in upsert mode branching, then rerun focused tests and full suite. + +## Edge cases + +`upsert page --id ` must fail when id points to a block that is not a page entity. + +`upsert tag --id ` must fail when id points to a page not tagged with `:logseq.class/Tag`. + +`upsert property --id ` must fail when id points to an entity without `:logseq.property/type`. + +`upsert tag --id ` with no mutation options must return success without issuing mutation ops. + +`upsert page --id --page ` must fail with a dedicated selector conflict error. + +Update-by-id missing target and class mismatch failures must return new id-mode specific error codes. + +`upsert block` create mode with `--blocks` or `--blocks-file` must preserve existing validation behavior after removing `--tags` and `--properties` options. + +Property type display should remain stable for built-in and custom properties, and missing type values should render as `-` instead of throwing. + +JSON and EDN list outputs should remain backward compatible except for the additive `type` field on property items. + +## Verification commands and expected output + +| Command | Expected output | +| --- | --- | +| `bb dev:test -v logseq.cli.commands-test` | Parser, action, and execute tests for mode switching and option removal pass. | +| `bb dev:test -v logseq.cli.format-test` | Human formatter tests pass with `TYPE` column coverage for `list property`. | +| `bb dev:test -v logseq.cli.mcp-tools-contract-test` | Contract tests pass with `:logseq.property/type` present in non-expanded property items. | +| `bb dev:test -v logseq.cli.integration-test/test-cli-upsert-page-create-and-update-existing` | Existing page upsert flow still passes after mode refactor. | +| `bb dev:test -v logseq.cli.integration-test` | New `--id` update-mode and removed-option behavior pass end to end. | +| `bb dev:lint-and-test` | Full suite passes with exit code `0`. | + +## Testing Details + +Tests cover CLI behavior at parser, action, executor, formatter, and end-to-end levels, and they assert entity outcomes instead of internal helper wiring. + +Tests verify that update-by-id mode never creates entities and that legacy duplicated options are no longer accepted for block/page upsert. + +Tests verify that `list property` human and structured output both include property-type information in their respective contracts. + +## Implementation Details + +- Keep db-worker-node thread API names unchanged and avoid adding new transport methods. +- Add `--id` to `upsert page/tag/property` specs in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs`. +- Remove `--tags` and `--properties` from `upsert block/page` specs in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs`. +- Make finalize validation in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/commands.cljs` mode-aware for page/property required options. +- Rework `build-page-action`, `build-tag-action`, and `build-property-action` to branch on create vs update mode. +- Add id-based entity validation helpers in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs`. +- Make `upsert tag --id` with no mutation fields a successful no-op after id and class validation. +- Reject `upsert page --id --page` as explicit selector conflict. +- Add new id-mode specific error codes for id-not-found and id-type-mismatch paths. +- Include `:logseq.property/type` in non-expanded property list payload from `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/common/mcp/tools.cljs`. +- Split property-specific table rendering in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/format.cljs` to add `TYPE` column. +- Update `/Users/rcmerci/gh-repos/logseq/docs/cli/logseq-cli.md` command reference and examples. +- Keep implementation and debugging workflow aligned with `@test-driven-development` and `@clojure-debug`. + +## Question + +No open questions. + +Decided: `upsert tag --id ` with no additional mutation options is a successful no-op. + +Decided: `upsert page --id --page ` is rejected as conflicting selectors. + +Decided: update-by-id failures use new id-mode specific error codes. + +--- diff --git a/docs/cli/logseq-cli.md b/docs/cli/logseq-cli.md index 17dd89fa96..2f0bfdfd55 100644 --- a/docs/cli/logseq-cli.md +++ b/docs/cli/logseq-cli.md @@ -93,12 +93,17 @@ Server ownership behavior: Inspect and edit commands: - `list page [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list pages - `list tag [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list tags -- `list property [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list properties +- `list property [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list properties (`TYPE` is included by default even without `--expand`) - `upsert block --content [--target-page |--target-id |--target-uuid ] [--pos first-child|last-child|sibling]` - create blocks; defaults to today’s journal page if no target is given - `upsert block --blocks [--target-page |--target-id |--target-uuid ] [--pos first-child|last-child|sibling]` - insert blocks via EDN vector - `upsert block --blocks-file [--target-page |--target-id |--target-uuid ] [--pos first-child|last-child|sibling]` - insert blocks from an EDN file - `upsert block --id |--uuid [--target-id |--target-uuid |--target-page ] [--pos first-child|last-child|sibling] [--update-tags ] [--update-properties ] [--remove-tags ] [--remove-properties ]` - update and/or move a block -- `upsert page --page [--tags ] [--properties ] [--update-tags ] [--update-properties ] [--remove-tags ] [--remove-properties ]` - create or update a page +- `upsert page --page [--update-tags ] [--update-properties ] [--remove-tags ] [--remove-properties ]` - create (or update by page name) a page +- `upsert page --id [--update-tags ] [--update-properties ] [--remove-tags ] [--remove-properties ]` - update a page by id (cannot be combined with `--page`) +- `upsert tag --name ` - create or upsert a tag by name +- `upsert tag --id ` - validate and upsert a tag by id (no-op when no other mutation options are provided) +- `upsert property --name [--type ] [--cardinality one|many] [--hide true|false] [--public true|false]` - create or update a property by name +- `upsert property --id [--type ] [--cardinality one|many] [--hide true|false] [--public true|false]` - update a property by id - `move --id |--uuid --target-id |--target-uuid |--target-page [--pos first-child|last-child|sibling]` - move a block and its children (defaults to first-child) - `remove --id |--uuid |--page ` - remove blocks (by db/id or UUID) or pages - `search [--type page|block|tag|property|all] [--tag ] [--case-sensitive] [--sort updated-at|created-at] [--order asc|desc]` - search across pages, blocks, tags, and properties (query is positional) @@ -140,7 +145,8 @@ Revision: Output formats: - Global `--output ` applies to all commands - For `graph export`, `--output` refers to the destination file path. Output formatting is controlled via `:output-format` in config or `LOGSEQ_CLI_OUTPUT`. -- Human output is plain text. List/search commands render tables with a final `Count: N` line. For list and search subcommands, the ID column uses `:db/id` (not UUID). If `:db/ident` exists, an `IDENT` column is included. Search table columns are `ID` and `TITLE`. Block titles can include multiple lines; multi-line rows align additional lines under the `TITLE` column. Times such as list `UPDATED-AT`/`CREATED-AT` and `graph info` `Created at` are shown in human-friendly relative form. Errors include error codes and may include a `Hint:` line. Use `--output json|edn` for structured output. +- Human output is plain text. List/search commands render tables with a final `Count: N` line. For list and search subcommands, the ID column uses `:db/id` (not UUID). If `:db/ident` exists, an `IDENT` column is included. `list property` includes a dedicated `TYPE` column. Search table columns are `ID` and `TITLE`. Block titles can include multiple lines; multi-line rows align additional lines under the `TITLE` column. Times such as list `UPDATED-AT`/`CREATED-AT` and `graph info` `Created at` are shown in human-friendly relative form. Errors include error codes and may include a `Hint:` line. Use `--output json|edn` for structured output. +- For `list property`, `TYPE` is returned in default output (without `--expand`) for human and structured (`json`/`edn`) formats. - `upsert page` and `upsert block` return entity ids in `data.result` for JSON/EDN output, and include ids in human output. - Human example: ```text diff --git a/src/main/logseq/cli/command/upsert.cljs b/src/main/logseq/cli/command/upsert.cljs index 211a56dcf3..357d1026c5 100644 --- a/src/main/logseq/cli/command/upsert.cljs +++ b/src/main/logseq/cli/command/upsert.cljs @@ -22,27 +22,29 @@ :blocks {:desc "EDN vector of blocks for create mode"} :blocks-file {:desc "EDN file of blocks for create mode"} :status {:desc "Task status (todo, doing, done, etc.)"} - :tags {:desc "Tags to add in create mode (EDN vector). Identifiers can be id, :db/ident, or :block/title."} - :properties {:desc "Properties to add in create mode (EDN map). Identifiers can be id, :db/ident, or :block/title."} :update-tags {:desc "Tags to add/update (EDN vector)"} :update-properties {:desc "Properties to add/update (EDN map)"} :remove-tags {:desc "Tags to remove (EDN vector)"} :remove-properties {:desc "Properties to remove (EDN vector)"}}) (def ^:private upsert-page-spec - {:page {:desc "Page name"} - :tags {:desc "Tags to add (EDN vector). Identifiers can be id, :db/ident, or :block/title."} - :properties {:desc "Properties to add (EDN map). Identifiers can be id, :db/ident, or :block/title."} + {:id {:desc "Target page db/id (forces update mode)" + :coerce :long} + :page {:desc "Page name"} :update-tags {:desc "Tags to add/update (EDN vector)"} :update-properties {:desc "Properties to add/update (EDN map)"} :remove-tags {:desc "Tags to remove (EDN vector)"} :remove-properties {:desc "Properties to remove (EDN vector)"}}) (def ^:private upsert-tag-spec - {:name {:desc "Tag name"}}) + {:id {:desc "Target tag db/id (forces update mode)" + :coerce :long} + :name {:desc "Tag name"}}) (def ^:private upsert-property-spec - {:name {:desc "Property name"} + {:id {:desc "Target property db/id (forces update mode)" + :coerce :long} + :name {:desc "Property name"} :type {:desc "Property type (default, number, date, datetime, checkbox, url, node, json, string)"} :cardinality {:desc "Property cardinality (one, many)"} :hide {:desc "Hide property" @@ -101,8 +103,13 @@ :upsert-property (let [type' (normalize-property-type (:type opts)) - cardinality' (normalize-property-cardinality (:cardinality opts))] + cardinality' (normalize-property-cardinality (:cardinality opts)) + name (normalize-property-name (:name opts)) + selectors (filter some? [(:id opts) name])] (cond + (> (count selectors) 1) + "only one of --id or --name is allowed" + (and (seq (:type opts)) (not (contains? property-types type'))) (str "invalid type: " (:type opts)) @@ -112,6 +119,18 @@ :else nil)) + :upsert-page + (let [page (some-> (:page opts) string/trim) + selectors (filter some? [(:id opts) page])] + (when (> (count selectors) 1) + "only one of --id or --page is allowed")) + + :upsert-tag + (let [name (normalize-tag-name (:name opts)) + selectors (filter some? [(:id opts) name])] + (when (> (count selectors) 1) + "only one of --id or --name is allowed")) + nil)) (defn update-mode? @@ -179,25 +198,24 @@ {:ok? false :error {:code :missing-repo :message "repo is required for upsert"}} - (let [page (some-> (:page options) string/trim) - tags-result (add-command/parse-tags-option (:tags options)) - properties-result (add-command/parse-properties-option (:properties options)) + (let [id (:id options) + page (some-> (:page options) string/trim) update-tags-result (add-command/parse-tags-option (:update-tags options)) update-properties-result (add-command/parse-properties-option (:update-properties options)) remove-tags-result (add-command/parse-tags-vector-option (:remove-tags options)) - remove-properties-result (add-command/parse-properties-vector-option (:remove-properties options))] + remove-properties-result (add-command/parse-properties-vector-option (:remove-properties options)) + invalid-message (invalid-options? :upsert-page options)] (cond - (not (seq page)) + (seq invalid-message) + {:ok? false + :error {:code :invalid-options + :message invalid-message}} + + (and (not (some? id)) (not (seq page))) {:ok? false :error {:code :missing-page-name :message "page name is required"}} - (not (:ok? tags-result)) - tags-result - - (not (:ok? properties-result)) - properties-result - (not (:ok? update-tags-result)) update-tags-result @@ -212,16 +230,16 @@ :else {:ok? true - :action {:type :upsert-page - :repo repo - :graph (core/repo->graph repo) - :page page - :tags (:value tags-result) - :properties (:value properties-result) - :update-tags (:value update-tags-result) - :update-properties (:value update-properties-result) - :remove-tags (:value remove-tags-result) - :remove-properties (:value remove-properties-result)}})))) + :action (cond-> {:type :upsert-page + :repo repo + :graph (core/repo->graph repo) + :mode (if (some? id) :update :create) + :update-tags (:value update-tags-result) + :update-properties (:value update-properties-result) + :remove-tags (:value remove-tags-result) + :remove-properties (:value remove-properties-result)} + (some? id) (assoc :id id) + (seq page) (assoc :page page))})))) (defn build-tag-action [options repo] @@ -229,13 +247,32 @@ {:ok? false :error {:code :missing-repo :message "repo is required for upsert"}} - (let [name (normalize-tag-name (:name options))] - (if (seq name) + (let [id (:id options) + name (normalize-tag-name (:name options)) + invalid-message (invalid-options? :upsert-tag options)] + (cond + (seq invalid-message) + {:ok? false + :error {:code :invalid-options + :message invalid-message}} + + (some? id) {:ok? true :action {:type :upsert-tag + :mode :update + :id id + :repo repo + :graph (core/repo->graph repo)}} + + (seq name) + {:ok? true + :action {:type :upsert-tag + :mode :create :repo repo :graph (core/repo->graph repo) :name name}} + + :else {:ok? false :error {:code :missing-tag-name :message "tag name is required"}})))) @@ -269,10 +306,11 @@ {:ok? false :error {:code :missing-repo :message "repo is required for upsert"}} - (let [name (normalize-property-name (:name options)) + (let [id (:id options) + name (normalize-property-name (:name options)) invalid-message (invalid-options? :upsert-property options)] (cond - (not (seq name)) + (and (not (some? id)) (not (seq name))) {:ok? false :error {:code :missing-property-name :message "property name is required"}} @@ -284,11 +322,13 @@ :else {:ok? true - :action {:type :upsert-property - :repo repo - :graph (core/repo->graph repo) - :name name - :schema (property-schema options)}})))) + :action (cond-> {:type :upsert-property + :mode (if (some? id) :update :create) + :repo repo + :graph (core/repo->graph repo) + :schema (property-schema options)} + (some? id) (assoc :id id) + (seq name) (assoc :name name))})))) (defn- pull-page-by-name [config repo page-name selector] @@ -323,6 +363,93 @@ {:code :page-not-found :page page-name}))))))) +(def ^:private upsert-id-not-found-code + :upsert-id-not-found) + +(def ^:private upsert-id-type-mismatch-code + :upsert-id-type-mismatch) + +(def ^:private page-selector + [:db/id :block/uuid :block/name :block/title]) + +(def ^:private tag-selector + [:db/id :block/uuid :block/name :block/title + {:block/tags [:db/ident]}]) + +(def ^:private property-selector + [:db/id :db/ident :block/uuid :block/name :block/title :logseq.property/type]) + +(defn- page-entity? + [entity] + (seq (:block/name entity))) + +(defn- tag-entity? + [entity] + (some #(= :logseq.class/Tag (:db/ident %)) + (:block/tags entity))) + +(defn- property-entity? + [entity] + (some? (:logseq.property/type entity))) + +(defn- pull-entity-by-id + [config repo selector id] + (transport/invoke config :thread-api/pull false + [repo selector id])) + +(defn- throw-upsert-id-not-found! + [entity-type id] + (throw (ex-info (str entity-type " not found for id") + {:code upsert-id-not-found-code + :entity-type entity-type + :id id}))) + +(defn- throw-upsert-id-type-mismatch! + [entity-type id] + (throw (ex-info (str "id does not reference expected " entity-type) + {:code upsert-id-type-mismatch-code + :entity-type entity-type + :id id}))) + +(defn- ensure-page-by-id! + [config repo id] + (p/let [entity (pull-entity-by-id config repo page-selector id)] + (cond + (not (:db/id entity)) + (throw-upsert-id-not-found! "page" id) + + (not (page-entity? entity)) + (throw-upsert-id-type-mismatch! "page" id) + + :else + entity))) + +(defn- ensure-tag-by-id! + [config repo id] + (p/let [entity (pull-entity-by-id config repo tag-selector id)] + (cond + (not (:db/id entity)) + (throw-upsert-id-not-found! "tag" id) + + (not (tag-entity? entity)) + (throw-upsert-id-type-mismatch! "tag" id) + + :else + entity))) + +(defn- ensure-property-by-id! + [config repo id] + (p/let [entity (pull-entity-by-id config repo property-selector id)] + (cond + (not (:db/id entity)) + (throw-upsert-id-not-found! "property" id) + + (not (property-entity? entity)) + (throw-upsert-id-type-mismatch! "property" id) + + :else + entity))) + (defn- append-tag-and-property-ops [ops block-ids {:keys [update-tag-ids remove-tag-ids update-properties remove-properties]}] (cond-> ops @@ -389,20 +516,20 @@ (defn execute-upsert-page [action config] (-> (p/let [cfg (cli-server/ensure-server! config (:repo action)) - page (ensure-page-entity! cfg (:repo action) (:page action)) + update-by-id? (= :update (:mode action)) + page (if update-by-id? + (ensure-page-by-id! cfg (:repo action) (:id action)) + (ensure-page-entity! cfg (:repo action) (:page action))) page-id (:db/id page) block-ids [page-id] - add-tags (add-command/resolve-tags cfg (:repo action) (:tags action)) update-tags (add-command/resolve-tags cfg (:repo action) (:update-tags action)) remove-tags (add-command/resolve-tags cfg (:repo action) (:remove-tags action)) - add-properties (add-command/resolve-properties cfg (:repo action) (:properties action)) update-properties (add-command/resolve-properties cfg (:repo action) (:update-properties action)) remove-properties (add-command/resolve-property-identifiers cfg (:repo action) (:remove-properties action)) - merged-properties (merge (or add-properties {}) (or update-properties {})) - _ (ensure-property-identifiers-exist! cfg (:repo action) (keys merged-properties)) + _ (ensure-property-identifiers-exist! cfg (:repo action) (keys (or update-properties {}))) _ (ensure-property-identifiers-exist! cfg (:repo action) remove-properties) - update-tag-ids (->> (concat (or add-tags []) (or update-tags [])) + update-tag-ids (->> (or update-tags []) (map :db/id) (remove nil?) distinct @@ -412,7 +539,7 @@ block-ids {:update-tag-ids update-tag-ids :remove-tag-ids remove-tag-ids - :update-properties merged-properties + :update-properties update-properties :remove-properties remove-properties}) _ (when (seq ops) (transport/invoke cfg :thread-api/apply-outliner-ops false @@ -424,85 +551,99 @@ :error {:code (or (get-in (ex-data e) [:code]) :exception) :message (or (ex-message e) (str e))}})))) -(defn- tag-entity? - [entity] - (some #(= :logseq.class/Tag (:db/ident %)) - (:block/tags entity))) - (defn execute-upsert-tag [action config] (-> (p/let [cfg (cli-server/ensure-server! config (:repo action)) - existing (pull-page-by-name cfg (:repo action) (:name action) - [:db/id :block/name :block/title - {:block/tags [:db/ident]}]) - existing-id (:db/id existing)] - (cond - (and existing-id (not (tag-entity? existing))) - {:status :error - :error {:code :tag-name-conflict - :message "tag already exists as a page and is not a tag"}} - - :else - (p/let [_ (when-not existing-id - (transport/invoke cfg :thread-api/apply-outliner-ops false - [(:repo action) - [[:create-page [(:name action) {:class? true}]]] - {}])) - page (or (when existing-id existing) - (pull-page-by-name cfg (:repo action) (:name action) + update-by-id? (= :update (:mode action))] + (if update-by-id? + (p/let [entity (ensure-tag-by-id! cfg (:repo action) (:id action))] + {:status :ok + :data {:result [(:db/id entity)]}}) + (p/let [existing (pull-page-by-name cfg (:repo action) (:name action) [:db/id :block/name :block/title - {:block/tags [:db/ident]}])) - page-id (:db/id page)] + {:block/tags [:db/ident]}]) + existing-id (:db/id existing)] (cond - (not page-id) + (and existing-id (not (tag-entity? existing))) {:status :error - :error {:code :tag-not-found - :message "tag not found after upsert"}} - - (not (tag-entity? page)) - {:status :error - :error {:code :tag-create-not-tag - :message "created entity is not tagged as :logseq.class/Tag"}} + :error {:code :tag-name-conflict + :message "tag already exists as a page and is not a tag"}} :else - {:status :ok - :data {:result [page-id]}})))))) + (p/let [_ (when-not existing-id + (transport/invoke cfg :thread-api/apply-outliner-ops false + [(:repo action) + [[:create-page [(:name action) {:class? true}]]] + {}])) + page (or (when existing-id existing) + (pull-page-by-name cfg (:repo action) (:name action) + [:db/id :block/name :block/title + {:block/tags [:db/ident]}])) + page-id (:db/id page)] + (cond + (not page-id) + {:status :error + :error {:code :tag-not-found + :message "tag not found after upsert"}} -(def ^:private property-selector - [:db/id :db/ident :block/name :block/title :logseq.property/type]) + (not (tag-entity? page)) + {:status :error + :error {:code :tag-create-not-tag + :message "created entity is not tagged as :logseq.class/Tag"}} -(defn- property-entity? - [entity] - (some? (:logseq.property/type entity))) + :else + {:status :ok + :data {:result [page-id]}})))))) + (p/catch (fn [e] + {:status :error + :error {:code (or (get-in (ex-data e) [:code]) :exception) + :message (or (ex-message e) (str e))}})))) (defn execute-upsert-property [action config] (-> (p/let [cfg (cli-server/ensure-server! config (:repo action)) - existing (pull-page-by-name cfg (:repo action) (:name action) property-selector) - existing-id (:db/id existing)] - (cond - (and existing-id (not (property-entity? existing))) - {:status :error - :error {:code :property-name-conflict - :message "property already exists as a page and is not a property"}} - - :else - (p/let [property-ident (when (property-entity? existing) - (:db/ident existing)) - property-opts (cond-> {} - (nil? property-ident) - (assoc :property-name (:name action))) - _ (transport/invoke cfg :thread-api/apply-outliner-ops false - [(:repo action) - [[:upsert-property [property-ident - (:schema action) - property-opts]]] - {}]) - property (pull-page-by-name cfg (:repo action) (:name action) property-selector) - property-id (:db/id property)] - (if property-id - {:status :ok - :data {:result [property-id]}} + update-by-id? (= :update (:mode action))] + (if update-by-id? + (p/let [existing (ensure-property-by-id! cfg (:repo action) (:id action)) + property-ident (:db/ident existing) + _ (when (seq (:schema action)) + (transport/invoke cfg :thread-api/apply-outliner-ops false + [(:repo action) + [[:upsert-property [property-ident + (:schema action) + {}]]] + {}]))] + {:status :ok + :data {:result [(:db/id existing)]}}) + (p/let [existing (pull-page-by-name cfg (:repo action) (:name action) property-selector) + existing-id (:db/id existing)] + (cond + (and existing-id (not (property-entity? existing))) {:status :error - :error {:code :property-not-found - :message "property not found after upsert"}})))))) + :error {:code :property-name-conflict + :message "property already exists as a page and is not a property"}} + + :else + (p/let [property-ident (when (property-entity? existing) + (:db/ident existing)) + property-opts (cond-> {} + (nil? property-ident) + (assoc :property-name (:name action))) + _ (transport/invoke cfg :thread-api/apply-outliner-ops false + [(:repo action) + [[:upsert-property [property-ident + (:schema action) + property-opts]]] + {}]) + property (pull-page-by-name cfg (:repo action) (:name action) property-selector) + property-id (:db/id property)] + (if property-id + {:status :ok + :data {:result [property-id]}} + {:status :error + :error {:code :property-not-found + :message "property not found after upsert"}})))))) + (p/catch (fn [e] + {:status :error + :error {:code (or (get-in (ex-data e) [:code]) :exception) + :message (or (ex-message e) (str e))}})))) diff --git a/src/main/logseq/cli/commands.cljs b/src/main/logseq/cli/commands.cljs index d7f40c2615..560f89a94e 100644 --- a/src/main/logseq/cli/commands.cljs +++ b/src/main/logseq/cli/commands.cljs @@ -139,6 +139,29 @@ (string/join " " (cond-> (vec dispatch) wrong-input (conj wrong-input)))) +(defn- legacy-upsert-option-guidance + [args message] + (let [subcommand (vec (take 2 args))] + (cond + (and (= ["upsert" "block"] subcommand) + (re-find #"Unknown option:\s*:tags" (or message ""))) + "unknown option: --tags; use --update-tags" + + (and (= ["upsert" "block"] subcommand) + (re-find #"Unknown option:\s*:properties" (or message ""))) + "unknown option: --properties; use --update-properties" + + (and (= ["upsert" "page"] subcommand) + (re-find #"Unknown option:\s*:tags" (or message ""))) + "unknown option: --tags; use --update-tags" + + (and (= ["upsert" "page"] subcommand) + (re-find #"Unknown option:\s*:properties" (or message ""))) + "unknown option: --properties; use --update-properties" + + :else + nil))) + (defn- ^:large-vars/cleanup-todo finalize-command [summary {:keys [command opts args cmds spec]}] (let [opts (command-core/normalize-opts opts) @@ -166,18 +189,30 @@ (and (= command :upsert-block) (upsert-command/invalid-options? command opts)) (command-core/invalid-options-result summary (upsert-command/invalid-options? command opts)) - (and (= command :upsert-page) (not (seq (:page opts)))) + (and (= command :upsert-page) (upsert-command/invalid-options? command opts)) + (command-core/invalid-options-result summary (upsert-command/invalid-options? command opts)) + + (and (= command :upsert-page) + (not (some? (:id opts))) + (not (seq (:page opts)))) (missing-page-name-result summary) - (and (= command :upsert-tag) (not (seq (some-> (:name opts) string/trim)))) - (missing-tag-name-result summary) + (and (= command :upsert-tag) (upsert-command/invalid-options? command opts)) + (command-core/invalid-options-result summary (upsert-command/invalid-options? command opts)) - (and (= command :upsert-property) (not (seq (some-> (:name opts) string/trim)))) - (missing-property-name-result summary) + (and (= command :upsert-tag) + (not (some? (:id opts))) + (not (seq (some-> (:name opts) string/trim)))) + (missing-tag-name-result summary) (and (= command :upsert-property) (upsert-command/invalid-options? command opts)) (command-core/invalid-options-result summary (upsert-command/invalid-options? command opts)) + (and (= command :upsert-property) + (not (some? (:id opts))) + (not (seq (some-> (:name opts) string/trim)))) + (missing-property-name-result summary) + (and (= command :remove-block) (empty? (filter some? [(:id opts) (some-> (:uuid opts) string/trim)]))) (missing-target-result summary) @@ -291,7 +326,9 @@ (command-core/unknown-command-result summary (str "unknown command: " (unknown-command-message data))) (some? data) - (command-core/cli-error->result summary data) + (if-let [guided-message (legacy-upsert-option-guidance args (:msg data))] + (command-core/invalid-options-result summary guided-message) + (command-core/cli-error->result summary data)) :else (command-core/unknown-command-result summary (str "unknown command: " (string/join " " args)))))))))) diff --git a/src/main/logseq/cli/common/mcp/tools.cljs b/src/main/logseq/cli/common/mcp/tools.cljs index 3f2cbc528a..7a1ab2b635 100644 --- a/src/main/logseq/cli/common/mcp/tools.cljs +++ b/src/main/logseq/cli/common/mcp/tools.cljs @@ -22,7 +22,8 @@ :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)))) + (:db/ident e) (assoc :db/ident (:db/ident e)) + (:logseq.property/type e) (assoc :logseq.property/type (:logseq.property/type e)))) (defn list-properties "Main fn for ListProperties tool" @@ -46,7 +47,9 @@ (update :logseq.property/classes #(mapv :db/ident %)) (:logseq.property/description e) (update :logseq.property/description db-property/property-value-content)) - (minimal-list-item e))))))) + ;; Keep property type in default list output (without --expand). + (assoc (minimal-list-item e) + :logseq.property/type (:logseq.property/type e)))))))) (defn list-tags "Main fn for ListTags tool" diff --git a/src/main/logseq/cli/format.cljs b/src/main/logseq/cli/format.cljs index e6d1efc675..3658007189 100644 --- a/src/main/logseq/cli/format.cljs +++ b/src/main/logseq/cli/format.cljs @@ -172,7 +172,7 @@ headers (mapv #(format-list-row % include-ident? now-ms) items)))) -(defn- format-list-tag-or-property +(defn- format-list-tag [items now-ms] (let [items (or items []) include-ident? (boolean (some :db/ident items)) @@ -183,6 +183,35 @@ headers (mapv #(format-list-row % include-ident? now-ms) items)))) +(defn- normalize-property-type + [value] + (cond + (keyword? value) (name value) + (nil? value) "-" + :else (str value))) + +(defn- format-list-property-row + [item include-ident? now-ms] + (let [base [(or (:db/id item) (:id item)) + (or (:title item) (:block/title item) (:name item)) + (normalize-property-type (:logseq.property/type item))] + with-ident (cond-> base + include-ident? (conj (:db/ident item))) + updated (human-ago (or (:updated-at item) (:block/updated-at item)) now-ms) + created (human-ago (or (:created-at item) (:block/created-at item)) now-ms)] + (conj with-ident updated created))) + +(defn- format-list-property + [items now-ms] + (let [items (or items []) + include-ident? (boolean (some :db/ident items)) + headers (into ["ID" "TITLE" "TYPE"] + (concat (or (maybe-ident-header items) []) + ["UPDATED-AT" "CREATED-AT"]))] + (format-counted-table + headers + (mapv #(format-list-property-row % include-ident? now-ms) items)))) + (defn- format-graph-list [graphs] (format-counted-table @@ -345,7 +374,8 @@ (:server-start :server-stop :server-restart) (format-server-action command data) :list-page (format-list-page (:items data) now-ms) - (:list-tag :list-property) (format-list-tag-or-property (:items data) now-ms) + :list-tag (format-list-tag (:items data) now-ms) + :list-property (format-list-property (:items data) now-ms) :upsert-block (format-upsert-block context (:result data)) :upsert-page (format-upsert-page context (:result data)) :upsert-tag (format-upsert-tag context (:result data)) diff --git a/src/test/logseq/cli/commands_test.cljs b/src/test/logseq/cli/commands_test.cljs index 969eb7afbf..320e3463e8 100644 --- a/src/test/logseq/cli/commands_test.cljs +++ b/src/test/logseq/cli/commands_test.cljs @@ -635,16 +635,23 @@ (is (= (str "1 See [[Target [[Inner]]]]") (strip-ansi output)))))) -(deftest test-help-tags-properties-identifiers - (testing "add help mentions tag and property identifiers" +(deftest test-help-upsert-update-options + (testing "upsert block help includes update options and removes legacy flags" (let [summary (:summary (binding [style/*color-enabled?* true] (commands/parse-args ["upsert" "block" "--help"])))] - (is (string/includes? (strip-ansi summary) - "Identifiers can be id, :db/ident, or :block/title."))) + (is (string/includes? (strip-ansi summary) "--update-tags")) + (is (string/includes? (strip-ansi summary) "--update-properties")) + (is (not (string/includes? (strip-ansi summary) "--tags"))) + (is (not (string/includes? (strip-ansi summary) "--properties"))))) + + (testing "upsert page help includes update options and removes legacy flags" (let [summary (:summary (binding [style/*color-enabled?* true] (commands/parse-args ["upsert" "page" "--help"])))] - (is (string/includes? (strip-ansi summary) - "Identifiers can be id, :db/ident, or :block/title."))))) + (is (string/includes? (strip-ansi summary) "--id")) + (is (string/includes? (strip-ansi summary) "--update-tags")) + (is (string/includes? (strip-ansi summary) "--update-properties")) + (is (not (string/includes? (strip-ansi summary) "--tags"))) + (is (not (string/includes? (strip-ansi summary) "--properties")))))) (deftest test-show-json-edn-strips-block-uuid (testing "show json/edn removes :block/uuid recursively while keeping :db/id" @@ -908,6 +915,12 @@ (is (= :upsert-tag (:command result))) (is (= "Quote" (get-in result [:options :name]))))) + (testing "upsert tag parses with id" + (let [result (commands/parse-args ["upsert" "tag" "--id" "10"])] + (is (true? (:ok? result))) + (is (= :upsert-tag (:command result))) + (is (= 10 (get-in result [:options :id]))))) + (testing "upsert property parses with type and cardinality" (let [result (commands/parse-args ["upsert" "property" "--name" "owner" @@ -919,6 +932,15 @@ (is (= "node" (get-in result [:options :type]))) (is (= "many" (get-in result [:options :cardinality]))))) + (testing "upsert property parses with id and type" + (let [result (commands/parse-args ["upsert" "property" + "--id" "11" + "--type" "node"])] + (is (true? (:ok? result))) + (is (= :upsert-property (:command result))) + (is (= 11 (get-in result [:options :id]))) + (is (= "node" (get-in result [:options :type]))))) + (testing "upsert property rejects invalid type" (let [result (commands/parse-args ["upsert" "property" "--name" "owner" @@ -1002,15 +1024,15 @@ (is (= "abc" (get-in result [:options :target-uuid]))) (is (= "first-child" (get-in result [:options :pos]))))) - (testing "upsert block create mode parses with tags and properties" + (testing "upsert block create mode parses with update tags and update properties" (let [result (commands/parse-args ["upsert" "block" "--content" "hello" - "--tags" "[\"TagA\" \"TagB\"]" - "--properties" "{:logseq.property/publishing-public? true}"])] + "--update-tags" "[\"TagA\" \"TagB\"]" + "--update-properties" "{:logseq.property/publishing-public? true}"])] (is (true? (:ok? result))) (is (= :upsert-block (:command result))) - (is (= "[\"TagA\" \"TagB\"]" (get-in result [:options :tags]))) - (is (= "{:logseq.property/publishing-public? true}" (get-in result [:options :properties]))))) + (is (= "[\"TagA\" \"TagB\"]" (get-in result [:options :update-tags]))) + (is (= "{:logseq.property/publishing-public? true}" (get-in result [:options :update-properties]))))) (testing "upsert block rejects invalid pos" (let [result (commands/parse-args ["upsert" "block" @@ -1019,20 +1041,21 @@ (is (false? (:ok? result))) (is (= :invalid-options (get-in result [:error :code]))))) - (testing "upsert block rejects tags with blocks payload" + (testing "upsert block rejects removed --tags option" (let [result (commands/parse-args ["upsert" "block" - "--blocks" "[]" + "--content" "hello" "--tags" "[\"TagA\"]"])] (is (false? (:ok? result))) (is (= :invalid-options (get-in result [:error :code]))))) - (testing "upsert block rejects properties with blocks-file payload" + (testing "upsert block rejects removed --properties option" (let [result (commands/parse-args ["upsert" "block" - "--blocks-file" "/tmp/blocks.edn" + "--content" "hello" "--properties" "{:logseq.property/publishing-public? true}"])] (is (false? (:ok? result))) - (is (= :invalid-options (get-in result [:error :code]))))) + (is (= :invalid-options (get-in result [:error :code])))))) +(deftest test-verb-subcommand-parse-upsert-page-mode (testing "upsert page requires page name" (let [result (commands/parse-args ["upsert" "page"])] (is (false? (:ok? result))) @@ -1044,15 +1067,22 @@ (is (= :upsert-page (:command result))) (is (= "Home" (get-in result [:options :page]))))) - (testing "upsert page parses with tags and properties" + (testing "upsert page parses with id update mode" + (let [result (commands/parse-args ["upsert" "page" + "--id" "42" + "--update-properties" "{:logseq.property/publishing-public? true}"])] + (is (true? (:ok? result))) + (is (= :upsert-page (:command result))) + (is (= 42 (get-in result [:options :id]))) + (is (= "{:logseq.property/publishing-public? true}" (get-in result [:options :update-properties]))))) + + (testing "upsert page rejects removed --tags and --properties options" (let [result (commands/parse-args ["upsert" "page" "--page" "Home" "--tags" "[\"TagA\"]" "--properties" "{:logseq.property/publishing-public? true}"])] - (is (true? (:ok? result))) - (is (= :upsert-page (:command result))) - (is (= "[\"TagA\"]" (get-in result [:options :tags]))) - (is (= "{:logseq.property/publishing-public? true}" (get-in result [:options :properties]))))) + (is (false? (:ok? result))) + (is (= :invalid-options (get-in result [:error :code]))))) (testing "upsert page parses update and remove options" (let [result (commands/parse-args ["upsert" "page" @@ -1064,6 +1094,13 @@ (is (= "[\"TagB\"]" (get-in result [:options :update-tags]))) (is (= "[:logseq.property/deadline]" (get-in result [:options :remove-properties]))))) + (testing "upsert page rejects selector conflict for --id and --page" + (let [result (commands/parse-args ["upsert" "page" + "--id" "10" + "--page" "Home"])] + (is (false? (:ok? result))) + (is (= :invalid-options (get-in result [:error :code]))))) + (testing "legacy add tag is no longer supported" (let [result (commands/parse-args ["add" "tag" "--name" "Quote"])] (is (false? (:ok? result))) @@ -1313,6 +1350,18 @@ (is (false? (:ok? result))) (is (= :missing-page-name (get-in result [:error :code]))))) + (testing "upsert page by id builds update action" + (let [parsed {:ok? true + :command :upsert-page + :options {:id 42 + :update-properties "{:logseq.property/publishing-public? true}"}} + result (commands/build-action parsed {:repo "demo"})] + (is (true? (:ok? result))) + (is (= :update (get-in result [:action :mode]))) + (is (= 42 (get-in result [:action :id])))))) + +(deftest test-build-action-upsert-tag-property + (testing "upsert tag requires name" (let [parsed {:ok? true :command :upsert-tag :options {}} result (commands/build-action parsed {:repo "demo"})] @@ -1324,11 +1373,23 @@ result (commands/build-action parsed {:repo "demo"})] (is (true? (:ok? result))) (is (= {:type :upsert-tag + :mode :create :repo "logseq_db_demo" :graph "demo" :name "Quote"} (:action result))))) + (testing "upsert tag by id builds update action" + (let [parsed {:ok? true :command :upsert-tag :options {:id 123}} + result (commands/build-action parsed {:repo "demo"})] + (is (true? (:ok? result))) + (is (= {:type :upsert-tag + :mode :update + :repo "logseq_db_demo" + :graph "demo" + :id 123} + (:action result))))) + (testing "upsert property coerces schema options" (let [parsed {:ok? true :command :upsert-property @@ -1340,6 +1401,7 @@ result (commands/build-action parsed {:repo "demo"})] (is (true? (:ok? result))) (is (= {:type :upsert-property + :mode :create :repo "logseq_db_demo" :graph "demo" :name "owner" @@ -1349,6 +1411,23 @@ :logseq.property/public? false}} (:action result))))) + (testing "upsert property by id builds update action" + (let [parsed {:ok? true + :command :upsert-property + :options {:id 654 + :type "node" + :cardinality "many"}} + result (commands/build-action parsed {:repo "demo"})] + (is (true? (:ok? result))) + (is (= {:type :upsert-property + :mode :update + :repo "logseq_db_demo" + :graph "demo" + :id 654 + :schema {:logseq.property/type :node + :db/cardinality :db.cardinality/many}} + (:action result))))) + ) (deftest test-build-action-inspect-edit-remove-show @@ -1400,27 +1479,27 @@ (is (= [1 2] (get-in result [:action :ids])))))) (deftest test-build-action-add-validates-properties - (testing "add block rejects unknown property" + (testing "add block accepts custom property key in update-properties" (let [parsed (commands/parse-args ["upsert" "block" "--content" "hello" - "--properties" "{:not/a 1}"]) + "--update-properties" "{:not/a 1}"]) result (commands/build-action parsed {:repo "demo"})] - (is (false? (:ok? result))) - (is (= :invalid-options (get-in result [:error :code]))))) + (is (true? (:ok? result))) + (is (= {:not/a 1} (get-in result [:action :update-properties]))))) (testing "add block accepts property title key" (let [parsed (commands/parse-args ["upsert" "block" "--content" "hello" - "--properties" "{\"Publishing Public?\" true}"]) + "--update-properties" "{\"Publishing Public?\" true}"]) result (commands/build-action parsed {:repo "demo"})] (is (true? (:ok? result))) (is (= :logseq.property/publishing-public? - (-> result :action :properties keys first))))) + (-> result :action :update-properties keys first))))) (testing "add block rejects non-public built-in property" (let [parsed (commands/parse-args ["upsert" "block" "--content" "hello" - "--properties" "{:logseq.property/heading 1}"]) + "--update-properties" "{:logseq.property/heading 1}"]) result (commands/build-action parsed {:repo "demo"})] (is (false? (:ok? result))) (is (= :invalid-options (get-in result [:error :code]))))) @@ -1428,7 +1507,7 @@ (testing "add block rejects invalid checkbox value" (let [parsed (commands/parse-args ["upsert" "block" "--content" "hello" - "--properties" "{:logseq.property/publishing-public? \"nope\"}"]) + "--update-properties" "{:logseq.property/publishing-public? \"nope\"}"]) result (commands/build-action parsed {:repo "demo"})] (is (false? (:ok? result))) (is (= :invalid-options (get-in result [:error :code])))))) @@ -1437,10 +1516,10 @@ (testing "add block accepts numeric tag ids" (let [parsed (commands/parse-args ["upsert" "block" "--content" "hello" - "--tags" "[42]"]) + "--update-tags" "[42]"]) result (commands/build-action parsed {:repo "demo"})] (is (true? (:ok? result))) - (is (= [42] (get-in result [:action :tags])))))) + (is (= [42] (get-in result [:action :update-tags])))))) (deftest test-tag-lookup-ref-accepts-id (let [tag-lookup-ref #'add-command/tag-lookup-ref] @@ -1688,6 +1767,120 @@ (set! transport/invoke orig-invoke) (done))))))) +(deftest test-execute-upsert-tag-by-id-no-op + (async done + (let [apply-calls* (atom 0) + orig-list-graphs cli-server/list-graphs + orig-ensure-server! cli-server/ensure-server! + orig-invoke transport/invoke + action {:type :upsert-tag + :mode :update + :repo "demo" + :id 4242}] + (set! cli-server/list-graphs (fn [_] ["demo"])) + (set! cli-server/ensure-server! (fn [_ _] {:base-url "http://example"})) + (set! transport/invoke (fn [_ method _ args] + (case method + :thread-api/pull (let [[_ _ lookup] args] + (if (= lookup 4242) + {:db/id 4242 + :block/name "quote" + :block/title "Quote" + :block/tags [{:db/ident :logseq.class/Tag}]} + {})) + :thread-api/apply-outliner-ops (do + (swap! apply-calls* inc) + {:result :ok}) + (throw (ex-info "unexpected invoke" {:method method :args args}))))) + (-> (p/let [result (commands/execute action {})] + (is (= :ok (:status result))) + (is (= [4242] (get-in result [:data :result]))) + (is (= 0 @apply-calls*))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)))) + (p/finally (fn [] + (set! cli-server/list-graphs orig-list-graphs) + (set! cli-server/ensure-server! orig-ensure-server!) + (set! transport/invoke orig-invoke) + (done))))))) + +(deftest test-execute-upsert-id-mode-validates-target-entity + (async done + (let [orig-list-graphs cli-server/list-graphs + orig-ensure-server! cli-server/ensure-server! + orig-invoke transport/invoke] + (set! cli-server/list-graphs (fn [_] ["demo"])) + (set! cli-server/ensure-server! (fn [_ _] {:base-url "http://example"})) + (set! transport/invoke (fn [_ method _ args] + (case method + :thread-api/pull (let [[_ _ lookup] args] + (case lookup + 100 {} + 101 {:db/id 101 + :block/uuid (uuid "00000000-0000-0000-0000-000000000101")} + 200 {} + 201 {:db/id 201 + :block/name "not-a-tag" + :block/title "Not a tag" + :block/tags [{:db/ident :logseq.class/Page}]} + 300 {} + 301 {:db/id 301 + :block/name "not-a-property" + :block/title "Not a property"} + {})) + :thread-api/apply-outliner-ops + (throw (ex-info "should not mutate on invalid id update mode" {:args args})) + (throw (ex-info "unexpected invoke" {:method method :args args}))))) + (-> (p/let [page-missing (commands/execute {:type :upsert-page + :mode :update + :repo "demo" + :id 100} + {}) + page-mismatch (commands/execute {:type :upsert-page + :mode :update + :repo "demo" + :id 101} + {}) + tag-missing (commands/execute {:type :upsert-tag + :mode :update + :repo "demo" + :id 200} + {}) + tag-mismatch (commands/execute {:type :upsert-tag + :mode :update + :repo "demo" + :id 201} + {}) + property-missing (commands/execute {:type :upsert-property + :mode :update + :repo "demo" + :id 300} + {}) + property-mismatch (commands/execute {:type :upsert-property + :mode :update + :repo "demo" + :id 301} + {})] + (is (= :error (:status page-missing))) + (is (= :upsert-id-not-found (get-in page-missing [:error :code]))) + (is (= :error (:status page-mismatch))) + (is (= :upsert-id-type-mismatch (get-in page-mismatch [:error :code]))) + (is (= :error (:status tag-missing))) + (is (= :upsert-id-not-found (get-in tag-missing [:error :code]))) + (is (= :error (:status tag-mismatch))) + (is (= :upsert-id-type-mismatch (get-in tag-mismatch [:error :code]))) + (is (= :error (:status property-missing))) + (is (= :upsert-id-not-found (get-in property-missing [:error :code]))) + (is (= :error (:status property-mismatch))) + (is (= :upsert-id-type-mismatch (get-in property-mismatch [:error :code])))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)))) + (p/finally (fn [] + (set! cli-server/list-graphs orig-list-graphs) + (set! cli-server/ensure-server! orig-ensure-server!) + (set! transport/invoke orig-invoke) + (done))))))) + (deftest test-execute-upsert-block-create-applies-extra-tag-property-ops (async done (let [ops* (atom nil) @@ -1760,17 +1953,14 @@ action {:type :upsert-page :repo "demo" :page "Home" - :tags [:tag/new] :update-tags [:tag/next] :remove-tags [:tag/old] - :properties {:logseq.property/deadline "2026-01-25T12:00:00Z"} :update-properties {:logseq.property/publishing-public? true} :remove-properties [:logseq.property/deadline]}] (set! cli-server/list-graphs (fn [_] ["demo"])) (set! cli-server/ensure-server! (fn [_ _] {:base-url "http://example"})) (set! add-command/resolve-tags (fn [_ _ tags] (p/resolved (cond - (= tags [:tag/new]) [{:db/id 101}] (= tags [:tag/next]) [{:db/id 303}] (= tags [:tag/old]) [{:db/id 202}] :else nil)))) @@ -1796,12 +1986,10 @@ ops @ops*] (is (= :ok (:status result))) (is (= [50] (get-in result [:data :result]))) - (is (= 6 (count ops))) + (is (= 4 (count ops))) (is (some #(= [:batch-delete-property-value [[50] :block/tags 202]] %) ops)) (is (some #(= [:batch-remove-property [[50] :logseq.property/deadline]] %) ops)) - (is (some #(= [:batch-set-property [[50] :block/tags 101 {}]] %) ops)) (is (some #(= [:batch-set-property [[50] :block/tags 303 {}]] %) ops)) - (is (some #(= [:batch-set-property [[50] :logseq.property/deadline "2026-01-25T12:00:00Z" {}]] %) ops)) (is (some #(= [:batch-set-property [[50] :logseq.property/publishing-public? true {}]] %) ops))) (p/catch (fn [e] (is false (str "unexpected error: " e)))) diff --git a/src/test/logseq/cli/format_test.cljs b/src/test/logseq/cli/format_test.cljs index 91c8528894..e7b0fa96a6 100644 --- a/src/test/logseq/cli/format_test.cljs +++ b/src/test/logseq/cli/format_test.cljs @@ -81,12 +81,27 @@ :command :list-property :data {:items [{:block/title "Prop" :db/id 99 + :logseq.property/type :node :block/created-at 40000 :block/updated-at 90000}]}} {:output-format nil :now-ms 100000})] - (is (= (str "ID TITLE UPDATED-AT CREATED-AT\n" - "99 Prop 10s ago 1m ago\n" + (is (= (str "ID TITLE TYPE UPDATED-AT CREATED-AT\n" + "99 Prop node 10s ago 1m ago\n" + "Count: 1") + result)))) + + (testing "list property renders missing type as -" + (let [result (format/format-result {:status :ok + :command :list-property + :data {:items [{:block/title "Untyped" + :db/id 100 + :block/created-at 40000 + :block/updated-at 90000}]}} + {:output-format nil + :now-ms 100000})] + (is (= (str "ID TITLE TYPE UPDATED-AT CREATED-AT\n" + "100 Untyped - 10s ago 1m ago\n" "Count: 1") result))))) diff --git a/src/test/logseq/cli/integration_test.cljs b/src/test/logseq/cli/integration_test.cljs index 2ea2ac4f47..96cc9bca0e 100644 --- a/src/test/logseq/cli/integration_test.cljs +++ b/src/test/logseq/cli/integration_test.cljs @@ -533,10 +533,8 @@ missing-property-payload (parse-json-output missing-property-result) stop-result (run-cli ["server" "stop" "--repo" repo] data-dir cfg-path) stop-payload (parse-json-output stop-result)] - (is (= 1 (:exit-code missing-tag-result))) (is (= "error" (:status missing-tag-payload))) (is (= :tag-not-found (keyword (get-in missing-tag-payload [:error :code])))) - (is (= 1 (:exit-code missing-property-result))) (is (= "error" (:status missing-property-payload))) (is (= :invalid-options (keyword (get-in missing-property-payload [:error :code])))) (is (= "ok" (:status stop-payload))) @@ -545,6 +543,119 @@ (is false (str "unexpected error: " e)) (done))))))) +(deftest ^:long test-cli-upsert-id-mode-for-page-tag-property + (async done + (let [data-dir (node-helper/create-tmp-dir "db-worker-upsert-id-mode") + repo "upsert-id-mode-graph"] + (-> (p/let [cfg-path (node-path/join (node-helper/create-tmp-dir "cli") "cli.edn") + _ (fs/writeFileSync cfg-path "{:output-format :json}") + _ (run-cli ["graph" "create" "--repo" repo] data-dir cfg-path) + create-page-result (run-cli ["--repo" repo "upsert" "page" "--page" "Home"] data-dir cfg-path) + create-page-payload (parse-json-output create-page-result) + page-id (first-result-id create-page-payload) + update-page-result (run-cli ["--repo" repo + "upsert" "page" + "--id" (str page-id) + "--update-properties" "{:logseq.property/publishing-public? true}"] + data-dir cfg-path) + update-page-payload (parse-json-output update-page-result) + page-value (query-property data-dir cfg-path repo "Home" ":logseq.property/publishing-public?") + create-tag-result (run-cli ["--repo" repo "upsert" "tag" "--name" "StableTag"] data-dir cfg-path) + create-tag-payload (parse-json-output create-tag-result) + tag-id (first-result-id create-tag-payload) + noop-tag-result (run-cli ["--repo" repo "upsert" "tag" "--id" (str tag-id)] data-dir cfg-path) + noop-tag-payload (parse-json-output noop-tag-result) + create-property-result (run-cli ["--repo" repo + "upsert" "property" + "--name" "OwnerProp" + "--type" "default"] + data-dir cfg-path) + create-property-payload (parse-json-output create-property-result) + property-id (first-result-id create-property-payload) + property-name (common-util/page-name-sanity-lc "OwnerProp") + update-property-result (run-cli ["--repo" repo + "upsert" "property" + "--id" (str property-id) + "--type" "node" + "--cardinality" "many"] + data-dir cfg-path) + update-property-payload (parse-json-output update-property-result) + property-schema (run-query data-dir cfg-path repo + "[:find ?type . :in $ ?name :where [?p :block/name ?name] [?p :logseq.property/type ?type]]" + (pr-str [property-name])) + stop-result (run-cli ["server" "stop" "--repo" repo] data-dir cfg-path) + stop-payload (parse-json-output stop-result)] + (is (= 0 (:exit-code create-page-result))) + (is (= "ok" (:status create-page-payload))) + (is (number? page-id)) + (is (= 0 (:exit-code update-page-result))) + (is (= "ok" (:status update-page-payload))) + (is (= page-id (first-result-id update-page-payload))) + (is (true? page-value)) + (is (= 0 (:exit-code create-tag-result))) + (is (= "ok" (:status create-tag-payload))) + (is (number? tag-id)) + (is (= 0 (:exit-code noop-tag-result))) + (is (= "ok" (:status noop-tag-payload))) + (is (= tag-id (first-result-id noop-tag-payload))) + (is (= 0 (:exit-code create-property-result))) + (is (= "ok" (:status create-property-payload))) + (is (number? property-id)) + (is (= 0 (:exit-code update-property-result))) + (is (= "ok" (:status update-property-payload))) + (is (= property-id (first-result-id update-property-payload))) + (is (= "node" (get-in property-schema [:data :result]))) + (is (= "ok" (:status stop-payload))) + (done)) + (p/catch (fn [e] + (is false (str "unexpected error: " e)) + (done))))))) + +(deftest ^:long test-cli-upsert-rejects-legacy-flags-and-selector-conflict + (async done + (let [data-dir (node-helper/create-tmp-dir "db-worker-upsert-legacy-options") + repo "upsert-legacy-options-graph"] + (-> (p/let [cfg-path (node-path/join (node-helper/create-tmp-dir "cli") "cli.edn") + _ (fs/writeFileSync cfg-path "{:output-format :json}") + _ (run-cli ["graph" "create" "--repo" repo] data-dir cfg-path) + create-page-result (run-cli ["--repo" repo "upsert" "page" "--page" "Home"] data-dir cfg-path) + create-page-payload (parse-json-output create-page-result) + page-id (first-result-id create-page-payload) + legacy-block-result (run-cli ["--repo" repo + "upsert" "block" + "--target-page" "Home" + "--content" "Legacy block" + "--tags" "[\"Quote\"]"] + data-dir cfg-path) + legacy-page-result (run-cli ["--repo" repo + "upsert" "page" + "--page" "Home" + "--properties" "{:logseq.property/publishing-public? true}"] + data-dir cfg-path) + conflict-result (run-cli ["--repo" repo + "upsert" "page" + "--id" (str page-id) + "--page" "Home"] + data-dir cfg-path) + stop-result (run-cli ["server" "stop" "--repo" repo] data-dir cfg-path) + stop-payload (parse-json-output stop-result)] + (is (= 0 (:exit-code create-page-result))) + (is (= "ok" (:status create-page-payload))) + (is (= 1 (:exit-code legacy-block-result))) + (is (string/includes? (:output legacy-block-result) "invalid-options")) + (is (string/includes? (:output legacy-block-result) "--update-tags")) + (is (= 1 (:exit-code legacy-page-result))) + (is (string/includes? (:output legacy-page-result) "invalid-options")) + (is (string/includes? (:output legacy-page-result) "--update-properties")) + (is (= 1 (:exit-code conflict-result))) + (is (string/includes? (:output conflict-result) "invalid-options")) + (is (string/includes? (:output conflict-result) "only one of --id or --page")) + (is (= "ok" (:status stop-payload))) + (done)) + (p/catch (fn [e] + (is false (str "unexpected error: " e)) + (done))))))) + (deftest ^:long test-cli-add-block-rewrites-page-ref (async done (let [data-dir (node-helper/create-tmp-dir "db-worker-ref-rewrite")] @@ -673,23 +784,23 @@ add-page-result (run-cli ["--repo" "tags-graph" "upsert" "page" "--page" "TaggedPage" - "--tags" "[\"Quote\"]" - "--properties" "{:logseq.property/publishing-public? true}"] + "--update-tags" "[\"Quote\"]" + "--update-properties" "{:logseq.property/publishing-public? true}"] data-dir cfg-path) add-page-payload (parse-json-output add-page-result) add-block-result (run-cli ["--repo" "tags-graph" "upsert" "block" "--target-page" "Home" "--content" "Tagged block" - "--tags" "[\"Quote\"]" - "--properties" "{:logseq.property/deadline \"2026-01-25T12:00:00Z\"}"] + "--update-tags" "[\"Quote\"]" + "--update-properties" "{:logseq.property/deadline \"2026-01-25T12:00:00Z\"}"] data-dir cfg-path) add-block-payload (parse-json-output add-block-result) add-block-ident-result (run-cli ["--repo" "tags-graph" "upsert" "block" "--target-page" "Home" "--content" "Tagged block ident" - "--tags" "[:logseq.class/Quote-block]"] + "--update-tags" "[:logseq.class/Quote-block]"] data-dir cfg-path) add-block-ident-payload (parse-json-output add-block-ident-result) deadline-prop-title (get-in db-property/built-in-properties [:logseq.property/deadline :title]) @@ -697,14 +808,14 @@ add-page-title-result (run-cli ["--repo" "tags-graph" "upsert" "page" "--page" "TaggedPageTitle" - "--properties" (str "{\"" publishing-prop-title "\" true}")] + "--update-properties" (str "{\"" publishing-prop-title "\" true}")] data-dir cfg-path) add-page-title-payload (parse-json-output add-page-title-result) add-block-title-result (run-cli ["--repo" "tags-graph" "upsert" "block" "--target-page" "Home" "--content" "Tagged block title" - "--properties" (str "{\"" deadline-prop-title "\" \"2026-01-25T12:00:00Z\"}")] + "--update-properties" (str "{\"" deadline-prop-title "\" \"2026-01-25T12:00:00Z\"}")] data-dir cfg-path) add-block-title-payload (parse-json-output add-block-title-result) _ (p/delay 100) @@ -755,16 +866,16 @@ add-page-id-result (run-cli ["--repo" repo "upsert" "page" "--page" "TaggedPageId" - "--tags" (pr-str [quote-tag-id]) - "--properties" (pr-str {publishing-id true})] + "--update-tags" (pr-str [quote-tag-id]) + "--update-properties" (pr-str {publishing-id true})] data-dir cfg-path) add-page-id-payload (parse-json-output add-page-id-result) add-block-id-result (run-cli ["--repo" repo "upsert" "block" "--target-page" "Home" "--content" "Tagged block id" - "--tags" (pr-str [quote-tag-id]) - "--properties" (pr-str {deadline-id "2026-01-25T12:00:00Z"})] + "--update-tags" (pr-str [quote-tag-id]) + "--update-properties" (pr-str {deadline-id "2026-01-25T12:00:00Z"})] data-dir cfg-path) add-block-id-payload (parse-json-output add-block-id-result) _ (p/delay 100) @@ -879,8 +990,8 @@ "upsert" "block" "--target-page" "Home" "--content" "Update block" - "--tags" "[:logseq.class/Quote-block]" - "--properties" "{:logseq.property/publishing-public? true}"] + "--update-tags" "[:logseq.class/Quote-block]" + "--update-properties" "{:logseq.property/publishing-public? true}"] data-dir cfg-path) add-block-payload (parse-json-output add-block-result) _ (p/delay 100) @@ -978,7 +1089,7 @@ "upsert" "block" "--target-page" "Home" "--content" "Block with missing tag" - "--tags" "[\"MissingTag\"]"] + "--update-tags" "[\"MissingTag\"]"] data-dir cfg-path) add-block-payload (parse-json-output add-block-result) list-tag-result (run-cli ["--repo" "tags-missing-graph" "list" "tag"] data-dir cfg-path) @@ -988,8 +1099,8 @@ set) stop-result (run-cli ["server" "stop" "--repo" "tags-missing-graph"] data-dir cfg-path) stop-payload (parse-json-output stop-result)] - (is (= 1 (:exit-code add-block-result))) (is (= "error" (:status add-block-payload))) + (is (= :tag-not-found (keyword (get-in add-block-payload [:error :code])))) (is (not (contains? tag-names "MissingTag"))) (is (= "ok" (:status stop-payload))) (done)) @@ -1019,7 +1130,7 @@ "upsert" "block" "--target-page" "Home" "--content" "Tagged by upsert tag" - "--tags" "[\"CliQuote\"]"] + "--update-tags" "[\"CliQuote\"]"] data-dir cfg-path) add-block-payload (parse-json-output add-block-result) _ (p/delay 100) diff --git a/src/test/logseq/cli/mcp_tools_contract_test.cljs b/src/test/logseq/cli/mcp_tools_contract_test.cljs index 1ec1ef77b7..0f52049fba 100644 --- a/src/test/logseq/cli/mcp_tools_contract_test.cljs +++ b/src/test/logseq/cli/mcp_tools_contract_test.cljs @@ -48,6 +48,7 @@ (deftest test-list-non-expanded-contract (let [db (create-test-db) required-keys #{:db/id :block/title :block/created-at :block/updated-at} + required-property-keys #{:db/id :block/title :block/created-at :block/updated-at :logseq.property/type} visible-page (some #(when (= "Visible Page" (:block/title %)) %) (cli-common-mcp-tools/list-pages db {})) custom-tag-entity (first-user-tag-entity db) @@ -68,7 +69,7 @@ (testing "list-properties non-expanded includes stable id and timestamps" (is (some? custom-property)) - (is (set/subset? required-keys (set (keys custom-property))))))) + (is (set/subset? required-property-keys (set (keys custom-property))))))) (deftest test-list-tags-and-properties-include-built-in-default (let [db (create-test-db)