From 2c4e1452aa56e7be636ca705711a7c4ba768dbf5 Mon Sep 17 00:00:00 2001 From: sam-shakeybridge-software <3429163+ssjoleary@users.noreply.github.com> Date: Fri, 15 May 2026 18:21:17 +0100 Subject: [PATCH 1/3] chore(paths): centralize XDG-aware path resolution --- bb.edn | 2 ++ src/eca_cli/core.clj | 6 +++- src/eca_cli/paths.clj | 26 +++++++++++++++ src/eca_cli/server.clj | 16 ++++++---- src/eca_cli/sessions.clj | 26 ++++++++------- src/eca_cli/upgrade.clj | 5 +-- test/eca_cli/paths_test.clj | 58 ++++++++++++++++++++++++++++++++++ test/eca_cli/sessions_test.clj | 58 ++++++++++++++++++++++++++-------- 8 files changed, 161 insertions(+), 36 deletions(-) create mode 100644 src/eca_cli/paths.clj create mode 100644 test/eca_cli/paths_test.clj diff --git a/bb.edn b/bb.edn index fe5cede..08ab561 100644 --- a/bb.edn +++ b/bb.edn @@ -18,6 +18,7 @@ (require '[eca-cli.view.blocks-test]) (require '[eca-cli.state-test]) (require '[eca-cli.sessions-test]) + (require '[eca-cli.paths-test]) (require '[eca-cli.upgrade-test]) (let [{:keys [fail error]} (clojure.test/run-tests 'eca-cli.chat-test @@ -28,6 +29,7 @@ 'eca-cli.view.blocks-test 'eca-cli.state-test 'eca-cli.sessions-test + 'eca-cli.paths-test 'eca-cli.upgrade-test)] (System/exit (if (pos? (+ fail error)) 1 0))))} itest {:doc "Run integration tests (requires tmux + ECA on PATH)" diff --git a/src/eca_cli/core.clj b/src/eca_cli/core.clj index 58dc2a9..2f759c5 100644 --- a/src/eca_cli/core.clj +++ b/src/eca_cli/core.clj @@ -2,6 +2,8 @@ (:require [babashka.cli :as cli] [babashka.nrepl.server :as nrepl-server] [charm.program :as program] + [clojure.java.io :as io] + [eca-cli.paths :as paths] [eca-cli.state :as state] [eca-cli.view :as view])) @@ -26,7 +28,9 @@ (let [opts (cli/parse-opts args {:spec cli-spec})] (when-let [port (:nrepl opts)] (println (str "Starting nREPL server on port " port "...")) - (redirect-stdio-to-file! (str (System/getProperty "user.home") "/.cache/eca/eca-cli-nrepl.log")) + (let [log (paths/nrepl-log-file)] + (io/make-parents log) + (redirect-stdio-to-file! (str log))) (nrepl-server/start-server! {:host "localhost" :port port :quiet true})) (.addShutdownHook (Runtime/getRuntime) (Thread. (fn [] diff --git a/src/eca_cli/paths.clj b/src/eca_cli/paths.clj new file mode 100644 index 0000000..aae941f --- /dev/null +++ b/src/eca_cli/paths.clj @@ -0,0 +1,26 @@ +(ns eca-cli.paths + "XDG-aware path resolution for eca-cli (cache, state, config)." + (:require [clojure.java.io :as io])) + +(defn- getenv [k] (System/getenv k)) + +(defn- expand-or [env-var fallback-relative] + (let [v (getenv env-var)] + (if (and v (seq v)) + (io/file v) + (io/file (System/getProperty "user.home") fallback-relative)))) + +(defn cache-home [] (expand-or "XDG_CACHE_HOME" ".cache")) +(defn state-home [] (expand-or "XDG_STATE_HOME" ".local/state")) +(defn config-home [] (expand-or "XDG_CONFIG_HOME" ".config")) + +(defn eca-cache-dir [] (io/file (cache-home) "eca")) +(defn eca-state-dir [] (io/file (state-home) "eca")) + +(defn sessions-file [] (io/file (eca-state-dir) "eca-cli-sessions.edn")) +(defn log-file [] (io/file (eca-state-dir) "eca-cli.log")) +(defn nrepl-log-file [] (io/file (eca-state-dir) "eca-cli-nrepl.log")) +(defn eca-binary [] (io/file (eca-cache-dir) "eca-cli" "eca")) + +(defn legacy-sessions-file [] + (io/file (System/getProperty "user.home") ".cache" "eca" "eca-cli-sessions.edn")) diff --git a/src/eca_cli/server.clj b/src/eca_cli/server.clj index 190894b..0138570 100644 --- a/src/eca_cli/server.clj +++ b/src/eca_cli/server.clj @@ -1,6 +1,8 @@ (ns eca-cli.server (:require [cheshire.core :as json] - [babashka.process :as proc]) + [babashka.process :as proc] + [clojure.java.io :as io] + [eca-cli.paths :as paths]) (:import [java.io BufferedInputStream BufferedWriter OutputStreamWriter] [java.util.concurrent LinkedBlockingQueue TimeUnit])) @@ -8,7 +10,7 @@ "Finds the ECA binary. Preference: eca-cli managed, PATH, editor plugin caches." [] (let [home (System/getProperty "user.home")] - (or (let [p (str home "/.cache/eca/eca-cli/eca")] + (or (let [p (str (paths/eca-binary))] (when (.exists (java.io.File. p)) p)) (some-> (proc/process ["which" "eca"] {:err :string :out :string}) deref :out clojure.string/trim @@ -20,10 +22,10 @@ (str home "/.emacs.d/eca/eca")])))) (defn- default-log-file [] - (let [dir (java.io.File. (str (System/getProperty "user.home") "/.cache/eca"))] - (.mkdirs dir) - (when (.isDirectory dir) - (java.io.File. dir "eca-cli.log")))) + (let [f (paths/log-file)] + (io/make-parents f) + (when (.isDirectory (.getParentFile f)) + f))) (defn spawn! "Spawns the ECA server process. Returns a map with :process, :reader, :writer, :queue." @@ -39,7 +41,7 @@ {}))) log (or log-file (default-log-file)) err (or log - (do (.println System/err "eca-cli: warning: could not create ~/.cache/eca/ — ECA logs will appear in terminal") + (do (.println System/err (str "eca-cli: warning: could not create " (.getParent (paths/log-file)) " — ECA logs will appear in terminal")) :inherit)) p (proc/process [binary "server"] {:err err :shutdown proc/destroy-tree}) diff --git a/src/eca_cli/sessions.clj b/src/eca_cli/sessions.clj index 42a45e8..9632b33 100644 --- a/src/eca_cli/sessions.clj +++ b/src/eca_cli/sessions.clj @@ -2,28 +2,30 @@ (:require [clojure.edn :as edn] [clojure.java.io :as io] [charm.program :as program] + [eca-cli.paths :as paths] [eca-cli.protocol :as protocol])) -(defn sessions-path [] - (str (System/getProperty "user.home") "/.cache/eca/eca-cli-sessions.edn")) +(defn- read-edn-file [^java.io.File f] + (try + (when (.exists f) (edn/read-string (slurp f))) + (catch Exception _ nil))) + +(defn- load-sessions [] + (let [current (io/file (paths/sessions-file))] + (or (read-edn-file current) + (read-edn-file (paths/legacy-sessions-file)) + {}))) (defn load-chat-id "Returns persisted chat-id for workspace, or nil." [workspace] - (try - (let [f (java.io.File. (sessions-path))] - (when (.exists f) - (get (edn/read-string (slurp f)) workspace))) - (catch Exception _ nil))) + (get (load-sessions) workspace)) (defn save-chat-id! "Saves chat-id for workspace. Passing nil removes the entry." [workspace chat-id] - (let [path (sessions-path) - existing (try - (let [f (java.io.File. path)] - (if (.exists f) (edn/read-string (slurp f)) {})) - (catch Exception _ {})) + (let [path (str (paths/sessions-file)) + existing (load-sessions) updated (if chat-id (assoc existing workspace chat-id) (dissoc existing workspace))] diff --git a/src/eca_cli/upgrade.clj b/src/eca_cli/upgrade.clj index 8f9e150..2c88c22 100644 --- a/src/eca_cli/upgrade.clj +++ b/src/eca_cli/upgrade.clj @@ -1,7 +1,8 @@ (ns eca-cli.upgrade (:require [babashka.process :as proc] [clojure.java.io :as io] - [clojure.string :as str])) + [clojure.string :as str] + [eca-cli.paths :as paths])) (def eca-version "0.130.0") @@ -18,7 +19,7 @@ :else (throw (ex-info (str "Unsupported platform: " os-name " " arch) {})))))) (defn dest-path [] - (str (System/getProperty "user.home") "/.cache/eca/eca-cli/eca")) + (str (paths/eca-binary))) (defn check-version "Returns a warning string if binary version doesn't match eca-version, else nil." diff --git a/test/eca_cli/paths_test.clj b/test/eca_cli/paths_test.clj new file mode 100644 index 0000000..da99307 --- /dev/null +++ b/test/eca_cli/paths_test.clj @@ -0,0 +1,58 @@ +(ns eca-cli.paths-test + (:require [clojure.test :refer [deftest is testing]] + [eca-cli.paths :as paths])) + +(defn- stub-env [m] + (fn [k] (get m k))) + +(deftest cache-home-with-env-test + (testing "honours XDG_CACHE_HOME when set" + (with-redefs [paths/getenv (stub-env {"XDG_CACHE_HOME" "/tmp/c"})] + (is (= "/tmp/c" (str (paths/cache-home)))))) + (testing "falls back to ~/.cache when unset" + (with-redefs [paths/getenv (stub-env {})] + (is (= (str (System/getProperty "user.home") "/.cache") + (str (paths/cache-home)))))) + (testing "empty string is treated as unset" + (with-redefs [paths/getenv (stub-env {"XDG_CACHE_HOME" ""})] + (is (= (str (System/getProperty "user.home") "/.cache") + (str (paths/cache-home))))))) + +(deftest state-home-with-env-test + (testing "honours XDG_STATE_HOME when set" + (with-redefs [paths/getenv (stub-env {"XDG_STATE_HOME" "/tmp/s"})] + (is (= "/tmp/s" (str (paths/state-home)))))) + (testing "falls back to ~/.local/state when unset" + (with-redefs [paths/getenv (stub-env {})] + (is (= (str (System/getProperty "user.home") "/.local/state") + (str (paths/state-home))))))) + +(deftest config-home-with-env-test + (testing "honours XDG_CONFIG_HOME when set" + (with-redefs [paths/getenv (stub-env {"XDG_CONFIG_HOME" "/tmp/cfg"})] + (is (= "/tmp/cfg" (str (paths/config-home)))))) + (testing "falls back to ~/.config when unset" + (with-redefs [paths/getenv (stub-env {})] + (is (= (str (System/getProperty "user.home") "/.config") + (str (paths/config-home))))))) + +(deftest eca-cache-dir-composition-test + (with-redefs [paths/getenv (stub-env {"XDG_CACHE_HOME" "/tmp/c"})] + (is (= "/tmp/c/eca" (str (paths/eca-cache-dir)))))) + +(deftest eca-state-dir-composition-test + (with-redefs [paths/getenv (stub-env {"XDG_STATE_HOME" "/tmp/s"})] + (is (= "/tmp/s/eca" (str (paths/eca-state-dir)))))) + +(deftest sessions-file-under-state-test + (with-redefs [paths/getenv (stub-env {"XDG_STATE_HOME" "/tmp/s"})] + (is (= "/tmp/s/eca/eca-cli-sessions.edn" (str (paths/sessions-file)))))) + +(deftest log-file-under-state-test + (with-redefs [paths/getenv (stub-env {"XDG_STATE_HOME" "/tmp/s"})] + (is (= "/tmp/s/eca/eca-cli.log" (str (paths/log-file)))) + (is (= "/tmp/s/eca/eca-cli-nrepl.log" (str (paths/nrepl-log-file)))))) + +(deftest eca-binary-under-cache-test + (with-redefs [paths/getenv (stub-env {"XDG_CACHE_HOME" "/tmp/c"})] + (is (= "/tmp/c/eca/eca-cli/eca" (str (paths/eca-binary)))))) diff --git a/test/eca_cli/sessions_test.clj b/test/eca_cli/sessions_test.clj index a8968e0..bb30f78 100644 --- a/test/eca_cli/sessions_test.clj +++ b/test/eca_cli/sessions_test.clj @@ -1,41 +1,71 @@ (ns eca-cli.sessions-test (:require [clojure.test :refer [deftest is testing]] + [eca-cli.paths :as paths] [eca-cli.sessions :as sessions])) +(defn- tmp-file + ([] (tmp-file true)) + ([create?] + (let [f (java.io.File/createTempFile "sessions" ".edn")] + (.deleteOnExit f) + (when-not create? (.delete f)) + f))) + +(defn- absent-file [] + (let [f (java.io.File/createTempFile "absent" ".edn")] + (.deleteOnExit f) + (.delete f) + f)) + (deftest load-chat-id-missing-file-test - (testing "returns nil when file does not exist" - (with-redefs [sessions/sessions-path (fn [] "/tmp/no-such-eca-sessions-file.edn")] + (testing "returns nil when neither current nor legacy file exists" + (with-redefs [paths/sessions-file (constantly (absent-file)) + paths/legacy-sessions-file (constantly (absent-file))] (is (nil? (sessions/load-chat-id "/some/workspace")))))) (deftest load-chat-id-found-test (testing "returns stored chat-id for workspace" - (let [f (java.io.File/createTempFile "sessions" ".edn")] - (.deleteOnExit f) + (let [f (tmp-file)] (spit f (pr-str {"/workspace" "chat-abc"})) - (with-redefs [sessions/sessions-path (fn [] (.getAbsolutePath f))] + (with-redefs [paths/sessions-file (constantly f) + paths/legacy-sessions-file (constantly (absent-file))] (is (= "chat-abc" (sessions/load-chat-id "/workspace"))))))) (deftest load-chat-id-missing-workspace-test (testing "returns nil when workspace not in file" - (let [f (java.io.File/createTempFile "sessions" ".edn")] - (.deleteOnExit f) + (let [f (tmp-file)] (spit f (pr-str {"/other" "chat-xyz"})) - (with-redefs [sessions/sessions-path (fn [] (.getAbsolutePath f))] + (with-redefs [paths/sessions-file (constantly f) + paths/legacy-sessions-file (constantly (absent-file))] (is (nil? (sessions/load-chat-id "/workspace"))))))) (deftest save-and-load-round-trip-test (testing "save then load returns same value" - (let [f (java.io.File/createTempFile "sessions" ".edn")] - (.deleteOnExit f) - (with-redefs [sessions/sessions-path (fn [] (.getAbsolutePath f))] + (let [f (tmp-file)] + (with-redefs [paths/sessions-file (constantly f) + paths/legacy-sessions-file (constantly (absent-file))] (sessions/save-chat-id! "/workspace" "chat-round-trip") (is (= "chat-round-trip" (sessions/load-chat-id "/workspace"))))))) (deftest save-nil-removes-entry-test (testing "save nil removes the workspace entry" - (let [f (java.io.File/createTempFile "sessions" ".edn")] - (.deleteOnExit f) - (with-redefs [sessions/sessions-path (fn [] (.getAbsolutePath f))] + (let [f (tmp-file)] + (with-redefs [paths/sessions-file (constantly f) + paths/legacy-sessions-file (constantly (absent-file))] (sessions/save-chat-id! "/workspace" "chat-to-remove") (sessions/save-chat-id! "/workspace" nil) (is (nil? (sessions/load-chat-id "/workspace"))))))) + +(deftest migration-read-old-path-test + (testing "load falls back to legacy path; next save writes to new path" + (let [legacy (tmp-file) + newf (absent-file)] + (spit legacy (pr-str {"/ws" "chat-legacy"})) + (with-redefs [paths/sessions-file (constantly newf) + paths/legacy-sessions-file (constantly legacy)] + (is (= "chat-legacy" (sessions/load-chat-id "/ws"))) + (sessions/save-chat-id! "/ws" "chat-new") + (is (.exists newf)) + (is (= {"/ws" "chat-new"} (clojure.edn/read-string (slurp newf)))) + (testing "legacy file is left untouched" + (is (= {"/ws" "chat-legacy"} (clojure.edn/read-string (slurp legacy))))))))) From 38a9edbd971cc322cbec833d9b1fe5c4e3de8171 Mon Sep 17 00:00:00 2001 From: sam-shakeybridge-software <3429163+ssjoleary@users.noreply.github.com> Date: Fri, 15 May 2026 19:38:30 +0100 Subject: [PATCH 2/3] fix(paths): make getenv public so tests can with-redefs it Addresses PR #1 review: paths/getenv was private (defn-) but tests referenced it directly in with-redefs, which fails at compile time on JVM Clojure with "var is not public". Making the seam explicitly public matches the intent of an injectable env lookup. SCI/Babashka was permissive about this, masking the latent issue. --- src/eca_cli/paths.clj | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/eca_cli/paths.clj b/src/eca_cli/paths.clj index aae941f..35fc191 100644 --- a/src/eca_cli/paths.clj +++ b/src/eca_cli/paths.clj @@ -2,7 +2,10 @@ "XDG-aware path resolution for eca-cli (cache, state, config)." (:require [clojure.java.io :as io])) -(defn- getenv [k] (System/getenv k)) +(defn getenv + "Indirection over `System/getenv` so tests can stub via `with-redefs`." + [k] + (System/getenv k)) (defn- expand-or [env-var fallback-relative] (let [v (getenv env-var)] From 22a66546643f95d9dc4c73eac87d18d22a898a5b Mon Sep 17 00:00:00 2001 From: sam-shakeybridge-software <3429163+ssjoleary@users.noreply.github.com> Date: Fri, 15 May 2026 20:33:34 +0100 Subject: [PATCH 3/3] =?UTF-8?q?refactor(paths):=20apply=20clj-refactor=20p?= =?UTF-8?q?ass=20=E2=80=94=20mechanism/policy=20tightening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove redundant File<->String roundtrips and name the log-file candidate once so the warning message describes the actual path attempted. Extracts ensure-writable as a context-free mechanism (parent-dir creation + isDirectory check), separated from the default-log-file policy. --- src/eca_cli/server.clj | 50 +++++++++++++++++++++------------------- src/eca_cli/sessions.clj | 13 +++++------ 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/eca_cli/server.clj b/src/eca_cli/server.clj index 0138570..509014c 100644 --- a/src/eca_cli/server.clj +++ b/src/eca_cli/server.clj @@ -10,8 +10,8 @@ "Finds the ECA binary. Preference: eca-cli managed, PATH, editor plugin caches." [] (let [home (System/getProperty "user.home")] - (or (let [p (str (paths/eca-binary))] - (when (.exists (java.io.File. p)) p)) + (or (let [f (paths/eca-binary)] + (when (.exists f) (str f))) (some-> (proc/process ["which" "eca"] {:err :string :out :string}) deref :out clojure.string/trim not-empty) @@ -21,33 +21,35 @@ (str home "/Library/Caches/nvim/eca/eca") (str home "/.emacs.d/eca/eca")])))) -(defn- default-log-file [] - (let [f (paths/log-file)] - (io/make-parents f) - (when (.isDirectory (.getParentFile f)) - f))) +(defn- ensure-writable + "Ensures parent dir exists and returns the file if its parent is a directory, else nil." + [^java.io.File f] + (io/make-parents f) + (when (.isDirectory (.getParentFile f)) + f)) (defn spawn! "Spawns the ECA server process. Returns a map with :process, :reader, :writer, :queue." ([] (spawn! {})) ([{:keys [path log-file]}] - (let [binary (or path (find-eca-binary)) - _ (when-not binary - (throw (ex-info - (str "ECA binary not found.\n" - "Install via a supported editor plugin or download from:\n" - " https://github.com/editor-code-assistant/eca/releases\n" - "Or specify path with --eca ") - {}))) - log (or log-file (default-log-file)) - err (or log - (do (.println System/err (str "eca-cli: warning: could not create " (.getParent (paths/log-file)) " — ECA logs will appear in terminal")) - :inherit)) - p (proc/process [binary "server"] - {:err err :shutdown proc/destroy-tree}) - reader (BufferedInputStream. (:out p)) - writer (BufferedWriter. (OutputStreamWriter. (:in p) "UTF-8")) - queue (LinkedBlockingQueue.)] + (let [binary (or path (find-eca-binary)) + _ (when-not binary + (throw (ex-info + (str "ECA binary not found.\n" + "Install via a supported editor plugin or download from:\n" + " https://github.com/editor-code-assistant/eca/releases\n" + "Or specify path with --eca ") + {}))) + candidate (or log-file (paths/log-file)) + log (ensure-writable candidate) + err (or log + (do (.println System/err (str "eca-cli: warning: could not create " (.getParent candidate) " — ECA logs will appear in terminal")) + :inherit)) + p (proc/process [binary "server"] + {:err err :shutdown proc/destroy-tree}) + reader (BufferedInputStream. (:out p)) + writer (BufferedWriter. (OutputStreamWriter. (:in p) "UTF-8")) + queue (LinkedBlockingQueue.)] {:process p :reader reader :writer writer diff --git a/src/eca_cli/sessions.clj b/src/eca_cli/sessions.clj index 9632b33..7d461c2 100644 --- a/src/eca_cli/sessions.clj +++ b/src/eca_cli/sessions.clj @@ -11,10 +11,9 @@ (catch Exception _ nil))) (defn- load-sessions [] - (let [current (io/file (paths/sessions-file))] - (or (read-edn-file current) - (read-edn-file (paths/legacy-sessions-file)) - {}))) + (or (read-edn-file (paths/sessions-file)) + (read-edn-file (paths/legacy-sessions-file)) + {})) (defn load-chat-id "Returns persisted chat-id for workspace, or nil." @@ -24,13 +23,13 @@ (defn save-chat-id! "Saves chat-id for workspace. Passing nil removes the entry." [workspace chat-id] - (let [path (str (paths/sessions-file)) + (let [f (paths/sessions-file) existing (load-sessions) updated (if chat-id (assoc existing workspace chat-id) (dissoc existing workspace))] - (io/make-parents path) - (spit path (pr-str updated)))) + (io/make-parents f) + (spit f (pr-str updated)))) ;; --- charm/program cmd builders for chat-list/open/delete ---