diff --git a/docs/agent-guide/logseq-cli/010-skill-update-warning.md b/docs/agent-guide/logseq-cli/010-skill-update-warning.md new file mode 100644 index 0000000000..2fea01e6d8 --- /dev/null +++ b/docs/agent-guide/logseq-cli/010-skill-update-warning.md @@ -0,0 +1,723 @@ +# Logseq CLI Skill Update Warning Implementation Plan + +Goal: When Logseq CLI prints the global help information, check whether the user already has an installed `logseq-cli` agent skill. If an installed skill file exists and its content differs from the built-in skill bundled with the current CLI, append a warning to the end of the global help output. + +Architecture: Keep this feature entirely in the Logseq CLI help path and existing skill command support. Do not involve graph commands, db-worker-node lifecycle, or db-worker `:thread-api` calls. + +Architecture: Reuse the existing built-in skill source resolution and install-target conventions from `logseq.cli.command.skill`, which currently supports `logseq skill show`, `logseq skill install`, and `logseq skill install --global`. + +Architecture: Append the warning only for global help output: `logseq`, `logseq --help`, `logseq -h`, and equivalent leading-global-option forms such as `logseq --output json --help`. Do not append it to command help such as `logseq show --help`, group help such as `logseq graph`, parse errors, version output, or normal command output. + +Architecture: Treat the warning as a local filesystem comparison. The CLI should compare UTF-8 skill markdown content exactly. No network request, no graph open, no db-worker-node start, and no new `:thread-api/*` method are required. + +Tech Stack: ClojureScript, Node `fs`/`os`/`path` APIs, `babashka.cli`, existing Logseq CLI command parsing, existing `logseq.cli.command.skill`, `logseq.cli.main`, CLI unit tests, and the Logseq review workflow. + +Related: Builds on the current implementation in `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/main.cljs`, `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/commands.cljs`, `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/core.cljs`, `/Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/skill.cljs`, and the db-worker-node boundary in `/Users/rcmerci/gh-repos/logseq/src/main/frontend/worker/db_worker_node.cljs`. + +## Problem statement + +The Logseq CLI ships a built-in agent skill at: + +```text +.agents/skills/logseq-cli/SKILL.md +``` + +The CLI exposes this built-in skill through: + +```text +logseq skill show +logseq skill install +logseq skill install --global +``` + +`logseq skill install` copies the bundled skill into the current working directory under: + +```text +/.agents/skills/logseq-cli/SKILL.md +``` + +`logseq skill install --global` copies it into the user's home directory under: + +```text +~/.agents/skills/logseq-cli/SKILL.md +``` + +After the CLI is upgraded, a user may still have an older installed copy of the `logseq-cli` skill. Agents that use the installed skill can then follow stale command guidance even though the CLI binary has newer behavior. + +The requested behavior is: + +1. When showing global help, also check whether the installed `logseq-cli` skill is up to date. +2. If the user has not installed the skill, show no warning. +3. If the user has installed the skill and the installed content differs from the bundled content, append a warning to the end of the help info. +4. Avoid adding a new db-worker `:thread-api` unless implementation work proves it is absolutely necessary. +5. After implementation, review the finished change with `logseq-review-workflow`. + +This feature is a CLI self-maintenance hint. It should not change graph data, command behavior, db-worker-node behavior, or skill installation behavior. + +## Current implementation snapshot + +### Global help generation + +Global help is generated in: + +```text +src/main/logseq/cli/command/core.cljs +``` + +The key function is: + +```text +logseq.cli.command.core/top-level-summary +``` + +It returns a markdown-like/plain-text help string containing: + +- usage line +- grouped commands +- global options +- command options hint + +Command parsing lives in: + +```text +src/main/logseq/cli/commands.cljs +``` + +`logseq.cli.commands/parse-args` builds the top-level summary before dispatching: + +```text +(let [summary (command-core/top-level-summary table) + {:keys [opts args]} (command-core/parse-leading-global-opts raw-args) + ...] + ...) +``` + +Global help is returned by `command-core/help-result` in at least these cases: + +- `logseq` +- `logseq --help` +- `logseq -h` +- exhausted input that falls back to the top-level summary + +Group help and command help also use `help-result`, but with different summary strings. + +### CLI run path + +The entrypoint is: + +```text +src/main/logseq/cli/main.cljs +``` + +`logseq.cli.main/run!` parses args first: + +```text +(commands/parse-args args) +``` + +If `(:help? parsed)` is true, it returns immediately without resolving config, ensuring root dir, building command actions, starting db-worker-node, or invoking graph APIs. + +Current human help output path: + +```text +{:exit-code 0 + :output (:summary parsed)} +``` + +Current structured help output path: + +```text +(format/format-result {:status :ok + :data {:message (:summary parsed)}} + {:output-format mode}) +``` + +This means the right place to append a dynamic warning is in `logseq.cli.main/run!` after parsing and before formatting the help result. That keeps `command.core/top-level-summary` deterministic and avoids a namespace cycle between `command.core` and `command.skill`. + +### Skill command implementation + +The skill utility command lives in: + +```text +src/main/logseq/cli/command/skill.cljs +``` + +Current constants: + +```text +skill-dir-name "logseq-cli" +skill-file-name "SKILL.md" +relative-skill-path [".agents" "skills" "logseq-cli" "SKILL.md"] +``` + +Current public helpers include: + +```text +resolve-install-target +resolve-skill-source-path +``` + +Current private helpers include: + +```text +source-path-candidates +resolve-action-source-path +resolve-install-destination +``` + +Current command behavior: + +- `skill show` resolves the bundled source skill and prints it. +- `skill install` resolves the bundled source skill, resolves a destination under `/.agents/skills/logseq-cli/SKILL.md`, and writes the source payload. +- `skill install --global` resolves the destination under `~/.agents/skills/logseq-cli/SKILL.md`. + +This code already knows the built-in source path and supported install destinations. The implementation should extend this namespace with reusable status helpers instead of duplicating path logic in `main.cljs`. + +### db-worker-node boundary + +The db-worker-node server lives in: + +```text +src/main/frontend/worker/db_worker_node.cljs +``` + +It exposes HTTP endpoints such as: + +```text +/healthz +/v1/events +/v1/invoke +/v1/shutdown +``` + +`/v1/invoke` forwards existing `:thread-api/*` calls to the worker runtime after repo validation and write-lock checks. + +Global CLI help currently exits before config resolution and before any db-worker-node lifecycle path. The skill update warning only needs to compare local files, so it should preserve that boundary: + +- no graph selection +- no root-dir creation +- no server discovery +- no worker start +- no `/v1/invoke` +- no new `:thread-api/*` + +If an implementation attempt appears to require db-worker-node, re-check the design. That would likely mean the check has accidentally been coupled to graph/runtime state instead of the filesystem skill files. + +## Desired behavior + +### No installed skill + +If neither supported installed skill file exists, global help should be unchanged. + +Supported installed locations should match the existing install command semantics: + +```text +/.agents/skills/logseq-cli/SKILL.md +~/.agents/skills/logseq-cli/SKILL.md +``` + +Examples that should not show a warning when no installed file exists: + +```text +logseq +logseq --help +logseq -h +logseq --output json --help +``` + +### Installed skill matches bundled skill + +If an installed skill file exists and its UTF-8 content exactly equals the bundled skill content, global help should be unchanged. + +This preserves a quiet help experience for users who already updated their installed skill. + +### Installed skill differs from bundled skill + +If one or more installed skill files exist and at least one installed file differs from the bundled skill content, append a warning to the end of the global help information. + +Recommended warning text: + +```text + +Warning: Installed logseq-cli skill is out of date. Run `logseq skill install` or `logseq skill install --global` to update it. +``` + +If implementation can safely identify stale scopes, prefer a more specific warning: + +```text + +Warning: Installed logseq-cli skill is out of date at . Run `logseq skill install` to update it. +``` + +or for global scope: + +```text + +Warning: Installed logseq-cli skill is out of date at . Run `logseq skill install --global` to update it. +``` + +If both local and global installed files exist and either differs, a single concise warning is enough. The warning may include both update commands, but it should remain at the end of help and avoid turning global help into a long diagnostic report. + +### Scope of global help + +Append the warning for top-level/global help only: + +```text +logseq +logseq --help +logseq -h +logseq --output human --help +logseq --output json --help +logseq --output edn --help +``` + +Do not append the warning for group help: + +```text +logseq graph +logseq graph --help +logseq list +logseq example +``` + +Do not append the warning for command help: + +```text +logseq show --help +logseq skill install --help +logseq query --help +``` + +Do not append the warning for non-help output: + +```text +logseq --version +logseq skill show +logseq skill install +logseq graph list +``` + +### Structured output + +Structured help modes currently wrap the help string in `:data :message`. + +When global help is requested with structured output, append the same warning to that message field: + +```text +logseq --output json --help +logseq --output edn --help +``` + +Do not add a separate structured field unless there is a strong reason. Keeping the warning in `message` preserves the existing help-result shape and satisfies the requirement that the warning appears at the end of the help info. + +## Design + +### 1. Add skill status helpers in `logseq.cli.command.skill` + +Add public, unit-testable helpers to `src/main/logseq/cli/command/skill.cljs`. + +Suggested API: + +```clojure +(defn installed-skill-targets + [{:keys [cwd home-dir]}] + ...) + +(defn installed-skill-update-status + [{:keys [cwd home-dir source-path]}] + ...) + +(defn format-installed-skill-warning + [status] + ...) +``` + +The exact names can change during implementation, but the responsibilities should remain separate: + +1. Resolve candidate installed paths. +2. Resolve/read the bundled source skill. +3. Read only installed files that actually exist. +4. Compare content. +5. Return data describing whether a warning is needed. +6. Format the user-facing warning in one place. + +Suggested status shape: + +```clojure +{:installed? true + :outdated? true + :outdated-targets [{:scope :local + :path "/work/.agents/skills/logseq-cli/SKILL.md" + :update-command "logseq skill install"}]} +``` + +For no installed skill: + +```clojure +{:installed? false + :outdated? false + :outdated-targets []} +``` + +For installed and current: + +```clojure +{:installed? true + :outdated? false + :outdated-targets []} +``` + +For source resolution/read failures, return a typed error instead of throwing from the status helper: + +```clojure +{:installed? true + :outdated? false + :error {:code :skill-source-not-found ...}} +``` + +The top-level help path can then avoid showing a false warning if comparison cannot be completed. Unit tests should still assert the typed error behavior so packaging/path bugs are visible to developers. + +Implementation notes: + +- Keep exact UTF-8 string equality as the freshness check. +- Do not normalize whitespace or line endings unless tests prove packaging changes introduce platform-only newline differences. +- Use `fs/existsSync` before reading installed files. +- Read source once. +- Reuse `resolve-skill-source-path` and the existing candidate logic. +- Reuse `resolve-install-target` for local/global paths where practical. +- Preserve existing `skill show` and `skill install` behavior. + +### 2. Detect global help in `logseq.cli.main` + +Add a small helper in `src/main/logseq/cli/main.cljs` to identify top-level help from the original argv and parse result. + +Suggested behavior: + +```text +global-help-info? = parsed is help AND leading global option parsing leaves no command args +``` + +This matches: + +- `[]` +- `["--help"]` +- `["-h"]` +- `["--output" "json" "--help"]` + +It does not match command or group help because those have command args after leading global options. + +Suggested helper responsibilities: + +```clojure +(defn- global-help-info? + [args parsed] + ...) +``` + +Use existing `command-core/parse-leading-global-opts` to avoid introducing a second argv parser. + +Do not add dynamic filesystem checks to `command-core/top-level-summary`. Keeping the check in `main.cljs` avoids making parse-only tests depend on local user files and prevents a circular dependency with `logseq.cli.command.skill`. + +### 3. Append warning before formatting help + +In the `(:help? parsed)` branch of `logseq.cli.main/run!`: + +1. Resolve output mode as today. +2. Compute `summary` from `(:summary parsed)`. +3. If `global-help-info?` is true, call the skill status helper and append the formatted warning when needed. +4. Use the final summary for both human and structured output. + +Suggested flow: + +```clojure +(let [mode (resolve-output-format args parsed nil nil) + summary (maybe-append-installed-skill-warning args parsed (:summary parsed))] + ...) +``` + +Keep the function synchronous unless implementation work finds a strong reason otherwise. The comparison reads two or three local files at most and happens only when global help is requested. + +### 4. Warning formatting + +Keep the warning concise, stable, and easy to test. + +Recommended baseline: + +```text +Warning: Installed logseq-cli skill is out of date. Run `logseq skill install` or `logseq skill install --global` to update it. +``` + +Optional scope-specific rendering: + +```text +Warning: Installed logseq-cli skill is out of date at . Run `` to update it. +``` + +If there are multiple stale installed targets, either: + +- render one generic warning with both install commands, or +- render one warning line with comma-separated paths. + +Do not render a multi-paragraph report. + +Do not warn when the installed file is missing. Missing means the user has not installed the skill through the supported install location, and the requirement explicitly says not to show a warning in that case. + +### 5. Avoid db-worker-node and new thread APIs + +This feature should be implemented without changing: + +```text +src/main/frontend/worker/db_worker_node.cljs +src/main/logseq/cli/server.cljs +src/main/logseq/cli/transport.cljs +``` + +Do not add a new `:thread-api/*` for skill status. The information is already available from local files in the CLI process. + +If a future implementation wants to expose skill status to a graph-aware command, prefer a normal CLI helper or command action first. A db-worker thread API would only be justified if the worker owns the data or side effect, which is not true here. + +## Implementation tasks + +### Task 1: Add skill content status helpers + +Files: + +```text +src/main/logseq/cli/command/skill.cljs +src/test/logseq/cli/command/skill_test.cljs +``` + +Steps: + +1. Add a helper to resolve local and global installed skill targets using existing install semantics. +2. Add a helper to resolve and read the built-in source skill. +3. Add a helper to read installed skill files only when they exist. +4. Compare installed content with built-in content. +5. Return status data that distinguishes: + - no installed skill + - installed and current + - installed and outdated + - comparison error +6. Add unit tests for each status case. + +Acceptance criteria: + +- No existing `skill show` or `skill install` tests regress. +- No installed file means `:installed? false` and no warning. +- Matching installed file means no warning. +- Different installed file means `:outdated? true`. +- Local and global install paths match current install command semantics. + +### Task 2: Append warning only for global help + +Files: + +```text +src/main/logseq/cli/main.cljs +src/test/logseq/cli/main_test.cljs +``` + +Steps: + +1. Add a private global-help detector based on raw args and parse result. +2. Add a private warning append helper that calls `skill-command/installed-skill-update-status`. +3. In the `(:help? parsed)` branch, replace direct use of `(:summary parsed)` with the possibly augmented summary. +4. Add tests with `p/with-redefs` or temporary files so the tests do not depend on the developer's real `~/.agents` files. +5. Cover human, JSON, and EDN help modes if feasible. + +Acceptance criteria: + +- `logseq --help` appends the warning when the mocked/temporary installed skill is stale. +- `logseq` with no args follows the same global help behavior. +- `logseq show --help` does not append the warning. +- `logseq graph` group help does not append the warning. +- `logseq --version` does not append the warning. +- `logseq --output json --help` keeps the warning inside `data.message` and preserves the existing structured result shape. + +### Task 3: Keep parsing deterministic + +Files: + +```text +src/main/logseq/cli/commands.cljs +src/main/logseq/cli/command/core.cljs +src/test/logseq/cli/commands_test.cljs +``` + +Expected change: ideally none. + +If implementation needs parse metadata, prefer adding metadata in `commands.cljs` rather than adding filesystem checks to `command.core.cljs`. + +Acceptance criteria: + +- `commands/parse-args` remains deterministic and does not read user skill files. +- Existing help output tests in `commands_test.cljs` remain stable. +- Any new metadata does not change the public command/action behavior unless explicitly tested. + +### Task 4: Verify no db-worker-node changes are needed + +Files: + +```text +src/main/frontend/worker/db_worker_node.cljs +src/main/logseq/cli/server.cljs +src/main/logseq/cli/transport.cljs +``` + +Expected change: none. + +Acceptance criteria: + +- No new `:thread-api/*` keyword is introduced for this feature. +- Global help still returns before config/root-dir/db-worker work. +- Existing db-worker-node tests are not affected. + +### Task 5: Review with `logseq-review-workflow` + +After implementation and tests, run the Logseq review workflow before considering the feature complete. + +Scope to review: + +```text +src/main/logseq/cli/main.cljs +src/main/logseq/cli/command/skill.cljs +src/test/logseq/cli/main_test.cljs +src/test/logseq/cli/command/skill_test.cljs +``` + +Likely review rule modules: + +- `.agents/skills/logseq-review-workflow/rules/common.md` +- `.agents/skills/logseq-review-workflow/rules/libraries/clojure-cljs.md` +- `.agents/skills/logseq-review-workflow/rules/libraries/babashka-cli.md` +- `.agents/skills/logseq-review-workflow/rules/libraries/shadow-cljs-node.md` +- `.agents/skills/logseq-review-workflow/rules/modules/logseq-cli.md` + +Review acceptance criteria: + +- The workflow is explicitly applied after implementation. +- Any findings are fixed or documented before handoff. +- The final response mentions the review scope and whether findings remain. + +## Test plan + +Targeted unit tests: + +```text +bb dev:test -v logseq.cli.command.skill-test/test-installed-skill-update-status +bb dev:test -v logseq.cli.main-test/test-global-help-appends-stale-skill-warning +bb dev:test -v logseq.cli.main-test/test-global-help-skill-warning-structured-output +``` + +Existing related tests to keep green: + +```text +bb dev:test -v logseq.cli.command.skill-test/test-execute-skill-show +bb dev:test -v logseq.cli.command.skill-test/test-execute-skill-install +bb dev:test -v logseq.cli.commands-test/test-help-output +bb dev:test -v logseq.cli.main-test/test-help-output-respects-structured-modes +``` + +Broader verification before submitting: + +```text +bb dev:lint-and-test +``` + +Manual checks, if a built CLI is available: + +```text +logseq --help +logseq --output json --help +logseq skill install +logseq --help +# then modify the installed SKILL.md or switch to a newer build +logseq --help +logseq show --help +``` + +Expected manual results: + +- No installed skill: no warning. +- Installed current skill: no warning. +- Installed stale skill: warning appears at the end of global help. +- Command help and group help: no warning. + +## Edge cases and decisions + +### Local vs global installed skill + +The CLI currently supports both local and global installation. The warning check should inspect both supported destinations. + +If neither exists, do not warn. + +If one exists and differs, warn. + +If both exist and either differs, warn once. The warning can mention both install commands unless scope-specific formatting is implemented. + +### User-edited installed skill + +A user may intentionally customize their installed `logseq-cli` skill. Exact content comparison will treat that as different and show an update warning. + +This is acceptable for the first implementation because there is no skill version metadata today. The warning says the installed skill differs from the bundled current skill; it does not overwrite anything. + +Do not introduce a skill metadata format or migration in this task unless explicitly requested. + +### Missing source skill + +If the bundled source skill cannot be resolved/read, that is a packaging or runtime bug. The status helper should return a typed error that unit tests can cover. + +The global help path should not show an update warning unless comparison succeeds and proves that an installed skill is outdated. This avoids false positives in help output. + +### File permissions + +If an installed skill file exists but cannot be read, return a typed comparison error from the helper. Do not claim the skill is outdated unless content comparison succeeds. + +If future work wants to expose this to users, add a separate diagnostic command or a verbose-only diagnostic. Do not add noisy errors to normal global help in this task. + +### Color/styling + +Existing help uses `logseq.cli.style` for bold headings/options. The warning can be plain text for test stability. + +If styling is added, ensure tests strip ANSI where needed and the warning remains readable when color is disabled. + +### Performance + +The global help path should read at most: + +- the bundled source skill once +- the local installed skill if it exists +- the global installed skill if it exists + +No recursive directory scan is needed. + +### Compatibility + +This change does not need backward compatibility shims. It adds a warning only when a supported installed skill file exists and differs from the bundled content. + +Do not add default values that mask invalid state inside the status helper. Return explicit status data instead. + +## Non-goals + +- Do not automatically reinstall or overwrite the user's skill. +- Do not add a new `logseq skill status` command in this task. +- Do not add a new db-worker `:thread-api/*`. +- Do not start db-worker-node during help. +- Do not check remote versions or network state. +- Do not add skill version metadata unless requested separately. +- Do not show the warning for users who have not installed the skill. + +## Final acceptance criteria + +The implementation is complete when all of the following are true: + +1. Global help checks installed `logseq-cli` skill freshness using local filesystem content comparison. +2. No warning is shown when no installed skill exists. +3. No warning is shown when installed skill content matches the bundled skill. +4. A warning is appended to the end of global help when an installed skill differs from the bundled skill. +5. The warning appears in structured help inside the existing help message field. +6. Group help, command help, version output, and normal command output do not show the warning. +7. No new `:thread-api/*` is added. +8. db-worker-node is not started or invoked for global help. +9. Targeted CLI tests pass. +10. `logseq-review-workflow` has been applied to the final implementation and any findings have been addressed or documented. diff --git a/src/main/logseq/cli/command/skill.cljs b/src/main/logseq/cli/command/skill.cljs index a6d3d312ed..52850cf905 100644 --- a/src/main/logseq/cli/command/skill.cljs +++ b/src/main/logseq/cli/command/skill.cljs @@ -72,6 +72,102 @@ (from-dir (node-path/join js/__dirname ".." "..")) (from-dir (node-path/join js/__dirname ".." ".." ".."))])) +(defn installed-skill-targets + [{:keys [cwd home-dir]}] + (let [env-home (or (some-> js/process .-env .-HOME) + (some-> js/process .-env .-USERPROFILE)) + home-dir (or home-dir env-home (.homedir os))] + (->> [{:scope :local + :target (resolve-install-target {:global? false + :cwd cwd + :home-dir home-dir}) + :update-command "logseq skill install"} + {:scope :global + :target (resolve-install-target {:global? true + :cwd cwd + :home-dir home-dir}) + :update-command "logseq skill install --global"}] + (keep (fn [{:keys [scope target update-command]}] + (when (:ok? target) + {:scope scope + :path (:file target) + :update-command update-command}))) + vec))) + +(defn- resolve-status-source-path + [{:keys [source-path source-candidates]}] + (if-let [source-path (some-> source-path str string/trim not-empty)] + {:ok? true :path source-path} + (resolve-skill-source-path (or source-candidates (source-path-candidates))))) + +(defn- read-skill-source + [options] + (let [{:keys [ok? path error]} (resolve-status-source-path options)] + (if-not ok? + {:ok? false + :error error} + (try + {:ok? true + :path path + :content (fs/readFileSync path "utf8")} + (catch :default e + {:ok? false + :error {:code :skill-source-read-failed + :message (str "failed to read skill source: " (or (.-message e) e)) + :source-path path + :cause (.-code e)}}))))) + +(defn- read-installed-skill + [{:keys [path] :as target}] + (try + {:ok? true + :target target + :content (fs/readFileSync path "utf8")} + (catch :default e + {:ok? false + :error {:code :skill-installed-read-failed + :message (str "failed to read installed skill: " (or (.-message e) e)) + :path path + :cause (.-code e)}}))) + +(defn installed-skill-update-status + [options] + (let [targets (installed-skill-targets options) + existing-targets (->> targets + (filter #(fs/existsSync (:path %))) + vec)] + (if-not (seq existing-targets) + {:installed? false + :outdated? false + :outdated-targets []} + (let [{:keys [ok? content error]} (read-skill-source options)] + (if-not ok? + {:installed? true + :outdated? false + :outdated-targets [] + :error error} + (let [source-content content + installed-results (mapv read-installed-skill existing-targets) + failed (first (remove :ok? installed-results))] + (if failed + {:installed? true + :outdated? false + :outdated-targets [] + :error (:error failed)} + (let [outdated-targets (->> installed-results + (keep (fn [{:keys [target content]}] + (when (not= content source-content) + target))) + vec)] + {:installed? true + :outdated? (boolean (seq outdated-targets)) + :outdated-targets outdated-targets})))))))) + +(defn format-installed-skill-warning + [status] + (when (:outdated? status) + "\n\nWarning: Installed logseq-cli skill is out of date. Run `logseq skill install` or `logseq skill install --global` to update it.")) + (defn- resolve-action-source-path [action] (if-let [source-path (some-> (:source-path action) str string/trim not-empty)] diff --git a/src/main/logseq/cli/main.cljs b/src/main/logseq/cli/main.cljs index 35a4c58b7b..def71700b5 100644 --- a/src/main/logseq/cli/main.cljs +++ b/src/main/logseq/cli/main.cljs @@ -3,6 +3,7 @@ (:refer-clojure :exclude [run!]) (:require [lambdaisland.glogi :as log] [logseq.cli.command.core :as command-core] + [logseq.cli.command.skill :as skill-command] [logseq.cli.commands :as commands] [logseq.cli.config :as config] [logseq.cli.format :as format] @@ -59,6 +60,20 @@ (parsed-output-format parsed) (parse-argv-output-format args))) +(defn- global-help-info? + [args parsed] + (and (:help? parsed) + (empty? (:args (command-core/parse-leading-global-opts args))))) + +(defn- maybe-append-installed-skill-warning + [args parsed summary] + (if-not (global-help-info? args parsed) + summary + (let [warning (-> (skill-command/installed-skill-update-status {}) + skill-command/format-installed-skill-warning)] + (cond-> summary + (seq warning) (str warning))))) + (defn- format-opts [args parsed cfg result] (if-let [mode (resolve-output-format args parsed cfg result)] @@ -106,7 +121,8 @@ (cond (:help? parsed) (p/resolved - (let [mode (resolve-output-format args parsed nil nil)] + (let [mode (resolve-output-format args parsed nil nil) + summary (maybe-append-installed-skill-warning args parsed (:summary parsed))] (attach-profile-lines profile-session parsed @@ -115,9 +131,9 @@ (profile/time! profile-session "cli.format-result" (fn [] (format/format-result {:status :ok - :data {:message (:summary parsed)}} + :data {:message summary}} {:output-format mode}))) - (:summary parsed))}))) + summary)}))) (not (:ok? parsed)) (p/resolved diff --git a/src/test/logseq/cli/command/skill_test.cljs b/src/test/logseq/cli/command/skill_test.cljs index 3ae07ab354..d5113cb4b1 100644 --- a/src/test/logseq/cli/command/skill_test.cljs +++ b/src/test/logseq/cli/command/skill_test.cljs @@ -118,6 +118,75 @@ (set! (.-readFileSync fs) read-file-sync) (set! (.-writeFileSync fs) write-file-sync))))) +(deftest test-installed-skill-targets + (let [targets (skill-command/installed-skill-targets {:cwd "/tmp/work" + :home-dir "/Users/demo"})] + (is (= [{:scope :local + :path "/tmp/work/.agents/skills/logseq-cli/SKILL.md" + :update-command "logseq skill install"} + {:scope :global + :path "/Users/demo/.agents/skills/logseq-cli/SKILL.md" + :update-command "logseq skill install --global"}] + targets)))) + +(deftest test-installed-skill-update-status + (let [tmp-root (.mkdtempSync fs (node-path/join (.tmpdir os) "logseq-skill-status-")) + source-path (node-path/join tmp-root "source" "SKILL.md") + cwd (node-path/join tmp-root "work") + home-dir (node-path/join tmp-root "home") + local-path (path/path-join cwd ".agents" "skills" "logseq-cli" "SKILL.md") + global-path (path/path-join home-dir ".agents" "skills" "logseq-cli" "SKILL.md")] + (try + (fs/mkdirSync (node-path/dirname source-path) #js {:recursive true}) + (fs/mkdirSync (node-path/dirname local-path) #js {:recursive true}) + (fs/mkdirSync (node-path/dirname global-path) #js {:recursive true}) + (fs/writeFileSync source-path "# current skill\n" "utf8") + + (testing "reports no installed skill without warning" + (let [status (skill-command/installed-skill-update-status {:cwd cwd + :home-dir home-dir + :source-path source-path})] + (is (= {:installed? false + :outdated? false + :outdated-targets []} + (select-keys status [:installed? :outdated? :outdated-targets]))) + (is (nil? (skill-command/format-installed-skill-warning status))))) + + (testing "reports installed current skill without warning" + (fs/writeFileSync local-path "# current skill\n" "utf8") + (let [status (skill-command/installed-skill-update-status {:cwd cwd + :home-dir home-dir + :source-path source-path})] + (is (true? (:installed? status))) + (is (false? (:outdated? status))) + (is (empty? (:outdated-targets status))) + (is (nil? (skill-command/format-installed-skill-warning status))))) + + (testing "reports stale installed skill and formats one concise warning" + (fs/writeFileSync global-path "# old skill\n" "utf8") + (let [status (skill-command/installed-skill-update-status {:cwd cwd + :home-dir home-dir + :source-path source-path}) + warning (skill-command/format-installed-skill-warning status)] + (is (true? (:installed? status))) + (is (true? (:outdated? status))) + (is (= [{:scope :global + :path global-path + :update-command "logseq skill install --global"}] + (:outdated-targets status))) + (is (string/includes? warning "Warning: Installed logseq-cli skill is out of date.")) + (is (string/includes? warning "logseq skill install")) + (is (string/includes? warning "logseq skill install --global")))) + + (testing "returns typed error when source cannot be read" + (let [status (skill-command/installed-skill-update-status {:cwd cwd + :home-dir home-dir + :source-path (node-path/join tmp-root "missing.md")})] + (is (false? (:outdated? status))) + (is (= :skill-source-read-failed (get-in status [:error :code]))))) + (finally + (fs/rmSync tmp-root #js {:recursive true :force true}))))) + (deftest test-execute-skill-install-preserves-other-skills (let [tmp-root (.mkdtempSync fs (node-path/join (.tmpdir os) "logseq-skill-test-")) source-path (node-path/join tmp-root "source-skill.md") diff --git a/src/test/logseq/cli/main_test.cljs b/src/test/logseq/cli/main_test.cljs index cba3b5a9ca..2b88b2bd0b 100644 --- a/src/test/logseq/cli/main_test.cljs +++ b/src/test/logseq/cli/main_test.cljs @@ -2,6 +2,7 @@ (:require [cljs.reader :as reader] [cljs.test :refer [async deftest is]] [clojure.string :as string] + [logseq.cli.command.skill :as skill-command] [logseq.cli.commands :as commands] [logseq.cli.main :as cli-main] [logseq.cli.test-helper :as test-helper] @@ -10,6 +11,15 @@ ["fs" :as fs] ["path" :as node-path])) +(defn- with-no-installed-skill-warning + [f] + (p/with-redefs [skill-command/installed-skill-update-status + (fn [_] + {:installed? false + :outdated? false + :outdated-targets []})] + (f))) + (deftest test-version-output (async done (-> (p/let [result (cli-main/run! ["--version"] {:exit? false})] @@ -25,10 +35,12 @@ (deftest test-help-output-omits-command-list (async done - (-> (p/let [result (cli-main/run! ["--help"] {:exit? false}) - output (:output result)] - (is (= 0 (:exit-code result))) - (is (not (string/includes? output "Commands: list page")))) + (-> (with-no-installed-skill-warning + (fn [] + (p/let [result (cli-main/run! ["--help"] {:exit? false}) + output (:output result)] + (is (= 0 (:exit-code result))) + (is (not (string/includes? output "Commands: list page")))))) (p/catch (fn [e] (is false (str "unexpected error: " e)) (done))) @@ -36,21 +48,72 @@ (deftest test-help-output-respects-structured-modes (async done - (-> (p/let [json-result (cli-main/run! ["--output" "json" "--help"] {:exit? false}) - edn-result (cli-main/run! ["--output" "edn" "--help"] {:exit? false}) - json-output (js->clj (js/JSON.parse (:output json-result)) :keywordize-keys true) - edn-output (reader/read-string (:output edn-result))] - (is (= 0 (:exit-code json-result))) - (is (= 0 (:exit-code edn-result))) - (is (= "ok" (:status json-output))) - (is (= :ok (:status edn-output))) - (is (string/includes? (get-in json-output [:data :message]) "Usage: logseq")) - (is (string/includes? (get-in edn-output [:data :message]) "Usage: logseq"))) + (-> (with-no-installed-skill-warning + (fn [] + (p/let [json-result (cli-main/run! ["--output" "json" "--help"] {:exit? false}) + edn-result (cli-main/run! ["--output" "edn" "--help"] {:exit? false}) + json-output (js->clj (js/JSON.parse (:output json-result)) :keywordize-keys true) + edn-output (reader/read-string (:output edn-result))] + (is (= 0 (:exit-code json-result))) + (is (= 0 (:exit-code edn-result))) + (is (= "ok" (:status json-output))) + (is (= :ok (:status edn-output))) + (is (string/includes? (get-in json-output [:data :message]) "Usage: logseq")) + (is (string/includes? (get-in edn-output [:data :message]) "Usage: logseq"))))) (p/catch (fn [e] (is false (str "unexpected error: " e)) (done))) (p/finally done)))) +(deftest test-global-help-appends-stale-skill-warning + (async done + (let [warning "\n\nWarning: Installed logseq-cli skill is out of date. Run `logseq skill install` or `logseq skill install --global` to update it." + status {:installed? true + :outdated? true + :outdated-targets [{:scope :local + :path "/tmp/work/.agents/skills/logseq-cli/SKILL.md" + :update-command "logseq skill install"}]}] + (-> (p/with-redefs [skill-command/installed-skill-update-status (fn [_] status)] + (p/let [help-result (cli-main/run! ["--help"] {:exit? false}) + no-args-result (cli-main/run! [] {:exit? false}) + command-help-result (cli-main/run! ["show" "--help"] {:exit? false}) + group-help-result (cli-main/run! ["graph"] {:exit? false}) + version-result (cli-main/run! ["--version"] {:exit? false})] + (is (= 0 (:exit-code help-result))) + (is (string/ends-with? (:output help-result) warning)) + (is (string/ends-with? (:output no-args-result) warning)) + (is (not (string/includes? (:output command-help-result) warning))) + (is (not (string/includes? (:output group-help-result) warning))) + (is (not (string/includes? (:output version-result) warning))))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)) + (done))) + (p/finally done))))) + +(deftest test-global-help-skill-warning-structured-output + (async done + (let [warning "\n\nWarning: Installed logseq-cli skill is out of date. Run `logseq skill install` or `logseq skill install --global` to update it." + status {:installed? true + :outdated? true + :outdated-targets [{:scope :global + :path "/Users/demo/.agents/skills/logseq-cli/SKILL.md" + :update-command "logseq skill install --global"}]}] + (-> (p/with-redefs [skill-command/installed-skill-update-status (fn [_] status)] + (p/let [json-result (cli-main/run! ["--output" "json" "--help"] {:exit? false}) + edn-result (cli-main/run! ["--output" "edn" "--help"] {:exit? false}) + json-output (js->clj (js/JSON.parse (:output json-result)) :keywordize-keys true) + edn-output (reader/read-string (:output edn-result))] + (is (= 0 (:exit-code json-result))) + (is (= 0 (:exit-code edn-result))) + (is (= "ok" (:status json-output))) + (is (= :ok (:status edn-output))) + (is (string/ends-with? (get-in json-output [:data :message]) warning)) + (is (string/ends-with? (get-in edn-output [:data :message]) warning)))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)) + (done))) + (p/finally done))))) + (deftest test-parse-error-output-respects-structured-modes (async done (-> (p/let [json-result (cli-main/run! ["--output" "json" "wat"] {:exit? false}) @@ -105,28 +168,32 @@ (deftest test-run-profile-lines-enabled (async done - (-> (p/let [result (cli-main/run! ["--profile" "--help"] {:exit? false}) - lines (:profile-lines result)] - (is (= 0 (:exit-code result))) - (is (seq lines)) - (when (seq lines) - (is (re-find #"^\d+ms command=help status=ok$" (first lines)))) - (is (some #(= "stages" (string/trim %)) lines)) - (is (some #(string/includes? % "cli.parse-args") lines)) - (is (some #(string/includes? % "cli.total") lines)) - (is (not-any? #(string/includes? % "[profile]") lines)) - (is (not-any? #(string/includes? % "count=") lines)) - (done)) + (-> (with-no-installed-skill-warning + (fn [] + (p/let [result (cli-main/run! ["--profile" "--help"] {:exit? false}) + lines (:profile-lines result)] + (is (= 0 (:exit-code result))) + (is (seq lines)) + (when (seq lines) + (is (re-find #"^\d+ms command=help status=ok$" (first lines)))) + (is (some #(= "stages" (string/trim %)) lines)) + (is (some #(string/includes? % "cli.parse-args") lines)) + (is (some #(string/includes? % "cli.total") lines)) + (is (not-any? #(string/includes? % "[profile]") lines)) + (is (not-any? #(string/includes? % "count=") lines)) + (done)))) (p/catch (fn [e] (is false (str "unexpected error: " e)) (done)))))) (deftest test-run-profile-lines-disabled (async done - (-> (p/let [result (cli-main/run! ["--help"] {:exit? false})] - (is (= 0 (:exit-code result))) - (is (nil? (:profile-lines result))) - (done)) + (-> (with-no-installed-skill-warning + (fn [] + (p/let [result (cli-main/run! ["--help"] {:exit? false})] + (is (= 0 (:exit-code result))) + (is (nil? (:profile-lines result))) + (done)))) (p/catch (fn [e] (is false (str "unexpected error: " e)) (done))))))