diff --git a/cli-e2e/spec/non_sync_cases.edn b/cli-e2e/spec/non_sync_cases.edn index dd1fac0800..97d026b92e 100644 --- a/cli-e2e/spec/non_sync_cases.edn +++ b/cli-e2e/spec/non_sync_cases.edn @@ -334,13 +334,13 @@ :setup ["{{cli}} --data-dir {{data-dir-arg}} --config {{config-path-arg}} --output json graph create --graph {{graph-arg}} >/dev/null" "{{cli}} --data-dir {{data-dir-arg}} --config {{config-path-arg}} --output json upsert page --graph {{graph-arg}} --page Home >/dev/null" "{{cli}} --data-dir {{data-dir-arg}} --config {{config-path-arg}} --output json upsert block --graph {{graph-arg}} --target-page Home --content 'Alpha block' >/dev/null"] - :cmds ["{{cli}} --data-dir {{data-dir-arg}} --config {{config-path-arg}} --output human upsert block --graph {{graph-arg}} --id \"$({{cli}} --data-dir {{data-dir-arg}} --config {{config-path-arg}} --output json query --graph {{graph-arg}} --query '[:find ?e . :where [?e :block/title \"Alpha block\"]]' | python3 -c 'import sys,json; print(json.load(sys.stdin)[\"data\"][\"result\"])')\" --content 'Updated by id' --status doing --update-tags '[:logseq.class/Quote-block]' --update-properties '{:logseq.property/publishing-public? true}' >/dev/null" + :cmds ["{{cli}} --data-dir {{data-dir-arg}} --config {{config-path-arg}} --output human upsert block --graph {{graph-arg}} --id \"$({{cli}} --data-dir {{data-dir-arg}} --config {{config-path-arg}} --output json query --graph {{graph-arg}} --query '[:find ?e . :where [?e :block/title \"Alpha block\"]]' | python3 -c 'import sys,json; print(json.load(sys.stdin)[\"data\"][\"result\"])')\" --content 'Updated by id' --update-tags '[:logseq.class/Quote-block]' --update-properties '{:logseq.property/publishing-public? true}' >/dev/null" "{{cli}} --data-dir {{data-dir-arg}} --config {{config-path-arg}} --output human show --graph {{graph-arg}} --page Home"] :expect {:exit 0 :stdout-contains ["Updated by id"]} :covers {:commands ["upsert block"] :options {:global ["--config" "--graph" "--data-dir" "--output"] - :upsert ["--id" "--status" "--update-tags" "--update-properties"]}} + :upsert ["--id" "--update-tags" "--update-properties"]}} :cleanup ["{{cli}} --data-dir {{data-dir-arg}} --config {{config-path-arg}} --output json server stop --graph {{graph-arg}}"] :tags [:upsert]} diff --git a/docs/agent-guide/080-logseq-cli-status-option-cleanup.md b/docs/agent-guide/080-logseq-cli-status-option-cleanup.md new file mode 100644 index 0000000000..22e8d4c3aa --- /dev/null +++ b/docs/agent-guide/080-logseq-cli-status-option-cleanup.md @@ -0,0 +1,202 @@ +# Logseq CLI Status Option Cleanup Implementation Plan + +Goal: align task-related CLI semantics by removing `--status` from `upsert block`, adding consistent `-c` aliases for all `--content` options, and improving invalid `--status` error output to include all available values. + +Architecture: keep the current `logseq-cli -> command parse/build -> transport/invoke -> db-worker-node thread-api` flow, but move `--status` invalid-value reporting to runtime validation so it can use graph data. + +Architecture: add a small db-worker-node read endpoint for task status values from the current graph, and reuse it in CLI command validation/error messaging. + +Tech Stack: ClojureScript, `babashka.cli`, existing command modules under `src/main/logseq/cli/command/*`, formatter/error layer in `src/main/logseq/cli/format.cljs`. + +Related: builds on `docs/agent-guide/078-logseq-cli-task-subcommands.md` and current command contracts in `docs/cli/logseq-cli.md`. + +## Problem statement + +Current `upsert block` still accepts `--status`, even though task semantics are already represented by `upsert task`. + +Current `--content` aliasing is inconsistent: search/upsert already support `-c`, but `list task --content` does not. + +Current invalid enum error output for `--status` does not list accepted values, which slows down CLI troubleshooting. + +Example current output: + +```text +logseq list task --status xxx +Error (invalid-options): Invalid value for option :status: xxx +``` + +Target UX should include available values in the error output. + +## Current baseline from implementation + +### 1) `upsert block --status` is currently part of command spec and update path + +`src/main/logseq/cli/command/upsert.cljs` currently includes `:status` inside `upsert-block-spec` and examples include `upsert block --id 123 --status done`. + +`upsert block` update mode currently delegates to `src/main/logseq/cli/command/update.cljs`, where `:status` is parsed/normalized and merged into `:update-properties` as `:logseq.property/status`. + +### 2) `--content` aliasing is partly consistent + +Current `:content` options with `:alias :c` exist in: + +- `src/main/logseq/cli/command/search.cljs` +- `src/main/logseq/cli/command/upsert.cljs` (`upsert block`, `upsert task`) + +Current `list task` in `src/main/logseq/cli/command/list.cljs` defines `:content` without alias. + +### 3) Invalid status message is parser-driven today, which blocks graph-derived values + +`src/main/logseq/cli/command/list.cljs` and `src/main/logseq/cli/command/upsert.cljs` currently define `:status` with static `:validate #{...}` sets. + +That causes invalid status values to fail at parse time, before repo/graph resolution and before any db query can run. + +With parse-time rejection, CLI cannot include values from the current graph. + +### 4) db-worker-node currently has no task-status-values endpoint + +`src/main/logseq/cli/common/db_worker.cljs` currently supports task listing/filtering, but does not expose a dedicated API to list available task status values from graph data. + +To satisfy the new requirement, we need a small read-only thread-api for status values and wire it through `src/main/frontend/worker/db_core.cljs`. + +## Scope + +In scope: + +1. Remove `--status` from `upsert block` option surface and help/examples. +2. Ensure every CLI `--content` option has `-c` alias. +3. When `--status` is invalid (e.g. `list task --status xxx`), show accepted values queried from the current graph in the error message. + +Out of scope: + +- New task query/filter semantics in db-worker. +- Changing task storage/property schema. +- New command groups beyond existing `upsert task` / `list task`. + +## Design decisions + +### Decision A: Task status writes move fully to `upsert task` + +`upsert block` will no longer expose `--status`. + +Task status changes should go through `upsert task --id|--uuid|--page|--content ... --status ...`. + +This removes overlap between generic block updates and task-specific semantics. + +### Decision B: Keep `-c` as the standard short alias for `--content` + +Add `:alias :c` to any remaining `:content` specs missing it (currently `list task`). + +No new short alias should be introduced for `content`. + +### Decision C: Validate `--status` against graph-derived values at runtime + +For `list task` / `upsert task` status validation, use values queried from the current graph instead of static hardcoded sets in command spec. + +This requires moving status value validation out of parse-time `:validate #{...}` and into command build/execute phase where graph/repo is available. + +Target output shape: + +```text +Error (invalid-options): Invalid value for option :status: xxx. Available values (from current graph): todo, doing, done +``` + +(Exact values come from the active graph query result; ordering should be deterministic, e.g. sorted by status title.) + +## Proposed implementation plan + +1. Add RED parser tests for `upsert block --status` to assert it becomes invalid (unknown option path). +2. Add RED parser tests for `list task -c ` to assert `-c` works as alias for `--content`. +3. Add RED command tests asserting invalid task status errors include graph-derived available values in message text. +4. Remove `:status` from `upsert-block-spec` in `src/main/logseq/cli/command/upsert.cljs`. +5. Remove/adjust `upsert block` examples and help text that mention `--status`. +6. If needed, add migration guidance in `src/main/logseq/cli/commands.cljs` for `upsert block --status` (guide user to `upsert task`). +7. Add `:alias :c` to `list-task-spec :content` in `src/main/logseq/cli/command/list.cljs`. +8. Remove parse-time static `:validate #{...}` for task `:status` options where graph-derived validation is required (`list task`, `upsert task`). +9. Add db-worker helper in `src/main/logseq/cli/common/db_worker.cljs` to list available task status values from the current graph. +10. Expose the helper via new thread-api in `src/main/frontend/worker/db_core.cljs`. +11. In CLI command layer (`list.cljs` and `upsert.cljs`), fetch status values for current repo/graph and validate `--status`; on invalid input return `:invalid-options` with available-values suffix from graph query result. +12. Keep output ordering deterministic (e.g. sorted asc by display name/ident) so error text and tests remain stable. +13. Update completion tests and command docs to reflect final option surface. +14. Update CLI user docs in `docs/cli/logseq-cli.md` for `upsert block`, `upsert task`, `list task -c`, and graph-derived status validation behavior. +15. Update CLI e2e inventory/cases where `upsert block --status` is currently listed/covered. +16. Run focused tests, then run full lint/test checks. + +## Files expected to change + +Primary implementation files: + +- `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/upsert.cljs` +- `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/list.cljs` +- `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/commands.cljs` +- `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/common/db_worker.cljs` +- `/Users/rcmerci/gh-repos/logseq/src/main/frontend/worker/db_core.cljs` + +Primary tests: + +- `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/commands_test.cljs` +- `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/completion_generator_test.cljs` +- `/Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/format_test.cljs` (only if formatted error snapshots/assertions are affected) + +Docs and e2e metadata: + +- `/Users/rcmerci/gh-repos/logseq/docs/cli/logseq-cli.md` +- `/Users/rcmerci/gh-repos/logseq/cli-e2e/spec/non_sync_inventory.edn` +- `/Users/rcmerci/gh-repos/logseq/cli-e2e/spec/non_sync_cases.edn` + +## Testing plan + +I will follow `@test-driven-development` and implement tests before behavior changes. + +### Unit / command tests + +Add tests in `commands_test.cljs` for: + +- `upsert block --id 1 --status done` returns `:invalid-options` (or unknown option mapped to invalid-options). +- `list task -c alpha` parses to `:list-task` with `:content "alpha"`. +- invalid status errors for task commands include a graph-derived available-values suffix (using mocked thread-api responses). + +### Completion tests + +Update `completion_generator_test.cljs` to assert `list task` offers `-c` along with `--content`. + +### Docs/e2e coverage checks + +Ensure inventory and cases no longer advertise `upsert block --status`. + +Add/update one case that validates status updates via `upsert task` path. + +### Verification commands + +- `bb dev:test -v logseq.cli.commands-test` +- `bb dev:test -v logseq.cli.completion-generator-test` +- `bb dev:test -v logseq.cli.format-test` (if touched) +- `bb -f cli-e2e/bb.edn test --skip-build` (if e2e specs are changed) +- `bb dev:lint-and-test` + +## Risks and mitigations + +Risk: users relying on `upsert block --status` scripts will break. + +Mitigation: provide explicit migration guidance in parser error messaging and update docs clearly. + +Risk: status values query may return empty or unexpected shapes for some graphs. + +Mitigation: normalize db-worker response contract, sort deterministically, and provide a clear fallback suffix when no status values are found. + +Risk: moving status validation from parse time to runtime may change where errors are raised. + +Mitigation: add focused parser/build/execute tests for `list task` and `upsert task` to lock error codes/messages and avoid regressions on other options. + +## Acceptance criteria + +1. `upsert block` help/spec no longer includes `--status`. +2. All CLI options named `--content` support `-c`. +3. `logseq list task --status xxx` shows invalid-value error plus a deterministic available-values list queried from the current graph. +4. Existing `upsert task --status ...` and `list task --status ...` happy paths remain green. +5. CLI docs and e2e inventory reflect the new surface. + +## Question + +No blocking question. + +--- diff --git a/docs/cli/logseq-cli.md b/docs/cli/logseq-cli.md index dd48b36358..a273e1e3b9 100644 --- a/docs/cli/logseq-cli.md +++ b/docs/cli/logseq-cli.md @@ -214,17 +214,19 @@ Inspect and edit commands: - `list page [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list pages (defaults to `--sort updated-at`) - `list tag [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list tags (defaults to `--sort updated-at`) - `list property [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list properties (defaults to `--sort updated-at`; `TYPE` and `CARDINALITY` are included by default even without `--expand`; missing schema cardinality is treated as `one`) -- `list task [--status ] [--priority ] [--content ] [--fields ] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list task nodes tagged with `#Task` (supports both pages and blocks; defaults to `--sort updated-at`) +- `list task [--status ] [--priority ] [-c|--content ] [--fields ] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list task nodes tagged with `#Task` (supports both pages and blocks; defaults to `--sort updated-at`) + - `--status` is validated at runtime using values from the current graph; invalid values return an error that includes available values from that graph. - `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 [--content ] [--status ] [--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 - - When both `--status` and `--update-properties` set `:logseq.property/status`, the value from `--update-properties` takes precedence. +- `upsert block --id |--uuid [--content ] [--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 + - `--status` is not supported on `upsert block`; use `upsert task --status ...` for task status updates. - `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 task --content [--target-page |--target-id |--target-uuid ] [--pos first-child|last-child|sibling] [--status ] [--priority ] [--update-tags ] [--update-properties ] [--remove-tags ] [--remove-properties ]` - create a task block and ensure `#Task` is attached - `upsert task --page [--status ] [--priority ] [--update-tags ] [--update-properties ] [--remove-tags ] [--remove-properties ]` - create/update a task page and ensure `#Task` is attached - `upsert task --id |--uuid [--status ] [--priority ] [--update-tags ] [--update-properties ] [--remove-tags ] [--remove-properties ]` - update an existing node and ensure `#Task` is attached + - `--status` is validated at runtime using values from the current graph; invalid values return an error that includes available values from that graph. - `upsert tag --name ` - create or upsert a tag by name - `upsert tag --id [--name ]` - validate a tag by id; when `--name` is provided, rename that tag id (no-op if normalized name is unchanged) - `upsert tag --id --name ` conflicts: returns `tag-name-conflict` when target name is a non-tag page, and `tag-rename-conflict` when target name is another existing tag diff --git a/src/main/logseq/cli/command/list.cljs b/src/main/logseq/cli/command/list.cljs index 779343131a..52bdcb391b 100644 --- a/src/main/logseq/cli/command/list.cljs +++ b/src/main/logseq/cli/command/list.cljs @@ -1,8 +1,8 @@ (ns logseq.cli.command.list "List-related CLI commands." (:require [clojure.string :as string] - [logseq.cli.command.add :as add-command] [logseq.cli.command.core :as core] + [logseq.cli.command.task-status :as task-status-command] [logseq.cli.server :as cli-server] [logseq.cli.transport :as transport] [promesa.core :as p])) @@ -124,13 +124,11 @@ (merge-with merge list-common-spec - {:status {:desc "Filter by task status" - :validate #{"todo" "doing" "done" "now" "later" "wait" "waiting" - "backlog" "canceled" "cancelled" - "in-review" "in_review" "inreview" "in-progress"}} + {:status {:desc "Filter by task status"} :priority {:desc "Filter by task priority" :validate #{"low" "medium" "high" "urgent"}} - :content {:desc "Filter by task title content"} + :content {:desc "Filter by task title content" + :alias :c} :sort {:validate (set (keys list-task-field-map))} :fields {:multiple-values (keys list-task-field-map)}})) @@ -279,18 +277,29 @@ [action config] (-> (p/let [cfg (cli-server/ensure-server! config (:repo action)) options (:options action) - normalized-options (cond-> options - (seq (some-> (:status options) string/trim)) - (assoc :status (add-command/normalize-status (:status options))) - (seq (some-> (:priority options) string/trim)) - (assoc :priority (normalize-priority (:priority options)))) - items (transport/invoke cfg :thread-api/cli-list-tasks false - [(:repo action) normalized-options]) - sort-field (effective-sort-field normalized-options) - order (or (:order normalized-options) "asc") - fields (parse-field-list (:fields normalized-options)) - sorted (apply-sort items sort-field order list-task-field-map) - limited (apply-offset-limit sorted (:offset normalized-options) (:limit normalized-options)) - final (apply-fields limited fields list-task-field-map)] - {:status :ok - :data {:items final}}))) + status-input (some-> (:status options) string/trim) + available-statuses (when (seq status-input) + (transport/invoke cfg :thread-api/q false + [(:repo action) + [task-status-command/status-closed-values-query]])) + resolved-status (when (seq status-input) + (task-status-command/resolve-status-ident status-input available-statuses))] + (if (and (seq status-input) (not resolved-status)) + {:status :error + :error {:code :invalid-options + :message (task-status-command/invalid-status-message status-input available-statuses)}} + (let [normalized-options (cond-> options + resolved-status + (assoc :status resolved-status) + (seq (some-> (:priority options) string/trim)) + (assoc :priority (normalize-priority (:priority options))))] + (p/let [items (transport/invoke cfg :thread-api/cli-list-tasks false + [(:repo action) normalized-options]) + sort-field (effective-sort-field normalized-options) + order (or (:order normalized-options) "asc") + fields (parse-field-list (:fields normalized-options)) + sorted (apply-sort items sort-field order list-task-field-map) + limited (apply-offset-limit sorted (:offset normalized-options) (:limit normalized-options)) + final (apply-fields limited fields list-task-field-map)] + {:status :ok + :data {:items final}})))))) diff --git a/src/main/logseq/cli/command/task_status.cljs b/src/main/logseq/cli/command/task_status.cljs new file mode 100644 index 0000000000..2c4521f859 --- /dev/null +++ b/src/main/logseq/cli/command/task_status.cljs @@ -0,0 +1,74 @@ +(ns logseq.cli.command.task-status + "Runtime task status helpers for graph-derived validation." + (:require [clojure.string :as string] + [logseq.cli.command.add :as add-command])) + +(def status-closed-values-query + '[:find [?status-ident ...] + :where + [?property :db/ident :logseq.property/status] + [?value :block/closed-value-property ?property] + [?value :db/ident ?status-ident]]) + +(defn- status-ident->value + [ident] + (when (keyword? ident) + (let [n (name ident)] + (when (string/starts-with? n "status.") + (subs n (count "status.")))))) + +(defn- normalize-token + [value] + (some-> value + str + string/trim + string/lower-case + (string/replace #"^:+" "") + (string/replace #"^logseq\.property/status\." "") + (string/replace #"^status\." "") + (string/replace #"[\s_]+" "-"))) + +(defn normalize-available-statuses + "Normalize db-worker status values into sorted maps of + `{:ident :value }` for deterministic matching/output." + [statuses] + (->> statuses + (keep (fn [item] + (let [ident (cond + (keyword? item) item + (map? item) (:ident item) + :else nil) + value (or (when (map? item) + (some-> (:value item) normalize-token)) + (some-> ident status-ident->value normalize-token))] + (when (and ident (seq value)) + {:ident ident :value value})))) + (sort-by (juxt :value (comp str :ident))) + distinct + vec)) + +(defn resolve-status-ident + "Resolve user `status-input` to one of `available-statuses` idents. + Returns nil when unresolved." + [status-input available-statuses] + (let [available-statuses (normalize-available-statuses available-statuses) + available-idents (set (map :ident available-statuses)) + by-value (into {} (map (juxt :value :ident) available-statuses)) + legacy (add-command/normalize-status status-input) + token (normalize-token status-input) + ident-from-token (when (seq token) + (keyword "logseq.property" (str "status." token)))] + (or (when (contains? available-idents legacy) legacy) + (get by-value token) + (when (contains? available-idents ident-from-token) + ident-from-token)))) + +(defn invalid-status-message + [status-input available-statuses] + (let [values (map :value (normalize-available-statuses available-statuses)) + available-text (if (seq values) + (string/join ", " values) + "(none)")] + (str "Invalid value for option :status: " status-input + ". Available values (from current graph): " + available-text))) diff --git a/src/main/logseq/cli/command/upsert.cljs b/src/main/logseq/cli/command/upsert.cljs index 536161c7be..f850219475 100644 --- a/src/main/logseq/cli/command/upsert.cljs +++ b/src/main/logseq/cli/command/upsert.cljs @@ -3,6 +3,7 @@ (:require [clojure.string :as string] [logseq.cli.command.add :as add-command] [logseq.cli.command.core :as core] + [logseq.cli.command.task-status :as task-status-command] [logseq.cli.command.update :as update-command] [logseq.cli.server :as cli-server] [logseq.cli.transport :as transport] @@ -32,10 +33,6 @@ :blocks-file {:desc "EDN file of blocks [create only]" :coerce common-graph/expand-home :complete :file} - :status {:desc "Set task status" - :validate #{"todo" "doing" "done" "now" "later" "wait" "waiting" - "backlog" "canceled" "cancelled" - "in-review" "in_review" "inreview" "in-progress"}} :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) [update only]"} @@ -70,9 +67,7 @@ :complete :pages} :pos {:desc "Position. Default: last-child" :validate #{"first-child" "last-child" "sibling"}} - :status {:desc "Set task status" - :validate #{"todo" "doing" "done" "now" "later" "wait" "waiting" - "backlog" "canceled" "cancelled" "in-review" "in-progress"}} + :status {:desc "Set task status"} :priority {:desc "Set task priority" :validate #{"low" "medium" "high" "urgent"}} :update-tags {:desc "Tags to add/update (EDN vector)"} @@ -107,8 +102,7 @@ "logseq upsert block --graph my-graph --id 123 --content \"Updated content\"" "logseq upsert block --graph my-graph --id 123 --target-page Home" "logseq upsert block --graph my-graph --target-page Meeting Notes --content \"AI summary of the discussion\" --update-tags '[\"AI-GENERATED\"]'" - "logseq upsert block --graph my-graph --blocks '[{:block/title \"A\"} {:block/title \"B\"}]'" - "logseq upsert block --graph my-graph --id 123 --status done"]}) + "logseq upsert block --graph my-graph --blocks '[{:block/title \"A\"} {:block/title \"B\"}]'"]}) (core/command-entry ["upsert" "page"] :upsert-page "Upsert page" upsert-page-spec {:examples ["logseq upsert page --graph my-graph --page Home --update-tags '[\"project\"]'" "logseq upsert page --graph my-graph --id 999 --update-properties '{:logseq.property/description \"Example\"}'"]}) @@ -379,7 +373,7 @@ :error {:code :invalid-options :message invalid-message}} - (and status-provided? (not status)) + (and status-provided? (not (seq status-text))) {:ok? false :error {:code :invalid-options :message (str "invalid status: " (:status options))}} @@ -426,6 +420,7 @@ (some? id) (assoc :id id) (seq uuid) (assoc :uuid uuid) (seq page) (assoc :page page) + (seq status-text) (assoc :status-input status-text) (and (seq content) (not= mode :page)) (assoc :content content))})))) (defn build-tag-action @@ -863,34 +858,65 @@ :error {:code (or (get-in (ex-data e) [:code]) :exception) :message (or (ex-message e) (str e))}})))) +(defn- normalize-status-input + [value] + (when (some? value) + (let [text (string/trim (if (string? value) value (str value)))] + (when (seq text) + text)))) + +(defn- resolve-task-status-action + [action cfg] + (let [status-input (or (normalize-status-input (:status-input action)) + (normalize-status-input (:status action)))] + (if (seq status-input) + (p/let [available-statuses (transport/invoke cfg :thread-api/q false + [(:repo action) + [task-status-command/status-closed-values-query]]) + resolved-status (task-status-command/resolve-status-ident status-input available-statuses)] + (if resolved-status + {:ok? true + :action (-> action + (assoc :status resolved-status) + (dissoc :status-input))} + {:ok? false + :error {:code :invalid-options + :message (task-status-command/invalid-status-message status-input available-statuses)}})) + (p/resolved {:ok? true :action action})))) + (defn execute-upsert-task [action config] - (-> (p/let [cfg (cli-server/ensure-server! config (:repo action))] - (case (:mode action) - :create - (p/let [result (add-command/execute-add-block (assoc action :type :add-block) config) - created-ids (vec (or (get-in result [:data :result]) [])) - _ (execute-upsert-task-ops! action cfg created-ids)] - {:status :ok - :data {:result created-ids}}) - - :page - (p/let [page (ensure-page-entity! cfg (:repo action) (:page action)) - page-id (:db/id page) - _ (execute-upsert-task-ops! action cfg [page-id])] - {:status :ok - :data {:result [page-id]}}) - - :update - (p/let [entity (ensure-task-node! cfg (:repo action) action) - node-id (:db/id entity) - _ (execute-upsert-task-ops! action cfg [node-id])] - {:status :ok - :data {:result [node-id]}}) - + (-> (p/let [cfg (cli-server/ensure-server! config (:repo action)) + status-check (resolve-task-status-action action cfg)] + (if-not (:ok? status-check) {:status :error - :error {:code :invalid-options - :message "invalid upsert task mode"}})) + :error (:error status-check)} + (let [action* (:action status-check)] + (case (:mode action*) + :create + (p/let [result (add-command/execute-add-block (assoc action* :type :add-block) config) + created-ids (vec (or (get-in result [:data :result]) [])) + _ (execute-upsert-task-ops! action* cfg created-ids)] + {:status :ok + :data {:result created-ids}}) + + :page + (p/let [page (ensure-page-entity! cfg (:repo action*) (:page action*)) + page-id (:db/id page) + _ (execute-upsert-task-ops! action* cfg [page-id])] + {:status :ok + :data {:result [page-id]}}) + + :update + (p/let [entity (ensure-task-node! cfg (:repo action*) action*) + node-id (:db/id entity) + _ (execute-upsert-task-ops! action* cfg [node-id])] + {:status :ok + :data {:result [node-id]}}) + + {:status :error + :error {:code :invalid-options + :message "invalid upsert task mode"}})))) (p/catch (fn [e] {:status :error :error {:code (or (get-in (ex-data e) [:code]) :exception) diff --git a/src/main/logseq/cli/commands.cljs b/src/main/logseq/cli/commands.cljs index 03e241fdb1..908558d517 100644 --- a/src/main/logseq/cli/commands.cljs +++ b/src/main/logseq/cli/commands.cljs @@ -181,6 +181,10 @@ (re-find #"Unknown option:\s*:properties" (or message ""))) "unknown option: --properties; use --update-properties" + (and (= ["upsert" "block"] subcommand) + (re-find #"Unknown option:\s*:status" (or message ""))) + "unknown option: --status; use upsert task --status" + (and (= ["upsert" "page"] subcommand) (re-find #"Unknown option:\s*:tags" (or message ""))) "unknown option: --tags; use --update-tags" diff --git a/src/test/logseq/cli/command/upsert_test.cljs b/src/test/logseq/cli/command/upsert_test.cljs index 61e1f2c2eb..2ea160f7d0 100644 --- a/src/test/logseq/cli/command/upsert_test.cljs +++ b/src/test/logseq/cli/command/upsert_test.cljs @@ -92,6 +92,11 @@ add-command/resolve-property-identifiers (fn [_ _ _ _] (p/resolved [])) transport/invoke (fn [_ method _ args] (case method + :thread-api/q + (p/resolved [:logseq.property/status.todo + :logseq.property/status.doing + :logseq.property/status.done]) + :thread-api/pull (let [[_ selector lookup] args] (cond diff --git a/src/test/logseq/cli/commands_test.cljs b/src/test/logseq/cli/commands_test.cljs index 5b2e905047..62fb81497a 100644 --- a/src/test/logseq/cli/commands_test.cljs +++ b/src/test/logseq/cli/commands_test.cljs @@ -1092,7 +1092,13 @@ (is (= 10 (get-in result [:options :limit]))) (is (= 2 (get-in result [:options :offset]))) (is (= "priority" (get-in result [:options :sort]))) - (is (= "desc" (get-in result [:options :order])))))) + (is (= "desc" (get-in result [:options :order]))))) + + (testing "list task supports short -c alias for --content" + (let [result (commands/parse-args ["list" "task" "-c" "alpha"])] + (is (true? (:ok? result))) + (is (= :list-task (:command result))) + (is (= "alpha" (get-in result [:options :content])))))) (deftest test-search-subcommand-parse (testing "search block parses --content option" @@ -1181,7 +1187,13 @@ (testing "list task rejects invalid priority" (let [result (commands/parse-args ["list" "task" "--priority" "wat"])] (is (false? (:ok? result))) - (is (= :invalid-options (get-in result [:error :code])))))) + (is (= :invalid-options (get-in result [:error :code]))))) + + (testing "list task defers unknown --status to runtime validation" + (let [result (commands/parse-args ["list" "task" "--status" "custom-review"])] + (is (true? (:ok? result))) + (is (= :list-task (:command result))) + (is (= "custom-review" (get-in result [:options :status])))))) (deftest test-list-execute-default-sort-updated-at (async done @@ -1255,6 +1267,67 @@ (is false (str "unexpected error: " e)))) (p/finally done)))) +(deftest test-task-runtime-invalid-status-includes-graph-values + (async done + (let [list-calls* (atom []) + upsert-calls* (atom [])] + (-> (p/with-redefs [cli-server/ensure-server! (fn [_ _] {:base-url "http://example"}) + transport/invoke (fn [_ method _ args] + (let [repo (first args)] + (cond + (= method :thread-api/q) + (do + (if (= repo "demo") + (swap! list-calls* conj method) + (swap! upsert-calls* conj method)) + [:logseq.property/status.todo + :logseq.property/status.done + :logseq.property/status.doing]) + + (= method :thread-api/cli-list-tasks) + (do + (swap! list-calls* conj method) + []) + + (= method :thread-api/pull) + (do + (swap! upsert-calls* conj method) + {:db/id 1}) + + (= method :thread-api/apply-outliner-ops) + (do + (swap! upsert-calls* conj method) + {:result :ok}) + + :else + (throw (ex-info "unexpected invoke" {:method method :args args})))))] + (p/let [list-result (list-command/execute-list-task + {:repo "demo" + :options {:status "invalid-status"}} + {}) + upsert-result (upsert-command/execute-upsert-task + {:repo "upsert-demo" + :mode :update + :id 1 + :status "invalid-status"} + {}) + list-message (or (some-> (get-in list-result [:error :message]) strip-ansi) "") + upsert-message (or (some-> (get-in upsert-result [:error :message]) strip-ansi) "")] + (is (= :error (:status list-result))) + (is (= :invalid-options (get-in list-result [:error :code]))) + (is (string/includes? list-message "Invalid value for option :status: invalid-status")) + (is (string/includes? list-message "Available values (from current graph): doing, done, todo")) + (is (= [:thread-api/q] @list-calls*)) + + (is (= :error (:status upsert-result))) + (is (= :invalid-options (get-in upsert-result [:error :code]))) + (is (string/includes? upsert-message "Invalid value for option :status: invalid-status")) + (is (string/includes? upsert-message "Available values (from current graph): doing, done, todo")) + (is (= [:thread-api/q] @upsert-calls*)))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)))) + (p/finally done))))) + (deftest test-verb-subcommand-parse-upsert-remove (testing "remove block parses with id" (let [result (commands/parse-args ["remove" "block" "--id" "10"])] @@ -1389,13 +1462,14 @@ (is (= :upsert-block (:command result))) (is (= "11111111-1111-1111-1111-111111111111" (get-in result [:options :uuid]))))) - (testing "upsert block update mode accepts status-only updates" + (testing "upsert block rejects --status and guides migration to upsert task" (let [result (commands/parse-args ["upsert" "block" "--id" "1" - "--status" "done"])] - (is (true? (:ok? result))) - (is (= :upsert-block (:command result))) - (is (= 1 (get-in result [:options :id]))) - (is (= "done" (get-in result [:options :status]))))) + "--status" "done"]) + message (strip-ansi (get-in result [:error :message]))] + (is (false? (:ok? result))) + (is (= :invalid-options (get-in result [:error :code]))) + (is (string/includes? message "--status")) + (is (string/includes? message "upsert task")))) (testing "upsert block update mode accepts content-only updates" (let [result (commands/parse-args ["upsert" "block" "--id" "1" @@ -1563,6 +1637,14 @@ (is (= "done" (get-in result [:options :status]))) (is (= "medium" (get-in result [:options :priority]))))) + (testing "upsert task defers unknown --status to runtime validation" + (let [result (commands/parse-args ["upsert" "task" + "--id" "42" + "--status" "custom-review"])] + (is (true? (:ok? result))) + (is (= :upsert-task (:command result))) + (is (= "custom-review" (get-in result [:options :status]))))) + (testing "upsert task requires selector, page, or content" (let [result (commands/parse-args ["upsert" "task"])] (is (false? (:ok? result))) diff --git a/src/test/logseq/cli/completion_generator_test.cljs b/src/test/logseq/cli/completion_generator_test.cljs index 4d9ac57af3..3fa4041511 100644 --- a/src/test/logseq/cli/completion_generator_test.cljs +++ b/src/test/logseq/cli/completion_generator_test.cljs @@ -54,7 +54,8 @@ (let [entries list-command/entries page-entry (first (filter #(= :list-page (:command %)) entries)) tag-entry (first (filter #(= :list-tag (:command %)) entries)) - property-entry (first (filter #(= :list-property (:command %)) entries))] + property-entry (first (filter #(= :list-property (:command %)) entries)) + task-entry (first (filter #(= :list-task (:command %)) entries))] (testing "page-spec :sort has some correct values" (is (contains? (get-in page-entry [:spec :sort :validate]) "title"))) (testing "tag-spec :sort has some correct values" @@ -68,7 +69,9 @@ (let [mv (get-in tag-entry [:spec :fields :multiple-values])] (is (seq mv)) (is (some #{"title"} mv)) - (is (some #{"uuid"} mv)))))) + (is (some #{"uuid"} mv)))) + (testing "list task :content has -c alias" + (is (= :c (get-in task-entry [:spec :content :alias])))))) (deftest test-upsert-spec-metadata (let [entries upsert-command/entries @@ -79,8 +82,8 @@ (testing "block-spec :pos has :validate set" (is (= #{"first-child" "last-child" "sibling"} (get-in block-entry [:spec :pos :validate])))) - (testing "block-spec :status has :validate set" - (is (seq (get-in block-entry [:spec :status :validate])))) + (testing "block-spec does not expose :status option" + (is (nil? (get-in block-entry [:spec :status])))) (testing "block-spec :target-page has :complete :pages" (is (= :pages (get-in block-entry [:spec :target-page :complete])))) (testing "block-spec :blocks-file has :complete :file" @@ -275,7 +278,8 @@ (is (not (string/includes? output "-c[Path to cli.edn (default ~/logseq/cli.edn)]:file:_files'")))) (testing "-c is available as content alias in command-specific completion" (is (re-find #"(?s)_logseq_search_block\(\).*?-c\[Search content text\]" output)) - (is (re-find #"(?s)_logseq_upsert_block\(\).*?-c\[Block content" output))) + (is (re-find #"(?s)_logseq_upsert_block\(\).*?-c\[Block content" output)) + (is (re-find #"(?s)_logseq_list_task\(\).*?-c\[Filter by task title content\]" output))) (testing ":alias emits grouping without --no- for global flags" (is (re-find #"\(-h --help\)" output)))))